Skip to content

Commit a3947d9

Browse files
FrancescoMolinaroFrancescoMolinaro
committed
[DSC-2599] refactor and fix form component error handling
1 parent 00e2122 commit a3947d9

3 files changed

Lines changed: 102 additions & 61 deletions

File tree

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

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -237,54 +237,75 @@ export class FormComponent implements OnDestroy, OnInit {
237237
this.formService.getForm(this.formId).pipe(
238238
filter((formState: FormEntry) => !!formState && (isNotEmpty(formState.errors) || isNotEmpty(this.formErrors))),
239239
map((formState) => formState.errors),
240-
distinctUntilChanged())
241-
.subscribe((errors: FormError[]) => {
242-
const { formGroup, formModel } = this;
243-
const existingMessages = new Set(this.formErrors.map(e => e.message));
244-
245-
errors
246-
.filter((error: FormError) => !existingMessages.has(error.message))
247-
.forEach((error: FormError) => {
248-
const { fieldId } = error;
249-
const { fieldIndex } = error;
250-
let field: AbstractControl;
251-
if (this.parentFormModel) {
252-
field = this.formBuilderService.getFormControlById(fieldId, formGroup.parent as UntypedFormGroup, formModel, fieldIndex);
253-
} else {
254-
field = this.formBuilderService.getFormControlById(fieldId, formGroup, formModel, fieldIndex);
255-
}
240+
distinctUntilChanged(),
241+
).subscribe((errors: FormError[]) => {
242+
const { formGroup, formModel } = this;
243+
244+
const prevMap = new Map<string, FormError>(
245+
this.formErrors.map(e => [`${e.fieldId}:${e.fieldIndex}`, e]),
246+
);
247+
const nextMap = new Map<string, FormError>(
248+
errors.map(e => [`${e.fieldId}:${e.fieldIndex}`, e]),
249+
);
250+
251+
if (isEqual(prevMap, nextMap)) {
252+
return;
253+
}
256254

257-
if (field) {
258-
const modelArrayIndex = fieldIndex > 0 ? fieldIndex : null;
259-
const model: DynamicFormControlModel = this.formBuilderService.findById(fieldId, formModel, modelArrayIndex);
260-
this.formService.addErrorToField(field, model, error.message);
261-
this.changeDetectorRef.detectChanges();
262-
}
263-
});
264-
265-
this.formErrors
266-
.filter((error: FormError) => findIndex(errors, {
267-
fieldId: error.fieldId,
268-
fieldIndex: error.fieldIndex,
269-
}) === -1)
270-
.forEach((error: FormError) => {
271-
const { fieldId } = error;
272-
const { fieldIndex } = error;
273-
let field: AbstractControl;
274-
if (this.parentFormModel) {
275-
field = this.formBuilderService.getFormControlById(fieldId, formGroup.parent as UntypedFormGroup, formModel, fieldIndex);
276-
} else {
277-
field = this.formBuilderService.getFormControlById(fieldId, formGroup, formModel, fieldIndex);
255+
const getControl = (err: FormError): AbstractControl | null => {
256+
return this.parentFormModel
257+
? this.formBuilderService.getFormControlById(err.fieldId, formGroup.parent as UntypedFormGroup, formModel, err.fieldIndex)
258+
: this.formBuilderService.getFormControlById(err.fieldId, formGroup, formModel, err.fieldIndex);
259+
};
260+
261+
const getModel = (err: FormError): DynamicFormControlModel => {
262+
const modelArrayIndex = err.fieldIndex > 0 ? err.fieldIndex : null;
263+
return this.formBuilderService.findById(err.fieldId, formModel, modelArrayIndex);
264+
};
265+
// Add or change (including revert) errors
266+
errors.forEach(next => {
267+
const key = `${next.fieldId}:${next.fieldIndex}`;
268+
const prev = prevMap.get(key);
269+
if (!prev || prev.message !== next.message) {
270+
// Remove old message if changed
271+
if (prev) {
272+
const prevControl = getControl(prev);
273+
if (prevControl) {
274+
const prevModel = getModel(prev);
275+
this.formService.removeErrorFromField(prevControl, prevModel, prev.message);
276+
this.formService.removeError(this.formId, prev.fieldId, prev.fieldIndex);
277+
this.formErrors.splice(findIndex(this.formErrors, prev), 1);
278278
}
279+
}
280+
// Add new message
281+
const control = getControl(next);
282+
if (control) {
283+
const model = getModel(next);
284+
this.formService.addErrorToField(control, model, next.message);
285+
this.formErrors.push(next);
286+
}
287+
}
288+
});
289+
290+
const removedErrors: FormError[] = [];
291+
// Remove errors for fields no longer present
292+
this.formErrors.forEach(prev => {
293+
const key = `${prev.fieldId}:${prev.fieldIndex}`;
294+
console.log('checking removal of', key, prevMap.has(key));
295+
if (!nextMap.has(key) && prevMap.has(key)) {
296+
const control = getControl(prev);
297+
if (control) {
298+
const model = getModel(prev);
299+
this.formService.removeErrorFromField(control, model, prev.message);
300+
this.formService.removeError(this.formId, prev.fieldId, prev.fieldIndex);
301+
removedErrors.push(prev);
302+
}
303+
}
304+
});
279305

280-
if (field) {
281-
const model: DynamicFormControlModel = this.formBuilderService.findById(fieldId, formModel, fieldIndex);
282-
this.formService.removeErrorFromField(field, model, error.message);
283-
}
284-
});
285-
this.formErrors = errors;
286-
this.changeDetectorRef.detectChanges();
287-
}),
306+
this.formErrors = this.formErrors.filter(error => !removedErrors.includes(error));
307+
this.changeDetectorRef.detectChanges();
308+
}),
288309
);
289310
}
290311

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222

2323
import { AppState } from '../../app.reducer';
2424
import { getMockFormBuilderService } from '../mocks/form-builder-service.mock';
25+
import { getMockTranslateService } from '../mocks/translate.service.mock';
2526
import { DynamicConcatModel } from './builder/ds-dynamic-form-ui/models/ds-dynamic-concat.model';
2627
import { FormBuilderService } from './builder/form-builder.service';
2728
import { formReducer } from './form.reducer';
@@ -40,6 +41,7 @@ describe('FormService test suite', () => {
4041
let service: FormService;
4142
let builderService: FormBuilderService;
4243
let formGroup: UntypedFormGroup;
44+
const translateService = getMockTranslateService();
4345

4446
const formModel: DynamicFormControlModel[] = [
4547
new DynamicInputModel({ id: 'author', value: 'test' }),
@@ -147,7 +149,7 @@ describe('FormService test suite', () => {
147149

148150
formGroup = new UntypedFormGroup({ author, title, date, description, addressLocation, name });
149151
controls = { author, title, date, description , addressLocation, name };
150-
service = new FormService(builderService, store);
152+
service = new FormService(builderService, store, translateService);
151153
}),
152154
)
153155
;
@@ -242,6 +244,17 @@ describe('FormService test suite', () => {
242244

243245
});
244246

247+
it('should show correct error message if a different error is already present in the field and a new is added', () => {
248+
let control = controls.description;
249+
let model = formModel.find((mdl: DynamicFormControlModel) => mdl.id === 'description');
250+
251+
service.addErrorToField(control, model, 'error.test.message');
252+
service.addErrorToField(control, model, 'error.test.newMessage');
253+
service.removeErrorFromField(control, model, 'error.test.message');
254+
255+
expect(control.errors).toEqual({ newMessage: true });
256+
});
257+
245258
it('should remove error from field', () => {
246259
let control = controls.description;
247260
let model = formModel.find((mdl: DynamicFormControlModel) => mdl.id === 'description');
@@ -265,7 +278,7 @@ describe('FormService test suite', () => {
265278

266279
service.removeErrorFromField(control, model, 'error.required');
267280

268-
expect(control.errors).toBeNull();
281+
expect(control.errors).toEqual({ required: null });
269282
});
270283

271284
it('should remove errors from fields of concat group', () => {
@@ -274,8 +287,8 @@ describe('FormService test suite', () => {
274287
let control = controls.name;
275288
let model = formModel.find((mdl: DynamicFormControlModel) => mdl.id === 'name_CONCAT_GROUP');
276289
let errorKeys: string[];
277-
278-
service.addErrorToField(control, model, 'Test error message');
290+
const messageKey = 'Test error message';
291+
service.addErrorToField(control, model, messageKey);
279292
errorKeys = Object.keys(control.errors);
280293

281294
service.removeErrorFromField(control, model, errorKeys[0]);
@@ -287,7 +300,7 @@ describe('FormService test suite', () => {
287300

288301
// the group's inputs should no longer have an error
289302
Object.values(control.controls).forEach((subControl: AbstractControl) => {
290-
expect(control.errors).toBeNull();
303+
expect(control.errors).toEqual({ [messageKey]: null });
291304
});
292305
});
293306

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

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
select,
1515
Store,
1616
} from '@ngrx/store';
17+
import { TranslateService } from '@ngx-translate/core';
1718
import uniqueId from 'lodash/uniqueId';
1819
import { Observable } from 'rxjs';
1920
import {
@@ -51,7 +52,8 @@ export class FormService {
5152

5253
constructor(
5354
private formBuilderService: FormBuilderService,
54-
private store: Store<AppState>) {
55+
private store: Store<AppState>,
56+
private trnslateService: TranslateService) {
5557
}
5658

5759
/**
@@ -159,15 +161,19 @@ export class FormService {
159161
}
160162
const errors: string[] = Object.keys(field.errors)
161163
.filter((errorKey) => field.errors[errorKey] === true)
162-
.map((errorKey) => `error.validation.${errorKey}`);
164+
.map((errorKey) => {
165+
const defaultErrorKey = `error.validation.${errorKey}`;
166+
const customErrorKey = `error.validation.${formId}.${errorKey}`;
167+
const hasDefaultLabel = this.trnslateService.instant(defaultErrorKey) !== defaultErrorKey;
168+
return hasDefaultLabel ? defaultErrorKey : customErrorKey;
169+
});
163170
errors.forEach((error) => this.addError(formId, fieldId, fieldIndex, error));
164171
}
165172

166173
public addErrorToField(field: AbstractControl, model: DynamicFormControlModel, message: string) {
167174

168175
const error = {}; // create the error object
169176
const errorKey = this.getValidatorNameFromMap(message);
170-
let errorMsg = message;
171177

172178
// if form control model has no errorMessages object, create it
173179
if (!model.errorMessages) {
@@ -178,11 +184,7 @@ export class FormService {
178184
if (isEmpty(model.errorMessages[errorKey])) {
179185
// put the error message in the form control model
180186
model.errorMessages[errorKey] = message;
181-
} else {
182-
// Use correct error messages from the model
183-
errorMsg = model.errorMessages[errorKey];
184187
}
185-
186188
if (!field.hasError(errorKey)) {
187189
error[errorKey] = true;
188190
// add the error in the form control
@@ -205,11 +207,11 @@ export class FormService {
205207
public removeErrorFromField(field: AbstractControl, model: DynamicFormControlModel, messageKey: string) {
206208
const error = {};
207209
const errorKey = this.getValidatorNameFromMap(messageKey);
208-
209210
if (field.hasError(errorKey)) {
210211
error[errorKey] = null;
211-
field.setErrors(error);
212-
field.clearValidators();
212+
const updatedError = { ...field.errors, ...error };
213+
field.setErrors(updatedError);
214+
field.setValidators(() => updatedError);
213215
field.updateValueAndValidity();
214216
}
215217

@@ -222,7 +224,12 @@ export class FormService {
222224
});
223225
}
224226

225-
field.markAsUntouched();
227+
const hasDifferentErrors = Object.keys(field.errors).filter((key) => field.errors[key]).length > 0;
228+
229+
if (isEmpty(field.errors) || !hasDifferentErrors) {
230+
field.markAsUntouched();
231+
}
232+
226233
}
227234

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

0 commit comments

Comments
 (0)