diff --git a/webapp/channels/src/components/backstage/components/backstage_list.tsx b/webapp/channels/src/components/backstage/components/backstage_list.tsx index 44afe92b932..9547e6a3591 100644 --- a/webapp/channels/src/components/backstage/components/backstage_list.tsx +++ b/webapp/channels/src/components/backstage/components/backstage_list.tsx @@ -24,6 +24,7 @@ type Props = { emptyText?: ReactNode; emptyTextSearch?: JSX.Element; helpText?: ReactNode; + error?: ReactNode; loading: boolean; searchPlaceholder?: string; nextPage?: () => void; @@ -153,6 +154,7 @@ const BackstageList = (remainingProps: Props) => { {addLink} + {remainingProps.error}
diff --git a/webapp/channels/src/components/integrations/bots/bots.test.tsx b/webapp/channels/src/components/integrations/bots/bots.test.tsx index e7cde914c57..47a6663bfcf 100644 --- a/webapp/channels/src/components/integrations/bots/bots.test.tsx +++ b/webapp/channels/src/components/integrations/bots/bots.test.tsx @@ -110,6 +110,226 @@ describe('components/integrations/bots/Bots', () => { expect(managedByTexts.filter((t) => t?.includes('plugin')).length).toBe(2); }); + it('paginates until a short page and processes bots beyond the first page', async () => { + // A full first page forces the component to request the next page. + const firstPage: Bot[] = []; + const allBots: Record = {}; + const allUsers: Record> = {}; + for (let i = 1; i <= 200; i++) { + const bot = TestHelper.getBotMock({user_id: String(i), username: `bot${i}`, display_name: `Bot ${i}`, delete_at: 0}); + firstPage.push(bot); + allBots[bot.user_id] = bot; + allUsers[bot.user_id] = TestHelper.getUserMock({id: bot.user_id}); + } + + const newestBot = TestHelper.getBotMock({user_id: '201', username: 'irisnewbot', display_name: 'Iris Newest Bot', delete_at: 0}); + allBots[newestBot.user_id] = newestBot; + allUsers[newestBot.user_id] = TestHelper.getUserMock({id: newestBot.user_id}); + + const loadBots = jest.fn((page?: number) => { + if (page === 0) { + return Promise.resolve({data: firstPage}); + } + if (page === 1) { + return Promise.resolve({data: [newestBot]}); + } + return Promise.resolve({data: []}); + }); + const getUser = jest.fn(); + + renderWithContext( + , + ); + + // The newest bot lives on the second page and is rendered once loading completes. + await waitFor(() => { + expect(screen.getByText(/Iris Newest Bot \(@irisnewbot\)/)).toBeInTheDocument(); + }); + + // Successive pages are requested with the server's max page size until a short page is returned. + expect(loadBots).toHaveBeenCalledWith(0, 200); + expect(loadBots).toHaveBeenCalledWith(1, 200); + expect(loadBots).toHaveBeenCalledTimes(2); + + // The second-page bot was accumulated and had its user details fetched. + expect(getUser).toHaveBeenCalledWith(newestBot.user_id); + }); + + it('requests one more page when the final data page is exactly full', async () => { + const makeFullPage = (start: number): Bot[] => { + const page: Bot[] = []; + for (let i = start; i < start + 200; i++) { + page.push(TestHelper.getBotMock({user_id: String(i), username: `bot${i}`, delete_at: 0})); + } + return page; + }; + + // Both data pages are exactly full, so termination requires a trailing empty page. + const secondPage = makeFullPage(200); + const lastBot = TestHelper.getBotMock({user_id: '400', username: 'bot400', delete_at: 0}); + secondPage[secondPage.length - 1] = lastBot; + + const loadBots = jest.fn((page?: number) => { + if (page === 0) { + return Promise.resolve({data: makeFullPage(0)}); + } + if (page === 1) { + return Promise.resolve({data: secondPage}); + } + return Promise.resolve({data: []}); + }); + const getUser = jest.fn(); + + renderWithContext( + , + ); + + await waitFor(() => expect(loadBots).toHaveBeenCalledTimes(3)); + expect(loadBots).toHaveBeenNthCalledWith(1, 0, 200); + expect(loadBots).toHaveBeenNthCalledWith(2, 1, 200); + expect(loadBots).toHaveBeenNthCalledWith(3, 2, 200); + expect(getUser).toHaveBeenCalledWith('400'); + }); + + it('surfaces an error and completes loading when the first page fetch fails', async () => { + const loadBots = jest.fn(() => Promise.resolve({error: {message: 'Failed to load bots'}})); + const getUser = jest.fn(); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByText('Bot accounts could not be loaded. Refresh the page to try again.')).toBeInTheDocument(); + }); + expect(screen.getByText('No bot accounts found')).toBeInTheDocument(); + expect(loadBots).toHaveBeenCalledTimes(1); + expect(loadBots).toHaveBeenCalledWith(0, 200); + expect(getUser).not.toHaveBeenCalled(); + }); + + it('surfaces an incomplete-list warning and shows loaded bots when a later page fetch fails', async () => { + const firstPage: Bot[] = []; + const allBots: Record = {}; + const allUsers: Record> = {}; + for (let i = 1; i <= 200; i++) { + const bot = TestHelper.getBotMock({user_id: String(i), username: `bot${i}`, display_name: `Bot ${i}`, delete_at: 0}); + firstPage.push(bot); + allBots[bot.user_id] = bot; + allUsers[bot.user_id] = TestHelper.getUserMock({id: bot.user_id}); + } + + // First page loads fully, but the second page fetch fails. + const loadBots = jest.fn((page?: number) => { + if (page === 0) { + return Promise.resolve({data: firstPage}); + } + return Promise.resolve({error: {message: 'Failed to load bots'}}); + }); + const getUser = jest.fn(); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByText('Some bot accounts could not be loaded, so this list may be incomplete. Refresh the page to try again.')).toBeInTheDocument(); + }); + + // The bots that did load are still rendered alongside the warning. + expect(screen.getByText(/Bot 1 \(@bot1\)/)).toBeInTheDocument(); + expect(loadBots).toHaveBeenCalledWith(0, 200); + expect(loadBots).toHaveBeenCalledWith(1, 200); + expect(loadBots).toHaveBeenCalledTimes(2); + expect(getUser).toHaveBeenCalledWith('1'); + }); + + it('completes loading when the first page fetch returns no data', async () => { + const loadBots = jest.fn(() => Promise.resolve({})); + const getUser = jest.fn(); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByText('No bot accounts found')).toBeInTheDocument(); + }); + expect(loadBots).toHaveBeenCalledTimes(1); + expect(loadBots).toHaveBeenCalledWith(0, 200); + expect(getUser).not.toHaveBeenCalled(); + }); + + it('stops after a single request when there are no bots', async () => { + const loadBots = jest.fn(() => Promise.resolve({data: []})); + const getUser = jest.fn(); + + renderWithContext( + , + ); + + await waitFor(() => { + expect(screen.getByText('No bot accounts found')).toBeInTheDocument(); + }); + expect(loadBots).toHaveBeenCalledTimes(1); + expect(loadBots).toHaveBeenCalledWith(0, 200); + expect(getUser).not.toHaveBeenCalled(); + }); + it('bot owner tokens', async () => { const bot1 = TestHelper.getBotMock({user_id: '1', owner_id: '1', username: 'bot1', display_name: 'Bot 1', delete_at: 0}); const bots = { diff --git a/webapp/channels/src/components/integrations/bots/bots.tsx b/webapp/channels/src/components/integrations/bots/bots.tsx index 298e518d0e6..933fea68d92 100644 --- a/webapp/channels/src/components/integrations/bots/bots.tsx +++ b/webapp/channels/src/components/integrations/bots/bots.tsx @@ -12,6 +12,7 @@ import type {RelationOneToOne} from '@mattermost/types/utilities'; import type {ActionResult} from 'mattermost-redux/types/actions'; +import AlertBanner from 'components/alert_banner'; import BackstageList from 'components/backstage/components/backstage_list'; import ExternalLink from 'components/external_link'; @@ -20,6 +21,10 @@ import * as Utils from 'utils/utils'; import Bot, {matchesFilter} from './bot'; +// The server clamps per_page to PerPageMaximum (200), so bots must be fetched +// in pages of that size and accumulated rather than in a single large request. +const BOTS_PER_PAGE = 200; + type Props = { /** @@ -101,9 +106,14 @@ type Props = { team: Team; } +// Distinguishes a clean load from a total failure (no bots fetched) and a +// partial failure (a later page failed, so the list is incomplete). +type LoadError = 'none' | 'full' | 'partial'; + type State = { loading: boolean; -} + loadError: LoadError; +}; export default class Bots extends React.PureComponent { public constructor(props: Props) { @@ -111,38 +121,95 @@ export default class Bots extends React.PureComponent { this.state = { loading: true, + loadError: 'none', }; } public componentDidMount(): void { - this.props.actions.loadBots( - Constants.Integrations.START_PAGE_NUM, - Constants.Integrations.PAGE_SIZE, - ).then( - (result) => { - if (result.data) { - const promises = []; + this.loadAllBots(); - for (const bot of result.data) { - // We don't need to wait for this and we need to accept failure in the case where bot.owner_id is a plugin id - this.props.actions.getUser(bot.owner_id); - - // We want to wait for these. - promises.push(this.props.actions.getUser(bot.user_id)); - promises.push(this.props.actions.getUserAccessTokensForUser(bot.user_id)); - } - - Promise.all(promises).then(() => { - this.setState({loading: false}); - }); - } - }, - ); if (this.props.appsEnabled) { this.props.actions.fetchAppsBotIDs(); } } + private async loadAllBots(): Promise { + const allBots: BotType[] = []; + let page = Constants.Integrations.START_PAGE_NUM; + let loadError: LoadError = 'none'; + + // Fetch successive pages until one comes back short, since the server + // caps each request at BOTS_PER_PAGE and never returns every bot at once. + for (;;) { + // eslint-disable-next-line no-await-in-loop + const result = await this.props.actions.loadBots(page, BOTS_PER_PAGE); + + // A failed fetch returns an error rather than data. Surface it so the + // user knows the list failed to load (first page) or is incomplete + // (a later page), instead of silently showing an empty/truncated list. + if (result.error) { + loadError = allBots.length > 0 ? 'partial' : 'full'; + break; + } + + if (!result.data) { + break; + } + + allBots.push(...result.data); + + if (result.data.length < BOTS_PER_PAGE) { + break; + } + page++; + } + + const promises = []; + for (const bot of allBots) { + // We don't need to wait for this and we need to accept failure in the case where bot.owner_id is a plugin id + this.props.actions.getUser(bot.owner_id); + + // We want to wait for these. + promises.push(this.props.actions.getUser(bot.user_id)); + promises.push(this.props.actions.getUserAccessTokensForUser(bot.user_id)); + } + + await Promise.all(promises); + this.setState({loading: false, loadError}); + } + + private renderLoadError(): JSX.Element | null { + if (this.state.loadError === 'none') { + return null; + } + + if (this.state.loadError === 'partial') { + return ( + + } + /> + ); + } + + return ( + + } + /> + ); + } + DisabledSection(props: {hasDisabled: boolean; disabledBots: JSX.Element[]; filter?: string}): JSX.Element | null { if (!props.hasDisabled) { return null; @@ -274,6 +341,7 @@ export default class Bots extends React.PureComponent { } searchPlaceholder={Utils.localizeMessage({id: 'bots.manage.search', defaultMessage: 'Search Bot Accounts'})} loading={this.state.loading} + error={this.renderLoadError()} > {this.bots} diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 806874c2bb5..11582dff08d 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -3936,6 +3936,8 @@ "bots.manage.empty": "No bot accounts found", "bots.manage.header": "Bot Accounts", "bots.manage.help1": "Use {botAccounts} to integrate with Mattermost through plugins or the API. Bot accounts are available to everyone on your server. ", + "bots.manage.load_error.full": "Bot accounts could not be loaded. Refresh the page to try again.", + "bots.manage.load_error.partial": "Some bot accounts could not be loaded, so this list may be incomplete. Refresh the page to try again.", "bots.manage.search": "Search Bot Accounts", "bots.managed_by": "Managed by ", "bots.token.confirm": "Delete",