Skip to content

Commit 4f5c606

Browse files
FrancescoMolinaroFrancescoMolinaro
committed
[DSC-2599] refactor and fix form component error handling
1 parent 4191984 commit 4f5c606

3 files changed

Lines changed: 105 additions & 64 deletions

File tree

‎src/app/shared/form/form.component.ts‎

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -194,54 +194,75 @@ export class FormComponent implements OnDestroy, OnInit {
194194
this.formService.getForm(this.formId).pipe(
195195
filter((formState: FormEntry) => !!formState && (isNotEmpty(formState.errors) || isNotEmpty(this.formErrors))),
196196
map((formState) => formState.errors),
197-
distinctUntilChanged())
198-
.subscribe((errors: FormError[]) => {
199-
const { formGroup, formModel } = this;
200-
const existingMessages = new Set(this.formErrors.map(e => e.message));
201-
202-
errors
203-
.filter((error: FormError) => !existingMessages.has(error.message))
204-
.forEach((error: FormError) => {
205-
const { fieldId } = error;
206-
const { fieldIndex } = error;
207-
let field: AbstractControl;
208-
if (!!this.parentFormModel) {
209-
field = this.formBuilderService.getFormControlById(fieldId, formGroup.parent as UntypedFormGroup, formModel, fieldIndex);
210-
} else {
211-
field = this.formBuilderService.getFormControlById(fieldId, formGroup, formModel, fieldIndex);
212-
}
197+
distinctUntilChanged(),
198+
).subscribe((errors: FormError[]) => {
199+
const { formGroup, formModel } = this;
200+
201+
const prevMap = new Map<string, FormError>(
202+
this.formErrors.map(e => [`${e.fieldId}:${e.fieldIndex}`, e]),
203+
);
204+
const nextMap = new Map<string, FormError>(
205+
errors.map(e => [`${e.fieldId}:${e.fieldIndex}`, e]),
206+
);
207+
208+
if (isEqual(prevMap, nextMap)) {
209+
return;
210+
}
213211

214-
if (field) {
215-
const modelArrayIndex = fieldIndex > 0 ? fieldIndex : null;
216-
const model: DynamicFormControlModel = this.formBuilderService.findById(fieldId, formModel, modelArrayIndex);
217-
this.formService.addErrorToField(field, model, error.message);
218-
this.changeDetectorRef.detectChanges();
219-
}
220-
});
221-
222-
this.formErrors
223-
.filter((error: FormError) => findIndex(errors, {
224-
fieldId: error.fieldId,
225-
fieldIndex: error.fieldIndex
226-
}) === -1)
227-
.forEach((error: FormError) => {
228-
const { fieldId } = error;
229-
const { fieldIndex } = error;
230-
let field: AbstractControl;
231-
if (!!this.parentFormModel) {
232-
field = this.formBuilderService.getFormControlById(fieldId, formGroup.parent as UntypedFormGroup, formModel, fieldIndex);
233-
} else {
234-
field = this.formBuilderService.getFormControlById(fieldId, formGroup, formModel, fieldIndex);
212+
const getControl = (err: FormError): AbstractControl | null => {
213+
return this.parentFormModel
214+
? this.formBuilderService.getFormControlById(err.fieldId, formGroup.parent as UntypedFormGroup, formModel, err.fieldIndex)
215+
: this.formBuilderService.getFormControlById(err.fieldId, formGroup, formModel, err.fieldIndex);
216+
};
217+
218+
const getModel = (err: FormError): DynamicFormControlModel => {
219+
const modelArrayIndex = err.fieldIndex > 0 ? err.fieldIndex : null;
220+
return this.formBuilderService.findById(err.fieldId, formModel, modelArrayIndex);
221+
};
222+
// Add or change (including revert) errors
223+
errors.forEach(next => {
224+
const key = `${next.fieldId}:${next.fieldIndex}`;
225+
const prev = prevMap.get(key);
226+
if (!prev || prev.message !== next.message) {
227+
// Remove old message if changed
228+
if (prev) {
229+
const prevControl = getControl(prev);
230+
if (prevControl) {
231+
const prevModel = getModel(prev);
232+
this.formService.removeErrorFromField(prevControl, prevModel, prev.message);
233+
this.formService.removeError(this.formId, prev.fieldId, prev.fieldIndex);
234+
this.formErrors.splice(findIndex(this.formErrors, prev), 1);
235235
}
236+
}
237+
// Add new message
238+
const control = getControl(next);
239+
if (control) {
240+
const model = getModel(next);
241+
this.formService.addErrorToField(control, model, next.message);
242+
this.formErrors.push(next);
243+
}
244+
}
245+
});
246+
247+
const removedErrors: FormError[] = [];
248+
// Remove errors for fields no longer present
249+
this.formErrors.forEach(prev => {
250+
const key = `${prev.fieldId}:${prev.fieldIndex}`;
251+
console.log('checking removal of', key, prevMap.has(key));
252+
if (!nextMap.has(key) && prevMap.has(key)) {
253+
const control = getControl(prev);
254+
if (control) {
255+
const model = getModel(prev);
256+
this.formService.removeErrorFromField(control, model, prev.message);
257+
this.formService.removeError(this.formId, prev.fieldId, prev.fieldIndex);
258+
removedErrors.push(prev);
259+
}
260+
}
261+
});
236262

237-
if (field) {
238-
const model: DynamicFormControlModel = this.formBuilderService.findById(fieldId, formModel);
239-
this.formService.removeErrorFromField(field, model, error.message);
240-
}
241-
});
242-
this.formErrors = errors;
243-
this.changeDetectorRef.detectChanges();
244-
})
263+
this.formErrors = this.formErrors.filter(error => !removedErrors.includes(error));
264+
this.changeDetectorRef.detectChanges();
265+
}),
245266
);
246267
}
247268

‎src/app/shared/form/form.service.spec.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { AppState } from '../../app.reducer';
1010
import { formReducer } from './form.reducer';
1111
import { getMockFormBuilderService } from '../mocks/form-builder-service.mock';
1212
import { DynamicConcatModel } from './builder/ds-dynamic-form-ui/models/ds-dynamic-concat.model';
13+
import { getMockTranslateService } from "../mocks/translate.service.mock";
1314

1415
describe('FormService test suite', () => {
1516
const config = {
@@ -24,6 +25,7 @@ describe('FormService test suite', () => {
2425
let service: FormService;
2526
let builderService: FormBuilderService;
2627
let formGroup: UntypedFormGroup;
28+
const translateService = getMockTranslateService();
2729

2830
const formModel: DynamicFormControlModel[] = [
2931
new DynamicInputModel({ id: 'author', value: 'test' }),
@@ -129,10 +131,10 @@ describe('FormService test suite', () => {
129131
name_CONCAT_SECOND_INPUT: new FormControl(undefined)
130132
});
131133

132-
formGroup = new UntypedFormGroup({ author, title, date, description, addressLocation, name });
133-
controls = { author, title, date, description , addressLocation, name };
134-
service = new FormService(builderService, store);
135-
})
134+
formGroup = new UntypedFormGroup({ author, title, date, description, addressLocation, name });
135+
controls = { author, title, date, description , addressLocation, name };
136+
service = new FormService(builderService, store, translateService);
137+
}),
136138
)
137139
;
138140

@@ -226,6 +228,17 @@ describe('FormService test suite', () => {
226228

227229
});
228230

231+
it('should show correct error message if a different error is already present in the field and a new is added', () => {
232+
let control = controls.description;
233+
let model = formModel.find((mdl: DynamicFormControlModel) => mdl.id === 'description');
234+
235+
service.addErrorToField(control, model, 'error.test.message');
236+
service.addErrorToField(control, model, 'error.test.newMessage');
237+
service.removeErrorFromField(control, model, 'error.test.message');
238+
239+
expect(control.errors).toEqual({ newMessage: true });
240+
});
241+
229242
it('should remove error from field', () => {
230243
let control = controls.description;
231244
let model = formModel.find((mdl: DynamicFormControlModel) => mdl.id === 'description');
@@ -249,7 +262,7 @@ describe('FormService test suite', () => {
249262

250263
service.removeErrorFromField(control, model, 'error.required');
251264

252-
expect(control.errors).toBeNull();
265+
expect(control.errors).toEqual({ required: null });
253266
});
254267

255268
it('should remove errors from fields of concat group', () => {
@@ -258,8 +271,8 @@ describe('FormService test suite', () => {
258271
let control = controls.name;
259272
let model = formModel.find((mdl: DynamicFormControlModel) => mdl.id === 'name_CONCAT_GROUP');
260273
let errorKeys: string[];
261-
262-
service.addErrorToField(control, model, 'Test error message');
274+
const messageKey = 'Test error message';
275+
service.addErrorToField(control, model, messageKey);
263276
errorKeys = Object.keys(control.errors);
264277

265278
service.removeErrorFromField(control, model, errorKeys[0]);
@@ -271,7 +284,7 @@ describe('FormService test suite', () => {
271284

272285
// the group's inputs should no longer have an error
273286
Object.values(control.controls).forEach((subControl: AbstractControl) => {
274-
expect(control.errors).toBeNull();
287+
expect(control.errors).toEqual({ [messageKey]: null });
275288
});
276289
});
277290

‎src/app/shared/form/form.service.ts‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@ import {
2222
import { FormEntry, FormError, FormTouchedState } from './form.reducer';
2323
import { environment } from '../../../environments/environment';
2424
import { DynamicLinkModel } from './builder/ds-dynamic-form-ui/models/ds-dynamic-link.model';
25+
import { TranslateService } from "@ngx-translate/core";
2526

2627
@Injectable()
2728
export class FormService {
2829

2930
constructor(
3031
private formBuilderService: FormBuilderService,
31-
private store: Store<AppState>) {
32+
private store: Store<AppState>,
33+
private trnslateService: TranslateService) {
3234
}
3335

3436
/**
@@ -136,15 +138,19 @@ export class FormService {
136138
}
137139
const errors: string[] = Object.keys(field.errors)
138140
.filter((errorKey) => field.errors[errorKey] === true)
139-
.map((errorKey) => `error.validation.${errorKey}`);
141+
.map((errorKey) => {
142+
const defaultErrorKey = `error.validation.${errorKey}`;
143+
const customErrorKey = `error.validation.${formId}.${errorKey}`;
144+
const hasDefaultLabel = this.trnslateService.instant(defaultErrorKey) !== defaultErrorKey;
145+
return hasDefaultLabel ? defaultErrorKey : customErrorKey;
146+
});
140147
errors.forEach((error) => this.addError(formId, fieldId, fieldIndex, error));
141148
}
142149

143150
public addErrorToField(field: AbstractControl, model: DynamicFormControlModel, message: string) {
144151

145152
const error = {}; // create the error object
146153
const errorKey = this.getValidatorNameFromMap(message);
147-
let errorMsg = message;
148154

149155
// if form control model has no errorMessages object, create it
150156
if (!model.errorMessages) {
@@ -155,11 +161,7 @@ export class FormService {
155161
if (isEmpty(model.errorMessages[errorKey])) {
156162
// put the error message in the form control model
157163
model.errorMessages[errorKey] = message;
158-
} else {
159-
// Use correct error messages from the model
160-
errorMsg = model.errorMessages[errorKey];
161164
}
162-
163165
if (!field.hasError(errorKey)) {
164166
error[errorKey] = true;
165167
// add the error in the form control
@@ -182,11 +184,11 @@ export class FormService {
182184
public removeErrorFromField(field: AbstractControl, model: DynamicFormControlModel, messageKey: string) {
183185
const error = {};
184186
const errorKey = this.getValidatorNameFromMap(messageKey);
185-
186187
if (field.hasError(errorKey)) {
187188
error[errorKey] = null;
188-
field.setErrors(error);
189-
field.clearValidators();
189+
const updatedError = { ...field.errors, ...error };
190+
field.setErrors(updatedError);
191+
field.setValidators(() => updatedError);
190192
field.updateValueAndValidity();
191193
}
192194

@@ -199,7 +201,12 @@ export class FormService {
199201
});
200202
}
201203

202-
field.markAsUntouched();
204+
const hasDifferentErrors = Object.keys(field.errors).filter((key) => field.errors[key]).length > 0;
205+
206+
if (isEmpty(field.errors) || !hasDifferentErrors) {
207+
field.markAsUntouched();
208+
}
209+
203210
}
204211

205212
public resetForm(formGroup: UntypedFormGroup, groupModel: DynamicFormControlModel[], formId: string) {

0 commit comments

Comments
 (0)