Skip to content

Commit

Permalink
UIBULKED-588 Displaying errors and warnings (#678)
Browse files Browse the repository at this point in the history
  • Loading branch information
vashjs authored Jan 24, 2025
1 parent 773b8dc commit 06cf887
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* [UIBULKED-597](https://folio-org.atlassian.net/browse/UIBULKED-597) Commit changes button enabled before preview is populated
* [UIBULKED-599](https://folio-org.atlassian.net/browse/UIBULKED-599) Change Administrative note type is not supported for MARC instances.
* [UIBULKED-589](https://folio-org.atlassian.net/browse/UIBULKED-589) Make options in the "Actions" dropdown in "Bulk edits" in alphabetical order.
* [UIBULKED-588](https://folio-org.atlassian.net/browse/IBULKED-588) Displaying errors and warnings.

## [4.2.2](https://github.com/folio-org/ui-bulk-edit/tree/v4.2.2) (2024-11-15)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useStripes } from '@folio/stripes/core';
import { PrevNextPagination } from '@folio/stripes-acq-components';
import css from '../Preview.css';
import { useSearchParams } from '../../../../../hooks';
import { CAPABILITIES, ERROR_PARAMETERS_KEYS } from '../../../../../constants';
import { CAPABILITIES, ERROR_PARAMETERS_KEYS, ERROR_TYPES } from '../../../../../constants';

const getParam = (error, key) => error.parameters.find(param => param.key === key)?.value;

Expand All @@ -24,6 +24,14 @@ const columnMapping = {

const visibleColumns = Object.keys(columnMapping);

const renderErrorType = (error) => {
if (!error.type || error.type === ERROR_TYPES.ERROR) {
return <FormattedMessage id="ui-bulk-edit.list.errors.table.status.ERROR" />;
}

return <FormattedMessage id="ui-bulk-edit.list.errors.table.status.WARNING" />;
};

const renderErrorMessage = (error, isLinkAvailable) => {
const link = getParam(error, ERROR_PARAMETERS_KEYS.LINK);

Expand All @@ -45,16 +53,19 @@ const renderErrorMessage = (error, isLinkAvailable) => {
};

const getResultsFormatter = ({ isLinkAvailable }) => ({
type: () => <FormattedMessage id="ui-bulk-edit.list.errors.table.status.ERROR" />,
type: renderErrorType,
key: error => getParam(error, ERROR_PARAMETERS_KEYS.IDENTIFIER),
message: error => renderErrorMessage(error, isLinkAvailable),
});

const ErrorsAccordion = ({
errors = [],
errorType,
totalErrors,
totalWarnings,
isFetching,
pagination,
onShowWarnings,
onChangePage,
}) => {
const { user, okapi } = useStripes();
Expand All @@ -65,13 +76,11 @@ const ErrorsAccordion = ({
const isLinkAvailable = (isCentralTenant && capabilities === CAPABILITIES.INSTANCE) || !isCentralTenant;
const resultsFormatter = getResultsFormatter({ isLinkAvailable });
const errorLength = errors.length;
// temporary solution to calculate total errors and warnings, until backend will provide it in scope of MODBULKOPS-451
const totalErrorsAndWarnings = errorType === ERROR_TYPES.ERROR ? totalErrors : totalErrors + totalWarnings;
const isWarningsCheckboxDisabled = !totalWarnings || !totalErrors;

const [opened, setOpened] = useState(!!errorLength);
const [showWarnings, setShowWarnings] = useState(false);

const handleShowWarnings = () => {
setShowWarnings(prev => !prev);
};

return (
<div className={css.previewAccordion}>
Expand All @@ -89,13 +98,14 @@ const ErrorsAccordion = ({
id="ui-bulk-edit.list.errors.info"
values={{
errors: totalErrors,
warnings: 0,
warnings: totalWarnings,
}}
/>
<Checkbox
label={<FormattedMessage id="ui-bulk-edit.list.errors.checkbox" />}
checked={showWarnings}
onChange={handleShowWarnings}
checked={!errorType}
onChange={onShowWarnings}
disabled={isWarningsCheckboxDisabled}
/>
</Layout>
</Headline>
Expand All @@ -113,7 +123,7 @@ const ErrorsAccordion = ({
{errors.length > 0 && (
<PrevNextPagination
{...pagination}
totalCount={totalErrors}
totalCount={totalErrorsAndWarnings}
disabled={false}
onChange={onChangePage}
/>
Expand All @@ -128,11 +138,14 @@ const ErrorsAccordion = ({
ErrorsAccordion.propTypes = {
errors: PropTypes.arrayOf(PropTypes.object),
totalErrors: PropTypes.number,
totalWarnings: PropTypes.number,
errorType: PropTypes.string,
isFetching: PropTypes.bool,
pagination: {
limit: PropTypes.number,
offset: PropTypes.number,
},
onShowWarnings: PropTypes.func,
onChangePage: PropTypes.func,
};

Expand Down
17 changes: 17 additions & 0 deletions src/components/BulkEditPane/BulkEditListResult/Preview/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { usePagination } from '../../../../hooks/usePagination';
import { useBulkOperationStats } from '../../../../hooks/useBulkOperationStats';
import { NoResultsMessage } from '../NoResultsMessage/NoResultsMessage';
import { useSearchParams } from '../../../../hooks';
import { useErrorType } from '../../../../hooks/useErrorType';

export const Preview = ({ id, title, isInitial, bulkDetails }) => {
const {
Expand All @@ -43,9 +44,15 @@ export const Preview = ({ id, title, isInitial, bulkDetails }) => {
const {
countOfRecords,
countOfErrors,
countOfWarnings,
visibleColumns,
} = useBulkOperationStats({ bulkDetails, step });

const { errorType, toggleErrorType } = useErrorType({
countOfErrors,
countOfWarnings
});

const {
pagination: previewPagination,
changePage: changePreviewPage,
Expand All @@ -71,10 +78,17 @@ export const Preview = ({ id, title, isInitial, bulkDetails }) => {

const { errors, isFetching: isErrorsFetching } = useErrorsPreview({
id,
step,
errorType,
enabled: isPreviewEnabled,
...errorsPagination,
});

const handleToggleWarnings = () => {
changeErrorPage(ERRORS_PAGINATION_CONFIG);
toggleErrorType();
};

if (!((bulkDetails.fqlQuery && criteria === CRITERIA.QUERY) || (criteria !== CRITERIA.QUERY && !bulkDetails.fqlQuery))) {
return <NoResultsMessage />;
}
Expand Down Expand Up @@ -117,7 +131,10 @@ export const Preview = ({ id, title, isInitial, bulkDetails }) => {
<ErrorsAccordion
errors={errors}
totalErrors={countOfErrors}
totalWarnings={countOfWarnings}
errorType={errorType}
onChangePage={changeErrorPage}
onShowWarnings={handleToggleWarnings}
pagination={errorsPagination}
isFetching={isErrorsFetching}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/components/shared/ProgressBar/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const getBulkOperationStep = (bulkOperation) => {
case bulkOperation.status === JOB_STATUSES.COMPLETED:
case (
bulkOperation.status === JOB_STATUSES.COMPLETED_WITH_ERRORS
&& Boolean(bulkOperation.committedNumOfErrors)
&& (Boolean(bulkOperation.committedNumOfErrors) || Boolean(bulkOperation.committedNumOfWarnings))
):
return EDITING_STEPS.COMMIT;
case bulkOperation.status === JOB_STATUSES.DATA_MODIFICATION:
Expand Down
7 changes: 6 additions & 1 deletion src/constants/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ export const PREVIEW_LIMITS = {
RECORDS: 100,
};

export const ERROR_TYPES = {
WARNING: 'WARNING',
ERROR: 'ERROR',
};

export const APPROACHES = {
IN_APP: 'IN_APP',
MANUAL: 'MANUAL',
Expand Down Expand Up @@ -151,7 +156,7 @@ export const ERROR_PARAMETERS_KEYS = {
};

export const RECORD_TYPES_MAPPING = {
[CAPABILITIES.HOLDING]: 'holding',
[CAPABILITIES.HOLDING]: 'holdings',
[CAPABILITIES.INSTANCE]: 'instance',
[CAPABILITIES.ITEM]: 'item',
[CAPABILITIES.USER]: 'user',
Expand Down
6 changes: 4 additions & 2 deletions src/hooks/api/useErrorsPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export const useErrorsPreview = ({
enabled,
offset = 0,
limit = PREVIEW_LIMITS.ERRORS,
step,
errorType
}) => {
const ky = useOkapiKy();
const [namespaceKey] = useNamespace({ key: PREVIEW_ERRORS_KEY });
Expand All @@ -18,8 +20,8 @@ export const useErrorsPreview = ({

const { data, isFetching } = useQuery(
{
queryKey: [namespaceKey, id, limit, offset],
queryFn: () => ky.get(`bulk-operations/${id}/errors`, { searchParams: { limit, offset } }).json(),
queryKey: [namespaceKey, id, limit, offset, errorType, step],
queryFn: () => ky.get(`bulk-operations/${id}/errors`, { searchParams: { limit, offset, errorType } }).json(),
onError: showErrorMessage,
onSuccess: showErrorMessage,
keepPreviousData: true,
Expand Down
21 changes: 16 additions & 5 deletions src/hooks/api/useErrorsPreview.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useNamespace, useOkapiKy } from '@folio/stripes/core';

import { useErrorMessages } from '../useErrorMessages';
import { useErrorsPreview, PREVIEW_ERRORS_KEY } from './useErrorsPreview';
import { PREVIEW_LIMITS } from '../../constants';
import { EDITING_STEPS, ERROR_TYPES, PREVIEW_LIMITS } from '../../constants';

jest.mock('react-query', () => ({
useQuery: jest.fn(),
Expand Down Expand Up @@ -40,15 +40,20 @@ describe('useErrorsPreview', () => {
isFetching: true,
});

const { result } = renderHook(() => useErrorsPreview({ id: '123', enabled: true }));
const { result } = renderHook(() => useErrorsPreview({
id: '123',
enabled: true,
errorType: ERROR_TYPES.ERROR,
step: EDITING_STEPS.UPLOAD,
}));

expect(result.current.errors).toEqual(['error1', 'error2']);
expect(result.current.isFetching).toBe(true);

expect(useNamespace).toHaveBeenCalledWith({ key: PREVIEW_ERRORS_KEY });
expect(useQuery).toHaveBeenCalledWith(
expect.objectContaining({
queryKey: [PREVIEW_ERRORS_KEY, '123', PREVIEW_LIMITS.ERRORS, 0],
queryKey: [PREVIEW_ERRORS_KEY, '123', PREVIEW_LIMITS.ERRORS, 0, ERROR_TYPES.ERROR, EDITING_STEPS.UPLOAD],
queryFn: expect.any(Function),
enabled: true,
})
Expand All @@ -62,10 +67,16 @@ describe('useErrorsPreview', () => {
return { data: null, isFetching: false };
});

renderHook(() => useErrorsPreview({ id: '123', enabled: true, offset: 10, limit: 20 }));
renderHook(() => useErrorsPreview({
id: '123',
enabled: true,
offset: 10,
limit: 20,
errorType: ERROR_TYPES.WARNING
}));

expect(mockGet).toHaveBeenCalledWith('bulk-operations/123/errors', {
searchParams: { limit: 20, offset: 10 },
searchParams: { limit: 20, offset: 10, errorType: ERROR_TYPES.WARNING },
});
});

Expand Down
7 changes: 7 additions & 0 deletions src/hooks/useBulkOperationStats.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { RootContext } from '../context/RootContext';
export const useBulkOperationStats = ({ bulkDetails, step }) => {
const { countOfRecords, setCountOfRecords, visibleColumns } = useContext(RootContext);
const [countOfErrors, setCountOfErrors] = useState(0);
const [countOfWarnings, setCountOfWarnings] = useState(0);
const [totalCount, setTotalCount] = useState(0);

useEffect(() => {
Expand All @@ -18,7 +19,12 @@ export const useBulkOperationStats = ({ bulkDetails, step }) => {
? bulkDetails.matchedNumOfErrors
: bulkDetails.committedNumOfErrors;

const countWarnings = isInitialPreview
? bulkDetails.matchedNumOfWarnings
: bulkDetails.committedNumOfWarnings;

setCountOfErrors(countErrors);
setCountOfWarnings(countWarnings);
setCountOfRecords(countRecords);
setTotalCount(isInitialPreview ? bulkDetails.totalNumOfRecords : bulkDetails.matchedNumOfRecords);
}, [
Expand All @@ -32,6 +38,7 @@ export const useBulkOperationStats = ({ bulkDetails, step }) => {
return {
countOfRecords,
countOfErrors,
countOfWarnings,
totalCount,
visibleColumns,
};
Expand Down
22 changes: 22 additions & 0 deletions src/hooks/useErrorType.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { useEffect, useState } from 'react';
import { ERROR_TYPES } from '../constants';

// empty string is used to reset the error type and show both errors and warnings
const getDynamicErrorType = (condition) => (condition ? '' : ERROR_TYPES.ERROR);

export const useErrorType = ({ countOfErrors, countOfWarnings }) => {
const hasOnlyWarnings = countOfErrors === 0 && countOfWarnings > 0;
const initialErrorType = getDynamicErrorType(hasOnlyWarnings);

const [errorType, setErrorType] = useState(initialErrorType);

const toggleErrorType = () => {
setErrorType(getDynamicErrorType(!!errorType));
};

useEffect(() => {
setErrorType(initialErrorType);
}, [initialErrorType]);

return { errorType, toggleErrorType };
};
48 changes: 48 additions & 0 deletions src/hooks/useErrorType.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { renderHook, act } from '@testing-library/react-hooks';
import { useErrorType } from './useErrorType';
import { ERROR_TYPES } from '../constants';

describe('useErrorType', () => {
it('should initialize with no error type if only warnings exist', () => {
const { result } = renderHook(() => useErrorType({ countOfErrors: 0, countOfWarnings: 1 }));
expect(result.current.errorType).toBe('');
});

it('should initialize with error type if errors exist', () => {
const { result } = renderHook(() => useErrorType({ countOfErrors: 1, countOfWarnings: 0 }));
expect(result.current.errorType).toBe(ERROR_TYPES.ERROR);
});

it('should toggle error type when toggleErrorType is called', () => {
const { result } = renderHook(() => useErrorType({ countOfErrors: 1, countOfWarnings: 0 }));

expect(result.current.errorType).toBe(ERROR_TYPES.ERROR);

act(() => {
result.current.toggleErrorType();
});
expect(result.current.errorType).toBe('');

act(() => {
result.current.toggleErrorType();
});
expect(result.current.errorType).toBe(ERROR_TYPES.ERROR);
});

it('should reset error type when the input props change', () => {
const { result, rerender } = renderHook(
({ countOfErrors, countOfWarnings }) => useErrorType({ countOfErrors, countOfWarnings }),
{
initialProps: { countOfErrors: 1, countOfWarnings: 0 },
}
);

expect(result.current.errorType).toBe(ERROR_TYPES.ERROR);

rerender({ countOfErrors: 0, countOfWarnings: 1 });
expect(result.current.errorType).toBe('');

rerender({ countOfErrors: 2, countOfWarnings: 0 });
expect(result.current.errorType).toBe(ERROR_TYPES.ERROR);
});
});

0 comments on commit 06cf887

Please sign in to comment.