From 072d9a9c19d4cbc84f0ffe3d70152338131d963e Mon Sep 17 00:00:00 2001 From: Colton Shaw <46071821+coltoneshaw@users.noreply.github.com> Date: Wed, 17 Apr 2024 07:32:32 -0400 Subject: [PATCH] Add post priority to incoming and outgoing webhook (#26671) * Added priority to incoming and outgoing webhook * fixed fat finger * Added priority tests * fixed golint --------- Co-authored-by: Mattermost Build --- server/channels/app/app_iface.go | 2 +- .../app/opentracing/opentracing_layer.go | 4 +- server/channels/app/webhook.go | 16 +++- server/channels/app/webhook_test.go | 73 ++++++++++++++++--- server/public/model/incoming_webhook.go | 1 + server/public/model/outgoing_webhook.go | 1 + 6 files changed, 79 insertions(+), 18 deletions(-) diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index 45352fdff7..8443a67554 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -547,7 +547,7 @@ type AppIface interface { CreateUserFromSignup(c request.CTX, user *model.User, redirect string) (*model.User, *model.AppError) CreateUserWithInviteId(c request.CTX, user *model.User, inviteId, redirect string) (*model.User, *model.AppError) CreateUserWithToken(c request.CTX, user *model.User, token *model.Token) (*model.User, *model.AppError) - CreateWebhookPost(c request.CTX, userID string, channel *model.Channel, text, overrideUsername, overrideIconURL, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string) (*model.Post, *model.AppError) + CreateWebhookPost(c request.CTX, userID string, channel *model.Channel, text, overrideUsername, overrideIconURL, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string, priority *model.PostPriority) (*model.Post, *model.AppError) DBHealthCheckDelete() error DBHealthCheckWrite() error DataRetention() einterfaces.DataRetentionInterface diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index f7dab9b31f..d6c788c88e 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -2851,7 +2851,7 @@ func (a *OpenTracingAppLayer) CreateUserWithToken(c request.CTX, user *model.Use return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) CreateWebhookPost(c request.CTX, userID string, channel *model.Channel, text string, overrideUsername string, overrideIconURL string, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string) (*model.Post, *model.AppError) { +func (a *OpenTracingAppLayer) CreateWebhookPost(c request.CTX, userID string, channel *model.Channel, text string, overrideUsername string, overrideIconURL string, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string, priority *model.PostPriority) (*model.Post, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CreateWebhookPost") @@ -2863,7 +2863,7 @@ func (a *OpenTracingAppLayer) CreateWebhookPost(c request.CTX, userID string, ch }() defer span.Finish() - resultVar0, resultVar1 := a.app.CreateWebhookPost(c, userID, channel, text, overrideUsername, overrideIconURL, overrideIconEmoji, props, postType, postRootId) + resultVar0, resultVar1 := a.app.CreateWebhookPost(c, userID, channel, text, overrideUsername, overrideIconURL, overrideIconEmoji, props, postType, postRootId, priority) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/server/channels/app/webhook.go b/server/channels/app/webhook.go index 4679063423..c7768ed6a7 100644 --- a/server/channels/app/webhook.go +++ b/server/channels/app/webhook.go @@ -180,7 +180,7 @@ func (a *App) TriggerWebhook(c request.CTX, payload *model.OutgoingWebhookPayloa if *a.Config().ServiceSettings.EnablePostIconOverride && hook.IconURL != "" && webhookResp.IconURL == "" { webhookResp.IconURL = hook.IconURL } - if _, err := a.CreateWebhookPost(c, hook.CreatorId, channel, text, webhookResp.Username, webhookResp.IconURL, "", webhookResp.Props, webhookResp.Type, postRootId); err != nil { + if _, err := a.CreateWebhookPost(c, hook.CreatorId, channel, text, webhookResp.Username, webhookResp.IconURL, "", webhookResp.Props, webhookResp.Type, postRootId, webhookResp.Priority); err != nil { c.Logger().Error("Failed to create response post.", mlog.Err(err)) } } @@ -304,13 +304,23 @@ func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. return splits, nil } -func (a *App) CreateWebhookPost(c request.CTX, userID string, channel *model.Channel, text, overrideUsername, overrideIconURL, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string) (*model.Post, *model.AppError) { +func (a *App) CreateWebhookPost(c request.CTX, userID string, channel *model.Channel, text, overrideUsername, overrideIconURL, overrideIconEmoji string, props model.StringInterface, postType string, postRootId string, priority *model.PostPriority) (*model.Post, *model.AppError) { // parse links into Markdown format text = linkWithTextRegex.ReplaceAllString(text, "[${2}](${1})") post := &model.Post{UserId: userID, ChannelId: channel.Id, Message: text, Type: postType, RootId: postRootId} post.AddProp("from_webhook", "true") + if priority != nil { + if priority.Priority == nil { + err := model.NewAppError("CreateWebhookPost", "api.context.invalid_param.app_error", map[string]any{"Name": "webhook.priority.priority"}, "Setting the priority of a post is required to use priority.", http.StatusBadRequest) + return nil, err + } + post.Metadata = &model.PostMetadata{ + Priority: priority, + } + } + if strings.HasPrefix(post.Type, model.PostSystemMessagePrefix) { err := model.NewAppError("CreateWebhookPost", "api.context.invalid_param.app_error", map[string]any{"Name": "post.type"}, "", http.StatusBadRequest) return nil, err @@ -830,7 +840,7 @@ func (a *App) HandleIncomingWebhook(c request.CTX, hookID string, req *model.Inc overrideIconURL = req.IconURL } - _, err := a.CreateWebhookPost(c, hook.UserId, channel, text, overrideUsername, overrideIconURL, req.IconEmoji, req.Props, webhookType, "") + _, err := a.CreateWebhookPost(c, hook.UserId, channel, text, overrideUsername, overrideIconURL, req.IconEmoji, req.Props, webhookType, "", req.Priority) return err } diff --git a/server/channels/app/webhook_test.go b/server/channels/app/webhook_test.go index 0b490f4d13..afae9655cf 100644 --- a/server/channels/app/webhook_test.go +++ b/server/channels/app/webhook_test.go @@ -296,21 +296,24 @@ func TestCreateWebhookPost(t *testing.T) { require.Nil(t, err) defer th.App.DeleteIncomingWebhook(hook.Id) - post, err := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", model.StringInterface{ - "attachments": []*model.SlackAttachment{ - { - Text: "text", + post, err := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", + model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "text", + }, }, + "webhook_display_name": hook.DisplayName, }, - "webhook_display_name": hook.DisplayName, - }, model.PostTypeSlackAttachment, "") + model.PostTypeSlackAttachment, + "", nil) require.Nil(t, err) assert.Contains(t, post.GetProps(), "from_webhook", "missing from_webhook prop") assert.Contains(t, post.GetProps(), "attachments", "missing attachments prop") assert.Contains(t, post.GetProps(), "webhook_display_name", "missing webhook_display_name prop") - _, err = th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", nil, model.PostTypeSystemGeneric, "") + _, err = th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", nil, model.PostTypeSystemGeneric, "", nil) require.NotNil(t, err, "Should have failed - bad post type") expectedText := "`<>|<>|`" @@ -321,7 +324,7 @@ func TestCreateWebhookPost(t *testing.T) { }, }, "webhook_display_name": hook.DisplayName, - }, model.PostTypeSlackAttachment, "") + }, model.PostTypeSlackAttachment, "", nil) require.Nil(t, err) assert.Equal(t, expectedText, post.Message) @@ -333,7 +336,7 @@ func TestCreateWebhookPost(t *testing.T) { }, }, "webhook_display_name": hook.DisplayName, - }, model.PostTypeSlackAttachment, "") + }, model.PostTypeSlackAttachment, "", nil) require.Nil(t, err) assert.Equal(t, expectedText, post.Message) @@ -361,13 +364,13 @@ Date: Thu Mar 1 19:46:48 2018 +0300 }, }, "webhook_display_name": hook.DisplayName, - }, model.PostTypeSlackAttachment, "") + }, model.PostTypeSlackAttachment, "", nil) require.Nil(t, err) assert.Equal(t, expectedText, post.Message) t.Run("should set webhook creator status to online", func(t *testing.T) { testCluster.ClearMessages() - _, appErr := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "text", "", "", "", model.StringInterface{}, model.PostTypeDefault, "") + _, appErr := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "text", "", "", "", model.StringInterface{}, model.PostTypeDefault, "", nil) require.Nil(t, appErr) msgs := testCluster.SelectMessages(func(msg *model.ClusterMessage) bool { @@ -381,6 +384,52 @@ Date: Thu Mar 1 19:46:48 2018 +0300 }) } +func TestCreateWebhookPostWithPriority(t *testing.T) { + testCluster := &testlib.FakeClusterInterface{} + th := SetupWithClusterMock(t, testCluster).InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = true }) + + hook, err := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &model.IncomingWebhook{ChannelId: th.BasicChannel.Id}) + require.Nil(t, err) + defer th.App.DeleteIncomingWebhook(hook.Id) + + testConditions := []model.PostPriority{ + { + Priority: model.NewString("high"), + RequestedAck: model.NewBool(true), + PersistentNotifications: model.NewBool(false), + }, + { + Priority: model.NewString(""), + RequestedAck: model.NewBool(true), + PersistentNotifications: model.NewBool(false), + }, + { + Priority: model.NewString("urgent"), + RequestedAck: model.NewBool(false), + PersistentNotifications: model.NewBool(true), + }, + } + + for _, conditions := range testConditions { + post, err := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, "foo @"+th.BasicUser.Username, "user", "http://iconurl", "", + model.StringInterface{"webhook_display_name": hook.DisplayName}, + model.PostTypeSlackAttachment, + "", + &conditions, + ) + + require.Nil(t, err) + + assert.Equal(t, post.Message, "foo @"+th.BasicUser.Username) + assert.Contains(t, post.GetProps(), "from_webhook", "missing from_webhook prop") + assert.Equal(t, *conditions.Priority, *post.GetPriority().Priority) + assert.Equal(t, *conditions.RequestedAck, *post.GetPriority().RequestedAck) + assert.Equal(t, *conditions.PersistentNotifications, *post.GetPriority().PersistentNotifications) + } +} func TestCreateWebhookPostLinks(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -405,7 +454,7 @@ func TestCreateWebhookPostLinks(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - post, err := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, tc.input, "", "", "", model.StringInterface{}, "", "") + post, err := th.App.CreateWebhookPost(th.Context, hook.UserId, th.BasicChannel, tc.input, "", "", "", model.StringInterface{}, "", "", nil) require.Nil(t, err) require.Equal(t, tc.expectedOutput, post.Message) }) diff --git a/server/public/model/incoming_webhook.go b/server/public/model/incoming_webhook.go index 47dfa235de..a84a13d41d 100644 --- a/server/public/model/incoming_webhook.go +++ b/server/public/model/incoming_webhook.go @@ -56,6 +56,7 @@ type IncomingWebhookRequest struct { Attachments []*SlackAttachment `json:"attachments"` Type string `json:"type"` IconEmoji string `json:"icon_emoji"` + Priority *PostPriority `json:"priority"` } func (o *IncomingWebhook) IsValid() *AppError { diff --git a/server/public/model/outgoing_webhook.go b/server/public/model/outgoing_webhook.go index 7683583039..3d7623672e 100644 --- a/server/public/model/outgoing_webhook.go +++ b/server/public/model/outgoing_webhook.go @@ -73,6 +73,7 @@ type OutgoingWebhookResponse struct { Attachments []*SlackAttachment `json:"attachments"` Type string `json:"type"` ResponseType string `json:"response_type"` + Priority *PostPriority `json:"priority"` } const OutgoingHookResponseTypeComment = "comment"