From d8d65b8ec7c713f7b79e6f4d6a31b8500a69ead1 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Tue, 28 Jan 2025 10:02:03 +0100 Subject: [PATCH] [MM-62079] Rewrite TestCreatePost (#29903) --- server/channels/api4/post_test.go | 201 ++++++++++++++++++------------ 1 file changed, 123 insertions(+), 78 deletions(-) diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index b40ab87339..b74ea898d2 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -38,70 +38,75 @@ func TestCreatePost(t *testing.T) { defer th.TearDown() client := th.Client - post := &model.Post{ - ChannelId: th.BasicChannel.Id, - Message: "#hashtag a" + model.NewId() + "a", - Props: model.StringInterface{model.PropsAddChannelMember: "no good"}, - DeleteAt: 101, + basicPost := func() *model.Post { + p := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "#hashtag a" + model.NewId() + "a", + DeleteAt: 101, + } + p.AddProp(model.PropsAddChannelMember, "no good") + return p } - rpost, resp, err2 := client.CreatePost(context.Background(), post) + post := basicPost() + rootPost, resp2, err2 := client.CreatePost(context.Background(), post) require.NoError(t, err2) - CheckCreatedStatus(t, resp) + CheckCreatedStatus(t, resp2) + require.NotNil(t, rootPost) - require.Equal(t, post.Message, rpost.Message, "message didn't match") - require.Equal(t, "#hashtag", rpost.Hashtags, "hashtag didn't match") - require.Empty(t, rpost.FileIds) - require.Equal(t, 0, int(rpost.EditAt), "newly created post shouldn't have EditAt set") - require.Nil(t, rpost.GetProp(model.PropsAddChannelMember), "newly created post shouldn't have Props['add_channel_member'] set") - require.Equal(t, 0, int(rpost.DeleteAt), "newly created post shouldn't have DeleteAt set") + require.Equal(t, post.Message, rootPost.Message, "message didn't match") + require.Equal(t, "#hashtag", rootPost.Hashtags, "hashtag didn't match") + require.Empty(t, rootPost.FileIds) + require.Equal(t, 0, int(rootPost.EditAt), "newly created post shouldn't have EditAt set") + require.Nil(t, rootPost.GetProp(model.PropsAddChannelMember), "newly created post shouldn't have Props['add_channel_member'] set") + require.Equal(t, 0, int(rootPost.DeleteAt), "newly created post shouldn't have DeleteAt set") - post.RootId = rpost.Id - _, _, err2 = client.CreatePost(context.Background(), post) + post = basicPost() + post.RootId = rootPost.Id + childPost, resp2, err2 := client.CreatePost(context.Background(), post) require.NoError(t, err2) - - post.RootId = "junk" - _, resp, err2 = client.CreatePost(context.Background(), post) - require.Error(t, err2) - CheckBadRequestStatus(t, resp) - - post2 := &model.Post{ChannelId: th.BasicChannel2.Id, Message: "zz" + model.NewId() + "a", CreateAt: 123} - rpost2, _, _ := client.CreatePost(context.Background(), post2) - require.NotEqual(t, post2.CreateAt, rpost2.CreateAt, "create at should not match") + CheckCreatedStatus(t, resp2) + require.NotNil(t, childPost) t.Run("with file uploaded by same user", func(t *testing.T) { - fileResp, _, err := client.UploadFile(context.Background(), []byte("data"), th.BasicChannel.Id, "test") + fileResp, resp, err := client.UploadFile(context.Background(), []byte("data"), th.BasicChannel.Id, "test") require.NoError(t, err) + CheckCreatedStatus(t, resp) fileId := fileResp.FileInfos[0].Id - postWithFiles, _, err := client.CreatePost(context.Background(), &model.Post{ + postWithFiles, resp, err := client.CreatePost(context.Background(), &model.Post{ ChannelId: th.BasicChannel.Id, Message: "with files", FileIds: model.StringArray{fileId}, }) require.NoError(t, err) + CheckCreatedStatus(t, resp) assert.Equal(t, model.StringArray{fileId}, postWithFiles.FileIds) - actualPostWithFiles, _, err := client.GetPost(context.Background(), postWithFiles.Id, "") + actualPostWithFiles, resp, err := client.GetPost(context.Background(), postWithFiles.Id, "") require.NoError(t, err) + CheckOKStatus(t, resp) assert.Equal(t, model.StringArray{fileId}, actualPostWithFiles.FileIds) }) t.Run("with file uploaded by different user", func(t *testing.T) { - fileResp, _, err := th.SystemAdminClient.UploadFile(context.Background(), []byte("data"), th.BasicChannel.Id, "test") + fileResp, resp, err := th.SystemAdminClient.UploadFile(context.Background(), []byte("data"), th.BasicChannel.Id, "test") require.NoError(t, err) + CheckCreatedStatus(t, resp) fileId := fileResp.FileInfos[0].Id - postWithFiles, _, err := client.CreatePost(context.Background(), &model.Post{ + postWithFiles, resp, err := client.CreatePost(context.Background(), &model.Post{ ChannelId: th.BasicChannel.Id, Message: "with files", FileIds: model.StringArray{fileId}, }) require.NoError(t, err) + CheckCreatedStatus(t, resp) assert.Empty(t, postWithFiles.FileIds) - actualPostWithFiles, _, err := client.GetPost(context.Background(), postWithFiles.Id, "") + actualPostWithFiles, resp, err := client.GetPost(context.Background(), postWithFiles.Id, "") require.NoError(t, err) + CheckOKStatus(t, resp) assert.Empty(t, actualPostWithFiles.FileIds) }) @@ -110,67 +115,73 @@ func TestCreatePost(t *testing.T) { require.Nil(t, appErr) fileId := fileInfo.Id - postWithFiles, _, err := client.CreatePost(context.Background(), &model.Post{ + postWithFiles, resp, err := client.CreatePost(context.Background(), &model.Post{ ChannelId: th.BasicChannel.Id, Message: "with files", FileIds: model.StringArray{fileId}, }) require.NoError(t, err) + CheckCreatedStatus(t, resp) assert.Equal(t, model.StringArray{fileId}, postWithFiles.FileIds) - actualPostWithFiles, _, err := client.GetPost(context.Background(), postWithFiles.Id, "") + actualPostWithFiles, resp, err := client.GetPost(context.Background(), postWithFiles.Id, "") require.NoError(t, err) + CheckOKStatus(t, resp) assert.Equal(t, model.StringArray{fileId}, actualPostWithFiles.FileIds) }) t.Run("Create posts without the USE_CHANNEL_MENTIONS Permission - returns ephemeral message with mentions and no ephemeral message without mentions", func(t *testing.T) { - WebSocketClient, err := th.CreateWebSocketClient() - WebSocketClient.Listen() - require.NoError(t, err) + wsClient := th.CreateConnectedWebSocketClient(t) defaultPerms := th.SaveDefaultRolePermissions() defer th.RestoreDefaultRolePermissions(defaultPerms) th.RemovePermissionFromRole(model.PermissionUseChannelMentions.Id, model.ChannelUserRoleId) - post.RootId = rpost.Id + post := basicPost() + post.RootId = rootPost.Id post.Message = "a post with no channel mentions" - _, _, err = client.CreatePost(context.Background(), post) + rPost, resp, err := client.CreatePost(context.Background(), post) require.NoError(t, err) + CheckCreatedStatus(t, resp) + require.NotNil(t, rPost) // Message with no channel mentions should result in no ephemeral message timeout := time.After(5 * time.Second) waiting := true for waiting { select { - case event := <-WebSocketClient.EventChannel: + case event := <-wsClient.EventChannel: require.NotEqual(t, model.WebsocketEventEphemeralMessage, event.EventType(), "should not have ephemeral message event") case <-timeout: waiting = false } } - post.RootId = rpost.Id post.Message = "a post with @channel" - _, _, err = client.CreatePost(context.Background(), post) + rPost, resp, err = client.CreatePost(context.Background(), post) require.NoError(t, err) + CheckCreatedStatus(t, resp) + require.NotNil(t, rPost) - post.RootId = rpost.Id post.Message = "a post with @all" - _, _, err = client.CreatePost(context.Background(), post) + rPost, resp, err = client.CreatePost(context.Background(), post) require.NoError(t, err) + CheckCreatedStatus(t, resp) + require.NotNil(t, rPost) - post.RootId = rpost.Id post.Message = "a post with @here" - _, _, err = client.CreatePost(context.Background(), post) + rPost, resp, err = client.CreatePost(context.Background(), post) require.NoError(t, err) + CheckCreatedStatus(t, resp) + require.NotNil(t, rPost) timeout = time.After(5 * time.Second) expectedEvents := 3 // 3 Posts created with @ mentions should result in 3 websocket events gotEvents := 0 for gotEvents < expectedEvents { select { - case event := <-WebSocketClient.EventChannel: + case event := <-wsClient.EventChannel: if event.EventType() == model.WebsocketEventEphemeralMessage { gotEvents++ } @@ -191,7 +202,7 @@ func TestCreatePost(t *testing.T) { *cfg.ServiceSettings.ExperimentalEnableHardenedMode = originalHardenedModeSetting }) - _, postResp, postErr := client.CreatePost(context.Background(), &model.Post{ + rpost, postResp, postErr := client.CreatePost(context.Background(), &model.Post{ ChannelId: th.BasicChannel.Id, Message: "with props", Props: model.StringInterface{model.PostPropsFromWebhook: "true"}, @@ -199,46 +210,80 @@ func TestCreatePost(t *testing.T) { require.Error(t, postErr) CheckBadRequestStatus(t, postResp) + assert.Nil(t, rpost) }) - post.RootId = "" - post.Type = model.PostTypeSystemGeneric - _, resp, err := client.CreatePost(context.Background(), post) - require.Error(t, err) - CheckBadRequestStatus(t, resp) + t.Run("invalid post type", func(t *testing.T) { + post := basicPost() + post.Type = model.PostTypeSystemGeneric + rpost, resp, err := client.CreatePost(context.Background(), post) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + assert.Nil(t, rpost) + }) - post.Type = "" - post.RootId = rpost2.Id - _, resp, err = client.CreatePost(context.Background(), post) - require.Error(t, err) - CheckBadRequestStatus(t, resp) + t.Run("invalid rootId type", func(t *testing.T) { + post := basicPost() + post.RootId = "junk" + rpost, resp, err := client.CreatePost(context.Background(), post) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + assert.Nil(t, rpost) + }) - post.RootId = "" - post.ChannelId = "junk" - _, resp, err = client.CreatePost(context.Background(), post) - require.Error(t, err) - CheckForbiddenStatus(t, resp) + t.Run("RootId points to child post", func(t *testing.T) { + post := basicPost() + post.RootId = childPost.Id + rpost, resp, err := client.CreatePost(context.Background(), post) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + assert.Nil(t, rpost) + }) - post.ChannelId = model.NewId() - _, resp, err = client.CreatePost(context.Background(), post) - require.Error(t, err) - CheckForbiddenStatus(t, resp) + t.Run("invalid ChannelId", func(t *testing.T) { + post := basicPost() + post.ChannelId = "junk" + rpost, resp, err := client.CreatePost(context.Background(), post) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + assert.Nil(t, rpost) + }) - r, err := client.DoAPIPost(context.Background(), "/posts", "garbage") - require.Error(t, err) - require.Equal(t, http.StatusBadRequest, r.StatusCode) + t.Run("invalid ChannelId", func(t *testing.T) { + post := basicPost() + post.ChannelId = model.NewId() + rpost, resp, err := client.CreatePost(context.Background(), post) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + assert.Nil(t, rpost) + }) - _, err = client.Logout(context.Background()) - require.NoError(t, err) - _, resp, err = client.CreatePost(context.Background(), post) - require.Error(t, err) - CheckUnauthorizedStatus(t, resp) + t.Run("invalid payload", func(t *testing.T) { + r, err := client.DoAPIPost(context.Background(), "/posts", "garbage") + require.Error(t, err) + require.Equal(t, http.StatusBadRequest, r.StatusCode) + }) - post.ChannelId = th.BasicChannel.Id - post.CreateAt = 123 - rpost, _, err = th.SystemAdminClient.CreatePost(context.Background(), post) - require.NoError(t, err) - require.Equal(t, post.CreateAt, rpost.CreateAt, "create at should match") + t.Run("not logged in", func(t *testing.T) { + resp, err := client.Logout(context.Background()) + require.NoError(t, err) + CheckOKStatus(t, resp) + + post := basicPost() + rpost, resp, err := client.CreatePost(context.Background(), post) + require.Error(t, err) + CheckUnauthorizedStatus(t, resp) + assert.Nil(t, rpost) + }) + + t.Run("CreateAt should match the one provided in the request", func(t *testing.T) { + post := basicPost() + post.CreateAt = 123 + rpost, resp, err := th.SystemAdminClient.CreatePost(context.Background(), post) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + assert.Equal(t, post.CreateAt, rpost.CreateAt, "create at should match") + }) t.Run("Should not be able to define the RemoteId of a post from the API", func(t *testing.T) { newPost := &model.Post{