Skip to content

Commit

Permalink
Merge pull request #1231 from ral-facilities/bugfix/fix-selection-err…
Browse files Browse the repository at this point in the history 
…ors-#1210

Fix selection errors #1210
  • Loading branch information
@louise-davies
louise-davies authored Jun 21, 2022
2 parents 400305b + 35e91e7 commit e3af540
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 70 deletions.
53 changes: 34 additions & 19 deletions packages/datagateway-common/src/api/cart.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,15 @@ describe('Cart api functions', () => {
expect(result.current.data).toEqual(mockData.cartItems);
});

it('sends axios request to add item to cart once mutate function is called and calls handleICATError on failure', async () => {
(axios.post as jest.Mock).mockRejectedValue({
message: 'Test error message',
});
it('sends axios request to add item to cart once mutate function is called and calls handleICATError on failure, with a retry on code 431', async () => {
(axios.post as jest.MockedFunction<typeof axios.post>)
.mockRejectedValueOnce({
code: '431',
message: 'Test 431 error message',
})
.mockRejectedValue({
message: 'Test error message',
});

const { result, waitFor } = renderHook(() => useAddToCart('dataset'), {
wrapper: createReactQueryWrapper(),
Expand All @@ -133,8 +138,10 @@ describe('Cart api functions', () => {

result.current.mutate([1, 2]);

await waitFor(() => result.current.isError);
await waitFor(() => result.current.isError, { timeout: 2000 });

expect(result.current.failureCount).toBe(2);
expect(handleICATError).toHaveBeenCalledTimes(1);
expect(handleICATError).toHaveBeenCalledWith({
message: 'Test error message',
});
Expand All @@ -143,7 +150,7 @@ describe('Cart api functions', () => {

describe('useRemoveFromCart', () => {
it('sends axios request to remove item from cart once mutate function is called and returns successful response', async () => {
(axios.delete as jest.Mock).mockResolvedValue({
(axios.post as jest.Mock).mockResolvedValue({
data: mockData,
});

Expand All @@ -161,22 +168,28 @@ describe('Cart api functions', () => {

await waitFor(() => result.current.isSuccess);

expect(axios.delete).toHaveBeenCalledWith(
const params = new URLSearchParams();
params.append('sessionId', '');
params.append('items', 'dataset 1, dataset 2');
params.append('remove', 'true');

expect(axios.post).toHaveBeenCalledWith(
'https://example.com/topcat/user/cart/TEST/cartItems',
{
params: {
sessionId: null,
items: 'dataset 1, dataset 2',
},
}

params
);
expect(result.current.data).toEqual(mockData.cartItems);
});

it('sends axios request to remove item from cart once mutate function is called and calls handleICATError on failure', async () => {
(axios.delete as jest.Mock).mockRejectedValue({
message: 'Test error message',
});
it('sends axios request to remove item from cart once mutate function is called and calls handleICATError on failure, with a retry on code 431', async () => {
(axios.post as jest.MockedFunction<typeof axios.post>)
.mockRejectedValueOnce({
code: '431',
message: 'Test 431 error message',
})
.mockRejectedValue({
message: 'Test error message',
});

const { result, waitFor } = renderHook(
() => useRemoveFromCart('dataset'),
Expand All @@ -185,13 +198,15 @@ describe('Cart api functions', () => {
}
);

expect(axios.delete).not.toHaveBeenCalled();
expect(axios.post).not.toHaveBeenCalled();
expect(result.current.isIdle).toBe(true);

result.current.mutate([1, 2]);

await waitFor(() => result.current.isError);
await waitFor(() => result.current.isError, { timeout: 2000 });

expect(result.current.failureCount).toBe(2);
expect(handleICATError).toHaveBeenCalledTimes(1);
expect(handleICATError).toHaveBeenCalledWith({
message: 'Test error message',
});
Expand Down
61 changes: 32 additions & 29 deletions packages/datagateway-common/src/api/cart.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,19 @@ export const fetchDownloadCart = (config: {
.then((response) => response.data.cartItems);
};

const addToCart = (
const addOrRemoveFromCart = (
entityType: 'investigation' | 'dataset' | 'datafile',
entityIds: number[],
config: { facilityName: string; downloadApiUrl: string }
config: { facilityName: string; downloadApiUrl: string },
remove?: boolean
): Promise<DownloadCartItem[]> => {
const { facilityName, downloadApiUrl } = config;
const params = new URLSearchParams();
params.append('sessionId', readSciGatewayToken().sessionId || '');
params.append('items', `${entityType} ${entityIds.join(`, ${entityType} `)}`);
if (typeof remove !== 'undefined') {
params.append('remove', remove.toString());
}

return axios
.post<DownloadCart>(
Expand All @@ -46,26 +50,6 @@ const addToCart = (
.then((response) => response.data.cartItems);
};

export const removeFromCart = (
entityType: 'investigation' | 'dataset' | 'datafile',
entityIds: number[],
config: { facilityName: string; downloadApiUrl: string }
): Promise<DownloadCartItem[]> => {
const { facilityName, downloadApiUrl } = config;

return axios
.delete<DownloadCart>(
`${downloadApiUrl}/user/cart/${facilityName}/cartItems`,
{
params: {
sessionId: readSciGatewayToken().sessionId,
items: `${entityType} ${entityIds.join(`, ${entityType} `)}`,
},
}
)
.then((response) => response.data.cartItems);
};

export const useCart = (): UseQueryResult<DownloadCartItem[], AxiosError> => {
const downloadApiUrl = useSelector(
(state: StateType) => state.dgcommon.urls.downloadApiUrl
Expand Down Expand Up @@ -106,18 +90,25 @@ export const useAddToCart = (

return useMutation(
(entityIds: number[]) =>
addToCart(entityType, entityIds, {
addOrRemoveFromCart(entityType, entityIds, {
facilityName,
downloadApiUrl,
}),
{
onSuccess: (data) => {
queryClient.setQueryData('cart', data);
},
retry: (failureCount, error) => {
// if we get 431 we know this is an intermittent error so retry
if (error.code === '431' && failureCount < 3) {
return true;
} else {
return false;
}
},
onError: (error) => {
handleICATError(error);
},
retry: retryICATErrors,
}
);
};
Expand All @@ -135,18 +126,30 @@ export const useRemoveFromCart = (

return useMutation(
(entityIds: number[]) =>
removeFromCart(entityType, entityIds, {
facilityName,
downloadApiUrl,
}),
addOrRemoveFromCart(
entityType,
entityIds,
{
facilityName,
downloadApiUrl,
},
true
),
{
onSuccess: (data) => {
queryClient.setQueryData('cart', data);
},
retry: (failureCount, error) => {
// if we get 431 we know this is an intermittent error so retry
if (error.code === '431' && failureCount < 3) {
return true;
} else {
return false;
}
},
onError: (error) => {
handleICATError(error);
},
retry: retryICATErrors,
}
);
};
22 changes: 22 additions & 0 deletions packages/datagateway-download/src/downloadApi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
Download,
readSciGatewayToken,
handleICATError,
DownloadCart,
DownloadCartItem,
} from 'datagateway-common';

export const removeAllDownloadCartItems: (settings: {
Expand All @@ -30,6 +32,26 @@ export const removeAllDownloadCartItems: (settings: {
});
};

export const removeFromCart = (
entityType: 'investigation' | 'dataset' | 'datafile',
entityIds: number[],
config: { facilityName: string; downloadApiUrl: string }
): Promise<DownloadCartItem[]> => {
const { facilityName, downloadApiUrl } = config;

return axios
.delete<DownloadCart>(
`${downloadApiUrl}/user/cart/${facilityName}/cartItems`,
{
params: {
sessionId: readSciGatewayToken().sessionId,
items: `${entityType} ${entityIds.join(`, ${entityType} `)}`,
},
}
)
.then((response) => response.data.cartItems);
};

export const getIsTwoLevel: (settings: {
idsUrl: string;
}) => Promise<boolean> = (settings: { idsUrl: string }) => {
Expand Down
46 changes: 33 additions & 13 deletions packages/datagateway-download/src/downloadApiHooks.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,27 +182,37 @@ describe('Download Cart API react-query hooks test', () => {
);
});

it('logs error upon unsuccessful response', async () => {
axios.delete = jest.fn().mockImplementation(() =>
Promise.reject({
message: 'Test error message',
})
);
it('logs error upon unsuccessful response, with a retry on code 431', async () => {
axios.delete = jest
.fn()
.mockImplementationOnce(() =>
Promise.reject({
code: '431',
message: 'Test 431 error message',
})
)
.mockImplementation(() =>
Promise.reject({
message: 'Test error message',
})
);

const { result, waitFor } = renderHook(() => useRemoveAllFromCart(), {
wrapper: createReactQueryWrapper(),
});

result.current.mutate();

await waitFor(() => result.current.isError);
await waitFor(() => result.current.isError, { timeout: 2000 });

expect(
axios.delete
).toHaveBeenCalledWith(
`${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`,
{ params: { sessionId: null, items: '*' } }
);
expect(result.current.failureCount).toBe(2);
expect(handleICATError).toHaveBeenCalledTimes(1);
expect(handleICATError).toHaveBeenCalledWith({
message: 'Test error message',
});
Expand Down Expand Up @@ -243,26 +253,36 @@ describe('Download Cart API react-query hooks test', () => {
});

it('logs error upon unsuccessful response', async () => {
axios.delete = jest.fn().mockImplementation(() =>
Promise.reject({
message: 'Test error message',
})
);
axios.delete = jest
.fn()
.mockImplementationOnce(() =>
Promise.reject({
code: '431',
message: 'Test 431 error message',
})
)
.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);
await waitFor(() => result.current.isError, { timeout: 2000 });

expect(
axios.delete
).toHaveBeenCalledWith(
`${mockedSettings.downloadApiUrl}/user/cart/${mockedSettings.facilityName}/cartItems`,
{ params: { sessionId: null, items: 'investigation 1' } }
);
expect(result.current.failureCount).toBe(2);
expect(handleICATError).toHaveBeenCalledTimes(1);
expect(handleICATError).toHaveBeenCalledWith({
message: 'Test error message',
});
Expand Down
20 changes: 17 additions & 3 deletions packages/datagateway-download/src/downloadApiHooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
DownloadCartItem,
handleICATError,
fetchDownloadCart,
removeFromCart,
retryICATErrors,
} from 'datagateway-common';
import { DownloadSettingsContext } from './ConfigProvider';
Expand All @@ -20,6 +19,7 @@ import {
import pLimit from 'p-limit';
import {
removeAllDownloadCartItems,
removeFromCart,
getSize,
getDatafileCount,
getIsTwoLevel,
Expand Down Expand Up @@ -60,10 +60,17 @@ export const useRemoveAllFromCart = (): UseMutationResult<
onSuccess: (data) => {
queryClient.setQueryData('cart', []);
},
retry: (failureCount, error) => {
// if we get 431 we know this is an intermittent error so retry
if (error.code === '431' && failureCount < 3) {
return true;
} else {
return false;
}
},
onError: (error) => {
handleICATError(error);
},
retry: retryICATErrors,
}
);
};
Expand All @@ -87,10 +94,17 @@ export const useRemoveEntityFromCart = (): UseMutationResult<
onSuccess: (data) => {
queryClient.setQueryData('cart', data);
},
retry: (failureCount, error) => {
// if we get 431 we know this is an intermittent error so retry
if (error.code === '431' && failureCount < 3) {
return true;
} else {
return false;
}
},
onError: (error) => {
handleICATError(error);
},
retry: retryICATErrors,
}
);
};
Expand Down
Loading

0 comments on commit e3af540

Please sign in to comment.