From df1ff4ec977b76587dfc50885874f08fd9748071 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 3 Aug 2017 18:25:47 -0500 Subject: PLT-7212: fix missing webhook post attachments (#7011) * fix missing webhook post attachments * make ProcessSlackAttachments return a new slice instead of modifying it --- app/webhook_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++ model/command_response.go | 14 ++++++------- model/incoming_webhook.go | 16 +++++++------- model/slack_attachment.go | 8 +++---- model/slack_attachment_test.go | 6 +++--- 5 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 app/webhook_test.go diff --git a/app/webhook_test.go b/app/webhook_test.go new file mode 100644 index 000000000..ae97454a6 --- /dev/null +++ b/app/webhook_test.go @@ -0,0 +1,47 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "testing" + + "github.com/mattermost/platform/model" + "github.com/mattermost/platform/utils" +) + +func TestCreateWebhookPost(t *testing.T) { + th := Setup().InitBasic() + defer TearDown() + + enableIncomingHooks := utils.Cfg.ServiceSettings.EnableIncomingWebhooks + defer func() { + utils.Cfg.ServiceSettings.EnableIncomingWebhooks = enableIncomingHooks + utils.SetDefaultRolesBasedOnConfig() + }() + utils.Cfg.ServiceSettings.EnableIncomingWebhooks = true + utils.SetDefaultRolesBasedOnConfig() + + hook, err := CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id}) + if err != nil { + t.Fatal(err.Error()) + } + defer DeleteIncomingWebhook(hook.Id) + + post, err := CreateWebhookPost(hook.UserId, hook.TeamId, th.BasicChannel.Id, "foo", "user", "http://iconurl", model.StringInterface{ + "attachments": []*model.SlackAttachment{ + &model.SlackAttachment{ + Text: "text", + }, + }, + }, model.POST_SLACK_ATTACHMENT) + if err != nil { + t.Fatal(err.Error()) + } + + for _, k := range []string{"from_webhook", "attachments"} { + if _, ok := post.Props[k]; !ok { + t.Fatal(k) + } + } +} diff --git a/model/command_response.go b/model/command_response.go index e3253acc0..27d39e173 100644 --- a/model/command_response.go +++ b/model/command_response.go @@ -14,12 +14,12 @@ const ( ) type CommandResponse struct { - ResponseType string `json:"response_type"` - Text string `json:"text"` - Username string `json:"username"` - IconURL string `json:"icon_url"` - GotoLocation string `json:"goto_location"` - Attachments SlackAttachments `json:"attachments"` + ResponseType string `json:"response_type"` + Text string `json:"text"` + Username string `json:"username"` + IconURL string `json:"icon_url"` + GotoLocation string `json:"goto_location"` + Attachments []*SlackAttachment `json:"attachments"` } func (o *CommandResponse) ToJson() string { @@ -40,7 +40,7 @@ func CommandResponseFromJson(data io.Reader) *CommandResponse { } o.Text = ExpandAnnouncement(o.Text) - o.Attachments.Process() + o.Attachments = ProcessSlackAttachments(o.Attachments) return &o } diff --git a/model/incoming_webhook.go b/model/incoming_webhook.go index de58093ff..ce755f889 100644 --- a/model/incoming_webhook.go +++ b/model/incoming_webhook.go @@ -28,13 +28,13 @@ type IncomingWebhook struct { } type IncomingWebhookRequest struct { - Text string `json:"text"` - Username string `json:"username"` - IconURL string `json:"icon_url"` - ChannelName string `json:"channel"` - Props StringInterface `json:"props"` - Attachments SlackAttachments `json:"attachments"` - Type string `json:"type"` + Text string `json:"text"` + Username string `json:"username"` + IconURL string `json:"icon_url"` + ChannelName string `json:"channel"` + Props StringInterface `json:"props"` + Attachments []*SlackAttachment `json:"attachments"` + Type string `json:"type"` } func (o *IncomingWebhook) ToJson() string { @@ -209,7 +209,7 @@ func IncomingWebhookRequestFromJson(data io.Reader) *IncomingWebhookRequest { } o.Text = ExpandAnnouncement(o.Text) - o.Attachments.Process() + o.Attachments = ProcessSlackAttachments(o.Attachments) return o } diff --git a/model/slack_attachment.go b/model/slack_attachment.go index a3199c44c..855838214 100644 --- a/model/slack_attachment.go +++ b/model/slack_attachment.go @@ -33,8 +33,6 @@ type SlackAttachmentField struct { Short bool `json:"short"` } -type SlackAttachments []*SlackAttachment - // To mention @channel via a webhook in Slack, the message should contain // , as explained at the bottom of this article: // https://get.slack.help/hc/en-us/articles/202009646-Making-announcements @@ -51,9 +49,9 @@ func ExpandAnnouncement(text string) string { // can be found in the text attribute, or in the pretext, text, title and value // attributes of the attachment structure. The Slack attachment structure is // documented here: https://api.slack.com/docs/attachments -func (a *SlackAttachments) Process() { +func ProcessSlackAttachments(a []*SlackAttachment) []*SlackAttachment { var nonNilAttachments []*SlackAttachment - for _, attachment := range *a { + for _, attachment := range a { if attachment == nil { continue } @@ -77,5 +75,5 @@ func (a *SlackAttachments) Process() { } attachment.Fields = nonNilFields } - *a = nonNilAttachments + return nonNilAttachments } diff --git a/model/slack_attachment_test.go b/model/slack_attachment_test.go index ff4df888f..2ba32baf2 100644 --- a/model/slack_attachment_test.go +++ b/model/slack_attachment_test.go @@ -10,8 +10,8 @@ func TestExpandAnnouncement(t *testing.T) { } } -func TestSlackAnnouncementProcess(t *testing.T) { - attachments := SlackAttachments{ +func TestProcessSlackAnnouncement(t *testing.T) { + attachments := []*SlackAttachment{ { Pretext: " pretext", Text: " text", @@ -25,7 +25,7 @@ func TestSlackAnnouncementProcess(t *testing.T) { }, }, nil, } - attachments.Process() + attachments = ProcessSlackAttachments(attachments) if len(attachments) != 1 || len(attachments[0].Fields) != 1 { t.Fail() } -- cgit v1.2.3-1-g7c22