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

Conflict Between Optional and Required Rules #844

Closed
clementbirkle opened this issue Aug 16, 2024 · 4 comments
Closed

Conflict Between Optional and Required Rules #844

clementbirkle opened this issue Aug 16, 2024 · 4 comments

Comments

@clementbirkle
Copy link
Contributor

clementbirkle commented Aug 16, 2024

✏️ Describe the bug
Optional should be skipped when a Required rule is applied (e.g., RequireWith, RequiredIf).

↪️ To Reproduce

it('this test should create an error during validation', function () {

    class UpdateProductData extends Data
    {
        public function __construct(
            #[RequiredWith('discountValue')]
            public DiscountType $discountType = new Optional,
            #[RequiredWith('discountType')]
            public float $discountValue = new Optional,
        ) {
        }
    }

    dd(UpdateProductData::validateAndCreate(['discountValue' => 20));
});

✅ Expected behavior
In this test, I expect an error message like 'The discountType is required when discountValue is present.', because discountValue is provided but discountType is not.

The issue arises due to two points:

  1. If a value is not provided for an optional type, the rule will be removed. This should not happen if the attribute has rules like RequiredWith, RequiredIf, etc.
  2. The SometimesRuleInferrer should not add the sometimes rule when the attribute has rules like RequiredWith, RequiredIf, etc.

🖥️ Versions

Laravel: 11
Laravel Data: 4.8.1
PHP: 8.2

I can try to make a PR if you think this is an issue.

@rubenvanassche
Copy link
Member

This should be fixed with the release later today

@clementbirkle
Copy link
Contributor Author

Thank you, @rubenvanassche, but this doesn't fully solve the problem for me.

For example:

it('this test should not create an error during validation', function () {

    class UpdateProductData extends Data
    {
        public function __construct(
            #[RequiredWith('discountValue')]
            public DiscountType $discountType = new Optional,
            #[RequiredWith('discountType')]
            public float $discountValue = new Optional,
        ) {
        }
    }

    dd(UpdateProductData::validateAndCreate([]));
});

This code currently throws an error, but it shouldn't, because neither discountType nor discountValue are provided. In this case, the Sometime rule shouldn't be removed.

@rubenvanassche
Copy link
Member

I cannot reproduce that, you know you're missing the optional type in the properties?

@clementbirkle
Copy link
Contributor Author

Sorry @rubenvanassche, my example was wrong!

Here is a correct example:

it('this test should not create an error during validation', function () {

    class UpdateProductData extends Data
    {
        public function __construct(
            public string|Optional $type = new Optional,
            #[RequiredUnless('type', 'bundle')]
            public float|null|Optional $price = new Optional,
        ) {
        }
    }

    UpdateProductData::validateAndCreate([]);
});

This code currently throws an error, but it shouldn't, because neither type nor price are provided. In this case, the Sometime rule shouldn't be removed.

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

No branches or pull requests

2 participants