Merge branch 'main' into develop

ral-facilities · Feb 2, 2024 · f1287e6 · f1287e6
2 parents 048af12 + 8c0b1b4
commit f1287e6
Show file tree

Hide file tree

Showing 13 changed files with 73 additions and 88 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Changelog
 
+## [v1.1.3](https://github.com/ral-facilities/datagateway/tree/v1.1.3) (2024-02-02)
+
+[Full Changelog](https://github.com/ral-facilities/datagateway/compare/v1.1.2...v1.1.3)
+
+**Fixed bugs:**
+
+- Fix performance of datafile table requests by extracting out investigation ID check to ID check functions [ffbb197](https://github.com/ral-facilities/datagateway/commit/ffbb197e333d93d35687252eaaba32fd475a5ffa) ([louise-davies](https://github.com/louise-davies))
+
 ## [v1.1.2](https://github.com/ral-facilities/datagateway/tree/v1.1.2) (2023-09-28)
 
 [Full Changelog](https://github.com/ral-facilities/datagateway/compare/v1.1.1...v1.1.2)

diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "datagateway",
   "private": true,
-  "version": "1.1.2",
+  "version": "1.1.3",
   "workspaces": [
     "packages/*"
   ],

diff --git a/packages/datagateway-common/package.json b/packages/datagateway-common/package.json
@@ -1,6 +1,6 @@
 {
   "name": "datagateway-common",
-  "version": "1.1.2",
+  "version": "1.1.3",
   "private": true,
   "files": [
     "lib"

diff --git a/packages/datagateway-dataview/package.json b/packages/datagateway-dataview/package.json
@@ -1,6 +1,6 @@
 {
   "name": "datagateway-dataview",
-  "version": "1.1.2",
+  "version": "1.1.3",
   "private": true,
   "dependencies": {
     "@craco/craco": "7.1.0",
@@ -22,7 +22,7 @@
     "axios": "1.6.1",
     "connected-react-router": "6.9.1",
     "custom-event-polyfill": "1.0.7",
-    "datagateway-common": "^1.1.2",
+    "datagateway-common": "^1.1.3",
     "date-fns": "2.30.0",
     "eslint-config-prettier": "8.10.0",
     "eslint-plugin-cypress": "2.15.1",

diff --git a/packages/datagateway-dataview/src/__mocks__/axios.ts b/packages/datagateway-dataview/src/__mocks__/axios.ts
@@ -23,6 +23,7 @@ const requests = {
   post: jest.fn(() => Promise.resolve({ data: {} })),
   delete: jest.fn(() => Promise.resolve({ data: {} })),
   CancelToken: axios.CancelToken,
+  isAxiosError: axios.isAxiosError,
 };
 
 export default requests;
diff --git a/packages/datagateway-dataview/src/page/idCheckFunctions.test.ts b/packages/datagateway-dataview/src/page/idCheckFunctions.test.ts
@@ -35,11 +35,18 @@ describe('ID check functions', () => {
     const store = configureStore()({});
 
     await checkInvestigationId(1, 2);
+    const params = new URLSearchParams();
+    params.append(
+      'where',
+      JSON.stringify({
+        id: {
+          eq: 2,
+        },
+      })
+    );
+    params.append('where', JSON.stringify({ 'investigation.id': { eq: 1 } }));
     expect(axios.get).toHaveBeenCalledWith('/datasets/findone', {
-      params: {
-        where: JSON.stringify({ id: { eq: 2 } }),
-        include: '"investigation"',
-      },
+      params,
       headers: { Authorization: 'Bearer null' },
     });
     (axios.get as jest.Mock).mockClear();
@@ -51,10 +58,7 @@ describe('ID check functions', () => {
 
     await checkInvestigationId(1, 2);
     expect(axios.get).toHaveBeenCalledWith('/test/datasets/findone', {
-      params: {
-        where: JSON.stringify({ id: { eq: 2 } }),
-        include: '"investigation"',
-      },
+      params,
       headers: { Authorization: 'Bearer null' },
     });
 
@@ -76,24 +80,33 @@ describe('ID check functions', () => {
 
       const result = await checkInvestigationId(1, 2);
       expect(result).toBe(true);
+      const params = new URLSearchParams();
+      params.append(
+        'where',
+        JSON.stringify({
+          id: {
+            eq: 2,
+          },
+        })
+      );
+      params.append('where', JSON.stringify({ 'investigation.id': { eq: 1 } }));
       expect(axios.get).toHaveBeenCalledWith('/datasets/findone', {
-        params: {
-          where: JSON.stringify({ id: { eq: 2 } }),
-          include: '"investigation"',
-        },
+        params,
         headers: { Authorization: 'Bearer null' },
       });
     });
     it('returns false on invalid investigation + dataset pair', async () => {
-      expect.assertions(1);
+      expect.assertions(2);
       (axios.get as jest.Mock).mockImplementation(() =>
-        Promise.resolve({
-          data: { id: 2, name: 'Test dataset', investigation: { id: 3 } },
+        Promise.reject({
+          response: { status: 404 },
+          isAxiosError: true,
         })
       );
 
       const result = await checkInvestigationId(1, 2);
       expect(result).toBe(false);
+      expect(handleICATError).not.toHaveBeenCalled();
     });
     it('returns false on HTTP error', async () => {
       expect.assertions(2);

diff --git a/packages/datagateway-dataview/src/page/idCheckFunctions.ts b/packages/datagateway-dataview/src/page/idCheckFunctions.ts
@@ -1,7 +1,6 @@
 import axios, { AxiosResponse } from 'axios';
 import {
   handleICATError,
-  Dataset,
   Investigation,
   ConfigureURLsType,
   readSciGatewayToken,
@@ -27,24 +26,34 @@ const unmemoizedCheckInvestigationId = (
   investigationId: number,
   datasetId: number
 ): Promise<boolean> => {
+  const params = new URLSearchParams();
+  params.append(
+    'where',
+    JSON.stringify({
+      id: {
+        eq: datasetId,
+      },
+    })
+  );
+  params.append(
+    'where',
+    JSON.stringify({ 'investigation.id': { eq: investigationId } })
+  );
   return axios
     .get(`${apiUrl}/datasets/findone`, {
-      params: {
-        where: JSON.stringify({
-          id: {
-            eq: datasetId,
-          },
-        }),
-        include: '"investigation"',
-      },
+      params,
       headers: {
         Authorization: `Bearer ${readSciGatewayToken().sessionId}`,
       },
     })
-    .then((response: AxiosResponse<Dataset>) => {
-      return response.data.investigation?.id === investigationId;
+    .then(() => {
+      return true;
     })
     .catch((error) => {
+      // 404 is valid response from API saying the investigation id is invalid
+      if (axios.isAxiosError(error) && error.response?.status === 404)
+        return false;
+      // handle other API errors
       handleICATError(error);
       return false;
     });

diff --git a/packages/datagateway-dataview/src/page/pageRouting.component.tsx b/packages/datagateway-dataview/src/page/pageRouting.component.tsx
@@ -110,6 +110,10 @@ const SafeISISDatafilesTable = React.memo(
               parseInt(props.instrumentChildId),
               parseInt(props.investigationId)
             ),
+            checkInvestigationId(
+              parseInt(props.investigationId),
+              parseInt(props.datasetId)
+            ),
           ]).then((values) => !values.includes(false))
         )(ISISDatafilesTable);
 
@@ -292,6 +296,10 @@ const SafeDLSDatafilesTable = React.memo(
     const SafeDLSDatafilesTable = withIdCheck(
       Promise.all([
         checkProposalName(props.proposalName, parseInt(props.investigationId)),
+        checkInvestigationId(
+          parseInt(props.investigationId),
+          parseInt(props.datasetId)
+        ),
       ]).then((values) => !values.includes(false))
     )(DLSDatafilesTable);
 

diff --git a/packages/datagateway-dataview/src/views/table/datafileTable.component.tsx b/packages/datagateway-dataview/src/views/table/datafileTable.component.tsx
@@ -56,12 +56,6 @@ const DatafileTable = (props: DatafileTableProps): React.ReactElement => {
         filterType: 'where',
         filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
       },
-      {
-        filterType: 'where',
-        filterValue: JSON.stringify({
-          'dataset.investigation.id': { eq: investigationId },
-        }),
-      },
     ],
     selectAllSetting
   );
@@ -76,25 +70,13 @@ const DatafileTable = (props: DatafileTableProps): React.ReactElement => {
       filterType: 'where',
       filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
     },
-    {
-      filterType: 'where',
-      filterValue: JSON.stringify({
-        'dataset.investigation.id': { eq: investigationId },
-      }),
-    },
   ]);
 
   const { fetchNextPage, data } = useDatafilesInfinite([
     {
       filterType: 'where',
       filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
     },
-    {
-      filterType: 'where',
-      filterValue: JSON.stringify({
-        'dataset.investigation.id': { eq: investigationId },
-      }),
-    },
   ]);
 
   const loadMoreRows = React.useCallback(

diff --git a/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.tsx b/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.tsx
@@ -57,12 +57,6 @@ const DLSDatafilesTable = (
         filterType: 'where',
         filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
       },
-      {
-        filterType: 'where',
-        filterValue: JSON.stringify({
-          'dataset.investigation.id': { eq: investigationId },
-        }),
-      },
     ],
     selectAllSetting
   );
@@ -77,12 +71,6 @@ const DLSDatafilesTable = (
       filterType: 'where',
       filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
     },
-    {
-      filterType: 'where',
-      filterValue: JSON.stringify({
-        'dataset.investigation.id': { eq: investigationId },
-      }),
-    },
   ]);
 
   // isMounted is used to disable queries when the component isn't fully mounted.
@@ -99,12 +87,6 @@ const DLSDatafilesTable = (
         filterType: 'where',
         filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
       },
-      {
-        filterType: 'where',
-        filterValue: JSON.stringify({
-          'dataset.investigation.id': { eq: investigationId },
-        }),
-      },
     ],
     isMounted
   );

diff --git a/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.tsx b/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.tsx
@@ -63,12 +63,6 @@ const ISISDatafilesTable = (
         filterType: 'where',
         filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
       },
-      {
-        filterType: 'where',
-        filterValue: JSON.stringify({
-          'dataset.investigation.id': { eq: investigationId },
-        }),
-      },
     ],
     selectAllSetting
   );
@@ -83,12 +77,6 @@ const ISISDatafilesTable = (
       filterType: 'where',
       filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
     },
-    {
-      filterType: 'where',
-      filterValue: JSON.stringify({
-        'dataset.investigation.id': { eq: investigationId },
-      }),
-    },
   ]);
 
   // isMounted is used to disable queries when the component isn't fully mounted.
@@ -105,12 +93,6 @@ const ISISDatafilesTable = (
         filterType: 'where',
         filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }),
       },
-      {
-        filterType: 'where',
-        filterValue: JSON.stringify({
-          'dataset.investigation.id': { eq: investigationId },
-        }),
-      },
     ],
     isMounted
   );

diff --git a/packages/datagateway-download/package.json b/packages/datagateway-download/package.json
@@ -1,6 +1,6 @@
 {
   "name": "datagateway-download",
-  "version": "1.1.2",
+  "version": "1.1.3",
   "private": true,
   "dependencies": {
     "@craco/craco": "7.1.0",
@@ -17,7 +17,7 @@
     "@types/react-router-dom": "5.3.3",
     "@types/react-virtualized": "9.21.10",
     "axios": "1.6.1",
-    "datagateway-common": "^1.1.2",
+    "datagateway-common": "^1.1.3",
     "date-fns": "2.30.0",
     "date-fns-tz": "2.0.0",
     "eslint-config-prettier": "8.10.0",

diff --git a/packages/datagateway-search/package.json b/packages/datagateway-search/package.json
@@ -1,6 +1,6 @@
 {
   "name": "datagateway-search",
-  "version": "1.1.2",
+  "version": "1.1.3",
   "private": true,
   "dependencies": {
     "@craco/craco": "7.1.0",
@@ -23,7 +23,7 @@
     "axios": "1.6.1",
     "connected-react-router": "6.9.1",
     "custom-event-polyfill": "1.0.7",
-    "datagateway-common": "^1.1.2",
+    "datagateway-common": "^1.1.3",
     "date-fns": "2.30.0",
     "eslint-config-prettier": "8.10.0",
     "eslint-plugin-cypress": "2.15.1",