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

[10.x] Fix breaking changes in before validation #49947

Closed
wants to merge 5 commits into from
Closed

[10.x] Fix breaking changes in before validation #49947

wants to merge 5 commits into from

Conversation

MrPunyapal
Copy link
Contributor

I had a PR #49871 to fix Issue #49863 but from this #49871 (comment) conversation it looks breaking change.

if it is a breaking change then this PR will solve it. added tests for it too.

$v = new Validator($trans, ['y' => '1970-01-01'], ['x' => 'nullable|date', 'y' => 'nullable|date|after:x']);
$this->assertTrue($v->passes());

$v = new Validator($trans, ['y' => '1970-01-01'], ['x' => 'nullable|date', 'y' => 'nullable|date|before:x']);
$this->assertTrue($v->passes());

@MrPunyapal MrPunyapal marked this pull request as draft February 1, 2024 16:46
@MrPunyapal
Copy link
Contributor Author

@calebdw 🤔

Comment on lines 5993 to 5994
$v = new Validator($trans, ['y' => '1970-01-01'], ['x' => 'nullable|date', 'y' => 'nullable|date|before:x']);
$this->assertTrue($v->passes());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe was ever true, so should be removed (or just reverse the condition). Essentially every date is "after" null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it can be after null but cannot be before null? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe was ever true, so should be removed (or just reverse the condition). Essentially every date is "after" null

okay before:x test was failing before adding all of those conditions too 🫡

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But after:x was not? Why did you close?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by mistakenly added with old branch

@MrPunyapal MrPunyapal closed this Feb 1, 2024
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.

Before/After validation rules ignore the validation state of the dependent field
2 participants