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

Make it possible to validate nested array input #33

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

erikwittek
Copy link
Contributor

This PR lets you define validation rules for nested array input:

Field::make('checkboxes')
    ->rules(['checkboxes.*' => 'in:optionA,optionB')

@ksassnowski
Copy link
Collaborator

Thanks for the PR, I didn't even realize that this wasn't currently possible. That being said, this implementation has a couple of issues:

  1. It forces you to specify the name of the field again as soon as you want to define a nested rule. This is redundant and a possible source of bugs (like typos).
  2. With the changes made in this PR, the field name (i.e. the parameter you pass to make) gets completely ignored as soon as you specify the rules as an associative array. This is completely opaque to the user, however, and could lead to hard to track down bugs. Which leads to the next point.
  3. Since the field name gets ignored if the rules are defined as an associative array, that means that as soon as you specify any rule in this way, you have to specify them all like this. Consider the case where you might want to define rules for both the field itself as well as some nested rules.
Field::make('checkbox')
    ->rules([
        'checkbox' => 'required_if:some_other_field',
        'checkbox.*' => 'in:optionA,optionB',
    ]);

Now the field name has to be mentioned three times.


Here's what I would propose instead.

  • The field name should only have to appear once when calling Field::make(...).
  • The rules method of the Field class stays as it is, i.e. it explicitly configures the rule for the field itself, not for any nested fields.
  • We add a separate method nestedRules (I'm open for better name suggestions) that only configures the nested rules for this field. This means that any rule you specify in there is implicitly scoped to {fieldname}.*.
Field::make('checkboxes')
    // resolves to `['checkboxes' => ['required_if:some_other_field']]`
    ->rules(['required_if:some_other_field'])
    // resolves to `['checkboxes.*' => ['in:optionA,optionB']]`
    ->nestedRules(['in:optionA,optionB'])

The nestedRules method could work similar to how your current PR works. If the provided array is not an associated array, we could implicitly treat it as the * rule. But you could also provide an associative array if you need to nest your rules even further like checkboxes.*.id.

Field::make('checkboxes')
    ->nestedRules(['in:optionA,optionB']);
// => ['checkboxes.*' => ['in:optionA,optionB']]

Field::make('options')
    ->nestedRules([
        'id' => 'required',
        'name' => ['string', 'max:255']
    ]);
// => [
//           'options.*.id' => 'required',
//           'options.*.name' => ['string', 'max:255']
//        ]

Notice how the names checkboxes and options are only mentioned once.

@erikwittek
Copy link
Contributor Author

Thank you very much for the Feedback.
We've discussed the solution you proposed already but decided against a separate method, because we preferred to have one single place to set all the validation rules.

Nevertheless, your points are valid, so what do you think about a second optional parameter on the ->rules() method:

function rules($rules, $nested_rules = [])

This would keep everything in one place but at the same time prevent the bugs you mentioned.

Also, I wouldn't set the * selector implicitly, because I'd prefer the visibility of what I have configured.
Additionally there are situations where you need to define rules for the properties of an object, rather than an array of objects:

Field::make('photos')
    ->rules([
        'photos.profile' => 'required|image',
    ]);

(Example from: https://laravel.com/docs/8.x/validation#validating-nested-array-input)

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.

None yet

2 participants