Skip to content

Commit

Permalink
Add error handling (#1681) (#1705)
Browse files Browse the repository at this point in the history 
* Add error handling (#1681)

* Add error handling

* First additional test change

* Add additional test

* Add additional test

---------

Co-authored-by: RiteshHMCTS <[email protected]>

* exui-1184 (#1670)

* exui-1184

* exui-1184

* exui-1184

* revert changes

* revert changes

* remove hidden pages fields from fromgroup

* rectified change

* rectified the code

* version updated

* Make accessibility change (#1701)

* Make accessibility change

* Update case-file-view-folder.component.html

* Update case-full-access-view.component.html

---------

Co-authored-by: RiteshHMCTS <[email protected]>

* document upload error reworked (#1689)

* document upload error reworked

* code review comment fix

* changed return message

* version number change

* added msg as constant

* error return

---------

Co-authored-by: RiteshHMCTS <[email protected]>

* Task/1194 security issue (#1687)

* error message added for rate limit for document uploads

* Update http-error.model.ts

* Update http-error.model.spec.ts

* version set to  "7.0.13",

* commented out code causing issue

* testing the changes

* cev update

* Update yarn-audit-known-issues

* Update write-document-field.component.ts

---------

Co-authored-by: Ritesh Dsouza <[email protected]>

* version updated

* unit test coverage

* more unit test

* unit test

---------

Co-authored-by: connorpgpmcelroy <[email protected]>
Co-authored-by: OgunyemiO <[email protected]>
Co-authored-by: Olu <[email protected]>
Co-authored-by: OgunyemiO <[email protected]>
  • Loading branch information
@RiteshHMCTS @connorpgpmcelroy
@OgunyemiO @olusegz07
5 people committed Apr 3, 2024
1 parent f173982 commit 0d6dcba
Show file tree
Hide file tree
Showing 15 changed files with 319 additions and 67 deletions.
10 changes: 10 additions & 0 deletions RELEASE-NOTES.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
## RELEASE NOTES

### Version 7.0.24
**EXUI-706** Clone - Insufficient Anti-Automation Mechanisms
**EXUI-724** Remove support for WA versions
**EXUI-830** Screen freezes when submitting event
**EXUI-1184** Data going missing when using the continue and previous buttons
**EXUI-1194** Rate limit document uploads
**EXUI-1304** Accessibilty issues
**EXUI-1453** Rework On Uploading a Unsupported File type error
**EXUI-1455** New EXUI manage-case PR has broken the POST /documentsv2 endpoint

### Version 7.0.23
**CSFD-95** If upload_timestamp is present on a case document pass it though to update case event. remove upload_timestamp if present on new file replacement

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hmcts/ccd-case-ui-toolkit",
"version": "7.0.23",
"version": "7.0.24",
"engines": {
"node": ">=18.17.0"
},
Expand Down
2 changes: 1 addition & 1 deletion projects/ccd-case-ui-toolkit/package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hmcts/ccd-case-ui-toolkit",
"version": "7.0.23",
"version": "7.0.24",
"engines": {
"node": ">=18.17.0"
},
Expand Down
127 changes: 124 additions & 3 deletions ...kit/src/lib/shared/components/case-editor/case-edit-page/case-edit-page.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
FormGroup,
FormsModule,
ReactiveFormsModule,
ValidationErrors,
ValidatorFn,
Validators,
} from '@angular/forms';
import { MatLegacyDialog as MatDialog, MatLegacyDialogRef as MatDialogRef } from '@angular/material/legacy-dialog';
Expand Down Expand Up @@ -490,7 +492,7 @@ describe('CaseEditPageComponent - all other tests', () => {
spyOn(caseEditDataService, 'setCaseLinkError').and.callThrough();
spyOn(caseEditDataService, 'clearFormValidationErrors').and.callFake(() => { });
spyOn(caseEditDataService, 'setTriggerSubmitEvent').and.callFake(() => { });
spyOn(pageValidationService, 'isPageValid').and.returnValue(true);
spyOn(pageValidationService, 'getInvalidFields').and.returnValue(true);
comp = fixture.componentInstance;
readOnly.display_context = 'READONLY';
wizardPage = createWizardPage([
Expand Down Expand Up @@ -1297,7 +1299,11 @@ describe('CaseEditPageComponent - all other tests', () => {

it('should display generic error heading and message when form error is set but no callback errors, warnings, or error details', () => {
// This tests CaseEditGenericErrorsComponent
spyOn(pageValidationService, 'isPageValid').and.returnValue(false);
spyOn(pageValidationService, 'getInvalidFields').and.returnValue([{
id: 'caseFieldIssue',
label: 'Case Field',
value: null
}]);
comp.caseEdit.error = {
status: 200,
callbackErrors: null,
Expand Down Expand Up @@ -1329,7 +1335,11 @@ describe('CaseEditPageComponent - all other tests', () => {

it('should display specific error heading and message, and callback data field validation errors (if any)', () => {
// This tests CaseEditGenericErrorsComponent
spyOn(pageValidationService, 'isPageValid').and.returnValue(false);
spyOn(pageValidationService, 'getInvalidFields').and.returnValue([{
id: 'caseFieldIssue',
label: 'Case Field',
value: null
}]);
comp.caseEdit.error = {
status: 422,
callbackErrors: null,
Expand Down Expand Up @@ -1594,6 +1604,15 @@ describe('CaseEditPageComponent - all other tests', () => {
});

describe('Check for Validation Error', () => {
function createMockValidator(): ValidatorFn {
return (control:AbstractControl) : ValidationErrors | null => {
const value = control.value;

if (!value) {
return {mockRequired:true};
}
}
}
const F_GROUP = new FormGroup({
data: new FormGroup({
Invalidfield1: new FormControl(null, Validators.required),
Expand All @@ -1604,6 +1623,7 @@ describe('CaseEditPageComponent - all other tests', () => {
]),
OrganisationField: new FormControl(null, Validators.required),
complexField1: new FormControl(null, Validators.required),
complexField2: new FormControl(null, createMockValidator()),
FlagLauncherField: new FormControl(null, {
validators: (
_: AbstractControl
Expand Down Expand Up @@ -1800,6 +1820,60 @@ describe('CaseEditPageComponent - all other tests', () => {
});
});

it('should correctly indicate there is no issue with the field if there is no error', () => {
const caseField = aCaseField(
'Invalidfield2',
'Invalidfield2',
'Text',
'MANDATORY',
null
);
wizardPage.case_fields.push(caseField);
wizardPage.isMultiColumn = () => false;
F_GROUP.setValue({
data: {
Invalidfield2: 'testing',
Invalidfield1: 'testing',
OrganisationField: '',
complexField1: '',
complexField2: '',
FlagLauncherField: null,
judicialUserField_judicialUserControl: null
},
});
comp.editForm = F_GROUP;
comp.currentPage = wizardPage;
fixture.detectChanges();
expect(comp.currentPageIsNotValid()).toBeFalsy();

comp.generateErrorMessage(wizardPage.case_fields);
comp.validationErrors.forEach((error) => {
expect(error.message).toEqual(
`The field that is causing the error cannot be determined but there is an error present. Please fill in more of the form`
);
});
});

it('should correctly indicate there is no issue with the field if there is no related form control', () => {
const caseField = aCaseField(
'NewField1',
'Field1',
'Text',
'MANDATORY',
null
);

wizardPage.case_fields.push(caseField);
comp.editForm = F_GROUP;

comp.generateErrorMessage(wizardPage.case_fields);
comp.validationErrors.forEach((error) => {
expect(error.message).toEqual(
`The field that is causing the error cannot be determined but there is an error present`
);
});
});

it('should validate minimum length field value and log error message', () => {
const caseField = aCaseField(
'Invalidfield2',
Expand All @@ -1816,6 +1890,7 @@ describe('CaseEditPageComponent - all other tests', () => {
Invalidfield1: 'test1',
OrganisationField: '',
complexField1: '',
complexField2: '',
FlagLauncherField: null,
judicialUserField_judicialUserControl: null
},
Expand Down Expand Up @@ -1849,6 +1924,7 @@ describe('CaseEditPageComponent - all other tests', () => {
Invalidfield1: 'test1',
OrganisationField: '',
complexField1: '',
complexField2: '',
FlagLauncherField: null,
judicialUserField_judicialUserControl: null
},
Expand Down Expand Up @@ -1911,6 +1987,51 @@ describe('CaseEditPageComponent - all other tests', () => {
});
});

it('should validate complex type fields and log error message when no error messages given', () => {
const complexSubField1: CaseField = aCaseField(
'childField1',
'childField1',
'Text',
'OPTIONAL',
1,
null,
true,
true
);
const complexSubField2: CaseField = aCaseField(
'childField2',
'childField2',
'Text',
'OPTIONAL',
2,
null,
false,
true
);
const withoutSubFailureComplexField: CaseField = aCaseField(
'complexField2',
'complexField2',
'Complex',
'OPTIONAL',
1,
[complexSubField1, complexSubField2],
true,
true
);
withoutSubFailureComplexField.isComplex = () => true;
wizardPage.isMultiColumn = () => false;
wizardPage.case_fields.push(withoutSubFailureComplexField);
comp.editForm = F_GROUP;
comp.currentPage = wizardPage;
fixture.detectChanges();
expect(comp.currentPageIsNotValid()).toBeTruthy();
comp.generateErrorMessage(wizardPage.case_fields);
expect(comp.validationErrors.length).toBe(1);
comp.validationErrors.forEach((error) => {
expect(error.message).toEqual(`There is an internal issue with complexField2 fields. The field that is causing the error cannot be determined but there is an error present`);
});
});

it('should validate FlagLauncher type field and log error message', () => {
const flagLauncherField: CaseField = aCaseField(
'FlagLauncherField',
Expand Down
40 changes: 33 additions & 7 deletions ...-toolkit/src/lib/shared/components/case-editor/case-edit-page/case-edit-page.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export class CaseEditPageComponent implements OnInit, AfterViewChecked, OnDestro
public formValuesChanged = false;
public pageChangeSubject: Subject<boolean> = new Subject();
public caseFields: CaseField[];
public failingCaseFields: CaseField[];
public validationErrors: CaseEditValidationError[] = [];
public hasPreviousPage$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
public callbackErrorsSubject: Subject<any> = new Subject();
Expand Down Expand Up @@ -158,7 +159,8 @@ export class CaseEditPageComponent implements OnInit, AfterViewChecked, OnDestro
}

public currentPageIsNotValid(): boolean {
return !this.pageValidationService.isPageValid(this.currentPage, this.editForm) ||
this.failingCaseFields = this.pageValidationService.getInvalidFields(this.currentPage, this.editForm);
return this.failingCaseFields.length > 0 ||
(this.isLinkedCasesJourney() && !this.isLinkedCasesJourneyAtFinalStep);
}

Expand All @@ -182,11 +184,17 @@ export class CaseEditPageComponent implements OnInit, AfterViewChecked, OnDestro
}

// Adding validation message to show it as Error Summary
public generateErrorMessage(fields: CaseField[], container?: AbstractControl, path?: string): void {
public generateErrorMessage(fields: CaseField[], container?: AbstractControl, path?: string): boolean {
const group: AbstractControl = container || this.editForm.controls['data'];
fields.filter(casefield => !this.caseFieldService.isReadOnly(casefield))
.filter(casefield => !this.pageValidationService.isHidden(casefield, this.editForm, path))
let validErrorFieldFound = false;
let validationErrorAmount = this.validationErrors.length;
const failingFields = fields.filter(casefield => !this.caseFieldService.isReadOnly(casefield))
.filter(casefield => !this.pageValidationService.isHidden(casefield, this.editForm, path));
// note that thougn these checks are on getinvalidfields they are needed for sub field checks
failingFields
.forEach(casefield => {
let errorPresent = true;
validErrorFieldFound = true;
const fieldElement = FieldsUtils.isCaseFieldOfType(casefield, ['JudicialUser'])
? group.get(`${casefield.id}_judicialUserControl`)
: group.get(casefield.id);
Expand Down Expand Up @@ -225,14 +233,14 @@ export class CaseEditPageComponent implements OnInit, AfterViewChecked, OnDestro
fieldElement.markAsDirty();
} else if (fieldElement.invalid) {
if (casefield.isComplex()) {
this.generateErrorMessage(casefield.field_type.complex_fields, fieldElement, id);
errorPresent = this.generateErrorMessage(casefield.field_type.complex_fields, fieldElement, id);
} else if (casefield.isCollection() && casefield.field_type.collection_field_type.type === 'Complex') {
const fieldArray = fieldElement as FormArray;
if (fieldArray['component'] && fieldArray['component']['collItems'] && fieldArray['component']['collItems'].length > 0) {
id = `${fieldArray['component']['collItems'][0].prefix}`;
}
fieldArray.controls.forEach((c: AbstractControl) => {
this.generateErrorMessage(casefield.field_type.collection_field_type.complex_fields, c.get('value'), id);
errorPresent = this.generateErrorMessage(casefield.field_type.collection_field_type.complex_fields, c.get('value'), id);
});
} else if (FieldsUtils.isCaseFieldOfType(casefield, ['FlagLauncher'])) {
this.validationErrors.push({
Expand All @@ -244,9 +252,27 @@ export class CaseEditPageComponent implements OnInit, AfterViewChecked, OnDestro
fieldElement.markAsDirty();
}
}
} else {
validErrorFieldFound = false;
}
if (!errorPresent && this.validationErrors.length === validationErrorAmount) {
// if no error messages have been added in internal field despite parent field failing
this.validationErrors.push({ id: casefield.id, message: `A field that is causing an error is ${casefield.id} but it is not producing a valid error message. Please ensure all details are correct` });
}
});
if (!validErrorFieldFound) {
path ? this.validationErrors.push({ id: path, message: `There is an internal issue with ${path} fields. The field that is causing the error cannot be determined but there is an error present` })
: this.validationErrors.push({ id: null, message: `The field that is causing the error cannot be determined but there is an error present` });
} else if (this.validationErrors.length === validationErrorAmount) {
// if no error messages have been generated
if (path) {
return false;
} else {
this.validationErrors.push({ id: null, message: `The field that is causing the error cannot be determined but there is an error present. Please fill in more of the form` })
}
}
CaseEditPageComponent.scrollToTop();
return true;
}

public navigateToErrorElement(elementId: string): void {
Expand All @@ -272,7 +298,7 @@ export class CaseEditPageComponent implements OnInit, AfterViewChecked, OnDestro
this.validationErrors.push({ id: 'next-button', message: 'Please select Next to go to the next page' });
CaseEditPageComponent.scrollToTop();
} else {
this.generateErrorMessage(this.currentPage.case_fields);
this.generateErrorMessage(this.failingCaseFields);
}
}

Expand Down
24 changes: 6 additions & 18 deletions ...se-ui-toolkit/src/lib/shared/components/case-editor/case-edit/case-edit.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ describe('CaseEditComponent', () => {
expect(wizard.nextPage).toHaveBeenCalled();
expect(routerStub.navigate).toHaveBeenCalled();
expect(component.form.get('data').get(CASE_FIELD_1.id)).not.toBeNull();
expect(component.form.get('data').get(CASE_FIELD_2.id).value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_2.id)).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3.id).value).not.toBeNull();
});

Expand Down Expand Up @@ -695,9 +695,7 @@ describe('CaseEditComponent', () => {
// 'PersonMiddleName' value expected to be null because this sub-field does not have
// retain_hidden_value = true, even though its parent Complex field does
expect(component.form.get('data').get(`${CASE_FIELD_2_COMPLEX.id}.PersonMiddleName`).value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COMPLEX.id)).not.toBeNull();
expect(component.form.get('data').get(`${CASE_FIELD_3_COMPLEX.id}.AddressLine1`)).not.toBeNull();
expect(component.form.get('data').get(`${CASE_FIELD_3_COMPLEX.id}.AddressLine1`).value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COMPLEX.id)).toBeNull();
});

it('should navigate to next page when next is called and clear hidden collection form field', () => {
Expand Down Expand Up @@ -739,11 +737,7 @@ describe('CaseEditComponent', () => {
// retain_hidden_value = true, even though its top-level collection field does
expect((component.form.get('data').get(CASE_FIELD_2_COLLECTION.id) as FormArray).at(0)
.get('value.PersonMiddleName').value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COLLECTION.id)).not.toBeNull();
expect((component.form.get('data').get(CASE_FIELD_3_COLLECTION.id) as FormArray).at(0)
.get('value.AddressLine1')).not.toBeNull();
expect((component.form.get('data').get(CASE_FIELD_3_COLLECTION.id) as FormArray).at(0)
.get('value.AddressLine1').value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COLLECTION.id)).toBeNull();
});

it('should not delete sub-field value if the FormGroup for the parent Complex hidden field cannot be determined', () => {
Expand Down Expand Up @@ -939,7 +933,7 @@ describe('CaseEditComponent', () => {
expect(wizard.previousPage).toHaveBeenCalled();
expect(routerStub.navigate).toHaveBeenCalled();
expect(component.form.get('data').get(CASE_FIELD_1.id)).not.toBeNull();
expect(component.form.get('data').get(CASE_FIELD_2.id).value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_2.id)).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3.id).value).not.toBeNull();
});

Expand Down Expand Up @@ -976,9 +970,7 @@ describe('CaseEditComponent', () => {
// 'PersonMiddleName' value expected to be null because this sub-field does not have
// retain_hidden_value = true, even though its parent Complex field does
expect(component.form.get('data').get(`${CASE_FIELD_2_COMPLEX.id}.PersonMiddleName`).value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COMPLEX.id)).not.toBeNull();
expect(component.form.get('data').get(`${CASE_FIELD_3_COMPLEX.id}.AddressLine1`)).not.toBeNull();
expect(component.form.get('data').get(`${CASE_FIELD_3_COMPLEX.id}.AddressLine1`).value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COMPLEX.id)).toBeNull();
});

it('should navigate to previous page when previous is called and clear hidden collection form field', () => {
Expand Down Expand Up @@ -1020,11 +1012,7 @@ describe('CaseEditComponent', () => {
// retain_hidden_value = true, even though its top-level collection field does
expect((component.form.get('data').get(CASE_FIELD_2_COLLECTION.id) as FormArray).at(0)
.get('value.PersonMiddleName').value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COLLECTION.id)).not.toBeNull();
expect((component.form.get('data').get(CASE_FIELD_3_COLLECTION.id) as FormArray).at(0)
.get('value.AddressLine1')).not.toBeNull();
expect((component.form.get('data').get(CASE_FIELD_3_COLLECTION.id) as FormArray).at(0)
.get('value.AddressLine1').value).toBeNull();
expect(component.form.get('data').get(CASE_FIELD_3_COLLECTION.id)).toBeNull();
});

it('should not delete sub-field value if the FormGroup for the parent Complex hidden field cannot be determined', () => {
Expand Down
Loading

0 comments on commit 0d6dcba

Please sign in to comment.