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

The User can able to return validation config conditionally #142

Closed
ajayojha opened this issue Mar 11, 2019 · 12 comments
Closed

The User can able to return validation config conditionally #142

ajayojha opened this issue Mar 11, 2019 · 12 comments

Comments

@ajayojha
Copy link
Member

Description

The user can able to return the validation config conditionally as needed. As discussed with @manudss on gitter. It should work by providing a new property of 'dynamicConfig' which returns the validation config. This will help when some of the calculation is required based upon the cross field value or current control value.

Describe the solution you'd like

The expected solution should be like as below :

export class User {
    @minNumber({
        dynamicConfig: (x, y) => {
            if (x.age > 18)
                return { value: 2500 };
            else
                return { value: 3500 };
        }
    })
    amount: any[];

    @prop()
    age: string;
}

As per the above code in the minNumber validation user can able to return the properties which are available in the NumberConfig interface excepts conditionalExpression.

@ajayojha ajayojha added this to To do in who is doing what? via automation Mar 11, 2019
@ajayojha ajayojha moved this from To do to @ajayojha in who is doing what? Mar 11, 2019
@manudss
Copy link

manudss commented Mar 11, 2019

return the properties which are available in the NumberConfig interface excepts conditionalExpression.

Why excepts conditionalExpression ? Maybe, we could need to change the funtion. If it is not causing some problem, maybe we can keep it.

@manudss
Copy link

manudss commented Mar 11, 2019

dynamicConfig function should have this format

dynamicConfig(currentFormGroup: formGroupValue, rootFormGroup: formGroupValue, C) Partial

@ajayojha
Copy link
Member Author

return the properties which are available in the NumberConfig interface excepts conditionalExpression.

Why excepts conditionalExpression ? Maybe, we could need to change the funtion. If it is not causing some problem, maybe we can keep it.

There are lots of things we have to take care in the framework if we provide a conditionalExpression in the 'dynamicConfig'.

I believe everything can be possible with the conditionalExpression.

Let's release without the conditionalExpression, latter on If we have a solid case for putting the conditionalExpression inside the dynamicConfig, then we will include the same.

Make sense?

@ajayojha
Copy link
Member Author

dynamicConfig function should have this format

dynamicConfig(currentFormGroup: formGroupValue, rootFormGroup: formGroupValue, C) Partial

As current and root is the FormGroup value as per the respective node(parent FormGroup and root FormGroup value).

What's C(the third parameter)?

@ajayojha
Copy link
Member Author

ajayojha commented Mar 12, 2019

@manudss I am planning to release the new version tonight and waiting for your inputs.

@manudss
Copy link

manudss commented Mar 12, 2019

What's C(the third parameter)?

The third parameters should be the current config object. And C was the generic types. As I have shown before.
But yes, my code was missing some information :

dynamicConfig(parent: { [key: string]: any }, root: { [key: string]: any }, currentConfig: C): Partial

@ajayojha
Copy link
Member Author

But why currentConfig is required, as every time it has been changed and as per the current implemented code, I haven't maintained the original state of the config object.

I would like to understand the scenario where this will be helpful.

@manudss
Copy link

manudss commented Mar 12, 2019

But why currentConfig is required, as every time it has been changed and as per the current implemented code, I haven't maintained the original state of the config object.

I would like to understand the scenario where this will be helpful.

ok, if you didn't maintenaned a state of config. It was juste if we would like to have the already defined config.

@ajayojha
Copy link
Member Author

But why currentConfig is required, as every time it has been changed and as per the current implemented code, I haven't maintained the original state of the config object.
I would like to understand the scenario where this will be helpful.

ok, if you didn't maintenaned a state of config. It was juste if we would like to have the already defined config.

Thanks for your inputs.
I am just to sharing my quick thought on the currentConfig. The currentConfig will be helpful in those cases where we need to add/remove some additional values in the existing property of the config. likewise, If I am using the allOf validator then some of the 'matchedValues' are fixed and few will be added/removed based upon passed condition in the dynamicConfig :

this.userFormGroup = this.formBuilder.group({
isRugby:false,
games:['',RxwebValidators.allOf(
{
matchValues:['Cricket','Chess'],
dynamicConfig:(x,y,c)=> {
if(x.isRugby)
c.matchValues.push('Rugby');
else
/// something else...
return c;
}
})]
})

Looks promising of current config. I will try to add this as well in our release.

@manudss
Copy link

manudss commented Mar 12, 2019

yes, it is for some case like this, that currentConfig should be usefull. or to keep the min value exemple. If in dynamicConfig, we would like in some case to add + 10 to the current Min Value. (or compute the default config with number of another fields).

@ajayojha
Copy link
Member Author

@manudss I have transformed the earlier provided compose validator code with the minNumber validator through 'dynamicConfig'. Please refer the stackblitz example.

Feel free to share your comments for the same.

@manudss
Copy link

manudss commented Mar 14, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants