From 225b5b63221020247a43ee8b67bee0a3d0d49a82 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 26 Mar 2022 01:50:06 +0000 Subject: [PATCH 01/62] Bump node-forge from 1.2.1 to 1.3.0 Bumps [node-forge](https://github.com/digitalbazaar/forge) from 1.2.1 to 1.3.0. - [Release notes](https://github.com/digitalbazaar/forge/releases) - [Changelog](https://github.com/digitalbazaar/forge/blob/main/CHANGELOG.md) - [Commits](https://github.com/digitalbazaar/forge/compare/v1.2.1...v1.3.0) --- updated-dependencies: - dependency-name: node-forge dependency-type: indirect ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index f2ed1428b..6f66da63c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10829,9 +10829,9 @@ node-fetch@2.6.7, node-fetch@^2.6.1: whatwg-url "^5.0.0" node-forge@^1.2.0: - version "1.2.1" - resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-1.2.1.tgz#82794919071ef2eb5c509293325cec8afd0fd53c" - integrity sha512-Fcvtbb+zBcZXbTTVwqGA5W+MKBj56UjVRevvchv5XrcyXbmNdesfZL37nlcWOfpgHhgmxApw3tQbTr4CqNmX4w== + version "1.3.0" + resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-1.3.0.tgz#37a874ea723855f37db091e6c186e5b67a01d4b2" + integrity sha512-08ARB91bUi6zNKzVmaj3QO7cr397uiDT2nJ63cHjyNtCTWIgvS47j3eT0WfzUwS9+6Z5YshRaoasFkXCKrIYbA== node-gyp@^5.0.2: version "5.0.5" From 690fbf029c24dbffe3ff35b316b7001f2f356fa4 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Mon, 28 Mar 2022 17:38:30 +0100 Subject: [PATCH 02/62] WIP: Refactor downloadCartTable to use React-Query to optimise it - think I mostly have everything working but no tests --- packages/datagateway-common/src/api/cart.tsx | 4 +- packages/datagateway-dataview/package.json | 1 - packages/datagateway-download/src/App.tsx | 46 +- .../datagateway-download/src/downloadApi.ts | 453 ++++++++++++------ .../downloadCartTable.component.tsx | 386 +++++++-------- 5 files changed, 513 insertions(+), 377 deletions(-) diff --git a/packages/datagateway-common/src/api/cart.tsx b/packages/datagateway-common/src/api/cart.tsx index f2c6a8eb0..040d4555c 100644 --- a/packages/datagateway-common/src/api/cart.tsx +++ b/packages/datagateway-common/src/api/cart.tsx @@ -12,7 +12,7 @@ import { UseMutationResult, } from 'react-query'; -const fetchDownloadCart = (config: { +export const fetchDownloadCart = (config: { facilityName: string; downloadApiUrl: string; }): Promise => { @@ -45,7 +45,7 @@ const addToCart = ( .then((response) => response.data.cartItems); }; -const removeFromCart = ( +export const removeFromCart = ( entityType: 'investigation' | 'dataset' | 'datafile', entityIds: number[], config: { facilityName: string; downloadApiUrl: string } diff --git a/packages/datagateway-dataview/package.json b/packages/datagateway-dataview/package.json index 962d5da78..af5ffa1be 100644 --- a/packages/datagateway-dataview/package.json +++ b/packages/datagateway-dataview/package.json @@ -23,7 +23,6 @@ "lodash.debounce": "^4.0.8", "lodash.memoize": "^4.1.2", "loglevel": "^1.8.0", - "p-limit": "^4.0.0", "react": "^16.13.1", "react-app-polyfill": "^3.0.0", "react-dom": "^16.11.0", diff --git a/packages/datagateway-download/src/App.tsx b/packages/datagateway-download/src/App.tsx index e852a94d0..1058ddbe3 100644 --- a/packages/datagateway-download/src/App.tsx +++ b/packages/datagateway-download/src/App.tsx @@ -16,6 +16,8 @@ import { } from 'datagateway-common'; import { DGThemeProvider } from 'datagateway-common'; import AdminDownloadStatusTable from './downloadStatus/adminDownloadStatusTable.component'; +import { QueryClientProvider, QueryClient } from 'react-query'; +import { ReactQueryDevtools } from 'react-query/devtools'; const generateClassName = createGenerateClassName({ productionPrefix: 'dgwd', @@ -26,6 +28,15 @@ const generateClassName = createGenerateClassName({ process.env.NODE_ENV === 'production' && !process.env.REACT_APP_E2E_TESTING, }); +const queryClient = new QueryClient({ + defaultOptions: { + queries: { + refetchOnWindowFocus: true, + staleTime: 300000, + }, + }, +}); + class App extends Component { public constructor(props: unknown) { super(props); @@ -84,22 +95,25 @@ class App extends Component { - Finished loading - } - > - - - - - - - - - - - + + Finished loading + } + > + + + + + + + + + + + + + diff --git a/packages/datagateway-download/src/downloadApi.ts b/packages/datagateway-download/src/downloadApi.ts index 6a110b13b..ddbf0e6fe 100644 --- a/packages/datagateway-download/src/downloadApi.ts +++ b/packages/datagateway-download/src/downloadApi.ts @@ -1,92 +1,126 @@ -import axios, { AxiosResponse } from 'axios'; +import React from 'react'; +import axios, { AxiosError, AxiosResponse } from 'axios'; import * as log from 'loglevel'; import { - DownloadCart, SubmitCart, DownloadCartItem, Datafile, Download, readSciGatewayToken, handleICATError, + fetchDownloadCart, + removeFromCart, + DownloadCartTableItem, } from 'datagateway-common'; +import { DownloadSettingsContext } from './ConfigProvider'; +import { + UseQueryResult, + useQuery, + useMutation, + UseMutationResult, + useQueryClient, + UseQueryOptions, + useQueries, +} from 'react-query'; +import pLimit from 'p-limit'; +import useDeepCompareEffect from 'use-deep-compare-effect'; -export const fetchDownloadCartItems: (settings: { - facilityName: string; - downloadApiUrl: string; -}) => Promise = (settings: { - facilityName: string; - downloadApiUrl: string; -}) => { - return axios - .get( - `${settings.downloadApiUrl}/user/cart/${settings.facilityName}`, - { - params: { - sessionId: readSciGatewayToken().sessionId, - }, - } - ) - .then((response) => { - return response.data.cartItems; - }) - .catch((error) => { - handleICATError(error); - return []; - }); +export const useCart = (): UseQueryResult< + DownloadCartTableItem[], + AxiosError +> => { + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + return useQuery( + 'cart', + () => + fetchDownloadCart({ + facilityName, + downloadApiUrl, + }), + { + onError: (error) => { + handleICATError(error); + }, + select: (cart): DownloadCartTableItem[] => { + return cart.map((cartItem) => ({ + ...cartItem, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + size: cartItem.size ?? -1, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + fileCount: cartItem.fileCount ?? -1, + })); + }, + staleTime: 0, + } + ); }; export const removeAllDownloadCartItems: (settings: { facilityName: string; downloadApiUrl: string; -}) => Promise = (settings: { +}) => Promise = (settings: { facilityName: string; downloadApiUrl: string; }) => { - return axios - .delete( - `${settings.downloadApiUrl}/user/cart/${settings.facilityName}/cartItems`, - { - params: { - sessionId: readSciGatewayToken().sessionId, - items: '*', - }, - } - ) - .then(() => { - // do nothing - }) - .catch(handleICATError); + return axios.delete( + `${settings.downloadApiUrl}/user/cart/${settings.facilityName}/cartItems`, + { + params: { + sessionId: readSciGatewayToken().sessionId, + items: '*', + }, + } + ); }; -export const removeDownloadCartItem: ( - entityId: number, - entityType: string, - settings: { - facilityName: string; - downloadApiUrl: string; - } -) => Promise = ( - entityId: number, - entityType: string, - settings: { - facilityName: string; - downloadApiUrl: string; - } -) => { - return axios - .delete( - `${settings.downloadApiUrl}/user/cart/${settings.facilityName}/cartItems`, - { - params: { - sessionId: readSciGatewayToken().sessionId, - items: `${entityType} ${entityId}`, - }, - } - ) - .then(() => { - // do nothing - }) - .catch(handleICATError); +export const useRemoveAllFromCart = (): UseMutationResult< + DownloadCartItem[], + AxiosError +> => { + const queryClient = useQueryClient(); + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + + return useMutation( + () => removeAllDownloadCartItems({ facilityName, downloadApiUrl }), + { + onSuccess: (data) => { + queryClient.setQueryData('cart', []); + }, + onError: (error) => { + handleICATError(error); + }, + } + ); +}; + +export const useRemoveEntityFromCart = (): UseMutationResult< + DownloadCartItem[], + AxiosError, + { entityId: number; entityType: 'investigation' | 'dataset' | 'datafile' } +> => { + const queryClient = useQueryClient(); + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + + return useMutation( + ({ entityId, entityType }) => + removeFromCart(entityType, [entityId], { + facilityName, + downloadApiUrl, + }), + { + onSuccess: (data) => { + queryClient.setQueryData('cart', data); + }, + onError: (error) => { + handleICATError(error); + }, + } + ); }; export const getIsTwoLevel: (settings: { @@ -96,13 +130,20 @@ export const getIsTwoLevel: (settings: { .get(`${settings.idsUrl}/isTwoLevel`) .then((response) => { return response.data; - }) - .catch((error) => { - handleICATError(error, false); - return false; }); }; +export const useIsTwoLevel = (): UseQueryResult => { + const settings = React.useContext(DownloadSettingsContext); + const { idsUrl } = settings; + return useQuery('isTwoLevel', () => getIsTwoLevel({ idsUrl }), { + onError: (error) => { + handleICATError(error); + }, + staleTime: Infinity, + }); +}; + export const submitCart: ( transport: string, emailAddress: string, @@ -144,13 +185,46 @@ export const submitCart: ( // Get the downloadId that was returned from the IDS server. const downloadId = response.data['downloadId']; return downloadId; - }) - .catch((error) => { - handleICATError(error); - return -1; }); }; +export const useSubmitCart = (): UseMutationResult< + number, + AxiosError, + { + transport: string; + emailAddress: string; + fileName: string; + zipType?: 'ZIP' | 'ZIP_AND_COMPRESS'; + } +> => { + const queryClient = useQueryClient(); + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + + return useMutation( + ({ transport, emailAddress, fileName, zipType }) => + submitCart( + transport, + emailAddress, + fileName, + { + facilityName, + downloadApiUrl, + }, + zipType + ), + { + onSuccess: (data) => { + queryClient.setQueryData('cart', data); + }, + onError: (error) => { + handleICATError(error); + }, + } + ); +}; + export const fetchDownloads: ( settings: { facilityName: string; downloadApiUrl: string }, queryOffset?: string @@ -412,10 +486,6 @@ export const getSize: ( .then((response) => { const size = response.data['fileSize'] as number; return size; - }) - .catch((error) => { - handleICATError(error, false); - return -1; }); } else { return axios @@ -429,14 +499,88 @@ export const getSize: ( }) .then((response) => { return response.data; - }) - .catch((error) => { - handleICATError(error, false); - return -1; }); } }; +const sizesLimit = pLimit(10); + +export const useSizes = ( + data: DownloadCartItem[] | undefined +): UseQueryResult[] => { + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, apiUrl, downloadApiUrl } = settings; + + const queryConfigs: UseQueryOptions< + number, + AxiosError, + number, + ['size', number] + >[] = React.useMemo(() => { + return data + ? data.map((cartItem) => { + const { entityId, entityType } = cartItem; + return { + queryKey: ['size', entityId], + queryFn: () => + sizesLimit(() => + getSize(entityId, entityType, { + facilityName, + apiUrl, + downloadApiUrl, + }) + ), + onError: (error) => { + handleICATError(error, false); + }, + staleTime: Infinity, + }; + }) + : []; + }, [data, facilityName, apiUrl, downloadApiUrl]); + + // useQueries doesn't allow us to specify type info, so ignore this line + // since we strongly type the queries object anyway + // we also need to prettier-ignore to make sure we don't wrap onto next line + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // prettier-ignore + const queries: UseQueryResult[] = useQueries(queryConfigs); + + const [sizes, setSizes] = React.useState< + UseQueryResult[] + >([]); + + const countAppliedRef = React.useRef(0); + + // when data changes (i.e. due to sorting or filtering) set the countAppliedRef + // back to 0 so we can restart the process, as well as clear sizes + React.useEffect(() => { + countAppliedRef.current = 0; + setSizes([]); + }, [data]); + + // need to use useDeepCompareEffect here because the array returned by useQueries + // is different every time this hook runs + useDeepCompareEffect(() => { + const currCountReturned = queries.reduce( + (acc, curr) => acc + (curr.isFetched ? 1 : 0), + 0 + ); + const batchMax = + sizes.length - currCountReturned < 10 + ? sizes.length - currCountReturned + : 10; + // this in effect batches our updates to only happen in batches >= 10 + if (currCountReturned - countAppliedRef.current >= batchMax) { + setSizes(queries); + countAppliedRef.current = currCountReturned; + } + }, [sizes, queries]); + + return sizes; +}; + export const getDatafileCount: ( entityId: number, entityType: string, @@ -447,7 +591,12 @@ export const getDatafileCount: ( settings: { apiUrl: string } ) => { if (entityType === 'datafile') { - return Promise.resolve(1); + // need to do this in a setTimeout to ensure it doesn't block the main thread + return new Promise((resolve) => + window.setTimeout(() => { + resolve(1); + }, 0) + ); } else if (entityType === 'dataset') { return axios .get(`${settings.apiUrl}/datafiles/count`, { @@ -465,10 +614,6 @@ export const getDatafileCount: ( }) .then((response) => { return response.data; - }) - .catch((error) => { - handleICATError(error, false); - return -1; }); } else { return axios @@ -487,72 +632,84 @@ export const getDatafileCount: ( }) .then((response) => { return response.data; - }) - .catch((error) => { - handleICATError(error, false); - return -1; }); } }; -export const getCartDatafileCount: ( - cartItems: DownloadCartItem[], - settings: { apiUrl: string } -) => Promise = ( - cartItems: DownloadCartItem[], - settings: { apiUrl: string } -) => { - const getDatafileCountPromises: Promise[] = []; - cartItems.forEach((cartItem) => - getDatafileCountPromises.push( - getDatafileCount(cartItem.entityId, cartItem.entityType, { - apiUrl: settings.apiUrl, - }) - ) - ); +const datafileCountslimit = pLimit(10); - return Promise.all(getDatafileCountPromises).then((counts) => - counts.reduce( - (accumulator, nextCount) => - nextCount > -1 ? accumulator + nextCount : accumulator, - 0 - ) - ); -}; +export const useDatafileCounts = ( + data: DownloadCartItem[] | undefined +): UseQueryResult[] => { + const settings = React.useContext(DownloadSettingsContext); + const { apiUrl } = settings; -export const getCartSize: ( - cartItems: DownloadCartItem[], - settings: { - facilityName: string; - apiUrl: string; - downloadApiUrl: string; - } -) => Promise = ( - cartItems: DownloadCartItem[], - settings: { - facilityName: string; - apiUrl: string; - downloadApiUrl: string; - } -) => { - const getSizePromises: Promise[] = []; - cartItems.forEach((cartItem) => - getSizePromises.push( - getSize(cartItem.entityId, cartItem.entityType, { - facilityName: settings.facilityName, - apiUrl: settings.apiUrl, - downloadApiUrl: settings.downloadApiUrl, - }) - ) - ); + const queryConfigs: UseQueryOptions< + number, + AxiosError, + number, + ['datafileCount', number] + >[] = React.useMemo(() => { + return data + ? data.map((cartItem) => { + const { entityId, entityType } = cartItem; + return { + queryKey: ['datafileCount', entityId], + queryFn: () => + datafileCountslimit(() => + getDatafileCount(entityId, entityType, { + apiUrl, + }) + ), + onError: (error) => { + handleICATError(error, false); + }, + staleTime: Infinity, + }; + }) + : []; + }, [data, apiUrl]); + + // useQueries doesn't allow us to specify type info, so ignore this line + // since we strongly type the queries object anyway + // we also need to prettier-ignore to make sure we don't wrap onto next line + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // prettier-ignore + const queries: UseQueryResult[] = useQueries(queryConfigs); + + const [datafileCounts, setDatafileCounts] = React.useState< + UseQueryResult[] + >([]); + + const countAppliedRef = React.useRef(0); - return Promise.all(getSizePromises).then((sizes) => - sizes.reduce( - (accumulator, nextSize) => - nextSize > -1 ? accumulator + nextSize : accumulator, + // when data changes (i.e. due to sorting or filtering) set the countAppliedRef + // back to 0 so we can restart the process, as well as clear datafileCounts + React.useEffect(() => { + countAppliedRef.current = 0; + setDatafileCounts([]); + }, [data]); + + // need to use useDeepCompareEffect here because the array returned by useQueries + // is different every time this hook runs + useDeepCompareEffect(() => { + const currCountReturned = queries.reduce( + (acc, curr) => acc + (curr.isFetched ? 1 : 0), 0 - ) - ); + ); + const batchMax = + datafileCounts.length - currCountReturned < 10 + ? datafileCounts.length - currCountReturned + : 10; + // this in effect batches our updates to only happen in batches >= 10 + if (currCountReturned - countAppliedRef.current >= batchMax) { + setDatafileCounts(queries); + countAppliedRef.current = currCountReturned; + } + }, [datafileCounts, queries]); + + return datafileCounts; }; export const getDataUrl = ( diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx index b3fcd77a4..4c316beba 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx @@ -8,6 +8,7 @@ import { DownloadCartItem, DownloadCartTableItem, TextFilter, + ColumnType, } from 'datagateway-common'; import { IconButton, @@ -16,6 +17,7 @@ import { Typography, Button, LinearProgress, + CircularProgress, createStyles, makeStyles, Theme, @@ -23,19 +25,19 @@ import { } from '@material-ui/core'; import { RemoveCircle } from '@material-ui/icons'; import { - fetchDownloadCartItems, - removeAllDownloadCartItems, - removeDownloadCartItem, - getSize, - getIsTwoLevel, - getDatafileCount, + useCart, + useRemoveEntityFromCart, + useIsTwoLevel, + useRemoveAllFromCart, + useSizes, + useDatafileCounts, } from '../downloadApi'; -import chunk from 'lodash.chunk'; import DownloadConfirmDialog from '../downloadConfirmation/downloadConfirmDialog.component'; import { DownloadSettingsContext } from '../ConfigProvider'; import { Trans, useTranslation } from 'react-i18next'; import { Link as RouterLink } from 'react-router-dom'; +import { useQueryClient } from 'react-query'; const useStyles = makeStyles((theme: Theme) => createStyles({ @@ -62,150 +64,87 @@ const DownloadCartTable: React.FC = ( const [filters, setFilters] = React.useState<{ [column: string]: { value?: string | number; type: string }; }>({}); - const [data, setData] = React.useState([]); - const [dataLoaded, setDataLoaded] = React.useState(false); - const [sizesLoaded, setSizesLoaded] = React.useState(true); - const [sizesFinished, setSizesFinished] = React.useState(true); const fileCountMax = settings.fileCountMax; const totalSizeMax = settings.totalSizeMax; const [showConfirmation, setShowConfirmation] = React.useState(false); - const [isTwoLevel, setIsTwoLevel] = React.useState(false); - const [t] = useTranslation(); - const dgDownloadElement = document.getElementById('datagateway-download'); + const { data: isTwoLevel } = useIsTwoLevel(); + const { mutateAsync: removeDownloadCartItem } = useRemoveEntityFromCart(); + const { + mutate: removeAllDownloadCartItems, + isLoading: removeAllPending, + } = useRemoveAllFromCart(); + const { data, isFetching: dataLoading } = useCart(); - const totalSize = React.useMemo(() => { - if (sizesFinished) { - return data.reduce((accumulator, nextItem) => { - if (nextItem.size > -1) { - return accumulator + nextItem.size; + const queryClient = useQueryClient(); + const setData = React.useCallback( + (newData: DownloadCartTableItem[]) => { + queryClient.setQueryData('cart', newData); + }, + [queryClient] + ); + + const fileCountQueries = useDatafileCounts(data); + const sizeQueries = useSizes(data); + + const fileCount = React.useMemo(() => { + return ( + fileCountQueries?.reduce((accumulator, nextItem) => { + if (nextItem.data && nextItem.data > -1) { + return accumulator + nextItem.data; } else { return accumulator; } - }, 0); - } else { - return -1; - } - }, [data, sizesFinished]); + }, 0) ?? -1 + ); + }, [fileCountQueries]); - const fileCount = React.useMemo(() => { - if (sizesFinished) { - return data.reduce((accumulator, nextItem) => { - if (nextItem.fileCount > -1) { - return accumulator + nextItem.fileCount; + const totalSize = React.useMemo(() => { + return ( + sizeQueries?.reduce((accumulator, nextItem) => { + if (nextItem.data && nextItem.data > -1) { + return accumulator + nextItem.data; } else { return accumulator; } - }, 0); - } else { - return -1; - } - }, [data, sizesFinished]); + }, 0) ?? -1 + ); + }, [sizeQueries]); - React.useEffect(() => { - const checkTwoLevel = async (): Promise => - setIsTwoLevel(await getIsTwoLevel({ idsUrl: settings.idsUrl })); + const sizesLoading = sizeQueries.some((query) => query.isLoading); + const fileCountsLoading = fileCountQueries.some((query) => query.isLoading); - if (settings.idsUrl) checkTwoLevel(); - }, [settings.idsUrl]); - React.useEffect(() => { - if ( - settings.facilityName && - settings.apiUrl && - settings.downloadApiUrl && - dgDownloadElement - ) - fetchDownloadCartItems({ - facilityName: settings.facilityName, - downloadApiUrl: settings.downloadApiUrl, - }).then((cartItems) => { - setData( - cartItems.map((cartItem) => ({ - ...cartItem, - size: -1, - fileCount: -1, - })) - ); - setDataLoaded(true); - setSizesLoaded(false); - setSizesFinished(false); - }); - }, [ - settings.facilityName, - settings.apiUrl, - settings.downloadApiUrl, - dgDownloadElement, - ]); - - React.useEffect(() => { - if (!sizesLoaded) { - const chunkSize = 10; - const chunkedData = chunk(data, chunkSize); - const allPromises: Promise[] = []; - chunkedData.forEach((chunk, chunkIndex) => { - const updatedData = [...data]; - const chunkPromises: Promise[] = []; - - const chunkIndexOffset = chunkIndex * chunkSize; - chunk.forEach((cartItem, index) => { - const promiseSize = getSize(cartItem.entityId, cartItem.entityType, { - facilityName: settings.facilityName, - apiUrl: settings.apiUrl, - downloadApiUrl: settings.downloadApiUrl, - }).then((size) => { - updatedData[chunkIndexOffset + index].size = size; - }); - const promiseFileCount = getDatafileCount( - cartItem.entityId, - cartItem.entityType, - { - apiUrl: settings.apiUrl, - } - ).then((fileCount) => { - updatedData[chunkIndexOffset + index].fileCount = fileCount; - }); - chunkPromises.push(promiseSize); - allPromises.push(promiseSize); - chunkPromises.push(promiseFileCount); - allPromises.push(promiseFileCount); - }); - - Promise.all(chunkPromises).then(() => { - setData(updatedData); - }); - }); - Promise.all(allPromises).then(() => { - setSizesFinished(true); - }); - setSizesLoaded(true); - } - }, [ - data, - sizesLoaded, - settings.facilityName, - settings.apiUrl, - settings.downloadApiUrl, - ]); + const [t] = useTranslation(); - const textFilter = (label: string, dataKey: string): React.ReactElement => ( - { - if (value) { - setFilters({ ...filters, [dataKey]: value }); - } else { - const { [dataKey]: value, ...restOfFilters } = filters; - setFilters(restOfFilters); - } - }} - value={filters[dataKey] as TextFilter} - /> + const textFilter = React.useCallback( + (label: string, dataKey: string): React.ReactElement => ( + { + if (value) { + setFilters({ ...filters, [dataKey]: value }); + } else { + const { [dataKey]: value, ...restOfFilters } = filters; + setFilters(restOfFilters); + } + }} + value={filters[dataKey] as TextFilter} + /> + ), + [filters] ); const sortedAndFilteredData = React.useMemo(() => { - const filteredData = data.filter((item) => { + const sizeAddedData = data?.map( + (item, index) => + ({ + ...item, + size: sizeQueries?.[index]?.data ?? -1, + } as DownloadCartTableItem) + ); + const filteredData = sizeAddedData?.filter((item) => { for (const [key, value] of Object.entries(filters)) { const tableValue = item[key]; if ( @@ -244,10 +183,81 @@ const DownloadCartTable: React.FC = ( return 0; } - return filteredData.sort(sortCartItems); - }, [data, sort, filters]); + return filteredData?.sort(sortCartItems); + }, [data, sort, filters, sizeQueries]); + + const columns: ColumnType[] = React.useMemo( + () => [ + { + label: t('downloadCart.name'), + dataKey: 'name', + filterComponent: textFilter, + }, + { + label: t('downloadCart.type'), + dataKey: 'entityType', + filterComponent: textFilter, + }, + { + label: t('downloadCart.size'), + dataKey: 'size', + cellContentRenderer: (props) => { + return formatBytes(props.cellData); + }, + // cellContentRenderer: (cellProps) => + // formatCountOrSize(sizeQueries[cellProps.rowIndex], true), + }, + ], + [t, textFilter] + ); + const onSort = React.useCallback( + (column: string, order: 'desc' | 'asc' | null) => { + if (order) { + setSort({ ...sort, [column]: order }); + } else { + const { [column]: order, ...restOfSort } = sort; + setSort(restOfSort); + } + }, + [sort] + ); + const actions = React.useMemo( + () => [ + function RemoveButton({ rowData }: TableActionProps) { + const cartItem = rowData as DownloadCartItem; + const { entityId, entityType } = cartItem; + const [isDeleting, setIsDeleting] = React.useState(false); + return ( + { + setIsDeleting(true); + removeDownloadCartItem({ + entityId, + entityType, + }).then(() => { + setIsDeleting(false); + }); + }} + > + + + ); + }, + ], + [removeDownloadCartItem, t] + ); - return data.length === 0 ? ( + return !dataLoading && data?.length === 0 ? (
= ( > search {' '} - for data. + for data?. @@ -291,7 +301,7 @@ const DownloadCartTable: React.FC = (
{/* Show loading progress if data is still being loaded */} - {!dataLoaded && ( + {dataLoading && ( @@ -310,74 +320,12 @@ const DownloadCartTable: React.FC = ( }} > { - return formatBytes(props.cellData); - }, - }, - ]} + columns={columns} sort={sort} - onSort={(column: string, order: 'desc' | 'asc' | null) => { - if (order) { - setSort({ ...sort, [column]: order }); - } else { - const { [column]: order, ...restOfSort } = sort; - setSort(restOfSort); - } - }} - data={sortedAndFilteredData} - loading={!dataLoaded} - actions={[ - function RemoveButton({ rowData }: TableActionProps) { - const cartItem = rowData as DownloadCartItem; - const [isDeleting, setIsDeleting] = React.useState(false); - return ( - { - setIsDeleting(true); - removeDownloadCartItem( - cartItem.entityId, - cartItem.entityType, - { - facilityName: settings.facilityName, - downloadApiUrl: settings.downloadApiUrl, - } - ).then(() => { - setData( - data.filter( - (item) => item.entityId !== cartItem.entityId - ) - ); - }); - }} - > - - - ); - }, - ]} + onSort={onSort} + data={sortedAndFilteredData ?? []} + loading={dataLoading} + actions={actions} /> @@ -391,17 +339,32 @@ const DownloadCartTable: React.FC = ( - + {fileCountsLoading && ( + + )} + {t('downloadCart.number_of_files')}:{' '} {fileCount !== -1 ? fileCount : 'Calculating...'} {fileCountMax !== -1 && ` / ${fileCountMax}`} - + + + {sizesLoading && ( + + )} + {t('downloadCart.total_size')}:{' '} {totalSize !== -1 ? formatBytes(totalSize) : 'Calculating...'} {totalSizeMax !== -1 && ` / ${formatBytes(totalSizeMax)}`} @@ -421,11 +384,12 @@ const DownloadCartTable: React.FC = ( id="removeAllButton" variant="contained" color="primary" + disabled={removeAllPending} onClick={() => removeAllDownloadCartItems({ facilityName: settings.facilityName, downloadApiUrl: settings.downloadApiUrl, - }).then(() => setData([])) + }) } > {t('downloadCart.remove_all')} @@ -441,6 +405,8 @@ const DownloadCartTable: React.FC = ( disabled={ fileCount <= 0 || totalSize <= 0 || + sizesLoading || + fileCountsLoading || (fileCountMax !== -1 && fileCount > fileCountMax) || (totalSizeMax !== -1 && totalSize > totalSizeMax) } @@ -456,7 +422,7 @@ const DownloadCartTable: React.FC = ( setShowConfirmation(false)} From 49d6efa77ea8ceb00107014fe62bd1a04efcc1ee Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Wed, 30 Mar 2022 13:39:55 +0100 Subject: [PATCH 03/62] Only disable download button when counts haven't loaded when there's limits set --- .../downloadCart/downloadCartTable.component.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx index 912412b1d..0fd29d5d9 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx @@ -204,8 +204,6 @@ const DownloadCartTable: React.FC = ( cellContentRenderer: (props) => { return formatBytes(props.cellData); }, - // cellContentRenderer: (cellProps) => - // formatCountOrSize(sizeQueries[cellProps.rowIndex], true), }, ], [t, textFilter] @@ -405,12 +403,14 @@ const DownloadCartTable: React.FC = ( variant="contained" color="primary" disabled={ - fileCount <= 0 || - totalSize <= 0 || - sizesLoading || - fileCountsLoading || - (fileCountMax !== -1 && fileCount > fileCountMax) || - (totalSizeMax !== -1 && totalSize > totalSizeMax) + (fileCountMax !== -1 && + (fileCount <= 0 || + fileCountsLoading || + fileCount > fileCountMax)) || + (totalSizeMax !== -1 && + (totalSize <= 0 || + sizesLoading || + totalSize > totalSizeMax)) } > {t('downloadCart.download')} From 6abec9eca84de40136a5f68c1ee1eb1e81b57064 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Wed, 30 Mar 2022 16:47:35 +0100 Subject: [PATCH 04/62] Update tests for downloadApi for new react-query hooks --- packages/datagateway-download/package.json | 6 +- ...wnloadApi.test.ts => downloadApi.test.tsx} | 607 +++++++----------- .../datagateway-download/src/downloadApi.ts | 131 ++-- .../downloadCartTable.component.tsx | 7 +- yarn.lock | 26 +- 5 files changed, 305 insertions(+), 472 deletions(-) rename packages/datagateway-download/src/{downloadApi.test.ts => downloadApi.test.tsx} (72%) diff --git a/packages/datagateway-download/package.json b/packages/datagateway-download/package.json index 931dea01e..893293aad 100644 --- a/packages/datagateway-download/package.json +++ b/packages/datagateway-download/package.json @@ -19,18 +19,22 @@ "i18next-http-backend": "^1.3.2", "lodash.chunk": "^4.2.0", "loglevel": "^1.8.0", + "p-limit": "3.1.0", "react": "^16.13.1", "react-dom": "^16.11.0", "react-i18next": "^11.15.4", + "react-query": "^3.18.1", "react-router-dom": "^5.3.0", "react-scripts": "5.0.0", "react-virtualized": "^9.22.3", "single-spa-react": "^4.3.1", + "tslib": "^2.3.0", "typescript": "4.5.3", - "tslib": "^2.3.0" + "use-deep-compare-effect": "^1.8.1" }, "devDependencies": { "@craco/craco": "^6.4.3", + "@testing-library/react-hooks": "^7.0.1", "@types/jsrsasign": "^9.0.3", "@types/lodash.chunk": "^4.2.6", "@typescript-eslint/eslint-plugin": "^5.5.0", diff --git a/packages/datagateway-download/src/downloadApi.test.ts b/packages/datagateway-download/src/downloadApi.test.tsx similarity index 72% rename from packages/datagateway-download/src/downloadApi.test.ts rename to packages/datagateway-download/src/downloadApi.test.tsx index 7e70edad7..cf78a795a 100644 --- a/packages/datagateway-download/src/downloadApi.test.ts +++ b/packages/datagateway-download/src/downloadApi.test.tsx @@ -3,22 +3,26 @@ import { DownloadCartItem, handleICATError } from 'datagateway-common'; import { downloadDeleted, downloadPreparedCart, - fetchDownloadCartItems, fetchDownloads, - getCartDatafileCount, - getCartSize, - getDatafileCount, getDownload, - getIsTwoLevel, - getSize, - removeAllDownloadCartItems, - removeDownloadCartItem, submitCart, getDataUrl, fetchAdminDownloads, adminDownloadDeleted, adminDownloadStatus, + useCart, + useRemoveAllFromCart, + useRemoveEntityFromCart, + useIsTwoLevel, + useSizes, + useDatafileCounts, } from './downloadApi'; +import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; +import React from 'react'; +import { createMemoryHistory } from 'history'; +import { QueryClient, QueryClientProvider, setLogger } from 'react-query'; +import { Router } from 'react-router-dom'; +import { DownloadSettingsContext } from './ConfigProvider'; jest.mock('datagateway-common', () => { const originalModule = jest.requireActual('datagateway-common'); @@ -52,13 +56,45 @@ const mockedSettings = { }, }; +// silence react-query errors +setLogger({ + log: console.log, + warn: console.warn, + error: jest.fn(), +}); + +const createTestQueryClient = (): QueryClient => + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + +const createReactQueryWrapper = (): WrapperComponent => { + const testQueryClient = createTestQueryClient(); + const history = createMemoryHistory(); + + const wrapper: WrapperComponent = ({ children }) => ( + + + + {children} + + + + ); + return wrapper; +}; + describe('Download Cart API functions test', () => { afterEach(() => { (handleICATError as jest.Mock).mockClear(); }); - describe('fetchDownloadCartItems', () => { - it('returns cartItems upon successful response', async () => { + describe('useCart', () => { + it('sends axios request to fetch cart and returns successful response', async () => { const downloadCartMockData = { cartItems: [ { @@ -83,48 +119,38 @@ describe('Download Cart API functions test', () => { userName: 'test user', }; - axios.get = jest.fn().mockImplementation(() => - Promise.resolve({ - data: downloadCartMockData, - }) - ); + axios.get = jest.fn().mockResolvedValue({ + data: downloadCartMockData, + }); - const returnData = await fetchDownloadCartItems({ - facilityName: mockedSettings.facilityName, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useCart(), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toBe(downloadCartMockData.cartItems); - expect(axios.get).toHaveBeenCalled(); - expect( - axios.get - ).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}`, - { params: { sessionId: null } } + await waitFor(() => result.current.isSuccess); + + expect(axios.get).toHaveBeenCalledWith( + 'https://example.com/downloadApi/user/cart/LILS', + { + params: { + sessionId: null, + }, + } ); + expect(result.current.data).toEqual(downloadCartMockData.cartItems); }); - it('returns empty array and logs error upon unsuccessful response', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); + it('sends axios request to fetch cart and calls handleICATError on failure', async () => { + axios.get = jest.fn().mockRejectedValue({ + message: 'Test error message', + }); - const returnData = await fetchDownloadCartItems({ - facilityName: mockedSettings.facilityName, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useCart(), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toEqual([]); - expect(axios.get).toHaveBeenCalled(); - expect( - axios.get - ).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}`, - { params: { sessionId: null } } - ); - expect(handleICATError).toHaveBeenCalled(); + await waitFor(() => result.current.isError); + expect(handleICATError).toHaveBeenCalledWith({ message: 'Test error message', }); @@ -143,12 +169,18 @@ describe('Download Cart API functions test', () => { }) ); - const returnData = await removeAllDownloadCartItems({ - facilityName: mockedSettings.facilityName, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toBeUndefined(); + expect(axios.delete).not.toHaveBeenCalled(); + expect(result.current.isIdle).toBe(true); + + result.current.mutate(); + + await waitFor(() => result.current.isSuccess); + + expect(result.current.data).toBeUndefined(); expect(axios.delete).toHaveBeenCalled(); expect( axios.delete @@ -158,27 +190,27 @@ describe('Download Cart API functions test', () => { ); }); - it('returns empty array and logs error upon unsuccessful response', async () => { + it('logs error upon unsuccessful response', async () => { axios.delete = jest.fn().mockImplementation(() => Promise.reject({ message: 'Test error message', }) ); - const returnData = await removeAllDownloadCartItems({ - facilityName: mockedSettings.facilityName, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toBeUndefined(); - expect(axios.delete).toHaveBeenCalled(); + result.current.mutate(); + + await waitFor(() => result.current.isError); + expect( axios.delete ).toHaveBeenCalledWith( `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, { params: { sessionId: null, items: '*' } } ); - expect(handleICATError).toHaveBeenCalled(); expect(handleICATError).toHaveBeenCalledWith({ message: 'Test error message', }); @@ -186,7 +218,7 @@ describe('Download Cart API functions test', () => { }); describe('removeDownloadCartItem', () => { - it('returns nothing upon successful response', async () => { + it('returns empty array upon successful response', async () => { axios.delete = jest.fn().mockImplementation(() => Promise.resolve({ data: { @@ -197,12 +229,18 @@ describe('Download Cart API functions test', () => { }) ); - const returnData = await removeDownloadCartItem(1, 'datafile', { - facilityName: mockedSettings.facilityName, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useRemoveEntityFromCart(), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toBeUndefined(); + expect(axios.delete).not.toHaveBeenCalled(); + expect(result.current.isIdle).toBe(true); + + result.current.mutate({ entityId: 1, entityType: 'datafile' }); + + await waitFor(() => result.current.isSuccess); + + expect(result.current.data).toEqual([]); expect(axios.delete).toHaveBeenCalled(); expect( axios.delete @@ -212,27 +250,27 @@ describe('Download Cart API functions test', () => { ); }); - it('returns empty array and logs error upon unsuccessful response', async () => { + it('logs error upon unsuccessful response', async () => { axios.delete = jest.fn().mockImplementation(() => Promise.reject({ message: 'Test error message', }) ); - const returnData = await removeDownloadCartItem(1, 'investigation', { - facilityName: mockedSettings.facilityName, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useRemoveEntityFromCart(), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toBeUndefined(); - expect(axios.delete).toHaveBeenCalled(); + result.current.mutate({ entityId: 1, entityType: 'investigation' }); + + await waitFor(() => result.current.isError); + expect( axios.delete ).toHaveBeenCalledWith( `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, { params: { sessionId: null, items: 'investigation 1' } } ); - expect(handleICATError).toHaveBeenCalled(); expect(handleICATError).toHaveBeenCalledWith({ message: 'Test error message', }); @@ -247,13 +285,16 @@ describe('Download Cart API functions test', () => { }) ); - const isTwoLevel = await getIsTwoLevel({ idsUrl: mockedSettings.idsUrl }); + const { result, waitFor } = renderHook(() => useIsTwoLevel(), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => result.current.isSuccess); - expect(isTwoLevel).toBe(true); - expect(axios.get).toHaveBeenCalled(); expect(axios.get).toHaveBeenCalledWith( `${mockedSettings.idsUrl}/isTwoLevel` ); + expect(result.current.data).toEqual(true); }); it('returns false in the event of an error and logs error upon unsuccessful response', async () => { @@ -263,20 +304,18 @@ describe('Download Cart API functions test', () => { }) ); - const isTwoLevel = await getIsTwoLevel({ idsUrl: mockedSettings.idsUrl }); + const { result, waitFor } = renderHook(() => useIsTwoLevel(), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => result.current.isError); - expect(isTwoLevel).toBe(false); - expect(axios.get).toHaveBeenCalled(); expect(axios.get).toHaveBeenCalledWith( `${mockedSettings.idsUrl}/isTwoLevel` ); - expect(handleICATError).toHaveBeenCalled(); - expect(handleICATError).toHaveBeenCalledWith( - { - message: 'Test error message', - }, - false - ); + expect(handleICATError).toHaveBeenCalledWith({ + message: 'Test error message', + }); }); }); @@ -460,107 +499,76 @@ describe('Download Cart API functions test', () => { }); }); - describe('getSize', () => { - it('returns a number upon successful response for datafile entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.resolve({ - data: { - id: 1, - name: 'test datafile', - fileSize: 1, - }, + describe('useSizes', () => { + it('returns the sizes of all the items in a cart', async () => { + axios.get = jest + .fn() + .mockImplementation((path) => { + if (path.includes('datafiles/')) { + return Promise.resolve({ + data: { + id: 1, + name: 'test datafile', + fileSize: 1, + }, + }); + } else { + return Promise.resolve({ + data: 1, + }); + } }) - ); - - const returnData = await getSize(1, 'datafile', { - facilityName: mockedSettings.facilityName, - apiUrl: mockedSettings.apiUrl, - downloadApiUrl: mockedSettings.downloadApiUrl, - }); + .mockImplementationOnce(() => + Promise.reject({ + message: 'simulating a failed response', + }) + ); - expect(returnData).toBe(1); - expect(axios.get).toHaveBeenCalled(); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/1`, + const cartItems: DownloadCartItem[] = [ { - headers: { Authorization: 'Bearer null' }, - } - ); - }); - - it('returns -1 and logs error upon unsuccessful response for datafile entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); - - const returnData = await getSize(1, 'datafile', { - facilityName: mockedSettings.facilityName, - apiUrl: mockedSettings.apiUrl, - downloadApiUrl: mockedSettings.downloadApiUrl, - }); - - expect(returnData).toBe(-1); - expect(axios.get).toHaveBeenCalled(); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/1`, + entityId: 1, + entityType: 'investigation', + id: 1, + name: 'INVESTIGATION 1', + parentEntities: [], + }, { - headers: { Authorization: 'Bearer null' }, - } - ); - expect(handleICATError).toHaveBeenCalled(); - expect(handleICATError).toHaveBeenCalledWith( + entityId: 2, + entityType: 'dataset', + id: 2, + name: 'DATASET 2', + parentEntities: [], + }, { - message: 'Test error message', + entityId: 3, + entityType: 'datafile', + id: 3, + name: 'DATAFILE 1', + parentEntities: [], }, - false - ); - }); - - it('returns a number upon successful response for non-datafile entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.resolve({ - data: 2, - }) - ); + { + entityId: 4, + entityType: 'investigation', + id: 4, + name: 'INVESTIGATION 1', + parentEntities: [], + }, + ]; - const returnData = await getSize(1, 'dataset', { - facilityName: mockedSettings.facilityName, - apiUrl: mockedSettings.apiUrl, - downloadApiUrl: mockedSettings.downloadApiUrl, + const { result, waitFor } = renderHook(() => useSizes(cartItems), { + wrapper: createReactQueryWrapper(), }); - expect(returnData).toBe(2); - expect(axios.get).toHaveBeenCalled(); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/getSize`, - { - params: { - sessionId: null, - facilityName: mockedSettings.facilityName, - entityType: 'dataset', - entityId: 1, - }, - } - ); - }); - - it('returns -1 and logs error upon unsuccessful response for non-datafile entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) + await waitFor(() => + result.current.every((query) => query.isSuccess || query.isError) ); - const returnData = await getSize(1, 'investigation', { - facilityName: mockedSettings.facilityName, - apiUrl: mockedSettings.apiUrl, - downloadApiUrl: mockedSettings.downloadApiUrl, - }); - - expect(returnData).toBe(-1); - expect(axios.get).toHaveBeenCalled(); + expect(result.current.map((query) => query.data)).toEqual([ + undefined, + 1, + 1, + 1, + ]); expect(axios.get).toHaveBeenCalledWith( `${mockedSettings.downloadApiUrl}/user/getSize`, { @@ -572,158 +580,36 @@ describe('Download Cart API functions test', () => { }, } ); - expect(handleICATError).toHaveBeenCalled(); - expect(handleICATError).toHaveBeenCalledWith( - { - message: 'Test error message', - }, - false - ); - }); - }); - - describe('getDatafileCount', () => { - it('returns 1 upon request for datafile entityType', async () => { - const returnData = await getDatafileCount(1, 'datafile', { - apiUrl: mockedSettings.apiUrl, - }); - - expect(returnData).toBe(1); - }); - - it('returns a number upon successful response for dataset entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.resolve({ - data: 2, - }) - ); - - const returnData = await getDatafileCount(1, 'dataset', { - apiUrl: mockedSettings.apiUrl, - }); - - expect(returnData).toBe(2); - expect(axios.get).toHaveBeenCalled(); expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/count`, + `${mockedSettings.downloadApiUrl}/user/getSize`, { params: { - where: { - 'dataset.id': { - eq: 1, - }, - }, - include: '"dataset"', + sessionId: null, + facilityName: mockedSettings.facilityName, + entityType: 'dataset', + entityId: 2, }, - headers: { Authorization: 'Bearer null' }, } ); - }); - - it('returns -1 and logs error upon unsuccessful response for dataset entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); - - const returnData = await getDatafileCount(1, 'dataset', { - apiUrl: mockedSettings.apiUrl, - }); - - expect(returnData).toBe(-1); - expect(axios.get).toHaveBeenCalled(); expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/count`, + `${mockedSettings.apiUrl}/datafiles/${3}`, { - params: { - where: { - 'dataset.id': { - eq: 1, - }, - }, - include: '"dataset"', + headers: { + Authorization: 'Bearer null', }, - headers: { Authorization: 'Bearer null' }, } ); - expect(handleICATError).toHaveBeenCalled(); expect(handleICATError).toHaveBeenCalledWith( { - message: 'Test error message', - }, - false - ); - }); - - it('returns a number upon successful response for investigation entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.resolve({ - data: 5, - }) - ); - - const returnData = await getDatafileCount(2, 'investigation', { - apiUrl: mockedSettings.apiUrl, - }); - - expect(returnData).toBe(5); - expect(axios.get).toHaveBeenCalled(); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/count`, - { - params: { - include: '{"dataset": "investigation"}', - where: { - 'dataset.investigation.id': { - eq: 2, - }, - }, - }, - headers: { Authorization: 'Bearer null' }, - } - ); - }); - - it('returns -1 and logs error upon unsuccessful response for investigation entityType', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); - - const returnData = await getDatafileCount(2, 'investigation', { - apiUrl: mockedSettings.apiUrl, - }); - - expect(returnData).toBe(-1); - expect(axios.get).toHaveBeenCalled(); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/count`, - { - params: { - include: '{"dataset": "investigation"}', - where: { - 'dataset.investigation.id': { - eq: 2, - }, - }, - }, - headers: { Authorization: 'Bearer null' }, - } - ); - expect(handleICATError).toHaveBeenCalled(); - expect(handleICATError).toHaveBeenCalledWith( - { - message: 'Test error message', + message: 'simulating a failed response', }, false ); }); }); - describe('getCartDatafileCount', () => { - it('returns an accurate count of a given cart', async () => { + describe('useDatafileCounts', () => { + it('returns the counts of all the items in a cart', async () => { axios.get = jest .fn() .mockImplementation(() => @@ -768,87 +654,54 @@ describe('Download Cart API functions test', () => { }, ]; - const returnData = await getCartDatafileCount(cartItems, { - apiUrl: mockedSettings.apiUrl, - }); - - expect(returnData).toBe(3); - expect(axios.get).toHaveBeenCalledTimes(3); - expect(handleICATError).toHaveBeenCalled(); - expect(handleICATError).toHaveBeenCalledWith( + const { result, waitFor } = renderHook( + () => useDatafileCounts(cartItems), { - message: 'simulating a failed response', - }, - false + wrapper: createReactQueryWrapper(), + } ); - }); - }); - describe('getCartSize', () => { - it('returns an accurate size of a given cart', async () => { - axios.get = jest - .fn() - .mockImplementation((path) => { - if (path.includes('datafiles/')) { - return Promise.resolve({ - data: { - id: 1, - name: 'test datafile', - fileSize: 1, - }, - }); - } else { - return Promise.resolve({ - data: 1, - }); - } - }) - .mockImplementationOnce(() => - Promise.reject({ - message: 'simulating a failed response', - }) - ); + await waitFor(() => + result.current.every((query) => query.isSuccess || query.isError) + ); - const cartItems: DownloadCartItem[] = [ - { - entityId: 1, - entityType: 'investigation', - id: 1, - name: 'INVESTIGATION 1', - parentEntities: [], - }, - { - entityId: 2, - entityType: 'investigation', - id: 2, - name: 'INVESTIGATION 2', - parentEntities: [], - }, + expect(result.current.map((query) => query.data)).toEqual([ + undefined, + 1, + 1, + 1, + ]); + expect(axios.get).toHaveBeenCalledTimes(3); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.apiUrl}/datafiles/count`, { - entityId: 3, - entityType: 'dataset', - id: 3, - name: 'DATASET 1', - parentEntities: [], - }, + params: { + where: { + 'dataset.investigation.id': { + eq: 2, + }, + }, + }, + headers: { + Authorization: 'Bearer null', + }, + } + ); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.apiUrl}/datafiles/count`, { - entityId: 4, - entityType: 'datafile', - id: 4, - name: 'DATAFILE 1', - parentEntities: [], - }, - ]; - - const returnData = await getCartSize(cartItems, { - facilityName: mockedSettings.facilityName, - apiUrl: mockedSettings.apiUrl, - downloadApiUrl: mockedSettings.downloadApiUrl, - }); - - expect(returnData).toBe(3); - expect(axios.get).toHaveBeenCalledTimes(4); - expect(handleICATError).toHaveBeenCalled(); + params: { + where: { + 'dataset.id': { + eq: 3, + }, + }, + }, + headers: { + Authorization: 'Bearer null', + }, + } + ); expect(handleICATError).toHaveBeenCalledWith( { message: 'simulating a failed response', diff --git a/packages/datagateway-download/src/downloadApi.ts b/packages/datagateway-download/src/downloadApi.ts index ddbf0e6fe..6b9c2228c 100644 --- a/packages/datagateway-download/src/downloadApi.ts +++ b/packages/datagateway-download/src/downloadApi.ts @@ -10,7 +10,6 @@ import { handleICATError, fetchDownloadCart, removeFromCart, - DownloadCartTableItem, } from 'datagateway-common'; import { DownloadSettingsContext } from './ConfigProvider'; import { @@ -25,10 +24,7 @@ import { import pLimit from 'p-limit'; import useDeepCompareEffect from 'use-deep-compare-effect'; -export const useCart = (): UseQueryResult< - DownloadCartTableItem[], - AxiosError -> => { +export const useCart = (): UseQueryResult => { const settings = React.useContext(DownloadSettingsContext); const { facilityName, downloadApiUrl } = settings; return useQuery( @@ -42,17 +38,6 @@ export const useCart = (): UseQueryResult< onError: (error) => { handleICATError(error); }, - select: (cart): DownloadCartTableItem[] => { - return cart.map((cartItem) => ({ - ...cartItem, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - size: cartItem.size ?? -1, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - fileCount: cartItem.fileCount ?? -1, - })); - }, staleTime: 0, } ); @@ -61,24 +46,29 @@ export const useCart = (): UseQueryResult< export const removeAllDownloadCartItems: (settings: { facilityName: string; downloadApiUrl: string; -}) => Promise = (settings: { +}) => Promise = (settings: { facilityName: string; downloadApiUrl: string; }) => { - return axios.delete( - `${settings.downloadApiUrl}/user/cart/${settings.facilityName}/cartItems`, - { - params: { - sessionId: readSciGatewayToken().sessionId, - items: '*', - }, - } - ); + return axios + .delete( + `${settings.downloadApiUrl}/user/cart/${settings.facilityName}/cartItems`, + { + params: { + sessionId: readSciGatewayToken().sessionId, + items: '*', + }, + } + ) + .then(() => { + // do nothing + }); }; export const useRemoveAllFromCart = (): UseMutationResult< - DownloadCartItem[], - AxiosError + void, + AxiosError, + void > => { const queryClient = useQueryClient(); const settings = React.useContext(DownloadSettingsContext); @@ -123,7 +113,7 @@ export const useRemoveEntityFromCart = (): UseMutationResult< ); }; -export const getIsTwoLevel: (settings: { +const getIsTwoLevel: (settings: { idsUrl: string; }) => Promise = (settings: { idsUrl: string }) => { return axios @@ -185,45 +175,50 @@ export const submitCart: ( // Get the downloadId that was returned from the IDS server. const downloadId = response.data['downloadId']; return downloadId; + }) + .catch((error) => { + handleICATError(error); + return -1; }); }; -export const useSubmitCart = (): UseMutationResult< - number, - AxiosError, - { - transport: string; - emailAddress: string; - fileName: string; - zipType?: 'ZIP' | 'ZIP_AND_COMPRESS'; - } -> => { - const queryClient = useQueryClient(); - const settings = React.useContext(DownloadSettingsContext); - const { facilityName, downloadApiUrl } = settings; - - return useMutation( - ({ transport, emailAddress, fileName, zipType }) => - submitCart( - transport, - emailAddress, - fileName, - { - facilityName, - downloadApiUrl, - }, - zipType - ), - { - onSuccess: (data) => { - queryClient.setQueryData('cart', data); - }, - onError: (error) => { - handleICATError(error); - }, - } - ); -}; +// TODO: refactor rest of dg-download to use react-query +// export const useSubmitCart = (): UseMutationResult< +// number, +// AxiosError, +// { +// transport: string; +// emailAddress: string; +// fileName: string; +// zipType?: 'ZIP' | 'ZIP_AND_COMPRESS'; +// } +// > => { +// const queryClient = useQueryClient(); +// const settings = React.useContext(DownloadSettingsContext); +// const { facilityName, downloadApiUrl } = settings; + +// return useMutation( +// ({ transport, emailAddress, fileName, zipType }) => +// submitCart( +// transport, +// emailAddress, +// fileName, +// { +// facilityName, +// downloadApiUrl, +// }, +// zipType +// ), +// { +// onSuccess: (data) => { +// queryClient.setQueryData('cart', data); +// }, +// onError: (error) => { +// handleICATError(error); +// }, +// } +// ); +// }; export const fetchDownloads: ( settings: { facilityName: string; downloadApiUrl: string }, @@ -459,7 +454,7 @@ export const adminDownloadStatus: ( }); }; -export const getSize: ( +const getSize: ( entityId: number, entityType: string, settings: { @@ -581,7 +576,7 @@ export const useSizes = ( return sizes; }; -export const getDatafileCount: ( +const getDatafileCount: ( entityId: number, entityType: string, settings: { apiUrl: string } @@ -606,7 +601,6 @@ export const getDatafileCount: ( eq: entityId, }, }, - include: '"dataset"', }, headers: { Authorization: `Bearer ${readSciGatewayToken().sessionId}`, @@ -619,7 +613,6 @@ export const getDatafileCount: ( return axios .get(`${settings.apiUrl}/datafiles/count`, { params: { - include: '{"dataset": "investigation"}', where: { 'dataset.investigation.id': { eq: entityId, diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx index 0fd29d5d9..0a55393bb 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx @@ -385,12 +385,7 @@ const DownloadCartTable: React.FC = ( color="primary" disabled={removingAll} startIcon={removingAll && } - onClick={() => - removeAllDownloadCartItems({ - facilityName: settings.facilityName, - downloadApiUrl: settings.downloadApiUrl, - }) - } + onClick={() => removeAllDownloadCartItems()} > {t('downloadCart.remove_all')} diff --git a/yarn.lock b/yarn.lock index f2ed1428b..936faf73c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11292,6 +11292,13 @@ p-finally@^1.0.0: resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae" integrity sha1-P7z7FbiZpEEjs0ttzBi3JDNqLK4= +p-limit@3.1.0, p-limit@^3.0.2: + version "3.1.0" + resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-3.1.0.tgz#e1daccbe78d0d1388ca18c64fea38e3e57e3706b" + integrity sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ== + dependencies: + yocto-queue "^0.1.0" + p-limit@^1.1.0: version "1.3.0" resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-1.3.0.tgz#b86bd5f0c25690911c7590fcbfc2010d54b3ccb8" @@ -11306,20 +11313,6 @@ p-limit@^2.0.0, p-limit@^2.2.0: dependencies: p-try "^2.0.0" -p-limit@^3.0.2: - version "3.1.0" - resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-3.1.0.tgz#e1daccbe78d0d1388ca18c64fea38e3e57e3706b" - integrity sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ== - dependencies: - yocto-queue "^0.1.0" - -p-limit@^4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-4.0.0.tgz#914af6544ed32bfa54670b061cafcbd04984b644" - integrity sha512-5b0R4txpzjPWVw/cXXUResoD4hb6U/x9BH08L7nw+GN1sezDzPdxeRvpc9c433fZhBan/wusjbCsqwqm4EIBIQ== - dependencies: - yocto-queue "^1.0.0" - p-locate@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-2.0.0.tgz#20a0103b222a70c8fd39cc2e580680f3dde5ec43" @@ -15735,8 +15728,3 @@ yocto-queue@^0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== - -yocto-queue@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-1.0.0.tgz#7f816433fb2cbc511ec8bf7d263c3b58a1a3c251" - integrity sha512-9bnSc/HEW2uRy67wc+T8UwauLuPJVn28jb+GtJY16iiKWyvmYJRXVT4UamsAEGQfPohgr2q4Tq0sQbQlxTfi1g== From b4f9d223d3a7ce798ef27f77ffa5c01788efe93b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 31 Mar 2022 14:11:10 +0000 Subject: [PATCH 05/62] Bump minimist from 1.2.5 to 1.2.6 Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6. - [Release notes](https://github.com/substack/minimist/releases) - [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6) --- updated-dependencies: - dependency-name: minimist dependency-type: indirect ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index f2ed1428b..5f1984487 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10600,9 +10600,9 @@ minimist-options@4.1.0: kind-of "^6.0.3" minimist@^1.1.1, minimist@^1.2.0, minimist@^1.2.3, minimist@^1.2.5: - version "1.2.5" - resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.5.tgz#67d66014b66a6a8aaa0c083c5fd58df4e4e97602" - integrity sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw== + version "1.2.6" + resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.6.tgz#8637a5b759ea0d6e98702cfb3a9283323c93af44" + integrity sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q== minipass-collect@^1.0.2: version "1.0.2" From c0a2f0720f42e3b6e68dc1af3306b2b90ce11c93 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Tue, 5 Apr 2022 10:29:14 +0100 Subject: [PATCH 06/62] Update downloadCart unit tests - also split downloadApi into api functions and rq hooks - this makes it easier to test and also smaller files - also add test for disabling download of carts with empty items --- ...wnloadApi.test.tsx => downloadApi.test.ts} | 484 +--------------- .../datagateway-download/src/downloadApi.ts | 294 +--------- .../src/downloadApiHooks.test.tsx | 524 ++++++++++++++++++ .../src/downloadApiHooks.ts | 292 ++++++++++ .../downloadCartTable.component.test.tsx.snap | 92 --- .../downloadCartTable.component.test.tsx | 253 ++++++--- .../downloadCartTable.component.tsx | 8 +- 7 files changed, 991 insertions(+), 956 deletions(-) rename packages/datagateway-download/src/{downloadApi.test.tsx => downloadApi.test.ts} (59%) create mode 100644 packages/datagateway-download/src/downloadApiHooks.test.tsx create mode 100644 packages/datagateway-download/src/downloadApiHooks.ts delete mode 100644 packages/datagateway-download/src/downloadCart/__snapshots__/downloadCartTable.component.test.tsx.snap diff --git a/packages/datagateway-download/src/downloadApi.test.tsx b/packages/datagateway-download/src/downloadApi.test.ts similarity index 59% rename from packages/datagateway-download/src/downloadApi.test.tsx rename to packages/datagateway-download/src/downloadApi.test.ts index cf78a795a..c02aee810 100644 --- a/packages/datagateway-download/src/downloadApi.test.tsx +++ b/packages/datagateway-download/src/downloadApi.test.ts @@ -1,5 +1,5 @@ import axios from 'axios'; -import { DownloadCartItem, handleICATError } from 'datagateway-common'; +import { handleICATError } from 'datagateway-common'; import { downloadDeleted, downloadPreparedCart, @@ -10,19 +10,7 @@ import { fetchAdminDownloads, adminDownloadDeleted, adminDownloadStatus, - useCart, - useRemoveAllFromCart, - useRemoveEntityFromCart, - useIsTwoLevel, - useSizes, - useDatafileCounts, } from './downloadApi'; -import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; -import React from 'react'; -import { createMemoryHistory } from 'history'; -import { QueryClient, QueryClientProvider, setLogger } from 'react-query'; -import { Router } from 'react-router-dom'; -import { DownloadSettingsContext } from './ConfigProvider'; jest.mock('datagateway-common', () => { const originalModule = jest.requireActual('datagateway-common'); @@ -56,269 +44,11 @@ const mockedSettings = { }, }; -// silence react-query errors -setLogger({ - log: console.log, - warn: console.warn, - error: jest.fn(), -}); - -const createTestQueryClient = (): QueryClient => - new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, - }); - -const createReactQueryWrapper = (): WrapperComponent => { - const testQueryClient = createTestQueryClient(); - const history = createMemoryHistory(); - - const wrapper: WrapperComponent = ({ children }) => ( - - - - {children} - - - - ); - return wrapper; -}; - describe('Download Cart API functions test', () => { afterEach(() => { (handleICATError as jest.Mock).mockClear(); }); - describe('useCart', () => { - it('sends axios request to fetch cart and returns successful response', async () => { - const downloadCartMockData = { - cartItems: [ - { - entityId: 1, - entityType: 'investigation', - id: 1, - name: 'INVESTIGATION 1', - parentEntities: [], - }, - { - entityId: 2, - entityType: 'dataset', - id: 2, - name: 'DATASET 2', - parentEntities: [], - }, - ], - createdAt: '2019-11-01T15:18:00Z', - facilityName: mockedSettings.facilityName, - id: 1, - updatedAt: '2019-11-01T15:18:00Z', - userName: 'test user', - }; - - axios.get = jest.fn().mockResolvedValue({ - data: downloadCartMockData, - }); - - const { result, waitFor } = renderHook(() => useCart(), { - wrapper: createReactQueryWrapper(), - }); - - await waitFor(() => result.current.isSuccess); - - expect(axios.get).toHaveBeenCalledWith( - 'https://example.com/downloadApi/user/cart/LILS', - { - params: { - sessionId: null, - }, - } - ); - expect(result.current.data).toEqual(downloadCartMockData.cartItems); - }); - - it('sends axios request to fetch cart and calls handleICATError on failure', async () => { - axios.get = jest.fn().mockRejectedValue({ - message: 'Test error message', - }); - - const { result, waitFor } = renderHook(() => useCart(), { - wrapper: createReactQueryWrapper(), - }); - - await waitFor(() => result.current.isError); - - expect(handleICATError).toHaveBeenCalledWith({ - message: 'Test error message', - }); - }); - }); - - describe('removeAllDownloadCartItems', () => { - it('returns nothing upon successful response', async () => { - axios.delete = jest.fn().mockImplementation(() => - Promise.resolve({ - data: { - cartItems: [], - facilityName: mockedSettings.facilityName, - userName: 'test user', - }, - }) - ); - - const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), { - wrapper: createReactQueryWrapper(), - }); - - expect(axios.delete).not.toHaveBeenCalled(); - expect(result.current.isIdle).toBe(true); - - result.current.mutate(); - - await waitFor(() => result.current.isSuccess); - - expect(result.current.data).toBeUndefined(); - expect(axios.delete).toHaveBeenCalled(); - expect( - axios.delete - ).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, - { params: { sessionId: null, items: '*' } } - ); - }); - - it('logs error upon unsuccessful response', async () => { - axios.delete = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); - - const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), { - wrapper: createReactQueryWrapper(), - }); - - result.current.mutate(); - - await waitFor(() => result.current.isError); - - expect( - axios.delete - ).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, - { params: { sessionId: null, items: '*' } } - ); - expect(handleICATError).toHaveBeenCalledWith({ - message: 'Test error message', - }); - }); - }); - - describe('removeDownloadCartItem', () => { - it('returns empty array upon successful response', async () => { - axios.delete = jest.fn().mockImplementation(() => - Promise.resolve({ - data: { - cartItems: [], - facilityName: mockedSettings.facilityName, - userName: 'test user', - }, - }) - ); - - const { result, waitFor } = renderHook(() => useRemoveEntityFromCart(), { - wrapper: createReactQueryWrapper(), - }); - - expect(axios.delete).not.toHaveBeenCalled(); - expect(result.current.isIdle).toBe(true); - - result.current.mutate({ entityId: 1, entityType: 'datafile' }); - - await waitFor(() => result.current.isSuccess); - - expect(result.current.data).toEqual([]); - expect(axios.delete).toHaveBeenCalled(); - expect( - axios.delete - ).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, - { params: { sessionId: null, items: 'datafile 1' } } - ); - }); - - it('logs error upon unsuccessful response', async () => { - axios.delete = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); - - const { result, waitFor } = renderHook(() => useRemoveEntityFromCart(), { - wrapper: createReactQueryWrapper(), - }); - - result.current.mutate({ entityId: 1, entityType: 'investigation' }); - - await waitFor(() => result.current.isError); - - expect( - axios.delete - ).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, - { params: { sessionId: null, items: 'investigation 1' } } - ); - expect(handleICATError).toHaveBeenCalledWith({ - message: 'Test error message', - }); - }); - }); - - describe('getIsTwoLevel', () => { - it('returns true if IDS is two-level', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.resolve({ - data: true, - }) - ); - - const { result, waitFor } = renderHook(() => useIsTwoLevel(), { - wrapper: createReactQueryWrapper(), - }); - - await waitFor(() => result.current.isSuccess); - - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.idsUrl}/isTwoLevel` - ); - expect(result.current.data).toEqual(true); - }); - - it('returns false in the event of an error and logs error upon unsuccessful response', async () => { - axios.get = jest.fn().mockImplementation(() => - Promise.reject({ - message: 'Test error message', - }) - ); - - const { result, waitFor } = renderHook(() => useIsTwoLevel(), { - wrapper: createReactQueryWrapper(), - }); - - await waitFor(() => result.current.isError); - - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.idsUrl}/isTwoLevel` - ); - expect(handleICATError).toHaveBeenCalledWith({ - message: 'Test error message', - }); - }); - }); - describe('submitCart', () => { it('returns the downloadId after the submitting cart', async () => { axios.post = jest.fn().mockImplementation(() => { @@ -498,218 +228,6 @@ describe('Download Cart API functions test', () => { expect(document.body.appendChild).toHaveBeenCalledWith(link); }); }); - - describe('useSizes', () => { - it('returns the sizes of all the items in a cart', async () => { - axios.get = jest - .fn() - .mockImplementation((path) => { - if (path.includes('datafiles/')) { - return Promise.resolve({ - data: { - id: 1, - name: 'test datafile', - fileSize: 1, - }, - }); - } else { - return Promise.resolve({ - data: 1, - }); - } - }) - .mockImplementationOnce(() => - Promise.reject({ - message: 'simulating a failed response', - }) - ); - - const cartItems: DownloadCartItem[] = [ - { - entityId: 1, - entityType: 'investigation', - id: 1, - name: 'INVESTIGATION 1', - parentEntities: [], - }, - { - entityId: 2, - entityType: 'dataset', - id: 2, - name: 'DATASET 2', - parentEntities: [], - }, - { - entityId: 3, - entityType: 'datafile', - id: 3, - name: 'DATAFILE 1', - parentEntities: [], - }, - { - entityId: 4, - entityType: 'investigation', - id: 4, - name: 'INVESTIGATION 1', - parentEntities: [], - }, - ]; - - const { result, waitFor } = renderHook(() => useSizes(cartItems), { - wrapper: createReactQueryWrapper(), - }); - - await waitFor(() => - result.current.every((query) => query.isSuccess || query.isError) - ); - - expect(result.current.map((query) => query.data)).toEqual([ - undefined, - 1, - 1, - 1, - ]); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/getSize`, - { - params: { - sessionId: null, - facilityName: mockedSettings.facilityName, - entityType: 'investigation', - entityId: 1, - }, - } - ); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.downloadApiUrl}/user/getSize`, - { - params: { - sessionId: null, - facilityName: mockedSettings.facilityName, - entityType: 'dataset', - entityId: 2, - }, - } - ); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/${3}`, - { - headers: { - Authorization: 'Bearer null', - }, - } - ); - expect(handleICATError).toHaveBeenCalledWith( - { - message: 'simulating a failed response', - }, - false - ); - }); - }); - - describe('useDatafileCounts', () => { - it('returns the counts of all the items in a cart', async () => { - axios.get = jest - .fn() - .mockImplementation(() => - Promise.resolve({ - data: 1, - }) - ) - .mockImplementationOnce(() => - Promise.reject({ - message: 'simulating a failed response', - }) - ); - - const cartItems: DownloadCartItem[] = [ - { - entityId: 1, - entityType: 'investigation', - id: 1, - name: 'INVESTIGATION 1', - parentEntities: [], - }, - { - entityId: 2, - entityType: 'investigation', - id: 2, - name: 'INVESTIGATION 2', - parentEntities: [], - }, - { - entityId: 3, - entityType: 'dataset', - id: 3, - name: 'DATASET 1', - parentEntities: [], - }, - { - entityId: 4, - entityType: 'datafile', - id: 4, - name: 'DATAFILE 1', - parentEntities: [], - }, - ]; - - const { result, waitFor } = renderHook( - () => useDatafileCounts(cartItems), - { - wrapper: createReactQueryWrapper(), - } - ); - - await waitFor(() => - result.current.every((query) => query.isSuccess || query.isError) - ); - - expect(result.current.map((query) => query.data)).toEqual([ - undefined, - 1, - 1, - 1, - ]); - expect(axios.get).toHaveBeenCalledTimes(3); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/count`, - { - params: { - where: { - 'dataset.investigation.id': { - eq: 2, - }, - }, - }, - headers: { - Authorization: 'Bearer null', - }, - } - ); - expect(axios.get).toHaveBeenCalledWith( - `${mockedSettings.apiUrl}/datafiles/count`, - { - params: { - where: { - 'dataset.id': { - eq: 3, - }, - }, - }, - headers: { - Authorization: 'Bearer null', - }, - } - ); - expect(handleICATError).toHaveBeenCalledWith( - { - message: 'simulating a failed response', - }, - false - ); - }); - }); }); describe('Download Status API functions test', () => { diff --git a/packages/datagateway-download/src/downloadApi.ts b/packages/datagateway-download/src/downloadApi.ts index 6b9c2228c..2c4d7107f 100644 --- a/packages/datagateway-download/src/downloadApi.ts +++ b/packages/datagateway-download/src/downloadApi.ts @@ -1,47 +1,12 @@ -import React from 'react'; -import axios, { AxiosError, AxiosResponse } from 'axios'; +import axios, { AxiosResponse } from 'axios'; import * as log from 'loglevel'; import { SubmitCart, - DownloadCartItem, Datafile, Download, readSciGatewayToken, handleICATError, - fetchDownloadCart, - removeFromCart, } from 'datagateway-common'; -import { DownloadSettingsContext } from './ConfigProvider'; -import { - UseQueryResult, - useQuery, - useMutation, - UseMutationResult, - useQueryClient, - UseQueryOptions, - useQueries, -} from 'react-query'; -import pLimit from 'p-limit'; -import useDeepCompareEffect from 'use-deep-compare-effect'; - -export const useCart = (): UseQueryResult => { - const settings = React.useContext(DownloadSettingsContext); - const { facilityName, downloadApiUrl } = settings; - return useQuery( - 'cart', - () => - fetchDownloadCart({ - facilityName, - downloadApiUrl, - }), - { - onError: (error) => { - handleICATError(error); - }, - staleTime: 0, - } - ); -}; export const removeAllDownloadCartItems: (settings: { facilityName: string; @@ -65,55 +30,7 @@ export const removeAllDownloadCartItems: (settings: { }); }; -export const useRemoveAllFromCart = (): UseMutationResult< - void, - AxiosError, - void -> => { - const queryClient = useQueryClient(); - const settings = React.useContext(DownloadSettingsContext); - const { facilityName, downloadApiUrl } = settings; - - return useMutation( - () => removeAllDownloadCartItems({ facilityName, downloadApiUrl }), - { - onSuccess: (data) => { - queryClient.setQueryData('cart', []); - }, - onError: (error) => { - handleICATError(error); - }, - } - ); -}; - -export const useRemoveEntityFromCart = (): UseMutationResult< - DownloadCartItem[], - AxiosError, - { entityId: number; entityType: 'investigation' | 'dataset' | 'datafile' } -> => { - const queryClient = useQueryClient(); - const settings = React.useContext(DownloadSettingsContext); - const { facilityName, downloadApiUrl } = settings; - - return useMutation( - ({ entityId, entityType }) => - removeFromCart(entityType, [entityId], { - facilityName, - downloadApiUrl, - }), - { - onSuccess: (data) => { - queryClient.setQueryData('cart', data); - }, - onError: (error) => { - handleICATError(error); - }, - } - ); -}; - -const getIsTwoLevel: (settings: { +export const getIsTwoLevel: (settings: { idsUrl: string; }) => Promise = (settings: { idsUrl: string }) => { return axios @@ -123,17 +40,6 @@ const getIsTwoLevel: (settings: { }); }; -export const useIsTwoLevel = (): UseQueryResult => { - const settings = React.useContext(DownloadSettingsContext); - const { idsUrl } = settings; - return useQuery('isTwoLevel', () => getIsTwoLevel({ idsUrl }), { - onError: (error) => { - handleICATError(error); - }, - staleTime: Infinity, - }); -}; - export const submitCart: ( transport: string, emailAddress: string, @@ -182,44 +88,6 @@ export const submitCart: ( }); }; -// TODO: refactor rest of dg-download to use react-query -// export const useSubmitCart = (): UseMutationResult< -// number, -// AxiosError, -// { -// transport: string; -// emailAddress: string; -// fileName: string; -// zipType?: 'ZIP' | 'ZIP_AND_COMPRESS'; -// } -// > => { -// const queryClient = useQueryClient(); -// const settings = React.useContext(DownloadSettingsContext); -// const { facilityName, downloadApiUrl } = settings; - -// return useMutation( -// ({ transport, emailAddress, fileName, zipType }) => -// submitCart( -// transport, -// emailAddress, -// fileName, -// { -// facilityName, -// downloadApiUrl, -// }, -// zipType -// ), -// { -// onSuccess: (data) => { -// queryClient.setQueryData('cart', data); -// }, -// onError: (error) => { -// handleICATError(error); -// }, -// } -// ); -// }; - export const fetchDownloads: ( settings: { facilityName: string; downloadApiUrl: string }, queryOffset?: string @@ -454,7 +322,7 @@ export const adminDownloadStatus: ( }); }; -const getSize: ( +export const getSize: ( entityId: number, entityType: string, settings: { @@ -498,85 +366,7 @@ const getSize: ( } }; -const sizesLimit = pLimit(10); - -export const useSizes = ( - data: DownloadCartItem[] | undefined -): UseQueryResult[] => { - const settings = React.useContext(DownloadSettingsContext); - const { facilityName, apiUrl, downloadApiUrl } = settings; - - const queryConfigs: UseQueryOptions< - number, - AxiosError, - number, - ['size', number] - >[] = React.useMemo(() => { - return data - ? data.map((cartItem) => { - const { entityId, entityType } = cartItem; - return { - queryKey: ['size', entityId], - queryFn: () => - sizesLimit(() => - getSize(entityId, entityType, { - facilityName, - apiUrl, - downloadApiUrl, - }) - ), - onError: (error) => { - handleICATError(error, false); - }, - staleTime: Infinity, - }; - }) - : []; - }, [data, facilityName, apiUrl, downloadApiUrl]); - - // useQueries doesn't allow us to specify type info, so ignore this line - // since we strongly type the queries object anyway - // we also need to prettier-ignore to make sure we don't wrap onto next line - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - // prettier-ignore - const queries: UseQueryResult[] = useQueries(queryConfigs); - - const [sizes, setSizes] = React.useState< - UseQueryResult[] - >([]); - - const countAppliedRef = React.useRef(0); - - // when data changes (i.e. due to sorting or filtering) set the countAppliedRef - // back to 0 so we can restart the process, as well as clear sizes - React.useEffect(() => { - countAppliedRef.current = 0; - setSizes([]); - }, [data]); - - // need to use useDeepCompareEffect here because the array returned by useQueries - // is different every time this hook runs - useDeepCompareEffect(() => { - const currCountReturned = queries.reduce( - (acc, curr) => acc + (curr.isFetched ? 1 : 0), - 0 - ); - const batchMax = - sizes.length - currCountReturned < 10 - ? sizes.length - currCountReturned - : 10; - // this in effect batches our updates to only happen in batches >= 10 - if (currCountReturned - countAppliedRef.current >= batchMax) { - setSizes(queries); - countAppliedRef.current = currCountReturned; - } - }, [sizes, queries]); - - return sizes; -}; - -const getDatafileCount: ( +export const getDatafileCount: ( entityId: number, entityType: string, settings: { apiUrl: string } @@ -629,82 +419,6 @@ const getDatafileCount: ( } }; -const datafileCountslimit = pLimit(10); - -export const useDatafileCounts = ( - data: DownloadCartItem[] | undefined -): UseQueryResult[] => { - const settings = React.useContext(DownloadSettingsContext); - const { apiUrl } = settings; - - const queryConfigs: UseQueryOptions< - number, - AxiosError, - number, - ['datafileCount', number] - >[] = React.useMemo(() => { - return data - ? data.map((cartItem) => { - const { entityId, entityType } = cartItem; - return { - queryKey: ['datafileCount', entityId], - queryFn: () => - datafileCountslimit(() => - getDatafileCount(entityId, entityType, { - apiUrl, - }) - ), - onError: (error) => { - handleICATError(error, false); - }, - staleTime: Infinity, - }; - }) - : []; - }, [data, apiUrl]); - - // useQueries doesn't allow us to specify type info, so ignore this line - // since we strongly type the queries object anyway - // we also need to prettier-ignore to make sure we don't wrap onto next line - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - // prettier-ignore - const queries: UseQueryResult[] = useQueries(queryConfigs); - - const [datafileCounts, setDatafileCounts] = React.useState< - UseQueryResult[] - >([]); - - const countAppliedRef = React.useRef(0); - - // when data changes (i.e. due to sorting or filtering) set the countAppliedRef - // back to 0 so we can restart the process, as well as clear datafileCounts - React.useEffect(() => { - countAppliedRef.current = 0; - setDatafileCounts([]); - }, [data]); - - // need to use useDeepCompareEffect here because the array returned by useQueries - // is different every time this hook runs - useDeepCompareEffect(() => { - const currCountReturned = queries.reduce( - (acc, curr) => acc + (curr.isFetched ? 1 : 0), - 0 - ); - const batchMax = - datafileCounts.length - currCountReturned < 10 - ? datafileCounts.length - currCountReturned - : 10; - // this in effect batches our updates to only happen in batches >= 10 - if (currCountReturned - countAppliedRef.current >= batchMax) { - setDatafileCounts(queries); - countAppliedRef.current = currCountReturned; - } - }, [datafileCounts, queries]); - - return datafileCounts; -}; - export const getDataUrl = ( preparedId: string, fileName: string, diff --git a/packages/datagateway-download/src/downloadApiHooks.test.tsx b/packages/datagateway-download/src/downloadApiHooks.test.tsx new file mode 100644 index 000000000..5498ddc1e --- /dev/null +++ b/packages/datagateway-download/src/downloadApiHooks.test.tsx @@ -0,0 +1,524 @@ +import axios from 'axios'; +import { DownloadCartItem, handleICATError } from 'datagateway-common'; +import { + useCart, + useRemoveAllFromCart, + useRemoveEntityFromCart, + useIsTwoLevel, + useSizes, + useDatafileCounts, +} from './downloadApiHooks'; +import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; +import React from 'react'; +import { createMemoryHistory } from 'history'; +import { QueryClient, QueryClientProvider, setLogger } from 'react-query'; +import { Router } from 'react-router-dom'; +import { DownloadSettingsContext } from './ConfigProvider'; + +jest.mock('datagateway-common', () => { + const originalModule = jest.requireActual('datagateway-common'); + + return { + __esModule: true, + ...originalModule, + handleICATError: jest.fn(), + }; +}); + +// Create our mocked datagateway-download mockedSettings file. +const mockedSettings = { + facilityName: 'LILS', + apiUrl: 'https://example.com/api', + downloadApiUrl: 'https://example.com/downloadApi', + idsUrl: 'https://example.com/ids', + fileCountMax: 5000, + totalSizeMax: 1000000000000, + accessMethods: { + https: { + idsUrl: 'https://example.com/ids', + displayName: 'HTTPS', + description: 'Example description for HTTPS access method.', + }, + globus: { + idsUrl: 'https://example.com/ids', + displayName: 'Globus', + description: 'Example description for Globus access method.', + }, + }, +}; + +// silence react-query errors +setLogger({ + log: console.log, + warn: console.warn, + error: jest.fn(), +}); + +const createTestQueryClient = (): QueryClient => + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + +const createReactQueryWrapper = (): WrapperComponent => { + const testQueryClient = createTestQueryClient(); + const history = createMemoryHistory(); + + const wrapper: WrapperComponent = ({ children }) => ( + + + + {children} + + + + ); + return wrapper; +}; + +describe('Download Cart API react-query hooks test', () => { + afterEach(() => { + (handleICATError as jest.Mock).mockClear(); + }); + + describe('useCart', () => { + it('sends axios request to fetch cart and returns successful response', async () => { + const downloadCartMockData = { + cartItems: [ + { + entityId: 1, + entityType: 'investigation', + id: 1, + name: 'INVESTIGATION 1', + parentEntities: [], + }, + { + entityId: 2, + entityType: 'dataset', + id: 2, + name: 'DATASET 2', + parentEntities: [], + }, + ], + createdAt: '2019-11-01T15:18:00Z', + facilityName: mockedSettings.facilityName, + id: 1, + updatedAt: '2019-11-01T15:18:00Z', + userName: 'test user', + }; + + axios.get = jest.fn().mockResolvedValue({ + data: downloadCartMockData, + }); + + const { result, waitFor } = renderHook(() => useCart(), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => result.current.isSuccess); + + expect(axios.get).toHaveBeenCalledWith( + 'https://example.com/downloadApi/user/cart/LILS', + { + params: { + sessionId: null, + }, + } + ); + expect(result.current.data).toEqual(downloadCartMockData.cartItems); + }); + + it('sends axios request to fetch cart and calls handleICATError on failure', async () => { + axios.get = jest.fn().mockRejectedValue({ + message: 'Test error message', + }); + + const { result, waitFor } = renderHook(() => useCart(), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => result.current.isError); + + expect(handleICATError).toHaveBeenCalledWith({ + message: 'Test error message', + }); + }); + }); + + describe('useRemoveAllFromCart', () => { + it('returns nothing upon successful response', async () => { + axios.delete = jest.fn().mockImplementation(() => + Promise.resolve({ + data: { + cartItems: [], + facilityName: mockedSettings.facilityName, + userName: 'test user', + }, + }) + ); + + const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), { + wrapper: createReactQueryWrapper(), + }); + + expect(axios.delete).not.toHaveBeenCalled(); + expect(result.current.isIdle).toBe(true); + + result.current.mutate(); + + await waitFor(() => result.current.isSuccess); + + expect(result.current.data).toBeUndefined(); + expect(axios.delete).toHaveBeenCalled(); + expect( + axios.delete + ).toHaveBeenCalledWith( + `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, + { params: { sessionId: null, items: '*' } } + ); + }); + + it('logs error upon unsuccessful response', async () => { + axios.delete = jest.fn().mockImplementation(() => + Promise.reject({ + message: 'Test error message', + }) + ); + + const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), { + wrapper: createReactQueryWrapper(), + }); + + result.current.mutate(); + + await waitFor(() => result.current.isError); + + expect( + axios.delete + ).toHaveBeenCalledWith( + `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, + { params: { sessionId: null, items: '*' } } + ); + expect(handleICATError).toHaveBeenCalledWith({ + message: 'Test error message', + }); + }); + }); + + describe('useRemoveEntityFromCart', () => { + it('returns empty array upon successful response', async () => { + axios.delete = jest.fn().mockImplementation(() => + Promise.resolve({ + data: { + cartItems: [], + facilityName: mockedSettings.facilityName, + userName: 'test user', + }, + }) + ); + + const { result, waitFor } = renderHook(() => useRemoveEntityFromCart(), { + wrapper: createReactQueryWrapper(), + }); + + expect(axios.delete).not.toHaveBeenCalled(); + expect(result.current.isIdle).toBe(true); + + result.current.mutate({ entityId: 1, entityType: 'datafile' }); + + await waitFor(() => result.current.isSuccess); + + expect(result.current.data).toEqual([]); + expect(axios.delete).toHaveBeenCalled(); + expect( + axios.delete + ).toHaveBeenCalledWith( + `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, + { params: { sessionId: null, items: 'datafile 1' } } + ); + }); + + it('logs error upon unsuccessful response', async () => { + axios.delete = jest.fn().mockImplementation(() => + Promise.reject({ + message: 'Test error message', + }) + ); + + const { result, waitFor } = renderHook(() => useRemoveEntityFromCart(), { + wrapper: createReactQueryWrapper(), + }); + + result.current.mutate({ entityId: 1, entityType: 'investigation' }); + + await waitFor(() => result.current.isError); + + expect( + axios.delete + ).toHaveBeenCalledWith( + `${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`, + { params: { sessionId: null, items: 'investigation 1' } } + ); + expect(handleICATError).toHaveBeenCalledWith({ + message: 'Test error message', + }); + }); + }); + + describe('useIsTwoLevel', () => { + it('returns true if IDS is two-level', async () => { + axios.get = jest.fn().mockImplementation(() => + Promise.resolve({ + data: true, + }) + ); + + const { result, waitFor } = renderHook(() => useIsTwoLevel(), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => result.current.isSuccess); + + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.idsUrl}/isTwoLevel` + ); + expect(result.current.data).toEqual(true); + }); + + it('returns false in the event of an error and logs error upon unsuccessful response', async () => { + axios.get = jest.fn().mockImplementation(() => + Promise.reject({ + message: 'Test error message', + }) + ); + + const { result, waitFor } = renderHook(() => useIsTwoLevel(), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => result.current.isError); + + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.idsUrl}/isTwoLevel` + ); + expect(handleICATError).toHaveBeenCalledWith({ + message: 'Test error message', + }); + }); + }); + + describe('useSizes', () => { + it('returns the sizes of all the items in a cart', async () => { + axios.get = jest + .fn() + .mockImplementation((path) => { + if (path.includes('datafiles/')) { + return Promise.resolve({ + data: { + id: 1, + name: 'test datafile', + fileSize: 1, + }, + }); + } else { + return Promise.resolve({ + data: 1, + }); + } + }) + .mockImplementationOnce(() => + Promise.reject({ + message: 'simulating a failed response', + }) + ); + + const cartItems: DownloadCartItem[] = [ + { + entityId: 1, + entityType: 'investigation', + id: 1, + name: 'INVESTIGATION 1', + parentEntities: [], + }, + { + entityId: 2, + entityType: 'dataset', + id: 2, + name: 'DATASET 2', + parentEntities: [], + }, + { + entityId: 3, + entityType: 'datafile', + id: 3, + name: 'DATAFILE 1', + parentEntities: [], + }, + { + entityId: 4, + entityType: 'investigation', + id: 4, + name: 'INVESTIGATION 1', + parentEntities: [], + }, + ]; + + const { result, waitFor } = renderHook(() => useSizes(cartItems), { + wrapper: createReactQueryWrapper(), + }); + + await waitFor(() => + result.current.every((query) => query.isSuccess || query.isError) + ); + + expect(result.current.map((query) => query.data)).toEqual([ + undefined, + 1, + 1, + 1, + ]); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.downloadApiUrl}/user/getSize`, + { + params: { + sessionId: null, + facilityName: mockedSettings.facilityName, + entityType: 'investigation', + entityId: 1, + }, + } + ); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.downloadApiUrl}/user/getSize`, + { + params: { + sessionId: null, + facilityName: mockedSettings.facilityName, + entityType: 'dataset', + entityId: 2, + }, + } + ); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.apiUrl}/datafiles/${3}`, + { + headers: { + Authorization: 'Bearer null', + }, + } + ); + expect(handleICATError).toHaveBeenCalledWith( + { + message: 'simulating a failed response', + }, + false + ); + }); + }); + + describe('useDatafileCounts', () => { + it('returns the counts of all the items in a cart', async () => { + axios.get = jest + .fn() + .mockImplementation(() => + Promise.resolve({ + data: 1, + }) + ) + .mockImplementationOnce(() => + Promise.reject({ + message: 'simulating a failed response', + }) + ); + + const cartItems: DownloadCartItem[] = [ + { + entityId: 1, + entityType: 'investigation', + id: 1, + name: 'INVESTIGATION 1', + parentEntities: [], + }, + { + entityId: 2, + entityType: 'investigation', + id: 2, + name: 'INVESTIGATION 2', + parentEntities: [], + }, + { + entityId: 3, + entityType: 'dataset', + id: 3, + name: 'DATASET 1', + parentEntities: [], + }, + { + entityId: 4, + entityType: 'datafile', + id: 4, + name: 'DATAFILE 1', + parentEntities: [], + }, + ]; + + const { result, waitFor } = renderHook( + () => useDatafileCounts(cartItems), + { + wrapper: createReactQueryWrapper(), + } + ); + + await waitFor(() => + result.current.every((query) => query.isSuccess || query.isError) + ); + + expect(result.current.map((query) => query.data)).toEqual([ + undefined, + 1, + 1, + 1, + ]); + expect(axios.get).toHaveBeenCalledTimes(3); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.apiUrl}/datafiles/count`, + { + params: { + where: { + 'dataset.investigation.id': { + eq: 2, + }, + }, + }, + headers: { + Authorization: 'Bearer null', + }, + } + ); + expect(axios.get).toHaveBeenCalledWith( + `${mockedSettings.apiUrl}/datafiles/count`, + { + params: { + where: { + 'dataset.id': { + eq: 3, + }, + }, + }, + headers: { + Authorization: 'Bearer null', + }, + } + ); + expect(handleICATError).toHaveBeenCalledWith( + { + message: 'simulating a failed response', + }, + false + ); + }); + }); +}); diff --git a/packages/datagateway-download/src/downloadApiHooks.ts b/packages/datagateway-download/src/downloadApiHooks.ts new file mode 100644 index 000000000..eb6836070 --- /dev/null +++ b/packages/datagateway-download/src/downloadApiHooks.ts @@ -0,0 +1,292 @@ +import React from 'react'; +import { AxiosError } from 'axios'; +import { + DownloadCartItem, + handleICATError, + fetchDownloadCart, + removeFromCart, +} from 'datagateway-common'; +import { DownloadSettingsContext } from './ConfigProvider'; +import { + UseQueryResult, + useQuery, + useMutation, + UseMutationResult, + useQueryClient, + UseQueryOptions, + useQueries, +} from 'react-query'; +import pLimit from 'p-limit'; +import useDeepCompareEffect from 'use-deep-compare-effect'; +import { + removeAllDownloadCartItems, + getSize, + getDatafileCount, + getIsTwoLevel, +} from './downloadApi'; + +export const useCart = (): UseQueryResult => { + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + return useQuery( + 'cart', + () => + fetchDownloadCart({ + facilityName, + downloadApiUrl, + }), + { + onError: (error) => { + handleICATError(error); + }, + staleTime: 0, + } + ); +}; + +export const useRemoveAllFromCart = (): UseMutationResult< + void, + AxiosError, + void +> => { + const queryClient = useQueryClient(); + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + + return useMutation( + () => removeAllDownloadCartItems({ facilityName, downloadApiUrl }), + { + onSuccess: (data) => { + queryClient.setQueryData('cart', []); + }, + onError: (error) => { + handleICATError(error); + }, + } + ); +}; + +export const useRemoveEntityFromCart = (): UseMutationResult< + DownloadCartItem[], + AxiosError, + { entityId: number; entityType: 'investigation' | 'dataset' | 'datafile' } +> => { + const queryClient = useQueryClient(); + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, downloadApiUrl } = settings; + + return useMutation( + ({ entityId, entityType }) => + removeFromCart(entityType, [entityId], { + facilityName, + downloadApiUrl, + }), + { + onSuccess: (data) => { + queryClient.setQueryData('cart', data); + }, + onError: (error) => { + handleICATError(error); + }, + } + ); +}; + +export const useIsTwoLevel = (): UseQueryResult => { + const settings = React.useContext(DownloadSettingsContext); + const { idsUrl } = settings; + return useQuery('isTwoLevel', () => getIsTwoLevel({ idsUrl }), { + onError: (error) => { + handleICATError(error); + }, + staleTime: Infinity, + }); +}; + +// TODO: refactor rest of dg-download to use react-query +// export const useSubmitCart = (): UseMutationResult< +// number, +// AxiosError, +// { +// transport: string; +// emailAddress: string; +// fileName: string; +// zipType?: 'ZIP' | 'ZIP_AND_COMPRESS'; +// } +// > => { +// const queryClient = useQueryClient(); +// const settings = React.useContext(DownloadSettingsContext); +// const { facilityName, downloadApiUrl } = settings; + +// return useMutation( +// ({ transport, emailAddress, fileName, zipType }) => +// submitCart( +// transport, +// emailAddress, +// fileName, +// { +// facilityName, +// downloadApiUrl, +// }, +// zipType +// ), +// { +// onSuccess: (data) => { +// queryClient.setQueryData('cart', data); +// }, +// onError: (error) => { +// handleICATError(error); +// }, +// } +// ); +// }; + +const sizesLimit = pLimit(10); + +export const useSizes = ( + data: DownloadCartItem[] | undefined +): UseQueryResult[] => { + const settings = React.useContext(DownloadSettingsContext); + const { facilityName, apiUrl, downloadApiUrl } = settings; + + const queryConfigs: UseQueryOptions< + number, + AxiosError, + number, + ['size', number] + >[] = React.useMemo(() => { + return data + ? data.map((cartItem) => { + const { entityId, entityType } = cartItem; + return { + queryKey: ['size', entityId], + queryFn: () => + sizesLimit(getSize, entityId, entityType, { + facilityName, + apiUrl, + downloadApiUrl, + }), + onError: (error) => { + handleICATError(error, false); + }, + staleTime: Infinity, + }; + }) + : []; + }, [data, facilityName, apiUrl, downloadApiUrl]); + + // useQueries doesn't allow us to specify type info, so ignore this line + // since we strongly type the queries object anyway + // we also need to prettier-ignore to make sure we don't wrap onto next line + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // prettier-ignore + const queries: UseQueryResult[] = useQueries(queryConfigs); + + const [sizes, setSizes] = React.useState< + UseQueryResult[] + >([]); + + const countAppliedRef = React.useRef(0); + + // when data changes (i.e. due to sorting or filtering) set the countAppliedRef + // back to 0 so we can restart the process, as well as clear sizes + React.useEffect(() => { + countAppliedRef.current = 0; + setSizes([]); + }, [data]); + + // need to use useDeepCompareEffect here because the array returned by useQueries + // is different every time this hook runs + useDeepCompareEffect(() => { + const currCountReturned = queries.reduce( + (acc, curr) => acc + (curr.isFetched ? 1 : 0), + 0 + ); + const batchMax = + sizes.length - currCountReturned < 10 + ? sizes.length - currCountReturned + : 10; + // this in effect batches our updates to only happen in batches >= 10 + if (currCountReturned - countAppliedRef.current >= batchMax) { + setSizes(queries); + countAppliedRef.current = currCountReturned; + } + }, [sizes, queries]); + + return sizes; +}; + +const datafileCountslimit = pLimit(10); + +export const useDatafileCounts = ( + data: DownloadCartItem[] | undefined +): UseQueryResult[] => { + const settings = React.useContext(DownloadSettingsContext); + const { apiUrl } = settings; + + const queryConfigs: UseQueryOptions< + number, + AxiosError, + number, + ['datafileCount', number] + >[] = React.useMemo(() => { + return data + ? data.map((cartItem) => { + const { entityId, entityType } = cartItem; + return { + queryKey: ['datafileCount', entityId], + queryFn: () => + datafileCountslimit(getDatafileCount, entityId, entityType, { + apiUrl, + }), + onError: (error) => { + handleICATError(error, false); + }, + staleTime: Infinity, + }; + }) + : []; + }, [data, apiUrl]); + + // useQueries doesn't allow us to specify type info, so ignore this line + // since we strongly type the queries object anyway + // we also need to prettier-ignore to make sure we don't wrap onto next line + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + // prettier-ignore + const queries: UseQueryResult[] = useQueries(queryConfigs); + + const [datafileCounts, setDatafileCounts] = React.useState< + UseQueryResult[] + >([]); + + const countAppliedRef = React.useRef(0); + + // when data changes (i.e. due to sorting or filtering) set the countAppliedRef + // back to 0 so we can restart the process, as well as clear datafileCounts + React.useEffect(() => { + countAppliedRef.current = 0; + setDatafileCounts([]); + }, [data]); + + // need to use useDeepCompareEffect here because the array returned by useQueries + // is different every time this hook runs + useDeepCompareEffect(() => { + const currCountReturned = queries.reduce( + (acc, curr) => acc + (curr.isFetched ? 1 : 0), + 0 + ); + const batchMax = + datafileCounts.length - currCountReturned < 10 + ? datafileCounts.length - currCountReturned + : 10; + // this in effect batches our updates to only happen in batches >= 10 + if (currCountReturned - countAppliedRef.current >= batchMax) { + setDatafileCounts(queries); + countAppliedRef.current = currCountReturned; + } + }, [datafileCounts, queries]); + + return datafileCounts; +}; diff --git a/packages/datagateway-download/src/downloadCart/__snapshots__/downloadCartTable.component.test.tsx.snap b/packages/datagateway-download/src/downloadCart/__snapshots__/downloadCartTable.component.test.tsx.snap deleted file mode 100644 index ade849f68..000000000 --- a/packages/datagateway-download/src/downloadCart/__snapshots__/downloadCartTable.component.test.tsx.snap +++ /dev/null @@ -1,92 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Download cart table component renders correctly 1`] = ` -
- - - - - - No data selected. - - - Browse - - - or - - - search - - - for data. - - - - - -
-`; diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx index 601d58660..d8581e8fd 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx @@ -1,58 +1,53 @@ import React from 'react'; -import { createShallow, createMount } from '@material-ui/core/test-utils'; +import { createMount } from '@material-ui/core/test-utils'; import DownloadCartTable from './downloadCartTable.component'; -import { DownloadCartItem } from 'datagateway-common'; -import { flushPromises } from '../setupTests'; import { - fetchDownloadCartItems, - removeAllDownloadCartItems, - removeDownloadCartItem, - getSize, - getDatafileCount, -} from '../downloadApi'; + DownloadCartItem, + fetchDownloadCart, + removeFromCart, +} from 'datagateway-common'; +import { flushPromises } from '../setupTests'; import { act } from 'react-dom/test-utils'; import { DownloadSettingsContext } from '../ConfigProvider'; import { Router } from 'react-router-dom'; import { ReactWrapper } from 'enzyme'; import { createMemoryHistory } from 'history'; +import { QueryClientProvider, QueryClient } from 'react-query'; +import { + getDatafileCount, + getSize, + removeAllDownloadCartItems, +} from '../downloadApi'; + +jest.mock('datagateway-common', () => { + const originalModule = jest.requireActual('datagateway-common'); + + return { + __esModule: true, + ...originalModule, + fetchDownloadCart: jest.fn(), + removeFromCart: jest.fn(), + }; +}); -jest.mock('../downloadApi'); +jest.mock('../downloadApi', () => { + const originalModule = jest.requireActual('../downloadApi'); + + return { + ...originalModule, + removeAllDownloadCartItems: jest.fn(), + getSize: jest.fn(), + getDatafileCount: jest.fn(), + getIsTwoLevel: jest.fn().mockResolvedValue(true), + }; +}); describe('Download cart table component', () => { - let shallow; let mount; let history; + let queryClient; - const cartItems: DownloadCartItem[] = [ - { - entityId: 1, - entityType: 'investigation', - id: 1, - name: 'INVESTIGATION 1', - parentEntities: [], - }, - { - entityId: 2, - entityType: 'investigation', - id: 2, - name: 'INVESTIGATION 2', - parentEntities: [], - }, - { - entityId: 3, - entityType: 'dataset', - id: 3, - name: 'DATASET 1', - parentEntities: [], - }, - { - entityId: 4, - entityType: 'datafile', - id: 4, - name: 'DATAFILE 1', - parentEntities: [], - }, - ]; + let cartItems: DownloadCartItem[] = []; // Create our mocked datagateway-download settings file. const mockedSettings = { @@ -77,11 +72,14 @@ describe('Download cart table component', () => { }; const createWrapper = (): ReactWrapper => { + queryClient = new QueryClient(); return mount(
- + + +
@@ -89,44 +87,72 @@ describe('Download cart table component', () => { }; beforeEach(() => { - shallow = createShallow({ untilSelector: 'div' }); mount = createMount(); history = createMemoryHistory(); - (fetchDownloadCartItems as jest.Mock).mockImplementation(() => - Promise.resolve(cartItems) - ); - (removeAllDownloadCartItems as jest.Mock).mockImplementation(() => - Promise.resolve() - ); - (removeDownloadCartItem as jest.Mock).mockImplementation(() => - Promise.resolve() - ); - (getSize as jest.Mock).mockImplementation(() => Promise.resolve(1)); - (getDatafileCount as jest.Mock).mockImplementation(() => - Promise.resolve(7) - ); + cartItems = [ + { + entityId: 1, + entityType: 'investigation', + id: 1, + name: 'INVESTIGATION 1', + parentEntities: [], + }, + { + entityId: 2, + entityType: 'investigation', + id: 2, + name: 'INVESTIGATION 2', + parentEntities: [], + }, + { + entityId: 3, + entityType: 'dataset', + id: 3, + name: 'DATASET 1', + parentEntities: [], + }, + { + entityId: 4, + entityType: 'datafile', + id: 4, + name: 'DATAFILE 1', + parentEntities: [], + }, + ]; + + (fetchDownloadCart as jest.MockedFunction< + typeof fetchDownloadCart + >).mockResolvedValue(cartItems); + (removeAllDownloadCartItems as jest.MockedFunction< + typeof removeAllDownloadCartItems + >).mockResolvedValue(null); + (removeFromCart as jest.MockedFunction< + typeof removeFromCart + >).mockImplementation((entityType, entityIds) => { + cartItems = cartItems.filter( + (item) => !entityIds.includes(item.entityId) + ); + return Promise.resolve(cartItems); + }); + + (getSize as jest.MockedFunction).mockResolvedValue(1); + (getDatafileCount as jest.MockedFunction< + typeof getDatafileCount + >).mockResolvedValue(7); }); afterEach(() => { mount.cleanUp(); - (fetchDownloadCartItems as jest.Mock).mockClear(); - (getSize as jest.Mock).mockClear(); - (getDatafileCount as jest.Mock).mockClear(); - (removeAllDownloadCartItems as jest.Mock).mockClear(); - (removeDownloadCartItem as jest.Mock).mockClear(); + jest.clearAllMocks(); jest.clearAllTimers(); jest.useRealTimers(); }); - it('renders correctly', () => { - const wrapper = shallow( - - ); - - expect(wrapper).toMatchSnapshot(); - }); + it('renders no cart message correctly', async () => { + (fetchDownloadCart as jest.MockedFunction< + typeof fetchDownloadCart + >).mockResolvedValue([]); - it('fetches the download cart on load', async () => { const wrapper = createWrapper(); await act(async () => { @@ -134,24 +160,21 @@ describe('Download cart table component', () => { wrapper.update(); }); - expect(fetchDownloadCartItems).toHaveBeenCalled(); + expect(wrapper.exists('[data-testid="no-selections-message"]')).toBe(true); }); - it('does not fetch the download cart on load if no dg-download element exists', async () => { - const wrapper = mount( - - - - - - ); + it('fetches the download cart on load', async () => { + const wrapper = createWrapper(); await act(async () => { await flushPromises(); wrapper.update(); }); - expect(fetchDownloadCartItems).not.toHaveBeenCalled(); + expect(fetchDownloadCart).toHaveBeenCalled(); + expect(wrapper.find('[aria-colindex=1]').find('p').first().text()).toEqual( + 'INVESTIGATION 1' + ); }); it('calculates sizes once cart items have been fetched', async () => { @@ -162,7 +185,7 @@ describe('Download cart table component', () => { wrapper.update(); }); - expect(getSize).toHaveBeenCalledTimes(4); + expect(getSize).toHaveBeenCalled(); expect(wrapper.find('[aria-colindex=3]').find('p').first().text()).toEqual( '1 B' ); @@ -180,6 +203,9 @@ describe('Download cart table component', () => { }); expect(getDatafileCount).toHaveBeenCalled(); + expect(wrapper.find('[aria-colindex=4]').find('p').first().text()).toEqual( + '7' + ); expect(wrapper.find('p#fileCountDisplay').text()).toEqual( expect.stringContaining('downloadCart.number_of_files: 28') ); @@ -246,9 +272,17 @@ describe('Download cart table component', () => { }); it('disables remove all button while request is processing', async () => { - (removeAllDownloadCartItems as jest.Mock).mockImplementation(() => { - return new Promise((resolve) => setTimeout(resolve, 2000)); - }); + // use this to manually resolve promise + let promiseResolve; + + (removeAllDownloadCartItems as jest.MockedFunction< + typeof removeAllDownloadCartItems + >).mockImplementation( + () => + new Promise((resolve) => { + promiseResolve = resolve; + }) + ); const wrapper = createWrapper(); @@ -269,13 +303,60 @@ describe('Download cart table component', () => { ).toBeTruthy(); await act(async () => { - await new Promise((r) => setTimeout(r, 2001)); + promiseResolve(); + await flushPromises(); wrapper.update(); }); expect(wrapper.exists('[data-testid="no-selections-message"]')).toBe(true); }); + it('disables download button when there are empty items in the cart ', async () => { + (getSize as jest.MockedFunction) + .mockResolvedValueOnce(1) + .mockResolvedValueOnce(0); + (getDatafileCount as jest.MockedFunction< + typeof getDatafileCount + >).mockResolvedValueOnce(0); + + const wrapper = createWrapper(); + + await act(async () => { + await flushPromises(); + wrapper.update(); + }); + + expect( + wrapper.find('button#downloadCartButton').prop('disabled') + ).toBeTruthy(); + + wrapper + .find(`button[aria-label="downloadCart.remove {name:INVESTIGATION 2}"]`) + .simulate('click'); + + await act(async () => { + await flushPromises(); + wrapper.update(); + }); + + expect( + wrapper.find('button#downloadCartButton').prop('disabled') + ).toBeTruthy(); + + wrapper + .find(`button[aria-label="downloadCart.remove {name:INVESTIGATION 1}"]`) + .simulate('click'); + + await act(async () => { + await flushPromises(); + wrapper.update(); + }); + + expect( + wrapper.find('button#downloadCartButton').prop('disabled') + ).toBeFalsy(); + }); + it("removes an item when said item's remove button is clicked", async () => { const wrapper = createWrapper(); @@ -307,8 +388,8 @@ describe('Download cart table component', () => { wrapper.update(); }); - expect(removeDownloadCartItem).toHaveBeenCalled(); - expect(removeDownloadCartItem).toHaveBeenCalledWith(2, 'investigation', { + expect(removeFromCart).toHaveBeenCalled(); + expect(removeFromCart).toHaveBeenCalledWith('investigation', [2], { facilityName: mockedSettings.facilityName, downloadApiUrl: mockedSettings.downloadApiUrl, }); diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx index f7facf27e..809c1b4c4 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx @@ -31,7 +31,7 @@ import { useRemoveAllFromCart, useSizes, useDatafileCounts, -} from '../downloadApi'; +} from '../downloadApiHooks'; import DownloadConfirmDialog from '../downloadConfirmation/downloadConfirmDialog.component'; import { DownloadSettingsContext } from '../ConfigProvider'; @@ -71,7 +71,7 @@ const DownloadCartTable: React.FC = ( const [showConfirmation, setShowConfirmation] = React.useState(false); const { data: isTwoLevel } = useIsTwoLevel(); - const { mutateAsync: removeDownloadCartItem } = useRemoveEntityFromCart(); + const { mutate: removeDownloadCartItem } = useRemoveEntityFromCart(); const { mutate: removeAllDownloadCartItems, isLoading: removingAll, @@ -239,7 +239,7 @@ const DownloadCartTable: React.FC = ( aria-label={t('downloadCart.remove', { name: cartItem.name, })} - key="remove" + key={`remove-${entityId}`} size="small" disabled={isDeleting} // Remove the download when clicked. @@ -248,8 +248,6 @@ const DownloadCartTable: React.FC = ( removeDownloadCartItem({ entityId, entityType, - }).then(() => { - setIsDeleting(false); }); }} > From 423f96676d114ce24a33ac75fa6c3e2ea8200f2e Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Tue, 5 Apr 2022 13:33:44 +0100 Subject: [PATCH 07/62] Refactor downloadTabs test to add QueryClientProvider --- .../downloadTab.component.test.tsx | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx b/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx index d801a741d..484b8b848 100644 --- a/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx +++ b/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx @@ -6,6 +6,8 @@ import { flushPromises } from '../setupTests'; import { DownloadSettingsContext } from '../ConfigProvider'; import { createMemoryHistory } from 'history'; import { Router } from 'react-router-dom'; +import { ReactWrapper } from 'enzyme'; +import { QueryClient, QueryClientProvider } from 'react-query'; // Create our mocked datagateway-download settings file. const mockedSettings = { @@ -45,19 +47,26 @@ describe('DownloadTab', () => { sessionStorage.clear(); }); + const createWrapper = (): ReactWrapper => { + const queryClient = new QueryClient(); + return mount( + + + + + + + + ); + }; + it('renders correctly', () => { const wrapper = shallow(); expect(wrapper).toMatchSnapshot(); }); it('renders the previously used tab based on sessionStorage', async () => { - let wrapper = mount( - - - - - - ); + let wrapper = createWrapper(); await act(async () => { await flushPromises(); @@ -95,11 +104,7 @@ describe('DownloadTab', () => { expect(sessionStorage.getItem('downloadStatusTab')).toEqual('1'); // Recreate the wrapper and expect it to show the download tab. - wrapper = mount( - - - - ); + wrapper = createWrapper(); await act(async () => { await flushPromises(); @@ -120,13 +125,7 @@ describe('DownloadTab', () => { }); it('shows the appropriate table when clicking between tabs', async () => { - const wrapper = mount( - - - - - - ); + const wrapper = createWrapper(); await act(async () => { await flushPromises(); From 3a8e2966e8f70c20620e323b504a0b5885913c68 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Tue, 5 Apr 2022 15:43:11 +0100 Subject: [PATCH 08/62] Fix cart e2e tests & add aria-label to loading spinners - also fixed bug in e2e server code serving the wrong settings file --- .../cypress/integration/downloadCart.spec.ts | 6 ++--- .../public/res/default.json | 1 + .../server/e2e-test-server.js | 10 ++++----- .../downloadCartTable.component.tsx | 22 +++++++++++++++---- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/datagateway-download/cypress/integration/downloadCart.spec.ts b/packages/datagateway-download/cypress/integration/downloadCart.spec.ts index 974fa6161..6b4a5bb04 100644 --- a/packages/datagateway-download/cypress/integration/downloadCart.spec.ts +++ b/packages/datagateway-download/cypress/integration/downloadCart.spec.ts @@ -102,7 +102,6 @@ describe('Download Cart', () => { it('should be able to remove individual items from the cart', () => { cy.intercept('DELETE', '**/topcat/user/cart/**').as('removeFromCart'); cy.contains('[role="button"]', 'Name').click(); - cy.contains('Calculating...', { timeout: 20000 }).should('not.exist'); cy.contains(/^DATASET 1$/).should('be.visible'); cy.get('[aria-label="Remove DATASET 1 from selection"]').click(); @@ -119,7 +118,6 @@ describe('Download Cart', () => { it('should be able to remove all items from the cart', () => { cy.intercept('DELETE', '**/topcat/user/cart/**').as('removeFromCart'); cy.contains('[role="button"]', 'Name').click(); - cy.contains('Calculating...', { timeout: 20000 }).should('not.exist'); cy.contains(/^DATASET 1$/).should('be.visible'); cy.contains('Remove All').click(); @@ -134,7 +132,9 @@ describe('Download Cart', () => { }); it('should be able open and close the download confirmation dialog', () => { - cy.contains('Calculating...', { timeout: 20000 }).should('not.exist'); + cy.get('[aria-label="Calculating"]', { timeout: 20000 }).should( + 'not.exist' + ); cy.contains('Download Selection').click(); cy.get('[aria-label="Download confirmation dialog"]').should('exist'); diff --git a/packages/datagateway-download/public/res/default.json b/packages/datagateway-download/public/res/default.json index 31ea8384a..9c02d7032 100644 --- a/packages/datagateway-download/public/res/default.json +++ b/packages/datagateway-download/public/res/default.json @@ -76,6 +76,7 @@ "type": "Type", "size": "Size", "fileCount": "File Count", + "calculating": "Calculating", "remove": "Remove {{name}} from selection", "remove_all": "Remove All", "download": "Download Selection", diff --git a/packages/datagateway-download/server/e2e-test-server.js b/packages/datagateway-download/server/e2e-test-server.js index b233b56a1..d8b59ed6b 100644 --- a/packages/datagateway-download/server/e2e-test-server.js +++ b/packages/datagateway-download/server/e2e-test-server.js @@ -4,15 +4,15 @@ var serveStatic = require('serve-static'); var app = express(); +app.get('/datagateway-download-settings.json', function (req, res) { + res.sendFile(path.resolve('./server/e2e-settings.json')); +}); + app.use( serveStatic(path.resolve('./build'), { index: ['index.html', 'index.htm'] }) ); -app.get('/datagateway-download-settings.json', function(req, res) { - res.sendFile(path.resolve('./server/e2e-settings.json')); -}); - -app.get('/*', function(req, res) { +app.get('/*', function (req, res) { res.sendFile(path.resolve('./build/index.html')); }); diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx index 809c1b4c4..611a45f73 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.tsx @@ -358,11 +358,18 @@ const DownloadCartTable: React.FC = ( style={{ marginRight: '1.2em' }} > {fileCountsLoading && ( - + )} {t('downloadCart.number_of_files')}:{' '} - {fileCount !== -1 ? fileCount : 'Calculating...'} + {fileCount !== -1 + ? fileCount + : `${t('downloadCart.calculating')}...`} {fileCountMax !== -1 && ` / ${fileCountMax}`} @@ -375,11 +382,18 @@ const DownloadCartTable: React.FC = ( style={{ marginRight: '1.2em' }} > {sizesLoading && ( - + )} {t('downloadCart.total_size')}:{' '} - {totalSize !== -1 ? formatBytes(totalSize) : 'Calculating...'} + {totalSize !== -1 + ? formatBytes(totalSize) + : `${t('downloadCart.calculating')}...`} {totalSizeMax !== -1 && ` / ${formatBytes(totalSizeMax)}`} From d4a08bd7b637c306186edb7562175dd7136ef2a1 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Tue, 5 Apr 2022 16:47:48 +0100 Subject: [PATCH 09/62] Use key correctly in actionCell - also memoise loadMoreRows --- .../src/table/cellRenderers/actionCell.component.tsx | 3 ++- .../datagateway-common/src/table/table.component.tsx | 12 ++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/datagateway-common/src/table/cellRenderers/actionCell.component.tsx b/packages/datagateway-common/src/table/cellRenderers/actionCell.component.tsx index a61731f9a..ab31d3794 100644 --- a/packages/datagateway-common/src/table/cellRenderers/actionCell.component.tsx +++ b/packages/datagateway-common/src/table/cellRenderers/actionCell.component.tsx @@ -10,7 +10,7 @@ type CellRendererProps = TableCellProps & { const ActionCell = React.memo( (props: CellRendererProps): React.ReactElement => { - const { className, actions, rowData } = props; + const { className, actions, rowData, rowIndex } = props; return ( {actions.map((TableAction, index) => ( diff --git a/packages/datagateway-common/src/table/table.component.tsx b/packages/datagateway-common/src/table/table.component.tsx index 8cca1d92b..7f0f67d5c 100644 --- a/packages/datagateway-common/src/table/table.component.tsx +++ b/packages/datagateway-common/src/table/table.component.tsx @@ -146,7 +146,6 @@ const VirtualizedTable = React.memo( allIds, onCheck, onUncheck, - loadMoreRows, loading, totalRowCount, detailsPanel, @@ -156,8 +155,8 @@ const VirtualizedTable = React.memo( } = props; if ( - (loadMoreRows && typeof totalRowCount === 'undefined') || - (totalRowCount && typeof loadMoreRows === 'undefined') + (props.loadMoreRows && typeof totalRowCount === 'undefined') || + (totalRowCount && typeof props.loadMoreRows === 'undefined') ) throw new Error( 'Only one of loadMoreRows and totalRowCount was defined - either define both for infinite loading functionality or neither for no infinite loading' @@ -272,6 +271,11 @@ const VirtualizedTable = React.memo( [widthProps, setWidthProps] ); + const loadMoreRows = React.useMemo( + () => props.loadMoreRows || (() => Promise.resolve()), + [props.loadMoreRows] + ); + const tableCellClass = clsx(classes.tableCell, classes.flexContainer); const tableCellNoPaddingClass = clsx( classes.tableCell, @@ -305,7 +309,7 @@ const VirtualizedTable = React.memo( return ( Promise.resolve())} + loadMoreRows={loadMoreRows} rowCount={rowCount} minimumBatchSize={25} > From b71d4eb5bbf891225ba607cb8be186aa5172b5e7 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Wed, 6 Apr 2022 12:24:32 +0100 Subject: [PATCH 10/62] Fix optimisation of sizes and counts - remove batched updates and increase the concurrency --- .../src/downloadApiHooks.ts | 72 ++----------------- 1 file changed, 6 insertions(+), 66 deletions(-) diff --git a/packages/datagateway-download/src/downloadApiHooks.ts b/packages/datagateway-download/src/downloadApiHooks.ts index eb6836070..3beb29aed 100644 --- a/packages/datagateway-download/src/downloadApiHooks.ts +++ b/packages/datagateway-download/src/downloadApiHooks.ts @@ -141,7 +141,7 @@ export const useIsTwoLevel = (): UseQueryResult => { // ); // }; -const sizesLimit = pLimit(10); +const sizesLimit = pLimit(20); export const useSizes = ( data: DownloadCartItem[] | undefined @@ -183,41 +183,10 @@ export const useSizes = ( // prettier-ignore const queries: UseQueryResult[] = useQueries(queryConfigs); - const [sizes, setSizes] = React.useState< - UseQueryResult[] - >([]); - - const countAppliedRef = React.useRef(0); - - // when data changes (i.e. due to sorting or filtering) set the countAppliedRef - // back to 0 so we can restart the process, as well as clear sizes - React.useEffect(() => { - countAppliedRef.current = 0; - setSizes([]); - }, [data]); - - // need to use useDeepCompareEffect here because the array returned by useQueries - // is different every time this hook runs - useDeepCompareEffect(() => { - const currCountReturned = queries.reduce( - (acc, curr) => acc + (curr.isFetched ? 1 : 0), - 0 - ); - const batchMax = - sizes.length - currCountReturned < 10 - ? sizes.length - currCountReturned - : 10; - // this in effect batches our updates to only happen in batches >= 10 - if (currCountReturned - countAppliedRef.current >= batchMax) { - setSizes(queries); - countAppliedRef.current = currCountReturned; - } - }, [sizes, queries]); - - return sizes; + return queries; }; -const datafileCountslimit = pLimit(10); +const datafileCountslimit = pLimit(20); export const useDatafileCounts = ( data: DownloadCartItem[] | undefined @@ -244,6 +213,8 @@ export const useDatafileCounts = ( handleICATError(error, false); }, staleTime: Infinity, + enabled: entityType !== 'datafile', + initialData: entityType === 'datafile' ? 1 : undefined, }; }) : []; @@ -257,36 +228,5 @@ export const useDatafileCounts = ( // prettier-ignore const queries: UseQueryResult[] = useQueries(queryConfigs); - const [datafileCounts, setDatafileCounts] = React.useState< - UseQueryResult[] - >([]); - - const countAppliedRef = React.useRef(0); - - // when data changes (i.e. due to sorting or filtering) set the countAppliedRef - // back to 0 so we can restart the process, as well as clear datafileCounts - React.useEffect(() => { - countAppliedRef.current = 0; - setDatafileCounts([]); - }, [data]); - - // need to use useDeepCompareEffect here because the array returned by useQueries - // is different every time this hook runs - useDeepCompareEffect(() => { - const currCountReturned = queries.reduce( - (acc, curr) => acc + (curr.isFetched ? 1 : 0), - 0 - ); - const batchMax = - datafileCounts.length - currCountReturned < 10 - ? datafileCounts.length - currCountReturned - : 10; - // this in effect batches our updates to only happen in batches >= 10 - if (currCountReturned - countAppliedRef.current >= batchMax) { - setDatafileCounts(queries); - countAppliedRef.current = currCountReturned; - } - }, [datafileCounts, queries]); - - return datafileCounts; + return queries; }; From fd0425391998a89a4056ec5de90e6d139f13167b Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Wed, 6 Apr 2022 15:28:22 +0100 Subject: [PATCH 11/62] Adding optional prop to filter by time in DatePicker #1175 Depending on if a filterByTime prop is passed, the date filter component will return either a KeyboardDatePicker or a KeyboardDateTimePicker. Thus, the admin download table now contains a KeyboardDateTimePicker for more accurate filtering of results --- packages/datagateway-common/src/api/index.tsx | 17 +- .../dateColumnFilter.component.tsx | 230 ++++++++++++------ .../adminDownloadStatusTable.component.tsx | 5 +- 3 files changed, 174 insertions(+), 78 deletions(-) diff --git a/packages/datagateway-common/src/api/index.tsx b/packages/datagateway-common/src/api/index.tsx index f5b0d9015..0310b6e2e 100644 --- a/packages/datagateway-common/src/api/index.tsx +++ b/packages/datagateway-common/src/api/index.tsx @@ -110,8 +110,8 @@ export const parseSearchToQuery = (queryParams: string): QueryParams => { let startDate = null; let endDate = null; - if (startDateString) startDate = new Date(startDateString + 'T00:00:00Z'); - if (endDateString) endDate = new Date(endDateString + 'T00:00:00Z'); + if (startDateString) startDate = new Date(startDateString); + if (endDateString) endDate = new Date(endDateString); // Create the query parameters object. const params: QueryParams = { @@ -214,14 +214,23 @@ export const getApiParams = ( searchParams.append( 'where', JSON.stringify({ - [column]: { gte: `${filter.startDate} 00:00:00` }, + [column]: { + gte: format( + Date.parse(filter.startDate), + 'yyyy-MM-dd HH:mm:ss' + ), + }, }) ); } if ('endDate' in filter && filter.endDate) { searchParams.append( 'where', - JSON.stringify({ [column]: { lte: `${filter.endDate} 23:59:59` } }) + JSON.stringify({ + [column]: { + lte: format(Date.parse(filter.endDate), 'yyyy-MM-dd HH:mm:ss'), + }, + }) ); } if ('type' in filter && filter.type) { diff --git a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx index f0f2a8b6d..816ea9efa 100644 --- a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx +++ b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx @@ -3,6 +3,7 @@ import DateFnsUtils from '@date-io/date-fns'; import { format, isValid, isEqual } from 'date-fns'; import { KeyboardDatePicker, + KeyboardDateTimePicker, MuiPickersUtilsProvider, } from '@material-ui/pickers'; import { MaterialUiPickersDate } from '@material-ui/pickers/typings/date'; @@ -31,6 +32,7 @@ interface UpdateFilterParams { otherDate: MaterialUiPickersDate; startDateOrEndDateChanged: 'startDate' | 'endDate'; onChange: (value: { startDate?: string; endDate?: string } | null) => void; + filterByTime?: boolean; } export function updateFilter({ @@ -39,6 +41,7 @@ export function updateFilter({ otherDate, startDateOrEndDateChanged, onChange, + filterByTime, }: UpdateFilterParams): void { if (!datesEqual(date, prevDate)) { if ( @@ -49,21 +52,29 @@ export function updateFilter({ } else { onChange({ [startDateOrEndDateChanged]: - date && isValid(date) ? format(date, 'yyyy-MM-dd') : undefined, + date && isValid(date) + ? format(date, filterByTime ? 'yyyy-MM-dd HH:mm:ss' : 'yyyy-MM-dd') + : undefined, [startDateOrEndDateChanged === 'startDate' ? 'endDate' : 'startDate']: otherDate && isValid(otherDate) - ? format(otherDate, 'yyyy-MM-dd') + ? format( + otherDate, + filterByTime ? 'yyyy-MM-dd HH:mm:ss' : 'yyyy-MM-dd' + ) : undefined, }); } } } -const DateColumnFilter = (props: { +interface DateColumnFilterProps { label: string; onChange: (value: { startDate?: string; endDate?: string } | null) => void; value: { startDate?: string; endDate?: string } | undefined; -}): React.ReactElement => { + filterByTime?: boolean; +} + +const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { //Need state to change otherwise wont update error messages for an invalid date const [startDate, setStartDate] = useState( props.value?.startDate ? new Date(props.value.startDate) : null @@ -79,74 +90,149 @@ const DateColumnFilter = (props: { return (
- - + {props.filterByTime ? ( + + + ) : ( + + + )} ); }; diff --git a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx index 59c4cc430..c100e877f 100644 --- a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx +++ b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx @@ -90,10 +90,10 @@ const AdminDownloadStatusTable: React.FC = () => { if (!Array.isArray(filter)) { if ('startDate' in filter || 'endDate' in filter) { const startDate = filter.startDate - ? `${filter.startDate} 00:00:00` + ? filter.startDate : '0000-01-01 00:00:00'; const endDate = filter.endDate - ? `${filter.endDate} 23:59:59` + ? filter.endDate : '9999-12-31 23:59:59'; queryOffset += ` AND UPPER(download.${column}) BETWEEN {ts '${startDate}'} AND {ts '${endDate}'}`; @@ -259,6 +259,7 @@ const AdminDownloadStatusTable: React.FC = () => { } }} value={filters[dataKey] as DateFilter} + filterByTime /> ); From 8922a27e42a5b01590599a4ecc19f11c28aac50f Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Wed, 6 Apr 2022 17:16:40 +0100 Subject: [PATCH 12/62] Removing seconds from date parsing, removing index.tsx changes #1175 Now completely removing the ability to set the seconds value in the admin download table date filtering. Previously, this was possible and causing issues with expected results. To align with the date picker GUI, we can now only go down to the minute level. Also removing changes made to index.tsx. This was to mistakenly account for time inputs to be filtered but this is not needed in index as it can only receive a set date value. As such, the start date will carry on with a time of 00:00:00 and the end date will carry on with a time of 23:59:59 inclusive --- packages/datagateway-common/src/api/index.tsx | 15 ++++----------- .../columnFilters/dateColumnFilter.component.tsx | 16 ++++++++-------- .../adminDownloadStatusTable.component.test.tsx | 8 ++++---- .../adminDownloadStatusTable.component.tsx | 4 ++-- 4 files changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/datagateway-common/src/api/index.tsx b/packages/datagateway-common/src/api/index.tsx index 0310b6e2e..2c344e77e 100644 --- a/packages/datagateway-common/src/api/index.tsx +++ b/packages/datagateway-common/src/api/index.tsx @@ -110,8 +110,8 @@ export const parseSearchToQuery = (queryParams: string): QueryParams => { let startDate = null; let endDate = null; - if (startDateString) startDate = new Date(startDateString); - if (endDateString) endDate = new Date(endDateString); + if (startDateString) startDate = new Date(startDateString + 'T00:00:00Z'); + if (endDateString) endDate = new Date(endDateString + 'T00:00:00Z'); // Create the query parameters object. const params: QueryParams = { @@ -214,12 +214,7 @@ export const getApiParams = ( searchParams.append( 'where', JSON.stringify({ - [column]: { - gte: format( - Date.parse(filter.startDate), - 'yyyy-MM-dd HH:mm:ss' - ), - }, + [column]: { gte: `${filter.startDate} 00:00:00` }, }) ); } @@ -227,9 +222,7 @@ export const getApiParams = ( searchParams.append( 'where', JSON.stringify({ - [column]: { - lte: format(Date.parse(filter.endDate), 'yyyy-MM-dd HH:mm:ss'), - }, + [column]: { lte: `${filter.endDate} 23:59:59` }, }) ); } diff --git a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx index 816ea9efa..c3039d75d 100644 --- a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx +++ b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx @@ -53,13 +53,13 @@ export function updateFilter({ onChange({ [startDateOrEndDateChanged]: date && isValid(date) - ? format(date, filterByTime ? 'yyyy-MM-dd HH:mm:ss' : 'yyyy-MM-dd') + ? format(date, filterByTime ? 'yyyy-MM-dd HH:mm' : 'yyyy-MM-dd') : undefined, [startDateOrEndDateChanged === 'startDate' ? 'endDate' : 'startDate']: otherDate && isValid(otherDate) ? format( otherDate, - filterByTime ? 'yyyy-MM-dd HH:mm:ss' : 'yyyy-MM-dd' + filterByTime ? 'yyyy-MM-dd HH:mm' : 'yyyy-MM-dd' ) : undefined, }); @@ -97,18 +97,18 @@ const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { allowKeyboardControl style={{ whiteSpace: 'nowrap' }} inputProps={{ 'aria-label': `${props.label} filter` }} - invalidDateMessage="Date format: yyyy-MM-dd HH:mm:ss." + invalidDateMessage="Date format: yyyy-MM-dd HH:mm." KeyboardButtonProps={{ size: 'small', 'aria-label': `${props.label} filter from, date/time picker`, }} id={props.label + ' filter from'} aria-hidden="true" - format="yyyy-MM-dd HH:mm:ss" + format="yyyy-MM-dd HH:mm" placeholder="From..." value={startDate} views={['year', 'month', 'date', 'hours', 'minutes']} - maxDate={endDate || new Date('2100-01-01T00:00:00Z')} + maxDate={endDate || new Date('2100-01-01 00:00')} maxDateMessage="Invalid date/time range" color="secondary" onChange={(date) => { @@ -132,18 +132,18 @@ const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { allowKeyboardControl style={{ whiteSpace: 'nowrap' }} inputProps={{ 'aria-label': `${props.label} filter` }} - invalidDateMessage="Date format: yyyy-MM-dd HH:mm:ss." + invalidDateMessage="Date format: yyyy-MM-dd HH:mm." KeyboardButtonProps={{ size: 'small', 'aria-label': `${props.label} filter to, date/time picker`, }} id={props.label + ' filter to'} aria-hidden="true" - format="yyyy-MM-dd HH:mm:ss" + format="yyyy-MM-dd HH:mm" placeholder="To..." value={endDate} views={['year', 'month', 'date', 'hours', 'minutes']} - minDate={startDate || new Date('1984-01-01T00:00:00Z')} + minDate={startDate || new Date('1984-01-01 00:00')} minDateMessage="Invalid date/time range" color="secondary" onChange={(date) => { diff --git a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.test.tsx b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.test.tsx index 6c65cd724..ac3388319 100644 --- a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.test.tsx +++ b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.test.tsx @@ -415,7 +415,7 @@ describe('Admin Download Status Table', () => { 'input[id="downloadStatus.createdAt filter from"]' ); await act(async () => { - dateFromFilterInput.instance().value = '2020-01-01'; + dateFromFilterInput.instance().value = '2020-01-01 00:00'; dateFromFilterInput.simulate('change'); await flushPromises(); wrapper.update(); @@ -423,7 +423,7 @@ describe('Admin Download Status Table', () => { expect(fetchAdminDownloads).toHaveBeenLastCalledWith( { downloadApiUrl: '', facilityName: '' }, - "WHERE UPPER(download.facilityName) = '' AND UPPER(download.createdAt) BETWEEN {ts '2020-01-01 00:00:00'} AND {ts '9999-12-31 23:59:59'} ORDER BY UPPER(download.id) ASC LIMIT 0, 50" + "WHERE UPPER(download.facilityName) = '' AND UPPER(download.createdAt) BETWEEN {ts '2020-01-01 00:00'} AND {ts '9999-12-31 23:59'} ORDER BY UPPER(download.id) ASC LIMIT 0, 50" ); // Get the Requested Data To filter input @@ -431,7 +431,7 @@ describe('Admin Download Status Table', () => { 'input[id="downloadStatus.createdAt filter to"]' ); await act(async () => { - dateToFilterInput.instance().value = '2020-01-02'; + dateToFilterInput.instance().value = '2020-01-02 23:59'; dateToFilterInput.simulate('change'); await flushPromises(); wrapper.update(); @@ -439,7 +439,7 @@ describe('Admin Download Status Table', () => { expect(fetchAdminDownloads).toHaveBeenLastCalledWith( { downloadApiUrl: '', facilityName: '' }, - "WHERE UPPER(download.facilityName) = '' AND UPPER(download.createdAt) BETWEEN {ts '2020-01-01 00:00:00'} AND {ts '2020-01-02 23:59:59'} ORDER BY UPPER(download.id) ASC LIMIT 0, 50" + "WHERE UPPER(download.facilityName) = '' AND UPPER(download.createdAt) BETWEEN {ts '2020-01-01 00:00'} AND {ts '2020-01-02 23:59'} ORDER BY UPPER(download.id) ASC LIMIT 0, 50" ); dateFromFilterInput.instance().value = ''; diff --git a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx index c100e877f..7b06d4095 100644 --- a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx +++ b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx @@ -91,10 +91,10 @@ const AdminDownloadStatusTable: React.FC = () => { if ('startDate' in filter || 'endDate' in filter) { const startDate = filter.startDate ? filter.startDate - : '0000-01-01 00:00:00'; + : '0000-01-01 00:00'; const endDate = filter.endDate ? filter.endDate - : '9999-12-31 23:59:59'; + : '9999-12-31 23:59'; queryOffset += ` AND UPPER(download.${column}) BETWEEN {ts '${startDate}'} AND {ts '${endDate}'}`; } From 4765ba34029e0f791310e248442bc339094d08da Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Wed, 6 Apr 2022 17:18:50 +0100 Subject: [PATCH 13/62] Removing extra formatting on index.tsx #1175 --- packages/datagateway-common/src/api/index.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/datagateway-common/src/api/index.tsx b/packages/datagateway-common/src/api/index.tsx index 2c344e77e..f5b0d9015 100644 --- a/packages/datagateway-common/src/api/index.tsx +++ b/packages/datagateway-common/src/api/index.tsx @@ -221,9 +221,7 @@ export const getApiParams = ( if ('endDate' in filter && filter.endDate) { searchParams.append( 'where', - JSON.stringify({ - [column]: { lte: `${filter.endDate} 23:59:59` }, - }) + JSON.stringify({ [column]: { lte: `${filter.endDate} 23:59:59` } }) ); } if ('type' in filter && filter.type) { From 416bec7e7ead05e0a08743e43dcb34029b66ab6c Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Wed, 6 Apr 2022 18:19:50 +0100 Subject: [PATCH 14/62] Add tests for DateTimePicker #1175 --- .../dateColumnFilter.component.test.tsx.snap | 179 ++++++- .../dateColumnFilter.component.test.tsx | 500 +++++++++++++----- 2 files changed, 529 insertions(+), 150 deletions(-) diff --git a/packages/datagateway-common/src/table/columnFilters/__snapshots__/dateColumnFilter.component.test.tsx.snap b/packages/datagateway-common/src/table/columnFilters/__snapshots__/dateColumnFilter.component.test.tsx.snap index 4cfefac25..79401e666 100644 --- a/packages/datagateway-common/src/table/columnFilters/__snapshots__/dateColumnFilter.component.test.tsx.snap +++ b/packages/datagateway-common/src/table/columnFilters/__snapshots__/dateColumnFilter.component.test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Date filter component renders correctly 1`] = ` +exports[`Date filter component DatePicker functionality useTextFilter hook returns a function which can generate a working text filter 1`] = `
`; -exports[`Date filter component useTextFilter hook returns a function which can generate a working text filter 1`] = ` +exports[`Date filter component DateTimePicker functionality useTextFilter hook returns a function which can generate a working text filter 1`] = ` `; + +exports[`Date filter component renders correctly 1`] = ` +
+ + + +`; diff --git a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.test.tsx b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.test.tsx index 535035651..31c909755 100644 --- a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.test.tsx +++ b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.test.tsx @@ -204,195 +204,419 @@ describe('Date filter component', () => { }); }); - it('calls the onChange method correctly when filling out the date inputs', () => { - const onChange = jest.fn(); - - const baseProps = { - label: 'test', - onChange, - }; + describe('DatePicker functionality', () => { + it('calls the onChange method correctly when filling out the date inputs', () => { + const onChange = jest.fn(); - const wrapper = mount(); + const baseProps = { + label: 'test', + onChange, + }; - const startDateFilterInput = wrapper.find('input').first(); - startDateFilterInput.instance().value = '2019-08-06'; - startDateFilterInput.simulate('change'); + const wrapper = mount(); - expect(onChange).toHaveBeenLastCalledWith({ - startDate: '2019-08-06', - }); + const startDateFilterInput = wrapper.find('input').first(); + startDateFilterInput.instance().value = '2019-08-06'; + startDateFilterInput.simulate('change'); - wrapper.setProps({ ...baseProps, value: { startDate: '2019-08-06' } }); - const endDateFilterInput = wrapper.find('input').last(); - endDateFilterInput.instance().value = '2019-08-06'; - endDateFilterInput.simulate('change'); + expect(onChange).toHaveBeenLastCalledWith({ + startDate: '2019-08-06', + }); - expect(onChange).toHaveBeenLastCalledWith({ - startDate: '2019-08-06', - endDate: '2019-08-06', - }); + wrapper.setProps({ ...baseProps, value: { startDate: '2019-08-06' } }); + const endDateFilterInput = wrapper.find('input').last(); + endDateFilterInput.instance().value = '2019-08-06'; + endDateFilterInput.simulate('change'); - wrapper.setProps({ - ...baseProps, - value: { + expect(onChange).toHaveBeenLastCalledWith({ startDate: '2019-08-06', endDate: '2019-08-06', - }, - }); - startDateFilterInput.instance().value = ''; - startDateFilterInput.simulate('change'); + }); - expect(onChange).toHaveBeenLastCalledWith({ - endDate: '2019-08-06', - }); + wrapper.setProps({ + ...baseProps, + value: { + startDate: '2019-08-06', + endDate: '2019-08-06', + }, + }); + startDateFilterInput.instance().value = ''; + startDateFilterInput.simulate('change'); - wrapper.setProps({ - ...baseProps, - value: { + expect(onChange).toHaveBeenLastCalledWith({ endDate: '2019-08-06', - }, - }); - endDateFilterInput.instance().value = ''; - endDateFilterInput.simulate('change'); + }); - expect(onChange).toHaveBeenLastCalledWith(null); - }); + wrapper.setProps({ + ...baseProps, + value: { + endDate: '2019-08-06', + }, + }); + endDateFilterInput.instance().value = ''; + endDateFilterInput.simulate('change'); - it('handles invalid date values correctly by not calling onChange, unless there was previously a value there', () => { - const onChange = jest.fn(); + expect(onChange).toHaveBeenLastCalledWith(null); + }); - const baseProps = { - label: 'test', - onChange, - }; + it('handles invalid date values correctly by not calling onChange, unless there was previously a value there', () => { + const onChange = jest.fn(); - const wrapper = mount(); + const baseProps = { + label: 'test', + onChange, + }; - const startDateFilterInput = wrapper.find('input').first(); - startDateFilterInput.instance().value = '2'; - startDateFilterInput.simulate('change'); + const wrapper = mount(); - expect(onChange).not.toHaveBeenCalled(); + const startDateFilterInput = wrapper.find('input').first(); + startDateFilterInput.instance().value = '2'; + startDateFilterInput.simulate('change'); - const endDateFilterInput = wrapper.find('input').last(); - endDateFilterInput.instance().value = '201'; - endDateFilterInput.simulate('change'); + expect(onChange).not.toHaveBeenCalled(); - expect(onChange).not.toHaveBeenCalled(); + const endDateFilterInput = wrapper.find('input').last(); + endDateFilterInput.instance().value = '201'; + endDateFilterInput.simulate('change'); - startDateFilterInput.instance().value = '2019-08-06'; - startDateFilterInput.simulate('change'); + expect(onChange).not.toHaveBeenCalled(); - expect(onChange).toHaveBeenLastCalledWith({ - startDate: '2019-08-06', - }); + startDateFilterInput.instance().value = '2019-08-06'; + startDateFilterInput.simulate('change'); - wrapper.setProps({ ...baseProps, value: { startDate: '2019-08-06' } }); - endDateFilterInput.instance().value = '2019-08-07'; - endDateFilterInput.simulate('change'); + expect(onChange).toHaveBeenLastCalledWith({ + startDate: '2019-08-06', + }); - expect(onChange).toHaveBeenLastCalledWith({ - startDate: '2019-08-06', - endDate: '2019-08-07', - }); + wrapper.setProps({ ...baseProps, value: { startDate: '2019-08-06' } }); + endDateFilterInput.instance().value = '2019-08-07'; + endDateFilterInput.simulate('change'); - wrapper.setProps({ - ...baseProps, - value: { + expect(onChange).toHaveBeenLastCalledWith({ startDate: '2019-08-06', endDate: '2019-08-07', - }, + }); + + wrapper.setProps({ + ...baseProps, + value: { + startDate: '2019-08-06', + endDate: '2019-08-07', + }, + }); + startDateFilterInput.instance().value = '2'; + startDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith({ + endDate: '2019-08-07', + }); + + wrapper.setProps({ + ...baseProps, + value: { + endDate: '2019-08-07', + }, + }); + endDateFilterInput.instance().value = '201'; + endDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith(null); }); - startDateFilterInput.instance().value = '2'; - startDateFilterInput.simulate('change'); - expect(onChange).toHaveBeenLastCalledWith({ - endDate: '2019-08-07', + it('displays error for invalid date', () => { + const onChange = jest.fn(); + + const baseProps = { + label: 'test', + onChange, + value: { + startDate: '2019-13-09', + endDate: '2019-08-32', + }, + }; + + const wrapper = mount(); + + expect(wrapper.find('p.Mui-error')).toHaveLength(2); + expect(wrapper.find('p.Mui-error').first().text()).toEqual( + 'Date format: yyyy-MM-dd.' + ); }); - wrapper.setProps({ - ...baseProps, - value: { - endDate: '2019-08-07', - }, + it('displays error for invalid date range', () => { + const onChange = jest.fn(); + + const baseProps = { + label: 'test', + onChange, + value: { + startDate: '2019-08-09', + endDate: '2019-08-08', + }, + }; + + const wrapper = mount(); + + expect(wrapper.find('p.Mui-error')).toHaveLength(2); + expect(wrapper.find('p.Mui-error').first().text()).toEqual( + 'Invalid date range' + ); }); - endDateFilterInput.instance().value = '201'; - endDateFilterInput.simulate('change'); - expect(onChange).toHaveBeenLastCalledWith(null); - }); + it('useTextFilter hook returns a function which can generate a working text filter', () => { + const pushFilter = jest.fn(); + (usePushFilter as jest.Mock).mockImplementation(() => pushFilter); + + const { result } = renderHook(() => useDateFilter({})); + let dateFilter; - it('displays error for invalid date', () => { - const onChange = jest.fn(); + act(() => { + dateFilter = result.current('Start Date', 'startDate'); + }); - const baseProps = { - label: 'test', - onChange, - value: { - startDate: '2019-13-09', - endDate: '2019-08-32', - }, - }; + const shallowWrapper = shallow(dateFilter); + expect(shallowWrapper).toMatchSnapshot(); - const wrapper = mount(); + const mountWrapper = mount(dateFilter); + const startDateFilterInput = mountWrapper.find('input').first(); + startDateFilterInput.instance().value = '2021-08-09'; + startDateFilterInput.simulate('change'); - expect(wrapper.find('p.Mui-error')).toHaveLength(2); - expect(wrapper.find('p.Mui-error').first().text()).toEqual( - 'Date format: yyyy-MM-dd.' - ); + expect(pushFilter).toHaveBeenLastCalledWith('startDate', { + startDate: '2021-08-09', + }); + + mountWrapper.setProps({ + ...mountWrapper.props(), + value: { startDate: '2021-08-09' }, + }); + startDateFilterInput.instance().value = ''; + startDateFilterInput.simulate('change'); + + expect(pushFilter).toHaveBeenCalledTimes(2); + expect(pushFilter).toHaveBeenLastCalledWith('startDate', null); + }); }); - it('displays error for invalid date range', () => { - const onChange = jest.fn(); + describe('DateTimePicker functionality', () => { + it('calls the onChange method correctly when filling out the date/time inputs', () => { + const onChange = jest.fn(); - const baseProps = { - label: 'test', - onChange, - value: { - startDate: '2019-08-09', - endDate: '2019-08-08', - }, - }; + const baseProps = { + label: 'test', + onChange, + }; - const wrapper = mount(); + const wrapper = mount(); - expect(wrapper.find('p.Mui-error')).toHaveLength(2); - expect(wrapper.find('p.Mui-error').first().text()).toEqual( - 'Invalid date range' - ); - }); + const startDateFilterInput = wrapper.find('input').first(); + startDateFilterInput.instance().value = '2019-08-06 00:00'; + startDateFilterInput.simulate('change'); - it('useTextFilter hook returns a function which can generate a working text filter', () => { - const pushFilter = jest.fn(); - (usePushFilter as jest.Mock).mockImplementation(() => pushFilter); + expect(onChange).toHaveBeenLastCalledWith({ + startDate: '2019-08-06 00:00', + }); - const { result } = renderHook(() => useDateFilter({})); - let dateFilter; + wrapper.setProps({ + ...baseProps, + value: { startDate: '2019-08-06 00:00' }, + }); + const endDateFilterInput = wrapper.find('input').last(); + endDateFilterInput.instance().value = '2019-08-06 23:59'; + endDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith({ + startDate: '2019-08-06 00:00', + endDate: '2019-08-06 23:59', + }); + + wrapper.setProps({ + ...baseProps, + value: { + startDate: '2019-08-06 00:00', + endDate: '2019-08-06 23:59', + }, + }); + startDateFilterInput.instance().value = ''; + startDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith({ + endDate: '2019-08-06 23:59', + }); + + wrapper.setProps({ + ...baseProps, + value: { + endDate: '2019-08-06 23:59', + }, + }); + endDateFilterInput.instance().value = ''; + endDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith(null); + }); + + it('handles invalid date values correctly by not calling onChange, unless there was previously a value there', () => { + const onChange = jest.fn(); + + const baseProps = { + label: 'test', + onChange, + }; + + const wrapper = mount(); + + const startDateFilterInput = wrapper.find('input').first(); + startDateFilterInput.instance().value = '2'; + startDateFilterInput.simulate('change'); + + expect(onChange).not.toHaveBeenCalled(); + + const endDateFilterInput = wrapper.find('input').last(); + endDateFilterInput.instance().value = '201'; + endDateFilterInput.simulate('change'); + + expect(onChange).not.toHaveBeenCalled(); + + startDateFilterInput.instance().value = '2019-08-06 00:00'; + startDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith({ + startDate: '2019-08-06 00:00', + }); + + wrapper.setProps({ + ...baseProps, + value: { startDate: '2019-08-06 00:00' }, + }); + endDateFilterInput.instance().value = '2019-08-07 00:00'; + endDateFilterInput.simulate('change'); - act(() => { - dateFilter = result.current('Start Date', 'startDate'); + expect(onChange).toHaveBeenLastCalledWith({ + startDate: '2019-08-06 00:00', + endDate: '2019-08-07 00:00', + }); + + wrapper.setProps({ + ...baseProps, + value: { + startDate: '2019-08-06 00:00', + endDate: '2019-08-07 00:00', + }, + }); + startDateFilterInput.instance().value = '2'; + startDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith({ + endDate: '2019-08-07 00:00', + }); + + wrapper.setProps({ + ...baseProps, + value: { + endDate: '2019-08-07 00:00', + }, + }); + endDateFilterInput.instance().value = '201'; + endDateFilterInput.simulate('change'); + + expect(onChange).toHaveBeenLastCalledWith(null); }); - const shallowWrapper = shallow(dateFilter); - expect(shallowWrapper).toMatchSnapshot(); + it('displays error for invalid date', () => { + const onChange = jest.fn(); + + const baseProps = { + label: 'test', + onChange, + value: { + startDate: '2019-13-09 00:00', + endDate: '2019-08-32 00:00', + }, + }; + + const wrapper = mount(); + + expect(wrapper.find('p.Mui-error')).toHaveLength(2); + expect(wrapper.find('p.Mui-error').first().text()).toEqual( + 'Date format: yyyy-MM-dd HH:mm.' + ); + }); - const mountWrapper = mount(dateFilter); - const startDateFilterInput = mountWrapper.find('input').first(); - startDateFilterInput.instance().value = '2021-08-09'; - startDateFilterInput.simulate('change'); + it('displays error for invalid time', () => { + const onChange = jest.fn(); - expect(pushFilter).toHaveBeenLastCalledWith('startDate', { - startDate: '2021-08-09', + const baseProps = { + label: 'test', + onChange, + value: { + startDate: '2019-13-09 00:60', + endDate: '2019-08-32 24:00', + }, + }; + + const wrapper = mount(); + + expect(wrapper.find('p.Mui-error')).toHaveLength(2); + expect(wrapper.find('p.Mui-error').first().text()).toEqual( + 'Date format: yyyy-MM-dd HH:mm.' + ); }); - mountWrapper.setProps({ - ...mountWrapper.props(), - value: { startDate: '2021-08-09' }, + it('displays error for invalid date/time range', () => { + const onChange = jest.fn(); + + const baseProps = { + label: 'test', + onChange, + value: { + startDate: '2019-08-08 12:00', + endDate: '2019-08-08 11:00', + }, + }; + + const wrapper = mount(); + + expect(wrapper.find('p.Mui-error')).toHaveLength(2); + expect(wrapper.find('p.Mui-error').first().text()).toEqual( + 'Invalid date/time range' + ); }); - startDateFilterInput.instance().value = ''; - startDateFilterInput.simulate('change'); - expect(pushFilter).toHaveBeenCalledTimes(2); - expect(pushFilter).toHaveBeenLastCalledWith('startDate', null); + // I don't believe this works with date+time due to time values not appearing in URL params + // TODO remove this? + it.skip('useTextFilter hook returns a function which can generate a working text filter', () => { + const pushFilter = jest.fn(); + (usePushFilter as jest.Mock).mockImplementation(() => pushFilter); + + const { result } = renderHook(() => useDateFilter({})); + let dateFilter; + + act(() => { + dateFilter = result.current('Start Date', 'startDate'); + }); + + const shallowWrapper = shallow(dateFilter); + expect(shallowWrapper).toMatchSnapshot(); + + const mountWrapper = mount(dateFilter); + const startDateFilterInput = mountWrapper.find('input').first(); + startDateFilterInput.instance().value = '2021-08-09 00:00'; + startDateFilterInput.simulate('change'); + + expect(pushFilter).toHaveBeenLastCalledWith('startDate', { + startDate: '2021-08-09 00:00', + }); + + mountWrapper.setProps({ + ...mountWrapper.props(), + value: { startDate: '2021-08-09 00:00' }, + }); + startDateFilterInput.instance().value = ''; + startDateFilterInput.simulate('change'); + + expect(pushFilter).toHaveBeenCalledTimes(2); + expect(pushFilter).toHaveBeenLastCalledWith('startDate', null); + }); }); }); From 1b597390f07d13f93989ad3d20d2e9af918f62e4 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Fri, 8 Apr 2022 16:03:59 +0100 Subject: [PATCH 15/62] Remove useDeepCompareEffect library from download --- packages/datagateway-download/package.json | 3 +-- packages/datagateway-download/src/downloadApiHooks.ts | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/datagateway-download/package.json b/packages/datagateway-download/package.json index 893293aad..d517cb52f 100644 --- a/packages/datagateway-download/package.json +++ b/packages/datagateway-download/package.json @@ -29,8 +29,7 @@ "react-virtualized": "^9.22.3", "single-spa-react": "^4.3.1", "tslib": "^2.3.0", - "typescript": "4.5.3", - "use-deep-compare-effect": "^1.8.1" + "typescript": "4.5.3" }, "devDependencies": { "@craco/craco": "^6.4.3", diff --git a/packages/datagateway-download/src/downloadApiHooks.ts b/packages/datagateway-download/src/downloadApiHooks.ts index 3beb29aed..e191516e6 100644 --- a/packages/datagateway-download/src/downloadApiHooks.ts +++ b/packages/datagateway-download/src/downloadApiHooks.ts @@ -17,7 +17,6 @@ import { useQueries, } from 'react-query'; import pLimit from 'p-limit'; -import useDeepCompareEffect from 'use-deep-compare-effect'; import { removeAllDownloadCartItems, getSize, From 1947ef661661c9e140a1f735798da03ac6407135 Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Wed, 13 Apr 2022 16:35:31 +0100 Subject: [PATCH 16/62] Unifying common date picker props #1175 Increases number of lines so worth deciding if this is beneficial or not --- .../dateColumnFilter.component.tsx | 66 ++++++++++--------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx index c3039d75d..863309b53 100644 --- a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx +++ b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { ReactElement, useState } from 'react'; import DateFnsUtils from '@date-io/date-fns'; import { format, isValid, isEqual } from 'date-fns'; import { @@ -74,6 +74,19 @@ interface DateColumnFilterProps { filterByTime?: boolean; } +interface DatePickerCommonProps { + clearable: boolean; + allowKeyboardControl: boolean; + invalidDateMessage: string; + ariaHidden: boolean; + format: string; + color: 'secondary' | 'primary' | undefined; + strictCompareDates?: boolean; + okLabel: ReactElement; + cancelLabel: ReactElement; + clearLabel: ReactElement; +} + const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { //Need state to change otherwise wont update error messages for an invalid date const [startDate, setStartDate] = useState( @@ -88,29 +101,43 @@ const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const buttonColour = (theme as any).colours?.blue; + const datePickerProps: DatePickerCommonProps = { + clearable: true, + allowKeyboardControl: true, + invalidDateMessage: 'Date format: yyyy-MM-dd.', + ariaHidden: true, + format: 'yyyy-MM-dd', + color: 'secondary', + okLabel: OK, + cancelLabel: Cancel, + clearLabel: Clear, + }; + + const dateTimePickerProps: DatePickerCommonProps = { + ...datePickerProps, + invalidDateMessage: 'Date format: yyyy-MM-dd HH:mm.', + format: 'yyyy-MM-dd HH:mm', + strictCompareDates: true, + }; + return (
{props.filterByTime ? ( ) : ( )} diff --git a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx index 7b06d4095..3282e116b 100644 --- a/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx +++ b/packages/datagateway-download/src/downloadStatus/adminDownloadStatusTable.component.tsx @@ -89,12 +89,8 @@ const AdminDownloadStatusTable: React.FC = () => { if (typeof filter === 'object') { if (!Array.isArray(filter)) { if ('startDate' in filter || 'endDate' in filter) { - const startDate = filter.startDate - ? filter.startDate - : '0000-01-01 00:00'; - const endDate = filter.endDate - ? filter.endDate - : '9999-12-31 23:59'; + const startDate = filter.startDate ?? '0000-01-01 00:00'; + const endDate = filter.endDate ?? '9999-12-31 23:59'; queryOffset += ` AND UPPER(download.${column}) BETWEEN {ts '${startDate}'} AND {ts '${endDate}'}`; } diff --git a/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.test.tsx b/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.test.tsx index 57b35e2b9..c5aec9b8e 100644 --- a/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.test.tsx +++ b/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.test.tsx @@ -461,7 +461,7 @@ describe('Download Status Table', () => { 'input[id="downloadStatus.createdAt filter from"]' ); - dateFromFilterInput.instance().value = '2020-01-01'; + dateFromFilterInput.instance().value = '2020-01-01 00:00'; dateFromFilterInput.simulate('change'); expect(wrapper.exists('[aria-rowcount=5]')).toBe(true); @@ -470,14 +470,14 @@ describe('Download Status Table', () => { 'input[id="downloadStatus.createdAt filter to"]' ); - dateToFilterInput.instance().value = '2020-01-02'; + dateToFilterInput.instance().value = '2020-01-02 23:59'; dateToFilterInput.simulate('change'); expect(wrapper.exists('[aria-rowcount=0]')).toBe(true); - dateFromFilterInput.instance().value = '2020-02-26'; + dateFromFilterInput.instance().value = '2020-02-26 00:00'; dateFromFilterInput.simulate('change'); - dateToFilterInput.instance().value = '2020-02-27'; + dateToFilterInput.instance().value = '2020-02-27 23:59'; dateToFilterInput.simulate('change'); expect(wrapper.exists('[aria-rowcount=2]')).toBe(true); diff --git a/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.tsx b/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.tsx index cf15be54c..43409976e 100644 --- a/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.tsx +++ b/packages/datagateway-download/src/downloadStatus/downloadStatusTable.component.tsx @@ -158,6 +158,7 @@ const DownloadStatusTable: React.FC = ( } }} value={filters[dataKey] as DateFilter} + filterByTime /> ); @@ -186,7 +187,7 @@ const DownloadStatusTable: React.FC = ( const tableTimestamp = toDate(tableValue).getTime(); const startTimestamp = toDate(value.startDate).getTime(); const endTimestamp = value.endDate - ? new Date(`${value.endDate} 23:59:59`).getTime() + ? new Date(value.endDate).getTime() : Date.now(); if ( From ae752f790c16df5c1dc40b3e4626435b6b841635 Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Thu, 14 Apr 2022 11:36:28 +0100 Subject: [PATCH 18/62] Fixing e2e test to factor in time #1175 --- .../cypress/integration/downloadStatus.spec.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/datagateway-download/cypress/integration/downloadStatus.spec.ts b/packages/datagateway-download/cypress/integration/downloadStatus.spec.ts index c5f98bf31..f092ba63a 100644 --- a/packages/datagateway-download/cypress/integration/downloadStatus.spec.ts +++ b/packages/datagateway-download/cypress/integration/downloadStatus.spec.ts @@ -1,3 +1,5 @@ +import { format } from 'date-fns-tz'; + describe('Download Status', () => { before(() => { // Ensure the downloads are cleared before running tests. @@ -129,7 +131,7 @@ describe('Download Status', () => { }); it('date between', () => { - cy.get('input[id="Requested Date filter from"]').type('2020-01-31'); + cy.get('input[id="Requested Date filter from"]').type('2020-01-31 00:00'); const date = new Date(); const month = date.toLocaleString('default', { month: 'long' }); @@ -156,7 +158,7 @@ describe('Download Status', () => { cy.get('input[id="Requested Date filter to"]').should( 'have.value', - date.toISOString().slice(0, 10) + format(date, 'yyyy-MM-dd HH:mm') ); // There should not be results for this time period. @@ -169,7 +171,7 @@ describe('Download Status', () => { cy.get('[aria-rowcount="4"]').should('exist'); cy.get('input[id="Requested Date filter from"]').type( - currDate.toISOString().slice(0, 10) + format(currDate, 'yyyy-MM-dd HH:mm') ); cy.get('[aria-rowindex="1"] [aria-colindex="1"]').should( From 9c1ead4a8c698835e361ff446246422d61e70659 Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Thu, 14 Apr 2022 15:08:32 +0100 Subject: [PATCH 19/62] Fixing unit tests in download #1215 --- .../downloadCartTable.component.test.tsx | 2 +- .../downloadTab/downloadTab.component.test.tsx | 16 ++-------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx index d8581e8fd..d69bc39aa 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx @@ -207,7 +207,7 @@ describe('Download cart table component', () => { '7' ); expect(wrapper.find('p#fileCountDisplay').text()).toEqual( - expect.stringContaining('downloadCart.number_of_files: 28') + expect.stringContaining('downloadCart.number_of_files: 22 / 5000') ); }); diff --git a/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx b/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx index 07b4ed912..9d8e16c2a 100644 --- a/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx +++ b/packages/datagateway-download/src/downloadTab/downloadTab.component.test.tsx @@ -138,13 +138,7 @@ describe('DownloadTab', () => { }); it('renders the selections tab on each mount', async () => { - let wrapper = mount( - - - - - - ); + let wrapper = createWrapper(); await act(async () => { await flushPromises(); @@ -162,13 +156,7 @@ describe('DownloadTab', () => { }); // Recreate the wrapper and expect it to show the selections tab. - wrapper = mount( - - - - - - ); + wrapper = createWrapper(); await act(async () => { await flushPromises(); From 1fc8737f5ebb465857c030282359e9c0c319950a Mon Sep 17 00:00:00 2001 From: Sam Glendenning Date: Thu, 14 Apr 2022 16:05:54 +0100 Subject: [PATCH 20/62] Removing change to downloadCartTable test #1215 Incorrectly "fixed" the test, there is still something going wrong here --- .../src/downloadCart/downloadCartTable.component.test.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx index d69bc39aa..aec179f7d 100644 --- a/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx +++ b/packages/datagateway-download/src/downloadCart/downloadCartTable.component.test.tsx @@ -206,8 +206,10 @@ describe('Download cart table component', () => { expect(wrapper.find('[aria-colindex=4]').find('p').first().text()).toEqual( '7' ); + + // TODO fix this. It seems getDatafileCount is not being called if entityType === 'datafile' expect(wrapper.find('p#fileCountDisplay').text()).toEqual( - expect.stringContaining('downloadCart.number_of_files: 22 / 5000') + expect.stringContaining('downloadCart.number_of_files: 28 / 5000') ); }); From cb7890fe4216a4d40c44714867a459deb8bb6237 Mon Sep 17 00:00:00 2001 From: Louise Davies Date: Tue, 19 Apr 2022 10:16:54 +0100 Subject: [PATCH 21/62] #1175 - use TypeScript Partial & mui-picker types to type datepicker common props - also extract more common props --- .../dateColumnFilter.component.tsx | 41 +++++-------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx index 6c2fda49c..ae6cac9e8 100644 --- a/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx +++ b/packages/datagateway-common/src/table/columnFilters/dateColumnFilter.component.tsx @@ -1,10 +1,12 @@ -import React, { ReactElement, useState } from 'react'; +import React, { useState } from 'react'; import DateFnsUtils from '@date-io/date-fns'; import { format, isValid, isEqual } from 'date-fns'; import { KeyboardDatePicker, KeyboardDateTimePicker, MuiPickersUtilsProvider, + KeyboardDatePickerProps, + KeyboardDateTimePickerProps, } from '@material-ui/pickers'; import { MaterialUiPickersDate } from '@material-ui/pickers/typings/date'; import { FiltersType, DateFilter } from '../../app.types'; @@ -74,18 +76,6 @@ interface DateColumnFilterProps { filterByTime?: boolean; } -interface DatePickerCommonProps { - clearable: boolean; - allowKeyboardControl: boolean; - invalidDateMessage: string; - format: string; - color: 'secondary' | 'primary' | undefined; - strictCompareDates?: boolean; - okLabel: ReactElement; - cancelLabel: ReactElement; - clearLabel: ReactElement; -} - const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { //Need state to change otherwise wont update error messages for an invalid date const [startDate, setStartDate] = useState( @@ -100,7 +90,7 @@ const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const buttonColour = (theme as any).colours?.blue; - const datePickerProps: DatePickerCommonProps = { + const datePickerProps: Partial = { clearable: true, allowKeyboardControl: true, invalidDateMessage: 'Date format: yyyy-MM-dd.', @@ -109,13 +99,18 @@ const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => { okLabel: OK, cancelLabel: Cancel, clearLabel: Clear, + style: { whiteSpace: 'nowrap' }, + 'aria-hidden': 'true', + inputProps: { 'aria-label': `${props.label} filter` }, + views: ['year', 'month', 'date'], }; - const dateTimePickerProps: DatePickerCommonProps = { + const dateTimePickerProps: Partial = { ...datePickerProps, invalidDateMessage: 'Date format: yyyy-MM-dd HH:mm.', format: 'yyyy-MM-dd HH:mm', strictCompareDates: true, + views: ['year', 'month', 'date', 'hours', 'minutes'], }; return ( @@ -124,17 +119,13 @@ const DateColumnFilter = (props: DateColumnFilterProps): React.ReactElement => {