Skip to content

Conversation

NickSdot
Copy link
Contributor

@NickSdot NickSdot commented Aug 15, 2021

I consider this feature as small but helpful 'quality of life' improvement with no backwards compatibility issues.

Problem

It is not possible to use $validator->sometimes() with nested arrays inwithValidator($validator) of a FormRequest class.

Given an array/request like this (or even more nested):

public function rules(): array
    {
        return [
            "channels"    => "required|array",
            "channels.*.type"  => "required|string|min:3",
            "channels.*.value" => "required|string|distinct|min:3",
            "channels.*.description"  => "nullable|string|min:3|max:80",
        ];
    }

The value attribute can be an url or an email, depending on the value of an other field. $validator->sometimes() is not aware of the current item/index. Dot notation with an asterix is also not working. So it seems there is no flexible way to to apply specifc rules for nested arrays, to archieve this:

$validator->sometimes('channels.*.value', 'email:rfc,dns', function($item) {
  return $item->type === 'email';
});

Solution

I assume it does not make sense to touch the existing sometimes() method. Hence, I would propose to add the following method to Illuminate\Validation\Validator:

    /**
     * Add conditions to a given nested field based on a Closure.
     *
     * @param  string  $parent
     * @param  string|array  $attribute
     * @param  string|array  $rules
     * @param  callable  $callback
     * @return $this
     */
    public function sometimesNested(string $parent, $attribute, $rules, callable $callback)
    {
        foreach ((array) $this->getValue($parent) as $index => $item) {

            $payload = new Fluent($item);

            if ($callback($payload)) {
                foreach ((array) $attribute as $key) {
                    $this->addRules([$parent . '.' . $index . '.' . $key => $rules]);
                }
            }
        }

        return $this;
    }

How it works: Basically the functionality is pretty much the same as in $validator->sometimes(). The main difference is, that we pass a 'parent' key name as a first parameter to the method, get the values and loop trough it. If the callback returns true we add the rules based on the parent/index/key combination.

Usage

Developers would have a convinient way to access and flexible validate nested items in a FormRequest class like this:

    public function withValidator(Validator $validator)
    {
        // validate field 'value' as email, if field 'type is email
        $validator->sometimesNested('channels', 'value', 'email:rfc,dns', function ($item) {
            return $item->type === 'email';
        });

        // validate field 'value' as url, if field 'type' IS NOT email
        $validator->sometimesNested('channels', 'value', 'url', function ($item) {
            return $item->type !== 'email';
        });
    }

@NickSdot
Copy link
Contributor Author

Checks here on Github fail with:

PHP Warning:  Declaration of Illuminate\Tests\Database\ProcessorTestPDOStub::lastInsertId(?string $sequence = NULL) should be compatible with PDO::lastInsertId($seqname = NULL) in /home/runner/work/framework/framework/tests/Database/DatabaseProcessorTest.php on line 46
PHP Parse error:  syntax error, unexpected '|', expecting variable (T_VARIABLE) in /home/runner/work/framework/framework/tests/Support/SupportReflectsClosuresTest.php on line 77
Error: Process completed with exit code 255.

Tbh, not sure why. Input welcome.

@chu121su12
Copy link
Contributor

There are union type support merged recently. That maybe the case as for the error coming from pipe character.

@NickSdot
Copy link
Contributor Author

NickSdot commented Aug 15, 2021

There are union type support merged recently. That maybe the case as for the error coming from pipe character.

Think you are right. Seems checks started to fail globally with #38383.

@driesvints
Copy link
Member

Hi, I merged #38398 into 8.x to get the test suite passing again. I've put your PR in draft. Please rebase your PR against 8.x to make the tests of this PR pass. After you've done that and tests pass, mark this PR as ready for review. Thanks.

@driesvints driesvints marked this pull request as draft August 16, 2021 10:28
@NickSdot NickSdot marked this pull request as ready for review August 16, 2021 12:05
@NickSdot
Copy link
Contributor Author

Done.

@taylorotwell
Copy link
Member

I don't think I would want an entirely different method here.

@NickSdot
Copy link
Contributor Author

@taylorotwell would it be an option to add an additional parameter to the end of the current sometimes() method (after the closure)?

Otherwise I don't see a way to get this done in the same method without breaking change.

Bit more feedback would be much appreciated.

@vpratfr
Copy link
Contributor

vpratfr commented Aug 17, 2021

I had the same issue.

Needed a lot of custom code to work around the * validation limitations.

For validation messages, this got closed without further thinking: #36587

For validation rules, basically I make a loop on the items to add a rule for each of them.

foreach ($items as $key => $value) {
    $validator->sometimes("items.$key.some_field", 'required', fn() => $items[$key]['other_field'] === 'email);
}

Validation error message translation/matching is handled via a custom subclass using the idea in the PR mentionned above (because basically when it fails, you get an error for items.1.some_field instead of items.*.some_field and that does not get matched in the custom validation attributes array).

Maybe in a couple of years some addition will be made to the framework when enough people report the same difficulty to work with items in validation rules/messages.

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.

5 participants