From 701d1dbd68f702ee72591cddcdd326602012f996 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Thu, 14 Nov 2024 16:57:15 +0800 Subject: [PATCH] optionally exclude threads in direct messages (#29042) * optionally exclude threads in direct messages * add excludeDirect option in client4 * Skip flaky test * refactor double negate --------- Co-authored-by: Mattermost Build --- server/channels/api4/channel_bookmark_test.go | 1 + server/channels/api4/user.go | 2 + .../channels/store/sqlstore/thread_store.go | 62 ++++++++++++------- .../channels/store/storetest/thread_store.go | 23 ++++++- server/public/model/client4.go | 3 + server/public/model/thread.go | 2 + 6 files changed, 69 insertions(+), 24 deletions(-) diff --git a/server/channels/api4/channel_bookmark_test.go b/server/channels/api4/channel_bookmark_test.go index 22fea29029b..de8f502e274 100644 --- a/server/channels/api4/channel_bookmark_test.go +++ b/server/channels/api4/channel_bookmark_test.go @@ -972,6 +972,7 @@ func TestUpdateChannelBookmarkSortOrder(t *testing.T) { }) t.Run("a websockets event should be fired as part of editing a bookmark's sort order", func(t *testing.T) { + t.Skip("MM-61301") now := model.GetMillis() webSocketClient, err := th.CreateWebSocketClient() require.NoError(t, err) diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go index 16105bb7372..ad939288743 100644 --- a/server/channels/api4/user.go +++ b/server/channels/api4/user.go @@ -3290,8 +3290,10 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) { options.After = r.URL.Query().Get("after") totalsOnlyStr := r.URL.Query().Get("totalsOnly") threadsOnlyStr := r.URL.Query().Get("threadsOnly") + excludeDirectStr := r.URL.Query().Get("excludeDirect") options.TotalsOnly, _ = strconv.ParseBool(totalsOnlyStr) options.ThreadsOnly, _ = strconv.ParseBool(threadsOnlyStr) + options.ExcludeDirect, _ = strconv.ParseBool(excludeDirectStr) // parameters are mutually exclusive if options.Before != "" && options.After != "" { diff --git a/server/channels/store/sqlstore/thread_store.go b/server/channels/store/sqlstore/thread_store.go index f579db55e28..2ada28a8156 100644 --- a/server/channels/store/sqlstore/thread_store.go +++ b/server/channels/store/sqlstore/thread_store.go @@ -134,11 +134,16 @@ func (s *SqlThreadStore) getTotalThreadsQuery(userId, teamId string, opts model. }) if teamId != "" { - query = query. - Where(sq.Or{ - sq.Eq{"Threads.ThreadTeamId": teamId}, - sq.Eq{"Threads.ThreadTeamId": ""}, - }) + if opts.ExcludeDirect { + query = query.Where(sq.Eq{"Threads.ThreadTeamId": teamId}) + } else { + query = query.Where( + sq.Or{ + sq.Eq{"Threads.ThreadTeamId": teamId}, + sq.Eq{"Threads.ThreadTeamId": ""}, + }, + ) + } } if !opts.Deleted { @@ -196,11 +201,16 @@ func (s *SqlThreadStore) GetTotalUnreadMentions(userId, teamId string, opts mode }) if teamId != "" { - query = query. - Where(sq.Or{ - sq.Eq{"Threads.ThreadTeamId": teamId}, - sq.Eq{"Threads.ThreadTeamId": ""}, - }) + if opts.ExcludeDirect { + query = query.Where(sq.Eq{"Threads.ThreadTeamId": teamId}) + } else { + query = query.Where( + sq.Or{ + sq.Eq{"Threads.ThreadTeamId": teamId}, + sq.Eq{"Threads.ThreadTeamId": ""}, + }, + ) + } } if !opts.Deleted { @@ -235,11 +245,16 @@ func (s *SqlThreadStore) GetTotalUnreadUrgentMentions(userId, teamId string, opt } if teamId != "" { - query = query. - Where(sq.Or{ - sq.Eq{"Threads.ThreadTeamId": teamId}, - sq.Eq{"Threads.ThreadTeamId": ""}, - }) + if opts.ExcludeDirect { + query = query.Where(sq.Eq{"Threads.ThreadTeamId": teamId}) + } else { + query = query.Where( + sq.Or{ + sq.Eq{"Threads.ThreadTeamId": teamId}, + sq.Eq{"Threads.ThreadTeamId": ""}, + }, + ) + } } if !opts.Deleted { @@ -296,14 +311,19 @@ func (s *SqlThreadStore) GetThreadsForUser(userId, teamId string, opts model.Get LeftJoin("PostsPriority ON PostsPriority.PostId = Threads.PostId") } - // If a team is specified, constrain to channels in that team or DMs/GMs without + // If a team is specified, constrain to channels in that team and if not excluded also return DMs/GMs without // a team at all. if teamId != "" { - query = query. - Where(sq.Or{ - sq.Eq{"Threads.ThreadTeamId": teamId}, - sq.Eq{"Threads.ThreadTeamId": ""}, - }) + if opts.ExcludeDirect { + query = query.Where(sq.Eq{"Threads.ThreadTeamId": teamId}) + } else { + query = query.Where( + sq.Or{ + sq.Eq{"Threads.ThreadTeamId": teamId}, + sq.Eq{"Threads.ThreadTeamId": ""}, + }, + ) + } } if !opts.Deleted { diff --git a/server/channels/store/storetest/thread_store.go b/server/channels/store/storetest/thread_store.go index 4b65f70c197..84738d1cc4f 100644 --- a/server/channels/store/storetest/thread_store.go +++ b/server/channels/store/storetest/thread_store.go @@ -916,6 +916,13 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { ChannelId: gm1.Id, UserId: user1ID, Message: model.NewRandomString(10), + Metadata: &model.PostMetadata{ + Priority: &model.PostPriority{ + Priority: model.NewPointer(model.PostPriorityUrgent), + RequestedAck: model.NewPointer(false), + PersistentNotifications: model.NewPointer(false), + }, + }, }) require.NoError(t, err) @@ -1077,6 +1084,9 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { {"team2, user1", user1ID, team2.Id, model.GetUserThreadsOpts{}, []*model.Post{ gm1post1, }}, + {"team1, user1, exclude direct", user1ID, team1.Id, model.GetUserThreadsOpts{ExcludeDirect: true}, []*model.Post{ + team1channel1post3, + }}, } for _, testCase := range testCases { @@ -1098,12 +1108,13 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { ExpectedThreads []*model.Post }{ {"all teams, user1", user1ID, "", model.GetUserThreadsOpts{}, []*model.Post{ - team1channel1post3, + team1channel1post3, gm1post1, }}, {"team1, user1", user1ID, team1.Id, model.GetUserThreadsOpts{}, []*model.Post{ - team1channel1post3, + team1channel1post3, gm1post1, }}, - {"team2, user1", user1ID, team2.Id, model.GetUserThreadsOpts{}, []*model.Post{}}, + {"team2, user1", user1ID, team2.Id, model.GetUserThreadsOpts{}, []*model.Post{gm1post1}}, + {"team1, user1, exclude direct", user1ID, team2.Id, model.GetUserThreadsOpts{}, []*model.Post{team1channel1post3}}, } for _, testCase := range testCases { @@ -1183,6 +1194,9 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { {"team2, user1", user1ID, team2.Id, model.GetUserThreadsOpts{}, []*model.Post{ team2channel1post1, dm1post1, gm1post1, }}, + {"team2, user1, exclude direct", user1ID, team2.Id, model.GetUserThreadsOpts{ExcludeDirect: true}, []*model.Post{ + team2channel1post1, + }}, {"team2, user1, unread", user1ID, team2.Id, model.GetUserThreadsOpts{Unread: true}, []*model.Post{ gm1post1, // (no unread in team2) }}, @@ -1192,6 +1206,9 @@ func testVarious(t *testing.T, rctx request.CTX, ss store.Store) { {"team2, user1, unread + deleted", user1ID, team2.Id, model.GetUserThreadsOpts{Unread: true, Deleted: true}, []*model.Post{ team2channel1post2deleted, gm1post1, }}, + {"team2, user1, unread + deleted + exclude direct", user1ID, team2.Id, model.GetUserThreadsOpts{Unread: true, Deleted: true, ExcludeDirect: true}, []*model.Post{ + team2channel1post2deleted, + }}, } for _, testCase := range testCases { diff --git a/server/public/model/client4.go b/server/public/model/client4.go index bc54ff2c5a3..52b0551e2d7 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -8724,6 +8724,9 @@ func (c *Client4) GetUserThreads(ctx context.Context, userId, teamId string, opt if options.TotalsOnly { v.Set("totalsOnly", "true") } + if options.ExcludeDirect { + v.Set("excludeDirect", fmt.Sprintf("%t", options.ExcludeDirect)) + } url := c.userThreadsRoute(userId, teamId) if len(v) > 0 { url += "?" + v.Encode() diff --git a/server/public/model/thread.go b/server/public/model/thread.go index b510d1196a6..eccaefc5276 100644 --- a/server/public/model/thread.go +++ b/server/public/model/thread.go @@ -88,6 +88,8 @@ type GetUserThreadsOpts struct { // IncludeIsUrgent will return IsUrgent field as well to assert is the thread is urgent or not IncludeIsUrgent bool + + ExcludeDirect bool } func (o *Thread) Etag() string {