diff --git a/CHANGELOG.md b/CHANGELOG.md index ed66c0e36..f0133d638 100644 --- 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/lerna.json b/lerna.json index d60e435fc..0575735e3 100644 --- a/lerna.json +++ b/lerna.json @@ -2,7 +2,7 @@ "packages": [ "packages/*" ], - "version": "1.1.2", + "version": "1.1.3", "npmClient": "yarn", "useWorkspaces": true } diff --git a/package.json b/package.json index 3028e8013..aa4a974f9 100644 --- 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 index b0256375e..901bb83ab 100644 --- 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 index 9d8cac9d4..f79c34917 100644 --- 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": { "@material-ui/core": "^4.11.3", @@ -14,7 +14,7 @@ "axios": "^0.26.0", "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.28.0", "history": "^4.10.1", "i18next": "^21.6.13", diff --git a/packages/datagateway-dataview/src/page/idCheckFunctions.test.ts b/packages/datagateway-dataview/src/page/idCheckFunctions.test.ts index c9ecf324a..42ea25157 100644 --- a/packages/datagateway-dataview/src/page/idCheckFunctions.test.ts +++ b/packages/datagateway-dataview/src/page/idCheckFunctions.test.ts @@ -34,11 +34,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: { id: { eq: 2 } }, - include: '"investigation"', - }, + params, headers: { Authorization: 'Bearer null' }, }); (axios.get as jest.Mock).mockClear(); @@ -50,10 +57,7 @@ describe('ID check functions', () => { await checkInvestigationId(1, 2); expect(axios.get).toHaveBeenCalledWith('/test/datasets/findone', { - params: { - where: { id: { eq: 2 } }, - include: '"investigation"', - }, + params, headers: { Authorization: 'Bearer null' }, }); @@ -75,24 +79,32 @@ 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: { 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 }, }) ); 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 index d5a9db00d..c6550e0a8 100644 --- 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 axios, { AxiosError, AxiosResponse } from 'axios'; import { handleICATError, - Dataset, Investigation, ConfigureURLsType, readSciGatewayToken, @@ -26,24 +25,33 @@ const unmemoizedCheckInvestigationId = ( investigationId: number, datasetId: number ): Promise => { + 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: { - id: { - eq: datasetId, - }, - }, - include: '"investigation"', - }, + params, headers: { Authorization: `Bearer ${readSciGatewayToken().sessionId}`, }, }) - .then((response: AxiosResponse) => { - return response.data.investigation?.id === investigationId; + .then(() => { + return true; }) .catch((error) => { + // 404 is valid response from API saying the investigation id is invalid + if ((error as AxiosError).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 index ef008beec..bcbb63c9a 100644 --- a/packages/datagateway-dataview/src/page/pageRouting.component.tsx +++ b/packages/datagateway-dataview/src/page/pageRouting.component.tsx @@ -61,12 +61,7 @@ const SafeDatafileTable = React.memo( parseInt(props.datasetId) ) )(DatafileTable); - return ( - - ); + return ; } ); SafeDatafileTable.displayName = 'SafeDatafileTable'; @@ -103,15 +98,14 @@ const SafeISISDatafilesTable = React.memo( parseInt(props.instrumentChildId), parseInt(props.investigationId) ), + checkInvestigationId( + parseInt(props.investigationId), + parseInt(props.datasetId) + ), ]).then((values) => !values.includes(false)) )(ISISDatafilesTable); - return ( - - ); + return ; } ); SafeISISDatafilesTable.displayName = 'SafeISISDatafilesTable'; @@ -279,15 +273,14 @@ 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); - return ( - - ); + return ; } ); SafeDLSDatafilesTable.displayName = 'SafeDLSDatafilesTable'; diff --git a/packages/datagateway-dataview/src/views/table/datafileTable.component.test.tsx b/packages/datagateway-dataview/src/views/table/datafileTable.component.test.tsx index c399cab66..dc546f655 100644 --- a/packages/datagateway-dataview/src/views/table/datafileTable.component.test.tsx +++ b/packages/datagateway-dataview/src/views/table/datafileTable.component.test.tsx @@ -51,7 +51,7 @@ describe('Datafile table component', () => { - + @@ -114,31 +114,18 @@ describe('Datafile table component', () => { it('calls the correct data fetching hooks on load', () => { const datasetId = '1'; - const investigationId = '2'; createWrapper(); expect(useDatafileCount).toHaveBeenCalledWith([ { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ]); expect(useDatafilesInfinite).toHaveBeenCalledWith([ { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ]); expect(useIds).toHaveBeenCalledWith( 'datafile', @@ -147,12 +134,6 @@ describe('Datafile table component', () => { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], true ); diff --git a/packages/datagateway-dataview/src/views/table/datafileTable.component.tsx b/packages/datagateway-dataview/src/views/table/datafileTable.component.tsx index e86176e67..644fba2ba 100644 --- a/packages/datagateway-dataview/src/views/table/datafileTable.component.tsx +++ b/packages/datagateway-dataview/src/views/table/datafileTable.component.tsx @@ -30,11 +30,10 @@ import { IndexRange } from 'react-virtualized'; interface DatafileTableProps { datasetId: string; - investigationId: string; } const DatafileTable = (props: DatafileTableProps): React.ReactElement => { - const { datasetId, investigationId } = props; + const { datasetId } = props; const [t] = useTranslation(); @@ -59,12 +58,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 ); @@ -82,12 +75,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 }, - }), - }, ]); const { fetchNextPage, data } = useDatafilesInfinite([ @@ -95,12 +82,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 }, - }), - }, ]); const loadMoreRows = React.useCallback( diff --git a/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.test.tsx b/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.test.tsx index 73ebee47d..78c7b943c 100644 --- a/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.test.tsx +++ b/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.test.tsx @@ -50,7 +50,7 @@ describe('DLS datafiles table component', () => { - + @@ -114,19 +114,12 @@ describe('DLS datafiles table component', () => { it('calls the correct data fetching hooks on load', () => { const datasetId = '1'; - const investigationId = '2'; createWrapper(); expect(useDatafileCount).toHaveBeenCalledWith([ { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ]); expect(useDatafilesInfinite).toHaveBeenCalledWith( [ @@ -134,12 +127,6 @@ describe('DLS datafiles table component', () => { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], expect.any(Boolean) ); @@ -150,12 +137,6 @@ describe('DLS datafiles table component', () => { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], true ); diff --git a/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.tsx b/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.tsx index ab541e321..c9a7c1b55 100644 --- a/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.tsx +++ b/packages/datagateway-dataview/src/views/table/dls/dlsDatafilesTable.component.tsx @@ -28,13 +28,12 @@ import { IndexRange } from 'react-virtualized'; interface DLSDatafilesTableProps { datasetId: string; - investigationId: string; } const DLSDatafilesTable = ( props: DLSDatafilesTableProps ): React.ReactElement => { - const { datasetId, investigationId } = props; + const { datasetId } = props; const [t] = useTranslation(); @@ -60,12 +59,6 @@ const DLSDatafilesTable = ( filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], selectAllSetting ); @@ -83,12 +76,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. @@ -105,12 +92,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.test.tsx b/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.test.tsx index 7efd7b9f8..7eabab819 100644 --- a/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.test.tsx +++ b/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.test.tsx @@ -50,7 +50,7 @@ describe('ISIS datafiles table component', () => { - + @@ -114,19 +114,12 @@ describe('ISIS datafiles table component', () => { it('calls the correct data fetching hooks on load', () => { const datasetId = '1'; - const investigationId = '2'; createWrapper(); expect(useDatafileCount).toHaveBeenCalledWith([ { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ]); expect(useDatafilesInfinite).toHaveBeenCalledWith( [ @@ -134,12 +127,6 @@ describe('ISIS datafiles table component', () => { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], expect.any(Boolean) ); @@ -150,12 +137,6 @@ describe('ISIS datafiles table component', () => { filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], true ); diff --git a/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.tsx b/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.tsx index 7c78899f1..38a10a528 100644 --- a/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.tsx +++ b/packages/datagateway-dataview/src/views/table/isis/isisDatafilesTable.component.tsx @@ -30,13 +30,12 @@ import { IndexRange } from 'react-virtualized'; interface ISISDatafilesTableProps { datasetId: string; - investigationId: string; } const ISISDatafilesTable = ( props: ISISDatafilesTableProps ): React.ReactElement => { - const { datasetId, investigationId } = props; + const { datasetId } = props; const [t] = useTranslation(); @@ -62,12 +61,6 @@ const ISISDatafilesTable = ( filterType: 'where', filterValue: JSON.stringify({ 'dataset.id': { eq: datasetId } }), }, - { - filterType: 'where', - filterValue: JSON.stringify({ - 'dataset.investigation.id': { eq: investigationId }, - }), - }, ], selectAllSetting ); @@ -85,12 +78,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. @@ -107,12 +94,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 index 2138e37c9..41b265733 100644 --- 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": { "@material-ui/core": "^4.11.3", @@ -11,7 +11,7 @@ "@types/react": "^17.0.2", "@types/react-dom": "^17.0.1", "axios": "^0.26.0", - "datagateway-common": "^1.1.2", + "datagateway-common": "^1.1.3", "date-fns": "^2.28.0", "date-fns-tz": "^1.1.6", "history": "^4.10.1", diff --git a/packages/datagateway-search/package.json b/packages/datagateway-search/package.json index c16cb3f59..81ea8f6a0 100644 --- 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": { "@date-io/date-fns": "^1.3.13", @@ -14,7 +14,7 @@ "axios": "^0.26.0", "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.28.0", "history": "^4.10.1", "i18next": "^21.6.13",