MM-20681 Fix custom post types not marking channel unread when using Mark as Unread (#13247)

* MM-20681 Fix custom post types not marking channel unread when using Mark as Unread

* Fix inverted if statements

app/post.go

@@ -1194,7 +1194,7 @@ func (a *App) countMentionsFromPost(user *model.User, post *model.Post) (int, *m
 			return 0, countErr
 		}
 
-		return int(count), countErr
+		return count, countErr
 	}
 
 	channelMember, err := a.GetChannelMember(channel.Id, user.Id)

model/post.go

@@ -350,6 +350,19 @@ func (o *Post) IsSystemMessage() bool {
 	return len(o.Type) >= len(POST_SYSTEM_MESSAGE_PREFIX) && o.Type[:len(POST_SYSTEM_MESSAGE_PREFIX)] == POST_SYSTEM_MESSAGE_PREFIX
 }
 
+func (o *Post) IsJoinLeaveMessage() bool {
+	return o.Type == POST_JOIN_LEAVE ||
+		o.Type == POST_ADD_REMOVE ||
+		o.Type == POST_JOIN_CHANNEL ||
+		o.Type == POST_LEAVE_CHANNEL ||
+		o.Type == POST_JOIN_TEAM ||
+		o.Type == POST_LEAVE_TEAM ||
+		o.Type == POST_ADD_TO_CHANNEL ||
+		o.Type == POST_REMOVE_FROM_CHANNEL ||
+		o.Type == POST_ADD_TO_TEAM ||
+		o.Type == POST_REMOVE_FROM_TEAM
+}
+
 func (p *Post) Patch(patch *PostPatch) {
 	if patch.IsPinned != nil {
 		p.IsPinned = *patch.IsPinned

store/sqlstore/channel_store.go

@@ -1759,31 +1759,44 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string)
 }
 
 // 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 := `
+func (s SqlChannelStore) CountPostsAfter(channelId string, timestamp int64, userId string) (int, *model.AppError) {
+	joinLeavePostTypes, params := MapStringsToQueryParams([]string{
+		// These types correspond to the ones checked by Post.IsJoinLeaveMessage
+		model.POST_JOIN_LEAVE,
+		model.POST_ADD_REMOVE,
+		model.POST_JOIN_CHANNEL,
+		model.POST_LEAVE_CHANNEL,
+		model.POST_JOIN_TEAM,
+		model.POST_LEAVE_TEAM,
+		model.POST_ADD_TO_CHANNEL,
+		model.POST_REMOVE_FROM_CHANNEL,
+		model.POST_ADD_TO_TEAM,
+		model.POST_REMOVE_FROM_TEAM,
+	}, "PostType")
+
+	query := `
 	SELECT count(*)
 	FROM Posts
 	WHERE
 		ChannelId = :ChannelId
 		AND CreateAt > :CreateAt
-		AND Type = '' -- This line causes MM-20681
+		AND Type NOT IN ` + joinLeavePostTypes + `
 		AND DeleteAt = 0
 	`
-	countParams := map[string]interface{}{
-		"ChannelId": channelId,
-		"CreateAt":  timestamp,
-	}
+
+	params["ChannelId"] = channelId
+	params["CreateAt"] = timestamp
 
 	if userId != "" {
-		countUnreadQuery += " AND UserId = :UserId"
-		countParams["UserId"] = userId
+		query += " AND UserId = :UserId"
+		params["UserId"] = userId
 	}
 
-	unread, err := s.GetReplica().SelectInt(countUnreadQuery, countParams)
+	unread, err := s.GetReplica().SelectInt(query, params)
 	if err != nil {
-		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 0, model.NewAppError("SqlChannelStore.CountPostsAfter", "store.sql_channel.count_posts_since.app_error", nil, fmt.Sprintf("channel_id=%s, timestamp=%d, err=%s", channelId, timestamp, err), http.StatusInternalServerError)
 	}
-	return unread, nil
+	return int(unread), nil
 }
 
 // UpdateLastViewedAtPost updates a ChannelMember as if the user last read the channel at the time of the given post.

store/sqlstore/post_store.go

@@ -102,11 +102,7 @@ func (s *SqlPostStore) Save(post *model.Post) (*model.Post, *model.AppError) {
 
 	time := post.UpdateAt
 
-	if post.Type != model.POST_JOIN_LEAVE && post.Type != model.POST_ADD_REMOVE &&
-		post.Type != model.POST_JOIN_CHANNEL && post.Type != model.POST_LEAVE_CHANNEL &&
-		post.Type != model.POST_JOIN_TEAM && post.Type != model.POST_LEAVE_TEAM &&
-		post.Type != model.POST_ADD_TO_CHANNEL && post.Type != model.POST_REMOVE_FROM_CHANNEL &&
-		post.Type != model.POST_ADD_TO_TEAM && post.Type != model.POST_REMOVE_FROM_TEAM {
+	if !post.IsJoinLeaveMessage() {
 		if _, err := s.GetMaster().Exec("UPDATE Channels SET LastPostAt = GREATEST(:LastPostAt, LastPostAt), TotalMsgCount = TotalMsgCount + 1 WHERE Id = :ChannelId", map[string]interface{}{"LastPostAt": time, "ChannelId": post.ChannelId}); err != nil {
 			mlog.Error("Error updating Channel LastPostAt.", mlog.Err(err))
 		}
@@ -203,7 +199,7 @@ func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int,
 
 	query := `
             SELECT
-                A.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = A.Id AND Posts.DeleteAt = 0) as ReplyCount 
+                A.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = A.Id AND Posts.DeleteAt = 0) as ReplyCount
             FROM
                 (SELECT
                     *
@@ -245,7 +241,7 @@ func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offse
 	var posts []*model.Post
 	query := `
 		SELECT
-			*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount 
+			*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id AND Posts.DeleteAt = 0) as ReplyCount
 		FROM Posts p
 		WHERE
 			Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category)

store/store.go

@@ -162,7 +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)
+	CountPostsAfter(channelId string, timestamp int64, userId string) (int, *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)

store/storetest/channel_store.go

@@ -57,6 +57,7 @@ func TestChannelStore(t *testing.T, ss store.Store, s SqlSupplier) {
 	t.Run("GetChannelCounts", func(t *testing.T) { testChannelStoreGetChannelCounts(t, ss) })
 	t.Run("GetMembersForUser", func(t *testing.T) { testChannelStoreGetMembersForUser(t, ss) })
 	t.Run("GetMembersForUserWithPagination", func(t *testing.T) { testChannelStoreGetMembersForUserWithPagination(t, ss) })
+	t.Run("CountPostsAfter", func(t *testing.T) { testCountPostsAfter(t, ss) })
 	t.Run("UpdateLastViewedAt", func(t *testing.T) { testChannelStoreUpdateLastViewedAt(t, ss) })
 	t.Run("IncrementMentionCount", func(t *testing.T) { testChannelStoreIncrementMentionCount(t, ss) })
 	t.Run("UpdateChannelMember", func(t *testing.T) { testUpdateChannelMember(t, ss) })
@@ -1594,6 +1595,150 @@ func testChannelStoreGetMembersForUserWithPagination(t *testing.T, ss store.Stor
 	assert.Len(t, *members, 1)
 }
 
+func testCountPostsAfter(t *testing.T, ss store.Store) {
+	t.Run("should count all posts with or without the given user ID", func(t *testing.T) {
+		userId1 := model.NewId()
+		userId2 := model.NewId()
+
+		channelId := model.NewId()
+
+		p1, err := ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1000,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1001,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId2,
+			ChannelId: channelId,
+			CreateAt:  1002,
+		})
+		require.Nil(t, err)
+
+		count, err := ss.Channel().CountPostsAfter(channelId, p1.CreateAt-1, "")
+		require.Nil(t, err)
+		assert.Equal(t, 3, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p1.CreateAt, "")
+		require.Nil(t, err)
+		assert.Equal(t, 2, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p1.CreateAt-1, userId1)
+		require.Nil(t, err)
+		assert.Equal(t, 2, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p1.CreateAt, userId1)
+		require.Nil(t, err)
+		assert.Equal(t, 1, count)
+	})
+
+	t.Run("should not count deleted posts", func(t *testing.T) {
+		userId1 := model.NewId()
+
+		channelId := model.NewId()
+
+		p1, err := ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1000,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1001,
+			DeleteAt:  1001,
+		})
+		require.Nil(t, err)
+
+		count, err := ss.Channel().CountPostsAfter(channelId, p1.CreateAt-1, "")
+		require.Nil(t, err)
+		assert.Equal(t, 1, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p1.CreateAt, "")
+		require.Nil(t, err)
+		assert.Equal(t, 0, count)
+	})
+
+	t.Run("should count system/bot messages, but not join/leave messages", func(t *testing.T) {
+		userId1 := model.NewId()
+
+		channelId := model.NewId()
+
+		p1, err := ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1000,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1001,
+			Type:      model.POST_JOIN_CHANNEL,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1002,
+			Type:      model.POST_REMOVE_FROM_CHANNEL,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1003,
+			Type:      model.POST_LEAVE_TEAM,
+		})
+		require.Nil(t, err)
+
+		p5, err := ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1004,
+			Type:      model.POST_HEADER_CHANGE,
+		})
+		require.Nil(t, err)
+
+		_, err = ss.Post().Save(&model.Post{
+			UserId:    userId1,
+			ChannelId: channelId,
+			CreateAt:  1005,
+			Type:      "custom_nps_survey",
+		})
+		require.Nil(t, err)
+
+		count, err := ss.Channel().CountPostsAfter(channelId, p1.CreateAt-1, "")
+		require.Nil(t, err)
+		assert.Equal(t, 3, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p1.CreateAt, "")
+		require.Nil(t, err)
+		assert.Equal(t, 2, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p5.CreateAt-1, "")
+		require.Nil(t, err)
+		assert.Equal(t, 2, count)
+
+		count, err = ss.Channel().CountPostsAfter(channelId, p5.CreateAt, "")
+		require.Nil(t, err)
+		assert.Equal(t, 1, count)
+	})
+}
+
 func testChannelStoreUpdateLastViewedAt(t *testing.T, ss store.Store) {
 	o1 := model.Channel{}
 	o1.TeamId = model.NewId()

store/storetest/mocks/ChannelStore.go

@@ -133,14 +133,14 @@ func (_m *ChannelStore) ClearCaches() {
 }
 
 // CountPostsAfter provides a mock function with given fields: channelId, timestamp, userId
-func (_m *ChannelStore) CountPostsAfter(channelId string, timestamp int64, userId string) (int64, *model.AppError) {
+func (_m *ChannelStore) CountPostsAfter(channelId string, timestamp int64, userId string) (int, *model.AppError) {
 	ret := _m.Called(channelId, timestamp, userId)
 
-	var r0 int64
-	if rf, ok := ret.Get(0).(func(string, int64, string) int64); ok {
+	var r0 int
+	if rf, ok := ret.Get(0).(func(string, int64, string) int); ok {
 		r0 = rf(channelId, timestamp, userId)
 	} else {
-		r0 = ret.Get(0).(int64)
+		r0 = ret.Get(0).(int)
 	}
 
 	var r1 *model.AppError