diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html index d267070281..a5d6d63418 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/date-picker/date-picker.component.html @@ -8,7 +8,7 @@ [size]="4" [(ngModel)]="initialYear" [value]="year" - [class.is-invalid]="showErrorMessages" + [invalid]="showErrorMessages" [placeholder]='yearPlaceholder' (blur)="onBlur($event)" (change)="onChange($event)" diff --git a/src/app/shared/form/form.component.ts b/src/app/shared/form/form.component.ts index 8d75d7f13a..fa3d6ae93c 100644 --- a/src/app/shared/form/form.component.ts +++ b/src/app/shared/form/form.component.ts @@ -254,6 +254,13 @@ export class FormComponent implements OnDestroy, OnInit { onBlur(event: DynamicFormControlEvent): void { this.blur.emit(event); + const control: FormControl = event.control; + const fieldIndex: number = (event.context && event.context.index) ? event.context.index : 0; + if (control.valid) { + this.formService.removeError(this.formId, event.model.name, fieldIndex); + } else { + this.formService.addControlErrors(control, this.formId, event.model.name, fieldIndex); + } } onCustomEvent(event: any) { diff --git a/src/app/shared/form/form.reducer.ts b/src/app/shared/form/form.reducer.ts index 982f5579b2..22094b2e5d 100644 --- a/src/app/shared/form/form.reducer.ts +++ b/src/app/shared/form/form.reducer.ts @@ -2,10 +2,13 @@ import { FormAction, FormActionTypes, FormAddError, - FormChangeAction, FormClearErrorsAction, + FormAddTouchedAction, + FormChangeAction, + FormClearErrorsAction, FormInitAction, - FormRemoveAction, FormRemoveErrorAction, - FormStatusChangeAction, FormAddTouchedAction + FormRemoveAction, + FormRemoveErrorAction, + FormStatusChangeAction } from './form.actions'; import { hasValue } from '../empty.util'; import { isEqual, uniqWith } from 'lodash'; @@ -82,12 +85,16 @@ function addFormErrors(state: FormState, action: FormAddError) { fieldIndex: action.payload.fieldIndex, message: action.payload.errorMessage }; - + const metadata = action.payload.fieldId.replace(/\_/g, '.'); + const touched = Object.assign({}, state[formId].touched, { + [metadata]: true + }); return Object.assign({}, state, { [formId]: { data: state[formId].data, valid: state[formId].valid, errors: state[formId].errors ? uniqWith(state[formId].errors.concat(error), isEqual) : [].concat(error), + touched } }); } else { diff --git a/src/app/shared/form/form.service.ts b/src/app/shared/form/form.service.ts index 05e7763cb0..8a7eb184a8 100644 --- a/src/app/shared/form/form.service.ts +++ b/src/app/shared/form/form.service.ts @@ -1,4 +1,4 @@ -import { map, distinctUntilChanged, filter } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map } from 'rxjs/operators'; import { Injectable } from '@angular/core'; import { AbstractControl, FormArray, FormControl, FormGroup } from '@angular/forms'; import { Observable } from 'rxjs'; @@ -11,12 +11,15 @@ import { DynamicFormControlEvent, DynamicFormControlModel } from '@ng-dynamic-fo import { isEmpty, isNotUndefined } from '../empty.util'; import { uniqueId } from 'lodash'; import { + FormAddError, + FormAddTouchedAction, FormChangeAction, FormInitAction, - FormRemoveAction, FormRemoveErrorAction, FormAddTouchedAction, + FormRemoveAction, + FormRemoveErrorAction, FormStatusChangeAction } from './form.actions'; -import { FormEntry, FormTouchedState } from './form.reducer'; +import { FormEntry, FormError, FormTouchedState } from './form.reducer'; import { environment } from '../../../environments/environment'; @Injectable() @@ -66,7 +69,7 @@ export class FormService { /** * Method to retrieve form's errors from state */ - public getFormErrors(formId: string): Observable { + public getFormErrors(formId: string): Observable { return this.store.pipe( select(formObjectFromIdSelector(formId)), filter((state) => isNotUndefined(state)), @@ -105,6 +108,34 @@ export class FormService { }); } + /** + * Check if form group has an invalid form control + * @param formGroup The form group to check + */ + public hasValidationErrors(formGroup: FormGroup | FormArray): boolean { + let hasErrors = false; + const fields: string[] = Object.keys(formGroup.controls); + for (const field of fields) { + const control = formGroup.get(field); + if (control instanceof FormControl) { + hasErrors = !control.valid && control.touched; + } else if (control instanceof FormGroup || control instanceof FormArray) { + hasErrors = this.hasValidationErrors(control); + } + if (hasErrors) { + break; + } + } + return hasErrors; + } + + public addControlErrors(field: AbstractControl, formId: string, fieldId: string, fieldIndex: number) { + const errors: string[] = Object.keys(field.errors) + .filter((errorKey) => field.errors[errorKey] === true) + .map((errorKey) => `error.validation.${errorKey}`); + errors.forEach((error) => this.addError(formId, fieldId, fieldIndex, error)); + } + public addErrorToField(field: AbstractControl, model: DynamicFormControlModel, message: string) { const error = {}; // create the error object const errorKey = this.getValidatorNameFromMap(message); @@ -186,7 +217,12 @@ export class FormService { this.store.dispatch(new FormAddTouchedAction(formId, ids)); } - public removeError(formId: string, eventModelId: string, fieldIndex: number) { - this.store.dispatch(new FormRemoveErrorAction(formId, eventModelId, fieldIndex)); + public addError(formId: string, fieldId: string, fieldIndex: number, message: string) { + const normalizedFieldId = fieldId.replace(/\./g, '_'); + this.store.dispatch(new FormAddError(formId, normalizedFieldId, fieldIndex, message)); + } + public removeError(formId: string, fieldId: string, fieldIndex: number) { + const normalizedFieldId = fieldId.replace(/\./g, '_'); + this.store.dispatch(new FormRemoveErrorAction(formId, normalizedFieldId, fieldIndex)); } } diff --git a/src/app/shared/mocks/form-service.mock.ts b/src/app/shared/mocks/form-service.mock.ts index d0510f3a68..0475250627 100644 --- a/src/app/shared/mocks/form-service.mock.ts +++ b/src/app/shared/mocks/form-service.mock.ts @@ -17,7 +17,9 @@ export function getMockFormService( resetForm: {}, validateAllFormFields: jasmine.createSpy('validateAllFormFields'), isValid: jasmine.createSpy('isValid'), - isFormInitialized: observableOf(true) + isFormInitialized: observableOf(true), + addError: jasmine.createSpy('addError'), + removeError: jasmine.createSpy('removeError'), }); } diff --git a/src/app/shared/mocks/submission.mock.ts b/src/app/shared/mocks/submission.mock.ts index eaebb38df8..04461598ce 100644 --- a/src/app/shared/mocks/submission.mock.ts +++ b/src/app/shared/mocks/submission.mock.ts @@ -60,7 +60,7 @@ export const mockSectionsErrors = [ } ]; -export const mockSectionsErrorsTwo = [ +export const mockSectionsErrorsTouchedField = [ { message: 'error.validation.required', paths: [ @@ -1020,7 +1020,127 @@ export const mockSubmissionState: SubmissionObjectState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + isLoading: false, + isValid: false, + removePending: false + } as any, + 'collection': { + config: '', + mandatory: true, + sectionType: 'collection', + visibility: { + main: 'HIDDEN', + other: 'HIDDEN' + }, + collapsed: false, + enabled: true, + data: {}, + errorsToShow: [], + isLoading: false, + isValid: false, + removePending: false + } as any, + 'traditionalpageone': { + header: 'submit.progressbar.describe.stepone', + config: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/traditionalpageone', + mandatory: true, + sectionType: 'submission-form', + collapsed: false, + enabled: true, + data: {}, + errorsToShow: [], + formId: '2_traditionalpageone', + isLoading: false, + isValid: false, + removePending: false + } as any, + 'traditionalpagetwo': { + header: 'submit.progressbar.describe.steptwo', + config: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/traditionalpagetwo', + mandatory: false, + sectionType: 'submission-form', + collapsed: false, + enabled: false, + data: {}, + errorsToShow: [], + isLoading: false, + isValid: false, + removePending: false + } as any, + 'detect-duplicate': { + header: 'submit.progressbar.detect-duplicate', + config: '', + mandatory: true, + sectionType: 'detect-duplicate', + collapsed: false, + enabled: true, + data: { + matches: {} + }, + errorsToShow: [], + isLoading: false, + isValid: false, + removePending: false + } as any, + 'upload': { + header: 'submit.progressbar.upload', + config: 'https://rest.api/dspace-spring-rest/api/config/submissionuploads/upload', + mandatory: true, + sectionType: 'upload', + collapsed: false, + enabled: true, + data: { + files: [] + }, + errorsToShow: [], + isLoading: false, + isValid: false, + removePending: false + } as any, + 'license': { + header: 'submit.progressbar.license', + config: '', + mandatory: true, + sectionType: 'license', + visibility: { + main: null, + other: 'READONLY' + }, + collapsed: false, + enabled: true, + data: {}, + errorsToShow: [], + isLoading: false, + isValid: false, + removePending: false + } as any + }, + isLoading: false, + savePending: false, + depositPending: false + } +}); + +export const mockSubmissionStateWithDuplicate: SubmissionObjectState = Object.assign({}, { + 826: { + collection: mockSubmissionCollectionId, + definition: 'traditional', + selfUrl: mockSubmissionSelfUrl, + activeSection: null, + sections: { + 'extraction': { + config: '', + mandatory: true, + sectionType: 'utils', + visibility: { + main: 'HIDDEN', + other: 'HIDDEN' + }, + collapsed: false, + enabled: true, + data: {}, + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1035,7 +1155,7 @@ export const mockSubmissionState: SubmissionObjectState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1047,7 +1167,7 @@ export const mockSubmissionState: SubmissionObjectState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], formId: '2_traditionalpageone', isLoading: false, isValid: false @@ -1060,7 +1180,7 @@ export const mockSubmissionState: SubmissionObjectState = Object.assign({}, { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1074,7 +1194,7 @@ export const mockSubmissionState: SubmissionObjectState = Object.assign({}, { data: { files: [] }, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1090,7 +1210,7 @@ export const mockSubmissionState: SubmissionObjectState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any @@ -1119,7 +1239,7 @@ export const mockSubmissionStateWithoutUpload: SubmissionObjectState = Object.as collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1134,7 +1254,7 @@ export const mockSubmissionStateWithoutUpload: SubmissionObjectState = Object.as collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1146,7 +1266,7 @@ export const mockSubmissionStateWithoutUpload: SubmissionObjectState = Object.as collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], formId: '2_traditionalpageone', isLoading: false, isValid: false @@ -1159,7 +1279,7 @@ export const mockSubmissionStateWithoutUpload: SubmissionObjectState = Object.as collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1175,7 +1295,7 @@ export const mockSubmissionStateWithoutUpload: SubmissionObjectState = Object.as collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any @@ -1198,7 +1318,7 @@ export const mockSectionsState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1213,7 +1333,7 @@ export const mockSectionsState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1225,7 +1345,7 @@ export const mockSectionsState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1237,7 +1357,7 @@ export const mockSectionsState = Object.assign({}, { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1249,7 +1369,7 @@ export const mockSectionsState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any, @@ -1265,7 +1385,7 @@ export const mockSectionsState = Object.assign({}, { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], isLoading: false, isValid: false } as any diff --git a/src/app/shared/testing/sections-service.stub.ts b/src/app/shared/testing/sections-service.stub.ts index c372cceadd..1628453bc8 100644 --- a/src/app/shared/testing/sections-service.stub.ts +++ b/src/app/shared/testing/sections-service.stub.ts @@ -18,4 +18,6 @@ export class SectionsServiceStub { setSectionError = jasmine.createSpy('setSectionError'); setSectionStatus = jasmine.createSpy('setSectionStatus'); computeSectionConfiguredMetadata = jasmine.createSpy('computeSectionConfiguredMetadata'); + getShownSectionErrors = jasmine.createSpy('getShownSectionErrors'); + getSectionServerErrors = jasmine.createSpy('getSectionServerErrors'); } diff --git a/src/app/submission/edit/submission-edit.component.html b/src/app/submission/edit/submission-edit.component.html index 19bcd6f079..b74d3861f8 100644 --- a/src/app/submission/edit/submission-edit.component.html +++ b/src/app/submission/edit/submission-edit.component.html @@ -3,6 +3,7 @@ [sections]="sections" [selfUrl]="selfUrl" [submissionDefinition]="submissionDefinition" + [submissionErrors]="submissionErrors" [item]="item" [submissionId]="submissionId"> diff --git a/src/app/submission/edit/submission-edit.component.ts b/src/app/submission/edit/submission-edit.component.ts index 154e73a765..c415b89b81 100644 --- a/src/app/submission/edit/submission-edit.component.ts +++ b/src/app/submission/edit/submission-edit.component.ts @@ -2,11 +2,11 @@ import { ChangeDetectorRef, Component, OnDestroy, OnInit } from '@angular/core'; import { ActivatedRoute, ParamMap, Router } from '@angular/router'; import { Subscription } from 'rxjs'; -import { filter, switchMap, debounceTime } from 'rxjs/operators'; +import { debounceTime, filter, switchMap } from 'rxjs/operators'; import { TranslateService } from '@ngx-translate/core'; import { WorkspaceitemSectionsObject } from '../../core/submission/models/workspaceitem-sections.model'; -import { hasValue, isEmpty, isNotNull, isNotEmptyOperator } from '../../shared/empty.util'; +import { hasValue, isEmpty, isNotEmptyOperator, isNotNull } from '../../shared/empty.util'; import { SubmissionDefinitionsModel } from '../../core/config/models/config-submission-definitions.model'; import { SubmissionService } from '../submission.service'; import { NotificationsService } from '../../shared/notifications/notifications.service'; @@ -18,6 +18,8 @@ import { getAllSucceededRemoteData } from '../../core/shared/operators'; import { ItemDataService } from '../../core/data/item-data.service'; import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; import { SubmissionJsonPatchOperationsService } from '../../core/submission/submission-json-patch-operations.service'; +import { SubmissionError } from '../objects/submission-objects.reducer'; +import parseSectionErrors from '../utils/parseSectionErrors'; /** * This component allows to edit an existing workspaceitem/workflowitem. @@ -53,6 +55,12 @@ export class SubmissionEditComponent implements OnDestroy, OnInit { */ public submissionDefinition: SubmissionDefinitionsModel; + /** + * The submission errors present in the submission object + * @type {SubmissionError} + */ + public submissionErrors: SubmissionError; + /** * The submission id * @type {string} @@ -86,6 +94,7 @@ export class SubmissionEditComponent implements OnDestroy, OnInit { * @param {ItemDataService} itemDataService * @param {SubmissionService} submissionService * @param {TranslateService} translate + * @param {SubmissionJsonPatchOperationsService} submissionJsonPatchOperationsService */ constructor(private changeDetectorRef: ChangeDetectorRef, private notificationsService: NotificationsService, @@ -112,6 +121,8 @@ export class SubmissionEditComponent implements OnDestroy, OnInit { this.notificationsService.info(null, this.translate.get('submission.general.cannot_submit')); this.router.navigate(['/mydspace']); } else { + const { errors } = submissionObjectRD.payload; + this.submissionErrors = parseSectionErrors(errors); this.submissionId = submissionObjectRD.payload.id.toString(); this.collectionId = (submissionObjectRD.payload.collection as Collection).id; this.selfUrl = submissionObjectRD.payload._links.self.href; diff --git a/src/app/submission/form/submission-form.component.spec.ts b/src/app/submission/form/submission-form.component.spec.ts index dd8e6d0ea3..bd504e09c1 100644 --- a/src/app/submission/form/submission-form.component.spec.ts +++ b/src/app/submission/form/submission-form.component.spec.ts @@ -126,6 +126,7 @@ describe('SubmissionFormComponent Component', () => { comp.submissionDefinition = submissionDefinition; comp.selfUrl = selfUrl; comp.sections = sectionsData; + comp.submissionErrors = null; comp.item = new Item(); submissionServiceStub.getSubmissionObject.and.returnValue(observableOf(submissionState)); diff --git a/src/app/submission/form/submission-form.component.ts b/src/app/submission/form/submission-form.component.ts index 6d4ddb4ca0..4cbffbca78 100644 --- a/src/app/submission/form/submission-form.component.ts +++ b/src/app/submission/form/submission-form.component.ts @@ -11,7 +11,7 @@ import { WorkspaceitemSectionsObject } from '../../core/submission/models/worksp import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { UploaderOptions } from '../../shared/uploader/uploader-options.model'; -import { SubmissionObjectEntry } from '../objects/submission-objects.reducer'; +import { SubmissionError, SubmissionObjectEntry } from '../objects/submission-objects.reducer'; import { SectionDataObject } from '../sections/models/section-data.model'; import { SubmissionService } from '../submission.service'; import { Item } from '../../core/shared/item.model'; @@ -41,6 +41,12 @@ export class SubmissionFormComponent implements OnChanges, OnDestroy { */ @Input() sections: WorkspaceitemSectionsObject; + /** + * The submission errors present in the submission object + * @type {SubmissionError} + */ + @Input() submissionErrors: SubmissionError; + /** * The submission self url * @type {string} @@ -163,7 +169,7 @@ export class SubmissionFormComponent implements OnChanges, OnDestroy { this.submissionDefinition, this.sections, this.item, - null); + this.submissionErrors); this.changeDetectorRef.detectChanges(); }) ); diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts index 823cbb5d82..bcdd01d5a1 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.spec.ts @@ -1,5 +1,5 @@ import { ChangeDetectorRef, Component, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; -import { waitForAsync, ComponentFixture, inject, TestBed } from '@angular/core/testing'; +import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing'; import { of as observableOf } from 'rxjs'; import { TranslateModule, TranslateService } from '@ngx-translate/core'; @@ -165,6 +165,7 @@ describe('SubmissionUploadFilesComponent Component', () => { submissionId, sectionId, mockSectionsData[sectionId], + expectedErrors[sectionId], expectedErrors[sectionId] ); }); @@ -188,6 +189,7 @@ describe('SubmissionUploadFilesComponent Component', () => { submissionId, sectionId, mockSectionsData[sectionId], + expectedErrors[sectionId], expectedErrors[sectionId] ); }); diff --git a/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts b/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts index b1b5051458..fa81fe2bbd 100644 --- a/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts +++ b/src/app/submission/form/submission-upload-files/submission-upload-files.component.ts @@ -143,7 +143,7 @@ export class SubmissionUploadFilesComponent implements OnChanges { } } }); - this.sectionService.updateSectionData(this.submissionId, sectionId, sectionData, sectionErrors); + this.sectionService.updateSectionData(this.submissionId, sectionId, sectionData, sectionErrors, sectionErrors); }); } diff --git a/src/app/submission/objects/submission-objects.actions.ts b/src/app/submission/objects/submission-objects.actions.ts index 49c3420e6e..4935acb792 100644 --- a/src/app/submission/objects/submission-objects.actions.ts +++ b/src/app/submission/objects/submission-objects.actions.ts @@ -1,7 +1,7 @@ import { Action } from '@ngrx/store'; import { type } from '../../shared/ngrx/type'; -import { SectionVisibility, SubmissionSectionError } from './submission-objects.reducer'; +import { SectionVisibility, SubmissionError, SubmissionSectionError } from './submission-objects.reducer'; import { WorkspaceitemSectionUploadFileObject } from '../../core/submission/models/workspaceitem-section-upload-file.model'; import { WorkspaceitemSectionDataType, @@ -206,7 +206,8 @@ export class UpdateSectionDataAction implements Action { submissionId: string; sectionId: string; data: WorkspaceitemSectionDataType; - errors: SubmissionSectionError[]; + errorsToShow: SubmissionSectionError[]; + serverValidationErrors: SubmissionSectionError[]; metadata: string[]; }; @@ -219,17 +220,20 @@ export class UpdateSectionDataAction implements Action { * the section's ID to add * @param data * the section's data - * @param errors - * the section's errors + * @param errorsToShow + * the list of the section's errors to show + * @param serverValidationErrors + * the list of the section errors detected by the server * @param metadata * the section's metadata */ constructor(submissionId: string, sectionId: string, data: WorkspaceitemSectionDataType, - errors: SubmissionSectionError[], + errorsToShow: SubmissionSectionError[], + serverValidationErrors: SubmissionSectionError[], metadata?: string[]) { - this.payload = { submissionId, sectionId, data, errors, metadata }; + this.payload = { submissionId, sectionId, data, errorsToShow, serverValidationErrors, metadata }; } } @@ -308,7 +312,7 @@ export class InitSubmissionFormAction implements Action { submissionDefinition: SubmissionDefinitionsModel; sections: WorkspaceitemSectionsObject; item: Item; - errors: SubmissionSectionError[]; + errors: SubmissionError; }; /** @@ -333,7 +337,7 @@ export class InitSubmissionFormAction implements Action { submissionDefinition: SubmissionDefinitionsModel, sections: WorkspaceitemSectionsObject, item: Item, - errors: SubmissionSectionError[]) { + errors: SubmissionError) { this.payload = { collectionId, submissionId, selfUrl, submissionDefinition, sections, item, errors }; } } diff --git a/src/app/submission/objects/submission-objects.effects.spec.ts b/src/app/submission/objects/submission-objects.effects.spec.ts index 3ed4a4450d..122ebd90ac 100644 --- a/src/app/submission/objects/submission-objects.effects.spec.ts +++ b/src/app/submission/objects/submission-objects.effects.spec.ts @@ -28,7 +28,7 @@ import { mockSectionsData, mockSectionsDataTwo, mockSectionsErrors, - mockSectionsErrorsTwo, + mockSectionsErrorsTouchedField, mockSubmissionCollectionId, mockSubmissionDefinition, mockSubmissionDefinitionResponse, @@ -358,18 +358,21 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, + errorsList.traditionalpageone || [], errorsList.traditionalpageone || [] ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, + errorsList.license || [], errorsList.license || [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, + errorsList.upload || [], errorsList.upload || [] ), }); @@ -408,25 +411,29 @@ describe('SubmissionObjectEffects test suite', () => { } }); - const errorsList = parseSectionErrors(mockSectionsErrorsTwo); + const errorsToShowList = parseSectionErrors(mockSectionsErrorsTouchedField); + const serverValidationErrorsList = parseSectionErrors(mockSectionsErrors); const expected = cold('--(bcd)-', { b: new UpdateSectionDataAction( submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, - errorsList.traditionalpageone + errorsToShowList.traditionalpageone, + serverValidationErrorsList.traditionalpageone ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, - errorsList.license || [] + errorsToShowList.license || [], + serverValidationErrorsList.license || [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, - errorsList.upload || [] + errorsToShowList.upload || [], + serverValidationErrorsList.upload || [] ), }); @@ -459,18 +466,21 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, + [], [] ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, + [], [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, + [], [] ), }); @@ -506,18 +516,21 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, + errorsList.traditionalpageone || [], errorsList.traditionalpageone || [] ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, + errorsList.license || [], errorsList.license || [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, + errorsList.upload || [], errorsList.upload || [] ), }); @@ -553,24 +566,28 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsDataTwo.traditionalpageone as any, + errorsList.traditionalpageone || [], errorsList.traditionalpageone || [] ), c: new UpdateSectionDataAction( submissionId, 'traditionalpagetwo', mockSectionsDataTwo.traditionalpagetwo as any, + errorsList.traditionalpagetwo || [], errorsList.traditionalpagetwo || [] ), d: new UpdateSectionDataAction( submissionId, 'license', mockSectionsDataTwo.license as any, + errorsList.license || [], errorsList.license || [] ), e: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsDataTwo.upload as any, + errorsList.upload || [], errorsList.upload || [] ), }); @@ -610,18 +627,21 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, - [] + [], + errorsList.traditionalpageone ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, + errorsList.license || [], errorsList.license || [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, + errorsList.upload || [], errorsList.upload || [] ), }); @@ -655,18 +675,21 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, + [], [] ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, + [], [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, + [], [] ), }); @@ -696,26 +719,28 @@ describe('SubmissionObjectEffects test suite', () => { } }); - const errorsList = parseSectionErrors(mockSectionsErrors); - console.log(errorsList); + const serverValidationErrorsList = parseSectionErrors(mockSectionsErrors); const expected = cold('--(bcd)-', { b: new UpdateSectionDataAction( submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, - [] + [], + serverValidationErrorsList.traditionalpageone ), c: new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, - errorsList.license || [] + serverValidationErrorsList.license || [], + serverValidationErrorsList.license || [] ), d: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, - errorsList.upload || [] + serverValidationErrorsList.upload || [], + serverValidationErrorsList.upload || [] ), }); @@ -750,24 +775,28 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsDataTwo.traditionalpageone as any, - [] + [], + errorsList.traditionalpageone ), c: new UpdateSectionDataAction( submissionId, 'traditionalpagetwo', mockSectionsDataTwo.traditionalpagetwo as any, + errorsList.traditionalpagetwo || [], errorsList.traditionalpagetwo || [] ), d: new UpdateSectionDataAction( submissionId, 'license', mockSectionsDataTwo.license as any, + errorsList.license || [], errorsList.license || [] ), e: new UpdateSectionDataAction( submissionId, 'upload', mockSectionsDataTwo.upload as any, + errorsList.upload || [], errorsList.upload || [] ), }); @@ -880,18 +909,21 @@ describe('SubmissionObjectEffects test suite', () => { submissionId, 'traditionalpageone', mockSectionsData.traditionalpageone as any, + errorsList.traditionalpageone || [], errorsList.traditionalpageone || [] ), new UpdateSectionDataAction( submissionId, 'license', mockSectionsData.license as any, + errorsList.license || [], errorsList.license || [] ), new UpdateSectionDataAction( submissionId, 'upload', mockSectionsData.upload as any, + errorsList.upload || [], errorsList.upload || [] ) ] diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index bc7bf45dba..b4ba1c2480 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -5,16 +5,7 @@ import { TranslateService } from '@ngx-translate/core'; import { isEqual, union } from 'lodash'; import { from as observableFrom, Observable, of as observableOf } from 'rxjs'; -import { - catchError, - filter, - map, - mergeMap, - switchMap, - take, - tap, - withLatestFrom -} from 'rxjs/operators'; +import { catchError, filter, map, mergeMap, switchMap, take, tap, withLatestFrom } from 'rxjs/operators'; import { SubmissionObject } from '../../core/submission/models/submission-object.model'; import { WorkflowItem } from '../../core/submission/models/workflowitem.model'; import { WorkspaceitemSectionUploadObject } from '../../core/submission/models/workspaceitem-section-upload.model'; @@ -52,13 +43,13 @@ import { UpdateSectionDataAction, UpdateSectionDataSuccessAction } from './submission-objects.actions'; -import {SubmissionObjectEntry, SubmissionSectionError, 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'; +import parseSectionErrorPaths, { SectionErrorPath } from '../utils/parseSectionErrorPaths'; import { FormState } from '../../shared/form/form.reducer'; @Injectable() @@ -83,7 +74,7 @@ export class SubmissionObjectEffects { } else { sectionData = action.payload.item.metadata; } - const sectionErrors = null; + const sectionErrors = isNotEmpty(action.payload.errors) ? (action.payload.errors[sectionId] || null) : null; mappedActions.push( new InitSectionAction( action.payload.submissionId, @@ -232,10 +223,7 @@ export class SubmissionObjectEffects { switchMap(([action, state]: [DepositSubmissionAction, any]) => { return this.submissionService.depositSubmission(state.submission.objects[action.payload.submissionId].selfUrl).pipe( map(() => new DepositSubmissionSuccessAction(action.payload.submissionId)), - catchError((error) => { - console.log('submission error', error); - return observableOf(new DepositSubmissionErrorAction(action.payload.submissionId)); - })); + catchError((error) => observableOf(new DepositSubmissionErrorAction(action.payload.submissionId)))); })); /** @@ -297,7 +285,7 @@ export class SubmissionObjectEffects { return item$.pipe( map((item: Item) => item.metadata), filter((metadata) => !isEqual(action.payload.data, metadata)), - map((metadata: any) => new UpdateSectionDataAction(action.payload.submissionId, action.payload.sectionId, metadata, action.payload.errors, action.payload.metadata)) + map((metadata: any) => new UpdateSectionDataAction(action.payload.submissionId, action.payload.sectionId, metadata, action.payload.errorsToShow, action.payload.serverValidationErrors, action.payload.metadata)) ); } else { return observableOf(new UpdateSectionDataSuccessAction()); @@ -413,7 +401,7 @@ export class SubmissionObjectEffects { const sectionForm = getForm(forms, currentState, sectionId); const filteredErrors = filterErrors(sectionForm, sectionErrors, currentState.sections[sectionId].sectionType, notify); - mappedActions.push(new UpdateSectionDataAction(submissionId, sectionId, sectionData, filteredErrors)); + mappedActions.push(new UpdateSectionDataAction(submissionId, sectionId, sectionData, filteredErrors, sectionErrors)); } }); } diff --git a/src/app/submission/objects/submission-objects.reducer.spec.ts b/src/app/submission/objects/submission-objects.reducer.spec.ts index 0431cdff79..2a24afae19 100644 --- a/src/app/submission/objects/submission-objects.reducer.spec.ts +++ b/src/app/submission/objects/submission-objects.reducer.spec.ts @@ -28,7 +28,8 @@ import { SaveSubmissionSectionFormAction, SaveSubmissionSectionFormErrorAction, SaveSubmissionSectionFormSuccessAction, - SectionStatusChangeAction, SubmissionObjectAction, + SectionStatusChangeAction, + SubmissionObjectAction, UpdateSectionDataAction } from './submission-objects.actions'; import { SectionsType } from '../sections/sections-type'; @@ -68,7 +69,7 @@ describe('submissionReducer test suite', () => { } }; - const action = new InitSubmissionFormAction(collectionId, submissionId, selfUrl, submissionDefinition, {}, new Item(), []); + const action = new InitSubmissionFormAction(collectionId, submissionId, selfUrl, submissionDefinition, {}, new Item(), null); const newState = submissionObjectReducer({}, action); expect(newState).toEqual(expectedState); @@ -237,12 +238,13 @@ describe('submissionReducer test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, - isValid: false + isValid: true } as any; - let action: any = new InitSubmissionFormAction(collectionId, submissionId, selfUrl, submissionDefinition, {}, new Item(), []); + let action: any = new InitSubmissionFormAction(collectionId, submissionId, selfUrl, submissionDefinition, {}, new Item(), null); let newState = submissionObjectReducer({}, action); action = new InitSectionAction( @@ -329,7 +331,7 @@ describe('submissionReducer test suite', () => { ] } as any; - const action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', data, []); + const action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', data, [], []); const newState = submissionObjectReducer(initState, action); expect(newState[826].sections.traditionalpageone.data).toEqual(data); @@ -340,7 +342,7 @@ describe('submissionReducer test suite', () => { } as any; const metadata = ['dc.title', 'dc.contributor.author']; - const action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', data, [], metadata); + const action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', data, [], [], metadata); const newState = submissionObjectReducer(initState, action); expect(newState[826].sections.traditionalpageone.metadata).toEqual(metadata); @@ -354,10 +356,10 @@ describe('submissionReducer test suite', () => { } ]; - const action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', {}, errors); + const action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', {}, errors, errors); const newState = submissionObjectReducer(initState, action); - expect(newState[826].sections.traditionalpageone.errors).toEqual(errors); + expect(newState[826].sections.traditionalpageone.errorsToShow).toEqual(errors); }); it('should remove all submission section errors properly', () => { @@ -366,7 +368,7 @@ describe('submissionReducer test suite', () => { newState = submissionObjectReducer(initState, action); - expect(newState[826].sections.traditionalpageone.errors).toEqual([]); + expect(newState[826].sections.traditionalpageone.errorsToShow).toEqual([]); }); it('should add submission section error properly', () => { @@ -378,7 +380,7 @@ describe('submissionReducer test suite', () => { const action = new InertSectionErrorsAction(submissionId, 'traditionalpageone', error); const newState = submissionObjectReducer(initState, action); - expect(newState[826].sections.traditionalpageone.errors).toEqual([error]); + expect(newState[826].sections.traditionalpageone.errorsToShow).toEqual([error]); }); it('should remove specified submission section error/s properly', () => { @@ -402,21 +404,21 @@ describe('submissionReducer test suite', () => { message: 'error.validation.required' }]; - let action: any = new UpdateSectionDataAction(submissionId, 'traditionalpageone', {}, errors); + let action: any = new UpdateSectionDataAction(submissionId, 'traditionalpageone', {}, errors, errors); let newState = submissionObjectReducer(initState, action); action = new DeleteSectionErrorsAction(submissionId, 'traditionalpageone', error); newState = submissionObjectReducer(newState, action); - expect(newState[826].sections.traditionalpageone.errors).toEqual(expectedErrors); + expect(newState[826].sections.traditionalpageone.errorsToShow).toEqual(expectedErrors); - action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', {}, errors); + action = new UpdateSectionDataAction(submissionId, 'traditionalpageone', {}, errors, errors); newState = submissionObjectReducer(initState, action); action = new DeleteSectionErrorsAction(submissionId, 'traditionalpageone', errors); newState = submissionObjectReducer(newState, action); - expect(newState[826].sections.traditionalpageone.errors).toEqual([]); + expect(newState[826].sections.traditionalpageone.errorsToShow).toEqual([]); }); it('should add a new file', () => { diff --git a/src/app/submission/objects/submission-objects.reducer.ts b/src/app/submission/objects/submission-objects.reducer.ts index a82bc72f4e..4159c56ae1 100644 --- a/src/app/submission/objects/submission-objects.reducer.ts +++ b/src/app/submission/objects/submission-objects.reducer.ts @@ -1,4 +1,4 @@ -import { hasValue, isNotEmpty, isNotNull, isUndefined } from '../../shared/empty.util'; +import { hasValue, isEmpty, isNotEmpty, isNotNull, isUndefined } from '../../shared/empty.util'; import { differenceWith, findKey, isEqual, uniqWith } from 'lodash'; import { @@ -97,9 +97,14 @@ export interface SubmissionSectionObject { data: WorkspaceitemSectionDataType; /** - * The list of the section errors + * The list of the section's errors to show. It contains the error list to display when section is not pristine */ - errors: SubmissionSectionError[]; + errorsToShow: SubmissionSectionError[]; + + /** + * The list of the section's errors detected by the server. They may not be shown yet if section is pristine + */ + serverValidationErrors: SubmissionSectionError[]; /** * A boolean representing if this section is loading @@ -117,6 +122,13 @@ export interface SubmissionSectionObject { formId: string; } +/** + * An interface to represent section error + */ +export interface SubmissionError { + [submissionId: string]: SubmissionSectionError[]; +} + /** * An interface to represent section error */ @@ -332,7 +344,7 @@ const removeError = (state: SubmissionObjectState, action: DeleteSectionErrorsAc if (Array.isArray(errors)) { filteredErrors = differenceWith(errors, errors, isEqual); } else { - filteredErrors = state[ submissionId ].sections[ sectionId ].errors + filteredErrors = state[ submissionId ].sections[ sectionId ].errorsToShow .filter((currentError) => currentError.path !== errors.path || !isEqual(currentError, errors)); } @@ -340,7 +352,7 @@ const removeError = (state: SubmissionObjectState, action: DeleteSectionErrorsAc [ submissionId ]: Object.assign({}, state[ submissionId ], { sections: Object.assign({}, state[ submissionId ].sections, { [ sectionId ]: Object.assign({}, state[ submissionId ].sections [ sectionId ], { - errors: filteredErrors + errorsToShow: filteredErrors }) }) }) @@ -354,13 +366,13 @@ const addError = (state: SubmissionObjectState, action: InertSectionErrorsAction const { submissionId, sectionId, error } = action.payload; if (hasValue(state[ submissionId ].sections[ sectionId ])) { - const errors = uniqWith(state[ submissionId ].sections[ sectionId ].errors.concat(error), isEqual); + const errorsToShow = uniqWith(state[ submissionId ].sections[ sectionId ].errorsToShow.concat(error), isEqual); return Object.assign({}, state, { [ submissionId ]: Object.assign({}, state[ submissionId ], { activeSection: state[ action.payload.submissionId ].activeSection, sections: Object.assign({}, state[ submissionId ].sections, { [ sectionId ]: Object.assign({}, state[ action.payload.submissionId ].sections [ action.payload.sectionId ], { - errors + errorsToShow }) }), }) @@ -378,7 +390,7 @@ const addError = (state: SubmissionObjectState, action: InertSectionErrorsAction * @param action * a RemoveSectionErrorsAction * @return SubmissionObjectState - * the new state, with the section's errors updated. + * the new state, with the section's errorsToShow updated. */ function removeSectionErrors(state: SubmissionObjectState, action: RemoveSectionErrorsAction): SubmissionObjectState { if (isNotEmpty(state[ action.payload.submissionId ]) @@ -387,7 +399,7 @@ function removeSectionErrors(state: SubmissionObjectState, action: RemoveSection [ action.payload.submissionId ]: Object.assign({}, state[ action.payload.submissionId ], { sections: Object.assign({}, state[ action.payload.submissionId ].sections, { [ action.payload.sectionId ]: Object.assign({}, state[ action.payload.submissionId ].sections [ action.payload.sectionId ], { - errors: [] + errorsToShow: [] }) }) }) @@ -644,9 +656,10 @@ function initSection(state: SubmissionObjectState, action: InitSectionAction): S collapsed: false, enabled: action.payload.enabled, data: action.payload.data, - errors: action.payload.errors || [], + errorsToShow: [], + serverValidationErrors: action.payload.errors || [], isLoading: false, - isValid: false + isValid: isEmpty(action.payload.errors) } }) }) @@ -702,7 +715,8 @@ function updateSectionData(state: SubmissionObjectState, action: UpdateSectionDa [ action.payload.sectionId ]: Object.assign({}, state[ action.payload.submissionId ].sections [ action.payload.sectionId ], { enabled: true, data: action.payload.data, - errors: action.payload.errors, + errorsToShow: action.payload.errorsToShow, + serverValidationErrors: action.payload.serverValidationErrors, metadata: reduceSectionMetadata(action.payload.metadata, state[ action.payload.submissionId ].sections [ action.payload.sectionId ].metadata) }) }) diff --git a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.spec.ts b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.spec.ts index 415e7a5be8..3757ca87b8 100644 --- a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.spec.ts +++ b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.spec.ts @@ -27,7 +27,8 @@ describe('SubmissionSectionCcLicensesComponent', () => { config: 'test config', mandatory: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], header: 'test header', id: 'test section id', sectionType: SectionsType.SubmissionForm diff --git a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts index 2c185d4a4e..9e6a5ae07f 100644 --- a/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts +++ b/src/app/submission/sections/cc-license/submission-section-cc-licenses.component.ts @@ -1,11 +1,7 @@ import { Component, Inject } from '@angular/core'; import { Observable, of as observableOf, Subscription } from 'rxjs'; -import { - Field, - Option, - SubmissionCcLicence -} from '../../../core/submission/models/submission-cc-license.model'; -import { getRemoteDataPayload, getFirstSucceededRemoteData } from '../../../core/shared/operators'; +import { Field, Option, SubmissionCcLicence } from '../../../core/submission/models/submission-cc-license.model'; +import { getFirstSucceededRemoteData, getRemoteDataPayload } from '../../../core/shared/operators'; import { distinctUntilChanged, filter, map, take } from 'rxjs/operators'; import { SubmissionCcLicenseDataService } from '../../../core/submission/submission-cc-license-data.service'; import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; @@ -234,7 +230,7 @@ export class SubmissionSectionCcLicensesComponent extends SectionModelComponent this.subscriptions.push( this.sectionService.getSectionState(this.submissionId, this.sectionData.id, SectionsType.CcLicense).pipe( filter((sectionState) => { - return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errors)); + return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errorsToShow)); }), distinctUntilChanged(), map((sectionState) => sectionState.data as WorkspaceitemSectionCcLicenseObject), diff --git a/src/app/submission/sections/container/section-container.component.spec.ts b/src/app/submission/sections/container/section-container.component.spec.ts index 324582c0c9..7568b17ea7 100644 --- a/src/app/submission/sections/container/section-container.component.spec.ts +++ b/src/app/submission/sections/container/section-container.component.spec.ts @@ -22,11 +22,12 @@ const sectionState = { header: 'submit.progressbar.describe.stepone', config: 'https://rest.api/dspace-spring-rest/api/config/submissionforms/traditionalpageone', mandatory: true, - sectionType: 'submission-form', + sectionType: SectionsType.SubmissionForm, collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false } as any; @@ -35,7 +36,8 @@ const sectionObject: SectionDataObject = { config: 'https://dspace7.4science.it/or2018/api/config/submissionforms/traditionalpageone', mandatory: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], header: 'submit.progressbar.describe.stepone', id: 'traditionalpageone', sectionType: SectionsType.SubmissionForm @@ -56,6 +58,7 @@ describe('SubmissionSectionContainerComponent test suite', () => { function init() { sectionsServiceStub.isSectionValid.and.returnValue(observableOf(true)); sectionsServiceStub.getSectionState.and.returnValue(observableOf(sectionState)); + sectionsServiceStub.getShownSectionErrors.and.returnValue(observableOf([])); submissionServiceStub.getActiveSectionId.and.returnValue(observableOf('traditionalpageone')); } diff --git a/src/app/submission/sections/form/section-form-operations.service.ts b/src/app/submission/sections/form/section-form-operations.service.ts index adba46bf3a..7c6c242d42 100644 --- a/src/app/submission/sections/form/section-form-operations.service.ts +++ b/src/app/submission/sections/form/section-form-operations.service.ts @@ -300,7 +300,6 @@ export class SectionFormOperationsService { const path = this.getFieldPathFromEvent(event); const value = this.getFieldValueFromChangeEvent(event); - console.log(value); if (this.formBuilder.isQualdropGroup(event.model as DynamicFormControlModel)) { this.dispatchOperationsFromMap(this.getQualdropValueMap(event), pathCombiner, event, previousValue); } else if (event.context && event.context instanceof DynamicFormArrayGroupModel) { diff --git a/src/app/submission/sections/form/section-form.component.spec.ts b/src/app/submission/sections/form/section-form.component.spec.ts index 126f7ac3f7..90861bce5e 100644 --- a/src/app/submission/sections/form/section-form.component.spec.ts +++ b/src/app/submission/sections/form/section-form.component.spec.ts @@ -44,6 +44,7 @@ import { SubmissionObjectDataService } from '../../../core/submission/submission import { ObjectCacheService } from '../../../core/cache/object-cache.service'; import { RequestService } from '../../../core/data/request.service'; import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { cold } from 'jasmine-marbles'; function getMockSubmissionFormsConfigService(): SubmissionFormsConfigService { return jasmine.createSpyObj('FormOperationsService', { @@ -59,7 +60,8 @@ const sectionObject: SectionDataObject = { config: 'https://dspace7.4science.it/or2018/api/config/submissionforms/traditionalpageone', mandatory: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], header: 'submit.progressbar.describe.stepone', id: 'traditionalpageone', sectionType: SectionsType.SubmissionForm @@ -200,6 +202,7 @@ describe('SubmissionSectionformComponent test suite', () => { formService.isValid.and.returnValue(observableOf(true)); formConfigService.findByHref.and.returnValue(observableOf(testFormConfiguration)); sectionsServiceStub.getSectionData.and.returnValue(observableOf(sectionData)); + sectionsServiceStub.getSectionServerErrors.and.returnValue(observableOf([])); const html = ` `; @@ -246,6 +249,7 @@ describe('SubmissionSectionformComponent test suite', () => { formService.isValid.and.returnValue(observableOf(true)); formConfigService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$(testFormConfiguration)); sectionsServiceStub.getSectionData.and.returnValue(observableOf(sectionData)); + sectionsServiceStub.getSectionServerErrors.and.returnValue(observableOf([])); spyOn(comp, 'initForm'); spyOn(comp, 'subscriptions'); @@ -253,7 +257,7 @@ describe('SubmissionSectionformComponent test suite', () => { fixture.detectChanges(); expect(compAsAny.formConfig).toEqual(testFormConfiguration); - expect(comp.sectionData.errors).toEqual([]); + expect(comp.sectionData.errorsToShow).toEqual([]); expect(comp.sectionData.data).toEqual(sectionData); expect(comp.isLoading).toBeFalsy(); expect(comp.initForm).toHaveBeenCalledWith(sectionData); @@ -322,7 +326,7 @@ describe('SubmissionSectionformComponent test suite', () => { }; const sectionError = []; comp.sectionData.data = {}; - comp.sectionData.errors = []; + comp.sectionData.errorsToShow = []; compAsAny.formData = {}; compAsAny.sectionMetadata = ['dc.title']; @@ -342,7 +346,7 @@ describe('SubmissionSectionformComponent test suite', () => { 'dc.title': [new FormFieldMetadataValueObject('test')] }; comp.sectionData.data = {}; - comp.sectionData.errors = []; + comp.sectionData.errorsToShow = []; compAsAny.formData = sectionData; compAsAny.sectionMetadata = ['dc.title']; @@ -368,7 +372,8 @@ describe('SubmissionSectionformComponent test suite', () => { it('should check for error', () => { comp.isUpdating = false; comp.formId = 'test'; - comp.sectionData.errors = []; + comp.sectionData.errorsToShow = []; + comp.sectionData.serverValidationErrors = []; comp.checksForErrors(parsedSectionErrors); @@ -379,7 +384,37 @@ describe('SubmissionSectionformComponent test suite', () => { parsedSectionErrors, [] ); - expect(comp.sectionData.errors).toEqual(parsedSectionErrors); + expect(comp.sectionData.errorsToShow).toEqual(parsedSectionErrors); + }); + + it('should return a valid status when form is valid and there are no server validation errors', () => { + formService.isValid.and.returnValue(observableOf(true)); + sectionsServiceStub.getSectionServerErrors.and.returnValue(observableOf([])); + const expected = cold('(b|)', { + b: true + }); + + expect(compAsAny.getSectionStatus()).toBeObservable(expected); + }); + + it('should return an invalid status when form is valid and there are server validation errors', () => { + formService.isValid.and.returnValue(observableOf(true)); + sectionsServiceStub.getSectionServerErrors.and.returnValue(observableOf(parsedSectionErrors)); + const expected = cold('(b|)', { + b: false + }); + + expect(compAsAny.getSectionStatus()).toBeObservable(expected); + }); + + it('should return an invalid status when form is not valid and there are no server validation errors', () => { + formService.isValid.and.returnValue(observableOf(false)); + sectionsServiceStub.getSectionServerErrors.and.returnValue(observableOf([])); + const expected = cold('(b|)', { + b: false + }); + + expect(compAsAny.getSectionStatus()).toBeObservable(expected); }); it('should subscribe to state properly', () => { @@ -392,7 +427,7 @@ describe('SubmissionSectionformComponent test suite', () => { }; const sectionState = { data: sectionData, - errors: parsedSectionErrors + errorsToShow: parsedSectionErrors }; formService.getFormData.and.returnValue(observableOf(formData)); @@ -402,7 +437,7 @@ describe('SubmissionSectionformComponent test suite', () => { expect(compAsAny.subs.length).toBe(2); expect(compAsAny.formData).toEqual(formData); - expect(comp.updateForm).toHaveBeenCalledWith(sectionState.data, sectionState.errors); + expect(comp.updateForm).toHaveBeenCalledWith(sectionState.data, sectionState.errorsToShow); }); diff --git a/src/app/submission/sections/form/section-form.component.ts b/src/app/submission/sections/form/section-form.component.ts index 99a2e12b47..8ae246dcca 100644 --- a/src/app/submission/sections/form/section-form.component.ts +++ b/src/app/submission/sections/form/section-form.component.ts @@ -2,15 +2,7 @@ import { ChangeDetectorRef, Component, Inject, ViewChild } from '@angular/core'; import { DynamicFormControlEvent, DynamicFormControlModel } from '@ng-dynamic-forms/core'; import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; -import { - distinctUntilChanged, - filter, - find, - map, - take, - tap, - mergeMap -} from 'rxjs/operators'; +import { distinctUntilChanged, filter, find, map, mergeMap, take, tap } from 'rxjs/operators'; import { TranslateService } from '@ngx-translate/core'; import { findIndex, isEqual } from 'lodash'; @@ -19,7 +11,7 @@ import { FormComponent } from '../../../shared/form/form.component'; import { FormService } from '../../../shared/form/form.service'; import { SectionModelComponent } from '../models/section.model'; import { SubmissionFormsConfigService } from '../../../core/config/submission-forms-config.service'; -import { hasValue, isNotEmpty, isUndefined } from '../../../shared/empty.util'; +import { hasValue, isEmpty, isNotEmpty, isUndefined } from '../../../shared/empty.util'; import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder/json-patch-operation-path-combiner'; import { SubmissionFormsModel } from '../../../core/config/models/config-submission-forms.model'; import { SubmissionSectionError, SubmissionSectionObject } from '../../objects/submission-objects.reducer'; @@ -183,7 +175,7 @@ export class SubmissionSectionformComponent extends SectionModelComponent { take(1)) .subscribe(([sectionData, workspaceItem]: [WorkspaceitemSectionFormObject, WorkspaceItem]) => { if (isUndefined(this.formModel)) { - this.sectionData.errors = []; + // this.sectionData.errorsToShow = []; this.workspaceItem = workspaceItem; // Is the first loading so init form this.initForm(sectionData); @@ -211,7 +203,14 @@ export class SubmissionSectionformComponent extends SectionModelComponent { * the section status */ protected getSectionStatus(): Observable { - return this.formService.isValid(this.formId); + const formStatus$ = this.formService.isValid(this.formId); + const serverValidationStatus$ = this.sectionService.getSectionServerErrors(this.submissionId, this.sectionData.id).pipe( + map((validationErrors) => isEmpty(validationErrors)) + ); + + return observableCombineLatest([formStatus$, serverValidationStatus$]).pipe( + map(([formValidation, serverSideValidation]: [boolean, boolean]) => formValidation && serverSideValidation) + ); } /** @@ -263,7 +262,7 @@ export class SubmissionSectionformComponent extends SectionModelComponent { this.submissionService.getSubmissionScope() ); const sectionMetadata = this.sectionService.computeSectionConfiguredMetadata(this.formConfig); - this.sectionService.updateSectionData(this.submissionId, this.sectionData.id, sectionData, [], sectionMetadata); + this.sectionService.updateSectionData(this.submissionId, this.sectionData.id, sectionData, this.sectionData.errorsToShow, this.sectionData.serverValidationErrors, sectionMetadata); } catch (e) { const msg: string = this.translate.instant('error.submission.sections.init-form-error') + e.toString(); @@ -296,10 +295,10 @@ export class SubmissionSectionformComponent extends SectionModelComponent { this.checksForErrors(errors); this.isUpdating = false; this.cdr.detectChanges(); - } else if (isNotEmpty(errors) || isNotEmpty(this.sectionData.errors)) { + } else if (isNotEmpty(errors) || isNotEmpty(this.sectionData.errorsToShow)) { this.checksForErrors(errors); } - } else if (isNotEmpty(errors) || isNotEmpty(this.sectionData.errors)) { + } else if (isNotEmpty(errors) || isNotEmpty(this.sectionData.errorsToShow)) { this.checksForErrors(errors); } @@ -315,8 +314,8 @@ export class SubmissionSectionformComponent extends SectionModelComponent { this.formService.isFormInitialized(this.formId).pipe( find((status: boolean) => status === true && !this.isUpdating)) .subscribe(() => { - this.sectionService.checkSectionErrors(this.submissionId, this.sectionData.id, this.formId, errors, this.sectionData.errors); - this.sectionData.errors = errors; + this.sectionService.checkSectionErrors(this.submissionId, this.sectionData.id, this.formId, errors, this.sectionData.errorsToShow); + this.sectionData.errorsToShow = errors; this.cdr.detectChanges(); }); } @@ -340,13 +339,13 @@ export class SubmissionSectionformComponent extends SectionModelComponent { */ this.sectionService.getSectionState(this.submissionId, this.sectionData.id, this.sectionData.sectionType).pipe( filter((sectionState: SubmissionSectionObject) => { - return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errors)); + return isNotEmpty(sectionState) && (isNotEmpty(sectionState.data) || isNotEmpty(sectionState.errorsToShow)); }), distinctUntilChanged()) .subscribe((sectionState: SubmissionSectionObject) => { this.fieldsOnTheirWayToBeRemoved = new Map(); this.sectionMetadata = sectionState.metadata; - this.updateForm(sectionState.data as WorkspaceitemSectionFormObject, sectionState.errors); + this.updateForm(sectionState.data as WorkspaceitemSectionFormObject, sectionState.errorsToShow); }) ); } @@ -367,11 +366,22 @@ export class SubmissionSectionformComponent extends SectionModelComponent { const metadata = this.formOperationsService.getFieldPathSegmentedFromChangeEvent(event); const value = this.formOperationsService.getFieldValueFromChangeEvent(event); - if (environment.submission.autosave.metadata.indexOf(metadata) !== -1 && isNotEmpty(value)) { + if ((environment.submission.autosave.metadata.indexOf(metadata) !== -1 && isNotEmpty(value)) || this.hasRelatedCustomError(metadata)) { this.submissionService.dispatchSave(this.submissionId); } } + private hasRelatedCustomError(medatata): boolean { + const index = findIndex(this.sectionData.errorsToShow, {path: this.pathCombiner.getPath(medatata).path}); + if (index !== -1) { + const error = this.sectionData.errorsToShow[index]; + const validator = error.message.replace('error.validation.', ''); + return !environment.form.validatorMap.hasOwnProperty(validator); + } else { + return false; + } + } + /** * Method called when a form dfFocus event is fired. * Initialize [FormFieldPreviousValueObject] instance. diff --git a/src/app/submission/sections/license/section-license.component.spec.ts b/src/app/submission/sections/license/section-license.component.spec.ts index 1dd9f36fe5..f7a3895052 100644 --- a/src/app/submission/sections/license/section-license.component.spec.ts +++ b/src/app/submission/sections/license/section-license.component.spec.ts @@ -70,7 +70,8 @@ const sectionObject: SectionDataObject = { acceptanceDate: null, granted: false }, - errors: [], + errorsToShow: [], + serverValidationErrors: [], header: 'submit.progressbar.describe.license', id: 'license', sectionType: SectionsType.License diff --git a/src/app/submission/sections/models/section-data.model.ts b/src/app/submission/sections/models/section-data.model.ts index 8feb78fa69..9078b5dfb9 100644 --- a/src/app/submission/sections/models/section-data.model.ts +++ b/src/app/submission/sections/models/section-data.model.ts @@ -18,9 +18,14 @@ export interface SectionDataObject { data: WorkspaceitemSectionDataType; /** - * The list of the section errors + * The list of the section's errors to show */ - errors: SubmissionSectionError[]; + errorsToShow: SubmissionSectionError[]; + + /** + * The list of the section's errors detected by the server + */ + serverValidationErrors: SubmissionSectionError[]; /** * The section header diff --git a/src/app/submission/sections/sections.directive.ts b/src/app/submission/sections/sections.directive.ts index ab7f01eeb0..a42cd4be78 100644 --- a/src/app/submission/sections/sections.directive.ts +++ b/src/app/submission/sections/sections.directive.ts @@ -6,7 +6,7 @@ import { uniq } from 'lodash'; import { SectionsService } from './sections.service'; import { hasValue, isNotEmpty, isNotNull } from '../../shared/empty.util'; -import { SubmissionSectionError, SubmissionSectionObject } from '../objects/submission-objects.reducer'; +import { SubmissionSectionError } from '../objects/submission-objects.reducer'; import parseSectionErrorPaths, { SectionErrorPath } from '../utils/parseSectionErrorPaths'; import { SubmissionService } from '../submission.service'; import { SectionsType } from './sections-type'; @@ -111,8 +111,7 @@ export class SectionsDirective implements OnDestroy, OnInit { })); this.subs.push( - this.sectionService.getSectionState(this.submissionId, this.sectionId, this.sectionType).pipe( - map((state: SubmissionSectionObject) => state.errors)) + this.sectionService.getShownSectionErrors(this.submissionId, this.sectionId, this.sectionType) .subscribe((errors: SubmissionSectionError[]) => { if (isNotEmpty(errors)) { errors.forEach((errorItem: SubmissionSectionError) => { diff --git a/src/app/submission/sections/sections.service.spec.ts b/src/app/submission/sections/sections.service.spec.ts index d199f1beae..9fbcedcc4d 100644 --- a/src/app/submission/sections/sections.service.spec.ts +++ b/src/app/submission/sections/sections.service.spec.ts @@ -28,13 +28,15 @@ import { SectionStatusChangeAction, UpdateSectionDataAction } from '../objects/submission-objects.actions'; -import { FormAddError, FormClearErrorsAction, FormRemoveErrorAction } from '../../shared/form/form.actions'; +import { FormClearErrorsAction } from '../../shared/form/form.actions'; import parseSectionErrors from '../utils/parseSectionErrors'; import { SubmissionScopeType } from '../../core/submission/submission-scope-type'; import { SubmissionSectionError } from '../objects/submission-objects.reducer'; import { getMockScrollToService } from '../../shared/mocks/scroll-to-service.mock'; import { storeModuleConfig } from '../../app.reducer'; import { SectionsType } from './sections-type'; +import { FormService } from '../../shared/form/form.service'; +import { getMockFormService } from '../../shared/mocks/form-service.mock'; describe('SectionsService test suite', () => { let notificationsServiceStub: NotificationsServiceStub; @@ -57,6 +59,8 @@ describe('SectionsService test suite', () => { select: jasmine.createSpy('select') }); + const formService: any = getMockFormService(); + beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [ @@ -74,6 +78,7 @@ describe('SectionsService test suite', () => { { provide: SubmissionService, useClass: SubmissionServiceStub }, { provide: TranslateService, useValue: getMockTranslateService() }, { provide: Store, useValue: store }, + { provide: FormService, useValue: formService }, SectionsService ] }).compileComponents(); @@ -98,22 +103,23 @@ describe('SectionsService test suite', () => { it('should dispatch a new FormAddError for each section\'s error', () => { service.checkSectionErrors(submissionId, sectionId, formId, sectionErrors[sectionId]); - expect(store.dispatch).toHaveBeenCalledWith(new FormAddError( + expect(formService.addError).toHaveBeenCalledWith( formId, - 'dc_contributor_author', + 'dc.contributor.author', 0, - 'error.validation.required')); + 'error.validation.required'); - expect(store.dispatch).toHaveBeenCalledWith(new FormAddError( + expect(formService.addError).toHaveBeenCalledWith( formId, - 'dc_title', + 'dc.title', 0, - 'error.validation.required')); + 'error.validation.required'); - expect(store.dispatch).toHaveBeenCalledWith(new FormAddError(formId, - 'dc_date_issued', + expect(formService.addError).toHaveBeenCalledWith( + formId, + 'dc.date.issued', 0, - 'error.validation.required')); + 'error.validation.required'); }); it('should dispatch a new FormRemoveErrorAction for each section\'s error that no longer exists', () => { @@ -123,21 +129,21 @@ describe('SectionsService test suite', () => { service.checkSectionErrors(submissionId, sectionId, formId, currentErrors, prevErrors); - expect(store.dispatch).toHaveBeenCalledWith(new FormAddError( + expect(formService.addError).toHaveBeenCalledWith( formId, - 'dc_contributor_author', + 'dc.contributor.author', 0, - 'error.validation.required')); + 'error.validation.required'); - expect(store.dispatch).toHaveBeenCalledWith(new FormAddError( + expect(formService.addError).toHaveBeenCalledWith( formId, - 'dc_title', + 'dc.title', 0, - 'error.validation.required')); - expect(store.dispatch).toHaveBeenCalledWith(new FormRemoveErrorAction( + 'error.validation.required'); + expect(formService.removeError).toHaveBeenCalledWith( formId, - 'dc_date_issued', - 0)); + 'dc.date.issued', + 0); }); }); @@ -417,7 +423,7 @@ describe('SectionsService test suite', () => { scheduler.schedule(() => service.updateSectionData(submissionId, sectionId, data, [])); scheduler.flush(); - expect(store.dispatch).toHaveBeenCalledWith(new UpdateSectionDataAction(submissionId, sectionId, data, [])); + expect(store.dispatch).toHaveBeenCalledWith(new UpdateSectionDataAction(submissionId, sectionId, data, [], [])); }); it('should dispatch a new UpdateSectionDataAction and display a new notification when section is not enabled', () => { @@ -429,7 +435,7 @@ describe('SectionsService test suite', () => { scheduler.schedule(() => service.updateSectionData(submissionId, sectionId, data, [])); scheduler.flush(); - expect(store.dispatch).toHaveBeenCalledWith(new UpdateSectionDataAction(submissionId, sectionId, data, [])); + expect(store.dispatch).toHaveBeenCalledWith(new UpdateSectionDataAction(submissionId, sectionId, data, [], [])); }); }); diff --git a/src/app/submission/sections/sections.service.ts b/src/app/submission/sections/sections.service.ts index 05e9a96267..dd68d42a87 100644 --- a/src/app/submission/sections/sections.service.ts +++ b/src/app/submission/sections/sections.service.ts @@ -1,11 +1,11 @@ import { Injectable } from '@angular/core'; import { combineLatest, Observable } from 'rxjs'; -import { distinctUntilChanged, filter, map, take } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map, mergeMap, take } from 'rxjs/operators'; import { Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; import { ScrollToConfigOptions, ScrollToService } from '@nicky-lenaers/ngx-scroll-to'; -import { findKey, isEqual } from 'lodash'; +import { findIndex, findKey, isEqual } from 'lodash'; import { SubmissionState } from '../submission.reducers'; import { hasValue, isEmpty, isNotEmpty, isNotUndefined } from '../../shared/empty.util'; @@ -27,11 +27,12 @@ import { submissionObjectFromIdSelector, submissionSectionDataFromIdSelector, submissionSectionErrorsFromIdSelector, - submissionSectionFromIdSelector + submissionSectionFromIdSelector, + submissionSectionServerErrorsFromIdSelector } from '../selectors'; import { SubmissionScopeType } from '../../core/submission/submission-scope-type'; import parseSectionErrorPaths, { SectionErrorPath } from '../utils/parseSectionErrorPaths'; -import { FormAddError, FormClearErrorsAction, FormRemoveErrorAction } from '../../shared/form/form.actions'; +import { FormClearErrorsAction } from '../../shared/form/form.actions'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { SubmissionService } from '../submission.service'; import { WorkspaceitemSectionDataType } from '../../core/submission/models/workspaceitem-sections.model'; @@ -39,6 +40,9 @@ import { SectionsType } from './sections-type'; import { normalizeSectionData } from '../../core/submission/submission-response-parsing.service'; import { SubmissionFormsModel } from '../../core/config/models/config-submission-forms.model'; import { parseReviver } from '@ng-dynamic-forms/core'; +import { FormService } from '../../shared/form/form.service'; +import { JsonPatchOperationPathCombiner } from '../../core/json-patch/builder/json-patch-operation-path-combiner'; +import { FormError } from '../../shared/form/form.reducer'; /** * A service that provides methods used in submission process. @@ -48,13 +52,15 @@ export class SectionsService { /** * Initialize service variables + * @param {FormService} formService * @param {NotificationsService} notificationsService * @param {ScrollToService} scrollToService * @param {SubmissionService} submissionService * @param {Store} store * @param {TranslateService} translate */ - constructor(private notificationsService: NotificationsService, + constructor(private formService: FormService, + private notificationsService: NotificationsService, private scrollToService: ScrollToService, private submissionService: SubmissionService, private store: Store, @@ -95,12 +101,9 @@ export class SectionsService { errorPaths.forEach((path: SectionErrorPath) => { if (path.fieldId) { - const fieldId = path.fieldId.replace(/\./g, '_'); - // Dispatch action to add form error to the state; - const formAddErrorAction = new FormAddError(formId, fieldId, path.fieldIndex, error.message); - this.store.dispatch(formAddErrorAction); - dispatchedErrors.push(fieldId); + this.formService.addError(formId, path.fieldId, path.fieldIndex, error.message); + dispatchedErrors.push(path.fieldId); } }); }); @@ -111,12 +114,9 @@ export class SectionsService { errorPaths.forEach((path: SectionErrorPath) => { if (path.fieldId) { - const fieldId = path.fieldId.replace(/\./g, '_'); - - if (!dispatchedErrors.includes(fieldId)) { + if (!dispatchedErrors.includes(path.fieldId)) { // Dispatch action to remove form error from the state; - const formRemoveErrorAction = new FormRemoveErrorAction(formId, fieldId, path.fieldIndex); - this.store.dispatch(formRemoveErrorAction); + this.formService.removeError(formId, path.fieldId, path.fieldIndex); } } }); @@ -174,7 +174,40 @@ export class SectionsService { } /** - * Return the error list object data for the specified section + * Get the list of validation errors present in the given section + * + * @param submissionId + * The submission id + * @param sectionId + * The section id + * @param sectionType + * The type of section for which retrieve errors + */ + getShownSectionErrors(submissionId: string, sectionId: string, sectionType: SectionsType): Observable { + let errorsState$: Observable; + if (sectionType !== SectionsType.SubmissionForm) { + errorsState$ = this.getSectionErrors(submissionId, sectionId); + } else { + errorsState$ = this.getSectionState(submissionId, sectionId, sectionType).pipe( + mergeMap((state: SubmissionSectionObject) => this.formService.getFormErrors(state.formId).pipe( + map((formErrors: FormError[]) => { + const pathCombiner = new JsonPatchOperationPathCombiner('sections', sectionId); + const sectionErrors = formErrors + .map((error) => ({ + path: pathCombiner.getPath(error.fieldId.replace(/\_/g, '.')).path, + message: error.message + } as SubmissionSectionError)) + .filter((sectionError: SubmissionSectionError) => findIndex(state.errorsToShow, {path: sectionError.path}) === -1); + return [...state.errorsToShow, ...sectionErrors]; + }) + )) + ); + } + + return errorsState$; + } + /** + * Return the error list to show for the specified section * * @param submissionId * The submission id @@ -188,6 +221,21 @@ export class SectionsService { distinctUntilChanged()); } + /** + * Return the error list detected by the server for the specified section + * + * @param submissionId + * The submission id + * @param sectionId + * The section id + * @return Observable + * observable of array of [SubmissionSectionError] + */ + public getSectionServerErrors(submissionId: string, sectionId: string): Observable { + return this.store.select(submissionSectionServerErrorsFromIdSelector(submissionId, sectionId)).pipe( + distinctUntilChanged()); + } + /** * Return the state object for the specified section * @@ -383,12 +431,23 @@ export class SectionsService { * The section id * @param data * The section data - * @param errors - * The list of section errors + * @param errorsToShow + * The list of the section's errors to show. It contains the error list + * to display when section is not pristine + * @param serverValidationErrors + * The list of the section's errors detected by the server. + * They may not be shown yet if section is pristine * @param metadata * The section metadata */ - public updateSectionData(submissionId: string, sectionId: string, data: WorkspaceitemSectionDataType, errors: SubmissionSectionError[] = [], metadata?: string[]) { + public updateSectionData( + submissionId: string, + sectionId: string, + data: WorkspaceitemSectionDataType, + errorsToShow: SubmissionSectionError[] = [], + serverValidationErrors: SubmissionSectionError[] = [], + metadata?: string[] + ) { if (isNotEmpty(data)) { const isAvailable$ = this.isSectionAvailable(submissionId, sectionId); const isEnabled$ = this.isSectionEnabled(submissionId, sectionId); @@ -397,7 +456,7 @@ export class SectionsService { take(1), filter(([available, enabled]: [boolean, boolean]) => available)) .subscribe(([available, enabled]: [boolean, boolean]) => { - this.store.dispatch(new UpdateSectionDataAction(submissionId, sectionId, data, errors, metadata)); + this.store.dispatch(new UpdateSectionDataAction(submissionId, sectionId, data, errorsToShow, serverValidationErrors, metadata)); }); } } diff --git a/src/app/submission/sections/upload/section-upload.component.spec.ts b/src/app/submission/sections/upload/section-upload.component.spec.ts index fad5026dca..214b1134d3 100644 --- a/src/app/submission/sections/upload/section-upload.component.spec.ts +++ b/src/app/submission/sections/upload/section-upload.component.spec.ts @@ -98,7 +98,8 @@ describe('SubmissionSectionUploadComponent test suite', () => { data: { files: [] }, - errors: [], + errorsToShow: [], + serverValidationErrors: [], header: 'submit.progressbar.describe.upload', id: 'upload-id', sectionType: SectionsType.Upload diff --git a/src/app/submission/selectors.ts b/src/app/submission/selectors.ts index 51c960b537..df33732381 100644 --- a/src/app/submission/selectors.ts +++ b/src/app/submission/selectors.ts @@ -61,5 +61,11 @@ export function submissionSectionDataFromIdSelector(submissionId: string, sectio export function submissionSectionErrorsFromIdSelector(submissionId: string, sectionId: string): MemoizedSelector { const submissionIdSelector = submissionSectionFromIdSelector(submissionId, sectionId); - return subStateSelector(submissionIdSelector, 'errors'); + return subStateSelector(submissionIdSelector, 'errorsToShow'); +} + + +export function submissionSectionServerErrorsFromIdSelector(submissionId: string, sectionId: string): MemoizedSelector { + const submissionIdSelector = submissionSectionFromIdSelector(submissionId, sectionId); + return subStateSelector(submissionIdSelector, 'serverValidationErrors'); } diff --git a/src/app/submission/submission.service.spec.ts b/src/app/submission/submission.service.spec.ts index ccf46a0c38..1e2be5b612 100644 --- a/src/app/submission/submission.service.spec.ts +++ b/src/app/submission/submission.service.spec.ts @@ -1,5 +1,5 @@ import { StoreModule } from '@ngrx/store'; -import { waitForAsync, fakeAsync, flush, TestBed, tick } from '@angular/core/testing'; +import { fakeAsync, flush, TestBed, tick, waitForAsync } from '@angular/core/testing'; import { ActivatedRoute, Router } from '@angular/router'; import { HttpHeaders } from '@angular/common/http'; @@ -32,9 +32,7 @@ import { SaveSubmissionSectionFormAction, SetActiveSectionAction } from './objects/submission-objects.actions'; -import { - createFailedRemoteDataObject, -} from '../shared/remote-data.utils'; +import { createFailedRemoteDataObject, } from '../shared/remote-data.utils'; import { getMockSearchService } from '../shared/mocks/search-service.mock'; import { getMockRequestService } from '../shared/mocks/request.service.mock'; import { RequestService } from '../core/data/request.service'; @@ -68,7 +66,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -83,7 +82,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -95,7 +95,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -107,7 +108,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -119,7 +121,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: true }, @@ -131,7 +134,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -143,7 +147,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -155,7 +160,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -167,7 +173,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -183,7 +190,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false } @@ -213,7 +221,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -228,7 +237,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -240,7 +250,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: true }, @@ -252,7 +263,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -264,7 +276,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: true }, @@ -276,7 +289,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -288,7 +302,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -300,7 +315,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: false, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }, @@ -312,7 +328,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: true }, @@ -328,7 +345,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: true } @@ -452,7 +470,7 @@ describe('SubmissionService test suite', () => { submissionDefinition, {}, new Item(), - [] + null ); const expected = new InitSubmissionFormAction( collectionId, @@ -461,7 +479,7 @@ describe('SubmissionService test suite', () => { submissionDefinition, {}, new Item(), - []); + null); expect((service as any).store.dispatch).toHaveBeenCalledWith(expected); }); @@ -564,7 +582,8 @@ describe('SubmissionService test suite', () => { mandatory: true, sectionType: 'submission-form', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.describe.indexing', @@ -573,7 +592,8 @@ describe('SubmissionService test suite', () => { mandatory: false, sectionType: 'submission-form', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.describe.publicationchannel', @@ -582,7 +602,8 @@ describe('SubmissionService test suite', () => { mandatory: true, sectionType: 'submission-form', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.describe.acknowledgement', @@ -591,7 +612,8 @@ describe('SubmissionService test suite', () => { mandatory: false, sectionType: 'submission-form', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.describe.identifiers', @@ -600,7 +622,8 @@ describe('SubmissionService test suite', () => { mandatory: false, sectionType: 'submission-form', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.describe.references', @@ -609,7 +632,8 @@ describe('SubmissionService test suite', () => { mandatory: false, sectionType: 'submission-form', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.upload', @@ -618,7 +642,8 @@ describe('SubmissionService test suite', () => { mandatory: true, sectionType: 'upload', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] }, { header: 'submit.progressbar.license', @@ -627,7 +652,8 @@ describe('SubmissionService test suite', () => { mandatory: true, sectionType: 'license', data: {}, - errors: [] + errorsToShow: [], + serverValidationErrors: [] } ] }); @@ -781,7 +807,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }; @@ -795,7 +822,8 @@ describe('SubmissionService test suite', () => { collapsed: false, enabled: true, data: {}, - errors: [], + errorsToShow: [], + serverValidationErrors: [], isLoading: false, isValid: false }; diff --git a/src/app/submission/submission.service.ts b/src/app/submission/submission.service.ts index 14838d0e9c..4b3df65de1 100644 --- a/src/app/submission/submission.service.ts +++ b/src/app/submission/submission.service.ts @@ -22,9 +22,9 @@ import { SetActiveSectionAction } from './objects/submission-objects.actions'; import { + SubmissionError, SubmissionObjectEntry, SubmissionSectionEntry, - SubmissionSectionError, SubmissionSectionObject } from './objects/submission-objects.reducer'; import { submissionObjectFromIdSelector } from './selectors'; @@ -183,7 +183,7 @@ export class SubmissionService { submissionDefinition: SubmissionDefinitionsModel, sections: WorkspaceitemSectionsObject, item: Item, - errors: SubmissionSectionError[]) { + errors: SubmissionError) { this.store.dispatch(new InitSubmissionFormAction(collectionId, submissionId, selfUrl, submissionDefinition, sections, item, errors)); } @@ -292,7 +292,8 @@ export class SubmissionService { sectionObject.config = sections[sectionId].config; sectionObject.mandatory = sections[sectionId].mandatory; sectionObject.data = sections[sectionId].data; - sectionObject.errors = sections[sectionId].errors; + sectionObject.errorsToShow = sections[sectionId].errorsToShow; + sectionObject.serverValidationErrors = sections[sectionId].serverValidationErrors; sectionObject.header = sections[sectionId].header; sectionObject.id = sectionId; sectionObject.sectionType = sections[sectionId].sectionType; diff --git a/src/app/submission/submit/submission-submit.component.ts b/src/app/submission/submit/submission-submit.component.ts index af1bf38539..a66d1e4656 100644 --- a/src/app/submission/submit/submission-submit.component.ts +++ b/src/app/submission/submit/submission-submit.component.ts @@ -1,19 +1,17 @@ import { ChangeDetectorRef, Component, OnDestroy, OnInit, ViewContainerRef } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { Subscription } from 'rxjs'; - -import { hasValue, isEmpty, isNotNull, isNotEmptyOperator } from '../../shared/empty.util'; -import { SubmissionDefinitionsModel } from '../../core/config/models/config-submission-definitions.model'; +import { BehaviorSubject, Subscription } from 'rxjs'; +import { debounceTime, switchMap } from 'rxjs/operators'; import { TranslateService } from '@ngx-translate/core'; + +import { hasValue, isEmpty, isNotEmptyOperator, isNotNull } from '../../shared/empty.util'; +import { SubmissionDefinitionsModel } from '../../core/config/models/config-submission-definitions.model'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { SubmissionService } from '../submission.service'; import { SubmissionObject } from '../../core/submission/models/submission-object.model'; -import { Collection } from '../../core/shared/collection.model'; import { Item } from '../../core/shared/item.model'; import { WorkspaceitemSectionsObject } from '../../core/submission/models/workspaceitem-sections.model'; -import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; -import { switchMap, debounceTime } from 'rxjs/operators'; import { getAllSucceededRemoteData } from '../../core/shared/operators'; import { RemoteData } from '../../core/data/remote-data'; import { ItemDataService } from '../../core/data/item-data.service';