From c72c605d1ef9f07310e10946fa1330c13eb74f48 Mon Sep 17 00:00:00 2001 From: andyjmaclean Date: Thu, 21 Nov 2024 14:00:56 +0100 Subject: [PATCH] MET-6255 Improve Error Handling --- .../cypress/e2e/app-network-errors.cy.ts | 12 +++ .../src/app/http-errors/errors.component.html | 7 +- .../src/app/http-errors/errors.component.scss | 24 ++++++ .../src/app/http-errors/errors.component.ts | 6 +- .../sandbox-navigation.component.html | 5 +- .../sandbox-navigation.component.spec.ts | 18 +++++ .../sandbox-navigation.component.ts | 9 +++ .../src/app/upload/upload.component.html | 2 +- .../src/app/upload/upload.component.ts | 77 +++++++++---------- 9 files changed, 114 insertions(+), 46 deletions(-) create mode 100644 projects/sandbox/src/app/http-errors/errors.component.scss diff --git a/projects/sandbox/cypress/e2e/app-network-errors.cy.ts b/projects/sandbox/cypress/e2e/app-network-errors.cy.ts index fc5b07bea..e66248c4d 100644 --- a/projects/sandbox/cypress/e2e/app-network-errors.cy.ts +++ b/projects/sandbox/cypress/e2e/app-network-errors.cy.ts @@ -16,6 +16,13 @@ context('Sandbox', () => { cy.visit('/dataset'); }); + const closeErrors = (): void => { + const selCloseErrors = '.close-errors'; + cy.get(selCloseErrors).should('have.length', 1); + cy.get(selCloseErrors).click(); + cy.get(selCloseErrors).should('not.exist'); + }; + it('should show an error when the data upload fails', () => { const code = '404'; cy.get(selectorLinkDatasetForm).click(); @@ -24,6 +31,7 @@ context('Sandbox', () => { cy.get(selectorErrors) .contains(code) .should('have.length', 1); + closeErrors(); }); it('should show an error when the progress data load fails', () => { @@ -33,6 +41,7 @@ context('Sandbox', () => { cy.get(selectorErrors) .contains(code) .should('have.length', 1); + closeErrors(); }); it('should show an error when the (dateset) problem-pattern load fails', () => { @@ -43,6 +52,7 @@ context('Sandbox', () => { cy.get(selectorErrors) .contains(code) .should('have.length', 1); + closeErrors(); }); it('should show an error when the record report load fails', () => { @@ -54,6 +64,7 @@ context('Sandbox', () => { cy.get(selectorErrors) .contains(code) .should('have.length', 1); + closeErrors(); }); it('should show an error when the (record) problem-pattern load fails', () => { @@ -65,6 +76,7 @@ context('Sandbox', () => { cy.get(selectorErrors) .contains(code) .should('have.length', 1); + closeErrors(); }); it('should remember the errors for each step', () => { diff --git a/projects/sandbox/src/app/http-errors/errors.component.html b/projects/sandbox/src/app/http-errors/errors.component.html index 251c4310f..c743f3999 100644 --- a/projects/sandbox/src/app/http-errors/errors.component.html +++ b/projects/sandbox/src/app/http-errors/errors.component.html @@ -1,4 +1,8 @@ -
+@if(error) { +
+ @if(onClose) { + + }
{{ error.statusText }}
@@ -6,3 +10,4 @@
  • {{ error.message }}
    • +} diff --git a/projects/sandbox/src/app/http-errors/errors.component.scss b/projects/sandbox/src/app/http-errors/errors.component.scss new file mode 100644 index 000000000..f1ace5579 --- /dev/null +++ b/projects/sandbox/src/app/http-errors/errors.component.scss @@ -0,0 +1,24 @@ +.close-errors { + clear: both; + float: right; + height: 1rem; + position: relative; + top: 2px; + z-index: 1; + + &::before, + &::after { + background: red; + content: ''; + display: block; + height: 0.175rem; + margin-top: 0.375rem; + transform: rotate(45deg); + width: 1rem; + } + + &::after { + margin-top: -0.175rem; + transform: rotate(-45deg); + } +} diff --git a/projects/sandbox/src/app/http-errors/errors.component.ts b/projects/sandbox/src/app/http-errors/errors.component.ts index 4a26b0baa..01b6b5664 100644 --- a/projects/sandbox/src/app/http-errors/errors.component.ts +++ b/projects/sandbox/src/app/http-errors/errors.component.ts @@ -1,13 +1,13 @@ -import { NgIf } from '@angular/common'; import { Component, Input } from '@angular/core'; import { HttpErrorResponse } from '@angular/common/http'; @Component({ selector: 'sb-http-errors', + styleUrls: ['./errors.component.scss'], templateUrl: './errors.component.html', - standalone: true, - imports: [NgIf] + standalone: true }) export class HttpErrorsComponent { @Input() error: HttpErrorResponse | undefined; + @Input() onClose: (() => void) | undefined; } diff --git a/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.html b/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.html index e04845e6e..8328752f9 100644 --- a/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.html +++ b/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.html @@ -259,6 +259,9 @@

    - +

    diff --git a/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.spec.ts b/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.spec.ts index bdb5f8f97..01f1ab1cd 100644 --- a/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.spec.ts +++ b/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.spec.ts @@ -622,6 +622,11 @@ describe('SandboxNavigatonComponent', () => { expect(component.sandboxNavConf[confIndex].error).toBeTruthy(); expect(component.progressData).toBeFalsy(); expect(component.formProgress.value.datasetToTrack).toBeTruthy(); + + component.currentStepIndex = confIndex; + component.clearError(); + expect(component.sandboxNavConf[confIndex].error).toBeFalsy(); + component.cleanup(); tick(apiSettings.interval); })); @@ -650,6 +655,9 @@ describe('SandboxNavigatonComponent', () => { expect(component.sandboxNavConf[index].error).toBeTruthy(); expect(component.recordReport).toBeFalsy(); + component.clearError(); + expect(component.sandboxNavConf[index].error).toBeFalsy(); + component.cleanup(); tick(apiSettings.interval); })); @@ -660,6 +668,12 @@ describe('SandboxNavigatonComponent', () => { component.submitDatasetProblemPatterns(); tick(1); expect(component.sandboxNavConf[stepIndexProblemsDataset].error).toBeTruthy(); + + component.clearError(); + expect(component.sandboxNavConf[stepIndexProblemsDataset].error).toBeTruthy(); + component.currentStepIndex = stepIndexProblemsDataset; + component.clearError(); + expect(component.sandboxNavConf[stepIndexProblemsDataset].error).toBeFalsy(); })); it('should handle problem pattern errors (record)', fakeAsync(() => { @@ -669,6 +683,10 @@ describe('SandboxNavigatonComponent', () => { component.submitRecordProblemPatterns(); tick(1); expect(component.sandboxNavConf[stepIndexProblemsRecord].error).toBeTruthy(); + + component.currentStepIndex = stepIndexProblemsRecord; + component.clearError(); + expect(component.sandboxNavConf[stepIndexProblemsRecord].error).toBeFalsy(); })); }); }); diff --git a/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.ts b/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.ts index 9cf03fbd7..0426e343f 100644 --- a/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.ts +++ b/projects/sandbox/src/app/sandbox-navigation/sandbox-navigation.component.ts @@ -194,6 +194,15 @@ export class SandboxNavigatonComponent extends DataPollingComponent implements O this.resetPageData(); } + /** + * clearError + * + * reset the step error + **/ + clearError(): void { + this.sandboxNavConf[this.currentStepIndex].error = undefined; + } + /** * getNavOrbConfigOuter * diff --git a/projects/sandbox/src/app/upload/upload.component.html b/projects/sandbox/src/app/upload/upload.component.html index 28e26532f..5da2f0667 100644 --- a/projects/sandbox/src/app/upload/upload.component.html +++ b/projects/sandbox/src/app/upload/upload.component.html @@ -180,5 +180,5 @@ Submit - + diff --git a/projects/sandbox/src/app/upload/upload.component.ts b/projects/sandbox/src/app/upload/upload.component.ts index 72439073b..dc28a96df 100644 --- a/projects/sandbox/src/app/upload/upload.component.ts +++ b/projects/sandbox/src/app/upload/upload.component.ts @@ -4,6 +4,7 @@ import { AbstractControl, FormBuilder, FormControl, + FormGroup, FormsModule, ReactiveFormsModule, ValidationErrors, @@ -65,10 +66,11 @@ export class UploadComponent extends DataPollingComponent { modalIdStepSizeInfo = 'id-modal-step-size-info'; error: HttpErrorResponse | undefined; + form: FormGroup; constructor() { super(); - + this.rebuildForm(); this.subs.push( this.sandbox.getCountries().subscribe((countries: Array) => { this.countryList = countries; @@ -93,48 +95,43 @@ export class UploadComponent extends DataPollingComponent { * invokes form reset after clearing file inputs from previous submission **/ rebuildForm(): void { - this.protocolFields.clearFileValue(); - this.xslFileField.clearFileValue(); - this.form.reset(); - this.form.controls.stepSize.setValue('1'); this.error = undefined; - } - - form = this.formBuilder.group({ - name: ['', [Validators.required, this.validateDatasetName]], - country: ['', [Validators.required]], - language: ['', [Validators.required]], - uploadProtocol: [ProtocolType.ZIP_UPLOAD, [Validators.required]], - url: ['', [Validators.required]], - stepSize: [ - '1', - [ - (control: AbstractControl): ValidationErrors | null => { - const value = control.value; - const parsedValue = parseInt(value); - const isNumeric = `${parsedValue}` === value; - - if (value) { - if (!isNumeric) { - return { nonNumeric: true }; - } else if (parsedValue < 1) { - return { min: true }; + this.form = this.formBuilder.group({ + name: ['', [Validators.required, this.validateDatasetName]], + country: ['', [Validators.required]], + language: ['', [Validators.required]], + uploadProtocol: [ProtocolType.ZIP_UPLOAD, [Validators.required]], + url: ['', [Validators.required]], + stepSize: [ + '1', + [ + (control: AbstractControl): ValidationErrors | null => { + const value = control.value; + const parsedValue = parseInt(value); + const isNumeric = `${parsedValue}` === value; + + if (value) { + if (!isNumeric) { + return { nonNumeric: true }; + } else if (parsedValue < 1) { + return { min: true }; + } + } else { + return { required: true }; } - } else { - return { required: true }; + return null; } - return null; - } - ] - ], - // eslint-disable-next-line @typescript-eslint/no-explicit-any - dataset: [(undefined as any) as File, [Validators.required]], - harvestUrl: ['', [Validators.required]], - setSpec: [''], - metadataFormat: [''], - sendXSLT: [false], - xsltFile: [''] - }); + ] + ], + // eslint-disable-next-line @typescript-eslint/no-explicit-any + dataset: [(undefined as any) as File, [Validators.required]], + harvestUrl: ['', [Validators.required]], + setSpec: [''], + metadataFormat: [''], + sendXSLT: [false], + xsltFile: [''] + }); + } /** * protocolIsValid