From 4b04defea6802634811c18b6c380f12c91749885 Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Thu, 17 Jun 2021 09:39:24 +0530 Subject: [PATCH] MM-35699: CRT off should behave same as CRT unsupported (#17755) Summary: CRT for a user that has turned it off should behave the same as though the client does not support CRT Ticket Link: https://mattermost.atlassian.net/browse/MM-35699 --- app/channel.go | 48 ++++++------- app/channel_test.go | 172 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 26 deletions(-) diff --git a/app/channel.go b/app/channel.go index b90db2b1f6..475d04d503 100644 --- a/app/channel.go +++ b/app/channel.go @@ -2454,9 +2454,21 @@ func (a *App) UpdateChannelLastViewedAt(channelIDs []string, userID string) *mod return nil } +func (a *App) isCRTEnabledForUser(userID string) bool { + if *a.Config().ServiceSettings.CollapsedThreads == model.COLLAPSED_THREADS_DISABLED { + return false + } + threadsEnabled := *a.Config().ServiceSettings.CollapsedThreads == model.COLLAPSED_THREADS_DEFAULT_ON + // check if a participant has overridden collapsed threads settings + if preference, err := a.Srv().Store.Preference().Get(userID, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_COLLAPSED_THREADS_ENABLED); err == nil { + threadsEnabled = preference.Value == "on" + } + return threadsEnabled +} + // MarkChanelAsUnreadFromPost will take a post and set the channel as unread from that one. func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string, collapsedThreadsSupported bool) (*model.ChannelUnreadAt, *model.AppError) { - if !collapsedThreadsSupported { + if !collapsedThreadsSupported || !a.isCRTEnabledForUser(userID) { return a.markChannelAsUnreadFromPostCRTUnsupported(postID, userID) } post, err := a.GetSinglePost(postID) @@ -2476,7 +2488,7 @@ func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string, collapse // if auto-follow is on // if threadmembership does not exists we create one and update - if *a.Config().ServiceSettings.ThreadAutoFollow && collapsedThreadsSupported { + if *a.Config().ServiceSettings.ThreadAutoFollow { threadId := post.RootId if post.RootId == "" { threadId = post.Id @@ -2525,15 +2537,9 @@ func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string, collapse thread.Post.SanitizeProps() payload := thread.ToJson() - sendEvent := *a.Config().ServiceSettings.CollapsedThreads == model.COLLAPSED_THREADS_DEFAULT_ON - if preference, err := a.Srv().Store.Preference().Get(userID, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_COLLAPSED_THREADS_ENABLED); err == nil { - sendEvent = preference.Value == "on" - } - if sendEvent { - message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_UPDATED, channel.TeamId, "", userID, nil) - message.Add("thread", payload) - a.Publish(message) - } + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_UPDATED, channel.TeamId, "", userID, nil) + message.Add("thread", payload) + a.Publish(message) } } } @@ -2637,11 +2643,7 @@ func (a *App) markChannelAsUnreadFromPostCRTUnsupported(postID string, userID st thread.Post.SanitizeProps() payload := thread.ToJson() - sendEvent := *a.Config().ServiceSettings.CollapsedThreads == model.COLLAPSED_THREADS_DEFAULT_ON - if preference, err := a.Srv().Store.Preference().Get(userID, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_COLLAPSED_THREADS_ENABLED); err == nil { - sendEvent = preference.Value == "on" - } - if sendEvent { + if a.isCRTEnabledForUser(userID) { message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_UPDATED, channel.TeamId, "", userID, nil) message.Add("thread", payload) a.Publish(message) @@ -2845,18 +2847,12 @@ func (a *App) MarkChannelsAsViewed(channelIDs []string, userID string, currentSe a.clearPushNotification(currentSessionId, userID, channelID) } - if !collapsedThreadsSupported { - // for compatibility with old clients, when channel is viewed - mark all threads in that channel as read - threadsEnabled := *a.Config().ServiceSettings.CollapsedThreads == model.COLLAPSED_THREADS_DEFAULT_ON - // check if a participant has overridden collapsed threads settings - if preference, err := a.Srv().Store.Preference().Get(userID, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_COLLAPSED_THREADS_ENABLED); err == nil { - threadsEnabled = preference.Value == "on" + if !collapsedThreadsSupported || !a.isCRTEnabledForUser(userID) { + if err := a.Srv().Store.Thread().MarkAllAsReadInChannels(userID, channelIDs); err != nil { + return nil, model.NewAppError("MarkChannelsAsViewed", "app.channel.update_last_viewed_at.app_error", nil, err.Error(), http.StatusInternalServerError) } - if threadsEnabled { - if err := a.Srv().Store.Thread().MarkAllAsReadInChannels(userID, channelIDs); err != nil { - return nil, model.NewAppError("MarkChannelsAsViewed", "app.channel.update_last_viewed_at.app_error", nil, err.Error(), http.StatusInternalServerError) - } + if a.isCRTEnabledForUser(userID) { timestamp := model.GetMillis() for _, channelID := range channelIDs { message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_THREAD_READ_CHANGED, "", channelID, userID, nil) diff --git a/app/channel_test.go b/app/channel_test.go index 98d4cc864e..4972f73679 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "net/http" + "os" "sort" "strings" "sync" @@ -1978,8 +1979,11 @@ func TestMarkChannelsAsViewedPanic(t *testing.T) { require.NoError(t, err) mockPreferenceStore := mocks.PreferenceStore{} mockPreferenceStore.On("Get", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(&model.Preference{Value: "test"}, nil) + mockThreadStore := mocks.ThreadStore{} + mockThreadStore.On("MarkAllAsReadInChannels", "userID", []string{"channelID"}).Return(nil) mockStore.On("Channel").Return(&mockChannelStore) mockStore.On("Preference").Return(&mockPreferenceStore) + mockStore.On("Thread").Return(&mockThreadStore) _, appErr := th.App.MarkChannelsAsViewed([]string{"channelID"}, "userID", th.Context.Session().Id, false) require.Nil(t, appErr) @@ -2027,3 +2031,171 @@ func TestGetMemberCountsByGroup(t *testing.T) { require.Nil(t, err) require.ElementsMatch(t, cmc, resp) } + +func TestViewChannelCollapsedThreadsTurnedOff(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + u1 := th.BasicUser + u2 := th.BasicUser2 + c1 := th.BasicChannel + th.AddUserToChannel(u2, c1) + + // Enable CRT + os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") + defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.COLLAPSED_THREADS_DEFAULT_ON + }) + + // Turn off CRT for user + preference := model.Preference{ + UserId: u1.Id, + Category: model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, + Name: model.PREFERENCE_NAME_COLLAPSED_THREADS_ENABLED, + Value: "off", + } + var preferences model.Preferences + preferences = append(preferences, preference) + err := th.App.Srv().Store.Preference().Save(&preferences) + require.NoError(t, err) + + // mention the user in a root post + post1 := &model.Post{ + ChannelId: c1.Id, + Message: "root post @" + u1.Username, + UserId: u2.Id, + } + rpost1, appErr := th.App.CreatePost(th.Context, post1, c1, false, true) + require.Nil(t, appErr) + + // mention the user in a reply post + post2 := &model.Post{ + ChannelId: c1.Id, + Message: "reply post @" + u1.Username, + UserId: u2.Id, + RootId: rpost1.Id, + } + _, appErr = th.App.CreatePost(th.Context, post2, c1, false, true) + require.Nil(t, appErr) + + // Check we have unread mention in the thread + threads, appErr := th.App.GetThreadsForUser(u1.Id, c1.TeamId, model.GetUserThreadsOpts{}) + require.Nil(t, appErr) + found := false + for _, thread := range threads.Threads { + if thread.PostId == rpost1.Id { + require.EqualValues(t, int64(1), thread.UnreadMentions) + found = true + break + } + } + require.Truef(t, found, "did not find created thread in user's threads") + + // Mark channel as read from a client that supports CRT + _, appErr = th.App.MarkChannelsAsViewed([]string{c1.Id}, u1.Id, th.Context.Session().Id, true) + require.Nil(t, appErr) + + // Thread should be marked as read because CRT has been turned off by user + threads, appErr = th.App.GetThreadsForUser(u1.Id, c1.TeamId, model.GetUserThreadsOpts{}) + require.Nil(t, appErr) + found = false + for _, thread := range threads.Threads { + if thread.PostId == rpost1.Id { + require.Zero(t, thread.UnreadMentions) + found = true + break + } + } + require.Truef(t, found, "did not find created thread in user's threads") +} + +func TestMarkChannelAsUnreadFromPostCollapsedThreadsTurnedOff(t *testing.T) { + // Enable CRT + os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") + defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + + th := Setup(t).InitBasic() + defer th.TearDown() + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ThreadAutoFollow = true + *cfg.ServiceSettings.CollapsedThreads = model.COLLAPSED_THREADS_DEFAULT_ON + }) + + th.AddUserToChannel(th.BasicUser2, th.BasicChannel) + + // Turn off CRT for user + preference := model.Preference{ + UserId: th.BasicUser.Id, + Category: model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, + Name: model.PREFERENCE_NAME_COLLAPSED_THREADS_ENABLED, + Value: "off", + } + var preferences model.Preferences + preferences = append(preferences, preference) + err := th.App.Srv().Store.Preference().Save(&preferences) + require.NoError(t, err) + + // user2: first root mention @user1 + // - user1: hello + // - user2: mention @u1 + // - user1: another repoy + // - user2: another mention @u1 + // user1: a root post + // user2: Another root mention @u1 + user1Mention := " @" + th.BasicUser.Username + rootPost1, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "first root mention" + user1Mention}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost1.Id, UserId: th.BasicUser.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hello"}, th.BasicChannel, false, false) + require.Nil(t, appErr) + replyPost1, appErr := th.App.CreatePost(th.Context, &model.Post{RootId: rootPost1.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "mention" + user1Mention}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost1.Id, UserId: th.BasicUser.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "another reply"}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost1.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "another mention" + user1Mention}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "a root post"}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "another root mention" + user1Mention}, th.BasicChannel, false, false) + require.Nil(t, appErr) + + t.Run("Mark reply post as unread", func(t *testing.T) { + _, err := th.App.MarkChannelAsUnreadFromPost(replyPost1.Id, th.BasicUser.Id, true) + require.Nil(t, err) + // Get channel unreads + // Easier to reason with ChannelUnread now, than channelUnreadAt from the previous call + channelUnread, err := th.App.GetChannelUnread(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, err) + + require.Equal(t, int64(3), channelUnread.MentionCount) + // MentionCountRoot should be zero for a user that has CRT turned off + require.Equal(t, int64(0), channelUnread.MentionCountRoot) + + require.Equal(t, int64(5), channelUnread.MsgCount) + // MentionCountRoot should be zero for a user that has CRT turned off + require.Equal(t, channelUnread.MsgCountRoot, int64(0)) + + threadMembership, err := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost1.Id) + require.Nil(t, err) + thread, err := th.App.GetThreadForUser(th.BasicTeam.Id, threadMembership, false) + require.Nil(t, err) + require.Equal(t, int64(2), thread.UnreadMentions) + require.Equal(t, int64(3), thread.UnreadReplies) + }) + + t.Run("Mark root post as unread", func(t *testing.T) { + _, err := th.App.MarkChannelAsUnreadFromPost(rootPost1.Id, th.BasicUser.Id, true) + require.Nil(t, err) + // Get channel unreads + // Easier to reason with ChannelUnread now, than channelUnreadAt from the previous call + channelUnread, err := th.App.GetChannelUnread(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, err) + + require.Equal(t, int64(4), channelUnread.MentionCount) + require.Equal(t, int64(2), channelUnread.MentionCountRoot) + + require.Equal(t, int64(7), channelUnread.MsgCount) + require.Equal(t, int64(3), channelUnread.MsgCountRoot) + }) +}