[PLT-6676] Make OutgoingWebhook to fire when post has no text content but only attachment (#6935)

* make OutgoingWebhook to fire when post has no text content but only attachment

* update per comment and modify payload & test

api/post_test.go

@@ -177,8 +177,9 @@ func TestCreatePostWithCreateAt(t *testing.T) {
 
 func testCreatePostWithOutgoingHook(
 	t *testing.T,
-	hookContentType string,
+	hookContentType, expectedContentType, message, triggerWord string,
-	expectedContentType string,
+	fileIds []string,
+	triggerWhen int,
 ) {
 	th := Setup().InitSystemAdmin()
 	Client := th.SystemAdminClient
@@ -221,7 +222,8 @@ func testCreatePostWithOutgoingHook(
 			UserName:    user.Username,
 			PostId:      post.Id,
 			Text:        post.Message,
-			TriggerWord: strings.Fields(post.Message)[0],
+			TriggerWord: triggerWord,
+			FileIds:     strings.Join(post.FileIds, ","),
 		}
 
 		// depending on the Content-Type, we expect to find a JSON or form encoded payload
@@ -256,11 +258,17 @@ func testCreatePostWithOutgoingHook(
 	defer ts.Close()
 
 	// create an outgoing webhook, passing it the test server URL
-	triggerWord := "bingo"
+	var triggerWords []string
+	if triggerWord != "" {
+		triggerWords = []string{triggerWord}
+	}
+
 	hook = &model.OutgoingWebhook{
 		ChannelId:    channel.Id,
+		TeamId:       team.Id,
 		ContentType:  hookContentType,
-		TriggerWords: []string{triggerWord},
+		TriggerWords: triggerWords,
+		TriggerWhen:  triggerWhen,
 		CallbackURLs: []string{ts.URL},
 	}
 
@@ -271,10 +279,10 @@ func testCreatePostWithOutgoingHook(
 	}
 
 	// create a post to trigger the webhook
-	message := triggerWord + " lorem ipusm"
 	post = &model.Post{
 		ChannelId: channel.Id,
 		Message:   message,
+		FileIds:   fileIds,
 	}
 
 	if result, err := Client.CreatePost(post); err != nil {
@@ -290,25 +298,34 @@ func testCreatePostWithOutgoingHook(
 	select {
 	case ok := <-success:
 		if !ok {
-			t.Fatal("Test server was sent an invalid webhook.")
+			t.Fatal("Test server did send an invalid webhook.")
 		}
 	case <-time.After(time.Second):
-		t.Fatal("Timeout, test server wasn't sent the webhook.")
+		t.Fatal("Timeout, test server did not send the webhook.")
 	}
 }
 
 func TestCreatePostWithOutgoingHook_form_urlencoded(t *testing.T) {
-	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded")
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "triggerword lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "triggerwordaaazzz lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "", "", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "", "", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
 }
 
 func TestCreatePostWithOutgoingHook_json(t *testing.T) {
-	testCreatePostWithOutgoingHook(t, "application/json", "application/json")
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerword lorem ipsum", "triggerword", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerwordaaazzz lorem ipsum", "triggerword", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_STARTS_WITH)
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerword lorem ipsum", "", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerwordaaazzz lorem ipsum", "", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
 }
 
 // hooks created before we added the ContentType field should be considered as
 // application/x-www-form-urlencoded
 func TestCreatePostWithOutgoingHook_no_content_type(t *testing.T) {
-	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded")
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerword lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerwordaaazzz lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerword lorem ipsum", "", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerwordaaazzz lorem ipsum", "", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_STARTS_WITH)
 }
 
 func TestUpdatePost(t *testing.T) {

api4/post_test.go

@@ -4,9 +4,13 @@
 package api4
 
 import (
+	"encoding/json"
 	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"reflect"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
@@ -101,6 +105,158 @@ func TestCreatePost(t *testing.T) {
 	}
 }
 
+func testCreatePostWithOutgoingHook(
+	t *testing.T,
+	hookContentType, expectedContentType, message, triggerWord string,
+	fileIds []string,
+	triggerWhen int,
+) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer TearDown()
+	user := th.SystemAdminUser
+	team := th.BasicTeam
+	channel := th.BasicChannel
+
+	enableOutgoingHooks := utils.Cfg.ServiceSettings.EnableOutgoingWebhooks
+	enableAdminOnlyHooks := utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations
+	defer func() {
+		utils.Cfg.ServiceSettings.EnableOutgoingWebhooks = enableOutgoingHooks
+		utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = enableAdminOnlyHooks
+		utils.SetDefaultRolesBasedOnConfig()
+	}()
+	utils.Cfg.ServiceSettings.EnableOutgoingWebhooks = true
+	*utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = true
+	utils.SetDefaultRolesBasedOnConfig()
+
+	var hook *model.OutgoingWebhook
+	var post *model.Post
+
+	// Create a test server that is the target of the outgoing webhook. It will
+	// validate the webhook body fields and write to the success channel on
+	// success/failure.
+	success := make(chan bool)
+	wait := make(chan bool, 1)
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		<-wait
+
+		requestContentType := r.Header.Get("Content-Type")
+		if requestContentType != expectedContentType {
+			t.Logf("Content-Type is %s, should be %s", requestContentType, expectedContentType)
+			success <- false
+			return
+		}
+
+		expectedPayload := &model.OutgoingWebhookPayload{
+			Token:       hook.Token,
+			TeamId:      hook.TeamId,
+			TeamDomain:  team.Name,
+			ChannelId:   post.ChannelId,
+			ChannelName: channel.Name,
+			Timestamp:   post.CreateAt,
+			UserId:      post.UserId,
+			UserName:    user.Username,
+			PostId:      post.Id,
+			Text:        post.Message,
+			TriggerWord: triggerWord,
+			FileIds:     strings.Join(post.FileIds, ","),
+		}
+
+		// depending on the Content-Type, we expect to find a JSON or form encoded payload
+		if requestContentType == "application/json" {
+			decoder := json.NewDecoder(r.Body)
+			o := &model.OutgoingWebhookPayload{}
+			decoder.Decode(&o)
+
+			if !reflect.DeepEqual(expectedPayload, o) {
+				t.Logf("JSON payload is %+v, should be %+v", o, expectedPayload)
+				success <- false
+				return
+			}
+		} else {
+			err := r.ParseForm()
+			if err != nil {
+				t.Logf("Error parsing form: %q", err)
+				success <- false
+				return
+			}
+
+			expectedFormValues, _ := url.ParseQuery(expectedPayload.ToFormValues())
+			if !reflect.DeepEqual(expectedFormValues, r.Form) {
+				t.Logf("Form values are: %q\n, should be: %q\n", r.Form, expectedFormValues)
+				success <- false
+				return
+			}
+		}
+
+		success <- true
+	}))
+	defer ts.Close()
+
+	// create an outgoing webhook, passing it the test server URL
+	var triggerWords []string
+	if triggerWord != "" {
+		triggerWords = []string{triggerWord}
+	}
+	
+	hook = &model.OutgoingWebhook{
+		ChannelId:    channel.Id,
+		TeamId:       team.Id,
+		ContentType:  hookContentType,
+		TriggerWords: triggerWords,
+		TriggerWhen:  triggerWhen,
+		CallbackURLs: []string{ts.URL},
+	}
+
+	hook, resp := th.SystemAdminClient.CreateOutgoingWebhook(hook)
+	CheckNoError(t, resp)
+
+	// create a post to trigger the webhook
+	post = &model.Post{
+		ChannelId: channel.Id,
+		Message:   message,
+		FileIds:   fileIds,
+	}
+
+	post, resp = th.SystemAdminClient.CreatePost(post)
+	CheckNoError(t, resp)
+
+	wait <- true
+
+	// We wait for the test server to write to the success channel and we make
+	// the test fail if that doesn't happen before the timeout.
+	select {
+	case ok := <-success:
+		if !ok {
+			t.Fatal("Test server did send an invalid webhook.")
+		}
+	case <-time.After(time.Second):
+		t.Fatal("Timeout, test server did not send the webhook.")
+	}
+}
+
+func TestCreatePostWithOutgoingHook_form_urlencoded(t *testing.T) {
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "triggerword lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "triggerwordaaazzz lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "", "", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/x-www-form-urlencoded", "application/x-www-form-urlencoded", "", "", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
+}
+
+func TestCreatePostWithOutgoingHook_json(t *testing.T) {
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerword lorem ipsum", "triggerword", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerwordaaazzz lorem ipsum", "triggerword", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_STARTS_WITH)
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerword lorem ipsum", "", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "application/json", "application/json", "triggerwordaaazzz lorem ipsum", "", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
+}
+
+// hooks created before we added the ContentType field should be considered as
+// application/x-www-form-urlencoded
+func TestCreatePostWithOutgoingHook_no_content_type(t *testing.T) {
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerword lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerwordaaazzz lorem ipsum", "triggerword", []string{"file_id_1"}, app.TRIGGERWORDS_STARTS_WITH)
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerword lorem ipsum", "", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_EXACT_MATCH)
+	testCreatePostWithOutgoingHook(t, "", "application/x-www-form-urlencoded", "triggerwordaaazzz lorem ipsum", "", []string{"file_id_1, file_id_2"}, app.TRIGGERWORDS_STARTS_WITH)
+}
+
 func TestUpdatePost(t *testing.T) {
 	th := Setup().InitBasic().InitSystemAdmin()
 	defer TearDown()

app/webhook.go

@@ -18,8 +18,8 @@ import (
 )
 
 const (
-	TRIGGERWORDS_FULL       = 0
+	TRIGGERWORDS_EXACT_MATCH = 0
-	TRIGGERWORDS_STARTSWITH = 1
+	TRIGGERWORDS_STARTS_WITH = 1
 )
 
 func handleWebhookEvents(post *model.Post, team *model.Team, channel *model.Channel, user *model.User) *model.AppError {
@@ -42,27 +42,30 @@ func handleWebhookEvents(post *model.Post, team *model.Team, channel *model.Chan
 		return nil
 	}
 
+	var firstWord, triggerWord string
+
 	splitWords := strings.Fields(post.Message)
-	if len(splitWords) == 0 {
+	if len(splitWords) > 0 {
-		return nil
+		firstWord = splitWords[0]
 	}
-	firstWord := splitWords[0]
 
 	relevantHooks := []*model.OutgoingWebhook{}
 	for _, hook := range hooks {
 		if hook.ChannelId == post.ChannelId || len(hook.ChannelId) == 0 {
 			if hook.ChannelId == post.ChannelId && len(hook.TriggerWords) == 0 {
 				relevantHooks = append(relevantHooks, hook)
-			} else if hook.TriggerWhen == TRIGGERWORDS_FULL && hook.HasTriggerWord(firstWord) {
+				triggerWord = ""
+			} else if hook.TriggerWhen == TRIGGERWORDS_EXACT_MATCH && hook.TriggerWordExactMatch(firstWord) {
 				relevantHooks = append(relevantHooks, hook)
-			} else if hook.TriggerWhen == TRIGGERWORDS_STARTSWITH && hook.TriggerWordStartsWith(firstWord) {
+				triggerWord = hook.GetTriggerWord(firstWord, true)
+			} else if hook.TriggerWhen == TRIGGERWORDS_STARTS_WITH && hook.TriggerWordStartsWith(firstWord) {
 				relevantHooks = append(relevantHooks, hook)
+				triggerWord = hook.GetTriggerWord(firstWord, false)
 			}
 		}
 	}
 
 	for _, hook := range relevantHooks {
-		go func(hook *model.OutgoingWebhook) {
 		payload := &model.OutgoingWebhookPayload{
 			Token:       hook.Token,
 			TeamId:      hook.TeamId,
@@ -74,8 +77,16 @@ func handleWebhookEvents(post *model.Post, team *model.Team, channel *model.Chan
 			UserName:    user.Username,
 			PostId:      post.Id,
 			Text:        post.Message,
-				TriggerWord: firstWord,
+			TriggerWord: triggerWord,
+			FileIds:     strings.Join(post.FileIds, ","),
 		}
+		go TriggerWebhook(payload, hook, post)
+	}
+
+	return nil
+}
+
+func TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.OutgoingWebhook, post *model.Post) {
 	var body io.Reader
 	var contentType string
 	if hook.ContentType == "application/json" {
@@ -105,11 +116,6 @@ func handleWebhookEvents(post *model.Post, team *model.Team, channel *model.Chan
 			}
 		}(url)
 	}
-
-		}(hook)
-	}
-
-	return nil
 }
 
 func CreateWebhookPost(userId, teamId, channelId, text, overrideUsername, overrideIconUrl string, props model.StringInterface, postType string) (*model.Post, *model.AppError) {

model/outgoing_webhook.go

@@ -42,6 +42,7 @@ type OutgoingWebhookPayload struct {
 	PostId      string `json:"post_id"`
 	Text        string `json:"text"`
 	TriggerWord string `json:"trigger_word"`
+	FileIds     string `json:"file_ids"`
 }
 
 func (o *OutgoingWebhookPayload) ToJSON() string {
@@ -66,6 +67,7 @@ func (o *OutgoingWebhookPayload) ToFormValues() string {
 	v.Set("post_id", o.PostId)
 	v.Set("text", o.Text)
 	v.Set("trigger_word", o.TriggerWord)
+	v.Set("file_ids", o.FileIds)
 
 	return v.Encode()
 }
@@ -198,8 +200,8 @@ func (o *OutgoingWebhook) PreUpdate() {
 	o.UpdateAt = GetMillis()
 }
 
-func (o *OutgoingWebhook) HasTriggerWord(word string) bool {
+func (o *OutgoingWebhook) TriggerWordExactMatch(word string) bool {
-	if len(o.TriggerWords) == 0 || len(word) == 0 {
+	if len(word) == 0 {
 		return false
 	}
 
@@ -213,7 +215,7 @@ func (o *OutgoingWebhook) HasTriggerWord(word string) bool {
 }
 
 func (o *OutgoingWebhook) TriggerWordStartsWith(word string) bool {
-	if len(o.TriggerWords) == 0 || len(word) == 0 {
+	if len(word) == 0 {
 		return false
 	}
 
@@ -225,3 +227,27 @@ func (o *OutgoingWebhook) TriggerWordStartsWith(word string) bool {
 
 	return false
 }
+
+func (o *OutgoingWebhook) GetTriggerWord(word string, isExactMatch bool) (triggerWord string) {
+	if len(word) == 0 {
+		return
+	}
+
+	if isExactMatch {
+		for _, trigger := range o.TriggerWords {
+			if trigger == word {
+				triggerWord = trigger
+				break
+			}
+		}
+	} else {
+		for _, trigger := range o.TriggerWords {
+			if strings.HasPrefix(word, trigger) {
+				triggerWord = trigger
+				break
+			}
+		}
+	}
+
+	return triggerWord
+}

model/outgoing_webhook_test.go

@@ -136,6 +136,7 @@ func TestOutgoingWebhookPayloadToFormValues(t *testing.T) {
 		PostId:      "PostId",
 		Text:        "Text",
 		TriggerWord: "TriggerWord",
+		FileIds:     "FileIds",
 	}
 	v := url.Values{}
 	v.Set("token", "Token")
@@ -149,6 +150,7 @@ func TestOutgoingWebhookPayloadToFormValues(t *testing.T) {
 	v.Set("post_id", "PostId")
 	v.Set("text", "Text")
 	v.Set("trigger_word", "TriggerWord")
+	v.Set("file_ids", "FileIds")
 	if got, want := p.ToFormValues(), v.Encode(); !reflect.DeepEqual(got, want) {
 		t.Fatalf("Got %+v, wanted %+v", got, want)
 	}