From 456299841a1163d95debd5fb0d749bb06208e75a Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Fri, 3 Jun 2022 21:56:36 +0530 Subject: [PATCH] MM-44412: correct usage of updateAt to createAt (#20263) * MM-44412: correct usage of updateAt to createAt * lint * correct usage of updateAt to createAt - CRT unsupported * remove dead code * remove unrequired tests * update test name Co-authored-by: Mattermod --- app/channel.go | 64 +------------- app/channel_test.go | 201 ++++++-------------------------------------- 2 files changed, 28 insertions(+), 237 deletions(-) diff --git a/app/channel.go b/app/channel.go index 94df003d2a..bc11b89c09 100644 --- a/app/channel.go +++ b/app/channel.go @@ -2585,66 +2585,6 @@ func (a *App) MarkChannelAsUnreadFromPost(postID string, userID string, collapse return nil, err } - // if auto-follow is on - // if threadmembership does not exists we create one and update - if *a.Config().ServiceSettings.ThreadAutoFollow { - threadId := post.RootId - if post.RootId == "" { - threadId = post.Id - } - - var nfErr *store.ErrNotFound - threadMembership, storeErr := a.Srv().Store.Thread().GetMembershipForUser(user.Id, threadId) - if storeErr != nil && !errors.As(storeErr, &nfErr) { - return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr.Error(), http.StatusInternalServerError) - } - // if this post was not followed before, create thread membership and update mention count - if threadMembership == nil { - opts := store.ThreadMembershipOpts{ - Following: false, - IncrementMentions: false, - UpdateFollowing: true, - UpdateViewedTimestamp: true, - UpdateParticipants: false, - } - threadMembership, storeErr = a.Srv().Store.Thread().MaintainMembership(user.Id, threadId, opts) - if storeErr != nil && !errors.As(storeErr, &nfErr) { - return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr.Error(), http.StatusInternalServerError) - } - threadData, storeErr2 := a.Srv().Store.Thread().Get(threadId) - if storeErr2 != nil { - return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, storeErr2.Error(), http.StatusInternalServerError) - } - 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) - } - threadMembership.UnreadMentions, err = a.countThreadMentions(user, post, channel.TeamId, post.UpdateAt-1) - if err != nil { - return nil, err - } - _, nErr = a.Srv().Store.Thread().UpdateMembership(threadMembership) - if nErr != nil { - return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError) - } - thread, nErr := a.Srv().Store.Thread().GetThreadForUser(channel.TeamId, threadMembership, true) - if nErr != nil { - return nil, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError) - } - a.sanitizeProfiles(thread.Participants, false) - thread.Post.SanitizeProps() - payload, jsonErr := json.Marshal(thread) - if jsonErr != nil { - mlog.Warn("Failed to encode thread to JSON") - } - message := model.NewWebSocketEvent(model.WebsocketEventThreadUpdated, channel.TeamId, "", userID, nil) - message.Add("thread", string(payload)) - a.Publish(message) - } - } - } - channelUnread, nErr := a.Srv().Store.Channel().UpdateLastViewedAtPost(post, userID, unreadMentions, unreadMentionsRoot, true) if nErr != nil { return channelUnread, model.NewAppError("MarkChannelAsUnreadFromPost", "app.channel.update_last_viewed_at_post.app_error", nil, nErr.Error(), http.StatusInternalServerError) @@ -2728,8 +2668,8 @@ func (a *App) markChannelAsUnreadFromPostCRTUnsupported(postID string, userID st } // If threadmembership already exists but user had previously unfollowed the thread, then follow the thread again. threadMembership.Following = true - threadMembership.LastViewed = post.UpdateAt - 1 - threadMembership.UnreadMentions, err = a.countThreadMentions(user, rootPost, channel.TeamId, post.UpdateAt-1) + threadMembership.LastViewed = post.CreateAt - 1 + threadMembership.UnreadMentions, err = a.countThreadMentions(user, rootPost, channel.TeamId, post.CreateAt-1) if err != nil { return nil, err } diff --git a/app/channel_test.go b/app/channel_test.go index fed99ffef0..be7c0e93d2 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -2046,78 +2046,6 @@ func TestMarkChannelsAsViewedPanic(t *testing.T) { require.Nil(t, appErr) } -func TestMarkChannelAsUnreadFromPostPanic(t *testing.T) { - th := SetupWithStoreMock(t) - defer th.TearDown() - - mockStore := th.App.Srv().Store.(*mocks.Store) - mockUserStore := mocks.UserStore{} - mockUserStore.On("Get", context.Background(), "userID").Return(&model.User{Id: "userID"}, nil) - mockUserStore.On("Count", mock.Anything).Return(int64(10), nil) - - mockChannelStore := mocks.ChannelStore{} - mockChannelStore.On("Get", "channelID", true).Return(&model.Channel{Id: "channelID"}, nil) - mockChannelStore.On("GetMember", context.Background(), "channelID", "userID").Return(&model.ChannelMember{ - NotifyProps: model.StringMap{ - model.PushNotifyProp: model.ChannelNotifyDefault, - }}, nil) - - mockSessionStore := mocks.SessionStore{} - mockOAuthStore := mocks.OAuthStore{} - var err error - th.App.ch.srv.userService, err = users.New(users.ServiceConfig{ - UserStore: &mockUserStore, - SessionStore: &mockSessionStore, - OAuthStore: &mockOAuthStore, - ConfigFn: th.App.ch.srv.Config, - LicenseFn: th.App.ch.srv.License, - }) - require.NoError(t, err) - - mockPreferenceStore := mocks.PreferenceStore{} - mockPreferenceStore.On("Get", "userID", model.PreferenceCategoryDisplaySettings, model.PreferenceNameCollapsedThreadsEnabled).Return(&model.Preference{Value: "on"}, nil) - - mockThreadStore := mocks.ThreadStore{} - mockThreadStore.On("GetMembershipForUser", "userID", "rootID").Return(nil, nil) - mockThreadStore.On("MaintainMembership", "userID", "rootID", mock.AnythingOfType("store.ThreadMembershipOpts")).Return(&model.ThreadMembership{}, nil) - mockThreadStore.On("Get", "rootID").Return(nil, errors.New("bad error")) // Returning an error from here causes the panic - - mockPostStore := mocks.PostStore{} - mockPostStore.On("GetMaxPostSize").Return(65535, nil) - mockPostStore.On("Get", context.Background(), "postID", model.GetPostsOptions{}, "userID").Return(&model.PostList{}, nil) - mockPostStore.On("GetPostsAfter", mock.AnythingOfType("model.GetPostsOptions")).Return(&model.PostList{}, nil) - mockPostStore.On("GetSingle", "postID", false).Return(&model.Post{ - Id: "postID", - RootId: "rootID", - ChannelId: "channelID", - }, nil) - - mockSystemStore := mocks.SystemStore{} - mockSystemStore.On("GetByName", "UpgradedFromTE").Return(&model.System{Name: "UpgradedFromTE", Value: "false"}, nil) - mockSystemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: "10"}, nil) - mockSystemStore.On("GetByName", "FirstServerRunTimestamp").Return(&model.System{Name: "FirstServerRunTimestamp", Value: "10"}, nil) - mockLicenseStore := mocks.LicenseStore{} - mockLicenseStore.On("Get", "").Return(&model.LicenseRecord{}, nil) - - mockStore.On("Channel").Return(&mockChannelStore) - mockStore.On("Preference").Return(&mockPreferenceStore) - mockStore.On("Thread").Return(&mockThreadStore) - mockStore.On("Post").Return(&mockPostStore) - mockStore.On("User").Return(&mockUserStore) - mockStore.On("System").Return(&mockSystemStore) - mockStore.On("License").Return(&mockLicenseStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) - - th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.ThreadAutoFollow = true - *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn - }) - - require.NotPanics(t, func() { - th.App.MarkChannelAsUnreadFromPost("postID", "userID", true) - }, "unexpected panic from MarkChannelAsUnreadFromPost") -} - func TestClearChannelMembersCache(t *testing.T) { th := SetupWithStoreMock(t) defer th.TearDown() @@ -2331,119 +2259,42 @@ func TestMarkChannelAsUnreadFromPostCollapsedThreadsTurnedOff(t *testing.T) { }) } -// TestMarkUnreadWithThreads asserts the behaviour of App.MarkChannelAsUnreadFromPost, but was -// originally written when that API accepted a followThread parameter. While tested, that parameter -// was never actually called as true, resulting in deadcode. -// -// When removing the parameter, one of the following tests failed, as it covered behaviour that -// was unused and now unsupported. The test has since been updated to reflect the new reality, -// but a careful examination of MarkChannelAsUnreadFromPost should be conducted as it's unclear -// why that API tries to manipulate thread memberships at all. Fixing that is left to another task. -func TestMarkUnreadWithThreads(t *testing.T) { +func TestMarkUnreadCRTOffUpdatesThreads(t *testing.T) { 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.CollapsedThreadsDefaultOn + *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOff }) - t.Run("Set unread mentions correctly", func(t *testing.T) { - t.Run("Never followed root post with no replies or mentions", func(t *testing.T) { - rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.MarkChannelAsUnreadFromPost(rootPost.Id, th.BasicUser.Id, true) - require.Nil(t, appErr) + t.Run("Mentions counted correctly if post is edited", func(t *testing.T) { + user3 := th.CreateUser() + defer th.App.PermanentDeleteUser(th.Context, user3) + rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "root post"}, th.BasicChannel, false, false) + require.Nil(t, appErr) + r1, appErr := th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "reply 1"}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "reply 2 @" + user3.Username}, th.BasicChannel, false, false) + require.Nil(t, appErr) + _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "reply 3"}, th.BasicChannel, false, false) + require.Nil(t, appErr) + editedPost := r1.Clone() + editedPost.Message += " edited" + _, appErr = th.App.UpdatePost(th.Context, editedPost, false) + require.Nil(t, appErr) - threadMembership, appErr := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost.Id) - require.Nil(t, appErr) - require.NotNil(t, threadMembership) - assert.Zero(t, threadMembership.UnreadMentions) - }) + th.LinkUserToTeam(user3, th.BasicTeam) + th.AddUserToChannel(user3, th.BasicChannel) - t.Run("Never followed root post with replies and no mentions", func(t *testing.T) { - rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.MarkChannelAsUnreadFromPost(rootPost.Id, th.BasicUser.Id, true) - require.Nil(t, appErr) - - threadMembership, appErr := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost.Id) - require.Nil(t, appErr) - require.NotNil(t, threadMembership) - assert.Zero(t, threadMembership.UnreadMentions) - }) - - t.Run("Never followed root post with replies and mentions", func(t *testing.T) { - rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi @" + th.BasicUser.Username}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.MarkChannelAsUnreadFromPost(rootPost.Id, th.BasicUser.Id, true) - require.Nil(t, appErr) - - threadMembership, appErr := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost.Id) - require.Nil(t, appErr) - require.NotNil(t, threadMembership) - assert.Equal(t, int64(1), threadMembership.UnreadMentions) - }) - - t.Run("Previously followed root post with no replies or mentions", func(t *testing.T) { - rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - appErr = th.App.UpdateThreadFollowForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost.Id, true) - require.Nil(t, appErr) - appErr = th.App.UpdateThreadFollowForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost.Id, false) - require.Nil(t, appErr) - - _, appErr = th.App.MarkChannelAsUnreadFromPost(rootPost.Id, th.BasicUser.Id, true) - require.Nil(t, appErr) - - threadMembership, appErr := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost.Id) - require.Nil(t, appErr) - require.NotNil(t, threadMembership) - assert.Zero(t, threadMembership.UnreadMentions) - }) - - t.Run("Previously followed root post with replies and no mentions", func(t *testing.T) { - rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - appErr = th.App.UpdateThreadFollowForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost.Id, true) - require.Nil(t, appErr) - appErr = th.App.UpdateThreadFollowForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost.Id, false) - require.Nil(t, appErr) - - _, appErr = th.App.MarkChannelAsUnreadFromPost(rootPost.Id, th.BasicUser.Id, true) - require.Nil(t, appErr) - - threadMembership, appErr := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost.Id) - require.Nil(t, appErr) - require.NotNil(t, threadMembership) - assert.Zero(t, threadMembership.UnreadMentions) - }) - - t.Run("Previously followed root post with replies and mentions", func(t *testing.T) { - rootPost, appErr := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi"}, th.BasicChannel, false, false) - require.Nil(t, appErr) - _, appErr = th.App.CreatePost(th.Context, &model.Post{RootId: rootPost.Id, UserId: th.BasicUser2.Id, CreateAt: model.GetMillis(), ChannelId: th.BasicChannel.Id, Message: "hi @" + th.BasicUser.Username}, th.BasicChannel, false, false) - require.Nil(t, appErr) - appErr = th.App.UpdateThreadFollowForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost.Id, true) - require.Nil(t, appErr) - appErr = th.App.UpdateThreadFollowForUser(th.BasicUser.Id, th.BasicTeam.Id, rootPost.Id, false) - require.Nil(t, appErr) - - _, appErr = th.App.MarkChannelAsUnreadFromPost(rootPost.Id, th.BasicUser.Id, true) - require.Nil(t, appErr) - - threadMembership, appErr := th.App.GetThreadMembershipForUser(th.BasicUser.Id, rootPost.Id) - require.Nil(t, appErr) - require.NotNil(t, threadMembership) - assert.Zero(t, threadMembership.UnreadMentions) - }) + _, appErr = th.App.MarkChannelAsUnreadFromPost(editedPost.Id, user3.Id, false) + require.Nil(t, appErr) + threadMembership, appErr := th.App.GetThreadMembershipForUser(user3.Id, rootPost.Id) + require.Nil(t, appErr) + require.NotNil(t, threadMembership) + require.True(t, threadMembership.Following) + assert.Equal(t, int64(1), threadMembership.UnreadMentions) }) }