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

Add support for form async validators + unique custom validator #15

Merged
merged 7 commits into from
May 30, 2019

Conversation

stissot
Copy link
Member

@stissot stissot commented May 24, 2019

Async Form Validators are useful to validate a form field value using a backend API.

This adds getFormAsyncValidators() similar to getFormValidators(), called by getFormConfig().

Is also adds the generic uniqueValidator() method returning an AsyncValidatorFn that can be used to check any model and field for existing values.

Usage in model (e.g Product.code):

    public getFormAsyncValidators(): FormAsyncValidators {
        return {
            code: [this.uniqueValidator('code')],
        };
    }

@stissot stissot requested a review from sambaptista May 24, 2019 10:47
@sambaptista
Copy link
Member

sambaptista commented May 24, 2019

Validators are relative to forms, not to model or services and should be standalone. They should be in their specific class instead of NaturalAbstractModelService.

Can you refactor into an NaturalValidators.unique(args). The count function may be out of the class context but in the same file without export mention.

NaturalValidators.ts

// Not exported variable, not in class neither. It's like a private function for current file.
const count = (queryVariablesManager, name) => {
   ...
}

export class NaturalValidators {

    public static unique(fieldName: string, service: AbstractModelService<any, any, ...> , apollo: Apollo, name: string)   {
       ...
       count(...);
        ...
    }
}

Usage would be more standard

public getFormAsyncValidators(): FormAsyncValidators {
        return {
            code: [NaturalValidators.unique('code', this.userService, this.apollo, 'user')],
            numberField : [Validators.min(10)] // angular native validator usage for comparison
        };
}

disabled: disabled,
};

config[key] = new NaturalFormControl(formState, null, asyncValidators[key]);
Copy link
Member

@sambaptista sambaptista May 24, 2019

Choose a reason for hiding this comment

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

I'm unsure about that "null". Don't change that for now, I'll test it by myself, and implement tests.

@sambaptista
Copy link
Member

sambaptista commented May 24, 2019

The rest seems ok, good job.

When the refactor is done, we can merge. We have to keep in mind that a debounce is still to do (in another commit/PR)

filter: {groups: [{conditions: [condition]}]},
};
qvm.set('variables', variables);
this.count(qvm).pipe(
Copy link
Member

Choose a reason for hiding this comment

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

Shouldnt this be returned ?

@stissot
Copy link
Member Author

stissot commented May 28, 2019

Thanks for the feedback. I've relocated the validator in its own class NaturalValidators.unique(), but left the public count() method in NaturalAbstractModel, because it needs to access properties (name, apollo) from the actual model to generate the query, and it can also be used to count objects matching the QueryVariables for other purposes than the validator. Also added some tests.

@stissot stissot requested a review from PowerKiKi May 29, 2019 08:24
@stissot stissot merged commit 4b8c021 into Ecodev:master May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants