From 305e4793c3276e66e6b4d019ef99989f552844cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 29 Sep 2021 23:16:25 +0200 Subject: [PATCH] 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 | 109 +++++++++++++++++------------ store/storetest/post_store.go | 128 ++++++++++++++++++++++++++++++++-- 2 files changed, 186 insertions(+), 51 deletions(-) diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index fa43b051b0..708d4c83ef 100644 --- a/store/sqlstore/post_store.go +++ b/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() - - 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 + return s.getFlaggedPosts(userId, "", "", offset, limit) } 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,22 +436,49 @@ func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int, WHERE Id IN - (SELECT - Name - FROM - Preferences - WHERE - UserId = :UserId - AND Category = :Category) - AND DeleteAt = 0 + ( + SELECT + Name + FROM + Preferences + WHERE + UserId = :UserId + 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 = '' + WHERE + ChannelId IN ( + SELECT + 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, "Offset": offset, "Limit": limit, "TeamId": teamId}); 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") } @@ -468,31 +490,26 @@ func (s *SqlPostStore) GetFlaggedPostsForTeam(userId, teamId string, offset int, 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) - AND ChannelId = :ChannelId - AND DeleteAt = 0 - 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 { - return nil, errors.Wrap(err, "failed to find Posts") - } - for _, post := range posts { - pl.AddPost(post) - pl.AddOrder(post.Id) +func (s *SqlPostStore) buildFlaggedPostTeamFilterClause(teamId string, queryParams map[string]interface{}) (string, map[string]interface{}) { + if teamId == "" { + return "", queryParams } - return pl, nil + 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) diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 1619bf65bc..43f9d4f4cd 100644 --- a/store/storetest/post_store.go +++ b/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) {