spatie / laravel-data

Powerful data objects for Laravel
https://spatie.be/docs/laravel-data/
MIT License
1.28k stars 209 forks source link

ExcludeIf not working #873

Closed scippio closed 3 weeks ago

scippio commented 1 month ago

✏️ Describe the bug I'm using ExcludeIf in attributes (see example below). When I have $type = 'user' I still have company_id as required...

↪️ To Reproduce Provide us a pest test like this one which shows the problem:

    class ChildData extends Data
    {
        public function __construct(
            public string $type,
            #[ExcludeIf('type','company')]
            public string $user_id,
            #[ExcludeIf('type','user')]
            public string $company_id,
        ) {
        }
    }
"exception": {
    "message": "The selected company id is required.",
    "errors": {
        "company_id": [
            "The selected company id is required."
        ],
    },

✅ Expected behavior The $company_id should not be required

🖥️ Versions

Laravel: 11.22.0 Laravel Data: 4.8.2 PHP: 8.3.6

rubenvanassche commented 3 weeks ago

I don't see how we could quickly solve this in laravel-data:

$v = Validator::make([
    'type' => 'user',
    'user_id' => '1',
], [
    'type' => ['required', 'string'],
    'user_id' => ['exclude_if:type,company', 'string', 'required'],
    'company_id' => ['exclude_if:type,user', 'string','required'],
]);

$v->validate(); // Passes

$v = Validator::make([
    'type' => 'user',
    'user_id' => '1',
], [
    'type' => ['required', 'string'],
    'user_id' => ['required', 'exclude_if:type,company', 'string',],
    'company_id' => ['required', 'exclude_if:type,user', 'string',],
]);

$v->validate(); // Fails due to company id being required

So depending on if required is before or after exclude_if the validator fails or won't. Laravel data first always adds required/nullable rules, then type rules and then attribute rules which causes this bug.

The easiest solution would be to write a custom rules method and overwrite the rule.

Also small thing: Exclude isn't going to exclude in laravel data since we don't use the validated payload, something we're going to fix in v5 but this is not the case right now. It might be better to use RequiredWith rules which do work as expected.