From 875a43a14ed732a7180a685d35ff9e39ea7b4809 Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Fri, 20 Nov 2020 15:02:56 +0100 Subject: [PATCH] [835] Auto-save in new Item Submission form breaks the form Section formId added to the section state. Error filtering during the parsing of the submission response. --- .../form/builder/form-builder.service.ts | 2 +- src/app/shared/form/form.component.ts | 1 + src/app/shared/form/form.reducer.ts | 2 +- src/app/shared/form/form.service.ts | 4 +- .../objects/submission-objects.actions.ts | 25 ++++++++++ .../objects/submission-objects.effects.ts | 47 +++++++++++++++++-- .../objects/submission-objects.reducer.ts | 37 +++++++++++++++ .../sections/form/section-form.component.html | 1 - .../sections/form/section-form.component.ts | 26 ++++------ .../submission/sections/sections.service.ts | 13 +++++ 10 files changed, 130 insertions(+), 28 deletions(-) diff --git a/src/app/shared/form/builder/form-builder.service.ts b/src/app/shared/form/builder/form-builder.service.ts index 256e657474..f0baa3f8a3 100644 --- a/src/app/shared/form/builder/form-builder.service.ts +++ b/src/app/shared/form/builder/form-builder.service.ts @@ -328,7 +328,7 @@ export class FormBuilderService extends DynamicFormService { model = model.parent as any; } - const iterateControlModels = (findGroupModel: DynamicFormControlModel[], controlModelIndex: number = 0): void => { + const iterateControlModels = (findGroupModel: DynamicFormControlModel[], controlModelIndex: number = 0): string[] => { let iterateResult = Object.create({}); // Iterate over all group's controls diff --git a/src/app/shared/form/form.component.ts b/src/app/shared/form/form.component.ts index 7a5d3932c8..43f9bdfa90 100644 --- a/src/app/shared/form/form.component.ts +++ b/src/app/shared/form/form.component.ts @@ -253,6 +253,7 @@ export class FormComponent implements OnDestroy, OnInit { } onFocus(event: DynamicFormControlEvent): void { + this.formService.setTouched(this.formId, this.formModel, event); this.focus.emit(event); } diff --git a/src/app/shared/form/form.reducer.ts b/src/app/shared/form/form.reducer.ts index 8a5c2a57b8..2a2b3cd2de 100644 --- a/src/app/shared/form/form.reducer.ts +++ b/src/app/shared/form/form.reducer.ts @@ -133,7 +133,7 @@ function initForm(state: FormState, action: FormInitAction): FormState { data: action.payload.formData, valid: action.payload.valid, errors: [], - additional: action.payload.formAdditional + additional: action.payload.formAdditional ? action.payload.formAdditional : {} }; if (!hasValue(state[action.payload.formId])) { return Object.assign({}, state, { diff --git a/src/app/shared/form/form.service.ts b/src/app/shared/form/form.service.ts index 6d208b08d2..8983bade44 100644 --- a/src/app/shared/form/form.service.ts +++ b/src/app/shared/form/form.service.ts @@ -161,8 +161,8 @@ export class FormService { return (environment.form.validatorMap.hasOwnProperty(validator)) ? environment.form.validatorMap[validator] : validator; } - public initForm(formId: string, model: DynamicFormControlModel[], valid: boolean, additional?: any) { - this.store.dispatch(new FormInitAction(formId, this.formBuilderService.getValueFromModel(model), valid, additional)); + public initForm(formId: string, model: DynamicFormControlModel[], valid: boolean) { + this.store.dispatch(new FormInitAction(formId, this.formBuilderService.getValueFromModel(model), valid)); } public setStatusChanged(formId: string, valid: boolean) { diff --git a/src/app/submission/objects/submission-objects.actions.ts b/src/app/submission/objects/submission-objects.actions.ts index 6fe42a149a..962f216c7d 100644 --- a/src/app/submission/objects/submission-objects.actions.ts +++ b/src/app/submission/objects/submission-objects.actions.ts @@ -40,6 +40,7 @@ export const SubmissionObjectActionTypes = { INIT_SECTION: type('dspace/submission/INIT_SECTION'), ENABLE_SECTION: type('dspace/submission/ENABLE_SECTION'), DISABLE_SECTION: type('dspace/submission/DISABLE_SECTION'), + SET_SECTION_FORM_ID: type('dspace/submission/SET_SECTION_FORM_ID'), SECTION_STATUS_CHANGE: type('dspace/submission/SECTION_STATUS_CHANGE'), SECTION_LOADING_STATUS_CHANGE: type('dspace/submission/SECTION_LOADING_STATUS_CHANGE'), UPDATE_SECTION_DATA: type('dspace/submission/UPDATE_SECTION_DATA'), @@ -256,6 +257,29 @@ export class RemoveSectionErrorsAction implements Action { } } +export class SetSectionFormId implements Action { + type = SubmissionObjectActionTypes.SET_SECTION_FORM_ID; + payload: { + submissionId: string; + sectionId: string; + formId: string; + }; + + /** + * Create a new SetSectionFormId + * + * @param submissionId + * the submission's ID + * @param sectionId + * the section's ID + * @param formId + * the section's formId + */ + constructor(submissionId: string, sectionId: string, formId: string) { + this.payload = { submissionId, sectionId, formId }; + } +} + // Submission actions export class CompleteInitSubmissionFormAction implements Action { @@ -782,6 +806,7 @@ export class DeleteUploadedFileAction implements Action { */ export type SubmissionObjectAction = DisableSectionAction | InitSectionAction + | SetSectionFormId | EnableSectionAction | InitSubmissionFormAction | ResetSubmissionFormAction diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 3a25d79330..07436fc87f 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -52,12 +52,13 @@ import { UpdateSectionDataAction, UpdateSectionDataSuccessAction } from './submission-objects.actions'; -import { SubmissionObjectEntry, SubmissionSectionObject } from './submission-objects.reducer'; +import {SubmissionObjectEntry, SubmissionSectionError, SubmissionSectionObject} from './submission-objects.reducer'; import { Item } from '../../core/shared/item.model'; import { RemoteData } from '../../core/data/remote-data'; import { getFirstSucceededRemoteDataPayload } from '../../core/shared/operators'; import { SubmissionObjectDataService } from '../../core/submission/submission-object-data.service'; import { followLink } from '../../shared/utils/follow-link-config.model'; +import parseSectionErrorPaths, {SectionErrorPath} from '../utils/parseSectionErrorPaths'; @Injectable() export class SubmissionObjectEffects { @@ -157,7 +158,8 @@ export class SubmissionObjectEffects { ofType(SubmissionObjectActionTypes.SAVE_SUBMISSION_FORM_SUCCESS), withLatestFrom(this.store$), map(([action, currentState]: [SaveSubmissionFormSuccessAction, any]) => { - return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], action.payload.submissionObject, action.payload.submissionId, action.payload.notify); + return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], + action.payload.submissionObject, action.payload.submissionId, currentState.forms, action.payload.notify); }), mergeMap((actions) => observableFrom(actions))); @@ -169,7 +171,8 @@ export class SubmissionObjectEffects { ofType(SubmissionObjectActionTypes.SAVE_SUBMISSION_SECTION_FORM_SUCCESS), withLatestFrom(this.store$), map(([action, currentState]: [SaveSubmissionSectionFormSuccessAction, any]) => { - return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], action.payload.submissionObject, action.payload.submissionId, false); + return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], + action.payload.submissionObject, action.payload.submissionId, currentState.forms, false); }), mergeMap((actions) => observableFrom(actions))); @@ -212,7 +215,8 @@ export class SubmissionObjectEffects { return new DepositSubmissionAction(action.payload.submissionId); } else { this.notificationsService.warning(null, this.translate.get('submission.sections.general.sections_not_valid')); - return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], response, action.payload.submissionId); + return this.parseSaveResponse((currentState.submission as SubmissionState).objects[action.payload.submissionId], + response, action.payload.submissionId, currentState.forms); } }), catchError(() => observableOf(new SaveSubmissionFormErrorAction(action.payload.submissionId)))); @@ -365,6 +369,7 @@ export class SubmissionObjectEffects { currentState: SubmissionObjectEntry, response: SubmissionObject[], submissionId: string, + forms, notify: boolean = true): SubmissionObjectAction[] { const mappedActions = []; @@ -404,10 +409,42 @@ export class SubmissionObjectEffects { if (notify && !currentState.sections[sectionId].enabled) { this.submissionService.notifyNewSection(submissionId, sectionId, currentState.sections[sectionId].sectionType); } - mappedActions.push(new UpdateSectionDataAction(submissionId, sectionId, sectionData, sectionErrors)); + + const sectionForm = forms[currentState.sections[sectionId].formId]; + const filteredErrors = filterErrors(sectionForm, sectionErrors, currentState.sections[sectionId].sectionType, notify); + + mappedActions.push(new UpdateSectionDataAction(submissionId, sectionId, sectionData, filteredErrors)); } }); } return mappedActions; } } + +/** + * Filter sectionErrors accordingly to this rules: + * 1. if notifications are enabled return all errors + * 2. if sectionType is different from submission-form return all errors + * 3. otherwise return errors only for those fields marked as touched inside the section form + * @param sectionForm + * @param sectionErrors + * @param notify + */ +function filterErrors(sectionForm, sectionErrors, sectionType, notify): any { + if (notify || sectionType !== SectionsType.SubmissionForm) { + return sectionErrors; + } + if (!sectionForm || !sectionForm.additional || !sectionForm.additional.touched) { + return []; + } + const filteredErrors = []; + sectionErrors.forEach((error: SubmissionSectionError) => { + const errorPaths: SectionErrorPath[] = parseSectionErrorPaths(error.path); + errorPaths.forEach((path: SectionErrorPath) => { + if (path.fieldId && sectionForm.additional.touched[path.fieldId]) { + filteredErrors.push(error); + } + }); + }); + return filteredErrors; +} diff --git a/src/app/submission/objects/submission-objects.reducer.ts b/src/app/submission/objects/submission-objects.reducer.ts index 6f7098532e..91caeb1e77 100644 --- a/src/app/submission/objects/submission-objects.reducer.ts +++ b/src/app/submission/objects/submission-objects.reducer.ts @@ -30,6 +30,7 @@ import { SaveSubmissionSectionFormSuccessAction, SectionStatusChangeAction, SetActiveSectionAction, + SetSectionFormId, SubmissionObjectAction, SubmissionObjectActionTypes, UpdateSectionDataAction @@ -109,6 +110,11 @@ export interface SubmissionSectionObject { * A boolean representing if this section is valid */ isValid: boolean; + + /** + * The formId related to this section + */ + formId: string; } /** @@ -263,6 +269,10 @@ export function submissionObjectReducer(state = initialState, action: Submission return initSection(state, action as InitSectionAction); } + case SubmissionObjectActionTypes.SET_SECTION_FORM_ID: { + return setSectionFormId(state, action as SetSectionFormId); + } + case SubmissionObjectActionTypes.ENABLE_SECTION: { return changeSectionState(state, action as EnableSectionAction, true); } @@ -646,6 +656,33 @@ function initSection(state: SubmissionObjectState, action: InitSectionAction): S } } +/** + * Set a section form id. + * + * @param state + * the current state + * @param action + * an SetSectionFormId + * @return SubmissionObjectState + * the new state + */ +function setSectionFormId(state: SubmissionObjectState, action: SetSectionFormId): SubmissionObjectState { + if (hasValue(state[ action.payload.submissionId ])) { + return Object.assign({}, state, { + [ action.payload.submissionId ]: Object.assign({}, state[ action.payload.submissionId ], { + sections: Object.assign({}, state[ action.payload.submissionId ].sections, { + [ action.payload.sectionId ]: { + ...state[ action.payload.submissionId ].sections [action.payload.sectionId], + formId: action.payload.formId + } + }) + }) + }); + } else { + return state; + } +} + /** * Update section's data. * diff --git a/src/app/submission/sections/form/section-form.component.html b/src/app/submission/sections/form/section-form.component.html index ab549315ed..894c8b1d17 100644 --- a/src/app/submission/sections/form/section-form.component.html +++ b/src/app/submission/sections/form/section-form.component.html @@ -2,7 +2,6 @@ ) => configData.payload), +======= + this.sectionService.dispatchSetSectionFormId(this.submissionId, this.sectionData.id, this.formId); + this.formConfigService.getConfigByHref(this.sectionData.config).pipe( + map((configData: ConfigData) => configData.payload), +>>>>>>> [835] Auto-save in new Item Submission form breaks the form tap((config: SubmissionFormsModel) => this.formConfig = config), flatMap(() => observableCombineLatest( @@ -265,7 +265,7 @@ export class SubmissionSectionformComponent extends SectionModelComponent { Object.keys(diffObj) .forEach((key) => { diffObj[key].forEach((value) => { - if (value.hasOwnProperty('value') && !isEmpty(value.value)) { + if (value.hasOwnProperty('value') && findIndex(this.formData[key], { value: value.value }) < 0) { diffResult.push(value); } }); @@ -288,7 +288,6 @@ export class SubmissionSectionformComponent extends SectionModelComponent { sectionData, this.submissionService.getSubmissionScope() ); - this.formBuilderService.enrichWithAdditionalData(this.formModel, this.formAdditionalData); this.sectionMetadata = this.sectionService.computeSectionConfiguredMetadata(this.formConfig); } catch (e) { @@ -341,9 +340,6 @@ export class SubmissionSectionformComponent extends SectionModelComponent { this.formService.isFormInitialized(this.formId).pipe( find((status: boolean) => status === true && !this.isUpdating)) .subscribe(() => { - - // TODO: filter these errors to only those that had been touched - this.sectionService.checkSectionErrors(this.submissionId, this.sectionData.id, this.formId, errors, this.sectionData.errors); this.sectionData.errors = errors; this.cdr.detectChanges(); @@ -364,12 +360,6 @@ export class SubmissionSectionformComponent extends SectionModelComponent { this.formData = formData; }), - this.formService.getFormAdditionalData(this.formId).pipe( - distinctUntilChanged()) - .subscribe((formAdditional) => { - this.formAdditionalData = formAdditional; - }), - /** * Subscribe to section state */ diff --git a/src/app/submission/sections/sections.service.ts b/src/app/submission/sections/sections.service.ts index 895bde13ec..7b9701671c 100644 --- a/src/app/submission/sections/sections.service.ts +++ b/src/app/submission/sections/sections.service.ts @@ -15,6 +15,7 @@ import { InertSectionErrorsAction, RemoveSectionErrorsAction, SectionStatusChangeAction, + SetSectionFormId, UpdateSectionDataAction } from '../objects/submission-objects.actions'; import { @@ -135,6 +136,18 @@ export class SectionsService { this.store.dispatch(new RemoveSectionErrorsAction(submissionId, sectionId)); } + /** + * Dispatch a new [SetSectionFormId] + * The submission id + * @param sectionId + * The section id + * @param formId + * The form id + */ + public dispatchSetSectionFormId(submissionId, sectionId, formId) { + this.store.dispatch(new SetSectionFormId(submissionId, sectionId, formId)); + } + /** * Return the data object for the specified section *