From 5f0aac70ac353ce242a03c2a5e45c9abeae978ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 5 Oct 2023 09:40:53 +0200 Subject: [PATCH] [MM-54497]Fix updatePost props (#24543) * Fix updatePost props and plugin hooks * Add more tests * Minor naming improvement * Revert plugin hooks changes * Rename post variable * Fix app layers --------- Co-authored-by: Mattermost Build --- server/channels/app/app_iface.go | 2 +- .../app/opentracing/opentracing_layer.go | 4 +-- server/channels/app/post.go | 36 +++++++++---------- server/channels/app/post_test.go | 19 ++++++++-- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index 927e33f4b2..4bffa50df6 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -1135,7 +1135,7 @@ type AppIface interface { UpdatePasswordAsUser(c request.CTX, userID, currentPassword, newPassword string) *model.AppError UpdatePasswordByUserIdSendEmail(c request.CTX, userID, newPassword, method string) *model.AppError UpdatePasswordSendEmail(c request.CTX, user *model.User, newPassword, method string) *model.AppError - UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) + UpdatePost(c *request.Context, receivedUpdatedPost *model.Post, safeUpdate bool) (*model.Post, *model.AppError) UpdatePreferences(userID string, preferences model.Preferences) *model.AppError UpdateRemoteCluster(rc *model.RemoteCluster) (*model.RemoteCluster, *model.AppError) UpdateRemoteClusterTopics(remoteClusterId string, topics string) (*model.RemoteCluster, *model.AppError) diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index 765d400244..94a91d7c8b 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -17706,7 +17706,7 @@ func (a *OpenTracingAppLayer) UpdatePasswordSendEmail(c request.CTX, user *model return resultVar0 } -func (a *OpenTracingAppLayer) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { +func (a *OpenTracingAppLayer) UpdatePost(c *request.Context, receivedUpdatedPost *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UpdatePost") @@ -17718,7 +17718,7 @@ func (a *OpenTracingAppLayer) UpdatePost(c *request.Context, post *model.Post, s }() defer span.Finish() - resultVar0, resultVar1 := a.app.UpdatePost(c, post, safeUpdate) + resultVar0, resultVar1 := a.app.UpdatePost(c, receivedUpdatedPost, safeUpdate) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/server/channels/app/post.go b/server/channels/app/post.go index ab4c6ec298..1d6cb7c65b 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -625,10 +625,10 @@ func (a *App) DeleteEphemeralPost(userID, postID string) { a.Publish(message) } -func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { - post.SanitizeProps() +func (a *App) UpdatePost(c *request.Context, receivedUpdatedPost *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { + receivedUpdatedPost.SanitizeProps() - postLists, nErr := a.Srv().Store().Post().Get(context.Background(), post.Id, model.GetPostsOptions{}, "", a.Config().GetSanitizeOptions()) + postLists, nErr := a.Srv().Store().Post().Get(context.Background(), receivedUpdatedPost.Id, model.GetPostsOptions{}, "", a.Config().GetSanitizeOptions()) if nErr != nil { var nfErr *store.ErrNotFound var invErr *store.ErrInvalidInput @@ -641,21 +641,21 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) return nil, model.NewAppError("UpdatePost", "app.post.get.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr) } } - oldPost := postLists.Posts[post.Id] + oldPost := postLists.Posts[receivedUpdatedPost.Id] var err *model.AppError if oldPost == nil { - err = model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+post.Id, http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.find.app_error", nil, "id="+receivedUpdatedPost.Id, http.StatusBadRequest) return nil, err } if oldPost.DeleteAt != 0 { - err = model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]any{"PostId": post.Id}, "", http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.permissions_details.app_error", map[string]any{"PostId": receivedUpdatedPost.Id}, "", http.StatusBadRequest) return nil, err } if oldPost.IsSystemMessage() { - err = model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+post.Id, http.StatusBadRequest) + err = model.NewAppError("UpdatePost", "api.post.update_post.system_message.app_error", nil, "id="+receivedUpdatedPost.Id, http.StatusBadRequest) return nil, err } @@ -670,17 +670,17 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) newPost := oldPost.Clone() - if newPost.Message != post.Message { - newPost.Message = post.Message + if newPost.Message != receivedUpdatedPost.Message { + newPost.Message = receivedUpdatedPost.Message newPost.EditAt = model.GetMillis() - newPost.Hashtags, _ = model.ParseHashtags(post.Message) + newPost.Hashtags, _ = model.ParseHashtags(receivedUpdatedPost.Message) } if !safeUpdate { - newPost.IsPinned = post.IsPinned - newPost.HasReactions = post.HasReactions - newPost.FileIds = post.FileIds - newPost.SetProps(post.GetProps()) + newPost.IsPinned = receivedUpdatedPost.IsPinned + newPost.HasReactions = receivedUpdatedPost.HasReactions + newPost.FileIds = receivedUpdatedPost.FileIds + newPost.SetProps(receivedUpdatedPost.GetProps()) } // Avoid deep-equal checks if EditAt was already modified through message change @@ -688,19 +688,19 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) newPost.EditAt = model.GetMillis() } - if err = a.FillInPostProps(c, post, nil); err != nil { + if err = a.FillInPostProps(c, newPost, nil); err != nil { return nil, err } - if post.IsRemote() { - oldPost.RemoteId = model.NewString(*post.RemoteId) + if receivedUpdatedPost.IsRemote() { + oldPost.RemoteId = model.NewString(*receivedUpdatedPost.RemoteId) } var rejectionReason string pluginContext := pluginContext(c) a.ch.RunMultiHook(func(hooks plugin.Hooks) bool { newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost.ForPlugin(), oldPost.ForPlugin()) - return post != nil + return receivedUpdatedPost != nil }, plugin.MessageWillBeUpdatedID) if newPost == nil { return nil, model.NewAppError("UpdatePost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index 123025bd8e..d7b46928b3 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -418,7 +418,15 @@ func TestPostChannelMentions(t *testing.T) { TeamId: th.BasicTeam.Id, }, false) require.Nil(t, err) + channelToMention2, err := th.App.CreateChannel(th.Context, &model.Channel{ + DisplayName: "Mention Test2", + Name: "mention-test2", + Type: model.ChannelTypeOpen, + TeamId: th.BasicTeam.Id, + }, false) + require.Nil(t, err) defer th.App.PermanentDeleteChannel(th.Context, channelToMention) + defer th.App.PermanentDeleteChannel(th.Context, channelToMention2) _, err = th.App.AddUserToChannel(th.Context, user, channel, false) require.Nil(t, err) @@ -440,15 +448,20 @@ func TestPostChannelMentions(t *testing.T) { }, }, post.GetProp("channel_mentions")) - post.Message = fmt.Sprintf("goodbye, ~%v!", channelToMention.Name) + post.Message = fmt.Sprintf("goodbye, ~%v!", channelToMention2.Name) result, err := th.App.UpdatePost(th.Context, post, false) require.Nil(t, err) assert.Equal(t, map[string]any{ - "mention-test": map[string]any{ - "display_name": "Mention Test", + "mention-test2": map[string]any{ + "display_name": "Mention Test2", "team_name": th.BasicTeam.Name, }, }, result.GetProp("channel_mentions")) + + result.Message = "no more mentions!" + result, err = th.App.UpdatePost(th.Context, result, false) + require.Nil(t, err) + assert.Nil(t, result.GetProp("channel_mentions")) } func TestImageProxy(t *testing.T) {