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

[8.x] Add unfilled_if & unfilled_with validation rules to Validator #34425

Closed

Conversation

mallardduck
Copy link
Contributor

@mallardduck mallardduck commented Sep 20, 2020

New Rules

  • unfilled_if: A rule to refuse a field's input when another field is a given value. Complements: required_if & filled.
  • unfilled_with: A to refuse a fields's input when other fields are passed. Complements: required_with & filled.

Adding these rules can help add more utility to validation to disallow field input when other specific field values, or fields, are passed.

Example

Rule usages:

    /**
     * Get the validation rules that apply to the request.
     *
     * @return array
     */
    public function rules()
    {
        return [
            'shape' => 'sometimes',
            'size' => 'nullable|integer',
            'width' => [
                'unfilled_with:size|unfilled_if:shape,square',
            ],
            'height' => [
                'unfilled_with:size|unfilled_if:shape,square',
            ],
        ];
    }

Note: This is a super simplified example, instead of nullable you'd probably want to use required_without_all:width,height or something similar IRL depending on use-case.

URL

Examples Request URL: http://myapp.test/avatar?name=Danno+Boone&size=96&width=64

Error

{
  "width": [
    "The width field must not be used when size field is passed."
  ]
}

PR also adds tests to cover new validation rules, and I have a translation string prepared to send to the main laravel repo.

    'unfilled_if' => 'The :attribute field must not be used when :other field is :value.',
    'unfilled_with' => 'The :attribute field must not be used when :values is present.',

@mallardduck mallardduck force-pushed the unfilledIf-validation-rule branch from 72c22eb to 86babcf Compare September 20, 2020 17:41
@mallardduck
Copy link
Contributor Author

mallardduck commented Sep 20, 2020

@GrahamCampbell - Here's a PR targeting 8.x, i'll shoot the one for laravel/laravel over too.

On second thought, I'm considering if I should implement an "Unfilled*" rule a bit differently to make it more flexible and keep the feature similar to existing rules.

Specifically I realized this type of rule seems more similar to the required_with type rule, since the value set itself doesn't matter, just that the field was used. In contrast it seems the *_if rules intend to trigger based on a field having a specific value.

Make the unfilled_if match required_if, and implement unfilled_with method similar to original goal.
@mallardduck mallardduck changed the title [8.x] Add an UnfilledIf validation rule to Validator [8.x] Add an unfilled_if & unfilled_with validation rules to Validator Sep 20, 2020
@mallardduck mallardduck changed the title [8.x] Add an unfilled_if & unfilled_with validation rules to Validator [8.x] Add unfilled_if & unfilled_with validation rules to Validator Sep 20, 2020
@taylorotwell
Copy link
Member

I'm not sure this is really necessary. If you know you want to ignore a given field's input on a given condition, just ignore it. No need to validate it.

@mallardduck
Copy link
Contributor Author

mallardduck commented Sep 21, 2020

@taylorotwell - Quick question, am I wrong for thinking of the Validator feature as an "HTTP Request" validator?

Your reply seems to indicate that I should think of the validator as a "Field Validator" and simply process the fields myself. However I always thought the documentation read as though this was an HTTP Request/Query validator. Given that the docs say: "validate incoming HTTP requests" I would assume my original interpretation is correct.

So while "just ignore it" is one option, I would argue that being able to reject a request for including incompatible flags is a valid use case. Thoughts?

Also, after a quick look, it seems the gits of this feature would be similar to Symfony's blank: https://symfony.com/doc/current/reference/constraints/Blank.html - just much more versitile.

@mallardduck mallardduck deleted the unfilledIf-validation-rule branch September 22, 2020 19:21
@jessarcher
Copy link
Member

I've wanted this on a few occasions as well and have previously extended the validator with a prohibited_if rule (seems like a good antonym for required_if).

For example, imagine a conditional field date_of_marriage that only displays when is_married is true. The payload is invalid if date_of_marriage has been provided when is_married is not true.

The existing exclude_if rule goes a long way to help with the above, but for my use case I need to completely reject a payload if it's contains contradictory information, rather than accept it and silently discard the contradicting data.

I don't mind implementing this as a custom rule, however it gets a bit nasty because it must re-implement the logic from the following protected Validator methods:

  • prepareValuesAndOther()
  • convertValuesToBoolean()
  • convertValuesToNull()

@mallardduck
Copy link
Contributor Author

Hey @jessarcher - assuming the logic of prohibited_if -great name BTW- would be essentially the same as my suggestion for unfilled_if then my extended validator package should help add that. However I'm totally going to take your better naming and update the rule and default validation error text for the rule accordingly.

Agreed on it being pretty nasty - in all honestly I just directly duplicated those methods in my package to get it working. One of many reasons it'd be preferable to have the rule as a stock part of laravel's validator.

Again, i'm going to update the rule name since I like your idea so much better, but the current itteration of the package is here: https://github.com/mallardduck/extended-validator-laravel

@jessarcher
Copy link
Member

jessarcher commented Mar 8, 2021

Hey @mallardduck, thanks for the package tip! Glad to help with the naming too 😁

I wonder whether a PR to make prepareValuesAndOther() public would be accepted, similar #35183. The other methods I mentioned wouldn't need to be public (at least for our purposes).

Edit: PR here

@mallardduck
Copy link
Contributor Author

Great work @jessarcher - once a tagged release of laravel/framework and illuminate/validation are shipped that include the now public and renamed parseDependentRuleParameters I'll update my validator package to use that instead of the copypasta code.

For now there's a new 2.0.0 tag of my package with the improved rule names, so once the feature is released i'll increment from the 2.X tag.

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.

3 participants