Improve updates of message table pagination. (#23013)

* Improve updates of message table pagination.

* Adding changelog

* Simplify state management by getting errors from store instead of response.

* Remove not needed type.

* Only show errors for related search type.

---------

Co-authored-by: Maksym Yadlovskyi <[email protected]>

Author: linuspahl

changelog/unreleased/issue-22321.toml

@@ -0,0 +1,5 @@
+type = "f"
+message = "Fixing issue with message table pagination on search page, where the active page was not set correctly when opening pages very fast."
+
+pulls = ["23013"]
+issues = ["22321"]

changelog/unreleased/issue-23002.toml

@@ -0,0 +1,5 @@
+type = "f"
+message = "Fixing issue with message table pagination on search page, where active page was not reset correctly when executing new search."
+
+pulls = ["23013"]
+issues = ["23002"]

graylog2-web-interface/src/components/common/PaginatedList.tsx

@@ -34,6 +34,7 @@ type Props = {
   pageSizes?: Array<number>;
   showPageSizeSelect?: boolean;
   totalItems: number;
+  enforcePageBounds?: boolean;
 };
 
 const ListBase = ({
@@ -48,6 +49,7 @@ const ListBase = ({
   setPagination,
   showPageSizeSelect,
   totalItems,
+  enforcePageBounds,
 }: Required<Props> & {
   currentPageSize: number;
   currentPage: number;
@@ -75,8 +77,10 @@ const ListBase = ({
   );
 
   useEffect(() => {
-    if (numberPages > 0 && currentPage > numberPages) _onChangePage(numberPages);
-  }, [currentPage, numberPages, _onChangePage]);
+    if (enforcePageBounds && numberPages > 0 && currentPage > numberPages) {
+      _onChangePage(numberPages);
+    }
+  }, [enforcePageBounds, currentPage, numberPages, _onChangePage]);
 
   return (
     <>
@@ -95,6 +99,7 @@ const ListBase = ({
       <IfInteractive>
         <div className={`text-center pagination-wrapper ${className ?? ''}`}>
           <Pagination
+            warnIfPageOutOfBounds={false}
             totalPages={numberPages}
             currentPage={currentPage}
             hidePreviousAndNextPageLinks={hidePreviousAndNextPageLinks}
@@ -171,38 +176,30 @@ const PaginatedList = ({
   showPageSizeSelect = true,
   totalItems,
   useQueryParameter = true,
+  enforcePageBounds = true,
 }: Props & {
   activePage?: number;
   pageSize?: number;
   useQueryParameter?: boolean;
 }) => {
+  const baseProps = {
+    enforcePageBounds: enforcePageBounds,
+    className: className,
+    hideFirstAndLastPageLinks: hideFirstAndLastPageLinks,
+    hidePreviousAndNextPageLinks: hidePreviousAndNextPageLinks,
+    onChange: onChange,
+    pageSizes: pageSizes,
+    pageSize: pageSize,
+    showPageSizeSelect: showPageSizeSelect,
+    totalItems: totalItems,
+  };
+
   if (useQueryParameter) {
-    return (
-      <ListBasedOnQueryParams
-        className={className}
-        hideFirstAndLastPageLinks={hideFirstAndLastPageLinks}
-        hidePreviousAndNextPageLinks={hidePreviousAndNextPageLinks}
-        onChange={onChange}
-        pageSizes={pageSizes}
-        pageSize={pageSize}
-        showPageSizeSelect={showPageSizeSelect}
-        totalItems={totalItems}>
-        {children}
-      </ListBasedOnQueryParams>
-    );
+    return <ListBasedOnQueryParams {...baseProps}>{children}</ListBasedOnQueryParams>;
   }
 
   return (
-    <ListWithOwnState
-      className={className}
-      hideFirstAndLastPageLinks={hideFirstAndLastPageLinks}
-      hidePreviousAndNextPageLinks={hidePreviousAndNextPageLinks}
-      onChange={onChange}
-      pageSizes={pageSizes}
-      pageSize={pageSize}
-      showPageSizeSelect={showPageSizeSelect}
-      totalItems={totalItems}
-      activePage={activePage}>
+    <ListWithOwnState {...baseProps} activePage={activePage}>
       {children}
     </ListWithOwnState>
   );

graylog2-web-interface/src/components/common/Pagination.tsx

@@ -32,6 +32,7 @@ type Props = {
   hideFirstAndLastPageLinks?: boolean;
   disabled?: boolean;
   onChange?: (nextPage: number) => void;
+  warnIfPageOutOfBounds?: boolean;
 };
 
 const StyledBootstrapPagination = styled(BootstrapPagination)(
@@ -185,14 +186,17 @@ const Pagination = ({
   hideFirstAndLastPageLinks = false,
   disabled = false,
   onChange = () => {},
+  warnIfPageOutOfBounds = true,
 }: Props) => {
   if (totalPages <= 1) {
     return null;
   }
 
   if (currentPage > totalPages) {
-    // eslint-disable-next-line no-console
-    console.warn('Pagination: `currentPage` prop should not be larger than `totalPages` prop.');
+    if (warnIfPageOutOfBounds) {
+      // eslint-disable-next-line no-console
+      console.warn('Pagination: `currentPage` prop should not be larger than `totalPages` prop.');
+    }
 
     return null;
   }

graylog2-web-interface/src/views/components/widgets/MessageList.test.tsx

@@ -19,6 +19,7 @@ import { render, screen, waitFor, within } from 'wrappedTestingLibrary';
 import * as Immutable from 'immutable';
 import userEvent from '@testing-library/user-event';
 
+import useSearchResult from 'views/hooks/useSearchResult';
 import { StoreMock as MockStore } from 'helpers/mocking';
 import asMock from 'helpers/mocking/AsMock';
 import { TIMESTAMP_FIELD, Messages } from 'views/Constants';
@@ -33,7 +34,6 @@ import { finishedLoading } from 'views/logic/slices/searchExecutionSlice';
 import type { AbsoluteTimeRange } from 'views/logic/queries/Query';
 import SearchResult from 'views/logic/SearchResult';
 import reexecuteSearchTypes from 'views/components/widgets/reexecuteSearchTypes';
-import type { SearchErrorResponse } from 'views/logic/SearchError';
 import TestStoreProvider from 'views/test/TestStoreProvider';
 import useViewsPlugin from 'views/test/testViewsPlugin';
 import useAutoRefresh from 'views/hooks/useAutoRefresh';
@@ -55,6 +55,7 @@ jest.mock('stores/inputs/InputsStore', () => ({
 }));
 
 jest.mock('views/hooks/useAutoRefresh');
+jest.mock('views/hooks/useSearchResult');
 
 const searchTypeResults = {
   'search-type-id': {
@@ -70,7 +71,13 @@ const dummySearchJobResults = {
   id: 'foo',
   owner: 'me',
   search_id: 'bar',
-  results: {},
+  results: {
+    'deadbeef': {
+      query: { search_types: [{ id: 'search-type-id', limit: 10000 }] },
+      execution_stats: {},
+      errors: [],
+    },
+  },
 };
 jest.mock('views/hooks/useActiveQueryId');
 jest.mock('views/components/widgets/useCurrentSearchTypesResults');
@@ -235,28 +242,28 @@ describe('MessageList', () => {
     await waitFor(() => expect(stopAutoRefresh).toHaveBeenCalledTimes(1));
   });
 
-  it('displays error description, when using pagination throws an error', async () => {
-    const dispatch = jest.fn().mockResolvedValue(
-      finishedLoading({
-        result: new SearchResult({
-          ...dummySearchJobResults,
-          errors: [
-            {
-              description: 'Error description',
-            } as SearchErrorResponse,
-          ],
-        }),
+  it('displays search result limit errors', async () => {
+    asMock(useSearchResult).mockReturnValue({
+      result: new SearchResult({
+        ...dummySearchJobResults,
+        errors: [
+          {
+            query_id: 'deadbeef',
+            search_type_id: 'search-type-id',
+            description: 'Error description',
+            backtrace: undefined,
+            type: 'result_window_limit',
+            result_window_limit: 10000,
+          },
+        ],
       }),
-    );
-    asMock(useViewsDispatch).mockReturnValue(dispatch);
-
-    const secondPageSize = 10;
-
-    render(<SimpleMessageList data={{ ...data, total: Messages.DEFAULT_LIMIT + secondPageSize }} />);
+    });
 
-    clickNextPageButton();
+    render(<SimpleMessageList />);
 
-    await screen.findByText('Error description');
+    await screen.findByText(
+      'Elasticsearch limits the search result to 10000 messages. With a page size of 10000 messages, you can use the first 1 pages. Error description',
+    );
   });
 
   it('calls render completion callback after first render', async () => {

graylog2-web-interface/src/views/components/widgets/MessageList.tsx

@@ -35,6 +35,7 @@ import useViewsDispatch from 'views/stores/useViewsDispatch';
 import reexecuteSearchTypes from 'views/components/widgets/reexecuteSearchTypes';
 import useOnSearchExecution from 'views/hooks/useOnSearchExecution';
 import useAutoRefresh from 'views/hooks/useAutoRefresh';
+import useSearchResult from 'views/hooks/useSearchResult';
 
 import RenderCompletionCallback from './RenderCompletionCallback';
 
@@ -49,11 +50,6 @@ const Wrapper = styled.div`
   }
 `;
 
-type Pagination = {
-  pageErrors: Array<{ description: string }>;
-  currentPage: number;
-};
-
 export type MessageListResult = {
   messages: Array<BackendMessage>;
   total: number;
@@ -65,12 +61,12 @@ type Props = WidgetComponentProps<MessagesWidgetConfig, MessageListResult> & {
   pageSize?: number;
 };
 
-const useResetPaginationOnSearchExecution = (setPagination: (pagination: Pagination) => void, currentPage) => {
+const useResetPaginationOnSearchExecution = (setCurrentPage: (pageNr: number) => void, currentPage) => {
   const resetPagination = useCallback(() => {
     if (currentPage !== 1) {
-      setPagination({ currentPage: 1, pageErrors: [] });
+      setCurrentPage(1);
     }
-  }, [currentPage, setPagination]);
+  }, [currentPage, setCurrentPage]);
   useOnSearchExecution(resetPagination);
 };
 
@@ -94,6 +90,13 @@ const useRenderCompletionCallback = () => {
   }, [renderCompletionCallback]);
 };
 
+const usePageErrors = (searchTypeId: string) => {
+  const searchResult = useSearchResult();
+  const errors = searchResult?.result?.errors ?? [];
+
+  return errors.filter((error) => error.searchTypeId === searchTypeId);
+};
+
 const MessageList = ({
   config,
   data: { id: searchTypeId, messages, total: totalMessages },
@@ -102,20 +105,19 @@ const MessageList = ({
   pageSize = Messages.DEFAULT_LIMIT,
   setLoadingState,
 }: Props) => {
-  const [{ currentPage, pageErrors }, setPagination] = useState<Pagination>({
-    pageErrors: [],
-    currentPage: 1,
-  });
+  const [currentPage, setCurrentPage] = useState<number>(1);
   const { stopAutoRefresh } = useAutoRefresh();
+
+  const pageErrors = usePageErrors(searchTypeId);
   const activeQueryId = useActiveQueryId();
   const searchTypes = useCurrentSearchTypesResults();
   const scrollContainerRef = useResetScrollPositionOnPageChange(currentPage);
   const dispatch = useViewsDispatch();
-  useResetPaginationOnSearchExecution(setPagination, currentPage);
+  useResetPaginationOnSearchExecution(setCurrentPage, currentPage);
   useRenderCompletionCallback();
 
   const handlePageChange = useCallback(
-    (pageNo: number) => {
+    (newCurrentPage: number) => {
       // execute search with new offset
       const { effectiveTimerange } = searchTypes[searchTypeId] as MessageResult;
       const searchTypePayload: SearchTypeOptions<{
@@ -124,21 +126,16 @@ const MessageList = ({
       }> = {
         [searchTypeId]: {
           limit: pageSize,
-          offset: pageSize * (pageNo - 1),
+          offset: pageSize * (newCurrentPage - 1),
         },
       };
 
       stopAutoRefresh();
       setLoadingState(true);
+      setCurrentPage(newCurrentPage);
 
-      dispatch(reexecuteSearchTypes(searchTypePayload, effectiveTimerange)).then((response) => {
-        const { result } = response.payload;
+      dispatch(reexecuteSearchTypes(searchTypePayload, effectiveTimerange)).then(() => {
         setLoadingState(false);
-
-        setPagination({
-          pageErrors: result.errors,
-          currentPage: pageNo,
-        });
       });
     },
     [dispatch, pageSize, searchTypeId, searchTypes, setLoadingState, stopAutoRefresh],
@@ -162,8 +159,11 @@ const MessageList = ({
           showPageSizeSelect={false}
           totalItems={totalMessages}
           pageSize={pageSize}
+          enforcePageBounds={false}
           useQueryParameter={false}>
-          {!pageErrors?.length ? (
+          {pageErrors.length > 0 ? (
+            <ErrorWidget errors={pageErrors} />
+          ) : (
             <MessageTable
               activeQueryId={activeQueryId}
               config={config}
@@ -173,8 +173,6 @@ const MessageList = ({
               setLoadingState={setLoadingState}
               messages={messages}
             />
-          ) : (
-            <ErrorWidget errors={pageErrors} />
           )}
         </PaginatedList>
       </Wrapper>

graylog2-web-interface/src/views/logic/SearchResult.ts

@@ -43,7 +43,7 @@ export type SearchJobResult = {
   owner: string;
   results: { [id: string]: any };
   search_id: SearchId;
-  errors: Array<SearchErrorResponse>;
+  errors: Array<SearchErrorResponse | ResultWindowLimitErrorResponse>;
 };
 
 class SearchResult {