Use window.location in useUpdateQueryParam to avoid staleness issues

ral-facilities · Mar 10, 2023 · 3c36cc6 · 3c36cc6
1 parent 0f2f28d
commit 3c36cc6
Show file tree

Hide file tree

Showing 5 changed files with 3,726 additions and 3,280 deletions.
diff --git a/packages/datagateway-common/src/api/index.tsx b/packages/datagateway-common/src/api/index.tsx
@@ -365,12 +365,13 @@ export const useUpdateQueryParam = (
   type: 'filters' | 'sort' | 'page' | 'results',
   updateMethod: 'push' | 'replace'
 ): ((param: FiltersType | SortType | number | null) => void) => {
-  const location = useLocation();
   const { push, replace } = useHistory();
   const functionToUse = updateMethod === 'push' ? push : replace;
   return React.useCallback(
     (param: FiltersType | SortType | number | null) => {
-      const query = parseSearchToQuery(location.search);
+      // need to use window.location.search and not useLocation to ensure we have the most
+      // up to date version of search when useUpdateQueryParam is called
+      const query = parseSearchToQuery(window.location.search);
 
       if (type === 'filters') {
         query.filters = param as FiltersType;
@@ -384,7 +385,7 @@ export const useUpdateQueryParam = (
 
       functionToUse({ search: `?${parseQueryToSearch(query).toString()}` });
     },
-    [location.search, type, functionToUse]
+    [type, functionToUse]
   );
 };
 

diff --git a/packages/datagateway-dataview/src/page/pageContainer.component.test.tsx b/packages/datagateway-dataview/src/page/pageContainer.component.test.tsx
@@ -8,7 +8,12 @@ import {
   DownloadCartItem,
   readSciGatewayToken,
 } from 'datagateway-common';
-import { createLocation, createMemoryHistory, History } from 'history';
+import {
+  createLocation,
+  createMemoryHistory,
+  createPath,
+  History,
+} from 'history';
 import { Router } from 'react-router-dom';
 
 import PageContainer, { paths } from './pageContainer.component';
@@ -95,6 +100,32 @@ describe('PageContainer - Tests', () => {
     });
     user = userEvent.setup();
 
+    delete window.location;
+    window.location = new URL(`http://localhost/`);
+
+    // below code keeps window.location in sync with history changes
+    // (needed because useUpdateQueryParam uses window.location not history)
+    const historyReplace = history.replace;
+    const historyReplaceSpy = jest.spyOn(history, 'replace');
+    historyReplaceSpy.mockImplementation((args) => {
+      historyReplace(args);
+      if (typeof args === 'string') {
+        window.location = new URL(`http://localhost${args}`);
+      } else {
+        window.location = new URL(`http://localhost${createPath(args)}`);
+      }
+    });
+    const historyPush = history.push;
+    const historyPushSpy = jest.spyOn(history, 'push');
+    historyPushSpy.mockImplementation((args) => {
+      historyPush(args);
+      if (typeof args === 'string') {
+        window.location = new URL(`http://localhost${args}`);
+      } else {
+        window.location = new URL(`http://localhost${createPath(args)}`);
+      }
+    });
+
     holder = document.createElement('div');
     holder.setAttribute('id', 'datagateway-search');
     document.body.appendChild(holder);

diff --git a/packages/datagateway-dataview/src/views/roleSelector.component.tsx b/packages/datagateway-dataview/src/views/roleSelector.component.tsx
@@ -101,7 +101,6 @@ const RoleSelector: React.FC = () => {
         {t('my_data_table.role_selector')}
       </InputLabel>
       <Select
-        id="role-selector"
         labelId="my-data-table-role-selector-label"
         value={roles?.includes(role) ? role : ''}
         onChange={handleChange}

diff --git a/packages/datagateway-search/src/searchPageContainer.component.test.tsx b/packages/datagateway-search/src/searchPageContainer.component.test.tsx
@@ -8,7 +8,7 @@ import {
   dGCommonInitialState,
   type DownloadCartItem,
 } from 'datagateway-common';
-import { createMemoryHistory, type History } from 'history';
+import { createMemoryHistory, createPath, type History } from 'history';
 import { Router } from 'react-router-dom';
 import SearchPageContainer, {
   getFilters,
@@ -71,6 +71,31 @@ describe('SearchPageContainer - Tests', () => {
     history = createMemoryHistory({
       initialEntries: ['/search/data'],
     });
+    delete window.location;
+    window.location = new URL(`http://localhost/search/data`);
+
+    // below code keeps window.location in sync with history changes
+    // (needed because useUpdateQueryParam uses window.location not history)
+    const historyReplace = history.replace;
+    const historyReplaceSpy = jest.spyOn(history, 'replace');
+    historyReplaceSpy.mockImplementation((args) => {
+      historyReplace(args);
+      if (typeof args === 'string') {
+        window.location = new URL(`http://localhost${args}`);
+      } else {
+        window.location = new URL(`http://localhost${createPath(args)}`);
+      }
+    });
+    const historyPush = history.push;
+    const historyPushSpy = jest.spyOn(history, 'push');
+    historyPushSpy.mockImplementation((args) => {
+      historyPush(args);
+      if (typeof args === 'string') {
+        window.location = new URL(`http://localhost${args}`);
+      } else {
+        window.location = new URL(`http://localhost${createPath(args)}`);
+      }
+    });
 
     window.localStorage.__proto__.removeItem = jest.fn();
     window.localStorage.__proto__.setItem = jest.fn();