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

[5.4] Allow adding dependent extensions to the Vallidator #18654

Merged
merged 2 commits into from
Apr 5, 2017
Merged

[5.4] Allow adding dependent extensions to the Vallidator #18654

merged 2 commits into from
Apr 5, 2017

Conversation

themsaid
Copy link
Member

@themsaid themsaid commented Apr 4, 2017

This PR contains the changes introduced in #18647

Tried to push my changes to the fork based on GitHub guide (https://github.com/blog/2247-improving-collaboration-with-forks) but for some reason all commits on the fork was deleted and the PR got closed. So sorry @rakesh-beedasy :/

@taylorotwell
Copy link
Member

Can you try to explain what this does and why it is needed? I could not understand the original PR.

@taylorotwell taylorotwell reopened this Apr 4, 2017
@themsaid
Copy link
Member Author

themsaid commented Apr 5, 2017

@taylorotwell here's an example:

Validator::extendDependent('contains', function ($attribute, $value, $parameters, $validator) {
   // The $parameters passed from the validator below is ['*.provider'], when we imply that this
   // custom rule is dependent the validator tends to replace the asterisks with the current
   // indices as per the original attribute we're validating, so *.provider will be replaced
   // with 0.provider, now we can use array_get() to get the value of the other field.

    // So this custom rule validates that the attribute value contains the value of the other given
    // attribute.
    return str_contains($value, 
            array_get($validator->getData(), $parameters[0])
    );
});


Validator::make(
    [['email' => '[email protected]', 'provider' => 'mail.com']],
    ['*.email' => 'contains:*.provider']
)->validate();

@rakesh-beedasy
Copy link
Contributor

namespace Illuminate\Validation;

class Validator implements ValidatorContract
{
    ....
    protected $dependentRules = [
        'RequiredWith', 'RequiredWithAll', 'RequiredWithout', 'RequiredWithoutAll',
        'RequiredIf', 'RequiredUnless', 'Confirmed', 'Same', 'Different', 'Unique',
        'Before', 'After', 'BeforeOrEqual', 'AfterOrEqual',
    ];
    ....
    /**
     * Validate a given attribute against a rule.
     *
     * @param  string  $attribute
     * @param  string  $rule
     * @return void
     */
    protected function validateAttribute($attribute, $rule)
    {
        ....
        // First we will get the correct keys for the given attribute in case the field is nested in
        // an array. Then we determine if the given rule accepts other field names as parameters.
        // If so, we will replace any asterisks found in the parameters with the correct keys.
        if (($keys = $this->getExplicitKeys($attribute)) &&
            $this->dependsOnOtherFields($rule)) {
            $parameters = $this->replaceAsterisksInParameters($parameters, $keys);
        }
        ....
    /**
     * Determine if the given rule depends on other fields.
     *
     * @param  string  $rule
     * @return bool
     */
    protected function dependsOnOtherFields($rule)
    {
        return in_array($rule, $this->dependentRules);
    }
....
}

Since $this->dependentRules is readonly $this->dependsOnOtherFields($rule) always returns false for any custom rule which contains a dependent field, hence $this->replaceAsterisksInParameters() never gets executed for custom rules.

When using "required_if" asterisk gets replaced fine since RequiredIf is present in readonly Illuminate\Validation\Validator::$dependentRules

//in some controller
$rules = ['items.*.field' => 'required_if:items.*.otherfield,value']

namespace Illuminate\Validation\Concerns;
trait ValidatesAttributes
{
    protected function validateRequiredIf($attribute, $value, $parameters)
    {
        //var_dump($attribute, $parameters[0])
        //$attribute as item.0.field, and $parameters[0] as item.0.otherfield  
        //$attribute as item.1.field, and $parameters[0] as item.0.otherfield 
        //hence item.0.field is required if item.0.otherfield and so on

When using a custom "required_if_in" which is a custom rule and array validation rules

//in some controller
$rules = ['items.*.field' => 'required_if_in:items.*.otherfield,value1,value2']
function ($attribute, $value, $parameters) {
    //var_dump($attribute, $parameters[0]);
    //$attribute as item.0.field, but $parameters[0] as item.*.otherfield  
    //$attribute as item.1.field, but $parameters[0] as item.*.otherfield 
    //item.0.field is required if in item.*.otherfield which is in fact an array of otherfield from an array of items

The idea would be as we can define custom implicit rules, it would be good if we could define custom dependent rules.

@Propaganistas
Copy link
Contributor

Great to see this has made it to core.

@rawaludin
Copy link

This is useful. Just found it today. Could this added to the docs?

@chuck-wood
Copy link

Let me just say that I love this addition. It really simplified my validation. Thanks guys!

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.

6 participants