diff --git a/app/notification.go b/app/notification.go index ea1b5d78de..5f9d5ec844 100644 --- a/app/notification.go +++ b/app/notification.go @@ -212,14 +212,16 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod return } } + + updateFollowing := *a.Config().ServiceSettings.ThreadAutoFollow if mentionType == ThreadMention || mentionType == CommentMention { incrementMentions = false + updateFollowing = false } - opts := store.ThreadMembershipOpts{ Following: true, IncrementMentions: incrementMentions, - UpdateFollowing: *a.Config().ServiceSettings.ThreadAutoFollow, + UpdateFollowing: updateFollowing, UpdateViewedTimestamp: userID == post.UserId, UpdateParticipants: userID == post.UserId, } diff --git a/app/post.go b/app/post.go index 563101f638..aefc15eb6a 100644 --- a/app/post.go +++ b/app/post.go @@ -362,7 +362,8 @@ func (a *App) CreatePost(c *request.Context, post *model.Post, channel *model.Ch // Make sure poster is following the thread if *a.Config().ServiceSettings.ThreadAutoFollow && rpost.RootId != "" { _, err := a.Srv().Store.Thread().MaintainMembership(user.Id, rpost.RootId, store.ThreadMembershipOpts{ - Following: true, + Following: true, + UpdateFollowing: true, }) if err != nil { mlog.Warn("Failed to update thread membership", mlog.Err(err)) diff --git a/app/post_test.go b/app/post_test.go index 0f5506a14b..c4c38ca6d8 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -2358,7 +2358,8 @@ func TestAutofollowOnPostingAfterUnfollow(t *testing.T) { // unfollow thread m, nErr := th.App.Srv().Store.Thread().MaintainMembership(user.Id, p1.Id, store.ThreadMembershipOpts{ - Following: false, + Following: false, + UpdateFollowing: true, }) require.NoError(t, nErr) require.False(t, m.Following) @@ -2398,3 +2399,54 @@ func TestGetPostIfAuthorized(t *testing.T) { _, err = th.App.GetPostIfAuthorized(post.Id, session1) require.Nil(t, err) } + +func TestShouldNotRefollowOnOthersReply(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + 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.CollapsedThreadsDefaultOn + }) + + channel := th.BasicChannel + user := th.BasicUser + user2 := th.BasicUser2 + appErr := th.App.JoinChannel(th.Context, channel, user.Id) + require.Nil(t, appErr) + appErr = th.App.JoinChannel(th.Context, channel, user2.Id) + require.Nil(t, appErr) + p1, err := th.App.CreatePost(th.Context, &model.Post{UserId: user.Id, ChannelId: channel.Id, Message: "Hi @" + user2.Username}, channel, false, false) + require.Nil(t, err) + _, err = th.App.CreatePost(th.Context, &model.Post{RootId: p1.Id, UserId: user2.Id, ChannelId: channel.Id, Message: "Hola"}, channel, false, false) + require.Nil(t, err) + + // User2 unfollows thread + m, nErr := th.App.Srv().Store.Thread().MaintainMembership(user2.Id, p1.Id, store.ThreadMembershipOpts{ + Following: false, + UpdateFollowing: true, + }) + require.NoError(t, nErr) + require.False(t, m.Following) + + // user posts in the thread + _, err = th.App.CreatePost(th.Context, &model.Post{RootId: p1.Id, UserId: user.Id, ChannelId: channel.Id, Message: "another reply"}, channel, false, false) + require.Nil(t, err) + + // User2 should still not be following the thread because they manually + // unfollowed the thread + m, err = th.App.GetThreadMembershipForUser(user2.Id, p1.Id) + require.Nil(t, err) + require.False(t, m.Following) + + // user posts in the thread mentioning user2 + _, err = th.App.CreatePost(th.Context, &model.Post{RootId: p1.Id, UserId: user.Id, ChannelId: channel.Id, Message: "reply with mention @" + user2.Username}, channel, false, false) + require.Nil(t, err) + + // User2 should now be following the thread because they were explicitly mentioned + m, err = th.App.GetThreadMembershipForUser(user2.Id, p1.Id) + require.Nil(t, err) + require.True(t, m.Following) +} diff --git a/app/user.go b/app/user.go index 698ff4151e..edfe0932b1 100644 --- a/app/user.go +++ b/app/user.go @@ -2271,7 +2271,7 @@ func (a *App) UpdateThreadReadForUser(userID, teamID, threadID string, timestamp opts := store.ThreadMembershipOpts{ Following: true, - UpdateFollowing: false, + UpdateFollowing: true, } membership, storeErr := a.Srv().Store.Thread().MaintainMembership(userID, threadID, opts) if storeErr != nil { @@ -2286,7 +2286,6 @@ func (a *App) UpdateThreadReadForUser(userID, teamID, threadID string, timestamp if err != nil { return nil, err } - membership.Following = true _, nErr := a.Srv().Store.Thread().UpdateMembership(membership) if nErr != nil { return nil, model.NewAppError("UpdateThreadReadForUser", "app.user.update_thread_read_for_user.app_error", nil, nErr.Error(), http.StatusInternalServerError) diff --git a/store/sqlstore/thread_store.go b/store/sqlstore/thread_store.go index e562276068..5e47bcc9e3 100644 --- a/store/sqlstore/thread_store.go +++ b/store/sqlstore/thread_store.go @@ -611,7 +611,7 @@ func (s *SqlThreadStore) MaintainMembership(userId, postId string, opts store.Th // b. mention count changed // c. user viewed a thread if err == nil { - followingNeedsUpdate := (opts.UpdateFollowing && !membership.Following || membership.Following != opts.Following) + followingNeedsUpdate := (opts.UpdateFollowing && (membership.Following != opts.Following)) if followingNeedsUpdate || opts.IncrementMentions || opts.UpdateViewedTimestamp { if followingNeedsUpdate { membership.Following = opts.Following diff --git a/store/store.go b/store/store.go index 03c9a6e8c9..d0fedc5fdb 100644 --- a/store/store.go +++ b/store/store.go @@ -934,7 +934,7 @@ type ThreadMembershipOpts struct { // IncrementMentions indicates whether or not the mentions count for // the thread should be incremented. IncrementMentions bool - // UpdateFollowing indicates whether or not a membership update should be forced. + // UpdateFollowing indicates whether or not the following state should be changed. UpdateFollowing bool // UpdateViewedTimestamp indicates whether or not the LastViewed field of the // membership should be updated.