-
Notifications
You must be signed in to change notification settings - Fork 371
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
[0.2] Allow extending of validation rules #443
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@glennjacobs need your input on this this is referring to this https://discord.com/channels/497326121441034240/916819091841486869/1001891823146520646 |
@alecritson thoughts on this? |
@glennjacobs I'm not against this but don't feel like the config is the correct place. Due to some validation using closures (like in the example above) this means that Laravel wouldn't be able to serialise the config for optimisation. Would it not be better to provide a method on the component that dev's can tap into at the component level? Something like: ProductEdit::extendValidation([
'name' => 'required|min:50',
]); |
Fair point. Will look into this approach |
now can extend validation by using component method ProductCreate::extendValidation(
[
'variant.sku' => ['required', 'min:8'],
'collections' => ['required', 'array', function ($product) {
return function ($attribute, $value, $fail) use ($product) {
$missing = [];
$collections = collect($value)->pluck('id', 'group_id');
$groups = CollectionGroup::whereIn('id', [1, 2])
->select(['id', 'name'])
->get()
->pluck('name', 'id');
$missing = $groups->diffKeys($collections)->all();
if (filled($missing)) $fail($product->translateAttribute('name') . " Require Collection Group from: " . implode(', ', $missing));
};
}],
]
); $product is able to pass from component by using |
@wychoong It's looking promising, I can see it's still in draft, is there still work you want to do on it? |
hey. I think in terms of implementation it is done. I kept it as draft is to get feedbacks and since this opens up a lot more validation, we also need to add few more changes to display the error message.
|
@wychoong should we have documentation for this? |
added doc |
hrmmm... you referring to this? not sure why its pending or should i click the resolve converstation? |
|
||
return collect($rules)->map(function ($rule) use ($parameters) { | ||
if ($rule instanceof Closure) { | ||
return app()->call($rule, $parameters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this idea is taken from Filament on allowing injecting value for closure
figured it out that i need to submit my "review" comment 😓 |
@wychoong the VuePress config file has conflicts now. You have changed the indentation from 2 spaces to 4 spaces, any reason for this? |
I think that was because of my VSCode does that, can you help on this as I’m away from computer till the weekend |
@glennjacobs it's ready for review |
this is a proof of concept on customising validation rules in hub. Current only applied for Product's collection.
now you can define more rules in config
result
issue
the main problem with the current implementation is unable pass in Model instance to those additional rules. it can be done but I don't think is good approach.