Only consider flagged posts that are on channels you are member of (#18380)

* Only consider flagged posts that are on channels you are member of

* Fix and add tests

store/sqlstore/post_store.go

@@ -410,26 +410,21 @@ func (s *SqlPostStore) Overwrite(post *model.Post) (*model.Post, error) {
 }
 
 func (s *SqlPostStore) GetFlaggedPosts(userId string, offset int, limit int) (*model.PostList, error) {
-	pl := model.NewPostList()
+	return s.getFlaggedPosts(userId, "", "", offset, limit)
-
-	var posts []*model.Post
-	if _, err := s.GetReplica().Select(&posts, "SELECT *, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category) AND DeleteAt = 0 ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset", map[string]interface{}{"UserId": userId, "Category": model.PreferenceCategoryFlaggedPost, "Offset": offset, "Limit": limit}); err != nil {
-		return nil, errors.Wrap(err, "failed to find Posts")
-	}
-
-	for _, post := range posts {
-		pl.AddPost(post)
-		pl.AddOrder(post.Id)
-	}
-
-	return pl, nil
 }
 
 func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int, limit int) (*model.PostList, error) {
+	return s.getFlaggedPosts(userId, "", teamId, offset, limit)
+}
+
+func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offset int, limit int) (*model.PostList, error) {
+	return s.getFlaggedPosts(userId, channelId, "", offset, limit)
+}
+
+func (s *SqlPostStore) getFlaggedPosts(userId, channelId, teamId string, offset int, limit int) (*model.PostList, error) {
 	pl := model.NewPostList()
 
 	var posts []*model.Post
-
 	query := `
             SELECT
                 A.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = (CASE WHEN A.RootId = '' THEN A.Id ELSE A.RootId END) AND Posts.DeleteAt = 0) as ReplyCount
@@ -441,51 +436,52 @@ func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int,
                 WHERE
                     Id
                 IN
-                    (SELECT
+                    (
+						SELECT
 							Name
 						FROM
 							Preferences
 						WHERE
 							UserId = :UserId
-                        AND Category = :Category)
+							AND Category = :Category
+					)
+					CHANNEL_FILTER
 					AND DeleteAt = 0
                 ) as A
             INNER JOIN Channels as B
                 ON B.Id = A.ChannelId
-            WHERE B.TeamId = :TeamId OR B.TeamId = ''
-            ORDER BY CreateAt DESC
-            LIMIT :Limit OFFSET :Offset`
-
-	if _, err := s.GetReplica().Select(&posts, query, map[string]interface{}{"UserId": userId, "Category": model.PreferenceCategoryFlaggedPost, "Offset": offset, "Limit": limit, "TeamId": teamId}); err != nil {
-		return nil, errors.Wrap(err, "failed to find Posts")
-	}
-
-	for _, post := range posts {
-		pl.AddPost(post)
-		pl.AddOrder(post.Id)
-	}
-
-	return pl, nil
-}
-
-func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offset int, limit int) (*model.PostList, error) {
-	pl := model.NewPostList()
-
-	var posts []*model.Post
-	query := `
-		SELECT
-			*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount
-		FROM Posts p
 			WHERE
-			Id IN (SELECT Name FROM Preferences WHERE UserId = :UserId AND Category = :Category)
+				ChannelId IN (
-			AND ChannelId = :ChannelId
+					SELECT
-			AND DeleteAt = 0
+						Id
+					FROM
+						Channels,
+						ChannelMembers
+					WHERE
+						Id = ChannelId
+						AND UserId = :UserId
+				)
+				TEAM_FILTER
             ORDER BY CreateAt DESC
             LIMIT :Limit OFFSET :Offset`
 
-	if _, err := s.GetReplica().Select(&posts, query, map[string]interface{}{"UserId": userId, "Category": model.PreferenceCategoryFlaggedPost, "ChannelId": channelId, "Offset": offset, "Limit": limit}); err != nil {
+	queryParams := map[string]interface{}{
+		"UserId":   userId,
+		"Category": model.PreferenceCategoryFlaggedPost,
+		"Offset":   offset,
+		"Limit":    limit,
+	}
+
+	teamClause, queryParams := s.buildFlaggedPostTeamFilterClause(teamId, queryParams)
+	query = strings.Replace(query, "TEAM_FILTER", teamClause, 1)
+
+	channelClause, queryParams := s.buildFlaggedPostChannelFilterClause(channelId, queryParams)
+	query = strings.Replace(query, "CHANNEL_FILTER", channelClause, 1)
+
+	if _, err := s.GetReplica().Select(&posts, query, queryParams); err != nil {
 		return nil, errors.Wrap(err, "failed to find Posts")
 	}
+
 	for _, post := range posts {
 		pl.AddPost(post)
 		pl.AddOrder(post.Id)
@@ -493,6 +489,27 @@ func (s *SqlPostStore) GetFlaggedPostsForChannel(userId, channelId string, offse
 
 	return pl, nil
 }
+
+func (s *SqlPostStore) buildFlaggedPostTeamFilterClause(teamId string, queryParams map[string]interface{}) (string, map[string]interface{}) {
+	if teamId == "" {
+		return "", queryParams
+	}
+
+	queryParams["TeamId"] = teamId
+
+	return "AND B.TeamId = :TeamId OR B.TeamId = ''", queryParams
+}
+
+func (s *SqlPostStore) buildFlaggedPostChannelFilterClause(channelId string, queryParams map[string]interface{}) (string, map[string]interface{}) {
+	if channelId == "" {
+		return "", queryParams
+	}
+
+	queryParams["ChannelId"] = channelId
+
+	return "AND ChannelId = :ChannelId", queryParams
+}
+
 func (s *SqlPostStore) getPostWithCollapsedThreads(id, userID string, extended bool) (*model.PostList, error) {
 	if id == "" {
 		return nil, store.NewErrInvalidInput("Post", "id", id)

store/storetest/post_store.go

@@ -1961,6 +1961,13 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlStor
 	require.NoError(t, err)
 	time.Sleep(2 * time.Millisecond)
 
+	m0 := &model.ChannelMember{}
+	m0.ChannelId = c1.Id
+	m0.UserId = o1.UserId
+	m0.NotifyProps = model.GetDefaultChannelNotifyProps()
+	_, err = ss.Channel().SaveMember(m0)
+	require.NoError(t, err)
+
 	o4 := &model.Post{}
 	o4.ChannelId = model.NewId()
 	o4.UserId = model.NewId()
@@ -1995,6 +2002,15 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlStor
 	require.NoError(t, err)
 	time.Sleep(2 * time.Millisecond)
 
+	// Post on channel where user is not a member
+	o6 := &model.Post{}
+	o6.ChannelId = model.NewId()
+	o6.UserId = m2.UserId
+	o6.Message = NewTestId()
+	o6, err = ss.Post().Save(o6)
+	require.NoError(t, err)
+	time.Sleep(2 * time.Millisecond)
+
 	r1, err := ss.Post().GetFlaggedPosts(o1.ChannelId, 0, 2)
 	require.NoError(t, err)
 
@@ -2094,16 +2110,39 @@ func testPostStoreGetFlaggedPostsForTeam(t *testing.T, ss store.Store, s SqlStor
 	require.NoError(t, err)
 	require.Len(t, r4.Order, 3, "should have 3 posts")
 
+	preferences = model.Preferences{
+		{
+			UserId:   o1.UserId,
+			Category: model.PreferenceCategoryFlaggedPost,
+			Name:     o6.Id,
+			Value:    "true",
+		},
+	}
+	err = ss.Preference().Save(preferences)
+	require.NoError(t, err)
+
+	r4, err = ss.Post().GetFlaggedPostsForTeam(o1.UserId, c1.TeamId, 0, 10)
+	require.NoError(t, err)
+	require.Len(t, r4.Order, 3, "should have 3 posts")
+
 	// Manually truncate Channels table until testlib can handle cleanups
 	s.GetMaster().Exec("TRUNCATE Channels")
 }
 
 func testPostStoreGetFlaggedPosts(t *testing.T, ss store.Store) {
+	c1 := &model.Channel{}
+	c1.TeamId = model.NewId()
+	c1.DisplayName = "Channel1"
+	c1.Name = NewTestId()
+	c1.Type = model.ChannelTypeOpen
+	c1, err := ss.Channel().Save(c1, -1)
+	require.NoError(t, err)
+
 	o1 := &model.Post{}
-	o1.ChannelId = model.NewId()
+	o1.ChannelId = c1.Id
 	o1.UserId = model.NewId()
 	o1.Message = NewTestId()
-	o1, err := ss.Post().Save(o1)
+	o1, err = ss.Post().Save(o1)
 	require.NoError(t, err)
 	time.Sleep(2 * time.Millisecond)
 
@@ -2124,6 +2163,22 @@ func testPostStoreGetFlaggedPosts(t *testing.T, ss store.Store) {
 	require.NoError(t, err)
 	time.Sleep(2 * time.Millisecond)
 
+	// Post on channel where user is not a member
+	o4 := &model.Post{}
+	o4.ChannelId = model.NewId()
+	o4.UserId = model.NewId()
+	o4.Message = NewTestId()
+	o4, err = ss.Post().Save(o4)
+	require.NoError(t, err)
+	time.Sleep(2 * time.Millisecond)
+
+	m0 := &model.ChannelMember{}
+	m0.ChannelId = o1.ChannelId
+	m0.UserId = o1.UserId
+	m0.NotifyProps = model.GetDefaultChannelNotifyProps()
+	_, err = ss.Channel().SaveMember(m0)
+	require.NoError(t, err)
+
 	r1, err := ss.Post().GetFlaggedPosts(o1.UserId, 0, 2)
 	require.NoError(t, err)
 	require.Empty(t, r1.Order, "should be empty")
@@ -2187,14 +2242,46 @@ func testPostStoreGetFlaggedPosts(t *testing.T, ss store.Store) {
 	r4, err = ss.Post().GetFlaggedPosts(o1.UserId, 0, 2)
 	require.NoError(t, err)
 	require.Len(t, r4.Order, 2, "should have 2 posts")
+
+	preferences = model.Preferences{
+		{
+			UserId:   o1.UserId,
+			Category: model.PreferenceCategoryFlaggedPost,
+			Name:     o4.Id,
+			Value:    "true",
+		},
+	}
+
+	nErr = ss.Preference().Save(preferences)
+	require.NoError(t, nErr)
+
+	r4, err = ss.Post().GetFlaggedPosts(o1.UserId, 0, 2)
+	require.NoError(t, err)
+	require.Len(t, r4.Order, 2, "should have 2 posts")
 }
 
 func testPostStoreGetFlaggedPostsForChannel(t *testing.T, ss store.Store) {
+	c1 := &model.Channel{}
+	c1.TeamId = model.NewId()
+	c1.DisplayName = "Channel1"
+	c1.Name = NewTestId()
+	c1.Type = model.ChannelTypeOpen
+	c1, err := ss.Channel().Save(c1, -1)
+	require.NoError(t, err)
+
+	c2 := &model.Channel{}
+	c2.TeamId = model.NewId()
+	c2.DisplayName = "Channel2"
+	c2.Name = NewTestId()
+	c2.Type = model.ChannelTypeOpen
+	c2, err = ss.Channel().Save(c2, -1)
+	require.NoError(t, err)
+
 	o1 := &model.Post{}
-	o1.ChannelId = model.NewId()
+	o1.ChannelId = c1.Id
 	o1.UserId = model.NewId()
 	o1.Message = NewTestId()
-	o1, err := ss.Post().Save(o1)
+	o1, err = ss.Post().Save(o1)
 	require.NoError(t, err)
 	time.Sleep(2 * time.Millisecond)
 
@@ -2217,13 +2304,36 @@ func testPostStoreGetFlaggedPostsForChannel(t *testing.T, ss store.Store) {
 	time.Sleep(2 * time.Millisecond)
 
 	o4 := &model.Post{}
-	o4.ChannelId = model.NewId()
+	o4.ChannelId = c2.Id
 	o4.UserId = model.NewId()
 	o4.Message = NewTestId()
 	o4, err = ss.Post().Save(o4)
 	require.NoError(t, err)
 	time.Sleep(2 * time.Millisecond)
 
+	// Post on channel where user is not a member
+	o5 := &model.Post{}
+	o5.ChannelId = model.NewId()
+	o5.UserId = model.NewId()
+	o5.Message = NewTestId()
+	o5, err = ss.Post().Save(o5)
+	require.NoError(t, err)
+	time.Sleep(2 * time.Millisecond)
+
+	m1 := &model.ChannelMember{}
+	m1.ChannelId = o1.ChannelId
+	m1.UserId = o1.UserId
+	m1.NotifyProps = model.GetDefaultChannelNotifyProps()
+	_, err = ss.Channel().SaveMember(m1)
+	require.NoError(t, err)
+
+	m2 := &model.ChannelMember{}
+	m2.ChannelId = o4.ChannelId
+	m2.UserId = o1.UserId
+	m2.NotifyProps = model.GetDefaultChannelNotifyProps()
+	_, err = ss.Channel().SaveMember(m2)
+	require.NoError(t, err)
+
 	r, err := ss.Post().GetFlaggedPostsForChannel(o1.UserId, o1.ChannelId, 0, 10)
 	require.NoError(t, err)
 	require.Empty(t, r.Order, "should be empty")
@@ -2273,6 +2383,14 @@ func testPostStoreGetFlaggedPostsForChannel(t *testing.T, ss store.Store) {
 	r, err = ss.Post().GetFlaggedPostsForChannel(o1.UserId, o4.ChannelId, 0, 10)
 	require.NoError(t, err)
 	require.Len(t, r.Order, 1, "should have 1 posts")
+
+	preference.Name = o5.Id
+	nErr = ss.Preference().Save(model.Preferences{preference})
+	require.NoError(t, nErr)
+
+	r, err = ss.Post().GetFlaggedPostsForChannel(o1.UserId, o5.ChannelId, 0, 10)
+	require.NoError(t, err)
+	require.Len(t, r.Order, 0, "should have 0 posts")
 }
 
 func testPostStoreGetPostsCreatedAt(t *testing.T, ss store.Store) {