From c250408382041b221cbfc4b51df563245c314381 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Wed, 20 Apr 2022 16:10:18 +0200 Subject: [PATCH 1/4] [CST-5418] Improve notification when cannot deposit --- .../submission/objects/submission-objects.effects.spec.ts | 4 +++- src/app/submission/objects/submission-objects.effects.ts | 8 +++++++- src/assets/i18n/en.json5 | 2 ++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/app/submission/objects/submission-objects.effects.spec.ts b/src/app/submission/objects/submission-objects.effects.spec.ts index b2bc054287..69ee74224e 100644 --- a/src/app/submission/objects/submission-objects.effects.spec.ts +++ b/src/app/submission/objects/submission-objects.effects.spec.ts @@ -884,6 +884,7 @@ describe('SubmissionObjectEffects test suite', () => { }); expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); + expect(notificationsServiceStub.warning).not.toHaveBeenCalled(); }); it('should return a SAVE_SUBMISSION_FORM_SUCCESS action when there are errors', () => { @@ -910,10 +911,11 @@ describe('SubmissionObjectEffects test suite', () => { submissionJsonPatchOperationsServiceStub.jsonPatchByResourceType.and.returnValue(observableOf(response)); const expected = cold('--b-', { - b: new SaveSubmissionFormSuccessAction(submissionId, response as any[]) + b: new SaveSubmissionFormSuccessAction(submissionId, response as any[], false) }); expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); + expect(notificationsServiceStub.warning).toHaveBeenCalled(); }); it('should catch errors and return a SAVE_SUBMISSION_FORM_ERROR', () => { diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 52e20ee149..8d53fedbf9 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -215,7 +215,13 @@ export class SubmissionObjectEffects { if (this.canDeposit(response)) { return new DepositSubmissionAction(action.payload.submissionId); } else { - return new SaveSubmissionFormSuccessAction(action.payload.submissionId, response); + this.notificationsService.warning( + null, + this.translate.instant('submission.sections.general.cannot_deposit'), + null, + true + ); + return new SaveSubmissionFormSuccessAction(action.payload.submissionId, response, false); } }), catchError(() => observableOf(new SaveSubmissionFormErrorAction(action.payload.submissionId)))); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index c3c68a6882..a698fab641 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3828,6 +3828,8 @@ "submission.sections.general.add-more": "Add more", + "submission.sections.general.cannot_deposit": "Deposit cannot be completed due to missing mandatory information.
Please add them for final submission.", + "submission.sections.general.collection": "Collection", "submission.sections.general.deposit_error_notice": "There was an issue when submitting the item, please try again later.", From 0506c5596ac714494befdd9078ead3839d780c44 Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 21 Apr 2022 11:30:15 +0200 Subject: [PATCH 2/4] [CST-5418] Use different param for notifications and errors in the parseSaveResponse method --- .../objects/submission-objects.actions.ts | 11 ++++++--- .../submission-objects.effects.spec.ts | 4 +++- .../objects/submission-objects.effects.ts | 24 ++++++++++++------- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/app/submission/objects/submission-objects.actions.ts b/src/app/submission/objects/submission-objects.actions.ts index 4709133818..c03093bb24 100644 --- a/src/app/submission/objects/submission-objects.actions.ts +++ b/src/app/submission/objects/submission-objects.actions.ts @@ -419,7 +419,8 @@ export class SaveSubmissionFormSuccessAction implements Action { payload: { submissionId: string; submissionObject: SubmissionObject[]; - notify?: boolean + showNotifications?: boolean; + showErrors?: boolean; }; /** @@ -429,9 +430,13 @@ export class SaveSubmissionFormSuccessAction implements Action { * the submission's ID * @param submissionObject * the submission's Object + * @param showNotifications + * a boolean representing if to show notifications on save + * @param showErrors + * a boolean representing if to show errors on save */ - constructor(submissionId: string, submissionObject: SubmissionObject[], notify?: boolean) { - this.payload = { submissionId, submissionObject, notify }; + constructor(submissionId: string, submissionObject: SubmissionObject[], showNotifications?: boolean, showErrors?: boolean) { + this.payload = { submissionId, submissionObject, showNotifications, showErrors }; } } diff --git a/src/app/submission/objects/submission-objects.effects.spec.ts b/src/app/submission/objects/submission-objects.effects.spec.ts index 69ee74224e..a1bb878aa5 100644 --- a/src/app/submission/objects/submission-objects.effects.spec.ts +++ b/src/app/submission/objects/submission-objects.effects.spec.ts @@ -237,6 +237,7 @@ describe('SubmissionObjectEffects test suite', () => { b: new SaveSubmissionFormSuccessAction( submissionId, mockSubmissionRestResponse as any, + true, true ) }); @@ -260,6 +261,7 @@ describe('SubmissionObjectEffects test suite', () => { b: new SaveSubmissionFormSuccessAction( submissionId, mockSubmissionRestResponse as any, + false, false ) }); @@ -911,7 +913,7 @@ describe('SubmissionObjectEffects test suite', () => { submissionJsonPatchOperationsServiceStub.jsonPatchByResourceType.and.returnValue(observableOf(response)); const expected = cold('--b-', { - b: new SaveSubmissionFormSuccessAction(submissionId, response as any[], false) + b: new SaveSubmissionFormSuccessAction(submissionId, response as any[], false, true) }); expect(submissionObjectEffects.saveAndDeposit$).toBeObservable(expected); diff --git a/src/app/submission/objects/submission-objects.effects.ts b/src/app/submission/objects/submission-objects.effects.ts index 8d53fedbf9..e20e5533ed 100644 --- a/src/app/submission/objects/submission-objects.effects.ts +++ b/src/app/submission/objects/submission-objects.effects.ts @@ -125,7 +125,7 @@ export class SubmissionObjectEffects { this.submissionService.getSubmissionObjectLinkName(), action.payload.submissionId, 'sections').pipe( - map((response: SubmissionObject[]) => new SaveSubmissionFormSuccessAction(action.payload.submissionId, response, action.payload.isManual)), + map((response: SubmissionObject[]) => new SaveSubmissionFormSuccessAction(action.payload.submissionId, response, action.payload.isManual, action.payload.isManual)), catchError(() => observableOf(new SaveSubmissionFormErrorAction(action.payload.submissionId)))); }))); @@ -151,7 +151,8 @@ export class SubmissionObjectEffects { 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, currentState.forms, action.payload.notify); + action.payload.submissionObject, action.payload.submissionId, currentState.forms, + action.payload.showNotifications, action.payload.showErrors); }), mergeMap((actions) => observableFrom(actions)))); @@ -164,7 +165,7 @@ export class SubmissionObjectEffects { 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, currentState.forms, false); + action.payload.submissionObject, action.payload.submissionId, currentState.forms, false, false); }), mergeMap((actions) => observableFrom(actions)))); @@ -221,7 +222,7 @@ export class SubmissionObjectEffects { null, true ); - return new SaveSubmissionFormSuccessAction(action.payload.submissionId, response, false); + return new SaveSubmissionFormSuccessAction(action.payload.submissionId, response, false, true); } }), catchError(() => observableOf(new SaveSubmissionFormErrorAction(action.payload.submissionId)))); @@ -367,18 +368,23 @@ export class SubmissionObjectEffects { * A boolean that indicate if show notification or not * @return SubmissionObjectAction[] * List of SubmissionObjectAction to dispatch + * @param showNotifications + * A boolean representing if to show notifications on save + * @param showErrors + * A boolean representing if to show errors on save */ protected parseSaveResponse( currentState: SubmissionObjectEntry, response: SubmissionObject[], submissionId: string, forms: FormState, - notify: boolean = true): SubmissionObjectAction[] { + showNotifications: boolean = true, + showErrors: boolean = true): SubmissionObjectAction[] { const mappedActions = []; if (isNotEmpty(response)) { - if (notify) { + if (showNotifications) { this.notificationsService.success(null, this.translate.get('submission.sections.general.save_success_notice')); } @@ -390,7 +396,7 @@ export class SubmissionObjectEffects { if (errors && !isEmpty(errors)) { // to avoid dispatching an action for every error, create an array of errors per section errorsList = parseSectionErrors(errors); - if (notify) { + if (showNotifications) { this.notificationsService.warning(null, this.translate.get('submission.sections.general.sections_not_valid')); } } @@ -409,12 +415,12 @@ export class SubmissionObjectEffects { continue; } - if (notify && !currentState.sections[sectionId].enabled) { + if (showNotifications && !currentState.sections[sectionId].enabled) { this.submissionService.notifyNewSection(submissionId, sectionId, currentState.sections[sectionId].sectionType); } const sectionForm = getForm(forms, currentState, sectionId); - const filteredErrors = filterErrors(sectionForm, sectionErrors, currentState.sections[sectionId].sectionType, notify); + const filteredErrors = filterErrors(sectionForm, sectionErrors, currentState.sections[sectionId].sectionType, showErrors); mappedActions.push(new UpdateSectionDataAction(submissionId, sectionId, sectionData, filteredErrors, sectionErrors)); } }); From fa1b7d11ad0df13c99550d99ecf4600da9fdf27e Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Thu, 21 Apr 2022 11:31:07 +0200 Subject: [PATCH 3/4] [CST-5418] Check success notification is not present on depositing when there are errors --- cypress/integration/submission.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/cypress/integration/submission.spec.ts b/cypress/integration/submission.spec.ts index c877479f44..009c50115b 100644 --- a/cypress/integration/submission.spec.ts +++ b/cypress/integration/submission.spec.ts @@ -42,6 +42,7 @@ describe('New Submission page', () => { cy.get('button#deposit').click(); // A warning alert should display. + cy.get('ds-notification div.alert-success').should('not.exist'); cy.get('ds-notification div.alert-warning').should('be.visible'); // First section should have an exclamation error in the header From eb57b28b52709a1fe1c10e5df680edc25045eece Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 26 Apr 2022 10:43:53 +0200 Subject: [PATCH 4/4] [CST-5418] Fix warning message --- src/assets/i18n/en.json5 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index a698fab641..a7ce942e7d 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -3828,7 +3828,7 @@ "submission.sections.general.add-more": "Add more", - "submission.sections.general.cannot_deposit": "Deposit cannot be completed due to missing mandatory information.
Please add them for final submission.", + "submission.sections.general.cannot_deposit": "Deposit cannot be completed due to errors in the form.
Please fill out all required fields to complete the deposit.", "submission.sections.general.collection": "Collection",