diff --git a/app/channel_test.go b/app/channel_test.go index fc546bbcb5..328b9256f0 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -1132,12 +1132,12 @@ func TestMarkChannelAsUnreadFromPost(t *testing.T) { th.CreatePost(dc) th.CreatePost(dc) - response, err := th.App.MarkChannelAsUnreadFromPost(dm1.Id, u1.Id) + response, err := th.App.MarkChannelAsUnreadFromPost(dm1.Id, u2.Id) assert.Nil(t, err) assert.Equal(t, int64(0), response.MsgCount) assert.Equal(t, int64(3), response.MentionCount) - unread, err := th.App.GetChannelUnread(dc.Id, u1.Id) + unread, err := th.App.GetChannelUnread(dc.Id, u2.Id) require.Nil(t, err) assert.Equal(t, int64(3), unread.MsgCount) assert.Equal(t, int64(3), unread.MentionCount) diff --git a/app/post.go b/app/post.go index 58947afe4f..610ab690a8 100644 --- a/app/post.go +++ b/app/post.go @@ -1180,7 +1180,7 @@ func (a *App) MaxPostSize() int { } // countMentionsFromPost returns the number of posts in the post's channel that mention the user after and including the -// given post. Returns the number of mentions or store.MentionAllPosts if the post is in a direct message channel. +// given post. func (a *App) countMentionsFromPost(user *model.User, post *model.Post) (int, *model.AppError) { channel, err := a.GetChannel(post.ChannelId) if err != nil { @@ -1188,7 +1188,13 @@ func (a *App) countMentionsFromPost(user *model.User, post *model.Post) (int, *m } if channel.Type == model.CHANNEL_DIRECT { - return store.MentionAllPosts, nil + // In a DM channel, every post made by the other user is a mention + count, countErr := a.Srv.Store.Channel().CountPostsAfter(post.ChannelId, post.CreateAt-1, channel.GetOtherUserIdForDM(user.Id)) + if countErr != nil { + return 0, countErr + } + + return int(count), countErr } channelMember, err := a.GetChannelMember(channel.Id, user.Id) diff --git a/app/post_test.go b/app/post_test.go index b1a6da895d..5f5abe1688 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -16,7 +16,6 @@ import ( "github.com/mattermost/mattermost-server/v5/einterfaces/mocks" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock" - "github.com/mattermost/mattermost-server/v5/store" "github.com/mattermost/mattermost-server/v5/store/storetest" ) @@ -1261,7 +1260,7 @@ func TestCountMentionsFromPost(t *testing.T) { assert.Equal(t, 2, count) }) - t.Run("should return store.MentionAllPosts for a direct channel", func(t *testing.T) { + t.Run("should return the number of posts made by the other user for a direct channel", func(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -1278,10 +1277,22 @@ func TestCountMentionsFromPost(t *testing.T) { }, channel, false) require.Nil(t, err) + _, err = th.App.CreatePost(&model.Post{ + UserId: user1.Id, + ChannelId: channel.Id, + Message: "test2", + }, channel, false) + require.Nil(t, err) + count, err := th.App.countMentionsFromPost(user2, post1) assert.Nil(t, err) - assert.Equal(t, store.MentionAllPosts, count) + assert.Equal(t, 2, count) + + count, err = th.App.countMentionsFromPost(user1, post1) + + assert.Nil(t, err) + assert.Equal(t, 0, count) }) t.Run("should not count mentions from the before the given post", func(t *testing.T) { diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index 6a728f8ddf..0b8f5f50ca 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1019,7 +1019,7 @@ func (s SqlChannelStore) GetPublicChannelsForTeam(teamId string, offset int, lim PublicChannels pc ON (pc.Id = Channels.Id) WHERE pc.TeamId = :TeamId - AND pc.DeleteAt = 0 + AND pc.DeleteAt = 0 ORDER BY pc.DisplayName LIMIT :Limit OFFSET :Offset @@ -1229,15 +1229,15 @@ func (s SqlChannelStore) GetDeleted(teamId string, offset int, limit int, userId channels := &model.ChannelList{} query := ` - SELECT * FROM Channels - WHERE (TeamId = :TeamId OR TeamId = '') - AND DeleteAt != 0 + SELECT * FROM Channels + WHERE (TeamId = :TeamId OR TeamId = '') + AND DeleteAt != 0 AND Type != 'P' UNION - SELECT * FROM Channels - WHERE (TeamId = :TeamId OR TeamId = '') - AND DeleteAt != 0 - AND Type = 'P' + SELECT * FROM Channels + WHERE (TeamId = :TeamId OR TeamId = '') + AND DeleteAt != 0 + AND Type = 'P' AND Id IN (SELECT ChannelId FROM ChannelMembers WHERE UserId = :UserId) ORDER BY DisplayName LIMIT :Limit OFFSET :Offset ` @@ -1758,25 +1758,30 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string) return times, nil } -// countPostsAfter returns the number of posts in the given channel created after but not including the given timestamp. -func (s SqlChannelStore) countPostsAfter(channelID string, since int64) (int64, *model.AppError) { +// CountPostsAfter returns the number of posts in the given channel created after but not including the given timestamp. If given a non-empty user ID, only counts posts made by that user. +func (s SqlChannelStore) CountPostsAfter(channelId string, timestamp int64, userId string) (int64, *model.AppError) { countUnreadQuery := ` SELECT count(*) FROM Posts WHERE - ChannelId = :channelId - AND CreateAt > :createAt - AND Type = '' + ChannelId = :ChannelId + AND CreateAt > :CreateAt + AND Type = '' -- This line causes MM-20681 AND DeleteAt = 0 ` countParams := map[string]interface{}{ - "channelId": channelID, - "createAt": since, + "ChannelId": channelId, + "CreateAt": timestamp, + } + + if userId != "" { + countUnreadQuery += " AND UserId = :UserId" + countParams["UserId"] = userId } unread, err := s.GetReplica().SelectInt(countUnreadQuery, countParams) if err != nil { - return 0, model.NewAppError("SqlChannelStore.countPostsAfter", "store.sql_channel.count_posts_since.app_error", countParams, fmt.Sprintf("channel_id=%s, since=%d, err=%s", channelID, since, err), http.StatusInternalServerError) + return 0, model.NewAppError("SqlChannelStore.CountPostsAfter", "store.sql_channel.count_posts_since.app_error", countParams, fmt.Sprintf("channel_id=%s, timestamp=%d, err=%s", channelId, timestamp, err), http.StatusInternalServerError) } return unread, nil } @@ -1787,16 +1792,11 @@ func (s SqlChannelStore) countPostsAfter(channelID string, since int64) (int64, func (s SqlChannelStore) UpdateLastViewedAtPost(unreadPost *model.Post, userID string, mentionCount int) (*model.ChannelUnreadAt, *model.AppError) { unreadDate := unreadPost.CreateAt - 1 - unread, appErr := s.countPostsAfter(unreadPost.ChannelId, unreadDate) + unread, appErr := s.CountPostsAfter(unreadPost.ChannelId, unreadDate, "") if appErr != nil { return nil, appErr } - if mentionCount == store.MentionAllPosts { - // Treat every unread post as a mention (like in a DM channel) - mentionCount = int(unread) - } - params := map[string]interface{}{ "mentions": mentionCount, "unreadCount": unread, diff --git a/store/store.go b/store/store.go index 6adeb45582..29092d95b8 100644 --- a/store/store.go +++ b/store/store.go @@ -9,10 +9,6 @@ import ( "github.com/mattermost/mattermost-server/v5/model" ) -const ( - MentionAllPosts = -1 -) - type StoreResult struct { Data interface{} Err *model.AppError @@ -166,6 +162,7 @@ type ChannelStore interface { PermanentDeleteMembersByChannel(channelId string) *model.AppError UpdateLastViewedAt(channelIds []string, userId string) (map[string]int64, *model.AppError) UpdateLastViewedAtPost(unreadPost *model.Post, userID string, mentionCount int) (*model.ChannelUnreadAt, *model.AppError) + CountPostsAfter(channelId string, timestamp int64, userId string) (int64, *model.AppError) IncrementMentionCount(channelId string, userId string) *model.AppError AnalyticsTypeCount(teamId string, channelType string) (int64, *model.AppError) GetMembersForUser(teamId string, userId string) (*model.ChannelMembers, *model.AppError) diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 8b02a73360..308cbaaa1c 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -132,6 +132,29 @@ func (_m *ChannelStore) ClearCaches() { _m.Called() } +// CountPostsAfter provides a mock function with given fields: channelId, timestamp, userId +func (_m *ChannelStore) CountPostsAfter(channelId string, timestamp int64, userId string) (int64, *model.AppError) { + ret := _m.Called(channelId, timestamp, userId) + + var r0 int64 + if rf, ok := ret.Get(0).(func(string, int64, string) int64); ok { + r0 = rf(channelId, timestamp, userId) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, int64, string) *model.AppError); ok { + r1 = rf(channelId, timestamp, userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // CreateDirectChannel provides a mock function with given fields: userId, otherUserId func (_m *ChannelStore) CreateDirectChannel(userId *model.User, otherUserId *model.User) (*model.Channel, *model.AppError) { ret := _m.Called(userId, otherUserId) @@ -769,20 +792,6 @@ func (_m *ChannelStore) GetGuestCount(channelId string, allowFromCache bool) (in return r0, r1 } -// GetGuestCountFromCache provides a mock function with given fields: channelId -func (_m *ChannelStore) GetGuestCountFromCache(channelId string) int64 { - ret := _m.Called(channelId) - - var r0 int64 - if rf, ok := ret.Get(0).(func(string) int64); ok { - r0 = rf(channelId) - } else { - r0 = ret.Get(0).(int64) - } - - return r0 -} - // GetMember provides a mock function with given fields: channelId, userId func (_m *ChannelStore) GetMember(channelId string, userId string) (*model.ChannelMember, *model.AppError) { ret := _m.Called(channelId, userId)