Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIBULKED-497: "Are you sure" preview displays outdated values after User changed selection on bulk edit form and clicked "Confirm changes" (follow-up) #632

Merged
merged 4 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import PropTypes from 'prop-types';
import { Modal } from '@folio/stripes/components';
import { buildSearch } from '@folio/stripes-acq-components';

import { Preloader } from '@folio/stripes-data-transfer-components';
import {
APPROACHES,
EDITING_STEPS,
Expand Down Expand Up @@ -81,12 +82,13 @@ export const BulkEditPreviewModal = ({
dismissible
onClose={onKeepEditing}
>
{open && (
{isPreviewLoading ?
<Preloader />
:
<BulkEditPreviewModalList
isPreviewEnabled={!isPreviewLoading}
onPreviewError={onKeepEditing}
/>
)}
}
</Modal>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import { useErrorMessages } from '../../../../hooks/useErrorMessages';


export const BulkEditPreviewModalList = ({
isPreviewEnabled,
onPreviewError,
}) => {
const { id: bulkOperationId } = usePathParams('/bulk-edit/:id');
Expand All @@ -54,7 +53,7 @@ export const BulkEditPreviewModalList = ({
});

const visibleColumnKeys = getVisibleColumnsKeys(visibleColumns);
const enabled = isPreviewEnabled && bulkDetails?.status === JOB_STATUSES.REVIEW_CHANGES;
const enabled = bulkDetails?.status === JOB_STATUSES.REVIEW_CHANGES;

const {
contentData,
Expand All @@ -79,7 +78,7 @@ export const BulkEditPreviewModalList = ({
...pagination,
});

if (!contentData || !isPreviewEnabled) return <Preloader />;
if (!contentData) return <Preloader />;

const renderMessageBanner = () => {
if (!bulkDetails?.processedNumOfRecords && currentRecordType === CAPABILITIES.INSTANCE) {
Expand Down Expand Up @@ -130,6 +129,5 @@ export const BulkEditPreviewModalList = ({
};

BulkEditPreviewModalList.propTypes = {
isPreviewEnabled: PropTypes.bool,
onPreviewError: PropTypes.func,
};
10 changes: 7 additions & 3 deletions src/hooks/useConfirmChanges.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useState } from 'react';
import { useQueryClient } from 'react-query';

import { useOkapiKy } from '@folio/stripes/core';
import {
PREVIEW_MODAL_KEY,
BULK_OPERATION_DETAILS_KEY,
Expand All @@ -15,8 +16,9 @@ import {
JOB_STATUSES,
} from '../constants';
import { useSearchParams } from './useSearchParams';
import { useErrorMessages } from './useErrorMessages';

import { useErrorMessages } from './useErrorMessages';
import { pollForStatus } from '../utils/pollForStatus';

export const useConfirmChanges = ({
updateFn,
Expand All @@ -25,6 +27,7 @@ export const useConfirmChanges = ({
onDownloadSuccess,
}) => {
const queryClient = useQueryClient();
const ky = useOkapiKy();
const searchParams = useSearchParams();
const { showErrorMessage } = useErrorMessages();

Expand Down Expand Up @@ -54,15 +57,16 @@ export const useConfirmChanges = ({
(preBulkOperation) => ({ ...preBulkOperation, status: JOB_STATUSES.DATA_MODIFICATION }),
);

updateFn(payload)
openPreviewModal();
pollForStatus(ky, bulkOperationId)
.then(() => updateFn(payload))
.then(() => bulkOperationStart({
id: bulkOperationId,
approach: APPROACHES.IN_APP,
step: EDITING_STEPS.EDIT,
}))
.then((response) => {
showErrorMessage(response);
openPreviewModal();
})
.catch((error) => {
showErrorMessage(error);
Expand Down
28 changes: 24 additions & 4 deletions src/hooks/useConfirmChanges.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@ import '../../test/jest/__mock__/reactIntl.mock';
import { useBulkOperationDetails, useBulkOperationStart, useFileDownload } from './api';
import { useSearchParams } from './useSearchParams';
import { useConfirmChanges } from './useConfirmChanges';
import { pollForStatus } from '../utils/pollForStatus';

jest.mock('react-query', () => ({
useQueryClient: jest.fn(),
}));

jest.mock('@folio/stripes/core', () => ({
...jest.requireActual('@folio/stripes/core'),
useOkapiKy: jest.fn(),
}));


jest.mock('@folio/stripes-acq-components', () => ({
useShowCallout: jest.fn(),
}));
Expand All @@ -27,6 +34,7 @@ jest.mock('./useSearchParams', () => ({
useSearchParams: jest.fn(),
}));

jest.mock('../utils/pollForStatus');

describe('useConfirmChanges', () => {
const mockCallout = jest.fn();
Expand All @@ -48,6 +56,7 @@ describe('useConfirmChanges', () => {
useBulkOperationStart.mockReturnValue({ bulkOperationStart: mockBulkOperationStart });
useSearchParams.mockReturnValue({ criteria: 'testCriteria', initialFileName: 'initialFileName' });
useFileDownload.mockReturnValue({ refetch: mockDownloadFile, isFetching: false });
pollForStatus.mockImplementation(() => Promise.resolve());

jest.clearAllMocks();
});
Expand Down Expand Up @@ -89,26 +98,34 @@ describe('useConfirmChanges', () => {
bulkOperationId: '123',
}));

// Call confirmChanges with a mock payload
act(() => {
result.current.confirmChanges({});
});

// Check if loading state is set
expect(result.current.isPreviewLoading).toBe(true);

// Wait for the next update after the async calls
await waitForNextUpdate(); // Wait for the polling to start

await waitForNextUpdate();

// Verify that the update function is called
expect(mockUpdateFn).toHaveBeenCalled();

// Verify that the bulkOperationStart function is called with the correct parameters
expect(mockBulkOperationStart).toHaveBeenCalledWith({
id: '123',
approach: 'IN_APP',
step: 'EDIT',
});

// Finally, check if loading state is set back to false
expect(result.current.isPreviewLoading).toBe(false);
});

it('should handle error in confirmChanges function', async () => {
mockUpdateFn.mockImplementationOnce(() => Promise.reject());
// Mock the update function to throw an error
mockUpdateFn.mockImplementationOnce(() => Promise.reject(new Error('Update failed')));

const { result, waitForNextUpdate } = renderHook(() => useConfirmChanges({
updateFn: mockUpdateFn,
Expand All @@ -120,12 +137,15 @@ describe('useConfirmChanges', () => {
result.current.confirmChanges({});
});

// Check if loading state is set
expect(result.current.isPreviewLoading).toBe(true);

// Wait for the next update after the async calls
await waitForNextUpdate();

// Check that loading state is set back to false after handling the error
expect(result.current.isPreviewLoading).toBe(false);
expect(result.current.isPreviewModalOpened).toBe(false);
expect(result.current.isPreviewModalOpened).toBe(false); // Modal should close on error
});

it('should call downloadFile from useFileDownload', () => {
Expand Down
15 changes: 15 additions & 0 deletions src/utils/pollForStatus.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { JOB_STATUSES } from '../constants';

export const pollForStatus = (ky, id) => {
const interval = 1000;
return new Promise((resolve, reject) => {
const intervalId = setInterval(async () => {
try {
const data = await ky.get(`bulk-operations/${id}`).json();
if (data.status !== JOB_STATUSES.DATA_MODIFICATION_IN_PROGRESS) {
clearInterval(intervalId); resolve(data.status);
}
} catch (error) { clearInterval(intervalId); reject(error); }
}, interval);
});
};
57 changes: 57 additions & 0 deletions src/utils/pollForStatus.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { useOkapiKy } from '@folio/stripes/core';
import { JOB_STATUSES } from '../constants';
import { pollForStatus } from './pollForStatus';

describe('pollForStatus', () => {
const mockGet = jest.fn(() => ({
json: (data) => Promise.resolve(data),
}));
jest.useFakeTimers();

beforeEach(() => {
mockGet.mockClear();
useOkapiKy.mockClear().mockReturnValue({
get: mockGet,
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should resolve with the final status when status is not DATA_MODIFICATION_IN_PROGRESS', async () => {
const finalStatus = 'COMPLETED';
mockGet
.mockImplementationOnce(() => ({
json: () => Promise.resolve({ status: JOB_STATUSES.DATA_MODIFICATION_IN_PROGRESS }),
}))
.mockImplementationOnce(() => ({
json: () => Promise.resolve({ status: finalStatus }),
}));

const promise = pollForStatus(useOkapiKy(), 'mockId');

jest.advanceTimersByTime(1000);
await Promise.resolve();
jest.advanceTimersByTime(1000);
await Promise.resolve();

await expect(promise).resolves.toBe(finalStatus);
expect(mockGet).toHaveBeenCalledTimes(2);
});

it('should reject with an error if the request fails', async () => {
const error = new Error('Request failed');
mockGet.mockImplementationOnce(() => {
throw error;
});

const promise = pollForStatus(useOkapiKy(), 'mockId');

jest.advanceTimersByTime(1000);
await Promise.resolve();

await expect(promise).rejects.toThrow('Request failed');
expect(mockGet).toHaveBeenCalledTimes(1);
});
});
Loading