From 65d758f52485e503435a11d997d1861a71f66f07 Mon Sep 17 00:00:00 2001 From: vashjs Date: Fri, 25 Oct 2024 16:04:45 +0200 Subject: [PATCH 1/6] UIBULKED-553 Replace generic error with the detailed error message - include bulk-operation --- .../BulkEditPreviewModalList.js | 6 ++-- src/hooks/api/useBulkOperationDetails.js | 31 ++++++++++++++----- src/hooks/useConfirmChanges.js | 2 +- src/hooks/useErrorMessages.js | 4 +-- src/hooks/useErrorMessages.test.js | 12 ------- 5 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js b/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js index f2f1d8d5..89657f30 100644 --- a/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js +++ b/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js @@ -22,8 +22,7 @@ import css from './BulkEditInAppPreviewModal.css'; import { usePagination } from '../../../../hooks/usePagination'; import { CAPABILITIES, - EDITING_STEPS, - JOB_STATUSES, + EDITING_STEPS, JOB_STATUSES, PAGINATION_CONFIG } from '../../../../constants'; import { @@ -33,6 +32,7 @@ import { import { useSearchParams } from '../../../../hooks'; import { RootContext } from '../../../../context/RootContext'; import { getVisibleColumnsKeys } from '../../../../utils/helpers'; +import { useErrorMessages } from '../../../../hooks/useErrorMessages'; export const BulkEditPreviewModalList = ({ @@ -44,6 +44,7 @@ export const BulkEditPreviewModalList = ({ const intl = useIntl(); const { visibleColumns } = useContext(RootContext); const { currentRecordType } = useSearchParams(); + const { showErrorMessage } = useErrorMessages(); const { pagination, changePage, @@ -62,6 +63,7 @@ export const BulkEditPreviewModalList = ({ capabilities: currentRecordType, queryOptions: { enabled: isPreviewEnabled && bulkDetails?.status !== JOB_STATUSES.DATA_MODIFICATION_IN_PROGRESS, + onSuccess: showErrorMessage, onError: () => { callout({ type: 'error', diff --git a/src/hooks/api/useBulkOperationDetails.js b/src/hooks/api/useBulkOperationDetails.js index 295d268c..231bb8c2 100644 --- a/src/hooks/api/useBulkOperationDetails.js +++ b/src/hooks/api/useBulkOperationDetails.js @@ -3,6 +3,8 @@ import { useQuery } from 'react-query'; import { useNamespace, useOkapiKy } from '@folio/stripes/core'; import { useHistory } from 'react-router-dom'; import { buildSearch } from '@folio/stripes-acq-components'; +import { JOB_STATUSES } from '../../constants'; +import { useErrorMessages } from '../useErrorMessages'; export const BULK_OPERATION_DETAILS_KEY = 'BULK_OPERATION_DETAILS_KEY'; @@ -16,18 +18,12 @@ export const useBulkOperationDetails = ({ const [namespaceKey] = useNamespace({ key: BULK_OPERATION_DETAILS_KEY }); const history = useHistory(); const [refetchInterval, setRefetchInterval] = useState(interval); - - const { data, isLoading } = useQuery({ - queryKey: [BULK_OPERATION_DETAILS_KEY, namespaceKey, id, refetchInterval, ...additionalQueryKeys], - enabled: !!id, - refetchInterval, - queryFn: () => ky.get(`bulk-operations/${id}`).json(), - ...options, - }); + const { showErrorMessage } = useErrorMessages(); const clearInterval = () => { setRefetchInterval(0); }; + const clearIntervalAndRedirect = (pathname, searchParams) => { clearInterval(); @@ -37,6 +33,25 @@ export const useBulkOperationDetails = ({ }); }; + const { data, isLoading } = useQuery({ + queryKey: [BULK_OPERATION_DETAILS_KEY, namespaceKey, id, refetchInterval, ...additionalQueryKeys], + enabled: !!id, + onError: (error) => { + showErrorMessage(error); + clearIntervalAndRedirect('/bulk-edit', ''); + }, + onSuccess: (response) => { + showErrorMessage(response); + + if (response.status === JOB_STATUSES.FAILED || response?.errorMessage) { + clearIntervalAndRedirect('/bulk-edit', ''); + } + }, + refetchInterval, + queryFn: () => ky.get(`bulk-operations/${id}`).json(), + ...options, + }); + return { bulkDetails: data, isLoading, diff --git a/src/hooks/useConfirmChanges.js b/src/hooks/useConfirmChanges.js index a65e0a6d..44242c09 100644 --- a/src/hooks/useConfirmChanges.js +++ b/src/hooks/useConfirmChanges.js @@ -33,7 +33,7 @@ export const useConfirmChanges = ({ const [isPreviewModalOpened, setIsPreviewModalOpened] = useState(false); const [isPreviewLoading, setIsPreviewLoading] = useState(false); - const { bulkDetails } = useBulkOperationDetails({ id: bulkOperationId, interval: 1000 * 3 }); + const { bulkDetails } = useBulkOperationDetails({ id: bulkOperationId, interval: 0 }); const { bulkOperationStart } = useBulkOperationStart(); const totalRecords = bulkDetails?.totalNumOfRecords; diff --git a/src/hooks/useErrorMessages.js b/src/hooks/useErrorMessages.js index d16582fe..2cbb19b6 100644 --- a/src/hooks/useErrorMessages.js +++ b/src/hooks/useErrorMessages.js @@ -14,9 +14,7 @@ export const useErrorMessages = () => { }; const showErrorMessage = (res) => { - const messageInBody = res?.errorMessage; - const messageWhenError = res?.message; - const message = messageInBody || messageWhenError; + const message = res?.errorMessage; // check if error message should be translated (if it's exist in translations) const prefixedMessageId = `ui-bulk-edit.${message}`; diff --git a/src/hooks/useErrorMessages.test.js b/src/hooks/useErrorMessages.test.js index 1cbd16ef..ff084091 100644 --- a/src/hooks/useErrorMessages.test.js +++ b/src/hooks/useErrorMessages.test.js @@ -56,18 +56,6 @@ describe('useErrorMessages', () => { expect(formatMessageMock).not.toHaveBeenCalled(); }); - it('should show the message from the response when no error message in body is available', () => { - const { result } = renderHook(() => useErrorMessages()); - const { showErrorMessage } = result.current; - - showErrorMessage({ message: 'Another error occurred' }); - - expect(showCalloutMock).toHaveBeenCalledWith({ - type: 'error', - message: 'Another error occurred', - }); - }); - it('should show the default error message when the response is an error object without a message', () => { const { result } = renderHook(() => useErrorMessages()); const { showErrorMessage } = result.current; From 17fe273a78d356fdb4ec5d0543fb430b2c836a57 Mon Sep 17 00:00:00 2001 From: vashjs Date: Fri, 25 Oct 2024 16:11:04 +0200 Subject: [PATCH 2/6] update --- .../BulkEditInAppPreviewModal/BulkEditPreviewModalList.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js b/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js index 89657f30..d84f124c 100644 --- a/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js +++ b/src/components/BulkEditPane/BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalList.js @@ -22,7 +22,8 @@ import css from './BulkEditInAppPreviewModal.css'; import { usePagination } from '../../../../hooks/usePagination'; import { CAPABILITIES, - EDITING_STEPS, JOB_STATUSES, + EDITING_STEPS, + JOB_STATUSES, PAGINATION_CONFIG } from '../../../../constants'; import { From b6f1462378d41960a8daabeda40d7d7ca8381f02 Mon Sep 17 00:00:00 2001 From: vashjs Date: Fri, 25 Oct 2024 16:15:25 +0200 Subject: [PATCH 3/6] update --- src/hooks/api/useBulkOperationDetails.js | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/hooks/api/useBulkOperationDetails.js b/src/hooks/api/useBulkOperationDetails.js index 231bb8c2..314df768 100644 --- a/src/hooks/api/useBulkOperationDetails.js +++ b/src/hooks/api/useBulkOperationDetails.js @@ -36,19 +36,25 @@ export const useBulkOperationDetails = ({ const { data, isLoading } = useQuery({ queryKey: [BULK_OPERATION_DETAILS_KEY, namespaceKey, id, refetchInterval, ...additionalQueryKeys], enabled: !!id, - onError: (error) => { - showErrorMessage(error); - clearIntervalAndRedirect('/bulk-edit', ''); - }, - onSuccess: (response) => { - showErrorMessage(response); + refetchInterval, + queryFn: async () => { + try { + const response = await ky.get(`bulk-operations/${id}`).json(); + + showErrorMessage(response); - if (response.status === JOB_STATUSES.FAILED || response?.errorMessage) { + if (response.status === JOB_STATUSES.FAILED || response?.errorMessage) { + clearIntervalAndRedirect('/bulk-edit', ''); + } + + return response; + } catch (e) { + showErrorMessage(e); clearIntervalAndRedirect('/bulk-edit', ''); + + return e; } }, - refetchInterval, - queryFn: () => ky.get(`bulk-operations/${id}`).json(), ...options, }); From b0d0f0cc09f6e0bf8e979878a8821d526e083fb9 Mon Sep 17 00:00:00 2001 From: vashjs Date: Fri, 25 Oct 2024 16:48:05 +0200 Subject: [PATCH 4/6] update --- src/hooks/useConfirmChanges.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useConfirmChanges.js b/src/hooks/useConfirmChanges.js index 44242c09..a65e0a6d 100644 --- a/src/hooks/useConfirmChanges.js +++ b/src/hooks/useConfirmChanges.js @@ -33,7 +33,7 @@ export const useConfirmChanges = ({ const [isPreviewModalOpened, setIsPreviewModalOpened] = useState(false); const [isPreviewLoading, setIsPreviewLoading] = useState(false); - const { bulkDetails } = useBulkOperationDetails({ id: bulkOperationId, interval: 0 }); + const { bulkDetails } = useBulkOperationDetails({ id: bulkOperationId, interval: 1000 * 3 }); const { bulkOperationStart } = useBulkOperationStart(); const totalRecords = bulkDetails?.totalNumOfRecords; From 5510d94822b4e69915ed1e6ce98257830107b48d Mon Sep 17 00:00:00 2001 From: vashjs Date: Fri, 25 Oct 2024 17:02:07 +0200 Subject: [PATCH 5/6] update --- src/hooks/api/useBulkOperationDetails.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/api/useBulkOperationDetails.js b/src/hooks/api/useBulkOperationDetails.js index 314df768..35ff3628 100644 --- a/src/hooks/api/useBulkOperationDetails.js +++ b/src/hooks/api/useBulkOperationDetails.js @@ -44,13 +44,13 @@ export const useBulkOperationDetails = ({ showErrorMessage(response); if (response.status === JOB_STATUSES.FAILED || response?.errorMessage) { - clearIntervalAndRedirect('/bulk-edit', ''); + clearInterval(); } return response; } catch (e) { showErrorMessage(e); - clearIntervalAndRedirect('/bulk-edit', ''); + clearInterval(); return e; } From 2620c5e5f39e61d3a11961f896bb9368644b4c0a Mon Sep 17 00:00:00 2001 From: vashjs Date: Fri, 25 Oct 2024 17:12:14 +0200 Subject: [PATCH 6/6] add simple tests --- src/hooks/api/useBulkOperationDetails.test.js | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 src/hooks/api/useBulkOperationDetails.test.js diff --git a/src/hooks/api/useBulkOperationDetails.test.js b/src/hooks/api/useBulkOperationDetails.test.js new file mode 100644 index 00000000..e4df975b --- /dev/null +++ b/src/hooks/api/useBulkOperationDetails.test.js @@ -0,0 +1,82 @@ +import { renderHook, act } from '@testing-library/react-hooks'; +import { useQuery } from 'react-query'; +import { useNamespace, useOkapiKy } from '@folio/stripes/core'; +import { useHistory } from 'react-router-dom'; +import { useBulkOperationDetails, BULK_OPERATION_DETAILS_KEY } from './useBulkOperationDetails'; +import { useErrorMessages } from '../useErrorMessages'; + + +jest.mock('@folio/stripes-acq-components', () => ({ + buildSearch: jest.fn(), +})); + +jest.mock('react-query', () => ({ + useQuery: jest.fn(), +})); + +jest.mock('@folio/stripes/core', () => ({ + useNamespace: jest.fn(), + useOkapiKy: jest.fn(), +})); + +jest.mock('react-router-dom', () => ({ + useHistory: jest.fn(), +})); + +jest.mock('../useErrorMessages', () => ({ + useErrorMessages: jest.fn(), +})); + +describe('useBulkOperationDetails', () => { + let kyMock; + let historyMock; + let showErrorMessageMock; + + beforeEach(() => { + kyMock = { get: jest.fn() }; + historyMock = { replace: jest.fn(), location: { search: '' } }; + showErrorMessageMock = jest.fn(); + + useOkapiKy.mockReturnValue(kyMock); + useNamespace.mockReturnValue(['namespace-key']); + useHistory.mockReturnValue(historyMock); + useErrorMessages.mockReturnValue({ showErrorMessage: showErrorMessageMock }); + + // Set a default implementation for useQuery to prevent infinite renders + useQuery.mockReturnValue({ data: null, isLoading: false }); + jest.clearAllMocks(); + }); + + it('should initialize with the correct refetch interval and query key', () => { + const id = 'test-id'; + const refetchInterval = 5000; + + const { result } = renderHook(() => useBulkOperationDetails({ id, interval: refetchInterval })); + + expect(result.current.isLoading).toBe(false); + expect(useQuery).toHaveBeenCalledWith( + expect.objectContaining({ + queryKey: [BULK_OPERATION_DETAILS_KEY, 'namespace-key', id, refetchInterval], + refetchInterval, + enabled: true, + }) + ); + }); + + it('should clear interval and redirect when clearIntervalAndRedirect is called', () => { + const id = 'test-id'; + const pathname = '/new-path'; + const searchParams = { query: 'test' }; + + const { result } = renderHook(() => useBulkOperationDetails({ id })); + + act(() => { + result.current.clearIntervalAndRedirect(pathname, searchParams); + }); + + expect(historyMock.replace).toHaveBeenCalledWith({ + pathname, + search: undefined, + }); + }); +});