Skip to content

Commit

Permalink
Merge pull request #25 from cloudnc/feat/fix-validation-and-add-neste…
Browse files Browse the repository at this point in the history
…d-errors

feat: fix validation and add nested errors
  • Loading branch information
zakhenry authored Mar 28, 2019
2 parents 59c1fec + 06d2f0f commit d4bc09d
Show file tree
Hide file tree
Showing 12 changed files with 272 additions and 51 deletions.
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,11 @@ export class VehicleProductComponent extends NgxSubFormRemapComponent<OneVehicle
}
}
```

### Handling errors

There's currently a weird behavior ~~[issue (?)](https://github.com/angular/angular/issues/18004)~~ when checking for form validity. CF that [issue](https://github.com/angular/angular/issues/18004) and that [comment](https://github.com/angular/angular/issues/18004#issuecomment-328806479). It is also detailed into [`listing.component.html`](https://github.com/cloudnc/ngx-sub-form/blob/master/src/app/main/listing/listing.component.html).

That said, let's move on to one of the great feature of ngx-sub-form: The possibility to have access to **nested** form errors. How? When extending `NgxSubFormComponent` or `NgxSubFormRemapComponent` you'll have access to the `formGroupErrors` property. That's it.

You can see the online demo [here](https://cloudnc.github.io/ngx-sub-form).
2 changes: 2 additions & 0 deletions cypress/helpers/data.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,5 @@ export const hardcodedElementToTestElement = (item: OneListing): ListElement =>

export const hardcodedElementsToTestList = (items: OneListing[]): ListElement[] =>
items.map(item => hardcodedElementToTestElement(item));

export const extractErrors = (errors: JQuery<HTMLElement>) => cy.wrap(JSON.parse(errors.text().trim()));
46 changes: 45 additions & 1 deletion cypress/helpers/dom.helper.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/// <reference types="Cypress" />

import { ListElement, FormElement } from './data.helper';
import { ListElement, FormElement, extractErrors } from './data.helper';
import { VehicleType } from '../../src/app/interfaces/vehicle.interface';
import { ListingType } from '../../src/app/interfaces/listing.interface';
import { DroidType } from '../../src/app/interfaces/droid.interface';

const getTextFromTag = (element: HTMLElement, tag: string): string =>
Cypress.$(element)
Expand All @@ -26,6 +28,9 @@ const getToggleValue = (element: HTMLElement, tag: string): boolean =>
.hasClass('mat-checked');

export const DOM = {
get createNewButton() {
return cy.get('*[data-create-new]');
},
get list() {
return {
get cy() {
Expand Down Expand Up @@ -60,6 +65,9 @@ export const DOM = {
get cy() {
return cy.get('app-listing');
},
get errors() {
return cy.get(`*[data-errors]`).then(extractErrors);
},
get obj(): Cypress.Chainable<FormElement> {
const getVehiculeObj = (element: HTMLElement, type: VehicleType) =>
({
Expand Down Expand Up @@ -104,6 +112,42 @@ export const DOM = {
.get()[0];
});
},
get elements() {
return {
get title() {
return cy.get(`*[data-input-title]`);
},
get imageUrl() {
return cy.get(`*[data-input-image-url]`);
},
get price() {
return cy.get(`*[data-input-price]`);
},
get selectListingType() {
return cy.get(`*[data-select-listing-type]`);
},
selectListingTypeByType: (type: ListingType) => {
DOM.form.elements.selectListingType.click();

return cy.get(`*[data-select-listing-type-option]`).within(() => cy.contains(type).click());
},
get droidForm() {
return {
get name() {
return cy.get(`*[data-input-name]`);
},
get selectDroidType() {
return cy.get(`*[data-select-droid-type]`);
},
selectDroidTypeByType: (type: DroidType) => {
DOM.form.elements.droidForm.selectDroidType.click();

return cy.get(`*[data-select-droid-type-option]`).within(() => cy.contains(type).click());
},
};
},
};
},
};
},
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"------------------ BASE COMMANDS -----------------": "",
"ng": "ng",
"prettier": "prettier",
"prettier:base": "yarn run prettier \"**/*.{js,json,css,md,ts,html,component.html}\"",
"prettier:base": "yarn run prettier \"**/*.{js,json,scss,md,ts,html,component.html}\"",
"prettier:write": "yarn run prettier:base --write",
"prettier:check": "yarn run prettier:base --list-different",
"cy": "cypress",
Expand Down
39 changes: 22 additions & 17 deletions projects/ngx-sub-form/src/lib/ngx-sub-form.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,14 @@ class SubComponent extends NgxSubFormComponent<Vehicle> {
describe(`NgxSubFormComponent`, () => {
let subComponent: SubComponent;

beforeEach(() => {
beforeEach((done: () => void) => {
subComponent = new SubComponent();
subComponent.formControlName = subComponent.formControlNames.color;

// we have to call `updateValueAndValidity` within the constructor in an async way
// and here we need to wait for it to run
setTimeout(() => {
done();
}, 0);
});

describe(`created`, () => {
Expand Down Expand Up @@ -87,36 +92,31 @@ describe(`NgxSubFormComponent`, () => {
});

describe(`validation`, () => {
// when formControlName is not passed, it means that the component is being used
// on the top level to get utilities from the lib (type safety for e.g.) but validation should not run in that case
it(`should return null if formControlName is not defined`, () => {
it(`should validate the field and return an object containing the errors if the formGroup is defined and invalid`, () => {
// set one of the control to be invalid
(subComponent.formGroup.get(subComponent.formControlNames.numberOfPeopleOnBoard) as FormControl).setValue(
MIN_NUMBER_OF_PEOPLE_ON_BOARD - 1,
);
subComponent.formGroup.markAsDirty();
subComponent.formControlName = undefined;
expect(subComponent.validate()).toBeNull();
expect(subComponent.validate()).toEqual({
[subComponent.formControlNames.numberOfPeopleOnBoard]: { min: { min: 5, actual: 4 } },
});
});

it(`should validate the field and return an object containing the formControlName set to true if the formGroup is defined, invalid and dirty`, () => {
subComponent.formControlName = subComponent.formControlNames.color;
it(`should give access to a formGroupErrors property`, () => {
// set one of the control to be invalid
(subComponent.formGroup.get(subComponent.formControlNames.numberOfPeopleOnBoard) as FormControl).setValue(
MIN_NUMBER_OF_PEOPLE_ON_BOARD - 1,
);
subComponent.formGroup.markAsDirty();
subComponent.formGroup.updateValueAndValidity();
expect(subComponent.validate()).toEqual({ [subComponent.formControlNames.color]: true });
expect(subComponent.formGroupErrors).toEqual({
[subComponent.formControlNames.numberOfPeopleOnBoard]: { min: { min: 5, actual: 4 } },
});
});

describe(`should validate the field and return null if the formGroup is`, () => {
it(`not defined`, () => {
subComponent.ngOnDestroy();

expect(subComponent.validate()).toBeNull();
});

it(`valid or pristine`, () => {
// by default in that example defined, valid and pristine
expect(subComponent.validate()).toBeNull();
Expand Down Expand Up @@ -236,9 +236,14 @@ class SubRemapComponent extends NgxSubFormRemapComponent<Vehicle, VehiculeForm>
describe(`NgxSubFormRemapComponent`, () => {
let subRemapComponent: SubRemapComponent;

beforeEach(() => {
beforeEach((done: () => void) => {
subRemapComponent = new SubRemapComponent();
subRemapComponent.formControlName = subRemapComponent.formControlNames.vehiculeColor;

// we have to call `updateValueAndValidity` within the constructor in an async way
// and here we need to wait for it to run
setTimeout(() => {
done();
}, 0);
});

describe(`value updated by the parent (write)`, () => {
Expand Down
48 changes: 32 additions & 16 deletions projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Input, OnDestroy, OnInit } from '@angular/core';
import { OnDestroy } from '@angular/core';
import { ControlValueAccessor, FormGroup, ValidationErrors, Validator } from '@angular/forms';
import { Subscription } from 'rxjs';
import { delay, tap } from 'rxjs/operators';
Expand All @@ -10,34 +10,53 @@ export abstract class NgxSubFormComponent<ControlInterface, FormInterface = Cont
return getControlsNames(this.getFormControls());
}

public get formGroupErrors(): ValidationErrors {
const errors: ValidationErrors = {};

for (const key in this.formGroup.controls) {
if (this.formGroup.controls.hasOwnProperty(key)) {
const control = this.formGroup.controls[key];

if (!control.valid) {
errors[key] = control.errors;
}
}
}

return errors;
}

public formGroup: FormGroup = new FormGroup(this.getFormControls());

protected onChange: Function | undefined = undefined;
protected onTouched: Function | undefined = undefined;

private subscription: Subscription | undefined = undefined;

@Input()
public formControlName: string | undefined = undefined;

// can't define them directly
protected abstract getFormControls(): Controls<FormInterface>;

constructor() {
// `setTimeout` and `updateValueAndValidity` are both required here
// indeed, if you check the demo you'll notice that without it, if
// you select `Droid` and `Assassin` for example the displayed errors
// are not yet defined for the field `assassinDroid`
// (until you change one of the value in that form)
setTimeout(() => {
this.formGroup.updateValueAndValidity({ emitEvent: false });
}, 0);
}

public validate(): ValidationErrors | null {
// @hack see below where defining this.formGroup to null
if (
// @hack see below where defining this.formGroup to null
!this.formGroup ||
this.formGroup.valid ||
this.formGroup.pristine ||
// when using NgxSubForm on the top level component to get type checking and other utilities
// but without binding it to a formControl, we might not have that value and thus, validate
// should be null (should not happen at all)
!this.formControlName
this.formGroup.valid
) {
return null;
}

return { [this.formControlName]: true };
return this.formGroupErrors;
}

public ngOnDestroy(): void {
Expand Down Expand Up @@ -66,17 +85,14 @@ export abstract class NgxSubFormComponent<ControlInterface, FormInterface = Cont
this.formGroup.setValue(this.transformToFormGroup(obj), {
emitEvent: false,
});

this.formGroup.markAsPristine();
this.formGroup.markAsUntouched();
}
} else {
// @todo clear form?
}
}

// ----------------------------------------------------
// ----------------------------------------------------
// ----------------------------------------------------
// that method can be overridden if the
// shape of the form needs to be modified
protected transformToFormGroup(obj: ControlInterface | null): FormInterface {
Expand Down
92 changes: 91 additions & 1 deletion src/app/app.spec.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import { DOM } from '../../cypress/helpers/dom.helper';
import { hardCodedListings } from './services/listings.data';
import { hardcodedElementsToTestList, FormElement } from '../../cypress/helpers/data.helper';
import { VehicleListing } from './interfaces/listing.interface';
import { VehicleListing, ListingType } from './interfaces/listing.interface';
import { Spaceship, Speeder } from './interfaces/vehicle.interface';
import { DroidType } from './interfaces/droid.interface';

context(`EJawa demo`, () => {
beforeEach(() => {
Expand Down Expand Up @@ -75,4 +76,93 @@ context(`EJawa demo`, () => {

DOM.form.obj.should('eql', expectedObj);
});

it(`should display the (nested) errors from the form`, () => {
DOM.createNewButton.click();

DOM.form.errors.should('eql', {
listingType: {
required: true,
},
title: {
required: true,
},
imageUrl: {
required: true,
},
price: {
required: true,
},
});

DOM.form.elements.selectListingTypeByType(ListingType.DROID);

DOM.form.errors.should('eql', {
droidProduct: {
droidType: {
required: true,
},
},
title: {
required: true,
},
imageUrl: {
required: true,
},
price: {
required: true,
},
});

DOM.form.elements.droidForm.selectDroidTypeByType(DroidType.ASSASSIN);

DOM.form.errors.should('eql', {
droidProduct: {
assassinDroid: {
color: {
required: true,
},
name: {
required: true,
},
weapons: {
required: true,
},
},
},
title: {
required: true,
},
imageUrl: {
required: true,
},
price: {
required: true,
},
});

DOM.form.elements.droidForm.name.type(`IG-86 sentinel`);

DOM.form.errors.should('eql', {
droidProduct: {
assassinDroid: {
color: {
required: true,
},
weapons: {
required: true,
},
},
},
title: {
required: true,
},
imageUrl: {
required: true,
},
price: {
required: true,
},
});
});
});
Loading

0 comments on commit d4bc09d

Please sign in to comment.