[MM-69656] Paginate Bot Accounts list so all bots load and are searchable (#37336) (#37356)

Automatic Merge

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) => {
                 </h1>
                 {addLink}
             </div>
+            {remainingProps.error}
             <div className='backstage-filters'>
                 <div className='backstage-filter__search'>
                     <SearchIcon/>

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<string, Bot> = {};
+        const allUsers: Record<string, ReturnType<typeof TestHelper.getUserMock>> = {};
+        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(
+            <Bots
+                bots={allBots}
+                team={team}
+                accessTokens={{}}
+                owners={{}}
+                users={allUsers}
+                actions={{...actions, loadBots, getUser}}
+                appsEnabled={false}
+                appsBotIDs={[]}
+            />,
+        );
+
+        // 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(
+            <Bots
+                bots={{}}
+                team={team}
+                accessTokens={{}}
+                owners={{}}
+                users={{}}
+                actions={{...actions, loadBots, getUser}}
+                appsEnabled={false}
+                appsBotIDs={[]}
+            />,
+        );
+
+        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(
+            <Bots
+                bots={{}}
+                team={team}
+                accessTokens={{}}
+                owners={{}}
+                users={{}}
+                actions={{...actions, loadBots, getUser}}
+                appsEnabled={false}
+                appsBotIDs={[]}
+            />,
+        );
+
+        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<string, Bot> = {};
+        const allUsers: Record<string, ReturnType<typeof TestHelper.getUserMock>> = {};
+        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(
+            <Bots
+                bots={allBots}
+                team={team}
+                accessTokens={{}}
+                owners={{}}
+                users={allUsers}
+                actions={{...actions, loadBots, getUser}}
+                appsEnabled={false}
+                appsBotIDs={[]}
+            />,
+        );
+
+        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(
+            <Bots
+                bots={{}}
+                team={team}
+                accessTokens={{}}
+                owners={{}}
+                users={{}}
+                actions={{...actions, loadBots, getUser}}
+                appsEnabled={false}
+                appsBotIDs={[]}
+            />,
+        );
+
+        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(
+            <Bots
+                bots={{}}
+                team={team}
+                accessTokens={{}}
+                owners={{}}
+                users={{}}
+                actions={{...actions, loadBots, getUser}}
+                appsEnabled={false}
+                appsBotIDs={[]}
+            />,
+        );
+
+        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 = {

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<Props, State> {
     public constructor(props: Props) {
@@ -111,38 +121,95 @@ export default class Bots extends React.PureComponent<Props, State> {
 
         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<void> {
+        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 (
+                <AlertBanner
+                    mode='warning'
+                    message={
+                        <FormattedMessage
+                            id='bots.manage.load_error.partial'
+                            defaultMessage='Some bot accounts could not be loaded, so this list may be incomplete. Refresh the page to try again.'
+                        />
+                    }
+                />
+            );
+        }
+
+        return (
+            <AlertBanner
+                mode='danger'
+                message={
+                    <FormattedMessage
+                        id='bots.manage.load_error.full'
+                        defaultMessage='Bot accounts could not be loaded. Refresh the page to try again.'
+                    />
+                }
+            />
+        );
+    }
+
     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<Props, State> {
                 }
                 searchPlaceholder={Utils.localizeMessage({id: 'bots.manage.search', defaultMessage: 'Search Bot Accounts'})}
                 loading={this.state.loading}
+                error={this.renderLoadError()}
             >
                 {this.bots}
             </BackstageList>

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",