diff --git a/api/v4/source/posts.yaml b/api/v4/source/posts.yaml index 0ed36e49a1..809c810382 100644 --- a/api/v4/source/posts.yaml +++ b/api/v4/source/posts.yaml @@ -1101,3 +1101,51 @@ $ref: "#/components/responses/NotFound" "501": $ref: "#/components/responses/NotImplemented" + + "/api/v4/posts/{post_id}/restore/{restore_version_id}": + post: + tags: + - posts + summary: Restores a past version of a post + description: > + Restores the post with `post_id` to its past version having the ID `restore_version_id`. + + ##### Permissions + + Must have `read_channel` permission for the channel the post is in. + Must have `edit_post` permission for the channel the post is being moved to. + Must be the author of the post being restored. + + + __Minimum server version__: 10.5 + operationId: RestorePostVersion + parameters: + - name: post_id + in: path + description: The identifier of the post to restore + required: true + schema: + type: string + - name: restore_version_id + in: path + description: The identifier of the past version of post to restore to + required: true + schema: + type: string + responses: + "200": + description: Post restored successful + content: + application/json: + schema: + $ref: "#/components/schemas/Post" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "404": + $ref: "#/components/responses/NotFound" + "501": + $ref: "#/components/responses/NotImplemented" diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index 8280877f5a..455760a5ee 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -10,6 +10,8 @@ import ( "strconv" "time" + "github.com/gorilla/mux" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/v8/channels/app" @@ -36,6 +38,7 @@ func (api *API) InitPost() { api.BaseRoutes.Posts.Handle("/search", api.APISessionRequiredDisableWhenBusy(searchPostsInAllTeams)).Methods(http.MethodPost) api.BaseRoutes.Post.Handle("", api.APISessionRequired(updatePost)).Methods(http.MethodPut) api.BaseRoutes.Post.Handle("/patch", api.APISessionRequired(patchPost)).Methods(http.MethodPut) + api.BaseRoutes.Post.Handle("/restore/{restore_version_id:[A-Za-z0-9]+}", api.APISessionRequired(restorePostVersion)).Methods(http.MethodPost) api.BaseRoutes.PostForUser.Handle("/set_unread", api.APISessionRequired(setPostUnread)).Methods(http.MethodPost) api.BaseRoutes.PostForUser.Handle("/reminder", api.APISessionRequired(setPostReminder)).Methods(http.MethodPost) @@ -859,8 +862,11 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventPriorState(originalPost) auditRec.AddEventObjectType("post") - // Updating the file_ids of a post is not a supported operation and will be ignored - post.FileIds = originalPost.FileIds + // passing a nil fileIds should not have any effect on a post's file IDs + // so, we restore the original file IDs in this case + if post.FileIds == nil { + post.FileIds = originalPost.FileIds + } if c.AppContext.Session().UserId != originalPost.UserId { if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), originalPost.ChannelId, model.PermissionEditOthersPosts) { @@ -876,7 +882,7 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } - rpost, err := c.App.UpdatePost(c.AppContext, c.App.PostWithProxyRemovedFromImageURLs(&post), false) + rpost, err := c.App.UpdatePost(c.AppContext, c.App.PostWithProxyRemovedFromImageURLs(&post), &model.UpdatePostOptions{SafeUpdate: false}) if err != nil { c.Err = err return @@ -914,9 +920,26 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) { } } - // Updating the file_ids of a post is not a supported operation and will be ignored - post.FileIds = nil + postPatchChecks(c, auditRec, post.Message) + if c.Err != nil { + return + } + patchedPost, err := c.App.PatchPost(c.AppContext, c.Params.PostId, c.App.PostPatchWithProxyRemovedFromImageURLs(&post), nil) + if err != nil { + c.Err = err + return + } + + auditRec.Success() + auditRec.AddEventResultState(patchedPost) + + if err := patchedPost.EncodeJSON(w); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + +func postPatchChecks(c *Context, auditRec *audit.Record, message *string) { originalPost, err := c.App.GetSinglePost(c.AppContext, c.Params.PostId, false) if err != nil { c.SetPermissionError(model.PermissionEditPost) @@ -926,6 +949,7 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddEventObjectType("post") var permission *model.Permission + if c.AppContext.Session().UserId == originalPost.UserId { permission = model.PermissionEditPost } else { @@ -937,23 +961,10 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) { return } - if *c.App.Config().ServiceSettings.PostEditTimeLimit != -1 && model.GetMillis() > originalPost.CreateAt+int64(*c.App.Config().ServiceSettings.PostEditTimeLimit*1000) && post.Message != nil { + if *c.App.Config().ServiceSettings.PostEditTimeLimit != -1 && model.GetMillis() > originalPost.CreateAt+int64(*c.App.Config().ServiceSettings.PostEditTimeLimit*1000) && message != nil { c.Err = model.NewAppError("patchPost", "api.post.update_post.permissions_time_limit.app_error", map[string]any{"timeLimit": *c.App.Config().ServiceSettings.PostEditTimeLimit}, "", http.StatusBadRequest) return } - - patchedPost, err := c.App.PatchPost(c.AppContext, c.Params.PostId, c.App.PostPatchWithProxyRemovedFromImageURLs(&post)) - if err != nil { - c.Err = err - return - } - - auditRec.Success() - auditRec.AddEventResultState(patchedPost) - - if err := patchedPost.EncodeJSON(w); err != nil { - c.Logger.Warn("Error while writing response", mlog.Err(err)) - } } func setPostUnread(c *Context, w http.ResponseWriter, r *http.Request) { @@ -1045,7 +1056,7 @@ func saveIsPinnedPost(c *Context, w http.ResponseWriter, isPinned bool) { patch := &model.PostPatch{} patch.IsPinned = model.NewPointer(isPinned) - patchedPost, err := c.App.PatchPost(c.AppContext, c.Params.PostId, patch) + patchedPost, err := c.App.PatchPost(c.AppContext, c.Params.PostId, patch, nil) if err != nil { c.Err = err return @@ -1285,6 +1296,55 @@ func getPostInfo(c *Context, w http.ResponseWriter, r *http.Request) { } } +func restorePostVersion(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequirePostId() + if c.Err != nil { + return + } + + props := mux.Vars(r) + restoreVersionId, ok := props["restore_version_id"] + if !ok { + c.SetInvalidParam("restore_version_id") + return + } + + auditRec := c.MakeAuditRecord("restorePostVersion", audit.Fail) + audit.AddEventParameter(auditRec, "id", c.Params.PostId) + audit.AddEventParameter(auditRec, "restore_version_id", restoreVersionId) + defer c.LogAuditRecWithLevel(auditRec, app.LevelContent) + + toRestorePost, err := c.App.GetSinglePost(c.AppContext, restoreVersionId, true) + if err != nil { + c.SetPermissionError(model.PermissionEditPost) + return + } + + // user can only restore their own posts + if c.AppContext.Session().UserId != toRestorePost.UserId { + c.SetPermissionError(model.PermissionEditPost) + return + } + + postPatchChecks(c, auditRec, &toRestorePost.Message) + if c.Err != nil { + return + } + + updatedPost, appErr := c.App.RestorePostVersion(c.AppContext, c.AppContext.Session().UserId, c.Params.PostId, restoreVersionId) + if appErr != nil { + c.Err = appErr + return + } + + auditRec.Success() + auditRec.AddEventResultState(updatedPost) + + if err := updatedPost.EncodeJSON(w); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + func hasPermittedWranglerRole(c *Context, user *model.User, channelMember *model.ChannelMember) bool { // If there are no configured PermittedWranglerRoles, skip the check if len(c.App.Config().WranglerSettings.PermittedWranglerRoles) == 0 { diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index 61702a457a..b40ab87339 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -1328,30 +1328,6 @@ func TestUpdatePost(t *testing.T) { assert.EqualValues(t, 0, rpost.EditAt, "Newly created post shouldn't have EditAt set") assert.Equal(t, model.StringArray(fileIds), rpost.FileIds, "FileIds should have been set") - t.Run("same message, fewer files", func(t *testing.T) { - msg := "zz" + model.NewId() + " update post" - rpost.Message = msg - rpost.UserId = "" - - rupost, _, err := client.UpdatePost(context.Background(), rpost.Id, &model.Post{ - Id: rpost.Id, - Message: rpost.Message, - FileIds: fileIds[0:2], // one fewer file id - }) - require.NoError(t, err) - - assert.Equal(t, rupost.Message, msg, "failed to updates") - assert.NotEqual(t, 0, rupost.EditAt, "EditAt not updated for post") - assert.Equal(t, model.StringArray(fileIds), rupost.FileIds, "FileIds should have not have been updated") - - actual, _, err := client.GetPost(context.Background(), rpost.Id, "") - require.NoError(t, err) - - assert.Equal(t, actual.Message, msg, "failed to updates") - assert.NotEqual(t, 0, actual.EditAt, "EditAt not updated for post") - assert.Equal(t, model.StringArray(fileIds), actual.FileIds, "FileIds should have not have been updated") - }) - t.Run("new message, invalid props", func(t *testing.T) { msg1 := "#hashtag a" + model.NewId() + " update post again" rpost.Message = msg1 @@ -1398,22 +1374,6 @@ func TestUpdatePost(t *testing.T) { }, channel, model.CreatePostFlags{SetOnline: true}) require.Nil(t, appErr) - t.Run("new message, add files", func(t *testing.T) { - up3 := &model.Post{ - Id: rpost3.Id, - ChannelId: channel.Id, - Message: "zz" + model.NewId() + " update post 3", - FileIds: fileIds[0:2], - } - rrupost3, _, err := client.UpdatePost(context.Background(), rpost3.Id, up3) - require.NoError(t, err) - assert.Empty(t, rrupost3.FileIds) - - actual, _, err := client.GetPost(context.Background(), rpost.Id, "") - require.NoError(t, err) - assert.Equal(t, model.StringArray(fileIds), actual.FileIds) - }) - t.Run("add slack attachments", func(t *testing.T) { up4 := &model.Post{ Id: rpost3.Id, @@ -1509,6 +1469,184 @@ func TestUpdatePost(t *testing.T) { _, _, err := th.SystemAdminClient.UpdatePost(context.Background(), rpost.Id, rpost) require.NoError(t, err) }) + + t.Run("should be able to add new files", func(t *testing.T) { + th.LoginBasic() + // create new file + fileResponse, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse.FileInfos)) + fileInfo := fileResponse.FileInfos[0] + + // create new post + post, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + }, channel, model.CreatePostFlags{SetOnline: true}) + + require.Nil(t, appErr) + require.NotNil(t, post) + + // update post with new file + post.FileIds = []string{fileInfo.Id} + _, _, err = client.UpdatePost(context.Background(), post.Id, post) + require.NoError(t, err) + + updatedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, post.Id, updatedPost.Id) + require.Equal(t, 1, len(updatedPost.FileIds)) + require.Equal(t, fileInfo.Id, updatedPost.FileIds[0]) + + // verify file is attached to the post + fetchedFileInfo, _, err := client.GetFileInfo(context.Background(), fileInfo.Id) + require.NoError(t, err) + require.Equal(t, fileInfo.Id, fetchedFileInfo.Id) + require.Equal(t, post.Id, fetchedFileInfo.PostId) + }) + + t.Run("should be able to remove files", func(t *testing.T) { + th.LoginBasic() + // create new file + fileResponse, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse.FileInfos)) + fileInfo := fileResponse.FileInfos[0] + + // create new post + post, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + FileIds: []string{fileInfo.Id}, + }, channel, model.CreatePostFlags{SetOnline: true}) + + require.Nil(t, appErr) + require.NotNil(t, post) + require.Equal(t, 1, len(post.FileIds)) + + // remove files from post + post.FileIds = []string{} + _, _, err = client.UpdatePost(context.Background(), post.Id, post) + require.NoError(t, err) + + updatedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, post.Id, updatedPost.Id) + require.Equal(t, 0, len(updatedPost.FileIds)) + + // verify file is removed from the post + postFileInfos, err := th.App.Srv().Store().FileInfo().GetForPost(post.Id, true, true, false) + require.NoError(t, err) + require.Equal(t, 1, len(postFileInfos)) + require.Equal(t, fileInfo.Id, postFileInfos[0].Id) + require.Greater(t, postFileInfos[0].DeleteAt, int64(0)) + }) + + t.Run("post files remain unchanged when fileIds is nil", func(t *testing.T) { + th.LoginBasic() + // create new file + fileResponse, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse.FileInfos)) + fileInfo := fileResponse.FileInfos[0] + + // create new post + post, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + FileIds: []string{fileInfo.Id}, + }, channel, model.CreatePostFlags{SetOnline: true}) + + require.Nil(t, appErr) + require.NotNil(t, post) + require.Equal(t, 1, len(post.FileIds)) + + // update post without specifying fileIds + post.FileIds = nil + post.Message = "updated message" + _, _, err = client.UpdatePost(context.Background(), post.Id, post) + require.NoError(t, err) + + updatedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, post.Id, updatedPost.Id) + require.Equal(t, 1, len(updatedPost.FileIds)) + require.Equal(t, fileInfo.Id, updatedPost.FileIds[0]) + require.Equal(t, "updated message", updatedPost.Message) + + // verify file is still part of the post + postFileInfos, err := th.App.Srv().Store().FileInfo().GetForPost(post.Id, true, false, false) + require.NoError(t, err) + require.Equal(t, 1, len(postFileInfos)) + require.Equal(t, fileInfo.Id, postFileInfos[0].Id) + require.Equal(t, int64(0), postFileInfos[0].DeleteAt) + }) + + t.Run("should be able to add and remove files simultaneously", func(t *testing.T) { + th.LoginBasic() + // create new file + fileResponse1, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse1.FileInfos)) + fileInfo1 := fileResponse1.FileInfos[0] + + fileResponse2, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse2.FileInfos)) + fileInfo2 := fileResponse2.FileInfos[0] + + // create new post + post, appErr := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + FileIds: model.StringArray{fileInfo1.Id, fileInfo2.Id}, + }, channel, model.CreatePostFlags{SetOnline: true}) + + require.Nil(t, appErr) + require.NotNil(t, post) + require.Equal(t, 2, len(post.FileIds)) + + // update post with new file + + fileResponse3, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse3.FileInfos)) + fileInfo3 := fileResponse3.FileInfos[0] + + fileResponse4, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse4.FileInfos)) + fileInfo4 := fileResponse4.FileInfos[0] + + post.FileIds = []string{fileInfo3.Id, fileInfo4.Id} + _, _, err = client.UpdatePost(context.Background(), post.Id, post) + require.NoError(t, err) + + updatedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, post.Id, updatedPost.Id) + require.Equal(t, 2, len(updatedPost.FileIds)) + require.Contains(t, updatedPost.FileIds, fileInfo3.Id) + require.Contains(t, updatedPost.FileIds, fileInfo4.Id) + + postFiles, err := th.App.Srv().Store().FileInfo().GetForPost(post.Id, true, true, false) + require.NoError(t, err) + require.Equal(t, 4, len(postFiles)) + + for _, postFile := range postFiles { + if postFile.Id == fileInfo1.Id || postFile.Id == fileInfo2.Id { + require.Greater(t, postFile.DeleteAt, int64(0)) + } + + if postFile.Id == fileInfo3.Id || postFile.Id == fileInfo4.Id { + require.Equal(t, postFile.PostId, post.Id) + } + } + }) } func TestUpdateOthersPostInDirectMessageChannel(t *testing.T) { @@ -1583,7 +1721,7 @@ func TestPatchPost(t *testing.T) { assert.Equal(t, "#otherhashtag other message", rpost.Message, "Message did not update properly") assert.Equal(t, *patch.Props, rpost.GetProps(), "Props did not update properly") assert.Equal(t, "#otherhashtag", rpost.Hashtags, "Message did not update properly") - assert.Equal(t, model.StringArray(fileIDs[0:2]), rpost.FileIds, "FileIds should not update") + assert.Equal(t, model.StringArray(fileIDs), rpost.FileIds, "FileIds should not update") assert.False(t, rpost.HasReactions, "HasReactions did not update properly") }) @@ -1731,6 +1869,135 @@ func TestPatchPost(t *testing.T) { require.Error(t, patchErr) CheckBadRequestStatus(t, patchResp) }) + + t.Run("should be able to add new files", func(t *testing.T) { + post, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "#hashtag a message", + CreateAt: model.GetMillis() - 2000, + }) + + require.NoError(t, err) + + fileResponse, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse.FileInfos)) + fileInfo := fileResponse.FileInfos[0] + + patch := &model.PostPatch{ + FileIds: &model.StringArray{fileInfo.Id}, + } + + _, _, err = client.PatchPost(context.Background(), post.Id, patch) + require.NoError(t, err) + + patchedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, 1, len(patchedPost.FileIds)) + require.Equal(t, fileInfo.Id, patchedPost.FileIds[0]) + }) + + t.Run("should be able to remove some files", func(t *testing.T) { + fileResponse1, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse1.FileInfos)) + fileInfo1 := fileResponse1.FileInfos[0] + + fileResponse2, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse2.FileInfos)) + fileInfo2 := fileResponse2.FileInfos[0] + + post, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "#hashtag a message", + CreateAt: model.GetMillis() - 2000, + FileIds: model.StringArray{fileInfo1.Id, fileInfo2.Id}, + }) + + require.NoError(t, err) + require.Equal(t, 2, len(post.FileIds)) + + patch := &model.PostPatch{ + FileIds: &model.StringArray{fileInfo2.Id}, + } + + _, _, err = client.PatchPost(context.Background(), post.Id, patch) + require.NoError(t, err) + + patchedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, 1, len(patchedPost.FileIds)) + require.Equal(t, fileInfo2.Id, patchedPost.FileIds[0]) + }) + + t.Run("should be able to remove all files", func(t *testing.T) { + fileResponse1, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse1.FileInfos)) + fileInfo1 := fileResponse1.FileInfos[0] + + fileResponse2, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse2.FileInfos)) + fileInfo2 := fileResponse2.FileInfos[0] + + post, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "#hashtag a message", + CreateAt: model.GetMillis() - 2000, + FileIds: model.StringArray{fileInfo1.Id, fileInfo2.Id}, + }) + + require.NoError(t, err) + require.Equal(t, 2, len(post.FileIds)) + + patch := &model.PostPatch{ + FileIds: &model.StringArray{}, + } + + _, _, err = client.PatchPost(context.Background(), post.Id, patch) + require.NoError(t, err) + + patchedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, 0, len(patchedPost.FileIds)) + }) + + t.Run("post files remain unchanged when fileIds is nil", func(t *testing.T) { + fileResponse1, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse1.FileInfos)) + fileInfo1 := fileResponse1.FileInfos[0] + + fileResponse2, _, err := client.UploadFile(context.Background(), data, channel.Id, "test.png") + require.NoError(t, err) + require.Equal(t, 1, len(fileResponse2.FileInfos)) + fileInfo2 := fileResponse2.FileInfos[0] + + post, _, err := client.CreatePost(context.Background(), &model.Post{ + ChannelId: channel.Id, + Message: "#hashtag a message", + CreateAt: model.GetMillis() - 2000, + FileIds: model.StringArray{fileInfo1.Id, fileInfo2.Id}, + }) + + require.NoError(t, err) + require.Equal(t, 2, len(post.FileIds)) + + patch := &model.PostPatch{ + FileIds: nil, + } + + _, _, err = client.PatchPost(context.Background(), post.Id, patch) + require.NoError(t, err) + + patchedPost, _, err := client.GetPost(context.Background(), post.Id, "") + require.NoError(t, err) + require.Equal(t, 2, len(patchedPost.FileIds)) + require.Contains(t, patchedPost.FileIds, fileInfo1.Id) + require.Contains(t, patchedPost.FileIds, fileInfo2.Id) + }) } func TestPinPost(t *testing.T) { @@ -4153,6 +4420,58 @@ func TestGetEditHistoryForPost(t *testing.T) { require.Error(t, err) CheckForbiddenStatus(t, resp) }) + + t.Run("edit history includes file metadata", func(t *testing.T) { + th.LoginBasic() + fileInfo1, appErr := th.App.UploadFile(th.Context, []byte("data"), th.BasicChannel.Id, "test") + require.Nil(t, appErr) + + fileInfo2, appErr := th.App.UploadFile(th.Context, []byte("data"), th.BasicChannel.Id, "test") + require.Nil(t, appErr) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "new message", + UserId: th.BasicUser.Id, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, + } + + createdPost, appErr := th.App.CreatePost(th.Context, post, th.BasicChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + require.Contains(t, createdPost.FileIds, fileInfo1.Id) + require.Contains(t, createdPost.FileIds, fileInfo2.Id) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message 1"), + } + _, response, err := client.PatchPost(context.Background(), createdPost.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message 2"), + } + _, response, err = client.PatchPost(context.Background(), createdPost.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message 3"), + } + _, response, err = client.PatchPost(context.Background(), createdPost.Id, patch) + require.NoError(t, err) + CheckOKStatus(t, response) + + editHistory, resp, err := client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, resp) + + for _, editHistoryItem := range editHistory { + require.Len(t, editHistoryItem.FileIds, 2) + require.Contains(t, editHistoryItem.FileIds, fileInfo1.Id) + require.Contains(t, editHistoryItem.FileIds, fileInfo2.Id) + } + }) } func TestCreatePostNotificationsWithCRT(t *testing.T) { @@ -4779,3 +5098,230 @@ func TestUnacknowledgePost(t *testing.T) { require.Error(t, err) CheckUnauthorizedStatus(t, resp) } + +func TestRestorePostVersion(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + client := th.Client + + t.Run("should restore post version successfully", func(t *testing.T) { + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message", + UserId: th.BasicUser.Id, + } + + createdPost, response, err := client.CreatePost(context.Background(), post) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + patch, response, err := client.PatchPost(context.Background(), createdPost.Id, &model.PostPatch{ + Message: model.NewPointer("edited message 1"), + }) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "edited message 1", patch.Message) + + patch, response, err = client.PatchPost(context.Background(), createdPost.Id, &model.PostPatch{ + Message: model.NewPointer("edited message 2"), + }) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "edited message 2", patch.Message) + + // verify edit history + editHistory, response, err := client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, 2, len(editHistory)) + require.Equal(t, "edited message 1", editHistory[0].Message) + require.Equal(t, "original message", editHistory[1].Message) + + // now we'll restore to the original version + restoredPost, response, err := client.RestorePostVersion(context.Background(), createdPost.Id, editHistory[1].Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "original message", restoredPost.Message) + require.Equal(t, createdPost.Id, restoredPost.Id) + + // verify restored post + fetchedPost, response, err := client.GetPost(context.Background(), createdPost.Id, "") + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "original message", fetchedPost.Message) + + // verify edit history after restoring + editHistory, response, err = client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, 3, len(editHistory)) + require.Equal(t, "edited message 2", editHistory[0].Message) + require.Equal(t, "edited message 1", editHistory[1].Message) + require.Equal(t, "original message", editHistory[2].Message) + }) + + t.Run("should restore post version successfully with files", func(t *testing.T) { + fileResp, _, err := client.UploadFile(context.Background(), []byte("data"), th.BasicChannel.Id, "test") + require.NoError(t, err) + fileId := fileResp.FileInfos[0].Id + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message", + UserId: th.BasicUser.Id, + FileIds: model.StringArray{fileId}, + } + + createdPost, response, err := client.CreatePost(context.Background(), post) + require.NoError(t, err) + CheckCreatedStatus(t, response) + require.Equal(t, 1, len(createdPost.FileIds)) + + patch, response, err := client.PatchPost(context.Background(), createdPost.Id, &model.PostPatch{ + Message: model.NewPointer("edited message 1"), + FileIds: &model.StringArray{}, + }) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "edited message 1", patch.Message) + require.Equal(t, 0, len(patch.FileIds)) + + // verify edit history + editHistory, response, err := client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, 1, len(editHistory)) + require.Equal(t, "original message", editHistory[0].Message) + require.Equal(t, 1, len(editHistory[0].FileIds)) + + // now we'll restore to the original version + restoredPost, response, err := client.RestorePostVersion(context.Background(), createdPost.Id, editHistory[0].Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "original message", restoredPost.Message) + require.Equal(t, createdPost.Id, restoredPost.Id) + require.Equal(t, 1, len(restoredPost.FileIds)) + + // verify restored post + fetchedPost, response, err := client.GetPost(context.Background(), createdPost.Id, "") + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "original message", fetchedPost.Message) + require.Equal(t, 1, len(fetchedPost.FileIds)) + + // verify edit history after restoring + editHistory, response, err = client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, 2, len(editHistory)) + require.Equal(t, "edited message 1", editHistory[0].Message) + require.Equal(t, 0, len(editHistory[0].FileIds)) + + require.Equal(t, "original message", editHistory[1].Message) + require.Equal(t, 1, len(editHistory[1].FileIds)) + }) + + t.Run("should get error when trying to restore non existent post ori history ID", func(t *testing.T) { + restoredPost, response, err := client.RestorePostVersion(context.Background(), model.NewId(), model.NewId()) + require.Error(t, err) + CheckForbiddenStatus(t, response) + require.Nil(t, restoredPost) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message", + UserId: th.BasicUser.Id, + } + + createdPost, response, err := client.CreatePost(context.Background(), post) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + restoredPost, response, err = client.RestorePostVersion(context.Background(), createdPost.Id, model.NewId()) + require.Error(t, err) + CheckForbiddenStatus(t, response) + require.Nil(t, restoredPost) + + post2 := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message 2", + UserId: th.BasicUser.Id, + } + + createdPost, response, err = client.CreatePost(context.Background(), post2) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + restoredPost, response, err = client.RestorePostVersion(context.Background(), createdPost.Id, post2.Id) + require.Error(t, err) + CheckNotFoundStatus(t, response) + require.Nil(t, restoredPost) + }) + + t.Run("user should not be able to restore someone else's post", func(t *testing.T) { + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message", + UserId: th.BasicUser.Id, + } + + createdPost, response, err := client.CreatePost(context.Background(), post) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + patch, response, err := client.PatchPost(context.Background(), createdPost.Id, &model.PostPatch{ + Message: model.NewPointer("edited message 1"), + }) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "edited message 1", patch.Message) + + // verify edit history + editHistory, response, err := client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, 1, len(editHistory)) + require.Equal(t, "original message", editHistory[0].Message) + + // now we'll restore to the original version + th.LoginBasic2() + restoredPost, response, err := th.Client.RestorePostVersion(context.Background(), createdPost.Id, editHistory[0].Id) + require.Error(t, err) + CheckForbiddenStatus(t, response) + require.Nil(t, restoredPost) + }) + + t.Run("system admin should not be able to restore someone else's post", func(t *testing.T) { + th.LoginBasic() + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "original message", + UserId: th.BasicUser.Id, + } + + createdPost, response, err := th.Client.CreatePost(context.Background(), post) + require.NoError(t, err) + CheckCreatedStatus(t, response) + + patch, response, err := th.Client.PatchPost(context.Background(), createdPost.Id, &model.PostPatch{ + Message: model.NewPointer("edited message 1"), + }) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, "edited message 1", patch.Message) + + // verify edit history + editHistory, response, err := th.Client.GetEditHistoryForPost(context.Background(), createdPost.Id) + require.NoError(t, err) + CheckOKStatus(t, response) + require.Equal(t, 1, len(editHistory)) + require.Equal(t, "original message", editHistory[0].Message) + + // now we'll restore to the original version + th.LoginSystemAdmin() + restoredPost, response, err := th.SystemAdminClient.RestorePostVersion(context.Background(), createdPost.Id, editHistory[0].Id) + require.Error(t, err) + CheckForbiddenStatus(t, response) + require.Nil(t, restoredPost) + }) +} diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index 8459ef20d4..97e777442d 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -975,7 +975,7 @@ type AppIface interface { OutgoingOAuthConnections() einterfaces.OutgoingOAuthConnectionInterface PatchChannel(c request.CTX, channel *model.Channel, patch *model.ChannelPatch, userID string) (*model.Channel, *model.AppError) PatchChannelMembersNotifyProps(c request.CTX, members []*model.ChannelMemberIdentifier, notifyProps map[string]string) ([]*model.ChannelMember, *model.AppError) - PatchPost(c request.CTX, postID string, patch *model.PostPatch) (*model.Post, *model.AppError) + PatchPost(c request.CTX, postID string, patch *model.PostPatch, patchPostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) PatchRemoteCluster(rcId string, patch *model.RemoteClusterPatch) (*model.RemoteCluster, *model.AppError) PatchRetentionPolicy(patch *model.RetentionPolicyWithTeamAndChannelIDs) (*model.RetentionPolicyWithTeamAndChannelCounts, *model.AppError) PatchRole(role *model.Role, patch *model.RolePatch) (*model.Role, *model.AppError) @@ -1043,6 +1043,7 @@ type AppIface interface { ResetSamlAuthDataToEmail(includeDeleted bool, dryRun bool, userIDs []string) (numAffected int, appErr *model.AppError) RestoreChannel(c request.CTX, channel *model.Channel, userID string) (*model.Channel, *model.AppError) RestoreGroup(groupID string) (*model.Group, *model.AppError) + RestorePostVersion(c request.CTX, userID, postID, restoreVersionID string) (*model.Post, *model.AppError) RestoreTeam(teamID string) *model.AppError RestrictUsersGetByPermissions(c request.CTX, userID string, options *model.UserGetOptions) (*model.UserGetOptions, *model.AppError) RestrictUsersSearchByPermissions(c request.CTX, userID string, options *model.UserSearchOptions) (*model.UserSearchOptions, *model.AppError) @@ -1200,7 +1201,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.CTX, receivedUpdatedPost *model.Post, safeUpdate bool) (*model.Post, *model.AppError) + UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, updatePostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) UpdatePreferences(c request.CTX, 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/channel_test.go b/server/channels/app/channel_test.go index 88e3b267db..d439b2777e 100644 --- a/server/channels/app/channel_test.go +++ b/server/channels/app/channel_test.go @@ -2647,7 +2647,7 @@ func TestMarkUnreadCRTOffUpdatesThreads(t *testing.T) { require.Nil(t, appErr) editedPost := r1.Clone() editedPost.Message += " edited" - _, appErr = th.App.UpdatePost(th.Context, editedPost, false) + _, appErr = th.App.UpdatePost(th.Context, editedPost, &model.UpdatePostOptions{SafeUpdate: false}) require.Nil(t, appErr) th.LinkUserToTeam(user3, th.BasicTeam) diff --git a/server/channels/app/draft.go b/server/channels/app/draft.go index be7eff6b8f..f945fc6899 100644 --- a/server/channels/app/draft.go +++ b/server/channels/app/draft.go @@ -116,7 +116,7 @@ func (a *App) getFileInfosForDraft(rctx request.CTX, draft *model.Draft) ([]*mod return nil, nil } - allFileInfos, err := a.Srv().Store().FileInfo().GetByIds(draft.FileIds) + allFileInfos, err := a.Srv().Store().FileInfo().GetByIds(draft.FileIds, false, true) if err != nil { return nil, model.NewAppError("GetFileInfosForDraft", "app.draft.get_for_draft.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/helper_test.go b/server/channels/app/helper_test.go index 2581342c6a..5f8da90249 100644 --- a/server/channels/app/helper_test.go +++ b/server/channels/app/helper_test.go @@ -50,6 +50,10 @@ type TestHelper struct { tempWorkspace string } +type PostOptions func(*model.Post) + +type PostPatchOptions func(patch *model.PostPatch) + func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer bool, updateConfig func(*model.Config), options []Option, tb testing.TB) *TestHelper { tempWorkspace, err := os.MkdirTemp("", "apptest") @@ -445,7 +449,7 @@ func (th *TestHelper) CreateGroupChannel(c request.CTX, user1 *model.User, user2 return channel } -func (th *TestHelper) CreatePost(channel *model.Channel) *model.Post { +func (th *TestHelper) CreatePost(channel *model.Channel, postOptions ...PostOptions) *model.Post { id := model.NewId() post := &model.Post{ @@ -455,6 +459,10 @@ func (th *TestHelper) CreatePost(channel *model.Channel) *model.Post { CreateAt: model.GetMillis() - 10000, } + for _, option := range postOptions { + option(post) + } + var err *model.AppError if post, err = th.App.CreatePost(th.Context, post, channel, model.CreatePostFlags{SetOnline: true}); err != nil { panic(err) @@ -772,6 +780,41 @@ func (th *TestHelper) AddPermissionToRole(permission string, roleName string) { } } +func (th *TestHelper) CreateFileInfo(userId, postId, channelId string) *model.FileInfo { + fileInfo := &model.FileInfo{ + Id: model.NewId(), + CreatorId: userId, + PostId: postId, + ChannelId: channelId, + CreateAt: model.GetMillis(), + Name: model.NewRandomString(10), + Path: model.NewRandomString(50), + } + + createdFileInfo, err := th.App.Srv().Store().FileInfo().Save(th.Context, fileInfo) + if err != nil { + panic(err) + } + + return createdFileInfo +} + +func (th *TestHelper) PostPatch(post *model.Post, message string, options ...PostPatchOptions) *model.Post { + postPatch := &model.PostPatch{ + Message: model.NewPointer(message), + } + for _, optionFunc := range options { + optionFunc(postPatch) + } + + updatedPost, appErr := th.App.PatchPost(th.Context, post.Id, postPatch, nil) + if appErr != nil { + panic(appErr) + } + + return updatedPost +} + // This function is copy of storetest/NewTestId // NewTestId is used for testing as a replacement for model.NewId(). It is a [A-Z0-9] string 26 // characters long. It replaces every odd character with a digit. diff --git a/server/channels/app/integration_action.go b/server/channels/app/integration_action.go index 9afd2a6d19..dc511cbc86 100644 --- a/server/channels/app/integration_action.go +++ b/server/channels/app/integration_action.go @@ -271,7 +271,7 @@ func (a *App) DoPostActionWithCookie(c request.CTX, postID, actionId, userID, se response.Update.IsPinned = originalIsPinned response.Update.HasReactions = originalHasReactions - if _, appErr = a.UpdatePost(c, response.Update, false); appErr != nil { + if _, appErr = a.UpdatePost(c, response.Update, &model.UpdatePostOptions{SafeUpdate: false}); appErr != nil { return "", appErr } } diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index ddc76bdf07..90a599339f 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -13433,7 +13433,7 @@ func (a *OpenTracingAppLayer) PatchChannelModerationsForChannel(c request.CTX, c return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) PatchPost(c request.CTX, postID string, patch *model.PostPatch) (*model.Post, *model.AppError) { +func (a *OpenTracingAppLayer) PatchPost(c request.CTX, postID string, patch *model.PostPatch, patchPostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PatchPost") @@ -13445,7 +13445,7 @@ func (a *OpenTracingAppLayer) PatchPost(c request.CTX, postID string, patch *mod }() defer span.Finish() - resultVar0, resultVar1 := a.app.PatchPost(c, postID, patch) + resultVar0, resultVar1 := a.app.PatchPost(c, postID, patch, patchPostOptions) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -15033,6 +15033,28 @@ func (a *OpenTracingAppLayer) RestoreGroup(groupID string) (*model.Group, *model return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) RestorePostVersion(c request.CTX, userID string, postID string, restoreVersionID string) (*model.Post, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RestorePostVersion") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.RestorePostVersion(c, userID, postID, restoreVersionID) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) RestoreTeam(teamID string) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.RestoreTeam") @@ -18660,7 +18682,7 @@ func (a *OpenTracingAppLayer) UpdatePasswordSendEmail(c request.CTX, user *model return resultVar0 } -func (a *OpenTracingAppLayer) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { +func (a *OpenTracingAppLayer) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, updatePostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UpdatePost") @@ -18672,7 +18694,7 @@ func (a *OpenTracingAppLayer) UpdatePost(c request.CTX, receivedUpdatedPost *mod }() defer span.Finish() - resultVar0, resultVar1 := a.app.UpdatePost(c, receivedUpdatedPost, safeUpdate) + resultVar0, resultVar1 := a.app.UpdatePost(c, receivedUpdatedPost, updatePostOptions) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/server/channels/app/plugin_api.go b/server/channels/app/plugin_api.go index d287b4a728..0823588340 100644 --- a/server/channels/app/plugin_api.go +++ b/server/channels/app/plugin_api.go @@ -744,7 +744,7 @@ func (api *PluginAPI) GetPostsForChannel(channelID string, page, perPage int) (* } func (api *PluginAPI) UpdatePost(post *model.Post) (*model.Post, *model.AppError) { - post, appErr := api.app.UpdatePost(api.ctx, post, false) + post, appErr := api.app.UpdatePost(api.ctx, post, &model.UpdatePostOptions{SafeUpdate: false}) if post != nil { post = post.ForPlugin() } diff --git a/server/channels/app/plugin_hooks_test.go b/server/channels/app/plugin_hooks_test.go index 49ae594203..4d4e92b65f 100644 --- a/server/channels/app/plugin_hooks_test.go +++ b/server/channels/app/plugin_hooks_test.go @@ -379,7 +379,7 @@ func TestHookMessageWillBeUpdated(t *testing.T) { require.Nil(t, err) assert.Equal(t, "message_", post.Message) post.Message = post.Message + "edited_" - post, err = th.App.UpdatePost(th.Context, post, true) + post, err = th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err) assert.Equal(t, "message_edited_fromplugin", post.Message) } @@ -427,7 +427,7 @@ func TestHookMessageHasBeenUpdated(t *testing.T) { require.Nil(t, err) assert.Equal(t, "message_", post.Message) post.Message = post.Message + "edited" - _, err = th.App.UpdatePost(th.Context, post, true) + _, err = th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err) } diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 5201d4d0cf..229eac960e 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -458,16 +458,7 @@ func (a *App) addPostPreviewProp(rctx request.CTX, post *model.Post) (*model.Pos } func (a *App) attachFilesToPost(rctx request.CTX, post *model.Post) *model.AppError { - var attachedIds []string - for _, fileID := range post.FileIds { - err := a.Srv().Store().FileInfo().AttachToPost(rctx, fileID, post.Id, post.ChannelId, post.UserId) - if err != nil { - rctx.Logger().Warn("Failed to attach file to post", mlog.String("file_id", fileID), mlog.String("post_id", post.Id), mlog.Err(err)) - continue - } - - attachedIds = append(attachedIds, fileID) - } + attachedIds := a.attachFileIDsToPost(rctx, post.Id, post.ChannelId, post.UserId, post.FileIds) if len(post.FileIds) != len(attachedIds) { // We couldn't attach all files to the post, so ensure that post.FileIds reflects what was actually attached @@ -481,6 +472,20 @@ func (a *App) attachFilesToPost(rctx request.CTX, post *model.Post) *model.AppEr return nil } +func (a *App) attachFileIDsToPost(rctx request.CTX, postID, channelID, userID string, fileIDs []string) []string { + var attachedIds []string + for _, fileID := range fileIDs { + err := a.Srv().Store().FileInfo().AttachToPost(rctx, fileID, postID, channelID, userID) + if err != nil { + rctx.Logger().Warn("Failed to attach file to post", mlog.String("file_id", fileID), mlog.String("post_id", postID), mlog.Err(err)) + continue + } + + attachedIds = append(attachedIds, fileID) + } + return attachedIds +} + // FillInPostProps should be invoked before saving posts to fill in properties such as // channel_mentions. // @@ -674,7 +679,11 @@ func (a *App) DeleteEphemeralPost(rctx request.CTX, userID, postID string) { a.Publish(message) } -func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { +func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, updatePostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) { + if updatePostOptions == nil { + updatePostOptions = model.DefaultUpdatePostOptions() + } + receivedUpdatedPost.SanitizeProps() postLists, nErr := a.Srv().Store().Post().Get(context.Background(), receivedUpdatedPost.Id, model.GetPostsOptions{}, "", a.Config().GetSanitizeOptions()) @@ -725,11 +734,17 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, safeUpd newPost.Hashtags, _ = model.ParseHashtags(receivedUpdatedPost.Message) } - if !safeUpdate { + if !updatePostOptions.SafeUpdate { newPost.IsPinned = receivedUpdatedPost.IsPinned newPost.HasReactions = receivedUpdatedPost.HasReactions - newPost.FileIds = receivedUpdatedPost.FileIds newPost.SetProps(receivedUpdatedPost.GetProps()) + + var fileIds []string + fileIds, appErr = a.processPostFileChanges(c, receivedUpdatedPost, oldPost, updatePostOptions) + if appErr != nil { + return nil, appErr + } + newPost.FileIds = fileIds } // Avoid deep-equal checks if EditAt was already modified through message change @@ -928,7 +943,11 @@ func (a *App) setupBroadcastHookForPermalink(rctx request.CTX, post *model.Post, return nil } -func (a *App) PatchPost(c request.CTX, postID string, patch *model.PostPatch) (*model.Post, *model.AppError) { +func (a *App) PatchPost(c request.CTX, postID string, patch *model.PostPatch, patchPostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) { + if patchPostOptions == nil { + patchPostOptions = model.DefaultUpdatePostOptions() + } + post, err := a.GetSinglePost(c, postID, false) if err != nil { return nil, err @@ -950,7 +969,8 @@ func (a *App) PatchPost(c request.CTX, postID string, patch *model.PostPatch) (* post.Patch(patch) - updatedPost, err := a.UpdatePost(c, post, false) + patchPostOptions.SafeUpdate = false + updatedPost, err := a.UpdatePost(c, post, patchPostOptions) if err != nil { return nil, err } @@ -2144,9 +2164,30 @@ func (a *App) GetEditHistoryForPost(postID string) ([]*model.Post, *model.AppErr } } + if appErr := a.populateEditHistoryFileMetadata(posts); appErr != nil { + return nil, appErr + } + return posts, nil } +func (a *App) populateEditHistoryFileMetadata(editHistoryPosts []*model.Post) *model.AppError { + for _, post := range editHistoryPosts { + fileInfos, err := a.Srv().Store().FileInfo().GetByIds(post.FileIds, true, true) + if err != nil { + return model.NewAppError("app.populateEditHistoryFileMetadata", "app.file_info.get_by_ids.app_error", map[string]any{"post_id": post.Id}, "", http.StatusInternalServerError).Wrap(err) + } + + if post.Metadata == nil { + post.Metadata = &model.PostMetadata{} + } + + post.Metadata.Files = fileInfos + } + + return nil +} + func (a *App) SetPostReminder(rctx request.CTX, postID, userID string, targetTime int64) *model.AppError { // Store the reminder in the DB reminder := &model.PostReminder{ diff --git a/server/channels/app/post_file_change.go b/server/channels/app/post_file_change.go new file mode 100644 index 0000000000..504c11f816 --- /dev/null +++ b/server/channels/app/post_file_change.go @@ -0,0 +1,70 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/public/utils" +) + +func (a *App) processPostFileChanges(rctx request.CTX, newPost, oldPost *model.Post, updatePostOptions *model.UpdatePostOptions) (model.StringArray, *model.AppError) { + newFileIDs := model.RemoveDuplicateStrings(newPost.FileIds) + oldFileIDs := model.RemoveDuplicateStrings(oldPost.FileIds) + + addedFileIDs, removedFileIDs, unchangedFileIDs := utils.FindExclusives(newFileIDs, oldFileIDs) + + if len(addedFileIDs) > 0 { + if updatePostOptions != nil && updatePostOptions.IsRestorePost { + err := a.Srv().Store().FileInfo().RestoreForPostByIds(rctx, newPost.Id, addedFileIDs) + if err != nil { + return nil, model.NewAppError("app.processPostFileChanges", "app.file_info.undelete_for_post_ids.app_error", map[string]any{"post_id": newPost.Id}, "", 0).Wrap(err) + } + } else { + a.attachNewFilesToPost(rctx, newPost, addedFileIDs, unchangedFileIDs) + } + } + + if len(removedFileIDs) > 0 { + if appErr := a.detachFilesFromPost(rctx, newPost.Id, removedFileIDs); appErr != nil { + return nil, appErr + } + } + + filesChanged := len(addedFileIDs) > 0 || len(removedFileIDs) > 0 + if filesChanged { + // if files were modified, invalidate the file metadata cache for the post + // so that the updated file metadata can be returned. + a.Srv().Store().FileInfo().InvalidateFileInfosForPostCache(newPost.Id, false) + } + + return newPost.FileIds, nil +} + +func (a *App) attachNewFilesToPost(rctx request.CTX, post *model.Post, addedFileIDs, unchangedFileIDs []string) { + // for newly added files, we need to attach them to the post + + // intentionally using UserID from session instead of post.UserID + // to support admin attaching files in someone else's post. + // Admins can edit other's posts, including message, removing existing files, + // and attaching new files. + // When an admin uploads new files, they are associated with their user ID. So, when attaching + // these file to a post, we need to search for their FileInfo entry + // by the admin's user ID and not the post author's user ID. + userId := rctx.Session().UserId + attachedFileIDs := a.attachFileIDsToPost(rctx, post.Id, post.ChannelId, userId, addedFileIDs) + if len(attachedFileIDs) != len(addedFileIDs) { + // if not all files could be attached, the final list of files + // is those that could be attached + the existing, unchanged files + post.FileIds = append(attachedFileIDs, unchangedFileIDs...) + } +} + +func (a *App) detachFilesFromPost(rctx request.CTX, postId string, removedFileIDs []string) *model.AppError { + if err := a.Srv().Store().FileInfo().DeleteForPostByIds(rctx, postId, removedFileIDs); err != nil { + return model.NewAppError("app.detachFilesFromPost", "app.file_info.delete_for_post_ids.app_error", map[string]any{"post_id": postId}, "", 0).Wrap(err) + } + + return nil +} diff --git a/server/channels/app/post_file_change_test.go b/server/channels/app/post_file_change_test.go new file mode 100644 index 0000000000..4c0fe36eb1 --- /dev/null +++ b/server/channels/app/post_file_change_test.go @@ -0,0 +1,274 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/stretchr/testify/require" +) + +func TestProcessPostFileChanges(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("no files", func(t *testing.T) { + oldPost := &model.Post{FileIds: []string{}} + newPost := &model.Post{FileIds: []string{}} + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 0, len(fileIds)) + }) + + t.Run("have files but nothing changed", func(t *testing.T) { + oldPost := &model.Post{FileIds: []string{"file_id_1", "file_id_2"}} + newPost := &model.Post{FileIds: []string{"file_id_1", "file_id_2"}} + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 2, len(fileIds)) + }) + + t.Run("one file deleted", func(t *testing.T) { + postId := model.NewId() + fileInfo1 := th.CreateFileInfo(th.BasicUser.Id, postId, th.BasicChannel.Id) + fileInfo2 := th.CreateFileInfo(th.BasicUser.Id, postId, th.BasicChannel.Id) + + oldPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, + } + + newPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id}, + } + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 1, len(fileIds)) + require.Equal(t, fileInfo1.Id, fileIds[0]) + + // verify file2 was soft deleted + updatedFileInfos, err := th.App.Srv().Store().FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + require.Equal(t, 2, len(updatedFileInfos)) + + for _, fileInfo := range updatedFileInfos { + if fileInfo.Id == fileInfo1.Id { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } else if fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Fail(t, "unexpected file info") + } + } + }) + + t.Run("one file added", func(t *testing.T) { + postId := model.NewId() + fileInfo1 := th.CreateFileInfo(th.BasicUser.Id, postId, th.BasicChannel.Id) + fileInfo2 := th.CreateFileInfo(th.BasicUser.Id, "", th.BasicChannel.Id) + + oldPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id}, + } + + newPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, + } + + th.Context.Session().UserId = th.BasicUser.Id + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 2, len(fileIds)) + require.Contains(t, fileIds, fileInfo1.Id) + require.Contains(t, fileIds, fileInfo2.Id) + + // verify file2 is attached to the post + updatedFileInfo2, err := th.App.Srv().Store().FileInfo().Get(fileInfo2.Id) + require.NoError(t, err) + require.Equal(t, postId, updatedFileInfo2.PostId) + }) + + t.Run("all files removed", func(t *testing.T) { + postId := model.NewId() + fileInfo1 := th.CreateFileInfo(th.BasicUser.Id, postId, th.BasicChannel.Id) + fileInfo2 := th.CreateFileInfo(th.BasicUser.Id, postId, th.BasicChannel.Id) + + oldPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, + } + + newPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{}, + } + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 0, len(fileIds)) + + // verify file2 was soft deleted + updatedFileInfos, err := th.App.Srv().Store().FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + require.Equal(t, 2, len(updatedFileInfos)) + + for _, fileInfo := range updatedFileInfos { + if fileInfo.Id == fileInfo1.Id || fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Fail(t, "unexpected file info") + } + } + }) + + t.Run("files added when no files existed", func(t *testing.T) { + fileInfo1 := th.CreateFileInfo(th.BasicUser.Id, "", th.BasicChannel.Id) + fileInfo2 := th.CreateFileInfo(th.BasicUser.Id, "", th.BasicChannel.Id) + + postId := model.NewId() + oldPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{}, + } + + newPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, + } + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 2, len(fileIds)) + require.Contains(t, fileIds, fileInfo1.Id) + require.Contains(t, fileIds, fileInfo2.Id) + + updatedFileInfo1, err := th.App.Srv().Store().FileInfo().Get(fileInfo2.Id) + require.NoError(t, err) + require.Equal(t, postId, updatedFileInfo1.PostId) + + updatedFileInfo2, err := th.App.Srv().Store().FileInfo().Get(fileInfo2.Id) + require.NoError(t, err) + require.Equal(t, postId, updatedFileInfo2.PostId) + }) + + t.Run("other post's attached file added", func(t *testing.T) { + postId := model.NewId() + fileInfo1 := th.CreateFileInfo(th.BasicUser.Id, postId, th.BasicChannel.Id) + fileInfo2 := th.CreateFileInfo(th.BasicUser.Id, model.NewId(), th.BasicChannel.Id) + + oldPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id}, + } + + newPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, + } + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 1, len(fileIds)) + require.Equal(t, fileInfo1.Id, fileIds[0]) + + // verify file2 is attached to the post + updatedFileInfo2, err := th.App.Srv().Store().FileInfo().Get(fileInfo2.Id) + require.NoError(t, err) + require.NotEqual(t, postId, updatedFileInfo2.PostId) + }) + + t.Run("when admin adds a file to other user's post", func(t *testing.T) { + postId := model.NewId() + + // admin uploads the files + fileInfo1 := th.CreateFileInfo(th.SystemAdminUser.Id, "", th.BasicChannel.Id) + fileInfo2 := th.CreateFileInfo(th.SystemAdminUser.Id, "", th.BasicChannel.Id) + + // basic user's post + oldPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + } + + newPost := &model.Post{ + Id: postId, + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "Message", + CreateAt: model.GetMillis() - 10000, + FileIds: []string{fileInfo1.Id, fileInfo2.Id}, // admin attaching two files + } + + // admin's session + th.Context.Session().UserId = th.SystemAdminUser.Id + + fileIds, appErr := th.App.processPostFileChanges(th.Context, newPost, oldPost, nil) + require.Nil(t, appErr) + require.Equal(t, 2, len(fileIds)) + require.Contains(t, fileIds, fileInfo1.Id) + require.Contains(t, fileIds, fileInfo2.Id) + + // verify files are attached to the post and still belong tyo the admin + updatedFileInfo1, err := th.App.Srv().Store().FileInfo().Get(fileInfo1.Id) + require.NoError(t, err) + require.Equal(t, postId, updatedFileInfo1.PostId) + require.Equal(t, th.SystemAdminUser.Id, updatedFileInfo1.CreatorId) + + updatedFileInfo2, err := th.App.Srv().Store().FileInfo().Get(fileInfo2.Id) + require.NoError(t, err) + require.Equal(t, postId, updatedFileInfo2.PostId) + require.Equal(t, th.SystemAdminUser.Id, updatedFileInfo2.CreatorId) + }) +} diff --git a/server/channels/app/post_restore.go b/server/channels/app/post_restore.go new file mode 100644 index 0000000000..c93443a5bc --- /dev/null +++ b/server/channels/app/post_restore.go @@ -0,0 +1,58 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "net/http" + + "github.com/mattermost/mattermost/server/v8/channels/store" + "github.com/pkg/errors" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" +) + +func (a *App) RestorePostVersion(c request.CTX, userID, postID, restoreVersionID string) (*model.Post, *model.AppError) { + toRestorePostVersion, err := a.Srv().Store().Post().GetSingle(c, restoreVersionID, true) + if err != nil { + var statusCode int + var notFoundErr *store.ErrNotFound + switch { + case errors.As(err, ¬FoundErr): + statusCode = http.StatusNotFound + default: + statusCode = http.StatusInternalServerError + } + + return nil, model.NewAppError("RestorePostVersion", "app.post.restore_post_version.get_single.app_error", nil, err.Error(), statusCode) + } + + // restoreVersionID needs to be an old version of postID + // this is only a safeguard and this should never happen in practice. + if toRestorePostVersion.OriginalId != postID { + return nil, model.NewAppError("RestorePostVersion", "app.post.restore_post_version.not_an_history_item.app_error", nil, "", http.StatusBadRequest) + } + + // the user needs to be the author of the post + // this is only a safeguard and this should never happen in practice. + if toRestorePostVersion.UserId != userID { + return nil, model.NewAppError("RestorePostVersion", "app.post.restore_post_version.not_allowed.app_error", nil, "", http.StatusForbidden) + } + + // the old version of post needs to be a deleted post + if toRestorePostVersion.DeleteAt == 0 { + return nil, model.NewAppError("RestorePostVersion", "app.post.restore_post_version.not_valid_post_history_item.app_error", nil, "", http.StatusBadRequest) + } + + postPatch := &model.PostPatch{ + Message: &toRestorePostVersion.Message, + FileIds: &toRestorePostVersion.FileIds, + } + + patchPostOptions := &model.UpdatePostOptions{ + IsRestorePost: true, + } + + return a.PatchPost(c, postID, postPatch, patchPostOptions) +} diff --git a/server/channels/app/post_restore_test.go b/server/channels/app/post_restore_test.go new file mode 100644 index 0000000000..b4b111e17b --- /dev/null +++ b/server/channels/app/post_restore_test.go @@ -0,0 +1,180 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "net/http" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/stretchr/testify/require" +) + +func TestRestorePostVersion(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("is able to restore a post version", func(t *testing.T) { + post := th.CreatePost(th.BasicChannel, func(p *model.Post) { + p.Message = "original message" + }) + th.PostPatch(post, "new message 2") + th.PostPatch(post, "new message 3") + + // verify post's state + fetchedPost, err := th.App.Srv().Store().Post().GetSingle(th.Context, post.Id, true) + require.NoError(t, err) + require.Equal(t, "new message 3", fetchedPost.Message) + + editHistory, appErr := th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, appErr) + require.Equal(t, 2, len(editHistory)) + require.Equal(t, "new message 2", editHistory[0].Message) + + // now we'll restore a post version + restoredPost, appErr := th.App.RestorePostVersion(th.Context, th.BasicUser.Id, post.Id, editHistory[0].Id) + require.Nil(t, appErr) + require.Equal(t, "new message 2", restoredPost.Message) + + // verify from database + fetchedPost, err = th.App.Srv().Store().Post().GetSingle(th.Context, post.Id, true) + require.NoError(t, err) + require.Equal(t, "new message 2", fetchedPost.Message) + + // verify that we now have 3 items in post's edit history + editHistory, appErr = th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, appErr) + require.Equal(t, 3, len(editHistory)) + require.Equal(t, "new message 3", editHistory[0].Message) + require.Equal(t, "new message 2", editHistory[1].Message) + require.Equal(t, "original message", editHistory[2].Message) + }) + + t.Run("is able to restore a post version including its files", func(t *testing.T) { + fileBytes := []byte("file contents") + fileInfo, appErr := th.App.UploadFile(th.Context, fileBytes, th.BasicChannel.Id, "file.txt") + require.Nil(t, appErr) + + post := th.CreatePost(th.BasicChannel, func(p *model.Post) { + p.FileIds = []string{fileInfo.Id} + p.Message = "original message" + }) + + // this update removes all files + th.PostPatch(post, "new message 2", func(p *model.PostPatch) { + p.FileIds = &model.StringArray{} + }) + // this update only changes the message + th.PostPatch(post, "new message 3") + + // verify post's state + fetchedPost, err := th.App.Srv().Store().Post().GetSingle(th.Context, post.Id, true) + require.NoError(t, err) + require.Equal(t, "new message 3", fetchedPost.Message) + require.Empty(t, fetchedPost.FileIds) + + editHistory, appErr := th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, appErr) + require.Equal(t, 2, len(editHistory)) + require.Equal(t, "new message 2", editHistory[0].Message) + require.Equal(t, 0, len(editHistory[0].FileIds)) + + require.Equal(t, "original message", editHistory[1].Message) + require.Equal(t, 1, len(editHistory[1].FileIds)) + + // now we'll restore a post version + restoredPost, appErr := th.App.RestorePostVersion(th.Context, th.BasicUser.Id, post.Id, editHistory[1].Id) + require.Nil(t, appErr) + require.Equal(t, "original message", restoredPost.Message) + require.Equal(t, 1, len(restoredPost.FileIds)) + + // verify from database + fetchedPost, err = th.App.Srv().Store().Post().GetSingle(th.Context, post.Id, true) + require.NoError(t, err) + require.Equal(t, "original message", fetchedPost.Message) + require.Equal(t, 1, len(fetchedPost.FileIds)) + + // verify edit history\ + editHistory, appErr = th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, appErr) + require.Equal(t, 3, len(editHistory)) + + require.Equal(t, "new message 3", editHistory[0].Message) + require.Equal(t, 0, len(editHistory[0].FileIds)) + + require.Equal(t, "new message 2", editHistory[1].Message) + require.Equal(t, 0, len(editHistory[1].FileIds)) + + require.Equal(t, "original message", editHistory[2].Message) + require.Equal(t, 1, len(editHistory[2].FileIds)) + }) + + t.Run("should return an error if trying to restore a post that is not in any edit history", func(t *testing.T) { + post := th.CreatePost(th.BasicChannel, func(p *model.Post) { + p.Message = "original message" + }) + th.PostPatch(post, "new message 2") + th.PostPatch(post, "new message 3") + + // verify post's state + fetchedPost, err := th.App.Srv().Store().Post().GetSingle(th.Context, post.Id, true) + require.NoError(t, err) + require.Equal(t, "new message 3", fetchedPost.Message) + + // now we'll restore a post version + otherPost := th.CreatePost(th.BasicChannel) + restoredPost, appErr := th.App.RestorePostVersion(th.Context, th.BasicUser.Id, post.Id, otherPost.Id) + require.NotNil(t, appErr) + require.Equal(t, http.StatusBadRequest, appErr.StatusCode) + require.Equal(t, "app.post.restore_post_version.not_an_history_item.app_error", appErr.Id) + require.Nil(t, restoredPost) + + // verify from database that the post wasn't modified + fetchedPost, err = th.App.Srv().Store().Post().GetSingle(th.Context, post.Id, true) + require.NoError(t, err) + require.Equal(t, "new message 3", fetchedPost.Message) + }) + + t.Run("should return an error if the post does not exist", func(t *testing.T) { + restoredPost, appErr := th.App.RestorePostVersion(th.Context, th.BasicUser.Id, model.NewId(), model.NewId()) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) + require.Equal(t, "app.post.restore_post_version.get_single.app_error", appErr.Id) + require.Nil(t, restoredPost) + }) + + t.Run("should return an error if the restore post does not exist", func(t *testing.T) { + post := th.CreatePost(th.BasicChannel) + + // now we'll restore a post version + invalidRestorePostIUd := model.NewId() + restoredPost, appErr := th.App.RestorePostVersion(th.Context, th.BasicUser.Id, post.Id, invalidRestorePostIUd) + require.NotNil(t, appErr) + require.Equal(t, http.StatusNotFound, appErr.StatusCode) + require.Equal(t, "app.post.restore_post_version.get_single.app_error", appErr.Id) + require.Nil(t, restoredPost) + }) + + t.Run("should return an error if trying to restore a post that is in some other posts edit history", func(t *testing.T) { + post := th.CreatePost(th.BasicChannel) + th.PostPatch(post, "new message 2") + + otherPost := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.Message = "other post original message" + }) + th.PostPatch(otherPost, "other post new message 2") + + otherPostEditHistory, appErr := th.App.GetEditHistoryForPost(otherPost.Id) + require.Nil(t, appErr) + require.Equal(t, 1, len(otherPostEditHistory)) + require.Equal(t, "other post original message", otherPostEditHistory[0].Message) + + // we'll specify post's ID and other post's version ID, his should fail + restoredPost, appErr := th.App.RestorePostVersion(th.Context, th.BasicUser.Id, post.Id, otherPostEditHistory[0].Id) + require.NotNil(t, appErr) + require.Equal(t, "app.post.restore_post_version.not_an_history_item.app_error", appErr.Id) + require.Equal(t, http.StatusBadRequest, appErr.StatusCode) + require.Nil(t, restoredPost) + }) +} diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index 3f12000da2..4cb9b9541f 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -269,7 +269,7 @@ func TestUpdatePostEditAt(t *testing.T) { post := th.BasicPost.Clone() post.IsPinned = true - saved, err := th.App.UpdatePost(th.Context, post, true) + saved, err := th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err) assert.Equal(t, saved.EditAt, post.EditAt, "shouldn't have updated post.EditAt when pinning post") post = saved.Clone() @@ -277,7 +277,7 @@ func TestUpdatePostEditAt(t *testing.T) { time.Sleep(time.Millisecond * 100) post.Message = model.NewId() - saved, err = th.App.UpdatePost(th.Context, post, true) + saved, err = th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err) assert.NotEqual(t, saved.EditAt, post.EditAt, "should have updated post.EditAt when updating post message") @@ -295,7 +295,7 @@ func TestUpdatePostTimeLimit(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.PostEditTimeLimit = -1 }) - _, err := th.App.UpdatePost(th.Context, post, true) + _, err := th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err) th.App.UpdateConfig(func(cfg *model.Config) { @@ -303,14 +303,14 @@ func TestUpdatePostTimeLimit(t *testing.T) { }) post.Message = model.NewId() - _, err = th.App.UpdatePost(th.Context, post, true) + _, err = th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err, "should allow you to edit the post") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.PostEditTimeLimit = 1 }) post.Message = model.NewId() - _, err = th.App.UpdatePost(th.Context, post, true) + _, err = th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err, "should allow you to edit an old post because the time check is applied above in the call hierarchy") th.App.UpdateConfig(func(cfg *model.Config) { @@ -326,7 +326,7 @@ func TestUpdatePostInArchivedChannel(t *testing.T) { post := th.CreatePost(archivedChannel) th.App.DeleteChannel(th.Context, archivedChannel, "") - _, err := th.App.UpdatePost(th.Context, post, true) + _, err := th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.NotNil(t, err) require.Equal(t, "api.post.update_post.can_not_update_post_in_deleted.error", err.Id) } @@ -472,7 +472,7 @@ func TestUpdatePostPluginHooks(t *testing.T) { require.Nil(t, err) post.Message = "new message" - updatedPost, err := th.App.UpdatePost(th.Context, post, false) + updatedPost, err := th.App.UpdatePost(th.Context, post, nil) require.Nil(t, updatedPost) require.NotNil(t, err) require.Equal(t, "Post rejected by plugin. rejected", err.Id) @@ -539,7 +539,7 @@ func TestUpdatePostPluginHooks(t *testing.T) { require.Nil(t, err) post.Message = "new message" - updatedPost, err := th.App.UpdatePost(th.Context, post, false) + updatedPost, err := th.App.UpdatePost(th.Context, post, nil) require.Nil(t, err) require.NotNil(t, updatedPost) require.Equal(t, "2 new message 1", updatedPost.Message) @@ -591,7 +591,7 @@ func TestPostChannelMentions(t *testing.T) { }, post.GetProp("channel_mentions")) post.Message = fmt.Sprintf("goodbye, ~%v!", channelToMention2.Name) - result, err := th.App.UpdatePost(th.Context, post, false) + result, err := th.App.UpdatePost(th.Context, post, nil) require.Nil(t, err) assert.Equal(t, map[string]any{ "mention-test2": map[string]any{ @@ -601,7 +601,7 @@ func TestPostChannelMentions(t *testing.T) { }, result.GetProp("channel_mentions")) result.Message = "no more mentions!" - result, err = th.App.UpdatePost(th.Context, result, false) + result, err = th.App.UpdatePost(th.Context, result, nil) require.Nil(t, err) assert.Nil(t, result.GetProp("channel_mentions")) } @@ -1123,7 +1123,7 @@ func TestCreatePost(t *testing.T) { go func() { defer wg.Done() post := previewPost.Clone() - th.App.UpdatePost(th.Context, post, false) + th.App.UpdatePost(th.Context, post, nil) }() } @@ -1194,7 +1194,7 @@ func TestPatchPost(t *testing.T) { Message: model.NewPointer("![image](" + imageURL + ")"), } - rpost, err = th.App.PatchPost(th.Context, rpost.Id, patch) + rpost, err = th.App.PatchPost(th.Context, rpost.Id, patch, nil) require.Nil(t, err) assert.Equal(t, "![image]("+proxiedImageURL+")", rpost.Message) }) @@ -1217,13 +1217,13 @@ func TestPatchPost(t *testing.T) { t.Run("Does not set prop when user has USE_CHANNEL_MENTIONS", func(t *testing.T) { patchWithNoMention := &model.PostPatch{Message: model.NewPointer("This patch has no channel mention")} - rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithNoMention) + rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithNoMention, nil) require.Nil(t, err) assert.Equal(t, rpost.GetProps(), model.StringInterface{}) patchWithMention := &model.PostPatch{Message: model.NewPointer("This patch has a mention now @here")} - rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithMention) + rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithMention, nil) require.Nil(t, err) assert.Equal(t, rpost.GetProps(), model.StringInterface{}) }) @@ -1233,13 +1233,13 @@ func TestPatchPost(t *testing.T) { th.RemovePermissionFromRole(model.PermissionUseChannelMentions.Id, model.ChannelAdminRoleId) patchWithNoMention := &model.PostPatch{Message: model.NewPointer("This patch still does not have a mention")} - rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithNoMention) + rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithNoMention, nil) require.Nil(t, err) assert.Equal(t, rpost.GetProps(), model.StringInterface{}) patchWithMention := &model.PostPatch{Message: model.NewPointer("This patch has a mention now @here")} - rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithMention) + rpost, err = th.App.PatchPost(th.Context, rpost.Id, patchWithMention, nil) require.Nil(t, err) assert.Equal(t, rpost.GetProp(model.PostPropsMentionHighlightDisabled), true) @@ -1433,7 +1433,7 @@ func TestPatchPostInArchivedChannel(t *testing.T) { post := th.CreatePost(archivedChannel) th.App.DeleteChannel(th.Context, archivedChannel, "") - _, err := th.App.PatchPost(th.Context, post.Id, &model.PostPatch{IsPinned: model.NewPointer(true)}) + _, err := th.App.PatchPost(th.Context, post.Id, &model.PostPatch{IsPinned: model.NewPointer(true)}, nil) require.NotNil(t, err) require.Equal(t, "api.post.patch_post.can_not_update_post_in_deleted.error", err.Id) } @@ -1539,7 +1539,7 @@ func TestUpdatePost(t *testing.T) { post.Id = rpost.Id post.Message = "![image](" + imageURL + ")" - rpost, err = th.App.UpdatePost(th.Context, post, false) + rpost, err = th.App.UpdatePost(th.Context, post, nil) require.Nil(t, err) assert.Equal(t, "![image]("+proxiedImageURL+")", rpost.Message) }) @@ -1578,7 +1578,7 @@ func TestUpdatePost(t *testing.T) { assert.Equal(t, model.StringInterface{}, testPost.GetProps()) testPost.Message = permalink - testPost, err = th.App.UpdatePost(th.Context, testPost, false) + testPost, err = th.App.UpdatePost(th.Context, testPost, nil) require.Nil(t, err) assert.Equal(t, model.StringInterface{model.PostPropsPreviewedPost: referencedPost.Id}, testPost.GetProps()) }) @@ -1639,7 +1639,7 @@ func TestUpdatePost(t *testing.T) { permalink := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) previewPost.Message = permalink - previewPost, err = th.App.UpdatePost(th.Context, previewPost, false) + previewPost, err = th.App.UpdatePost(th.Context, previewPost, nil) require.Nil(t, err) require.Len(t, previewPost.Metadata.Embeds, testCase.Length) @@ -3126,7 +3126,7 @@ func TestSharedChannelSyncForPostActions(t *testing.T) { }, channel, model.CreatePostFlags{SetOnline: true}) require.Nil(t, err, "Creating a post should not error") - _, err = th.App.UpdatePost(th.Context, post, true) + _, err = th.App.UpdatePost(th.Context, post, &model.UpdatePostOptions{SafeUpdate: true}) require.Nil(t, err, "Updating a post should not error") require.Len(t, sharedChannelService.channelNotifications, 2) @@ -3410,7 +3410,6 @@ func TestComputeLastAccessiblePostTime(t *testing.T) { } func TestGetEditHistoryForPost(t *testing.T) { - t.Skip("This needs fixing, OriginalId seems to be empty for all posts") th := Setup(t).InitBasic() defer th.TearDown() @@ -3427,7 +3426,7 @@ func TestGetEditHistoryForPost(t *testing.T) { patch := &model.PostPatch{ Message: model.NewPointer("new message edited"), } - _, err1 := th.App.PatchPost(th.Context, rpost.Id, patch) + _, err1 := th.App.PatchPost(th.Context, rpost.Id, patch, nil) require.Nil(t, err1) // update the post message again @@ -3435,14 +3434,13 @@ func TestGetEditHistoryForPost(t *testing.T) { Message: model.NewPointer("new message edited again"), } - _, err2 := th.App.PatchPost(th.Context, rpost.Id, patch) + _, err2 := th.App.PatchPost(th.Context, rpost.Id, patch, nil) require.Nil(t, err2) - // get the edit history - edits, err := th.App.GetEditHistoryForPost(post.Id) - require.Nil(t, err) - t.Run("should return the edit history", func(t *testing.T) { + edits, err := th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, err) + require.Len(t, edits, 2) require.Equal(t, "new message edited", edits[0].Message) require.Equal(t, "new message", edits[1].Message) @@ -3453,6 +3451,103 @@ func TestGetEditHistoryForPost(t *testing.T) { require.NotNil(t, err) require.Empty(t, edits) }) + + t.Run("edit history should contain file metadata", func(t *testing.T) { + fileBytes := []byte("file contents") + fileInfo, err := th.App.UploadFile(th.Context, fileBytes, th.BasicChannel.Id, "file.txt") + require.Nil(t, err) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "new message", + UserId: th.BasicUser.Id, + FileIds: model.StringArray{fileInfo.Id}, + } + + _, err = th.App.CreatePost(th.Context, post, th.BasicChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, err) + + patch := &model.PostPatch{ + Message: model.NewPointer("new message edited"), + } + _, appErr := th.App.PatchPost(th.Context, post.Id, patch, nil) + require.Nil(t, appErr) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message edited 2"), + } + _, appErr = th.App.PatchPost(th.Context, post.Id, patch, nil) + require.Nil(t, appErr) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message edited 3"), + } + _, appErr = th.App.PatchPost(th.Context, post.Id, patch, nil) + require.Nil(t, appErr) + + edits, err := th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, err) + + require.Len(t, edits, 3) + + for _, edit := range edits { + require.Len(t, edit.FileIds, 1) + require.Equal(t, fileInfo.Id, edit.FileIds[0]) + require.Len(t, edit.Metadata.Files, 1) + require.Equal(t, fileInfo.Id, edit.Metadata.Files[0].Id) + } + }) + + t.Run("edit history should contain file metadata even if the file info is deleted", func(t *testing.T) { + fileBytes := []byte("file contents") + fileInfo, appErr := th.App.UploadFile(th.Context, fileBytes, th.BasicChannel.Id, "file.txt") + require.Nil(t, appErr) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "new message", + UserId: th.BasicUser.Id, + FileIds: model.StringArray{fileInfo.Id}, + } + + _, appErr = th.App.CreatePost(th.Context, post, th.BasicChannel, model.CreatePostFlags{SetOnline: true}) + require.Nil(t, appErr) + + patch := &model.PostPatch{ + Message: model.NewPointer("new message edited"), + } + _, appErr = th.App.PatchPost(th.Context, post.Id, patch, nil) + require.Nil(t, appErr) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message edited 2"), + } + _, appErr = th.App.PatchPost(th.Context, post.Id, patch, nil) + require.Nil(t, appErr) + + patch = &model.PostPatch{ + Message: model.NewPointer("new message edited 3"), + } + _, appErr = th.App.PatchPost(th.Context, post.Id, patch, nil) + require.Nil(t, appErr) + + // now delete the file info, and it should still be include in edit history metadata + _, err := th.App.Srv().Store().FileInfo().DeleteForPost(th.Context, post.Id) + require.NoError(t, err) + + edits, appErr := th.App.GetEditHistoryForPost(post.Id) + require.Nil(t, appErr) + + require.Len(t, edits, 3) + + for _, edit := range edits { + require.Len(t, edit.FileIds, 1) + require.Equal(t, fileInfo.Id, edit.FileIds[0]) + require.Len(t, edit.Metadata.Files, 1) + require.Equal(t, fileInfo.Id, edit.Metadata.Files[0].Id) + require.Greater(t, edit.Metadata.Files[0].DeleteAt, int64(0)) + } + }) } func TestCopyWranglerPostlist(t *testing.T) { @@ -3731,3 +3826,121 @@ func TestSendTestMessage(t *testing.T) { assert.NotEmpty(t, post.GetProp(model.PostPropsForceNotification)) }) } + +func TestPopulateEditHistoryFileMetadata(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + t.Run("should populate file metadata for all posts", func(t *testing.T) { + fileInfo1, err := th.App.Srv().Store().FileInfo().Save(th.Context, + &model.FileInfo{ + CreatorId: th.BasicUser.Id, + Path: "path.txt", + }) + require.NoError(t, err) + + fileInfo2, err := th.App.Srv().Store().FileInfo().Save(th.Context, + &model.FileInfo{ + CreatorId: th.BasicUser.Id, + Path: "path.txt", + }) + require.NoError(t, err) + + post1 := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.FileIds = model.StringArray{fileInfo1.Id} + }) + + post2 := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.FileIds = model.StringArray{fileInfo2.Id} + }) + + appErr := th.App.populateEditHistoryFileMetadata([]*model.Post{post1, post2}) + require.Nil(t, appErr) + + require.Len(t, post1.Metadata.Files, 1) + require.Equal(t, fileInfo1.Id, post1.Metadata.Files[0].Id) + + require.Len(t, post2.Metadata.Files, 1) + require.Equal(t, fileInfo2.Id, post2.Metadata.Files[0].Id) + }) + + t.Run("should populate file metadata even for deleted posts", func(t *testing.T) { + fileInfo1, err := th.App.Srv().Store().FileInfo().Save(th.Context, + &model.FileInfo{ + CreatorId: th.BasicUser.Id, + Path: "path.txt", + }) + require.NoError(t, err) + + fileInfo2, err := th.App.Srv().Store().FileInfo().Save(th.Context, + &model.FileInfo{ + CreatorId: th.BasicUser.Id, + Path: "path.txt", + }) + require.NoError(t, err) + + post1 := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.FileIds = model.StringArray{fileInfo1.Id} + }) + + post2 := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.FileIds = model.StringArray{fileInfo2.Id} + }) + + _, appErr := th.App.DeletePost(th.Context, post1.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + _, appErr = th.App.DeletePost(th.Context, post2.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + appErr = th.App.populateEditHistoryFileMetadata([]*model.Post{post1, post2}) + require.Nil(t, appErr) + + require.Len(t, post1.Metadata.Files, 1) + require.Equal(t, fileInfo1.Id, post1.Metadata.Files[0].Id) + + require.Len(t, post2.Metadata.Files, 1) + require.Equal(t, fileInfo2.Id, post2.Metadata.Files[0].Id) + }) + + t.Run("should populate file metadata even for deleted fileInfos", func(t *testing.T) { + fileInfo1, err := th.App.Srv().Store().FileInfo().Save(th.Context, + &model.FileInfo{ + CreatorId: th.BasicUser.Id, + Path: "path.txt", + }) + require.NoError(t, err) + + fileInfo2, err := th.App.Srv().Store().FileInfo().Save(th.Context, + &model.FileInfo{ + CreatorId: th.BasicUser.Id, + Path: "path.txt", + }) + require.NoError(t, err) + + post1 := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.FileIds = model.StringArray{fileInfo1.Id} + }) + + post2 := th.CreatePost(th.BasicChannel, func(post *model.Post) { + post.FileIds = model.StringArray{fileInfo2.Id} + }) + + _, err = th.App.Srv().Store().FileInfo().DeleteForPost(th.Context, post1.Id) + require.NoError(t, err) + + _, err = th.App.Srv().Store().FileInfo().DeleteForPost(th.Context, post2.Id) + require.NoError(t, err) + + appErr := th.App.populateEditHistoryFileMetadata([]*model.Post{post1, post2}) + require.Nil(t, appErr) + + require.Len(t, post1.Metadata.Files, 1) + require.Equal(t, fileInfo1.Id, post1.Metadata.Files[0].Id) + require.Greater(t, post1.Metadata.Files[0].DeleteAt, int64(0)) + + require.Len(t, post2.Metadata.Files, 1) + require.Equal(t, fileInfo2.Id, post2.Metadata.Files[0].Id) + require.Greater(t, post2.Metadata.Files[0].DeleteAt, int64(0)) + }) +} diff --git a/server/channels/store/localcachelayer/file_info_layer.go b/server/channels/store/localcachelayer/file_info_layer.go index 9081cb6663..3c5623eb17 100644 --- a/server/channels/store/localcachelayer/file_info_layer.go +++ b/server/channels/store/localcachelayer/file_info_layer.go @@ -5,6 +5,7 @@ package localcachelayer import ( "bytes" + "fmt" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/channels/store" @@ -50,6 +51,41 @@ func (s LocalCacheFileInfoStore) GetForPost(postId string, readFromMaster, inclu return fileInfos, nil } +func (s LocalCacheFileInfoStore) GetByIds(ids []string, includeDeleted, allowFromCache bool) ([]*model.FileInfo, error) { + if !allowFromCache { + return s.FileInfoStore.GetByIds(ids, includeDeleted, allowFromCache) + } + + var fileIdsToFetch []string + var fileInfos []*model.FileInfo + + for _, fileId := range ids { + cacheKey := fmt.Sprintf("%s_%t", fileId, includeDeleted) + + var fileInfo *model.FileInfo + if err := s.rootStore.doStandardReadCache(s.rootStore.fileInfoCache, cacheKey, &fileInfo); err == nil { + fileInfos = append(fileInfos, fileInfo) + } else { + fileIdsToFetch = append(fileIdsToFetch, fileId) + } + } + + if len(fileIdsToFetch) > 0 { + fetchedFileInfos, err := s.FileInfoStore.GetByIds(fileIdsToFetch, includeDeleted, false) + if err != nil { + return nil, err + } + + for _, fileInfo := range fetchedFileInfos { + cacheKey := fmt.Sprintf("%s_%t", fileInfo.Id, includeDeleted) + s.rootStore.doStandardAddToCache(s.rootStore.fileInfoCache, cacheKey, fileInfo) + fileInfos = append(fileInfos, fileInfo) + } + } + + return fileInfos, nil +} + func (s LocalCacheFileInfoStore) ClearCaches() { s.rootStore.fileInfoCache.Purge() if s.rootStore.metrics != nil { diff --git a/server/channels/store/localcachelayer/file_info_layer_test.go b/server/channels/store/localcachelayer/file_info_layer_test.go index 02ef93c1db..14c44566c9 100644 --- a/server/channels/store/localcachelayer/file_info_layer_test.go +++ b/server/channels/store/localcachelayer/file_info_layer_test.go @@ -62,4 +62,19 @@ func TestFileInfoStoreCache(t *testing.T) { cachedStore.FileInfo().GetForPost("123", true, true, true) mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 2) }) + + t.Run("GetByIds cache test", func(t *testing.T) { + mockStore := getMockStore(t) + mockCacheProvider := getMockCacheProvider() + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider, logger) + require.NoError(t, err) + + fileInfos, err := cachedStore.FileInfo().GetByIds([]string{"123"}, true, true) + require.NoError(t, err) + assert.Equal(t, fileInfos, []*model.FileInfo{&fakeFileInfo}) + mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetByIds", 1) + assert.Equal(t, fileInfos, []*model.FileInfo{&fakeFileInfo}) + cachedStore.FileInfo().GetForPost("123", true, true, true) + mockStore.FileInfo().(*mocks.FileInfoStore).AssertNumberOfCalls(t, "GetForPost", 1) + }) } diff --git a/server/channels/store/localcachelayer/main_test.go b/server/channels/store/localcachelayer/main_test.go index bafb2278a7..530755709c 100644 --- a/server/channels/store/localcachelayer/main_test.go +++ b/server/channels/store/localcachelayer/main_test.go @@ -64,6 +64,7 @@ func getMockStore(t *testing.T) *mocks.Store { mockFileInfoStore := mocks.FileInfoStore{} mockFileInfoStore.On("GetForPost", "123", true, true, false).Return([]*model.FileInfo{&fakeFileInfo}, nil) mockFileInfoStore.On("GetForPost", "123", true, true, true).Return([]*model.FileInfo{&fakeFileInfo}, nil) + mockFileInfoStore.On("GetByIds", []string{"123"}, true, false).Return([]*model.FileInfo{&fakeFileInfo}, nil) mockStore.On("FileInfo").Return(&mockFileInfoStore) fakeWebhook := model.IncomingWebhook{Id: "123"} diff --git a/server/channels/store/opentracinglayer/opentracinglayer.go b/server/channels/store/opentracinglayer/opentracinglayer.go index 31681c08aa..aea598d92d 100644 --- a/server/channels/store/opentracinglayer/opentracinglayer.go +++ b/server/channels/store/opentracinglayer/opentracinglayer.go @@ -3943,6 +3943,24 @@ func (s *OpenTracingLayerFileInfoStore) DeleteForPost(c request.CTX, postID stri return result, err } +func (s *OpenTracingLayerFileInfoStore) DeleteForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.DeleteForPostByIds") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.FileInfoStore.DeleteForPostByIds(rctx, postId, fileIDs) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerFileInfoStore) Get(id string) (*model.FileInfo, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.Get") @@ -3961,7 +3979,7 @@ func (s *OpenTracingLayerFileInfoStore) Get(id string) (*model.FileInfo, error) return result, err } -func (s *OpenTracingLayerFileInfoStore) GetByIds(ids []string) ([]*model.FileInfo, error) { +func (s *OpenTracingLayerFileInfoStore) GetByIds(ids []string, includeDeleted bool, allowFromCache bool) ([]*model.FileInfo, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.GetByIds") s.Root.Store.SetContext(newCtx) @@ -3970,7 +3988,7 @@ func (s *OpenTracingLayerFileInfoStore) GetByIds(ids []string) ([]*model.FileInf }() defer span.Finish() - result, err := s.FileInfoStore.GetByIds(ids) + result, err := s.FileInfoStore.GetByIds(ids, includeDeleted, allowFromCache) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) @@ -4208,6 +4226,24 @@ func (s *OpenTracingLayerFileInfoStore) PermanentDeleteForPost(rctx request.CTX, return err } +func (s *OpenTracingLayerFileInfoStore) RestoreForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.RestoreForPostByIds") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.FileInfoStore.RestoreForPostByIds(rctx, postId, fileIDs) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerFileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "FileInfoStore.Save") diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 71a1096aa7..97dc64c2f3 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -4425,6 +4425,27 @@ func (s *RetryLayerFileInfoStore) DeleteForPost(c request.CTX, postID string) (s } +func (s *RetryLayerFileInfoStore) DeleteForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + + tries := 0 + for { + err := s.FileInfoStore.DeleteForPostByIds(rctx, postId, fileIDs) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerFileInfoStore) Get(id string) (*model.FileInfo, error) { tries := 0 @@ -4446,11 +4467,11 @@ func (s *RetryLayerFileInfoStore) Get(id string) (*model.FileInfo, error) { } -func (s *RetryLayerFileInfoStore) GetByIds(ids []string) ([]*model.FileInfo, error) { +func (s *RetryLayerFileInfoStore) GetByIds(ids []string, includeDeleted bool, allowFromCache bool) ([]*model.FileInfo, error) { tries := 0 for { - result, err := s.FileInfoStore.GetByIds(ids) + result, err := s.FileInfoStore.GetByIds(ids, includeDeleted, allowFromCache) if err == nil { return result, nil } @@ -4725,6 +4746,27 @@ func (s *RetryLayerFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postI } +func (s *RetryLayerFileInfoStore) RestoreForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + + tries := 0 + for { + err := s.FileInfoStore.RestoreForPostByIds(rctx, postId, fileIDs) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerFileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { tries := 0 diff --git a/server/channels/store/searchlayer/file_info_layer.go b/server/channels/store/searchlayer/file_info_layer.go index 1811475d1f..c5af456974 100644 --- a/server/channels/store/searchlayer/file_info_layer.go +++ b/server/channels/store/searchlayer/file_info_layer.go @@ -209,7 +209,7 @@ func (s SearchFileInfoStore) Search(rctx request.CTX, paramsList []*model.Search // Get the files filesList := model.NewFileInfoList() if len(fileIds) > 0 { - files, nErr := s.FileInfoStore.GetByIds(fileIds) + files, nErr := s.FileInfoStore.GetByIds(fileIds, false, true) if nErr != nil { return nil, nErr } diff --git a/server/channels/store/sqlstore/file_info_store.go b/server/channels/store/sqlstore/file_info_store.go index 3d2bbb0241..a48bcd12c3 100644 --- a/server/channels/store/sqlstore/file_info_store.go +++ b/server/channels/store/sqlstore/file_info_store.go @@ -132,14 +132,17 @@ func (fs SqlFileInfoStore) Save(rctx request.CTX, info *model.FileInfo) (*model. return info, nil } -func (fs SqlFileInfoStore) GetByIds(ids []string) ([]*model.FileInfo, error) { +func (fs SqlFileInfoStore) GetByIds(ids []string, includeDeleted, allowFromCache bool) ([]*model.FileInfo, error) { query := fs.getQueryBuilder(). Select(fs.queryFields...). From("FileInfo"). Where(sq.Eq{"FileInfo.Id": ids}). - Where(sq.Eq{"FileInfo.DeleteAt": 0}). OrderBy("FileInfo.CreateAt DESC") + if !includeDeleted { + query = query.Where(sq.Eq{"FileInfo.DeleteAt": 0}) + } + queryString, args, err := query.ToSql() if err != nil { return nil, errors.Wrap(err, "file_info_tosql") @@ -455,6 +458,27 @@ func (fs SqlFileInfoStore) DeleteForPost(rctx request.CTX, postId string) (strin return postId, nil } +func (fs SqlFileInfoStore) DeleteForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + query := fs.getQueryBuilder(). + Update("FileInfo"). + Set("DeleteAt", model.GetMillis()). + Where(sq.Eq{ + "PostId": postId, + "Id": fileIDs, + }) + + queryString, args, err := query.ToSql() + if err != nil { + return errors.Wrap(err, "SqlFileInfoStore.DeleteForPostByIds: failed to generate sql from query") + } + + if _, err := fs.GetMaster().Exec(queryString, args...); err != nil { + return errors.Wrap(err, "SqlFileInfoStore.DeleteForPostByIds: failed to soft delete FileInfo from database") + } + + return nil +} + func (fs SqlFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) error { if _, err := fs.GetMaster().Exec(`DELETE FROM FileInfo WHERE PostId = ?`, postID); err != nil { return errors.Wrapf(err, "failed to delete FileInfo with PostId=%s", postID) @@ -796,3 +820,24 @@ func (fs *SqlFileInfoStore) GetUptoNSizeFileTime(n int64) (int64, error) { return createAt, nil } + +func (fs SqlFileInfoStore) RestoreForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + query := fs.getQueryBuilder(). + Update("FileInfo"). + Set("DeleteAt", 0). + Where(sq.Eq{ + "PostId": postId, + "Id": fileIDs, + }) + + queryString, args, err := query.ToSql() + if err != nil { + return errors.Wrap(err, "SqlFileInfoStore.RestoreForPostByIds: failed to generate sql from query") + } + + if _, err := fs.GetMaster().Exec(queryString, args...); err != nil { + return errors.Wrap(err, "SqlFileInfoStore.RestoreForPostByIds: failed to undelete FileInfo from database") + } + + return nil +} diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 55fe23c9a3..6438d77344 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -722,7 +722,7 @@ type FileInfoStore interface { Upsert(rctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) Get(id string) (*model.FileInfo, error) GetFromMaster(id string) (*model.FileInfo, error) - GetByIds(ids []string) ([]*model.FileInfo, error) + GetByIds(ids []string, includeDeleted, allowFromCache bool) ([]*model.FileInfo, error) GetByPath(path string) (*model.FileInfo, error) GetForPost(postID string, readFromMaster, includeDeleted, allowFromCache bool) ([]*model.FileInfo, error) GetForUser(userID string) ([]*model.FileInfo, error) @@ -730,6 +730,8 @@ type FileInfoStore interface { InvalidateFileInfosForPostCache(postID string, deleted bool) AttachToPost(c request.CTX, fileID string, postID string, channelID, creatorID string) error DeleteForPost(c request.CTX, postID string) (string, error) + DeleteForPostByIds(rctx request.CTX, postId string, fileIDs []string) error + RestoreForPostByIds(rctx request.CTX, postId string, fileIDs []string) error PermanentDeleteForPost(rctx request.CTX, postID string) error PermanentDelete(c request.CTX, fileID string) error PermanentDeleteBatch(ctx request.CTX, endTime int64, limit int64) (int64, error) diff --git a/server/channels/store/storetest/file_info_store.go b/server/channels/store/storetest/file_info_store.go index 736bdde485..8335045df4 100644 --- a/server/channels/store/storetest/file_info_store.go +++ b/server/channels/store/storetest/file_info_store.go @@ -38,6 +38,9 @@ func TestFileInfoStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStor t.Run("GetStorageUsage", func(t *testing.T) { testFileInfoGetStorageUsage(t, rctx, ss) }) t.Run("GetUptoNSizeFileTime", func(t *testing.T) { testGetUptoNSizeFileTime(t, rctx, ss, s) }) t.Run("FileInfoPermanentDeleteForPost", func(t *testing.T) { testPermanentDeleteForPost(t, rctx, ss) }) + t.Run("FileInfoGetByIds", func(t *testing.T) { testGetByIds(t, rctx, ss) }) + t.Run("FileInfoDeleteForPostByIds", func(t *testing.T) { testDeleteForPostByIds(t, rctx, ss) }) + t.Run("FileInfoRestoreForPostByIds", func(t *testing.T) { testRestoreUndeleteForPostByIds(t, rctx, ss) }) } func testFileInfoSaveGet(t *testing.T, rctx request.CTX, ss store.Store) { @@ -967,3 +970,566 @@ func testPermanentDeleteForPost(t *testing.T, rctx request.CTX, ss store.Store) require.NoError(t, err) assert.Len(t, postInfos, 0) } + +func testGetByIds(t *testing.T, rctx request.CTX, ss store.Store) { + t.Run("Should get single file info", func(t *testing.T) { + info, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + CreatorId: model.NewId(), + Path: "file.txt", + }) + require.NoError(t, err) + require.NotEqual(t, len(info.Id), 0) + + defer func() { + ss.FileInfo().PermanentDelete(rctx, info.Id) + }() + + fileInfos, err := ss.FileInfo().GetByIds([]string{info.Id}, false, true) + require.NoError(t, err) + require.Len(t, fileInfos, 1) + require.Equal(t, info.Id, fileInfos[0].Id) + }) + + t.Run("Should get multiple file info", func(t *testing.T) { + info1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + CreatorId: model.NewId(), + Path: "file.txt", + }) + require.NoError(t, err) + require.NotEqual(t, len(info1.Id), 0) + + // waiting 1 second to add deterministic difference between the two file info's CreateAt time + time.Sleep(1 * time.Second) + + info2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + CreatorId: model.NewId(), + Path: "file.txt", + }) + require.NoError(t, err) + require.NotEqual(t, len(info2.Id), 0) + + defer func() { + ss.FileInfo().PermanentDelete(rctx, info1.Id) + ss.FileInfo().PermanentDelete(rctx, info2.Id) + }() + + fileInfos, err := ss.FileInfo().GetByIds([]string{info1.Id, info2.Id}, false, true) + require.NoError(t, err) + require.Len(t, fileInfos, 2) + require.Equal(t, info1.Id, fileInfos[1].Id) + require.Equal(t, info2.Id, fileInfos[0].Id) + }) + + t.Run("Should get deleted file infos when specified", func(t *testing.T) { + postId := model.NewId() + + info1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + CreatorId: model.NewId(), + Path: "file.txt", + PostId: postId, + }) + require.NoError(t, err) + require.NotEqual(t, len(info1.Id), 0) + + // waiting 1 second to add deterministic difference between the two file info's CreateAt time + time.Sleep(1 * time.Second) + + info2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + CreatorId: model.NewId(), + Path: "file.txt", + PostId: postId, + }) + require.NoError(t, err) + require.NotEqual(t, len(info2.Id), 0) + + defer func() { + ss.FileInfo().PermanentDelete(rctx, info1.Id) + ss.FileInfo().PermanentDelete(rctx, info2.Id) + }() + + // we'll delete the two file infos + _, err = ss.FileInfo().DeleteForPost(rctx, postId) + require.NoError(t, err) + + fileInfosIncludingDeleted, err := ss.FileInfo().GetByIds([]string{info1.Id, info2.Id}, true, true) + require.NoError(t, err) + require.Len(t, fileInfosIncludingDeleted, 2) + require.Equal(t, info2.Id, fileInfosIncludingDeleted[0].Id) + require.Greater(t, fileInfosIncludingDeleted[0].DeleteAt, int64(0)) + require.Equal(t, info1.Id, fileInfosIncludingDeleted[1].Id) + require.Greater(t, fileInfosIncludingDeleted[1].DeleteAt, int64(0)) + + // verifying that the file infos are not returned when IncludeDeleted is false + fileInfosExcludingDeleted, err := ss.FileInfo().GetByIds([]string{info1.Id, info2.Id}, false, true) + require.NoError(t, err) + require.Len(t, fileInfosExcludingDeleted, 0) + }) +} + +func testDeleteForPostByIds(t *testing.T, rctx request.CTX, ss store.Store) { + t.Run("base case", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + if fileInfo.Id == fileInfo1.Id || fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + } + }) + + t.Run("with empty array", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + }) + + t.Run("duplicate fileInfo Ids specified", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + if fileInfo.Id == fileInfo1.Id || fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + } + }) + + t.Run("non existent fileInfo IDs specified", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{model.NewId(), model.NewId()}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + }) + + t.Run("non existent postID specified", func(t *testing.T) { + err := ss.FileInfo().DeleteForPostByIds(rctx, model.NewId(), []string{model.NewId()}) + require.NoError(t, err) + }) + + t.Run("delete already deleted fileInfos", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + if fileInfo.Id == fileInfo1.Id || fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + } + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err = ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + if fileInfo.Id == fileInfo1.Id || fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + } + }) +} + +func testRestoreUndeleteForPostByIds(t *testing.T, rctx request.CTX, ss store.Store) { + t.Run("base case", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + if fileInfo.Id == fileInfo1.Id || fileInfo.Id == fileInfo2.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + } + + // now we'll un-delete the files + err = ss.FileInfo().RestoreForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err = ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + require.Equal(t, fileInfo.DeleteAt, int64(0)) + } + }) + + t.Run("with empty array it should not impact any post files", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + err = ss.FileInfo().RestoreForPostByIds(rctx, postId, []string{}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + }) + + t.Run("duplicate fileInfo Ids specified", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + // delete file infos + err = ss.FileInfo().DeleteForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id, fileInfo3.Id}) + require.NoError(t, err) + + // verify file infos are deleted + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } + + // undelete them specifying duplicate file info ids + err = ss.FileInfo().RestoreForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id, fileInfo2.Id, fileInfo2.Id}) + require.NoError(t, err) + + // verify file infos are deleted + fileInfos, err = ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + if fileInfo.Id == fileInfo3.Id { + require.Greater(t, fileInfo.DeleteAt, int64(0)) + } else { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + } + }) + + t.Run("non existent fileInfo IDs and postId specified", func(t *testing.T) { + err := ss.FileInfo().RestoreForPostByIds(rctx, model.NewId(), []string{model.NewId(), model.NewId()}) + require.NoError(t, err) + }) + + t.Run("undelete already undeleted fileInfos", func(t *testing.T) { + now := model.GetMillis() + postId := model.NewId() + + fileInfo1, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo1.Id) + + fileInfo2, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file2.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo2.Id) + + fileInfo3, err := ss.FileInfo().Save(rctx, &model.FileInfo{ + PostId: postId, + CreatorId: model.NewId(), + Size: 10, + Path: "file1.txt", + CreateAt: now, + }) + require.NoError(t, err) + defer ss.FileInfo().PermanentDelete(rctx, fileInfo3.Id) + + err = ss.FileInfo().RestoreForPostByIds(rctx, postId, []string{fileInfo1.Id, fileInfo2.Id}) + require.NoError(t, err) + + fileInfos, err := ss.FileInfo().GetForPost(postId, true, true, false) + require.NoError(t, err) + + for _, fileInfo := range fileInfos { + require.Equal(t, int64(0), fileInfo.DeleteAt) + } + }) +} diff --git a/server/channels/store/storetest/mocks/FileInfoStore.go b/server/channels/store/storetest/mocks/FileInfoStore.go index 94de49ed5d..a86eb04b31 100644 --- a/server/channels/store/storetest/mocks/FileInfoStore.go +++ b/server/channels/store/storetest/mocks/FileInfoStore.go @@ -94,6 +94,24 @@ func (_m *FileInfoStore) DeleteForPost(c request.CTX, postID string) (string, er return r0, r1 } +// DeleteForPostByIds provides a mock function with given fields: rctx, postId, fileIDs +func (_m *FileInfoStore) DeleteForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + ret := _m.Called(rctx, postId, fileIDs) + + if len(ret) == 0 { + panic("no return value specified for DeleteForPostByIds") + } + + var r0 error + if rf, ok := ret.Get(0).(func(request.CTX, string, []string) error); ok { + r0 = rf(rctx, postId, fileIDs) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Get provides a mock function with given fields: id func (_m *FileInfoStore) Get(id string) (*model.FileInfo, error) { ret := _m.Called(id) @@ -124,9 +142,9 @@ func (_m *FileInfoStore) Get(id string) (*model.FileInfo, error) { return r0, r1 } -// GetByIds provides a mock function with given fields: ids -func (_m *FileInfoStore) GetByIds(ids []string) ([]*model.FileInfo, error) { - ret := _m.Called(ids) +// GetByIds provides a mock function with given fields: ids, includeDeleted, allowFromCache +func (_m *FileInfoStore) GetByIds(ids []string, includeDeleted bool, allowFromCache bool) ([]*model.FileInfo, error) { + ret := _m.Called(ids, includeDeleted, allowFromCache) if len(ret) == 0 { panic("no return value specified for GetByIds") @@ -134,19 +152,19 @@ func (_m *FileInfoStore) GetByIds(ids []string) ([]*model.FileInfo, error) { var r0 []*model.FileInfo var r1 error - if rf, ok := ret.Get(0).(func([]string) ([]*model.FileInfo, error)); ok { - return rf(ids) + if rf, ok := ret.Get(0).(func([]string, bool, bool) ([]*model.FileInfo, error)); ok { + return rf(ids, includeDeleted, allowFromCache) } - if rf, ok := ret.Get(0).(func([]string) []*model.FileInfo); ok { - r0 = rf(ids) + if rf, ok := ret.Get(0).(func([]string, bool, bool) []*model.FileInfo); ok { + r0 = rf(ids, includeDeleted, allowFromCache) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.FileInfo) } } - if rf, ok := ret.Get(1).(func([]string) error); ok { - r1 = rf(ids) + if rf, ok := ret.Get(1).(func([]string, bool, bool) error); ok { + r1 = rf(ids, includeDeleted, allowFromCache) } else { r1 = ret.Error(1) } @@ -487,6 +505,24 @@ func (_m *FileInfoStore) PermanentDeleteForPost(rctx request.CTX, postID string) return r0 } +// RestoreForPostByIds provides a mock function with given fields: rctx, postId, fileIDs +func (_m *FileInfoStore) RestoreForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + ret := _m.Called(rctx, postId, fileIDs) + + if len(ret) == 0 { + panic("no return value specified for RestoreForPostByIds") + } + + var r0 error + if rf, ok := ret.Get(0).(func(request.CTX, string, []string) error); ok { + r0 = rf(rctx, postId, fileIDs) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Save provides a mock function with given fields: ctx, info func (_m *FileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { ret := _m.Called(ctx, info) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 4e564e2a32..27e5f48745 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -3604,6 +3604,22 @@ func (s *TimerLayerFileInfoStore) DeleteForPost(c request.CTX, postID string) (s return result, err } +func (s *TimerLayerFileInfoStore) DeleteForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + start := time.Now() + + err := s.FileInfoStore.DeleteForPostByIds(rctx, postId, fileIDs) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("FileInfoStore.DeleteForPostByIds", success, elapsed) + } + return err +} + func (s *TimerLayerFileInfoStore) Get(id string) (*model.FileInfo, error) { start := time.Now() @@ -3620,10 +3636,10 @@ func (s *TimerLayerFileInfoStore) Get(id string) (*model.FileInfo, error) { return result, err } -func (s *TimerLayerFileInfoStore) GetByIds(ids []string) ([]*model.FileInfo, error) { +func (s *TimerLayerFileInfoStore) GetByIds(ids []string, includeDeleted bool, allowFromCache bool) ([]*model.FileInfo, error) { start := time.Now() - result, err := s.FileInfoStore.GetByIds(ids) + result, err := s.FileInfoStore.GetByIds(ids, includeDeleted, allowFromCache) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -3843,6 +3859,22 @@ func (s *TimerLayerFileInfoStore) PermanentDeleteForPost(rctx request.CTX, postI return err } +func (s *TimerLayerFileInfoStore) RestoreForPostByIds(rctx request.CTX, postId string, fileIDs []string) error { + start := time.Now() + + err := s.FileInfoStore.RestoreForPostByIds(rctx, postId, fileIDs) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("FileInfoStore.RestoreForPostByIds", success, elapsed) + } + return err +} + func (s *TimerLayerFileInfoStore) Save(ctx request.CTX, info *model.FileInfo) (*model.FileInfo, error) { start := time.Now() diff --git a/server/i18n/en.json b/server/i18n/en.json index 62e0fda99a..f116aaa755 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -5138,6 +5138,10 @@ "id": "app.file.cloud.get.app_error", "translation": "Can not fetch the file as it is past the cloud plan's limit." }, + { + "id": "app.file_info.delete_for_post_ids.app_error", + "translation": "Failed to remove the requested files from database" + }, { "id": "app.file_info.get.app_error", "translation": "Unable to get the file info." @@ -5146,6 +5150,10 @@ "id": "app.file_info.get.gif.app_error", "translation": "Could not decode gif." }, + { + "id": "app.file_info.get_by_ids.app_error", + "translation": "Unable to get the file infos by ids for post edit history." + }, { "id": "app.file_info.get_by_post_id.app_error", "translation": "Failed to find files for post." @@ -5178,6 +5186,10 @@ "id": "app.file_info.set_searchable_content.app_error", "translation": "Unable to set the searchable content of the file." }, + { + "id": "app.file_info.undelete_for_post_ids.app_error", + "translation": "Failed to restore post file attachments." + }, { "id": "app.get_user_team_scheduled_posts.error", "translation": "Error occurred fetching scheduled posts." @@ -6354,6 +6366,22 @@ "id": "app.post.permanent_delete_post.error", "translation": "Failed to permanently delete post." }, + { + "id": "app.post.restore_post_version.get_single.app_error", + "translation": "Failed to get the old post version." + }, + { + "id": "app.post.restore_post_version.not_allowed.app_error", + "translation": "You do not have the appropriate permissions." + }, + { + "id": "app.post.restore_post_version.not_an_history_item.app_error", + "translation": "The provided post history ID does not correspond to any history item for the specified post." + }, + { + "id": "app.post.restore_post_version.not_valid_post_history_item.app_error", + "translation": "The provided post history ID does not correspond to a post history item." + }, { "id": "app.post.save.app_error", "translation": "Unable to save the Post." diff --git a/server/platform/services/sharedchannel/mock_AppIface_test.go b/server/platform/services/sharedchannel/mock_AppIface_test.go index 7417dee94c..09e6d57887 100644 --- a/server/platform/services/sharedchannel/mock_AppIface_test.go +++ b/server/platform/services/sharedchannel/mock_AppIface_test.go @@ -531,9 +531,9 @@ func (_m *MockAppIface) SendEphemeralPost(c request.CTX, userId string, post *mo return r0 } -// UpdatePost provides a mock function with given fields: c, post, safeUpdate -func (_m *MockAppIface) UpdatePost(c request.CTX, post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { - ret := _m.Called(c, post, safeUpdate) +// UpdatePost provides a mock function with given fields: c, post, updatePostOptions +func (_m *MockAppIface) UpdatePost(c request.CTX, post *model.Post, updatePostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) { + ret := _m.Called(c, post, updatePostOptions) if len(ret) == 0 { panic("no return value specified for UpdatePost") @@ -541,19 +541,19 @@ func (_m *MockAppIface) UpdatePost(c request.CTX, post *model.Post, safeUpdate b var r0 *model.Post var r1 *model.AppError - if rf, ok := ret.Get(0).(func(request.CTX, *model.Post, bool) (*model.Post, *model.AppError)); ok { - return rf(c, post, safeUpdate) + if rf, ok := ret.Get(0).(func(request.CTX, *model.Post, *model.UpdatePostOptions) (*model.Post, *model.AppError)); ok { + return rf(c, post, updatePostOptions) } - if rf, ok := ret.Get(0).(func(request.CTX, *model.Post, bool) *model.Post); ok { - r0 = rf(c, post, safeUpdate) + if rf, ok := ret.Get(0).(func(request.CTX, *model.Post, *model.UpdatePostOptions) *model.Post); ok { + r0 = rf(c, post, updatePostOptions) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Post) } } - if rf, ok := ret.Get(1).(func(request.CTX, *model.Post, bool) *model.AppError); ok { - r1 = rf(c, post, safeUpdate) + if rf, ok := ret.Get(1).(func(request.CTX, *model.Post, *model.UpdatePostOptions) *model.AppError); ok { + r1 = rf(c, post, updatePostOptions) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*model.AppError) diff --git a/server/platform/services/sharedchannel/service.go b/server/platform/services/sharedchannel/service.go index 5807329145..1d2c182fea 100644 --- a/server/platform/services/sharedchannel/service.go +++ b/server/platform/services/sharedchannel/service.go @@ -58,7 +58,7 @@ type AppIface interface { AddUserToTeamByTeamId(c request.CTX, teamId string, user *model.User) *model.AppError PermanentDeleteChannel(c request.CTX, channel *model.Channel) *model.AppError CreatePost(c request.CTX, post *model.Post, channel *model.Channel, flags model.CreatePostFlags) (savedPost *model.Post, err *model.AppError) - UpdatePost(c request.CTX, post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) + UpdatePost(c request.CTX, post *model.Post, updatePostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) DeletePost(c request.CTX, postID, deleteByID string) (*model.Post, *model.AppError) SaveReactionForPost(c request.CTX, reaction *model.Reaction) (*model.Reaction, *model.AppError) DeleteReactionForPost(c request.CTX, reaction *model.Reaction) *model.AppError diff --git a/server/platform/services/sharedchannel/sync_recv.go b/server/platform/services/sharedchannel/sync_recv.go index b50b67f3ac..7cb72e28a7 100644 --- a/server/platform/services/sharedchannel/sync_recv.go +++ b/server/platform/services/sharedchannel/sync_recv.go @@ -403,7 +403,7 @@ func (scs *Service) upsertSyncPost(post *model.Post, targetChannel *model.Channe } } else if post.EditAt > rpost.EditAt || post.Message != rpost.Message { // update post - rpost, appErr = scs.app.UpdatePost(request.EmptyContext(scs.server.Log()), post, false) + rpost, appErr = scs.app.UpdatePost(request.EmptyContext(scs.server.Log()), post, nil) if appErr == nil { scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Updated sync post", mlog.String("post_id", post.Id), diff --git a/server/public/model/client4.go b/server/public/model/client4.go index 522d81a43e..09fafb7a5e 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -9369,3 +9369,17 @@ func (c *Client4) GetFilteredUsersStats(ctx context.Context, options *UserCountO } return &stats, BuildResponse(r), nil } + +func (c *Client4) RestorePostVersion(ctx context.Context, postId, versionId string) (*Post, *Response, error) { + r, err := c.DoAPIPost(ctx, c.postRoute(postId)+"/restore/"+versionId, "") + if err != nil { + return nil, BuildResponse(r), err + } + + defer closeBody(r) + var restoredPost *Post + if err := json.NewDecoder(r.Body).Decode(&restoredPost); err != nil { + return nil, nil, NewAppError("RestorePostVersion", "api.unmarshal_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return restoredPost, BuildResponse(r), nil +} diff --git a/server/public/model/post.go b/server/public/model/post.go index 73d3c2d1f9..9f2f740f88 100644 --- a/server/public/model/post.go +++ b/server/public/model/post.go @@ -105,7 +105,7 @@ type Post struct { Props StringInterface `json:"props"` // Deprecated: use GetProps() Hashtags string `json:"hashtags"` Filenames StringArray `json:"-"` // Deprecated, do not use this field any more - FileIds StringArray `json:"file_ids,omitempty"` + FileIds StringArray `json:"file_ids"` PendingPostId string `json:"pending_post_id"` HasReactions bool `json:"has_reactions,omitempty"` RemoteId *string `json:"remote_id,omitempty"` @@ -953,3 +953,15 @@ func (o *Post) CleanPost() *Post { o.EditAt = 0 return o } + +type UpdatePostOptions struct { + SafeUpdate bool + IsRestorePost bool +} + +func DefaultUpdatePostOptions() *UpdatePostOptions { + return &UpdatePostOptions{ + SafeUpdate: false, + IsRestorePost: false, + } +} diff --git a/server/public/utils/array.go b/server/public/utils/array.go new file mode 100644 index 0000000000..54b9caa03f --- /dev/null +++ b/server/public/utils/array.go @@ -0,0 +1,45 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package utils + +// FindExclusives returns three arrays: +// 1. Items exclusive to arr1 +// 2. Items exclusive to arr2 +// 3. Items common to both arr1 and arr2 +func FindExclusives[T comparable](arr1, arr2 []T) ([]T, []T, []T) { + // Create maps to track the presence of elements in each array + existsInArr1 := make(map[T]bool) + existsInArr2 := make(map[T]bool) + + // Populate the maps with the elements from both arrays + for _, elem := range arr1 { + existsInArr1[elem] = true + } + for _, elem := range arr2 { + existsInArr2[elem] = true + } + + // Slices for results + var uniqueToArr1 []T + var uniqueToArr2 []T + var common []T + + // Find elements unique to arr1 and common elements + for elem := range existsInArr1 { + if existsInArr2[elem] { + common = append(common, elem) + } else { + uniqueToArr1 = append(uniqueToArr1, elem) + } + } + + // Find elements unique to arr2 + for elem := range existsInArr2 { + if !existsInArr1[elem] { + uniqueToArr2 = append(uniqueToArr2, elem) + } + } + + return uniqueToArr1, uniqueToArr2, common +} diff --git a/server/public/utils/array_test.go b/server/public/utils/array_test.go new file mode 100644 index 0000000000..643bc0a9f3 --- /dev/null +++ b/server/public/utils/array_test.go @@ -0,0 +1,451 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package utils + +import ( + "reflect" + "sort" + "testing" + "time" +) + +func TestFindExclusives(t *testing.T) { + t.Run("integers", func(t *testing.T) { + tests := []struct { + name string + arr1, arr2 []int + expectedExclusive1 []int + expectedExclusive2 []int + expectedCommon []int + }{ + // Basic test with non-overlapping elements + { + name: "No overlap", + arr1: []int{1, 2, 3}, + arr2: []int{4, 5, 6}, + expectedExclusive1: []int{1, 2, 3}, + expectedExclusive2: []int{4, 5, 6}, + expectedCommon: nil, + }, + // Fully overlapping arrays + { + name: "Full overlap", + arr1: []int{1, 2, 3}, + arr2: []int{1, 2, 3}, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: []int{1, 2, 3}, + }, + // Partial overlap + { + name: "Partial overlap", + arr1: []int{1, 2, 3, 4}, + arr2: []int{3, 4, 5, 6}, + expectedExclusive1: []int{1, 2}, + expectedExclusive2: []int{5, 6}, + expectedCommon: []int{3, 4}, + }, + // Duplicates within arrays + { + name: "Duplicates in arr1", + arr1: []int{1, 2, 2, 3}, + arr2: []int{2, 4, 4}, + expectedExclusive1: []int{1, 3}, + expectedExclusive2: []int{4}, + expectedCommon: []int{2}, + }, + { + name: "Duplicates in arr2", + arr1: []int{1, 2, 3}, + arr2: []int{2, 2, 3, 3}, + expectedExclusive1: []int{1}, + expectedExclusive2: nil, + expectedCommon: []int{2, 3}, + }, + // Edge cases + { + name: "Both arrays nil", + arr1: nil, + arr2: nil, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "Both arrays empty", + arr1: []int{}, + arr2: []int{}, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "One empty array", + arr1: []int{1, 2, 3}, + arr2: nil, + expectedExclusive1: []int{1, 2, 3}, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "One element in each array", + arr1: []int{1}, + arr2: []int{2}, + expectedExclusive1: []int{1}, + expectedExclusive2: []int{2}, + expectedCommon: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exclusive1, exclusive2, common := FindExclusives(tt.arr1, tt.arr2) + + sort.Ints(exclusive1) + sort.Ints(exclusive2) + sort.Ints(common) + sort.Ints(tt.expectedExclusive1) + sort.Ints(tt.expectedExclusive2) + sort.Ints(tt.expectedCommon) + + if !reflect.DeepEqual(exclusive1, tt.expectedExclusive1) { + t.Errorf("Exclusive to arr1: expected %v, got %v", tt.expectedExclusive1, exclusive1) + } + if !reflect.DeepEqual(exclusive2, tt.expectedExclusive2) { + t.Errorf("Exclusive to arr2: expected %v, got %v", tt.expectedExclusive2, exclusive2) + } + if !reflect.DeepEqual(common, tt.expectedCommon) { + t.Errorf("Common elements: expected %v, got %v", tt.expectedCommon, common) + } + }) + } + }) + + t.Run("strings", func(t *testing.T) { + tests := []struct { + name string + arr1, arr2 []string + expectedExclusive1 []string + expectedExclusive2 []string + expectedCommon []string + }{ + // Basic test with non-overlapping elements + { + name: "No overlap", + arr1: []string{"a", "b", "c"}, + arr2: []string{"d", "e", "f"}, + expectedExclusive1: []string{"a", "b", "c"}, + expectedExclusive2: []string{"d", "e", "f"}, + expectedCommon: nil, + }, + // Fully overlapping arrays + { + name: "Full overlap", + arr1: []string{"a", "b", "c"}, + arr2: []string{"a", "b", "c"}, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: []string{"a", "b", "c"}, + }, + // Partial overlap + { + name: "Partial overlap", + arr1: []string{"a", "b", "c", "d"}, + arr2: []string{"c", "d", "e", "f"}, + expectedExclusive1: []string{"a", "b"}, + expectedExclusive2: []string{"e", "f"}, + expectedCommon: []string{"c", "d"}, + }, + // Duplicates within arrays + { + name: "Duplicates in arr1", + arr1: []string{"a", "b", "b", "c"}, + arr2: []string{"b", "d", "d"}, + expectedExclusive1: []string{"a", "c"}, + expectedExclusive2: []string{"d"}, + expectedCommon: []string{"b"}, + }, + { + name: "Duplicates in arr2", + arr1: []string{"a", "b", "c"}, + arr2: []string{"b", "b", "c", "c"}, + expectedExclusive1: []string{"a"}, + expectedExclusive2: nil, + expectedCommon: []string{"b", "c"}, + }, + // Edge cases + { + name: "Both arrays nil", + arr1: nil, + arr2: nil, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "Both arrays empty", + arr1: []string{}, + arr2: []string{}, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "One empty array", + arr1: []string{"a", "b", "c"}, + arr2: nil, + expectedExclusive1: []string{"a", "b", "c"}, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "One element in each array", + arr1: []string{"a"}, + arr2: []string{"b"}, + expectedExclusive1: []string{"a"}, + expectedExclusive2: []string{"b"}, + expectedCommon: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exclusive1, exclusive2, common := FindExclusives(tt.arr1, tt.arr2) + + sort.Strings(exclusive1) + sort.Strings(exclusive2) + sort.Strings(common) + sort.Strings(tt.expectedExclusive1) + sort.Strings(tt.expectedExclusive2) + sort.Strings(tt.expectedCommon) + + if !reflect.DeepEqual(exclusive1, tt.expectedExclusive1) { + t.Errorf("Exclusive to arr1: expected %v, got %v", tt.expectedExclusive1, exclusive1) + } + if !reflect.DeepEqual(exclusive2, tt.expectedExclusive2) { + t.Errorf("Exclusive to arr2: expected %v, got %v", tt.expectedExclusive2, exclusive2) + } + if !reflect.DeepEqual(common, tt.expectedCommon) { + t.Errorf("Common elements: expected %v, got %v", tt.expectedCommon, common) + } + }) + } + }) + + t.Run("dates", func(t *testing.T) { + tests := []struct { + name string + arr1, arr2 []time.Time + expectedExclusive1 []time.Time + expectedExclusive2 []time.Time + expectedCommon []time.Time + }{ + // Basic test with non-overlapping elements + { + name: "No overlap", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + arr2: []time.Time{ + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 5, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 6, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive2: []time.Time{ + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 5, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 6, 0, 0, 0, 0, time.UTC), + }, + expectedCommon: nil, + }, + // Fully overlapping arrays + { + name: "Full overlap", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + arr2: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + }, + // Partial overlap + { + name: "Partial overlap", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + }, + arr2: []time.Time{ + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 5, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 6, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive2: []time.Time{ + time.Date(2023, 1, 5, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 6, 0, 0, 0, 0, time.UTC), + }, + expectedCommon: []time.Time{ + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + }, + }, + // Duplicates within arrays + { + name: "Duplicates in arr1", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + arr2: []time.Time{ + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive2: []time.Time{ + time.Date(2023, 1, 4, 0, 0, 0, 0, time.UTC), + }, + expectedCommon: []time.Time{ + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + }, + }, + { + name: "Duplicates in arr2", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + arr2: []time.Time{ + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive2: nil, + expectedCommon: []time.Time{ + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + }, + // Edge cases + { + name: "Both arrays nil", + arr1: nil, + arr2: nil, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "Both arrays empty", + arr1: []time.Time{}, + arr2: []time.Time{}, + expectedExclusive1: nil, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "One empty array", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + arr2: nil, + expectedExclusive1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + time.Date(2023, 1, 3, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive2: nil, + expectedCommon: nil, + }, + { + name: "One element in each array", + arr1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + }, + arr2: []time.Time{ + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive1: []time.Time{ + time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC), + }, + expectedExclusive2: []time.Time{ + time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC), + }, + expectedCommon: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exclusive1, exclusive2, common := FindExclusives(tt.arr1, tt.arr2) + + sort.Slice(exclusive1, func(i, j int) bool { + return exclusive1[i].Before(exclusive1[j]) + }) + sort.Slice(exclusive2, func(i, j int) bool { + return exclusive2[i].Before(exclusive2[j]) + }) + sort.Slice(common, func(i, j int) bool { + return common[i].Before(common[j]) + }) + sort.Slice(tt.expectedExclusive1, func(i, j int) bool { + return tt.expectedExclusive1[i].Before(tt.expectedExclusive1[j]) + }) + sort.Slice(tt.expectedExclusive2, func(i, j int) bool { + return tt.expectedExclusive2[i].Before(tt.expectedExclusive2[j]) + }) + sort.Slice(tt.expectedCommon, func(i, j int) bool { + return tt.expectedCommon[i].Before(tt.expectedCommon[j]) + }) + + if !reflect.DeepEqual(exclusive1, tt.expectedExclusive1) { + t.Errorf("Exclusive to arr1: expected %v, got %v", tt.expectedExclusive1, exclusive1) + } + if !reflect.DeepEqual(exclusive2, tt.expectedExclusive2) { + t.Errorf("Exclusive to arr2: expected %v, got %v", tt.expectedExclusive2, exclusive2) + } + if !reflect.DeepEqual(common, tt.expectedCommon) { + t.Errorf("Common elements: expected %v, got %v", tt.expectedCommon, common) + } + }) + } + }) +} diff --git a/webapp/channels/src/actions/post_actions.test.ts b/webapp/channels/src/actions/post_actions.test.ts index fd6340685f..c6200cfbab 100644 --- a/webapp/channels/src/actions/post_actions.test.ts +++ b/webapp/channels/src/actions/post_actions.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import type {FileInfo} from '@mattermost/types/files'; +import type {FileInfo, FilesState} from '@mattermost/types/files'; import type {Post} from '@mattermost/types/posts'; import {ChannelTypes, SearchTypes} from 'mattermost-redux/action_types'; @@ -10,6 +10,7 @@ import {Posts} from 'mattermost-redux/constants'; import * as Actions from 'actions/post_actions'; +import test_helper from 'packages/mattermost-redux/test/test_helper'; import mockStore from 'tests/test_store'; import {Constants, ActionTypes, RHSStates} from 'utils/constants'; import * as PostUtils from 'utils/post_utils'; @@ -300,8 +301,25 @@ describe('Actions.Posts', () => { }); test('setEditingPost', async () => { + const state = JSON.parse(JSON.stringify(initialState)) as GlobalState; + + state.entities.posts.posts[latestPost.id] = { + ...latestPost, + file_ids: ['file_id_1', 'file_id_2'], + } as Post; + + state.entities.files = { + files: { + file_id_1: test_helper.getFileInfoMock({id: 'file_id_1', post_id: 'latest_post_id'}), + file_id_2: test_helper.getFileInfoMock({id: 'file_id_2', post_id: 'latest_post_id'}), + }, + fileIdsByPostId: { + [latestPost.id]: ['file_id_1', 'file_id_2'], + }, + } as unknown as FilesState; + // should allow to edit and should fire an action - let testStore = mockStore({...initialState}); + let testStore = mockStore({...state}); const {data} = await testStore.dispatch(Actions.setEditingPost('latest_post_id', 'test')); expect(data).toEqual(true); @@ -313,7 +331,29 @@ describe('Actions.Posts', () => { {data: {isRHS: false, postId: 'latest_post_id', refocusId: 'test', show: true}, type: ActionTypes.TOGGLE_EDITING_POST}, ); expect(actions[0].payload[1]).toEqual( - {args: ['edit_draft_latest_post_id', {id: 'latest_post_id', user_id: 'current_user_id', message: 'test msg', channel_id: 'current_channel_id', type: 'normal'}], type: 'MOCK_SET_GLOBAL_ITEM'}, + { + args: [ + 'edit_draft_latest_post_id', + { + id: 'latest_post_id', + user_id: 'current_user_id', + message: 'test msg', + channel_id: 'current_channel_id', + type: 'normal', + file_ids: [ + 'file_id_1', + 'file_id_2', + ], + metadata: { + files: [ + test_helper.getFileInfoMock({id: 'file_id_1', post_id: 'latest_post_id'}), + test_helper.getFileInfoMock({id: 'file_id_2', post_id: 'latest_post_id'}), + ], + }, + }, + ], + type: 'MOCK_SET_GLOBAL_ITEM', + }, ); const general = { @@ -321,7 +361,7 @@ describe('Actions.Posts', () => { serverVersion: '5.4.0', config: {PostEditTimeLimit: -1}, } as unknown as GlobalState['entities']['general']; - const withLicenseState = {...initialState}; + const withLicenseState = {...state}; withLicenseState.entities.general = { ...withLicenseState.entities.general, ...general, @@ -338,7 +378,7 @@ describe('Actions.Posts', () => { // should not allow edit for pending post const newLatestPost = {...latestPost, pending_post_id: latestPost.id} as Post; - const withPendingPostState = {...initialState}; + const withPendingPostState = {...state}; withPendingPostState.entities.posts.posts[latestPost.id] = newLatestPost; testStore = mockStore(withPendingPostState); @@ -349,11 +389,11 @@ describe('Actions.Posts', () => { // should not save draft when it already exists const stateWithDraft = { - ...initialState, + ...state, storage: { - ...initialState.storage, + ...state.storage, storage: { - ...initialState.storage.storage, + ...state.storage.storage, edit_draft_latest_post_id: { timestamp: new Date(), value: {id: 'latest_post_id', user_id: 'current_user_id', message: 'test msg', channel_id: 'current_channel_id', type: 'normal'}, diff --git a/webapp/channels/src/actions/post_actions.ts b/webapp/channels/src/actions/post_actions.ts index 1c658c2c58..0d46318430 100644 --- a/webapp/channels/src/actions/post_actions.ts +++ b/webapp/channels/src/actions/post_actions.ts @@ -15,6 +15,7 @@ import * as PostActions from 'mattermost-redux/actions/posts'; import {createSchedulePost} from 'mattermost-redux/actions/scheduled_posts'; import * as ThreadActions from 'mattermost-redux/actions/threads'; import {getChannel, getMyChannelMember as getMyChannelMemberSelector} from 'mattermost-redux/selectors/entities/channels'; +import {makeGetFilesForPost} from 'mattermost-redux/selectors/entities/files'; import {getConfig} from 'mattermost-redux/selectors/entities/general'; import * as PostSelectors from 'mattermost-redux/selectors/entities/posts'; import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; @@ -323,9 +324,18 @@ export function unpinPost(postId: string): ActionFuncAsync { } export function setEditingPost(postId = '', refocusId = '', isRHS = false): ActionFunc { + const getFilesForPost = makeGetFilesForPost(); + return (dispatch, getState) => { const state = getState(); - const post = PostSelectors.getPost(state, postId); + let post = PostSelectors.getPost(state, postId); + + // getPost selectors doesn't include post's file metadata, so we need to add it manually + if (post.file_ids?.length) { + // if the post has files, get their metadata and insert it into the post object + const files = getFilesForPost(state, postId); + post = {...post, metadata: {...post.metadata, files}}; + } if (!post || post.pending_post_id === postId) { return {data: false}; diff --git a/webapp/channels/src/components/__snapshots__/file_upload_overlay.test.tsx.snap b/webapp/channels/src/components/__snapshots__/file_upload_overlay.test.tsx.snap deleted file mode 100644 index c816b92dad..0000000000 --- a/webapp/channels/src/components/__snapshots__/file_upload_overlay.test.tsx.snap +++ /dev/null @@ -1,112 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`components/FileUploadOverlay should match snapshot when file upload is showing with no overlay type 1`] = ` -
-
-
- Files - - - - - Logo -
-
-
-`; - -exports[`components/FileUploadOverlay should match snapshot when file upload is showing with overlay type of center 1`] = ` -
-
-
- Files - - - - - Logo -
-
-
-`; - -exports[`components/FileUploadOverlay should match snapshot when file upload is showing with overlay type of right 1`] = ` -
-
-
- Files - - - - - Logo -
-
-
-`; diff --git a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.scss b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.scss index 27a7706cf2..871b8fa577 100644 --- a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.scss +++ b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.scss @@ -22,6 +22,7 @@ padding: unset; } } + position: relative; display: flex; width: 100%; @@ -94,6 +95,7 @@ flex: 1; border: 2px solid rgba(var(--center-channel-color-rgb), 0.16); border-radius: 4px; + background-color: var(--center-channel-bg); &:focus-visible, &:focus-within, diff --git a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.test.tsx b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.test.tsx index 431cfdbaf5..b608cfaf0d 100644 --- a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.test.tsx +++ b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.test.tsx @@ -14,12 +14,13 @@ import type Textbox from 'components/textbox/textbox'; import mergeObjects from 'packages/mattermost-redux/test/merge_objects'; import {renderWithContext, userEvent, screen} from 'tests/react_testing_utils'; -import {StoragePrefixes} from 'utils/constants'; +import {Locations, StoragePrefixes} from 'utils/constants'; import {TestHelper} from 'utils/test_helper'; import type {PostDraft} from 'types/store/draft'; import AdvancedTextEditor from './advanced_text_editor'; +import type {Props} from './advanced_text_editor'; jest.mock('actions/views/drafts', () => ({ ...jest.requireActual('actions/views/drafts'), @@ -412,4 +413,36 @@ describe('components/avanced_text_editor/advanced_text_editor', () => { expect(screen.getByText('Editing this message with an \'@mention\' will not notify the recipient.')).toBeVisible(); }); + + it('should have file upload overlay', () => { + const props: Props = { + ...baseProps, + }; + + const {container, rerender} = renderWithContext( + , + ); + expect(container.querySelector('#createPostFileDropOverlay')).toBeVisible(); + + props.postId = 'post_id_1'; + rerender(); + expect(container.querySelector('#createCommentFileDropOverlay')).toBeVisible(); + + // in center channel editing a post + props.isInEditMode = true; + rerender(); + expect(container.querySelector('#editPostFileDropOverlay')).toBeVisible(); + + // in RHS editing a post + props.location = Locations.RHS_COMMENT; + rerender(); + expect(container.querySelector('#editPostFileDropOverlay')).toBeVisible(); + + // in threads + props.isThreadView = true; + rerender(); + expect(container.querySelector('#editPostFileDropOverlay')).toBeVisible(); + }); }); diff --git a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx index 9701b8d9de..c0c3b3ad0d 100644 --- a/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx +++ b/webapp/channels/src/components/advanced_text_editor/advanced_text_editor.tsx @@ -33,6 +33,11 @@ import {makeAsyncComponent} from 'components/async_load'; import AutoHeightSwitcher from 'components/common/auto_height_switcher'; import useDidUpdate from 'components/common/hooks/useDidUpdate'; import DeletePostModal from 'components/delete_post_modal'; +import { + DropOverlayIdCreateComment, + DropOverlayIdCreatePost, + DropOverlayIdEditPost, FileUploadOverlay, +} from 'components/file_upload_overlay/file_upload_overlay'; import RhsSuggestionList from 'components/suggestion/rhs_suggestion_list'; import SuggestionList from 'components/suggestion/suggestion_list'; import Textbox from 'components/textbox'; @@ -83,7 +88,7 @@ import './advanced_text_editor.scss'; const FileLimitStickyBanner = makeAsyncComponent('FileLimitStickyBanner', lazy(() => import('components/file_limit_sticky_banner'))); -type Props = { +export type Props = { /** * location of the advanced text editor in the UI (center channel / RHS) @@ -667,6 +672,27 @@ const AdvancedTextEditor = ({ /> ); + const fileUploadOverlay = useMemo(() => { + const overlayType = isRHS ? 'right' : 'center'; + const direction = 'horizontal'; + + return isInEditMode ? ( + + ) : ( + + ); + }, [isInEditMode, isRHS]); + const showFormattingSpacer = isMessageLong || showPreview || attachmentPreview || isRHS || isThreadView; const containsAtMentionsInMessage = allAtMentions(draft?.message)?.length > 0; @@ -712,6 +738,7 @@ const AdvancedTextEditor = ({ className={'AdvancedTextEditor__body'} disabled={isDisabled} > + {fileUploadOverlay}
{ + // new object creation is needed here to support sending a draft with files. + // In case of draft, the PostDraft object is fetched from the redux store, which is immutable. + // When user clicks 'Send Now' in drafts list, it will otherwise try to seta field on an immutable object. + // Hence, creating a new object here. + return { + ...draft, + file_ids: draft.fileInfos.map((fileInfo) => fileInfo.id), + }; + }, []); + const showNotifyAllModal = useCallback((mentions: string[], channelTimezoneCount: number, memberNotifyCount: number, onConfirm: () => void) => { dispatch(openModal({ modalId: ModalIdentifiers.NOTIFY_CONFIRM_MODAL, @@ -248,7 +259,7 @@ const useSubmit = ( })); }, [dispatch]); - const handleSubmit = useCallback(async (submittingDraft = draft, schedulingInfo?: SchedulingInfo, options?: CreatePostOptions) => { + const handleSubmit = useCallback(async (submittingDraftParam = draft, schedulingInfo?: SchedulingInfo, options?: CreatePostOptions) => { if (!channel) { return; } @@ -257,6 +268,7 @@ const useSubmit = ( return; } + const submittingDraft = setUpdatedFileIds(submittingDraftParam); setShowPreview(false); isDraftSubmitting.current = true; diff --git a/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx b/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx index 83bd8d220c..22e17fe907 100644 --- a/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx +++ b/webapp/channels/src/components/advanced_text_editor/use_upload_files.tsx @@ -14,7 +14,7 @@ import {getCurrentLocale} from 'selectors/i18n'; import FilePreview from 'components/file_preview'; import type {FilePreviewInfo} from 'components/file_preview/file_preview'; import FileUpload from 'components/file_upload'; -import type {FileUpload as FileUploadClass} from 'components/file_upload/file_upload'; +import type {FileUpload as FileUploadClass, TextEditorLocationType} from 'components/file_upload/file_upload'; import type TextboxClass from 'components/textbox/textbox'; import type {PostDraft} from 'types/store/draft'; @@ -34,7 +34,7 @@ const useUploadFiles = ( handleDraftChange: (draft: PostDraft, options?: {instant?: boolean; show?: boolean}) => void, focusTextbox: (forceFocust?: boolean) => void, setServerError: (err: (ServerError & { submittedMessage?: string }) | null) => void, - isInEditMode: boolean, + isPostBeingEdited?: boolean, ): [React.ReactNode, React.ReactNode] => { const locale = useSelector(getCurrentLocale); @@ -153,12 +153,14 @@ const useUploadFiles = ( ); } - let postType = 'post'; - if (postId) { + let postType: TextEditorLocationType = 'post'; + if (isPostBeingEdited) { + postType = 'edit_post'; + } else if (postId) { postType = isThreadView ? 'thread' : 'comment'; } - const fileUploadJSX = isDisabled || isInEditMode ? null : ( + const fileUploadJSX = isDisabled ? null : ( { id='app-content' className='app__content' > - + {this.props.isChannelBookmarksEnabled && } } -> +exports[`FileAttachment should match snapshot when file is deleted 1`] = ` +
- - - -
-
-
- - test.png - - - PNG - - - 100B - -
-
- - - -
-
- -`; - -exports[`FileAttachment should match snapshot, regular file 1`] = ` -} -> -
-
test.pdf PDF 100B
- - - + + + +
- +
+`; + +exports[`FileAttachment should match snapshot with thumbnail disabled 1`] = ` +
+
+ +
+ +
+
+
+ + test.pdf + + + PDF + + + 100B + +
+
+
+ + + +
+
+
+
+`; + +exports[`FileAttachment should match snapshot, after change from file to image 1`] = ` +
+
+ +
+ +
+
+
+ + test.png + + + PNG + + + 100B + +
+
+
+ + + +
+
+
+
+`; + +exports[`FileAttachment should match snapshot, regular file 1`] = ` +
+
+ +
+ +
+
+
+ + test.pdf + + + PDF + + + 100B + +
+
+
+ + + +
+
+
+
`; exports[`FileAttachment should match snapshot, regular image 1`] = ` -} -> +
test.png PNG 100B
- - - + + + +
- +
`; exports[`FileAttachment should match snapshot, small image 1`] = ` -} -> +
test.png PNG 100B
- - - + + + +
- +
`; exports[`FileAttachment should match snapshot, svg image 1`] = ` -} -> +
-
test.svg SVG 100B
- - - + + + +
- +
`; exports[`FileAttachment should match snapshot, when file is not loaded 1`] = ` -} -> +
test.pdf JPG 100B
- - - + + + +
- +
`; exports[`FileAttachment should match snapshot, with compact display 1`] = ` -} -> +
- - - + + + + + + + + + + + + + + test.pdf +
- +
`; exports[`FileAttachment should match snapshot, without compact display and without can download 1`] = ` -} -> +
-
test.pdf PDF 100B @@ -632,5 +585,66 @@ exports[`FileAttachment should match snapshot, without compact display and witho
- +
+`; + +exports[`FileAttachment should not render menu items when disable actions is set 1`] = ` +
+
+ +
+ +
+
+
+ + test.pdf + + + PDF + + + 100B + +
+
+
+ + + +
+
+
+
`; diff --git a/webapp/channels/src/components/file_attachment/file_attachment.test.tsx b/webapp/channels/src/components/file_attachment/file_attachment.test.tsx index 085b7477ba..eecae09f26 100644 --- a/webapp/channels/src/components/file_attachment/file_attachment.test.tsx +++ b/webapp/channels/src/components/file_attachment/file_attachment.test.tsx @@ -1,13 +1,13 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {shallow} from 'enzyme'; +import {screen, fireEvent} from '@testing-library/react'; import React from 'react'; import type {GlobalState} from '@mattermost/types/store'; import type {DeepPartial} from '@mattermost/types/utilities'; -import {renderWithContext, screen} from 'tests/react_testing_utils'; +import {renderWithContext} from 'tests/react_testing_utils'; import FileAttachment from './file_attachment'; @@ -65,8 +65,8 @@ describe('FileAttachment', () => { }; test('should match snapshot, regular file', () => { - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('non archived file does not show archived elements', () => { @@ -103,8 +103,8 @@ describe('FileAttachment', () => { size: 100, }; const props = {...baseProps, fileInfo}; - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('should match snapshot, small image', () => { @@ -117,8 +117,8 @@ describe('FileAttachment', () => { size: 100, }; const props = {...baseProps, fileInfo}; - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('should match snapshot, svg image', () => { @@ -131,8 +131,8 @@ describe('FileAttachment', () => { size: 100, }; const props = {...baseProps, fileInfo}; - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('should match snapshot, after change from file to image', () => { @@ -144,26 +144,26 @@ describe('FileAttachment', () => { height: 400, size: 100, }; - const wrapper = shallow(); - wrapper.setProps({...baseProps, fileInfo}); - expect(wrapper).toMatchSnapshot(); + const {rerender, container} = renderWithContext(); + rerender(); + expect(container).toMatchSnapshot(); }); test('should match snapshot, with compact display', () => { const props = {...baseProps, compactDisplay: true}; - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('should match snapshot, without compact display and without can download', () => { const props = {...baseProps, canDownloadFiles: false}; - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('should match snapshot, when file is not loaded', () => { - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); }); test('should blur file attachment link after click', () => { @@ -172,12 +172,12 @@ describe('FileAttachment', () => { const link = screen.getByText(baseProps.fileInfo.name); const blur = jest.spyOn(link, 'blur'); - screen.getByText(baseProps.fileInfo.name).click(); + fireEvent.click(link); expect(blur).toHaveBeenCalled(); }); describe('archived file', () => { - test('shows archived image instead of real image and explanatory test in compact mode', () => { + test('shows archived image instead of real image and explanatory text in compact mode', () => { const props = { ...baseProps, fileInfo: { @@ -192,7 +192,7 @@ describe('FileAttachment', () => { screen.getByText(/archived/); }); - test('shows archived image instead of real image and explanatory test in full mode', () => { + test('shows archived image instead of real image and explanatory text in full mode', () => { const props = { ...baseProps, fileInfo: { @@ -207,4 +207,34 @@ describe('FileAttachment', () => { screen.getByText(/This file is archived/); }); }); + + test('should match snapshot when file is deleted', () => { + const props = { + ...baseProps, + fileInfo: { + ...baseFileInfo, + delete_at: 10000000, + }, + }; + const {container} = renderWithContext(); + expect(container).toMatchSnapshot(); + }); + + test('should match snapshot with thumbnail disabled', () => { + const {container} = renderWithContext( + ); + expect(container).toMatchSnapshot(); + }); + + test('should not render menu items when disable actions is set', () => { + const {container} = renderWithContext( + ); + expect(container).toMatchSnapshot(); + }); }); diff --git a/webapp/channels/src/components/file_attachment/file_attachment.tsx b/webapp/channels/src/components/file_attachment/file_attachment.tsx index e69a15364d..aa15443f4b 100644 --- a/webapp/channels/src/components/file_attachment/file_attachment.tsx +++ b/webapp/channels/src/components/file_attachment/file_attachment.tsx @@ -52,6 +52,8 @@ type Props = PropsFromRedux & { compactDisplay?: boolean; disablePreview?: boolean; handleFileDropdownOpened?: (open: boolean) => void; + disableThumbnail?: boolean; + disableActions?: boolean; }; export default function FileAttachment(props: Props) { @@ -81,12 +83,14 @@ export default function FileAttachment(props: Props) { } const fileType = getFileType(fileInfo.extension); - if (fileType === FileTypes.IMAGE) { - const thumbnailUrl = getFileThumbnailUrl(fileInfo.id); + if (!props.disableThumbnail) { + if (fileType === FileTypes.IMAGE) { + const thumbnailUrl = getFileThumbnailUrl(fileInfo.id); - loadImage(thumbnailUrl, handleImageLoaded); - } else if (fileInfo.extension === FileTypes.SVG && props.enableSVGs) { - loadImage(getFileUrl(fileInfo.id), handleImageLoaded); + loadImage(thumbnailUrl, handleImageLoaded); + } else if (fileInfo.extension === FileTypes.SVG && props.enableSVGs) { + loadImage(getFileUrl(fileInfo.id), handleImageLoaded); + } } }; @@ -116,10 +120,12 @@ export default function FileAttachment(props: Props) { }, [props.fileInfo.extension, props.fileInfo.id, props.enableSVGs]); const onAttachmentClick = (e: React.MouseEvent) => { - if (props.fileInfo.archived) { + e.preventDefault(); + e.stopPropagation(); + + if (props.fileInfo.archived || props.disablePreview) { return; } - e.preventDefault(); if ('blur' in e.target) { (e.target as HTMLElement).blur(); @@ -264,13 +270,16 @@ export default function FileAttachment(props: Props) { href='#' onClick={onAttachmentClick} > - {loaded ? ( + {loaded && !props.disableThumbnail ? ( ) : ( -
+ )} ); @@ -313,7 +322,7 @@ export default function FileAttachment(props: Props) {
); - if (!fileInfo.archived) { + if (!fileInfo.archived && !props.disableActions) { fileActions = renderFileMenuItems(); } } diff --git a/webapp/channels/src/components/file_attachment/index.ts b/webapp/channels/src/components/file_attachment/index.ts index fbc3b1912a..2ab1187e65 100644 --- a/webapp/channels/src/components/file_attachment/index.ts +++ b/webapp/channels/src/components/file_attachment/index.ts @@ -17,11 +17,15 @@ import type {GlobalState} from 'types/store'; import FileAttachment from './file_attachment'; -function mapStateToProps(state: GlobalState) { +export type OwnProps = { + preventDownload?: boolean; +} + +function mapStateToProps(state: GlobalState, ownProps: OwnProps) { const config = getConfig(state); return { - canDownloadFiles: canDownloadFiles(config), + canDownloadFiles: !ownProps.preventDownload && canDownloadFiles(config), enableSVGs: config.EnableSVGs === 'true', enablePublicLink: config.EnablePublicLink === 'true', pluginMenuItems: getFilesDropdownPluginMenuItems(state), diff --git a/webapp/channels/src/components/file_attachment_list/file_attachment_list.test.tsx b/webapp/channels/src/components/file_attachment_list/file_attachment_list.test.tsx index c4ec472ef8..a4f8d75648 100644 --- a/webapp/channels/src/components/file_attachment_list/file_attachment_list.test.tsx +++ b/webapp/channels/src/components/file_attachment_list/file_attachment_list.test.tsx @@ -1,15 +1,17 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {shallow} from 'enzyme'; +import {screen} from '@testing-library/react'; import React from 'react'; -import FileAttachment from 'components/file_attachment'; -import SingleImageView from 'components/single_image_view'; +import type {PostMetadata} from '@mattermost/types/posts'; +import {renderWithContext} from 'tests/react_testing_utils'; import {TestHelper} from 'utils/test_helper'; -import FileAttachmentList from './file_attachment_list'; +import type {GlobalState} from 'types/store'; + +import FileAttachmentList from './index'; describe('FileAttachmentList', () => { const post = TestHelper.getPostMock({ @@ -17,9 +19,9 @@ describe('FileAttachmentList', () => { file_ids: ['file_id_1', 'file_id_2', 'file_id_3'], }); const fileInfos = [ - TestHelper.getFileInfoMock({id: 'file_id_3', name: 'image_3.png', extension: 'png', create_at: 3}), - TestHelper.getFileInfoMock({id: 'file_id_2', name: 'image_2.png', extension: 'png', create_at: 2}), - TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image_1.png', extension: 'png', create_at: 1}), + TestHelper.getFileInfoMock({id: 'file_id_3', name: 'image_3.png', extension: 'png', create_at: 3, post_id: post.id}), + TestHelper.getFileInfoMock({id: 'file_id_2', name: 'image_2.png', extension: 'png', create_at: 2, post_id: post.id}), + TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image_1.png', extension: 'png', create_at: 1, post_id: post.id}), ]; const baseProps = { post, @@ -35,83 +37,218 @@ describe('FileAttachmentList', () => { }, }; + const defaultState = { + entities: { + general: { + config: { + EnableSVGs: 'true', + }, + }, + posts: { + posts: { + post_id: post, + }, + }, + files: { + files: { + file_id_1: fileInfos[2], + file_id_2: fileInfos[1], + file_id_3: fileInfos[0], + }, + fileIdsByPostId: { + post_id: ['file_id_1', 'file_id_2', 'file_id_3'], + }, + }, + }, + } as unknown as GlobalState; + test('should render a FileAttachment for a single file', () => { const props = { ...baseProps, - fileCount: 1, - fileInfos: [ - TestHelper.getFileInfoMock({ - id: 'file_id_1', - name: 'file.txt', - extension: 'txt', - }), - ], }; - const wrapper = shallow( - , - ); + renderWithContext(, defaultState); - expect(wrapper.find(FileAttachment).exists()).toBe(true); + expect(screen.getByTestId('fileAttachmentList').querySelectorAll('.post-image__column').length).toBe(3); }); test('should render multiple, sorted FileAttachments for multiple files', () => { - const wrapper = shallow( - , - ); + renderWithContext(, defaultState); - expect(wrapper.find(FileAttachment)).toHaveLength(3); - expect(wrapper.find(FileAttachment).first().prop('fileInfo').id).toBe('file_id_1'); - expect(wrapper.find(FileAttachment).last().prop('fileInfo').id).toBe('file_id_3'); + const fileAttachments = Array.from(screen.getByTestId('fileAttachmentList').querySelectorAll('.post-image__column')); + expect(fileAttachments.length).toBe(3); + expect(fileAttachments[0]?.textContent?.includes('image_1.png')).toBe(true); + expect(fileAttachments[1]?.textContent?.includes('image_2.png')).toBe(true); + expect(fileAttachments[2]?.textContent?.includes('image_3.png')).toBe(true); }); test('should render a SingleImageView for a single image', () => { const props = { ...baseProps, - fileCount: 1, - fileInfos: [ - TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image.png', extension: 'png'}), - ], + post: { + ...baseProps.post, + file_ids: ['file_id_1'], + }, }; - const wrapper = shallow( - , - ); + const state = { + ...defaultState, + entities: { + files: { + files: { + file_id_1: fileInfos[0], + }, + fileIdsByPostId: { + post_id: ['file_id_1'], + }, + }, + }, + } as unknown as GlobalState; - expect(wrapper.find(SingleImageView).exists()).toBe(true); + const {container} = renderWithContext(, state); + + expect(container.querySelector('.file-view--single')).toBeInTheDocument(); }); test('should render a SingleImageView for an SVG with SVG previews enabled', () => { + const state = { + ...defaultState, + entities: { + general: { + config: { + EnableSVGs: 'true', + }, + }, + files: { + files: { + file_id_1: TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image.svg', extension: 'svg'}), + }, + fileIdsByPostId: { + post_id: ['file_id_1'], + }, + }, + }, + } as unknown as GlobalState; + const props = { ...baseProps, enableSVGs: true, - fileCount: 1, - fileInfos: [ - TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image.svg', extension: 'svg'}), - ], }; - const wrapper = shallow( - , - ); + const {container} = renderWithContext(, state); - expect(wrapper.find(SingleImageView).exists()).toBe(true); + expect(container.querySelector('.file-view--single')).toBeInTheDocument(); }); test('should render a FileAttachment for an SVG with SVG previews disabled', () => { + const state = { + ...defaultState, + entities: { + general: { + config: { + EnableSVGs: 'false', + }, + }, + files: { + files: { + file_id_1: TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image.svg', extension: 'svg'}), + }, + fileIdsByPostId: { + post_id: ['file_id_1'], + }, + }, + }, + } as unknown as GlobalState; + const props = { ...baseProps, - fileCount: 1, - fileInfos: [ - TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image.svg', extension: 'svg'}), - ], }; - const wrapper = shallow( - , - ); + renderWithContext(, state); - expect(wrapper.find(SingleImageView).exists()).toBe(false); - expect(wrapper.find(FileAttachment).exists()).toBe(true); + expect(screen.getByTestId('fileAttachmentList').querySelector('.file-view--single')).not.toBeInTheDocument(); + expect(screen.getByTestId('fileAttachmentList').querySelector('.post-image__column')).toBeInTheDocument(); + }); + + test('should render deleted files', () => { + const state = { + ...defaultState, + entities: { + files: { + files: { + file_id_1: TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image_1.png', extension: 'png', create_at: 1, delete_at: 4}), + file_id_2: TestHelper.getFileInfoMock({id: 'file_id_2', name: 'image_2.png', extension: 'png', create_at: 2, delete_at: 4}), + file_id_3: TestHelper.getFileInfoMock({id: 'file_id_3', name: 'image_3.png', extension: 'png', create_at: 3, delete_at: 4}), + }, + fileIdsByPostId: { + post_id: ['file_id_1', 'file_id_2', 'file_id_3'], + }, + }, + }, + } as unknown as GlobalState; + + const props = { + ...baseProps, + }; + renderWithContext(, state); + + const fileAttachments = screen.getByTestId('fileAttachmentList').querySelectorAll('.post-image__column'); + expect(fileAttachments.length).toBe(3); + expect(fileAttachments[0]?.textContent?.includes('image_1.png')).toBe(true); + expect(fileAttachments[1]?.textContent?.includes('image_2.png')).toBe(true); + expect(fileAttachments[2]?.textContent?.includes('image_3.png')).toBe(true); + }); + + test('should render file list in edit history RHS', () => { + const fileInfo1 = TestHelper.getFileInfoMock({id: 'file_id_1', name: 'image_1.png', extension: 'png', create_at: 1, delete_at: 4}); + const fileInfo2 = TestHelper.getFileInfoMock({id: 'file_id_2', name: 'image_2.png', extension: 'png', create_at: 2, delete_at: 4}); + const fileInfo3 = TestHelper.getFileInfoMock({id: 'file_id_3', name: 'image_3.png', extension: 'png', create_at: 3, delete_at: 4}); + + const state = { + ...defaultState, + entities: { + files: { + files: { + file_id_1: fileInfo1, + file_id_2: fileInfo2, + file_id_3: fileInfo3, + }, + fileIdsByPostId: { + post_id: ['file_id_1', 'file_id_2', 'file_id_3'], + }, + }, + posts: { + posts: { + post_id: { + ...post, + metadata: { + files: [fileInfo1, fileInfo2, fileInfo3], + }, + }, + }, + }, + }, + } as unknown as GlobalState; + + // in edit history RHS, files are deleted and download and context menus are disabled + const props = { + ...baseProps, + isEditHistory: true, + disableDownload: true, + disableActions: true, + post: { + ...post, + metadata: { + files: [fileInfo3, fileInfo2, fileInfo1], + } as PostMetadata, + }, + }; + renderWithContext(, state); + + const fileAttachments = screen.getByTestId('fileAttachmentList').querySelectorAll('.post-image__column'); + expect(fileAttachments.length).toBe(3); + expect(fileAttachments[0]?.textContent?.includes('image_1.png')).toBe(true); + expect(fileAttachments[1]?.textContent?.includes('image_2.png')).toBe(true); + expect(fileAttachments[2]?.textContent?.includes('image_3.png')).toBe(true); }); }); diff --git a/webapp/channels/src/components/file_attachment_list/file_attachment_list.tsx b/webapp/channels/src/components/file_attachment_list/file_attachment_list.tsx index bc484723cb..863b4054e6 100644 --- a/webapp/channels/src/components/file_attachment_list/file_attachment_list.tsx +++ b/webapp/channels/src/components/file_attachment_list/file_attachment_list.tsx @@ -39,6 +39,11 @@ export default function FileAttachmentList(props: Props) { } = props; const sortedFileInfos = useMemo(() => sortFileInfos(fileInfos ? [...fileInfos] : [], locale), [fileInfos, locale]); + + if (fileInfos.length === 0) { + return null; + } + if (fileInfos && fileInfos.length === 1 && !fileInfos[0].archived) { const fileType = getFileType(fileInfos[0].extension); @@ -50,6 +55,7 @@ export default function FileAttachmentList(props: Props) { postId={props.post.id} compactDisplay={compactDisplay} isInPermalink={isInPermalink} + disableActions={props.disableActions} /> ); } @@ -63,6 +69,7 @@ export default function FileAttachmentList(props: Props) { if (sortedFileInfos && sortedFileInfos.length > 0) { for (let i = 0; i < sortedFileInfos.length; i++) { const fileInfo = sortedFileInfos[i]; + const isDeleted = fileInfo.delete_at > 0; postFiles.push( , ); } diff --git a/webapp/channels/src/components/file_attachment_list/index.ts b/webapp/channels/src/components/file_attachment_list/index.ts index d745a54091..7e203ede73 100644 --- a/webapp/channels/src/components/file_attachment_list/index.ts +++ b/webapp/channels/src/components/file_attachment_list/index.ts @@ -6,9 +6,13 @@ import type {ConnectedProps} from 'react-redux'; import {bindActionCreators} from 'redux'; import type {Dispatch} from 'redux'; +import type {FileInfo} from '@mattermost/types/files'; import type {Post} from '@mattermost/types/posts'; -import {makeGetFilesForPost} from 'mattermost-redux/selectors/entities/files'; +import { + makeGetFilesForEditHistory, + makeGetFilesForPost, +} from 'mattermost-redux/selectors/entities/files'; import {getConfig} from 'mattermost-redux/selectors/entities/general'; import {openModal} from 'actions/views/modals'; @@ -24,14 +28,25 @@ export type OwnProps = { compactDisplay?: boolean; isInPermalink?: boolean; handleFileDropdownOpened?: (open: boolean) => void; + isEditHistory?: boolean; + disableDownload?: boolean; + disableActions?: boolean; } function makeMapStateToProps() { const selectFilesForPost = makeGetFilesForPost(); + const getFilesForEditHistory = makeGetFilesForEditHistory(); return function mapStateToProps(state: GlobalState, ownProps: OwnProps) { const postId = ownProps.post ? ownProps.post.id : ''; - const fileInfos = selectFilesForPost(state, postId); + + var fileInfos: FileInfo[]; + + if (ownProps.isEditHistory) { + fileInfos = getFilesForEditHistory(state, ownProps.post); + } else { + fileInfos = selectFilesForPost(state, postId); + } let fileCount = 0; if (ownProps.post.metadata && ownProps.post.metadata.files) { diff --git a/webapp/channels/src/components/file_upload/file_upload.test.tsx b/webapp/channels/src/components/file_upload/file_upload.test.tsx index 035b316a8f..771a70269d 100644 --- a/webapp/channels/src/components/file_upload/file_upload.test.tsx +++ b/webapp/channels/src/components/file_upload/file_upload.test.tsx @@ -8,13 +8,13 @@ import type {FileInfo} from '@mattermost/types/files'; import {General} from 'mattermost-redux/constants'; -import FileUpload, {type FileUpload as FileUploadClass} from 'components/file_upload/file_upload'; - import {shallowWithIntl} from 'tests/helpers/intl-test-helper'; import {clearFileInput} from 'utils/utils'; import type {FilesWillUploadHook} from 'types/store/plugins'; +import FileUpload, {type FileUpload as FileUploadClass} from './file_upload'; + const generatedIdRegex = /[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12}/; jest.mock('utils/file_utils', () => { @@ -66,12 +66,14 @@ describe('components/FileUpload', () => { onUploadError: jest.fn(), onUploadStart: jest.fn(), onUploadProgress: jest.fn(), - postType: 'post', + postType: 'post' as const, maxFileSize: MaxFileSize, canUploadFiles: true, rootId: 'root_id', pluginFileUploadMethods: [], pluginFilesWillUploadHooks: [], + centerChannelPostBeingEdited: false, + rhsPostBeingEdited: false, actions: { uploadFile, }, diff --git a/webapp/channels/src/components/file_upload/file_upload.tsx b/webapp/channels/src/components/file_upload/file_upload.tsx index 26527199f0..6257710f66 100644 --- a/webapp/channels/src/components/file_upload/file_upload.tsx +++ b/webapp/channels/src/components/file_upload/file_upload.tsx @@ -14,6 +14,11 @@ import type {FileInfo, FileUploadResponse} from '@mattermost/types/files'; import type {UploadFile} from 'actions/file_actions'; import type {FilePreviewInfo} from 'components/file_preview/file_preview'; +import { + DropOverlayIdCreateComment, + DropOverlayIdEditPost, + DropOverlayIdRHS, +} from 'components/file_upload_overlay/file_upload_overlay'; import KeyboardShortcutSequence, {KEYBOARD_SHORTCUTS} from 'components/keyboard_shortcuts/keyboard_shortcuts_sequence'; import Menu from 'components/widgets/menu/menu'; import MenuWrapper from 'components/widgets/menu/menu_wrapper'; @@ -75,6 +80,8 @@ const customStyles = { top: 'auto', }; +export type TextEditorLocationType = 'post' | 'comment' | 'thread' | 'edit_post'; + export type Props = { channelId: string; @@ -125,7 +132,7 @@ export type Props = { /** * Type of the object which the uploaded file is attached to */ - postType: string; + postType: TextEditorLocationType; /** * The maximum uploaded file size. @@ -147,6 +154,10 @@ export type Props = { * Function called when xhr fires progress event. */ onUploadProgress: (filePreviewInfo: FilePreviewInfo) => void; + + centerChannelPostBeingEdited: boolean; + rhsPostBeingEdited: boolean; + actions: { /** @@ -179,19 +190,60 @@ export class FileUpload extends PureComponent { this.fileInput = React.createRef(); } - componentDidMount() { - if (this.props.postType === 'post') { - this.registerDragEvents('.row.main', '.center-file-overlay'); - } else if (this.props.postType === 'comment') { - this.registerDragEvents('.post-right__container', '.right-file-overlay'); - } else if (this.props.postType === 'thread') { - this.registerDragEvents('.ThreadPane', '.right-file-overlay'); + getDragEventDefinition = () => { + let containerSelector: string; + let overlaySelector: string; + + switch (this.props.postType) { + case 'post': { + containerSelector = this.props.centerChannelPostBeingEdited ? 'form#create_post .AdvancedTextEditor__body' : '.row.main'; + overlaySelector = this.props.centerChannelPostBeingEdited ? '#createPostFileDropOverlay' : '.center-file-overlay'; + break; } + case 'comment': { + containerSelector = this.props.rhsPostBeingEdited ? '#sidebar-right .post-create__container .AdvancedTextEditor__body' : '.post-right__container'; + overlaySelector = this.props.rhsPostBeingEdited ? '#' + DropOverlayIdCreateComment : '#' + DropOverlayIdRHS; + break; + } + case 'thread': { + containerSelector = this.props.rhsPostBeingEdited ? '.post-create__container .AdvancedTextEditor__body' : '.ThreadPane'; + overlaySelector = this.props.rhsPostBeingEdited ? '#createPostFileDropOverlay' : '.right-file-overlay'; + break; + } + case 'edit_post': { + containerSelector = '.post--editing'; + overlaySelector = '#' + DropOverlayIdEditPost; + break; + } + } + + return { + containerSelector, + overlaySelector, + }; + }; + + componentDidMount() { + const {containerSelector, overlaySelector} = this.getDragEventDefinition(); + this.registerDragEvents(containerSelector, overlaySelector); document.addEventListener('paste', this.pasteUpload); document.addEventListener('keydown', this.keyUpload); } + componentDidUpdate(prevProps: Readonly) { + // when a post starts or finishes being edited, we need to + // clear existing drag handlers and register fresh ones in the right place. + if ( + prevProps.centerChannelPostBeingEdited !== this.props.centerChannelPostBeingEdited || + prevProps.rhsPostBeingEdited !== this.props.rhsPostBeingEdited + ) { + this.unbindDragsterEvents?.(); + const {containerSelector, overlaySelector} = this.getDragEventDefinition(); + this.registerDragEvents(containerSelector, overlaySelector); + } + } + componentWillUnmount() { document.removeEventListener('paste', this.pasteUpload); document.removeEventListener('keydown', this.keyUpload); @@ -368,13 +420,19 @@ export class FileUpload extends PureComponent { }; registerDragEvents = (containerSelector: string, overlaySelector: string) => { - const overlay = document.querySelector(overlaySelector); + let overlay = document.querySelector(overlaySelector); const dragTimeout = new DelayedAction(() => { overlay?.classList.add('hidden'); }); const enter = (e: CustomEvent) => { + // this null check is to deal with the race condition between rendering the post edit advanced text editor + // and this hook querying the same in DOM to register event handler on it. + if (!overlay) { + overlay = document.querySelector(overlaySelector); + } + const files = e.detail.dataTransfer; if (!isUriDrop(files) && isFileTransfer(files)) { overlay?.classList.remove('hidden'); diff --git a/webapp/channels/src/components/file_upload/index.ts b/webapp/channels/src/components/file_upload/index.ts index 4ea30b6721..7251cab965 100644 --- a/webapp/channels/src/components/file_upload/index.ts +++ b/webapp/channels/src/components/file_upload/index.ts @@ -9,6 +9,7 @@ import {getConfig} from 'mattermost-redux/selectors/entities/general'; import {uploadFile} from 'actions/file_actions'; import {getCurrentLocale} from 'selectors/i18n'; +import {getEditingPostDetailsAndPost} from 'selectors/posts'; import {canUploadFiles} from 'utils/file_utils'; @@ -21,12 +22,18 @@ function mapStateToProps(state: GlobalState) { const config = getConfig(state); const maxFileSize = parseInt(config.MaxFileSize || '', 10); + const editingPost = getEditingPostDetailsAndPost(state); + const centerChannelPostBeingEdited = editingPost.show && !editingPost.isRHS; + const rhsPostBeingEdited = editingPost.show && editingPost.isRHS; + return { maxFileSize, canUploadFiles: canUploadFiles(config), locale: getCurrentLocale(state), pluginFileUploadMethods: state.plugins.components.FileUploadMethod, pluginFilesWillUploadHooks: state.plugins.components.FilesWillUploadHook as unknown as FilesWillUploadHook[], + centerChannelPostBeingEdited, + rhsPostBeingEdited, }; } diff --git a/webapp/channels/src/components/file_upload_overlay.tsx b/webapp/channels/src/components/file_upload_overlay.tsx deleted file mode 100644 index f32afd821a..0000000000 --- a/webapp/channels/src/components/file_upload_overlay.tsx +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import React from 'react'; -import {FormattedMessage, useIntl} from 'react-intl'; - -import fileOverlayImage from 'images/filesOverlay.png'; -import overlayLogoImage from 'images/logoWhite.png'; - -type Props = { - overlayType: string; -} - -const FileUploadOverlay = (props: Props) => { - const {formatMessage} = useIntl(); - - let overlayClass = 'file-overlay hidden'; - if (props.overlayType === 'right') { - overlayClass += ' right-file-overlay'; - } else if (props.overlayType === 'center') { - overlayClass += ' center-file-overlay'; - } - - return ( -
-
-
- Files - - - - - Logo -
-
-
- ); -}; - -export default FileUploadOverlay; diff --git a/webapp/channels/src/components/file_upload_overlay/__snapshots__/file_upload_overlay.test.tsx.snap b/webapp/channels/src/components/file_upload_overlay/__snapshots__/file_upload_overlay.test.tsx.snap new file mode 100644 index 0000000000..be44051330 --- /dev/null +++ b/webapp/channels/src/components/file_upload_overlay/__snapshots__/file_upload_overlay.test.tsx.snap @@ -0,0 +1,79 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`components/FileUploadOverlay should match snapshot when file upload is showing with no overlay type 1`] = ` +
+
+
+ + +
+
+
+`; + +exports[`components/FileUploadOverlay should match snapshot when file upload is showing with overlay type of center 1`] = ` +
+
+
+ + +
+
+
+`; + +exports[`components/FileUploadOverlay should match snapshot when file upload is showing with overlay type of right 1`] = ` +
+
+
+ + +
+
+
+`; diff --git a/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.scss b/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.scss new file mode 100644 index 0000000000..9372f1da0d --- /dev/null +++ b/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.scss @@ -0,0 +1,88 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +@use "utils/functions"; +@use "utils/mixins"; +@use "utils/variables"; + +.file-overlay { + position: absolute; + z-index: 13; + top: 0; + left: 0; + width: 100%; + height: 100%; + color: variables.$white; + font-size: functions.em(20px); + font-weight: 600; + pointer-events: none; + text-align: center; + + .overlay__indent { + @include mixins.clearfix; + @include mixins.alpha-property(background-color, variables.$black, 0.75); + + position: relative; + display: flex; + height: 100%; + align-items: center; + justify-content: center; + } + + &.right-file-overlay { + font-size: functions.em(18px); + + .overlay__files { + width: 150px; + } + } + + .overlay__circle { + display: flex; + width: 300px; + height: 300px; + max-height: 100%; + flex-direction: column; + flex-wrap: wrap; + align-items: center; + justify-content: center; + gap: 20px; + pointer-events: none; + + &.horizontal { + width: max-content; + flex-direction: row; + + .overlay__files { + width: auto; + height: 96px; + max-height: 80%; + } + + span { + display: flex; + align-items: center; + } + } + } + + .overlay__files { + display: block; + width: 128px; + } + + .overlay__logo { + position: absolute; + bottom: 30px; + left: 50%; + width: 100px; + margin-left: -50px; + opacity: 0.3; + } + + .fa { + display: inline-block; + margin-right: 8px; + font-size: 1.1em; + } +} diff --git a/webapp/channels/src/components/file_upload_overlay.test.tsx b/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.test.tsx similarity index 84% rename from webapp/channels/src/components/file_upload_overlay.test.tsx rename to webapp/channels/src/components/file_upload_overlay/file_upload_overlay.test.tsx index ea5ac30910..7f2371bdd3 100644 --- a/webapp/channels/src/components/file_upload_overlay.test.tsx +++ b/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.test.tsx @@ -4,13 +4,14 @@ import {shallow} from 'enzyme'; import React from 'react'; -import FileUploadOverlay from 'components/file_upload_overlay'; +import FileUploadOverlay from 'components/file_upload_overlay/index'; describe('components/FileUploadOverlay', () => { test('should match snapshot when file upload is showing with no overlay type', () => { const wrapper = shallow( , ); @@ -21,6 +22,7 @@ describe('components/FileUploadOverlay', () => { const wrapper = shallow( , ); @@ -31,6 +33,7 @@ describe('components/FileUploadOverlay', () => { const wrapper = shallow( , ); diff --git a/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.tsx b/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.tsx new file mode 100644 index 0000000000..ac24447bbb --- /dev/null +++ b/webapp/channels/src/components/file_upload_overlay/file_upload_overlay.tsx @@ -0,0 +1,61 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import classNames from 'classnames'; +import React from 'react'; +import {FormattedMessage} from 'react-intl'; + +import fileOverlayImage from 'images/fileOverlay.svg'; + +import './file_upload_overlay.scss'; + +export const DropOverlayIdEditPost = 'editPostFileDropOverlay'; +export const DropOverlayIdCreateComment = 'createCommentFileDropOverlay'; +export const DropOverlayIdCreatePost = 'createPostFileDropOverlay'; +export const DropOverlayIdThreads = 'threadView'; +export const DropOverlayIdCenterChannel = 'centerChannelFileDropOverlay'; +export const DropOverlayIdRHS = 'rhsFileDropOverlay'; + +type Props = { + overlayType: string; + id: string; + isInEditMode?: boolean; + direction?: 'horizontal' | 'vertical'; +} + +export const FileUploadOverlay = (props: Props) => { + let overlayClass = 'file-overlay hidden'; + if (props.overlayType === 'right') { + overlayClass += ' right-file-overlay'; + } else if (props.overlayType === 'center') { + overlayClass += ' center-file-overlay'; + } + + if (props.isInEditMode) { + overlayClass += ' post_edit_mode'; + } + + const mode = props.direction || 'vertical'; + + return ( +
+
+
+ + +
+
+
+ ); +}; diff --git a/webapp/channels/src/components/file_upload_overlay/index.ts b/webapp/channels/src/components/file_upload_overlay/index.ts new file mode 100644 index 0000000000..12c127a0aa --- /dev/null +++ b/webapp/channels/src/components/file_upload_overlay/index.ts @@ -0,0 +1,6 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {FileUploadOverlay} from './file_upload_overlay'; + +export default FileUploadOverlay; diff --git a/webapp/channels/src/components/info_toast/__snapshots__/info_toast.test.tsx.snap b/webapp/channels/src/components/info_toast/__snapshots__/info_toast.test.tsx.snap index 02f33e69b4..6dc7f56e95 100644 --- a/webapp/channels/src/components/info_toast/__snapshots__/info_toast.test.tsx.snap +++ b/webapp/channels/src/components/info_toast/__snapshots__/info_toast.test.tsx.snap @@ -1,34 +1,38 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`components/InfoToast should match snapshot 1`] = ` - +
- + + + test - +
- +
`; diff --git a/webapp/channels/src/components/info_toast/info_toast.scss b/webapp/channels/src/components/info_toast/info_toast.scss index 296e09c7e1..ddbe8beab3 100644 --- a/webapp/channels/src/components/info_toast/info_toast.scss +++ b/webapp/channels/src/components/info_toast/info_toast.scss @@ -9,14 +9,16 @@ align-items: center; padding: 8px; border-radius: 4px; - background: var(--center-channel-text); + background: rgb(var(--center-channel-color-rgb)); box-shadow: 0 4px 6px rgba(0, 0, 0, 0.12); - color: var(--sidebar-text); + color: rgb(var(--center-channel-bg-rgb)); font-weight: 600; grid-template-columns: min-content auto min-content auto; line-height: 20px; .info-toast__icon_button { + border: none; + background: none; color: inherit; } diff --git a/webapp/channels/src/components/info_toast/info_toast.test.tsx b/webapp/channels/src/components/info_toast/info_toast.test.tsx index a0088fda33..3ae6decc0e 100644 --- a/webapp/channels/src/components/info_toast/info_toast.test.tsx +++ b/webapp/channels/src/components/info_toast/info_toast.test.tsx @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {shallow} from 'enzyme'; +import {render, screen, fireEvent} from '@testing-library/react'; import React from 'react'; import type {ComponentProps} from 'react'; @@ -21,34 +21,22 @@ describe('components/InfoToast', () => { }; test('should match snapshot', () => { - const wrapper = shallow( - , - ); - - expect(wrapper).toMatchSnapshot(); + const {container} = render(); + expect(container).toMatchSnapshot(); }); test('should close the toast on undo', () => { - const wrapper = shallow( - , - ); + render(); - wrapper.find('button').simulate('click'); + fireEvent.click(screen.getByText(/undo/i)); + expect(baseProps.content.undo).toHaveBeenCalled(); expect(baseProps.onExited).toHaveBeenCalled(); }); test('should close the toast on close button click', () => { - const wrapper = shallow( - , - ); + render(); - wrapper.find('.info-toast__icon_button').simulate('click'); + fireEvent.click(screen.getByRole('button', {name: /close/i})); expect(baseProps.onExited).toHaveBeenCalled(); }); }); diff --git a/webapp/channels/src/components/info_toast/info_toast.tsx b/webapp/channels/src/components/info_toast/info_toast.tsx index 074712485f..500e286a47 100644 --- a/webapp/channels/src/components/info_toast/info_toast.tsx +++ b/webapp/channels/src/components/info_toast/info_toast.tsx @@ -6,8 +6,6 @@ import React, {useEffect, useCallback} from 'react'; import {useIntl} from 'react-intl'; import {CSSTransition} from 'react-transition-group'; -import IconButton from '@mattermost/compass-components/components/icon-button'; // eslint-disable-line no-restricted-imports - import './info_toast.scss'; type Props = { @@ -64,13 +62,13 @@ function InfoToast({content, onExited, className}: Props): JSX.Element { })} )} - + aria-label={formatMessage({id: 'general_button.close', defaultMessage: 'Close'})} + > + +
); diff --git a/webapp/channels/src/components/post/post_component.test.tsx b/webapp/channels/src/components/post/post_component.test.tsx index c634ea1712..112016f347 100644 --- a/webapp/channels/src/components/post/post_component.test.tsx +++ b/webapp/channels/src/components/post/post_component.test.tsx @@ -324,4 +324,139 @@ describe('PostComponent', () => { }); }); }); + + describe('file list', () => { + test('should show file list in post', () => { + const fileInfo1 = TestHelper.getFileInfoMock({id: 'fileId1', name: 'file1.jpg'}); + const fileInfo2 = TestHelper.getFileInfoMock({id: 'fileId2', name: 'file2.jpg'}); + const fileInfo3 = TestHelper.getFileInfoMock({id: 'fileId3', name: 'file3.jpg'}); + + const post = TestHelper.getPostMock({file_ids: [fileInfo1.id, fileInfo2.id, fileInfo3.id]}); + + const state: DeepPartial = { + entities: { + posts: { + posts: { + [post.id]: post, + }, + }, + files: { + files: { + [fileInfo1.id]: fileInfo1, + [fileInfo2.id]: fileInfo2, + [fileInfo3.id]: fileInfo3, + }, + fileIdsByPostId: { + [baseProps.post.id]: ['fileId1', 'fileId2', 'fileId3'], + }, + }, + }, + }; + + const props = { + ...baseProps, + post, + }; + + const {container} = renderWithContext(, state); + expect(screen.getByTestId('fileAttachmentList')).toBeInTheDocument(); + expect(container.querySelectorAll('.post-image__column')).toHaveLength(3); + expect(container.querySelectorAll('.post-image__column')[0]).toHaveTextContent(fileInfo1.name); + expect(container.querySelectorAll('.post-image__column')[1]).toHaveTextContent(fileInfo2.name); + expect(container.querySelectorAll('.post-image__column')[2]).toHaveTextContent(fileInfo3.name); + }); + + test('should show file list in edit container when editing', () => { + const fileInfo1 = TestHelper.getFileInfoMock({id: 'fileId1', name: 'file1.jpg'}); + const fileInfo2 = TestHelper.getFileInfoMock({id: 'fileId2', name: 'file2.jpg'}); + const fileInfo3 = TestHelper.getFileInfoMock({id: 'fileId3', name: 'file3.jpg'}); + + const team = TestHelper.getTeamMock({id: 'team_id'}); + const channel = TestHelper.getChannelMock({team_id: team.id}); + + const post = TestHelper.getPostMock({ + file_ids: [fileInfo1.id, fileInfo2.id, fileInfo3.id], + channel_id: channel.id, + metadata: { + files: [fileInfo1, fileInfo2, fileInfo3], + }, + }); + + const state: DeepPartial = { + entities: { + posts: { + posts: { + [post.id]: post, + }, + }, + files: { + files: { + [fileInfo1.id]: fileInfo1, + [fileInfo2.id]: fileInfo2, + [fileInfo3.id]: fileInfo3, + }, + fileIdsByPostId: { + [post.id]: [fileInfo1.id, fileInfo2.id, fileInfo3.id], + }, + }, + channels: { + channels: { + [channel.id]: channel, + }, + roles: { + [channel.id]: new Set(['channel_member']), + }, + }, + teams: { + teams: { + [team.id]: team, + }, + }, + roles: { + roles: { + channel_member: {permissions: ['create_post']}, + }, + }, + }, + views: { + posts: { + editingPost: { + postId: post.id, + show: true, + }, + }, + }, + storage: { + storage: { + edit_draft_id: { + value: { + ...post, + }, + }, + }, + }, + }; + + const props = { + ...baseProps, + post, + isPostBeingEdited: true, + }; + + const {container} = renderWithContext(, state); + + // advanced text editor should be visible + expect(container.querySelector('.AdvancedTextEditor__body')).toBeInTheDocument(); + + // file attachment list should be visible inside advanced text editor + expect(container.querySelector('.AdvancedTextEditor__body .file-preview__container')).toBeInTheDocument(); + expect(container.querySelectorAll('.post-image__column')).toHaveLength(3); + expect(container.querySelectorAll('.post-image__column')[0]).toHaveTextContent(fileInfo1.name); + expect(container.querySelectorAll('.post-image__column')[1]).toHaveTextContent(fileInfo2.name); + expect(container.querySelectorAll('.post-image__column')[2]).toHaveTextContent(fileInfo3.name); + + // additionally, files should not be visible outside the advanced text editor + expect(screen.queryByTestId('fileAttachmentList')).not.toBeInTheDocument(); + }); + }); }); diff --git a/webapp/channels/src/components/post/post_component.tsx b/webapp/channels/src/components/post/post_component.tsx index 482760e2e6..6f81e7497f 100644 --- a/webapp/channels/src/components/post/post_component.tsx +++ b/webapp/channels/src/components/post/post_component.tsx @@ -516,6 +516,8 @@ const PostComponent = (props: Props): JSX.Element => { postAriaLabelDivTestId = 'rhsPostView'; } + const showFileAttachments = post.file_ids && post.file_ids.length > 0 && !props.isPostBeingEdited; + return ( <> {(isSearchResultItem || (props.location !== Locations.CENTER && (props.isPinnedPosts || props.isFlaggedPosts))) && } @@ -643,12 +645,13 @@ const PostComponent = (props: Props): JSX.Element => { slot2={} onTransitionEnd={() => document.dispatchEvent(new Event(AppEvents.FOCUS_EDIT_TEXTBOX))} /> - {post.file_ids && post.file_ids.length > 0 && - + { + showFileAttachments && + }
{props.isPostAcknowledgementsEnabled && post.metadata?.priority?.requested_ack && ( diff --git a/webapp/channels/src/components/post_edit_history/__snapshots__/post_edit_history.test.tsx.snap b/webapp/channels/src/components/post_edit_history/__snapshots__/post_edit_history.test.tsx.snap index fafb3778ad..7b7563e527 100644 --- a/webapp/channels/src/components/post_edit_history/__snapshots__/post_edit_history.test.tsx.snap +++ b/webapp/channels/src/components/post_edit_history/__snapshots__/post_edit_history.test.tsx.snap @@ -226,7 +226,6 @@ exports[`components/post_edit_history should match snapshot 1`] = ` id="searchResult_post_id" > ); @@ -175,14 +202,13 @@ const EditedPostItem = ({post, isCurrent = false, postCurrentVersion, theme, act - + > + + ); @@ -202,16 +228,14 @@ const EditedPostItem = ({post, isCurrent = false, postCurrentVersion, theme, act >
diff --git a/webapp/channels/src/components/threading/thread_viewer/__snapshots__/thread_viewer.test.tsx.snap b/webapp/channels/src/components/threading/thread_viewer/__snapshots__/thread_viewer.test.tsx.snap index 152e31716e..51f30cfdff 100644 --- a/webapp/channels/src/components/threading/thread_viewer/__snapshots__/thread_viewer.test.tsx.snap +++ b/webapp/channels/src/components/threading/thread_viewer/__snapshots__/thread_viewer.test.tsx.snap @@ -9,6 +9,7 @@ exports[`components/threading/ThreadViewer should match snapshot 1`] = ` className="post-right-comments-container" > {
<> - + {this.props.selected && ( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/webapp/channels/src/images/filesOverlay.png b/webapp/channels/src/images/filesOverlay.png deleted file mode 100644 index b74e4f4519..0000000000 Binary files a/webapp/channels/src/images/filesOverlay.png and /dev/null differ diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts index 1220ff6439..76863d3718 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/posts.ts @@ -1317,3 +1317,23 @@ export function unacknowledgePost(postId: string): ActionFuncAsync { return {data}; }; } + +export function restorePostVersion(postId: string, restoreVersionId: string, connectionId: string): ActionFuncAsync { + return async (dispatch, getState) => { + try { + await Client4.restorePostVersion(postId, restoreVersionId, connectionId); + } catch (error) { + // Send to error bar if it's an edit post error about time limit. + if (error.server_error_id === 'api.post.update_post.permissions_time_limit.app_error') { + dispatch(logError({type: 'announcement', message: error.message}, true)); + } else { + dispatch(logError(error)); + } + + forceLogoutIfNecessary(error, dispatch, getState); + return {error}; + } + + return {data: true}; + }; +} diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/files.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/files.ts index 3eac07b4af..d420d08a23 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/files.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/files.ts @@ -2,6 +2,7 @@ // See LICENSE.txt for license information. import type {FileInfo, FileSearchResultItem} from '@mattermost/types/files'; +import type {Post} from '@mattermost/types/posts'; import type {GlobalState} from '@mattermost/types/store'; import {createSelector} from 'mattermost-redux/selectors/create_selector'; @@ -37,7 +38,7 @@ export function makeGetFilesForPost(): (state: GlobalState, postId: string) => F 'makeGetFilesForPost', getAllFiles, getFilesIdsForPost, - getCurrentUserLocale, + (state) => getCurrentUserLocale(state), (allFiles, fileIdsForPost, locale) => { const fileInfos = fileIdsForPost.map((id) => allFiles[id]).filter((id) => Boolean(id)); @@ -46,6 +47,18 @@ export function makeGetFilesForPost(): (state: GlobalState, postId: string) => F ); } +export function makeGetFilesForEditHistory(): (state: GlobalState, editHistoryPost: Post) => FileInfo[] { + return createSelector( + 'makeGetFilesForEditHistory', + (state) => getCurrentUserLocale(state), + (state: GlobalState, editHistoryPost: Post) => editHistoryPost, + (userLocal, editHistoryPost) => { + const fileInfos = editHistoryPost?.metadata?.files ? [...editHistoryPost.metadata.files] : []; + return sortFileInfos(fileInfos, userLocal); + }, + ); +} + export const getSearchFilesResults: (state: GlobalState) => FileSearchResultItem[] = createSelector( 'getSearchFilesResults', getAllFilesFromSearch, diff --git a/webapp/channels/src/sass/components/_post.scss b/webapp/channels/src/sass/components/_post.scss index 22b8931b2b..362af297af 100644 --- a/webapp/channels/src/sass/components/_post.scss +++ b/webapp/channels/src/sass/components/_post.scss @@ -133,76 +133,6 @@ } } -.file-overlay { - position: absolute; - z-index: 13; - top: 0; - left: 0; - width: 100%; - height: 100%; - color: variables.$white; - font-size: functions.em(20px); - font-weight: 600; - pointer-events: none; - text-align: center; - - .overlay__indent { - @include mixins.clearfix; - @include mixins.alpha-property(background-color, variables.$black, 0.75); - - position: relative; - height: 100%; - } - - &.right-file-overlay { - font-size: functions.em(18px); - - .overlay__circle { - width: 300px; - height: 300px; - margin: -150px 0 0 -150px; - } - - .overlay__files { - width: 150px; - margin: 60px auto 15px; - } - } - - .overlay__circle { - position: absolute; - top: 50%; - left: 50%; - width: 370px; - height: 370px; - border-radius: 500px; - margin: -185px 0 0 -185px; - pointer-events: none; - - @include mixins.alpha-property(background, variables.$black, 0.7); - } - - .overlay__files { - display: block; - margin: 75px auto 20px; - } - - .overlay__logo { - position: absolute; - bottom: 30px; - left: 50%; - width: 100px; - margin-left: -50px; - opacity: 0.3; - } - - .fa { - display: inline-block; - margin-right: 8px; - font-size: 1.1em; - } -} - #post-list { position: relative; height: 100%; diff --git a/webapp/channels/src/sass/responsive/_mobile.scss b/webapp/channels/src/sass/responsive/_mobile.scss index 48f0a79772..6397185a5d 100644 --- a/webapp/channels/src/sass/responsive/_mobile.scss +++ b/webapp/channels/src/sass/responsive/_mobile.scss @@ -1278,12 +1278,6 @@ .overlay__circle { width: 300px; height: 300px; - margin: -150px 0 0 -150px; - } - - .overlay__files { - width: 150px; - margin: 60px auto 15px; } } diff --git a/webapp/channels/src/selectors/drafts.ts b/webapp/channels/src/selectors/drafts.ts index 9ae50ab4fd..3cc8a7c736 100644 --- a/webapp/channels/src/selectors/drafts.ts +++ b/webapp/channels/src/selectors/drafts.ts @@ -115,32 +115,43 @@ export function makeGetDraft() { rootId: '', }); - return (state: GlobalState, channelId: string, rootId = '', storageKey = ''): PostDraft => { - let prefixStorageKey = StoragePrefixes.DRAFT; - let suffixStorageKey = channelId; - if (rootId) { - prefixStorageKey = StoragePrefixes.COMMENT_DRAFT; - suffixStorageKey = rootId; - } - const key = storageKey || `${prefixStorageKey}${suffixStorageKey}`; + return createSelector( + 'makeGetDraft', + (_: GlobalState, channelId: string) => channelId, + (_: GlobalState, channelId: string, rootId = '') => rootId, + (state: GlobalState, channelId: string, rootId = '', storageKey = '') => { + let prefixStorageKey = StoragePrefixes.DRAFT; + let suffixStorageKey = channelId; + if (rootId) { + prefixStorageKey = StoragePrefixes.COMMENT_DRAFT; + suffixStorageKey = rootId; + } + const key = storageKey || `${prefixStorageKey}${suffixStorageKey}`; - const retrievedDraft = getGlobalItem(state, key, DEFAULT_DRAFT); + return getGlobalItem(state, key, DEFAULT_DRAFT); + }, + (channelId, rootId, retrievedDraftParam) => { + let retrievedDraft = retrievedDraftParam; + if (retrievedDraft.metadata?.files) { + retrievedDraft = {...retrievedDraft, fileInfos: retrievedDraft.metadata.files}; + } - // Check if the draft has the required values in its properties - const isDraftWithRequiredValues = typeof retrievedDraft.message !== 'undefined' && typeof retrievedDraft.uploadsInProgress !== 'undefined' && typeof retrievedDraft.fileInfos !== 'undefined'; + // Check if the draft has the required values in its properties + const isDraftWithRequiredValues = typeof retrievedDraft.message !== 'undefined' && typeof retrievedDraft.uploadsInProgress !== 'undefined' && typeof retrievedDraft.fileInfos !== 'undefined'; - // Check if draft's channelId or rootId mismatches with the passed one - const isDraftMismatched = retrievedDraft.channelId !== channelId || retrievedDraft.rootId !== rootId; + // Check if draft's channelId or rootId mismatches with the passed one + const isDraftMismatched = retrievedDraft.channelId !== channelId || retrievedDraft.rootId !== rootId; - if (isDraftWithRequiredValues && !isDraftMismatched) { - return retrievedDraft; - } + if (isDraftWithRequiredValues && !isDraftMismatched) { + return retrievedDraft; + } - return { - ...DEFAULT_DRAFT, - ...retrievedDraft, - channelId, - rootId, - }; - }; + return { + ...DEFAULT_DRAFT, + ...retrievedDraft, + channelId, + rootId, + }; + }, + ); } diff --git a/webapp/channels/src/types/store/draft.ts b/webapp/channels/src/types/store/draft.ts index 3e9403332e..9d36c3b4d2 100644 --- a/webapp/channels/src/types/store/draft.ts +++ b/webapp/channels/src/types/store/draft.ts @@ -29,13 +29,14 @@ export type PostDraft = { requested_ack?: boolean; persistent_notifications?: boolean; }; + files?: FileInfo[]; }; }; export function isPostDraftEmpty(draft: PostDraft): boolean { const hasMessage = draft.message.trim() !== ''; - const hasAttachment = draft.fileInfos.length > 0 || draft.file_ids?.length; - const hasUploadingFiles = draft.uploadsInProgress.length > 0; + const hasAttachment = draft.fileInfos?.length > 0; + const hasUploadingFiles = draft.uploadsInProgress?.length > 0; return !hasMessage && !hasAttachment && !hasUploadingFiles; } diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 14ff7fb905..d934199b5c 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -4270,6 +4270,13 @@ export default class Client4 { {method: 'delete', headers: {'Connection-Id': connectionId}}, ); }; + + restorePostVersion = (postId: string, restoreVersionId: string, connectionId: string) => { + return this.doFetchWithResponse( + `${this.getPostRoute(postId)}/restore/${restoreVersionId}`, + {method: 'post', headers: {'Connection-Id': connectionId}}, + ); + }; } export function parseAndMergeNestedHeaders(originalHeaders: any) {