MM-34871 CRT: Participants of thread include non-replying followers (and past followers) (#17447)

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
Eli Yukelzon
2021-04-21 09:48:30 +03:00
коммит произвёл GitHub
родитель 321645696d
Коммит 56198a99c6
11 изменённых файлов: 89 добавлений и 34 удалений

Просмотреть файл

@@ -2388,9 +2388,10 @@ func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string) (*model.
threadMembership, _ := a.Srv().Store.Thread().GetMembershipForUser(user.Id, threadId)
if threadMembership == nil {
threadMembership, _ = a.Srv().Store.Thread().MaintainMembership(user.Id, threadId, true, true, true, true)
threadMembership, _ = a.Srv().Store.Thread().MaintainMembership(user.Id, threadId, true, true, true, true, false)
}
if threadMembership != nil && threadMembership.Following {
threadData, _ := a.Srv().Store.Thread().Get(threadId)
if threadData != nil && threadMembership != nil && threadMembership.Following {
channel, nErr := a.Srv().Store.Channel().Get(post.ChannelId, true)
if nErr != nil {
return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError)

Просмотреть файл

@@ -185,7 +185,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
go func(userID string) {
defer close(mac)
_, incrementMentions := mentions.Mentions[userID]
_, err := a.Srv().Store.Thread().MaintainMembership(userID, post.RootId, true, incrementMentions, *a.Config().ServiceSettings.ThreadAutoFollow, userID == post.UserId)
_, err := a.Srv().Store.Thread().MaintainMembership(userID, post.RootId, true, incrementMentions, *a.Config().ServiceSettings.ThreadAutoFollow, userID == post.UserId, userID == post.UserId)
if err != nil {
mac <- model.NewAppError("SendNotifications", "app.channel.autofollow.app_error", nil, err.Error(), http.StatusInternalServerError)
return

Просмотреть файл

@@ -1924,6 +1924,58 @@ func TestThreadMembership(t *testing.T) {
})
}
func TestFollowThreadSkipsParticipants(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.COLLAPSED_THREADS_DEFAULT_ON
})
channel := th.BasicChannel
user := th.BasicUser
user2 := th.BasicUser2
sysadmin := th.SystemAdminUser
appErr := th.App.JoinChannel(channel, user.Id)
require.Nil(t, appErr)
appErr = th.App.JoinChannel(channel, user2.Id)
require.Nil(t, appErr)
_, appErr = th.App.JoinUserToTeam(th.BasicTeam, sysadmin, sysadmin.Id)
require.Nil(t, appErr)
appErr = th.App.JoinChannel(channel, sysadmin.Id)
require.Nil(t, appErr)
p1, err := th.App.CreatePost(&model.Post{UserId: user.Id, ChannelId: channel.Id, Message: "Hi @" + sysadmin.Username}, channel, false, false)
require.Nil(t, err)
_, err = th.App.CreatePost(&model.Post{RootId: p1.Id, UserId: user.Id, ChannelId: channel.Id, Message: "Hola"}, channel, false, false)
require.Nil(t, err)
thread, err := th.App.GetThreadForUser(user.Id, th.BasicTeam.Id, p1.Id, false)
require.Nil(t, err)
require.Len(t, thread.Participants, 1) // length should be 1, the original poster, since sysadmin was just mentioned but didn't post
_, err = th.App.CreatePost(&model.Post{RootId: p1.Id, UserId: sysadmin.Id, ChannelId: channel.Id, Message: "sysadmin reply"}, channel, false, false)
require.Nil(t, err)
thread, err = th.App.GetThreadForUser(user.Id, th.BasicTeam.Id, p1.Id, false)
require.Nil(t, err)
require.Len(t, thread.Participants, 2) // length should be 2, the original poster and sysadmin, since sysadmin participated now
// another user follows the thread
th.App.UpdateThreadFollowForUser(user2.Id, th.BasicTeam.Id, p1.Id, true)
thread, err = th.App.GetThreadForUser(user2.Id, th.BasicTeam.Id, p1.Id, false)
require.Nil(t, err)
require.Len(t, thread.Participants, 2) // length should be 2, since follow shouldn't update participant list, only user1 and sysadmin are participants
for _, p := range thread.Participants {
require.True(t, p.Id == sysadmin.Id || p.Id == user.Id)
}
}
func TestAutofollowBasedOnRootPost(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
@@ -1986,13 +2038,13 @@ func TestCollapsedThreadFetch(t *testing.T) {
require.Nil(t, err)
thread, nErr := th.App.Srv().Store.Thread().Get(postRoot.Id)
require.NoError(t, nErr)
require.Len(t, thread.Participants, 2)
require.Len(t, thread.Participants, 1)
th.App.MarkChannelAsUnreadFromPost(postRoot.Id, user1.Id)
l, err := th.App.GetPostsForChannelAroundLastUnread(channel.Id, user1.Id, 10, 10, true, true, false)
require.Nil(t, err)
require.Len(t, l.Order, 1)
require.EqualValues(t, 1, l.Posts[postRoot.Id].ReplyCount)
require.EqualValues(t, []string{user1.Id, user2.Id}, []string{l.Posts[postRoot.Id].Participants[0].Id, l.Posts[postRoot.Id].Participants[1].Id})
require.EqualValues(t, []string{user1.Id}, []string{l.Posts[postRoot.Id].Participants[0].Id})
require.Empty(t, l.Posts[postRoot.Id].Participants[0].Email)
require.NotZero(t, l.Posts[postRoot.Id].LastReplyAt)
require.True(t, l.Posts[postRoot.Id].IsFollowing)

Просмотреть файл

@@ -2428,7 +2428,7 @@ func (a *App) UpdateThreadsReadForUser(userID, teamID string) *model.AppError {
}
func (a *App) UpdateThreadFollowForUser(userID, teamID, threadID string, state bool) *model.AppError {
_, err := a.Srv().Store.Thread().MaintainMembership(userID, threadID, state, false, true, false)
_, err := a.Srv().Store.Thread().MaintainMembership(userID, threadID, state, false, true, false, false)
if err != nil {
return model.NewAppError("UpdateThreadFollowForUser", "app.user.update_thread_follow_for_user.app_error", nil, err.Error(), http.StatusInternalServerError)
}

Просмотреть файл

@@ -8812,7 +8812,7 @@ func (s *OpenTracingLayerThreadStore) GetThreadsForUser(userId string, teamID st
return result, err
}
func (s *OpenTracingLayerThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) (*model.ThreadMembership, error) {
func (s *OpenTracingLayerThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool, updateParticipants bool) (*model.ThreadMembership, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "ThreadStore.MaintainMembership")
s.Root.Store.SetContext(newCtx)
@@ -8821,7 +8821,7 @@ func (s *OpenTracingLayerThreadStore) MaintainMembership(userID string, postID s
}()
defer span.Finish()
result, err := s.ThreadStore.MaintainMembership(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
result, err := s.ThreadStore.MaintainMembership(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants)
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)

Просмотреть файл

@@ -9588,11 +9588,11 @@ func (s *RetryLayerThreadStore) GetThreadsForUser(userId string, teamID string,
}
func (s *RetryLayerThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) (*model.ThreadMembership, error) {
func (s *RetryLayerThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool, updateParticipants bool) (*model.ThreadMembership, error) {
tries := 0
for {
result, err := s.ThreadStore.MaintainMembership(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
result, err := s.ThreadStore.MaintainMembership(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants)
if err == nil {
return result, nil
}

Просмотреть файл

@@ -472,7 +472,7 @@ func (s *SqlThreadStore) DeleteMembershipForUser(userId string, postId string) e
return nil
}
func (s *SqlThreadStore) MaintainMembership(userId, postId string, following, incrementMentions, updateFollowing, updateViewedTimestamp bool) (*model.ThreadMembership, error) {
func (s *SqlThreadStore) MaintainMembership(userId, postId string, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants bool) (*model.ThreadMembership, error) {
membership, err := s.GetMembershipForUser(userId, postId)
now := utils.MillisFromTime(time.Now())
// if memebership exists, update it if:
@@ -518,13 +518,15 @@ func (s *SqlThreadStore) MaintainMembership(userId, postId string, following, in
return nil, err
}
thread, err := s.Get(postId)
if err != nil {
return nil, err
}
if !thread.Participants.Contains(userId) {
thread.Participants = append(thread.Participants, userId)
_, err = s.Update(thread)
if updateParticipants {
thread, err2 := s.Get(postId)
if err2 != nil {
return nil, err2
}
if !thread.Participants.Contains(userId) {
thread.Participants = append(thread.Participants, userId)
_, err = s.Update(thread)
}
}
return membership, err
}

Просмотреть файл

@@ -289,7 +289,7 @@ type ThreadStore interface {
GetMembershipsForUser(userId, teamID string) ([]*model.ThreadMembership, error)
GetMembershipForUser(userId, postID string) (*model.ThreadMembership, error)
DeleteMembershipForUser(userId, postID string) error
MaintainMembership(userID, postID string, following, incrementMentions, updateFollowing, updateViewedTimestamp bool) (*model.ThreadMembership, error)
MaintainMembership(userID, postID string, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants bool) (*model.ThreadMembership, error)
CollectThreadsWithNewerReplies(userId string, channelIds []string, timestamp int64) ([]string, error)
UpdateUnreadsByChannel(userId string, changedThreads []string, timestamp int64, updateViewedTimestamp bool) error
}

Просмотреть файл

@@ -203,13 +203,13 @@ func (_m *ThreadStore) GetThreadsForUser(userId string, teamID string, opts mode
return r0, r1
}
// MaintainMembership provides a mock function with given fields: userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp
func (_m *ThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) (*model.ThreadMembership, error) {
ret := _m.Called(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
// MaintainMembership provides a mock function with given fields: userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants
func (_m *ThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool, updateParticipants bool) (*model.ThreadMembership, error) {
ret := _m.Called(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants)
var r0 *model.ThreadMembership
if rf, ok := ret.Get(0).(func(string, string, bool, bool, bool, bool) *model.ThreadMembership); ok {
r0 = rf(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
if rf, ok := ret.Get(0).(func(string, string, bool, bool, bool, bool, bool) *model.ThreadMembership); ok {
r0 = rf(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*model.ThreadMembership)
@@ -217,8 +217,8 @@ func (_m *ThreadStore) MaintainMembership(userID string, postID string, followin
}
var r1 error
if rf, ok := ret.Get(1).(func(string, string, bool, bool, bool, bool) error); ok {
r1 = rf(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
if rf, ok := ret.Get(1).(func(string, string, bool, bool, bool, bool, bool) error); ok {
r1 = rf(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants)
} else {
r1 = ret.Error(1)
}

Просмотреть файл

@@ -235,7 +235,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
t.Run("Thread last updated is changed when channel is updated after UpdateLastViewedAtPost", func(t *testing.T) {
newPosts := makeSomePosts()
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false)
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false, false)
require.NoError(t, e)
m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
require.NoError(t, err1)
@@ -256,7 +256,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
t.Run("Thread last updated is changed when channel is updated after IncrementMentionCount", func(t *testing.T) {
newPosts := makeSomePosts()
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false)
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false, false)
require.NoError(t, e)
m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
require.NoError(t, err1)
@@ -276,7 +276,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
t.Run("Thread last updated is changed when channel is updated after UpdateLastViewedAt", func(t *testing.T) {
newPosts := makeSomePosts()
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false)
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false, false)
require.NoError(t, e)
m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
require.NoError(t, err1)
@@ -297,13 +297,13 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
t.Run("Thread membership 'viewed' timestamp is updated properly", func(t *testing.T) {
newPosts := makeSomePosts()
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false)
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false, false)
require.NoError(t, e)
m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
require.NoError(t, err1)
require.Equal(t, int64(0), m.LastViewed)
_, e = ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, true)
_, e = ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, true, false)
require.NoError(t, e)
m2, err2 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
require.NoError(t, err2)
@@ -312,7 +312,7 @@ func testThreadStorePopulation(t *testing.T, ss store.Store) {
t.Run("Thread last updated is changed when channel is updated after UpdateLastViewedAtPost for mark unread", func(t *testing.T) {
newPosts := makeSomePosts()
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false)
_, e := ss.Thread().MaintainMembership(newPosts[0].UserId, newPosts[0].Id, true, false, true, false, false)
require.NoError(t, e)
m, err1 := ss.Thread().GetMembershipForUser(newPosts[0].UserId, newPosts[0].Id)
require.NoError(t, err1)

Просмотреть файл

@@ -7942,10 +7942,10 @@ func (s *TimerLayerThreadStore) GetThreadsForUser(userId string, teamID string,
return result, err
}
func (s *TimerLayerThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool) (*model.ThreadMembership, error) {
func (s *TimerLayerThreadStore) MaintainMembership(userID string, postID string, following bool, incrementMentions bool, updateFollowing bool, updateViewedTimestamp bool, updateParticipants bool) (*model.ThreadMembership, error) {
start := timemodule.Now()
result, err := s.ThreadStore.MaintainMembership(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp)
result, err := s.ThreadStore.MaintainMembership(userID, postID, following, incrementMentions, updateFollowing, updateViewedTimestamp, updateParticipants)
elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second)
if s.Root.Metrics != nil {