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

UIQM-743: Prevent handleSubmit from running if there is any validation issue, instead of using the complete form API. #770

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* [UIQM-730](https://issues.folio.org/browse/UIQM-730) Create/Edit/Derive MARC record - Retain focus when MARC record validation rules error display. Show validation issues toasts.
* [UIQM-740](https://issues.folio.org/browse/UIQM-740) Don't show warn/fail error toasts when there are no warns/fails.
* [UIQM-728](https://issues.folio.org/browse/UIQM-728) Keep focus on last focused element when user cancels on confirmation modals.
* [UIQM-743](https://issues.folio.org/browse/UIQM-743) Prevent `handleSubmit` from running if there is any validation issue, instead of using the `complete` form API.

## [9.0.2] (IN PROGRESS)

Expand Down
7 changes: 7 additions & 0 deletions src/QuickMarcEditor/QuickMarcEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ const QuickMarcEditor = ({
const {
setValidationErrors,
continueAfterSave,
validationErrorsRef,
} = useContext(QuickMarcContext);
const { hasErrorIssues, isBackEndValidationMarcType } = useValidation();

Expand Down Expand Up @@ -310,6 +311,11 @@ const QuickMarcEditor = ({
}
}

// if validation has any issues - cancel submit
if (!isEmpty(validationErrorsRef.current)) {
return;
}

handleSubmit(e)
?.then(handleSubmitResponse)
?.finally(closeModals);
Expand All @@ -329,6 +335,7 @@ const QuickMarcEditor = ({
focusLastFocusedInput,
showValidationIssuesToasts,
continueAfterSave,
validationErrorsRef,
]);

const paneFooter = useMemo(() => {
Expand Down
26 changes: 26 additions & 0 deletions src/QuickMarcEditor/QuickMarcEditor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,32 @@ describe('Given QuickMarcEditor', () => {
});
});

describe('when saving form with validation issue', () => {
it('should prevent submitting', async () => {
const {
getByText,
getByTestId,
} = renderQuickMarcEditor({}, {
quickMarcContext: {
validationErrorsRef: {
current: {
[MISSING_FIELD_ID]: [
{ id: 'some warning', severity: 'warn', values: {} },
],
},
},
},
});

const contentField = getByTestId('content-field-3');

fireEvent.change(contentField, { target: { value: '' } });
await fireEvent.click(getByText('stripes-acq-components.FormFooter.save'));

expect(onSubmitMock).not.toHaveBeenCalled();
});
});

describe('when saving form without validation warnings or errors', () => {
beforeEach(async () => {
mockValidate.mockClear().mockResolvedValue({});
Expand Down
69 changes: 0 additions & 69 deletions src/QuickMarcEditor/useSaveRecord/useSaveRecord.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ const mockOnSave = jest.fn();
const mockActualizeLinks = jest.fn((formValuesToProcess) => Promise.resolve(formValuesToProcess));
const mockUpdateMarcRecord = jest.fn().mockResolvedValue();
const mockValidateFetch = jest.fn().mockResolvedValue({});
const mockComplete = jest.fn();

const _api = null;
const basePath = '/base-path';

const locations = [{
Expand Down Expand Up @@ -719,29 +717,6 @@ describe('useSaveRecord', () => {
});

describe('when creating', () => {
describe('when there is a validation error', () => {
it('should stop submitting', async () => {
const { result } = renderHook(useSaveRecord, {
initialProps: getInitialProps(MARC_TYPES.BIB),
wrapper: getWrapper({
quickMarcContext: {
action: QUICK_MARC_ACTIONS.CREATE,
validationErrorsRef: {
current: {
'id-with-error': [{ id: 'some-error' }],
},
},
},
}),
});

await result.current.onSubmit(null, _api, mockComplete);

expect(mockComplete).toHaveBeenCalled();
expect(getMutator().quickMarcEditMarcRecord.POST).not.toHaveBeenCalled();
});
});

describe('when marc type is not a bib', () => {
it('should not call actualizeLinks', async () => {
const marcType = MARC_TYPES.HOLDINGS;
Expand Down Expand Up @@ -1156,28 +1131,6 @@ describe('useSaveRecord', () => {
});

describe('when editing', () => {
describe('when there is a validation error', () => {
it('should stop submitting', async () => {
const { result } = renderHook(useSaveRecord, {
initialProps: getInitialProps(MARC_TYPES.BIB),
wrapper: getWrapper({
quickMarcContext: {
action: QUICK_MARC_ACTIONS.EDIT,
validationErrorsRef: {
current: {
'id-with-error': [{ id: 'some-error' }],
},
},
},
}),
});

await result.current.onSubmit(null, _api, mockComplete);

expect(mockComplete).toHaveBeenCalled();
});
});

describe('when marc type is not a bib', () => {
it('should not call actualizeLinks', async () => {
const marcType = MARC_TYPES.AUTHORITY;
Expand Down Expand Up @@ -1672,28 +1625,6 @@ describe('useSaveRecord', () => {
});

describe('when deriving', () => {
describe('when there is a validation error', () => {
it('should stop submitting', async () => {
const { result } = renderHook(useSaveRecord, {
initialProps: getInitialProps(MARC_TYPES.BIB),
wrapper: getWrapper({
quickMarcContext: {
action: QUICK_MARC_ACTIONS.DERIVE,
validationErrorsRef: {
current: {
'id-with-error': [{ id: 'some-error' }],
},
},
},
}),
});

await result.current.onSubmit(null, _api, mockComplete);

expect(mockComplete).toHaveBeenCalled();
});
});

it('should actualize links', async () => {
const action = QUICK_MARC_ACTIONS.DERIVE;
const marcType = MARC_TYPES.BIB;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
useLocation,
useParams,
} from 'react-router-dom';
import isEmpty from 'lodash/isEmpty';
import noop from 'lodash/noop';
import isNil from 'lodash/isNil';

Expand Down Expand Up @@ -56,7 +55,6 @@ const useSubmitRecord = ({
basePath,
initialValues,
instance,
validationErrorsRef,
continueAfterSave,
relatedRecordVersion,
} = useContext(QuickMarcContext);
Expand Down Expand Up @@ -95,12 +93,7 @@ const useSubmitRecord = ({
});
}, [basePath, marcType, location, history, refreshPageData]);

const onCreate = useCallback(async (formValues, _api, complete) => {
// if validation has any issues - cancel submit
if (!isEmpty(validationErrorsRef.current)) {
return complete();
}

const onCreate = useCallback(async (formValues, _api) => {
const formValuesToProcess = prepareForSubmit(formValues);

let formValuesToHydrate;
Expand Down Expand Up @@ -168,22 +161,16 @@ const useSubmitRecord = ({
showCallout,
prepareForSubmit,
actualizeLinks,
validationErrorsRef,
marcType,
continueAfterSave,
mutator,
processEditingAfterCreation,
redirectToRecord,
]);

const onEdit = useCallback(async (formValues, _api, complete) => {
const onEdit = useCallback(async (formValues, _api) => {
let is1xxOr010Updated = false;

// if validation has any issues - cancel submit
if (!isEmpty(validationErrorsRef.current)) {
return complete();
}

if (marcType === MARC_TYPES.AUTHORITY && linksCount > 0) {
is1xxOr010Updated = are010Or1xxUpdated(initialValues.records, formValues.records);
}
Expand Down Expand Up @@ -293,20 +280,14 @@ const useSubmitRecord = ({
locale,
updateMarcRecord,
isRequestToCentralTenantFromMember,
validationErrorsRef,
relatedRecordVersion,
_externalId,
_instanceId,
continueAfterSave,
redirectToRecord,
]);

const onDerive = useCallback(async (formValues, _api, complete) => {
// if validation has any issues - cancel submit
if (!isEmpty(validationErrorsRef.current)) {
return complete();
}

const onDerive = useCallback(async (formValues, _api) => {
const formValuesToProcess = prepareForSubmit(formValues);

showCallout({ messageId: 'ui-quick-marc.record.saveNew.onSave' });
Expand Down Expand Up @@ -379,7 +360,6 @@ const useSubmitRecord = ({
showCallout,
prepareForSubmit,
actualizeLinks,
validationErrorsRef,
continueAfterSave,
mutator,
processEditingAfterCreation,
Expand Down
Loading