From ed4cab7aa2020eb59407a8130fb07f6b263a38bb Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Mon, 7 Oct 2024 09:13:31 -0400 Subject: [PATCH] [MM-54021][MM-58736] Remove limit on loading channel members on initial load, reload members on reconnect, separate loading of members from channels on initial load (#28310) * [MM-54021][MM-58736] Remove limit on loading channel members on initial load, reload members on reconnect, separate loading of members from channels on initial load * PR feedback --------- Co-authored-by: Mattermost Build --- .../src/actions/websocket_actions.jsx | 2 + .../src/actions/websocket_actions.test.jsx | 1 + .../suggestion/switch_channel_provider.tsx | 6 +- .../src/components/team_controller/index.ts | 5 +- .../team_controller/team_controller.tsx | 7 +- .../src/actions/channels.test.ts | 23 +++++++ .../mattermost-redux/src/actions/channels.ts | 66 ++++++++++--------- 7 files changed, 71 insertions(+), 39 deletions(-) diff --git a/webapp/channels/src/actions/websocket_actions.jsx b/webapp/channels/src/actions/websocket_actions.jsx index 2b93208f68..b8a1323db9 100644 --- a/webapp/channels/src/actions/websocket_actions.jsx +++ b/webapp/channels/src/actions/websocket_actions.jsx @@ -31,6 +31,7 @@ import { getChannelStats, markMultipleChannelsAsRead, getChannelMemberCountsByGroup, + fetchAllMyChannelMembers, } from 'mattermost-redux/actions/channels'; import {getCloudSubscription} from 'mattermost-redux/actions/cloud'; import {clearErrors, logError} from 'mattermost-redux/actions/errors'; @@ -235,6 +236,7 @@ export function reconnect() { } dispatch(loadChannelsForCurrentUser()); + dispatch(fetchAllMyChannelMembers()); if (mostRecentPost) { dispatch(syncPostsInChannel(currentChannelId, mostRecentPost.create_at)); diff --git a/webapp/channels/src/actions/websocket_actions.test.jsx b/webapp/channels/src/actions/websocket_actions.test.jsx index 2ee1f0c94b..78ca412a07 100644 --- a/webapp/channels/src/actions/websocket_actions.test.jsx +++ b/webapp/channels/src/actions/websocket_actions.test.jsx @@ -62,6 +62,7 @@ jest.mock('mattermost-redux/actions/users', () => ({ jest.mock('mattermost-redux/actions/channels', () => ({ getChannelStats: jest.fn(() => ({type: 'GET_CHANNEL_STATS'})), + fetchAllMyChannelMembers: jest.fn(() => ({type: 'FETCH_ALL_MY_CHANNEL_MEMBERS'})), })); jest.mock('actions/post_actions', () => ({ diff --git a/webapp/channels/src/components/suggestion/switch_channel_provider.tsx b/webapp/channels/src/components/suggestion/switch_channel_provider.tsx index 020caffb5f..d96b40d579 100644 --- a/webapp/channels/src/components/suggestion/switch_channel_provider.tsx +++ b/webapp/channels/src/components/suggestion/switch_channel_provider.tsx @@ -13,7 +13,7 @@ import type {UserProfile} from '@mattermost/types/users'; import type {RelationOneToOne} from '@mattermost/types/utilities'; import {UserTypes} from 'mattermost-redux/action_types'; -import {fetchAllMyTeamsChannelsAndChannelMembersREST, searchAllChannels} from 'mattermost-redux/actions/channels'; +import {fetchAllMyTeamsChannels, searchAllChannels} from 'mattermost-redux/actions/channels'; import {logError} from 'mattermost-redux/actions/errors'; import {Client4} from 'mattermost-redux/client'; import {Preferences} from 'mattermost-redux/constants'; @@ -838,12 +838,12 @@ export default class SwitchChannelProvider extends Provider { if (!teamId) { return; } - const channelsAsync = this.store.dispatch(fetchAllMyTeamsChannelsAndChannelMembersREST()); + const channelsAsync = this.store.dispatch(fetchAllMyTeamsChannels()); let channels; try { const {data} = await channelsAsync; - channels = data.channels as Channel[]; + channels = data as Channel[]; } catch (err) { this.store.dispatch(logError(err)); return; diff --git a/webapp/channels/src/components/team_controller/index.ts b/webapp/channels/src/components/team_controller/index.ts index 1cff6ac154..07e2844a15 100644 --- a/webapp/channels/src/components/team_controller/index.ts +++ b/webapp/channels/src/components/team_controller/index.ts @@ -5,7 +5,7 @@ import {connect} from 'react-redux'; import type {ConnectedProps} from 'react-redux'; import type {RouteComponentProps} from 'react-router-dom'; -import {fetchAllMyTeamsChannelsAndChannelMembersREST, fetchChannelsAndMembers, unsetActiveChannelOnServer} from 'mattermost-redux/actions/channels'; +import {fetchAllMyTeamsChannels, fetchAllMyChannelMembers, fetchChannelsAndMembers, unsetActiveChannelOnServer} from 'mattermost-redux/actions/channels'; import {getCurrentChannelId} from 'mattermost-redux/selectors/entities/channels'; import {getLicense, getConfig} from 'mattermost-redux/selectors/entities/general'; import {getCurrentTeamId, getMyTeams} from 'mattermost-redux/selectors/entities/teams'; @@ -53,7 +53,8 @@ function mapStateToProps(state: GlobalState, ownProps: OwnProps) { const mapDispatchToProps = { fetchChannelsAndMembers, - fetchAllMyTeamsChannelsAndChannelMembersREST, + fetchAllMyTeamsChannels, + fetchAllMyChannelMembers, markAsReadOnFocus, initializeTeam, joinTeam, diff --git a/webapp/channels/src/components/team_controller/team_controller.tsx b/webapp/channels/src/components/team_controller/team_controller.tsx index 89fa10b0ea..7d2fe7aa26 100644 --- a/webapp/channels/src/components/team_controller/team_controller.tsx +++ b/webapp/channels/src/components/team_controller/team_controller.tsx @@ -55,13 +55,14 @@ function TeamController(props: Props) { useEffect(() => { InitialLoadingScreen.stop(); - async function fetchInitialChannels() { - await props.fetchAllMyTeamsChannelsAndChannelMembersREST(); + async function fetchAllChannels() { + await props.fetchAllMyTeamsChannels(); setInitialChannelsLoaded(true); } - fetchInitialChannels(); + props.fetchAllMyChannelMembers(); + fetchAllChannels(); }, []); useEffect(() => { 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 f86e4514cc..793ff5b937 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 @@ -2088,4 +2088,27 @@ describe('Actions.Channels', () => { expect(channelMemberCounts['group-2'].channel_member_count).toEqual(999); expect(channelMemberCounts['group-2'].channel_member_timezones_count).toEqual(131); }); + + it('fetchAllMyChannelMembers', async () => { + const store = configureStore({ + entities: { + users: { + currentUserId: 'some-user-id', + }, + }, + }); + + nock(Client4.getBaseRoute()).get( + '/users/some-user-id/channel_members?page=0&per_page=200'). + reply(200, [...Array(200).keys()].map((index) => ({channel_id: `channel-${index}`, user_id: 'some-user-id'}))); + nock(Client4.getBaseRoute()).get( + '/users/some-user-id/channel_members?page=1&per_page=200'). + reply(200, [...Array(200).keys()].map((index) => ({channel_id: `channel-${index + 200}`, user_id: 'some-user-id'}))); + nock(Client4.getBaseRoute()).get( + '/users/some-user-id/channel_members?page=2&per_page=200'). + reply(200, [...Array(100).keys()].map((index) => ({channel_id: `channel-${index + 400}`, user_id: 'some-user-id'}))); + + await store.dispatch(Actions.fetchAllMyChannelMembers()); + expect(Object.keys(store.getState().entities.channels.myMembers).length).toBe(500); + }); }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts index ad550537a3..1f5fe90716 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/channels.ts @@ -464,32 +464,43 @@ export function fetchChannelsAndMembers(teamId: string): ActionFuncAsync<{channe }; } -export function fetchAllMyTeamsChannelsAndChannelMembersREST(): ActionFuncAsync { +export function fetchAllMyChannelMembers(): ActionFuncAsync { return async (dispatch, getState) => { const state = getState(); const {currentUserId} = state.entities.users; - let channels; + let channelsMembers: ChannelMembership[] = []; - let allMembers = true; + let hasMoreMembers = true; let page = 0; - do { - try { + try { + while (hasMoreMembers) { + // Expected to disable since we don't have number of pages, so we can't use Promise.all // eslint-disable-next-line no-await-in-loop - await Client4.getAllChannelsMembers(currentUserId, page, 200).then( - // eslint-disable-next-line no-loop-func - (data) => { - channelsMembers = [...channelsMembers, ...data]; - page++; - if (data.length < 200) { - allMembers = false; - } - }); - } catch (error) { - forceLogoutIfNecessary(error, dispatch, getState); - dispatch(logError(error)); - return {error}; + const data = await Client4.getAllChannelsMembers(currentUserId, page, 200); + channelsMembers = [...channelsMembers, ...data]; + if (data.length < 200) { + hasMoreMembers = false; + } + page++; } - } while (allMembers && page <= 2); + } catch (error) { + forceLogoutIfNecessary(error, dispatch, getState); + dispatch(logError(error)); + return {error}; + } + + dispatch({ + type: ChannelTypes.RECEIVED_MY_CHANNEL_MEMBERS, + data: channelsMembers, + currentUserId, + }); + return {data: channelsMembers}; + }; +} + +export function fetchAllMyTeamsChannels(): ActionFuncAsync { + return async (dispatch, getState) => { + let channels; try { channels = await Client4.getAllTeamsChannels(); } catch (error) { @@ -498,18 +509,11 @@ export function fetchAllMyTeamsChannelsAndChannelMembersREST(): ActionFuncAsync return {error}; } - dispatch(batchActions([ - { - type: ChannelTypes.RECEIVED_ALL_CHANNELS, - data: channels, - }, - { - type: ChannelTypes.RECEIVED_MY_CHANNEL_MEMBERS, - data: channelsMembers, - currentUserId, - }, - ])); - return {data: {channels, channelsMembers}}; + dispatch({ + type: ChannelTypes.RECEIVED_ALL_CHANNELS, + data: channels, + }); + return {data: channels}; }; }