From b4d8b6239c5018d6903eb11aa65a56213809a81e Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Fri, 18 Oct 2024 15:41:24 +0200 Subject: [PATCH] Excludes remote channels from channel search (#28708) * Excludes remote channels from channel search This is done through a new body parameter in the SearchAllChannels endpoint that allows to search for local only channels, which are either channels that are shared but marked as homed locally, or channels that are not shared at all. * fix lint * Fix tests --------- Co-authored-by: Caleb Roseland --- api/v4/source/channels.yaml | 9 ++++-- server/channels/api4/channel.go | 1 + server/channels/api4/channel_test.go | 30 ++++++++++++++++++- server/channels/app/channel.go | 1 + .../channels/store/sqlstore/channel_store.go | 10 +++++++ server/channels/store/store.go | 1 + .../channels/store/storetest/channel_store.go | 24 +++++++++++++++ server/public/model/channel.go | 1 + server/public/model/channel_search.go | 1 + .../modals/shared_channels_add_modal.tsx | 2 +- .../src/actions/channels.test.ts | 6 ++-- webapp/platform/client/src/client4.ts | 3 +- webapp/platform/types/src/channels.ts | 1 + 13 files changed, 82 insertions(+), 8 deletions(-) diff --git a/api/v4/source/channels.yaml b/api/v4/source/channels.yaml index 5641c171db..379422a35f 100644 --- a/api/v4/source/channels.yaml +++ b/api/v4/source/channels.yaml @@ -287,7 +287,6 @@ __Minimum server version__: 5.26 - deleted: type: boolean description: > @@ -314,7 +313,6 @@ required to use this parameter. __Minimum server version__: 5.35 - include_search_by_id: type: boolean default: false @@ -322,6 +320,13 @@ If set to true, returns channels where given search 'term' matches channel ID. __Minimum server version__: 5.35 + exclude_remote: + type: boolean + default: false + description: > + If set to true, only returns channels that are local to this server. + + __Minimum server version__: 10.2 description: The search terms and logic to use in the search. required: true responses: diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index 40377bf6bc..01166da9fb 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -1299,6 +1299,7 @@ func searchAllChannels(c *Context, w http.ResponseWriter, r *http.Request) { ExcludeGroupConstrained: props.ExcludeGroupConstrained, ExcludePolicyConstrained: props.ExcludePolicyConstrained, IncludeSearchById: props.IncludeSearchById, + ExcludeRemote: props.ExcludeRemote, Public: props.Public, Private: props.Private, IncludeDeleted: includeDeleted, diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 2bdb82e771..c7ab7662a9 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -1696,7 +1696,7 @@ func TestSearchArchivedChannels(t *testing.T) { } func TestSearchAllChannels(t *testing.T) { - th := Setup(t).InitBasic() + th := setupForSharedChannels(t).InitBasic() th.LoginSystemManager() defer th.TearDown() client := th.Client @@ -1737,6 +1737,29 @@ func TestSearchAllChannels(t *testing.T) { }) require.NoError(t, err) + // share the open and private channels, one homed locally and the + // other remotely + sco := &model.SharedChannel{ + ChannelId: openChannel.Id, + TeamId: openChannel.TeamId, + Home: true, + ShareName: "testsharelocal", + CreatorId: th.BasicChannel.CreatorId, + } + _, scoErr := th.App.ShareChannel(th.Context, sco) + require.NoError(t, scoErr) + + scp := &model.SharedChannel{ + ChannelId: privateChannel.Id, + TeamId: privateChannel.TeamId, + Home: false, + RemoteId: model.NewId(), + ShareName: "testshareremote", + CreatorId: th.BasicChannel.CreatorId, + } + _, scpErr := th.App.ShareChannel(th.Context, scp) + require.NoError(t, scpErr) + testCases := []struct { Description string Search *model.ChannelSearch @@ -1847,6 +1870,11 @@ func TestSearchAllChannels(t *testing.T) { &model.ChannelSearch{Term: "SearchAllChannels", ExcludeGroupConstrained: true}, []string{openChannel.Id, privateChannel.Id}, }, + { + "Search for local only channels", + &model.ChannelSearch{Term: "SearchAllChannels", ExcludeRemote: true}, + []string{openChannel.Id, groupConstrainedChannel.Id}, + }, } for _, testCase := range testCases { t.Run(testCase.Description, func(t *testing.T) { diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index d93735037e..6e433bb35c 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -2923,6 +2923,7 @@ func (a *App) SearchAllChannels(c request.CTX, term string, opts model.ChannelSe PolicyID: opts.PolicyID, IncludePolicyID: opts.IncludePolicyID, IncludeSearchById: opts.IncludeSearchById, + ExcludeRemote: opts.ExcludeRemote, ExcludePolicyConstrained: opts.ExcludePolicyConstrained, Public: opts.Public, Private: opts.Private, diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index ec81005e38..052009c757 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -3411,6 +3411,16 @@ func (s SqlChannelStore) channelSearchQuery(opts *store.ChannelSearchOpts) sq.Se }) } + if opts.ExcludeRemote { + // local channels either have a SharedChannels record with + // home set to true, or don't have a SharedChannels record at all + query = query.LeftJoin("SharedChannels ON c.Id = SharedChannels.ChannelId"). + Where(sq.Or{ + sq.Eq{"SharedChannels.Home": true}, + sq.Eq{"SharedChannels.ChannelId": nil}, + }) + } + return query } diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 2681c73030..d0ca780484 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1074,6 +1074,7 @@ type ChannelSearchOpts struct { IncludePolicyID bool IncludeTeamInfo bool IncludeSearchById bool + ExcludeRemote bool CountOnly bool Public bool Private bool diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index 5a226902ad..39c5987d39 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -6506,6 +6506,29 @@ func testChannelStoreSearchAllChannels(t *testing.T, rctx request.CTX, ss store. }) require.NoError(t, nErr) + // Mark o12 and o14 as shared, o13 will be homed locally and o14 + // will be homed remotely + sc12 := &model.SharedChannel{ + ChannelId: o12.Id, + TeamId: o12.TeamId, + CreatorId: model.NewId(), + ShareName: "testsharelocal", + Home: true, + } + _, scErr := ss.SharedChannel().Save(sc12) + require.NoError(t, scErr) + + sc14 := &model.SharedChannel{ + ChannelId: o14.Id, + TeamId: o14.TeamId, + CreatorId: model.NewId(), + ShareName: "testshareremote", + Home: false, + RemoteId: model.NewId(), + } + _, sc2Err := ss.SharedChannel().Save(sc14) + require.NoError(t, sc2Err) + testCases := []struct { Description string Term string @@ -6550,6 +6573,7 @@ func testChannelStoreSearchAllChannels(t *testing.T, rctx request.CTX, ss store. {"Filter team 1 and team 2, public and private and exclude group constrained", "", store.ChannelSearchOpts{IncludeDeleted: false, TeamIds: []string{t1.Id, t2.Id}, Public: true, Private: true, ExcludeGroupConstrained: true, Page: model.NewPointer(0), PerPage: model.NewPointer(5)}, model.ChannelList{&o1, &o2, &o3, &o4, &o6}, 12}, {"Filter deleted returns only deleted channels", "", store.ChannelSearchOpts{Deleted: true, Page: model.NewPointer(0), PerPage: model.NewPointer(5)}, model.ChannelList{&o13}, 1}, {"Search ChannelA by id", o1.Id, store.ChannelSearchOpts{IncludeDeleted: false, Page: model.NewPointer(0), PerPage: model.NewPointer(5), IncludeSearchById: true}, model.ChannelList{&o1}, 1}, + {"Filter excluding remote channels", "", store.ChannelSearchOpts{IncludeDeleted: false, ExcludeRemote: true}, model.ChannelList{&o1, &o2, &o3, &o4, &o5, &o6, &o7, &o8, &o9, &o10, &o11, &o12}, 0}, } for _, testCase := range testCases { diff --git a/server/public/model/channel.go b/server/public/model/channel.go index d70f01197f..786a4f281d 100644 --- a/server/public/model/channel.go +++ b/server/public/model/channel.go @@ -180,6 +180,7 @@ type ChannelSearchOpts struct { ExcludePolicyConstrained bool IncludePolicyID bool IncludeSearchById bool + ExcludeRemote bool Public bool Private bool Page *int diff --git a/server/public/model/channel_search.go b/server/public/model/channel_search.go index 7c933cf574..6e41f622dd 100644 --- a/server/public/model/channel_search.go +++ b/server/public/model/channel_search.go @@ -17,6 +17,7 @@ type ChannelSearch struct { Private bool `json:"private"` IncludeDeleted bool `json:"include_deleted"` IncludeSearchById bool `json:"include_search_by_id"` + ExcludeRemote bool `json:"exclude_remote"` Deleted bool `json:"deleted"` Page *int `json:"page,omitempty"` PerPage *int `json:"per_page,omitempty"` diff --git a/webapp/channels/src/components/admin_console/secure_connections/modals/shared_channels_add_modal.tsx b/webapp/channels/src/components/admin_console/secure_connections/modals/shared_channels_add_modal.tsx index e7af5f5b4c..c89e19bc9f 100644 --- a/webapp/channels/src/components/admin_console/secure_connections/modals/shared_channels_add_modal.tsx +++ b/webapp/channels/src/components/admin_console/secure_connections/modals/shared_channels_add_modal.tsx @@ -79,7 +79,7 @@ function SharedChannelsAddModal({ return []; } - const {data} = await dispatch(searchAllChannels(query, {page: 0, per_page: 20, signal})); + const {data} = await dispatch(searchAllChannels(query, {page: 0, per_page: 20, exclude_remote: true, signal})); if (data) { return data.channels.filter(({id}) => { const remote = remotesByChannelId?.[id]; diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/channels.test.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/channels.test.ts index 793ff5b937..9191655365 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/channels.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/channels.test.ts @@ -1132,7 +1132,7 @@ describe('Actions.Channels', () => { ); nock(Client4.getBaseRoute()). - post('/channels/search?include_deleted=false'). + post('/channels/search?include_deleted=false&exclude_remote=false'). reply(200, [TestHelper.basicChannel, userChannel]); await store.dispatch(Actions.searchAllChannels('test', {})); @@ -1143,7 +1143,7 @@ describe('Actions.Channels', () => { } nock(Client4.getBaseRoute()). - post('/channels/search?include_deleted=false'). + post('/channels/search?include_deleted=false&exclude_remote=false'). reply(200, {channels: [TestHelper.basicChannel, userChannel], total_count: 2}); let response = await store.dispatch(Actions.searchAllChannels('test', {exclude_default_channels: false, page: 0, per_page: 100})); @@ -1156,7 +1156,7 @@ describe('Actions.Channels', () => { expect(response.data.channels.length === 2).toBeTruthy(); nock(Client4.getBaseRoute()). - post('/channels/search?include_deleted=true'). + post('/channels/search?include_deleted=true&exclude_remote=false'). reply(200, {channels: [TestHelper.basicChannel, userChannel], total_count: 2}); response = await store.dispatch(Actions.searchAllChannels('test', {exclude_default_channels: false, page: 0, per_page: 100, include_deleted: true})); diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 7f6282c03c..773c658918 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -1961,7 +1961,8 @@ export default class Client4 { }; const includeDeleted = Boolean(opts.include_deleted); const nonAdminSearch = Boolean(opts.nonAdminSearch); - let queryParams: {include_deleted?: boolean; system_console?: boolean} = {include_deleted: includeDeleted}; + const excludeRemote = Boolean(opts.exclude_remote); + let queryParams: {include_deleted?: boolean; system_console?: boolean; exclude_remote?: boolean} = {include_deleted: includeDeleted, exclude_remote: excludeRemote}; if (nonAdminSearch) { queryParams = {system_console: false}; delete body.nonAdminSearch; diff --git a/webapp/platform/types/src/channels.ts b/webapp/platform/types/src/channels.ts index 76606c30c3..94a3c2f71f 100644 --- a/webapp/platform/types/src/channels.ts +++ b/webapp/platform/types/src/channels.ts @@ -211,6 +211,7 @@ export type ChannelSearchOpts = { private?: boolean; include_deleted?: boolean; include_search_by_id?: boolean; + exclude_remote?: boolean; deleted?: boolean; page?: number; per_page?: number;