Merge pull request #1148 from ral-facilities/fix-preserved-table-stat…

…e-filter-bug-#1146

Fix filtering and similar being on empty page when there are results #1146

src/common/preservedTableState.component.test.tsx

@@ -549,6 +549,89 @@ describe('Preserved table state functions', () => {
       );
     });
 
+    it('onColumnFiltersChange updates the state and url correctly when page index is different', async () => {
+      const { result } = renderHookWithBrowserRouterURL(
+        () =>
+          usePreservedTableState({
+            storeInUrl: true,
+          }),
+        '/'
+      );
+
+      // The first change should define the default order (MRT will call this once on page load, with the initial default
+      // state defined either inside the defaultState/the initialState given)
+      act(() =>
+        result.current.onPreservedStatesChange.onPaginationChange({
+          pageSize: 15,
+          pageIndex: 0,
+        })
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+          JSON.stringify({
+            pageSize: 15,
+            pageIndex: 0,
+          })
+        )
+      );
+
+      // Change the page to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onPaginationChange({
+          pageSize: 30,
+          pageIndex: 5,
+        })
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+          JSON.stringify({
+            pageSize: 30,
+            pageIndex: 5,
+          })
+        )
+      );
+
+      // Change the state to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onColumnFiltersChange([
+          { id: 'catalogueItem.name', value: 'test' },
+        ])
+      );
+
+      await waitFor(() =>
+        expect(
+          JSON.stringify(result.current.preservedState.columnFilters)
+        ).toBe(JSON.stringify([{ id: 'catalogueItem.name', value: 'test' }]))
+      );
+
+      // Change the state to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onColumnFiltersChange([
+          { id: 'catalogueItem.name', value: 'test' },
+        ])
+      );
+
+      await waitFor(() =>
+        expect(
+          JSON.stringify(result.current.preservedState.columnFilters)
+        ).toBe(JSON.stringify([{ id: 'catalogueItem.name', value: 'test' }]))
+      );
+
+      // Should have kept the page size but reset the index
+      expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+        JSON.stringify({
+          pageSize: 30,
+          pageIndex: 0,
+        })
+      );
+
+      expect(window.location.search).toBe(
+        '?state=N4IgxgYiBcDaoEsAmNwEMAuaA2B7A5gK4CmAkhsQLYB0AdmpcSADQgBuOJMoGAngA5NoIAM4YATglr4W7TkJAUxIAL4qAuq37cQ-NPmIBlBAC8hAZgAMW-WVpJiADxiW1QA'
+      );
+    });
+
     it('onSortingChange updates the state and url correctly', async () => {
       const { result } = renderHookWithBrowserRouterURL(
         () => usePreservedTableState({ storeInUrl: true }),
@@ -772,6 +855,71 @@ describe('Preserved table state functions', () => {
       expect(window.location.search).toBe('');
     });
 
+    it('onGlobalFilterChange updates the state and url correctly when page index is different', async () => {
+      const { result } = renderHookWithBrowserRouterURL(
+        () => usePreservedTableState({ storeInUrl: true }),
+        '/'
+      );
+
+      // The first change should define the default order (MRT will call this once on page load, with the initial default
+      // state defined either inside the defaultState/the initialState given)
+      act(() =>
+        result.current.onPreservedStatesChange.onPaginationChange({
+          pageSize: 15,
+          pageIndex: 0,
+        })
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+          JSON.stringify({
+            pageSize: 15,
+            pageIndex: 0,
+          })
+        )
+      );
+
+      // Change the page to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onPaginationChange({
+          pageSize: 30,
+          pageIndex: 5,
+        })
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+          JSON.stringify({
+            pageSize: 30,
+            pageIndex: 5,
+          })
+        )
+      );
+
+      // Change the state to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onGlobalFilterChange(
+          'test filter'
+        )
+      );
+
+      await waitFor(() =>
+        expect(result.current.preservedState.globalFilter).toBe('test filter')
+      );
+
+      // Should have kept the page size but reset the index
+      expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+        JSON.stringify({
+          pageSize: 30,
+          pageIndex: 0,
+        })
+      );
+
+      expect(window.location.search).toBe(
+        '?state=N4IgDiBcpghg5gUwMoEsBeioGYAMAacBRASQDsATRADylwF9D4AxVAGyhABdEBnLgAQAzdjwBOIekA'
+      );
+    });
+
     it('onGroupingChange updates the state and url correctly', async () => {
       const { result } = renderHookWithBrowserRouterURL(
         () =>
@@ -851,6 +999,76 @@ describe('Preserved table state functions', () => {
       expect(window.location.search).toBe('');
     });
 
+    it('onGroupingChange updates the state and url correctly when page index is different', async () => {
+      const { result } = renderHookWithBrowserRouterURL(
+        () =>
+          usePreservedTableState({
+            storeInUrl: true,
+          }),
+        '/'
+      );
+
+      // The first change should define the default order (MRT will call this once on page load, with the initial default
+      // state defined either inside the defaultState/the initialState given)
+      act(() =>
+        result.current.onPreservedStatesChange.onPaginationChange({
+          pageSize: 15,
+          pageIndex: 0,
+        })
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+          JSON.stringify({
+            pageSize: 15,
+            pageIndex: 0,
+          })
+        )
+      );
+
+      // Change the page to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onPaginationChange({
+          pageSize: 30,
+          pageIndex: 5,
+        })
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+          JSON.stringify({
+            pageSize: 30,
+            pageIndex: 5,
+          })
+        )
+      );
+
+      // Change the state to a non-default value
+      act(() =>
+        result.current.onPreservedStatesChange.onGroupingChange([
+          'catalogueItem.is_obsolete',
+        ])
+      );
+
+      await waitFor(() =>
+        expect(JSON.stringify(result.current.preservedState.grouping)).toBe(
+          JSON.stringify(['catalogueItem.is_obsolete'])
+        )
+      );
+
+      // Should have kept the page size but reset the index
+      expect(JSON.stringify(result.current.preservedState.pagination)).toBe(
+        JSON.stringify({
+          pageSize: 30,
+          pageIndex: 0,
+        })
+      );
+
+      expect(window.location.search).toBe(
+        '?state=N4IgDiBcpghg5gUwMoEsBeioGYAMAacBRASQDsATRADylwF9D4oBtEAY1gBdYAbAe3gBXUl0QBbAHSoAzgH1%2BAIxn9eiMSAC69IA'
+      );
+    });
+
     it('onColumnOrderChange updates the state and url correctly', async () => {
       const { result } = renderHookWithBrowserRouterURL(
         () => usePreservedTableState({ storeInUrl: true }),
@@ -1047,7 +1265,7 @@ describe('Preserved table state functions', () => {
         '/'
       );
 
-      // The first change should define the default order (MRT will call this once on page load, with the intial default
+      // The first change should define the default order (MRT will call this once on page load, with the initial default
       // state defined either inside the defaultState/the initialState given)
       act(() =>
         result.current.onPreservedStatesChange.onPaginationChange({

src/common/preservedTableState.component.tsx

@@ -25,7 +25,7 @@ interface State {
 }
 
 /* State but where undefined => should not be present in the url */
-interface StatePartial extends Partial<State> {}
+type StatePartial = Partial<State>;
 
 /* Column filter value but defined as it will be stored in URL search params (includes type information) */
 interface SearchParamsColumnFilterValue {
@@ -259,14 +259,10 @@ export const usePreservedTableState = (props?: UsePreservedTableStateProps) => {
         firstUpdate.current?.p || { pageSize: 15, pageIndex: 0 },
     }),
     // Need to also update when firstUpdate.current?.x changes, for some reason it claims its not used here when it is
+    // We also need to intentionally ignore props?.initialState?.x as these may not be in a memo, and are only set
+    // once initially anyway
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [
-      props?.initialState?.columnVisibility,
-      props?.initialState?.grouping,
-      props?.initialState?.pagination,
-      firstUpdate.current?.cO,
-      firstUpdate.current?.p,
-    ]
+    [firstUpdate.current?.cO, firstUpdate.current?.p]
   );
 
   // Convert the state stored into the url to one that can be used
@@ -315,6 +311,22 @@ export const usePreservedTableState = (props?: UsePreservedTableStateProps) => {
     []
   );
 
+  // This will return a reset pagination `p` state to the default page index. This is so that
+  // things like filtering reset the page index to avoid getting stuck on a non-existent page.
+  // This recreates similar behaviour to `autoResetPageIndex` but in a controlled way.
+  const getResetPaginationState = useCallback(
+    (prevP?: MRT_PaginationState) => {
+      const newPState: MRT_PaginationState = {
+        ...defaultState.p,
+        pageSize: prevP?.pageSize ?? defaultState.p.pageSize,
+      };
+      return JSON.stringify(newPState) === JSON.stringify(defaultState.p)
+        ? undefined
+        : newPState;
+    },
+    [defaultState.p]
+  );
+
   // Below are setters for MRT onChange events, these should obtain the value and update it in the
   // parsed search params using a value of undefined only when it is no longer needed in the url
   // (presumably because it is now the default value/no longer needed)
@@ -361,10 +373,11 @@ export const usePreservedTableState = (props?: UsePreservedTableStateProps) => {
         return {
           ...prevState,
           cF: isDefaultState ? undefined : newValue,
+          p: getResetPaginationState(prevState.p),
         };
       });
     },
-    [defaultState.cF, updateSearchParams]
+    [defaultState.cF, getResetPaginationState, updateSearchParams]
   );
 
   const setSorting = useCallback(
@@ -427,10 +440,11 @@ export const usePreservedTableState = (props?: UsePreservedTableStateProps) => {
         return {
           ...prevState,
           gFil: newValue === '' ? undefined : newValue,
+          p: getResetPaginationState(prevState.p),
         };
       });
     },
-    [defaultState.gFil, updateSearchParams]
+    [defaultState.gFil, getResetPaginationState, updateSearchParams]
   );
 
   const setGroupingState = useCallback(
@@ -446,10 +460,11 @@ export const usePreservedTableState = (props?: UsePreservedTableStateProps) => {
         return {
           ...prevState,
           g: isDefaultState ? undefined : newValue,
+          p: getResetPaginationState(prevState.p),
         };
       });
     },
-    [defaultState.g, updateSearchParams]
+    [defaultState.g, getResetPaginationState, updateSearchParams]
   );
 
   const setColumnOrder = useCallback(