diff --git a/api4/post.go b/api4/post.go index 54327bcbdd..6d314350e9 100644 --- a/api4/post.go +++ b/api4/post.go @@ -501,9 +501,6 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } - // Updating the file_ids of a post is not a supported operation and will be ignored - post.FileIds = nil - if !c.App.SessionHasPermissionToChannelByPost(c.App.Session, c.Params.PostId, model.PERMISSION_EDIT_POST) { c.SetPermissionError(model.PERMISSION_EDIT_POST) return @@ -515,6 +512,9 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Updating the file_ids of a post is not a supported operation and will be ignored + post.FileIds = originalPost.FileIds + if c.App.Session.UserId != originalPost.UserId { if !c.App.SessionHasPermissionToChannelByPost(c.App.Session, c.Params.PostId, model.PERMISSION_EDIT_OTHERS_POSTS) { c.SetPermissionError(model.PERMISSION_EDIT_OTHERS_POSTS) diff --git a/api4/post_test.go b/api4/post_test.go index b00878b50a..b4a9176d7a 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -587,95 +587,154 @@ func TestUpdatePost(t *testing.T) { th.App.SetLicense(model.NewTestLicense()) - post := &model.Post{ChannelId: channel.Id, Message: "zz" + model.NewId() + "a"} - rpost, resp := Client.CreatePost(post) - CheckNoError(t, resp) - - if rpost.Message != post.Message { - t.Fatal("full name didn't match") + fileIds := make([]string, 3) + data, err := testutils.ReadTestFile("test.png") + require.Nil(t, err) + for i := 0; i < len(fileIds); i++ { + fileResp, resp := Client.UploadFile(data, channel.Id, "test.png") + CheckNoError(t, resp) + fileIds[i] = fileResp.FileInfos[0].Id } - if rpost.EditAt != 0 { - t.Fatal("Newly created post shouldn't have EditAt set") - } + rpost, err := th.App.CreatePost(&model.Post{ + UserId: th.BasicUser.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + FileIds: fileIds, + }, channel, false) + require.Nil(t, err) - msg := "zz" + model.NewId() + " update post" - rpost.Message = msg - rpost.UserId = "" + assert.Equal(t, rpost.Message, rpost.Message, "full name didn't match") + 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") - rupost, resp := Client.UpdatePost(rpost.Id, rpost) - CheckNoError(t, resp) + t.Run("same message, fewer files", func(t *testing.T) { + msg := "zz" + model.NewId() + " update post" + rpost.Message = msg + rpost.UserId = "" - if rupost.Message != msg { - t.Fatal("failed to updates") - } - if rupost.EditAt == 0 { - t.Fatal("EditAt not updated for post") - } + rupost, resp := Client.UpdatePost(rpost.Id, &model.Post{ + Id: rpost.Id, + Message: rpost.Message, + FileIds: fileIds[0:2], // one fewer file id + }) + CheckNoError(t, resp) - msg1 := "#hashtag a" + model.NewId() + " update post again" - rpost.Message = msg1 - rpost.Props[model.PROPS_ADD_CHANNEL_MEMBER] = "no good" - rrupost, resp := Client.UpdatePost(rpost.Id, rpost) - CheckNoError(t, resp) + 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") - if rrupost.Message != msg1 && rrupost.Hashtags != "#hashtag" { - t.Fatal("failed to updates") - } + actual, resp := Client.GetPost(rpost.Id, "") + CheckNoError(t, resp) - if rrupost.Props[model.PROPS_ADD_CHANNEL_MEMBER] != nil { - t.Fatal("failed to sanitize Props['add_channel_member'], should be nil") - } - - rpost2, err := th.App.CreatePost(&model.Post{ChannelId: channel.Id, Message: "zz" + model.NewId() + "a", Type: model.POST_JOIN_LEAVE, UserId: th.BasicUser.Id}, channel, false) - if err != nil { - t.Fatal(err) - } - - up2 := &model.Post{Id: rpost2.Id, ChannelId: channel.Id, Message: "zz" + model.NewId() + " update post 2"} - _, resp = Client.UpdatePost(rpost2.Id, up2) - CheckBadRequestStatus(t, resp) - - rpost3, err := th.App.CreatePost(&model.Post{ChannelId: channel.Id, Message: "zz" + model.NewId() + "a", UserId: th.BasicUser.Id}, channel, false) - if err != nil { - t.Fatal(err) - } - - fileIds := model.StringArray{"abcdef", "geh"} - up3 := &model.Post{Id: rpost3.Id, ChannelId: channel.Id, Message: "zz" + model.NewId() + " update post 3", FileIds: fileIds} - rrupost3, resp := Client.UpdatePost(rpost3.Id, up3) - CheckNoError(t, resp) - assert.Empty(t, rrupost3.FileIds) - - up4 := &model.Post{Id: rpost3.Id, ChannelId: channel.Id, Message: "zz" + model.NewId() + " update post 3"} - up4.AddProp("attachments", []model.SlackAttachment{ - { - Text: "Hello World", - }, + 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") }) - rrupost3, resp = Client.UpdatePost(rpost3.Id, up4) - CheckNoError(t, resp) - assert.NotEqual(t, rpost3.EditAt, rrupost3.EditAt) - assert.NotEqual(t, rpost3.Attachments(), rrupost3.Attachments()) - Client.Logout() - _, resp = Client.UpdatePost(rpost.Id, rpost) - CheckUnauthorizedStatus(t, resp) + t.Run("new message, invalid props", func(t *testing.T) { + msg1 := "#hashtag a" + model.NewId() + " update post again" + rpost.Message = msg1 + rpost.Props[model.PROPS_ADD_CHANNEL_MEMBER] = "no good" + rrupost, resp := Client.UpdatePost(rpost.Id, rpost) + CheckNoError(t, resp) - th.LoginBasic2() - _, resp = Client.UpdatePost(rpost.Id, rpost) - CheckForbiddenStatus(t, resp) + assert.Equal(t, msg1, rrupost.Message, "failed to update message") + assert.Equal(t, "#hashtag", rrupost.Hashtags, "failed to update hashtags") + assert.Nil(t, rrupost.Props[model.PROPS_ADD_CHANNEL_MEMBER], "failed to sanitize Props['add_channel_member'], should be nil") - Client.Logout() + actual, resp := Client.GetPost(rpost.Id, "") + CheckNoError(t, resp) - th.LoginTeamAdmin() - _, resp = Client.UpdatePost(rpost.Id, rpost) - CheckForbiddenStatus(t, resp) + assert.Equal(t, msg1, actual.Message, "failed to update message") + assert.Equal(t, "#hashtag", actual.Hashtags, "failed to update hashtags") + assert.Nil(t, actual.Props[model.PROPS_ADD_CHANNEL_MEMBER], "failed to sanitize Props['add_channel_member'], should be nil") + }) - Client.Logout() + t.Run("join/leave post", func(t *testing.T) { + rpost2, err := th.App.CreatePost(&model.Post{ + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + Type: model.POST_JOIN_LEAVE, + UserId: th.BasicUser.Id, + }, channel, false) + require.Nil(t, err) - _, resp = th.SystemAdminClient.UpdatePost(rpost.Id, rpost) - CheckNoError(t, resp) + up2 := &model.Post{ + Id: rpost2.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + " update post 2", + } + _, resp := Client.UpdatePost(rpost2.Id, up2) + CheckBadRequestStatus(t, resp) + }) + + rpost3, err := th.App.CreatePost(&model.Post{ + ChannelId: channel.Id, + Message: "zz" + model.NewId() + "a", + UserId: th.BasicUser.Id, + }, channel, false) + require.Nil(t, err) + + 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, resp := Client.UpdatePost(rpost3.Id, up3) + CheckNoError(t, resp) + assert.Empty(t, rrupost3.FileIds) + + actual, resp := Client.GetPost(rpost.Id, "") + CheckNoError(t, resp) + assert.Equal(t, model.StringArray(fileIds), actual.FileIds) + }) + + t.Run("add slack attachments", func(t *testing.T) { + up4 := &model.Post{ + Id: rpost3.Id, + ChannelId: channel.Id, + Message: "zz" + model.NewId() + " update post 3", + } + up4.AddProp("attachments", []model.SlackAttachment{ + { + Text: "Hello World", + }, + }) + rrupost3, resp := Client.UpdatePost(rpost3.Id, up4) + CheckNoError(t, resp) + assert.NotEqual(t, rpost3.EditAt, rrupost3.EditAt) + assert.NotEqual(t, rpost3.Attachments(), rrupost3.Attachments()) + }) + + t.Run("logged out", func(t *testing.T) { + Client.Logout() + _, resp := Client.UpdatePost(rpost.Id, rpost) + CheckUnauthorizedStatus(t, resp) + }) + + t.Run("different user", func(t *testing.T) { + th.LoginBasic2() + _, resp := Client.UpdatePost(rpost.Id, rpost) + CheckForbiddenStatus(t, resp) + + Client.Logout() + }) + + t.Run("different user, but team admin", func(t *testing.T) { + th.LoginTeamAdmin() + _, resp := Client.UpdatePost(rpost.Id, rpost) + CheckForbiddenStatus(t, resp) + + Client.Logout() + }) + + t.Run("different user, but system admin", func(t *testing.T) { + _, resp := th.SystemAdminClient.UpdatePost(rpost.Id, rpost) + CheckNoError(t, resp) + }) } func TestUpdateOthersPostInDirectMessageChannel(t *testing.T) { @@ -711,98 +770,105 @@ func TestPatchPost(t *testing.T) { th.App.SetLicense(model.NewTestLicense()) + fileIds := make([]string, 3) + data, err := testutils.ReadTestFile("test.png") + require.Nil(t, err) + for i := 0; i < len(fileIds); i++ { + fileResp, resp := Client.UploadFile(data, channel.Id, "test.png") + CheckNoError(t, resp) + fileIds[i] = fileResp.FileInfos[0].Id + } + post := &model.Post{ ChannelId: channel.Id, IsPinned: true, Message: "#hashtag a message", Props: model.StringInterface{"channel_header": "old_header"}, - FileIds: model.StringArray{"file1", "file2"}, + FileIds: fileIds[0:2], HasReactions: true, } post, _ = Client.CreatePost(post) - patch := &model.PostPatch{} + var rpost *model.Post + t.Run("new message, props, files, HasReactions bit", func(t *testing.T) { + patch := &model.PostPatch{} - patch.IsPinned = model.NewBool(false) - patch.Message = model.NewString("#otherhashtag other message") - patch.Props = new(model.StringInterface) - *patch.Props = model.StringInterface{"channel_header": "new_header"} - patch.FileIds = new(model.StringArray) - *patch.FileIds = model.StringArray{"file1", "otherfile2", "otherfile3"} - patch.HasReactions = model.NewBool(false) + patch.IsPinned = model.NewBool(false) + patch.Message = model.NewString("#otherhashtag other message") + patch.Props = &model.StringInterface{"channel_header": "new_header"} + patchFileIds := model.StringArray(fileIds) // one extra file + patch.FileIds = &patchFileIds + patch.HasReactions = model.NewBool(false) - rpost, resp := Client.PatchPost(post.Id, patch) - CheckNoError(t, resp) + var resp *model.Response + rpost, resp = Client.PatchPost(post.Id, patch) + CheckNoError(t, resp) - if rpost.IsPinned { - t.Fatal("IsPinned did not update properly") - } - if rpost.Message != "#otherhashtag other message" { - t.Fatal("Message did not update properly") - } - if len(rpost.Props) != 1 { - t.Fatal("Props did not update properly") - } - if !reflect.DeepEqual(rpost.Props, *patch.Props) { - t.Fatal("Props did not update properly") - } - if rpost.Hashtags != "#otherhashtag" { - t.Fatal("Message did not update properly") - } - if len(rpost.FileIds) == 3 { - t.Fatal("FileIds should not update properly") - } - if reflect.DeepEqual(rpost.FileIds, *patch.FileIds) { - t.Fatal("FileIds should not update") - } - if rpost.HasReactions { - t.Fatal("HasReactions did not update properly") - } + assert.False(t, rpost.IsPinned, "IsPinned did not update properly") + assert.Equal(t, "#otherhashtag other message", rpost.Message, "Message did not update properly") + assert.Equal(t, *patch.Props, rpost.Props, "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.False(t, rpost.HasReactions, "HasReactions did not update properly") + }) - patch2 := &model.PostPatch{} - attachments := []model.SlackAttachment{ - { - Text: "Hello World", - }, - } - patch2.Props = new(model.StringInterface) - *patch2.Props = model.StringInterface{"attachments": attachments} - - rpost2, resp := Client.PatchPost(post.Id, patch2) - CheckNoError(t, resp) - assert.NotEmpty(t, rpost2.Props["attachments"]) - assert.NotEqual(t, rpost.EditAt, rpost2.EditAt) - - if r, err := Client.DoApiPut("/posts/"+post.Id+"/patch", "garbage"); err == nil { - t.Fatal("should have errored") - } else { - if r.StatusCode != http.StatusBadRequest { - t.Log("actual: " + strconv.Itoa(r.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusBadRequest)) - t.Fatal("wrong status code") + t.Run("add slack attachments", func(t *testing.T) { + patch2 := &model.PostPatch{} + attachments := []model.SlackAttachment{ + { + Text: "Hello World", + }, } - } + patch2.Props = &model.StringInterface{"attachments": attachments} - _, resp = Client.PatchPost("junk", patch) - CheckBadRequestStatus(t, resp) + rpost2, resp := Client.PatchPost(post.Id, patch2) + CheckNoError(t, resp) + assert.NotEmpty(t, rpost2.Props["attachments"]) + assert.NotEqual(t, rpost.EditAt, rpost2.EditAt) + }) - _, resp = Client.PatchPost(GenerateTestId(), patch) - CheckForbiddenStatus(t, resp) + t.Run("invalid requests", func(t *testing.T) { + r, err := Client.DoApiPut("/posts/"+post.Id+"/patch", "garbage") + require.EqualError(t, err, ": Invalid or missing post in request body, ") + require.Equal(t, http.StatusBadRequest, r.StatusCode, "wrong status code") - Client.Logout() - _, resp = Client.PatchPost(post.Id, patch) - CheckUnauthorizedStatus(t, resp) + patch := &model.PostPatch{} + _, resp := Client.PatchPost("junk", patch) + CheckBadRequestStatus(t, resp) + }) - th.LoginBasic2() - _, resp = Client.PatchPost(post.Id, patch) - CheckForbiddenStatus(t, resp) + t.Run("unknown post", func(t *testing.T) { + patch := &model.PostPatch{} + _, resp := Client.PatchPost(GenerateTestId(), patch) + CheckForbiddenStatus(t, resp) + }) - th.LoginTeamAdmin() - _, resp = Client.PatchPost(post.Id, patch) - CheckForbiddenStatus(t, resp) + t.Run("logged out", func(t *testing.T) { + Client.Logout() + patch := &model.PostPatch{} + _, resp := Client.PatchPost(post.Id, patch) + CheckUnauthorizedStatus(t, resp) + }) - _, resp = th.SystemAdminClient.PatchPost(post.Id, patch) - CheckNoError(t, resp) + t.Run("different user", func(t *testing.T) { + th.LoginBasic2() + patch := &model.PostPatch{} + _, resp := Client.PatchPost(post.Id, patch) + CheckForbiddenStatus(t, resp) + }) + + t.Run("different user, but team admin", func(t *testing.T) { + th.LoginTeamAdmin() + patch := &model.PostPatch{} + _, resp := Client.PatchPost(post.Id, patch) + CheckForbiddenStatus(t, resp) + }) + + t.Run("different user, but system admin", func(t *testing.T) { + patch := &model.PostPatch{} + _, resp := th.SystemAdminClient.PatchPost(post.Id, patch) + CheckNoError(t, resp) + }) } func TestPinPost(t *testing.T) {