[Lens][Agg based] Fix table pagination bug for both lens and agg based (

elastic#139160)
Mpdreamz · Sep 6, 2022 · 8fc3e56 · 8fc3e56
1 parent 71b1bc9
commit 8fc3e56
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 6 deletions.
diff --git a/src/plugins/vis_types/table/public/components/table_vis_cell.tsx b/src/plugins/vis_types/table/public/components/table_vis_cell.tsx
@@ -15,9 +15,10 @@ import { FormattedColumns } from '../types';
 export const createTableVisCell =
   (rows: DatatableRow[], formattedColumns: FormattedColumns, autoFitRowToContent?: boolean) =>
   ({ rowIndex, columnId }: EuiDataGridCellValueElementProps) => {
-    const rowValue = rows[rowIndex][columnId];
+    // incoming data might change and put the current page out of bounds - check whether row actually exists
+    const rowValue = rows[rowIndex]?.[columnId];
     const column = formattedColumns[columnId];
-    const content = column.formatter.convert(rowValue, 'html');
+    const content = column?.formatter.convert(rowValue, 'html');
 
     const cellContent = (
       <div

diff --git a/src/plugins/vis_types/table/public/components/table_vis_columns.tsx b/src/plugins/vis_types/table/public/components/table_vis_columns.tsx
@@ -61,7 +61,8 @@ export const createGridColumns = (
     const cellActions = formattedColumn.filterable
       ? [
           ({ rowIndex, columnId, Component }: EuiDataGridColumnCellActionProps) => {
-            const rowValue = rows[rowIndex][columnId];
+            // incoming data might change and put the current page out of bounds - check whether row actually exists
+            const rowValue = rows[rowIndex]?.[columnId];
             if (rowValue == null) return null;
             const cellContent = formattedColumn.formatter.convert(rowValue);
 
@@ -96,7 +97,8 @@ export const createGridColumns = (
             );
           },
           ({ rowIndex, columnId, Component }: EuiDataGridColumnCellActionProps) => {
-            const rowValue = rows[rowIndex][columnId];
+            // incoming data might change and put the current page out of bounds - check whether row actually exists
+            const rowValue = rows[rowIndex]?.[columnId];
             if (rowValue == null) return null;
             const cellContent = formattedColumn.formatter.convert(rowValue);
 

diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/columns.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/columns.tsx
@@ -63,7 +63,8 @@ export const createGridColumns = (
     rowIndex,
     columnId,
   }: Pick<EuiDataGridColumnCellActionProps, 'rowIndex' | 'columnId'>) => {
-    const rowValue = table.rows[rowIndex][columnId];
+    // incoming data might change and put the current page out of bounds - check whether row actually exists
+    const rowValue = table.rows[rowIndex]?.[columnId];
     const column = columnsReverseLookup?.[columnId];
     const contentsIsDefined = rowValue != null;
 

diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx
@@ -739,7 +739,9 @@ describe('DatatableComponent', () => {
     it('enables pagination', async () => {
       const { data, args } = sampleArgs();
 
-      args.pageSize = 10;
+      data.rows = new Array(10).fill({ a: 'shoes', b: 1588024800000, c: 3 });
+
+      args.pageSize = 2;
 
       const wrapper = mount(
         <DatatableComponent
@@ -772,6 +774,57 @@ describe('DatatableComponent', () => {
       expect(updatedConfig?.pageSize).toBe(args.pageSize);
     });
 
+    it('resets page position if rows change so page will be empty', async () => {
+      const { data, args } = sampleArgs();
+
+      data.rows = new Array(10).fill({ a: 'shoes', b: 1588024800000, c: 3 });
+
+      args.pageSize = 2;
+
+      const wrapper = mount(
+        <DatatableComponent
+          data={data}
+          args={args}
+          formatFactory={(x) => x as unknown as IFieldFormat}
+          dispatchEvent={onDispatchEvent}
+          getType={jest.fn()}
+          paletteService={chartPluginMock.createPaletteRegistry()}
+          uiSettings={{ get: jest.fn() } as unknown as IUiSettingsClient}
+          renderMode="edit"
+          interactive
+          renderComplete={renderComplete}
+        />
+      );
+      const newIndex = 3;
+      act(() => wrapper.find(EuiDataGrid).prop('pagination')?.onChangePage(newIndex));
+      wrapper.update();
+
+      expect(wrapper.find(EuiDataGrid).prop('pagination')?.pageIndex).toBe(newIndex);
+
+      wrapper.setProps({
+        data: {
+          ...data,
+          rows: new Array(20).fill({ a: 'shoes', b: 1588024800000, c: 3 }),
+        },
+      });
+
+      await waitForWrapperUpdate(wrapper);
+
+      // keeps existing page if more data is added
+      expect(wrapper.find(EuiDataGrid).prop('pagination')?.pageIndex).toBe(newIndex);
+
+      wrapper.setProps({
+        data: {
+          ...data,
+          rows: new Array(3).fill({ a: 'shoes', b: 1588024800000, c: 3 }),
+        },
+      });
+
+      await waitForWrapperUpdate(wrapper);
+      // resets to the last page if the current page becomes out of bounds
+      expect(wrapper.find(EuiDataGrid).prop('pagination')?.pageIndex).toBe(1);
+    });
+
     it('disables pagination by default', async () => {
       const { data, args } = sampleArgs();
 

diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx
@@ -114,6 +114,25 @@ export const DatatableComponent = (props: DatatableRenderProps) => {
   const firstTableRef = useRef(firstLocalTable);
   firstTableRef.current = firstLocalTable;
 
+  useEffect(() => {
+    if (!pagination?.pageIndex && !pagination?.pageSize) return;
+    const lastPageIndex = Math.ceil(firstLocalTable.rows.length / pagination.pageSize) - 1;
+    /**
+     * When the underlying data changes, there might be a case when actual pagination page
+     * doesn't exist anymore - if the number of rows has decreased.
+     * Set the last page as an actual.
+     */
+    setPagination((pag) => {
+      if (!pag) {
+        return pag;
+      }
+      return {
+        pageIndex: pag.pageIndex > lastPageIndex ? lastPageIndex : pag.pageIndex,
+        pageSize: pag.pageSize,
+      };
+    });
+  }, [pagination?.pageIndex, pagination?.pageSize, firstLocalTable.rows.length]);
+
   const untransposedDataRef = useRef(props.untransposedData);
   untransposedDataRef.current = props.untransposedData;