Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lib): formGroup validator oneOf #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ export class CrewMemberComponent extends NgxSubFormComponent<CrewMember> {

- `emitNullOnDestroy`: By default is set to `true` for `NgxSubFormComponent`, `NgxSubFormRemapComponent` and to `false` for `NgxRootFormComponent` and `NgxAutomaticRootFormComponent`. When set to `true`, if the sub form component is being destroyed, it will emit one last value: `null`. It might be useful to set it to `false` for e.g. when you've got a form accross multiple tabs and once a part of the form is filled you want to destroy it
- `emitInitialValueOnInit`: By default is set to `true` for `NgxSubFormComponent`, `NgxSubFormRemapComponent` and to `false` for `NgxRootFormComponent` and `NgxAutomaticRootFormComponent`. When set to `true`, the sub form component will emit the first value straight away (default one unless the component above as a value already set on the `formControl`)
- `ngxSubFormValidators`: An object containing validator methods. Currently, only`oneOf`is available. It lets you specify on a`formGroup`using`getFormGroupControlOptions` that amongst some properties, exactly one should be defined (nothing more, nothing less)

**Hooks**

Expand Down Expand Up @@ -553,7 +554,7 @@ class PasswordSubFormComponent extends NgxSubFormComponent<PasswordForm> {
};
}

public getFormGroupControlOptions(): FormGroupOptions<PasswordForm> {
protected getFormGroupControlOptions(): FormGroupOptions<PasswordForm> {
return {
validators: [
formGroup => {
Expand Down
13 changes: 13 additions & 0 deletions projects/ngx-sub-form/src/lib/ngx-sub-form-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ export class MissingFormControlsError<T extends string> extends Error {
}
}

export class OneOfValidatorRequiresMoreThanOneFieldError extends Error {
constructor() {
super(`"oneOf" validator requires to have at least 2 keys`);
}
}

export class OneOfValidatorUnknownFieldError extends Error {
constructor(field: string) {
super(`"oneOf" validator requires to keys from the FormInterface and "${field}" is not`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

}
}

export const NGX_SUB_FORM_HANDLE_VALUE_CHANGES_RATE_STRATEGIES = {
debounce: <T, U>(time: number): ReturnType<NgxSubFormComponent<T, U>['handleEmissionRate']> => obs =>
obs.pipe(debounce(() => timer(time))),
Expand All @@ -85,6 +97,7 @@ export const NGX_SUB_FORM_HANDLE_VALUE_CHANGES_RATE_STRATEGIES = {
* If the component already has a `ngOnDestroy` method defined, it will call this first.
* Note that the component *must* implement OnDestroy for this to work (the typings will enforce this anyway)
*/
/** @internal */
export function takeUntilDestroyed<T>(component: OnDestroy): (source: Observable<T>) => Observable<T> {
return (source: Observable<T>): Observable<T> => {
const onDestroy = new Subject();
Expand Down
122 changes: 121 additions & 1 deletion projects/ngx-sub-form/src/lib/ngx-sub-form.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="jasmine" />

import { FormControl, Validators, FormArray } from '@angular/forms';
import { FormControl, Validators, FormArray, ValidatorFn } from '@angular/forms';
import {
FormGroupOptions,
NgxSubFormComponent,
Expand All @@ -11,6 +11,8 @@ import {
ArrayPropertyKey,
ArrayPropertyValue,
NgxFormWithArrayControls,
OneOfValidatorRequiresMoreThanOneFieldError,
OneOfValidatorUnknownFieldError,
} from '../public_api';
import { Observable } from 'rxjs';

Expand Down Expand Up @@ -442,10 +444,37 @@ describe(`NgxSubFormComponent`, () => {
}
}

interface DroidForm {
assassinDroid: { type: 'Assassin' };
medicalDroid: { type: 'Medical' };
}

class DroidFormComponent extends NgxSubFormComponent<DroidForm> {
protected getFormControls() {
return {
assassinDroid: new FormControl(null),
medicalDroid: new FormControl(null),
};
}

public getFormGroupControlOptions(): FormGroupOptions<DroidForm> {
return {
validators: [this.ngxSubFormValidators.oneOf([['assassinDroid', 'medicalDroid']])],
};
}

// testing utility
public setValidatorOneOf(keysArray: (keyof DroidForm)[][]): void {
this.formGroup.setValidators([(this.ngxSubFormValidators.oneOf(keysArray) as unknown) as ValidatorFn]);
}
}

let validatedSubComponent: ValidatedSubComponent;
let droidFormComponent: DroidFormComponent;

beforeEach((done: () => void) => {
validatedSubComponent = new ValidatedSubComponent();
droidFormComponent = new DroidFormComponent();

// we have to call `updateValueAndValidity` within the constructor in an async way
// and here we need to wait for it to run
Expand Down Expand Up @@ -473,6 +502,97 @@ describe(`NgxSubFormComponent`, () => {
}, 0);
}, 0);
});

describe('ngxSubFormValidators', () => {
it('oneOf should throw an error if no value or only one in the array', () => {
expect(() => droidFormComponent.setValidatorOneOf(undefined as any)).toThrow(
new OneOfValidatorRequiresMoreThanOneFieldError(),
);

expect(() => droidFormComponent.setValidatorOneOf([])).toThrow(
new OneOfValidatorRequiresMoreThanOneFieldError(),
);

expect(() => droidFormComponent.setValidatorOneOf([[]])).toThrow(
new OneOfValidatorRequiresMoreThanOneFieldError(),
);

expect(() => droidFormComponent.setValidatorOneOf([['assassinDroid']])).toThrow(
new OneOfValidatorRequiresMoreThanOneFieldError(),
);

expect(() => droidFormComponent.setValidatorOneOf([['assassinDroid', 'medicalDroid']])).not.toThrow();
});

it('oneOf should throw an error if there is an unknown key', () => {
droidFormComponent.setValidatorOneOf([['unknown 1' as any, 'unknown 2' as any]]);
expect(() => droidFormComponent.formGroup.updateValueAndValidity()).toThrow(
new OneOfValidatorUnknownFieldError('unknown 1'),
);

droidFormComponent.setValidatorOneOf([['assassinDroid', 'unknown 2' as any]]);
expect(() => droidFormComponent.formGroup.updateValueAndValidity()).toThrow(
new OneOfValidatorUnknownFieldError('unknown 2'),
);
});

it('oneOf should return an object (representing the error) if all the values are null', (done: () => void) => {
const spyOnChange = jasmine.createSpy();
droidFormComponent.registerOnChange(spyOnChange);

droidFormComponent.formGroup.patchValue({ assassinDroid: null, medicalDroid: null });

setTimeout(() => {
expect(droidFormComponent.validate()).toEqual({ formGroup: { oneOf: [['assassinDroid', 'medicalDroid']] } });
expect(droidFormComponent.formGroupErrors).toEqual({
formGroup: { oneOf: [['assassinDroid', 'medicalDroid']] },
});

droidFormComponent.formGroup.patchValue({
assassinDroid: { type: 'Assassin' },
medicalDroid: null,
});
setTimeout(() => {
expect(droidFormComponent.validate()).toEqual(null);
expect(droidFormComponent.formGroupErrors).toEqual(null);
done();
}, 0);
}, 0);
});

it('oneOf should return an object (error) if more than one value are not [null or undefined]', (done: () => void) => {
const spyOnChange = jasmine.createSpy();
droidFormComponent.registerOnChange(spyOnChange);

droidFormComponent.formGroup.patchValue({ assassinDroid: null, medicalDroid: null });

setTimeout(() => {
expect(droidFormComponent.validate()).toEqual({ formGroup: { oneOf: [['assassinDroid', 'medicalDroid']] } });

droidFormComponent.formGroup.patchValue({
assassinDroid: { type: 'Assassin' },
medicalDroid: { type: 'Medical' },
});

setTimeout(() => {
expect(droidFormComponent.validate()).toEqual({
formGroup: { oneOf: [['assassinDroid', 'medicalDroid']] },
});

droidFormComponent.formGroup.patchValue({
assassinDroid: null,
medicalDroid: { type: 'Medical' },
});

setTimeout(() => {
expect(droidFormComponent.validate()).toEqual(null);

done();
}, 0);
}, 0);
}, 0);
});
});
});
});

Expand Down
67 changes: 66 additions & 1 deletion projects/ngx-sub-form/src/lib/ngx-sub-form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@ import {
isNullOrUndefined,
ControlsType,
ArrayPropertyKey,
OneOfValidatorRequiresMoreThanOneFieldError,
OneOfValidatorUnknownFieldError,
} from './ngx-sub-form-utils';
import { FormGroupOptions, NgxFormWithArrayControls, OnFormUpdate, TypedFormGroup } from './ngx-sub-form.types';
import {
FormGroupOptions,
NgxFormWithArrayControls,
OnFormUpdate,
TypedFormGroup,
TypedValidatorFn,
} from './ngx-sub-form.types';

type MapControlFunction<FormInterface, MapValue> = (ctrl: AbstractControl, key: keyof FormInterface) => MapValue;
type FilterControlFunction<FormInterface> = (ctrl: AbstractControl, key: keyof FormInterface) => boolean;
Expand Down Expand Up @@ -66,6 +74,63 @@ export abstract class NgxSubFormComponent<ControlInterface, FormInterface = Cont

private controlKeys: (keyof FormInterface)[] = [];

// instead of having the validators defined in the utils file
// we define them here to have access to the form types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that really just means we should extract the types to their own file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point was to get access to the form types without having to specify them when calling the validator but I hear your point... (CF next answer)


// `ngxSubFormValidators` should be used at a group level validation
// with the `getFormGroupControlOptions` hook
protected ngxSubFormValidators = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I understand this pattern is following the angular pattern of namespace.validatorname, I think that concept by angular is actually a bad idea, because it is unfriendly to tree shakers - if you use one validator, youre basically guaranteed to have to import them all. I think a simple pure function per validator, in it's own file is the most tree shaking friendly option

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started exactly how you described it.
But then realized we'd have to pass the types as argument =/
Not impossible... not really awful but slightly more heavy 🤷‍♂️?

Although, I do have to admit that it feels wrong to have validators defined within the class and require the class to be instantiated before being able to use them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making another pass on this one, do we really care about tree shaking here? I wouldn't expect ngx-sub-form to embed plenty of validators (if so they should really be extracted into a separated package IMO). Plus, I'm not sure that using a namespace is better for tree shaking 🤔 but yes could have functions where we need to pass the type to.

oneOf(keysArray: (keyof FormInterface)[][]): TypedValidatorFn<FormInterface> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this an array of arrays?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because one form component may have multiple properties that are polymorphic.

Example:

interface User {
  id: string;
  vehicle: MotorBike | Car | Truck;
  animal: Cat | Dog;
}

In the case we want to handle them without breaking down into sub components we can do:

oneOf([
  ['motorBikeVehicle', 'CarVehicle', 'TruckVehicle'],
  ['CatVehicle', 'DogVehicle']
])

if (!keysArray || !keysArray.length || keysArray.some(keys => !keys || keys.length < 2)) {
throw new OneOfValidatorRequiresMoreThanOneFieldError();
}

return (formGroup: TypedFormGroup<FormInterface>) => {
const oneOfErrors: (keyof FormInterface)[][] = keysArray.reduce(
(acc, keys) => {
if (!keys.length) {
return acc;
}

let nbNotNull = 0;
let cpt = 0;

while (cpt < keys.length && nbNotNull < 2) {
const key: keyof FormInterface = keys[cpt];

const control: AbstractControl | null = formGroup.get(key as string);

if (!control) {
throw new OneOfValidatorUnknownFieldError(key as string);
}

if (!isNullOrUndefined(control.value)) {
nbNotNull++;
}

cpt++;
}

if (nbNotNull !== 1) {
acc.push(keys);
}

return acc;
},
[] as (keyof FormInterface)[][],
);

if (oneOfErrors.length === 0) {
return null;
}

return {
oneOf: oneOfErrors,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing some complex requirement here, but this feels like it could be simplified to the following (no idea if this compiles ;) )

const notNullKeys: Array<keyof FormInterface> = keysArray.filter(() => {
  const control: AbstractControl | null = formGroup.get(key as string);

  if (!control) {
    throw new OneOfValidatorUnknownFieldError(key as string);
  }

  return !isNullOrUndefined(control.value);
});

if (notNullKeys === 1) {
  return null;
}

return { oneOf: notNullKeys };

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That code doesn't handle if multiple values are defined. If there's more than 1 value, which one to pick? That should just be an error IMO

};
},
};

// when developing the lib it's a good idea to set the formGroup type
// to current + `| undefined` to catch a bunch of possible issues
// see @note form-group-undefined
Expand Down
15 changes: 15 additions & 0 deletions src/app/app.spec.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ context(`EJawa demo`, () => {
DOM.createNewButton.click();

DOM.form.errors.obj.should('eql', {
formGroup: {
oneOf: [['vehicleProduct', 'droidProduct']],
},
listingType: {
required: true,
},
Expand All @@ -98,7 +101,13 @@ context(`EJawa demo`, () => {
DOM.form.elements.selectListingTypeByType(ListingType.VEHICLE);

DOM.form.errors.obj.should('eql', {
formGroup: {
oneOf: [['vehicleProduct', 'droidProduct']],
},
vehicleProduct: {
formGroup: {
oneOf: [['speeder', 'spaceship']],
},
vehicleType: {
required: true,
},
Expand Down Expand Up @@ -180,7 +189,13 @@ context(`EJawa demo`, () => {
DOM.form.elements.selectListingTypeByType(ListingType.DROID);

DOM.form.errors.obj.should('eql', {
formGroup: {
oneOf: [['vehicleProduct', 'droidProduct']],
},
droidProduct: {
formGroup: {
oneOf: [['assassinDroid', 'astromechDroid', 'protocolDroid', 'medicalDroid']],
},
droidType: {
required: true,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Component } from '@angular/core';
import { FormControl, Validators } from '@angular/forms';
import { Controls, NgxSubFormRemapComponent, subformComponentProviders } from 'ngx-sub-form';
import { Controls, NgxSubFormRemapComponent, subformComponentProviders, FormGroupOptions } from 'ngx-sub-form';
import {
AssassinDroid,
AstromechDroid,
Expand Down Expand Up @@ -64,4 +64,12 @@ export class DroidProductComponent extends NgxSubFormRemapComponent<OneDroid, On
throw new UnreachableCase(formValue.droidType);
}
}

protected getFormGroupControlOptions(): FormGroupOptions<OneDroidForm> {
return {
validators: [
this.ngxSubFormValidators.oneOf([['assassinDroid', 'astromechDroid', 'protocolDroid', 'medicalDroid']]),
],
};
}
}
7 changes: 7 additions & 0 deletions src/app/main/listing/listing-form/listing-form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
// NGX_SUB_FORM_HANDLE_VALUE_CHANGES_RATE_STRATEGIES,
DataInput,
NgxRootFormComponent,
FormGroupOptions,
} from 'ngx-sub-form';
import { tap } from 'rxjs/operators';
import { ListingType, OneListing } from 'src/app/interfaces/listing.interface';
Expand Down Expand Up @@ -89,4 +90,10 @@ export class ListingFormComponent extends NgxRootFormComponent<OneListing, OneLi
...commonValues,
};
}

protected getFormGroupControlOptions(): FormGroupOptions<OneListingForm> {
return {
validators: [this.ngxSubFormValidators.oneOf([['vehicleProduct', 'droidProduct']])],
};
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Component } from '@angular/core';
import { FormControl, Validators } from '@angular/forms';
import { Controls, NgxSubFormRemapComponent, subformComponentProviders } from 'ngx-sub-form';
import { Controls, NgxSubFormRemapComponent, subformComponentProviders, FormGroupOptions } from 'ngx-sub-form';
import { OneVehicle, Spaceship, Speeder, VehicleType } from 'src/app/interfaces/vehicle.interface';
import { UnreachableCase } from 'src/app/shared/utils';

Expand Down Expand Up @@ -47,4 +47,10 @@ export class VehicleProductComponent extends NgxSubFormRemapComponent<OneVehicle
throw new UnreachableCase(formValue.vehicleType);
}
}

protected getFormGroupControlOptions(): FormGroupOptions<OneVehicleForm> {
return {
validators: [this.ngxSubFormValidators.oneOf([['speeder', 'spaceship']])],
};
}
}
2 changes: 1 addition & 1 deletion src/readme/password-sub-form.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class PasswordSubFormComponent extends NgxSubFormComponent<PasswordForm> {
};
}

public getFormGroupControlOptions(): FormGroupOptions<PasswordForm> {
protected getFormGroupControlOptions(): FormGroupOptions<PasswordForm> {
return {
validators: [
formGroup => {
Expand Down