diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index 31522e02d75..77e77931d5b 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -5329,6 +5329,32 @@ func TestGetPostStripActionIntegrations(t *testing.T) { require.Nil(t, action["integration"]) } +func waitForPostReminderEphemeral(t *testing.T, userWSClient *model.WebSocketClient, timeout time.Duration) *model.Post { + t.Helper() + timer := time.NewTimer(timeout) + defer timer.Stop() + for { + select { + case ev := <-userWSClient.EventChannel: + if ev.EventType() != model.WebsocketEventEphemeralMessage { + continue + } + data := ev.GetData() + postJSON, ok := data["post"].(string) + require.True(t, ok) + var parsedPost model.Post + err := json.Unmarshal([]byte(postJSON), &parsedPost) + require.NoError(t, err) + if parsedPost.GetProp("type") != model.PostTypeReminder { + continue + } + return &parsedPost + case <-timer.C: + require.Fail(t, "timed out waiting for post reminder ephemeral websocket event") + return nil + } + } +} func TestPostReminder(t *testing.T) { th := Setup(t).InitBasic(t) @@ -5352,39 +5378,53 @@ func TestPostReminder(t *testing.T) { user, _, err := client.GetUser(context.Background(), post.UserId, "") require.NoError(t, err) - var caught bool - func() { - for { - select { - case ev := <-userWSClient.EventChannel: - if ev.EventType() == model.WebsocketEventEphemeralMessage { - caught = true - data := ev.GetData() + parsedPost := waitForPostReminderEphemeral(t, userWSClient, 15*time.Second) + require.NotNil(t, parsedPost) - post, ok := data["post"].(string) - require.True(t, ok) + assert.Equal(t, model.PostTypeEphemeral, parsedPost.Type) + assert.Equal(t, th.BasicUser.Id, parsedPost.UserId) + assert.Equal(t, th.BasicPost.Id, parsedPost.RootId) - var parsedPost model.Post - err := json.Unmarshal([]byte(post), &parsedPost) - require.NoError(t, err) + require.Equal(t, float64(targetTime), parsedPost.GetProp("target_time").(float64)) + require.Equal(t, th.BasicPost.Id, parsedPost.GetProp("post_id").(string)) + require.Equal(t, user.Username, parsedPost.GetProp("username").(string)) + require.Equal(t, th.BasicTeam.Name, parsedPost.GetProp("team_name").(string)) +} - assert.Equal(t, model.PostTypeEphemeral, parsedPost.Type) - assert.Equal(t, th.BasicUser.Id, parsedPost.UserId) - assert.Equal(t, th.BasicPost.Id, parsedPost.RootId) +func TestPostReminderReplyEphemeralUsesThreadRoot(t *testing.T) { + th := Setup(t).InitBasic(t) + client := th.Client + userWSClient := th.CreateConnectedWebSocketClient(t) - require.Equal(t, float64(targetTime), parsedPost.GetProp("target_time").(float64)) - require.Equal(t, th.BasicPost.Id, parsedPost.GetProp("post_id").(string)) - require.Equal(t, user.Username, parsedPost.GetProp("username").(string)) - require.Equal(t, th.BasicTeam.Name, parsedPost.GetProp("team_name").(string)) - return - } - case <-time.After(15 * time.Second): - return - } - } - }() + replyPost := &model.Post{ + ChannelId: th.BasicChannel.Id, + RootId: th.BasicPost.Id, + Message: "reply for reminder " + model.NewId(), + } + replyCreated, _, err := client.CreatePost(context.Background(), replyPost) + require.NoError(t, err) - require.Truef(t, caught, "User should have received %s event", model.WebsocketEventEphemeralMessage) + targetTime := time.Now().UTC().Unix() + 1 + resp, err := client.SetPostReminder(context.Background(), &model.PostReminder{ + TargetTime: targetTime, + PostId: replyCreated.Id, + UserId: th.BasicUser.Id, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + + user, _, err := client.GetUser(context.Background(), replyCreated.UserId, "") + require.NoError(t, err) + + parsedPost := waitForPostReminderEphemeral(t, userWSClient, 15*time.Second) + require.NotNil(t, parsedPost) + + assert.Equal(t, model.PostTypeEphemeral, parsedPost.Type) + assert.Equal(t, th.BasicPost.Id, parsedPost.RootId, "ephemeral should be threaded under the thread root so RHS/center thread views show the ack") + assert.Equal(t, replyCreated.Id, parsedPost.GetProp("post_id").(string)) + require.Equal(t, float64(targetTime), parsedPost.GetProp("target_time").(float64)) + require.Equal(t, user.Username, parsedPost.GetProp("username").(string)) + require.Equal(t, th.BasicTeam.Name, parsedPost.GetProp("team_name").(string)) } func TestPostGetInfo(t *testing.T) { diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 1ea06c5fc55..3c92420a32c 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -2678,18 +2678,27 @@ func (a *App) populateEditHistoryFileMetadata(editHistoryPosts []*model.Post) *m } func (a *App) SetPostReminder(rctx request.CTX, postID, userID string, targetTime int64) *model.AppError { - // Store the reminder in the DB + remindedPost, postErr := a.GetSinglePost(rctx, postID, false) + if postErr != nil { + return postErr + } + // Ephemeral ack must use the thread root so CRT/RHS thread views (keyed by root id) show the confirmation. + ephemeralRootID := remindedPost.Id + if remindedPost.RootId != "" { + ephemeralRootID = remindedPost.RootId + } + + metadata, err := a.Srv().Store().Post().GetPostReminderMetadata(postID) + if err != nil { + return model.NewAppError("SetPostReminder", model.NoTranslation, nil, "", http.StatusInternalServerError).Wrap(err) + } + reminder := &model.PostReminder{ PostId: postID, UserId: userID, TargetTime: targetTime, } - err := a.Srv().Store().Post().SetPostReminder(reminder) - if err != nil { - return model.NewAppError("SetPostReminder", model.NoTranslation, nil, "", http.StatusInternalServerError).Wrap(err) - } - - metadata, err := a.Srv().Store().Post().GetPostReminderMetadata(postID) + err = a.Srv().Store().Post().SetPostReminder(reminder) if err != nil { return model.NewAppError("SetPostReminder", model.NoTranslation, nil, "", http.StatusInternalServerError).Wrap(err) } @@ -2710,7 +2719,7 @@ func (a *App) SetPostReminder(rctx request.CTX, postID, userID string, targetTim Id: model.NewId(), CreateAt: model.GetMillis(), UserId: userID, - RootId: postID, + RootId: ephemeralRootID, ChannelId: metadata.ChannelID, // It's okay to keep this non-translated. This is just a fallback. // The webapp will parse the timestamp and show that in user's local timezone.