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.5] Fixed date comparison validators failing when a format is specified #20940

Merged
merged 4 commits into from
Sep 5, 2017

Conversation

pantherdd
Copy link
Contributor

[ This is a follow-up to the closed PRs #20789 and #20868. ]

Fixed Validator failing on 'before_or_equal:today' when the input is today's date and a format is specified

The issue was that the time part was bleeding through from the current timestamp in DateTime::createFromFormat.

Test case:

  • Validation rule: 'birth_date' => ['required', 'date_format:Y-m-d', 'before_or_equal:today']
  • '2017-08-27' was parsed as '2017-08-27 22:41:37'.
  • 'today' was parsed as '2017-08-27 00:00:00'.
  • Validation failed.

Some further information I found (thanks to @arjasco 's comment on the previous PR):


@taylorotwell "Isn't this going to break applications? What if people want the time part?"

If you have time fields in the date format, the validation will still use them.
http://php.net/manual/en/datetime.createfromformat.php

If format contains the character !, then portions of the generated time not provided in format, as well as values to the left-hand side of the !, will be set to corresponding values from the Unix epoch.

>>> DateTime::createFromFormat('!Y-m-d H:i:s', '2017-08-28 18:57:24')
=> DateTime {#890
     +"date": "2017-08-28 18:57:24.000000",
     +"timezone_type": 3,
     +"timezone": "Europe/Paris",
   }

I have also added a number of date validation tests in a second commit. These tests deal with the time parts, and show that the fix I made is not breaking anything.

If you still think this change might cause issues, please provide a test case that this PR causes to fail, so that I have a chance of fixing the problem.


[ When porting my fix to branch 5.5, I added extra test cases for the new date_equals validator to have that covered too. This exercise led me to a bug in its implementation, which I also fixed. ]

Fixed Validator failing on 'date_equals' when a format is specified

Also added a number of test cases.
The issue was that === always returns false for the two separate DateTime instances returned by getDateTimeWithOptionalFormat.


The fix can easily be applied to branch 5.4 too, so that users of the previous version get the benefits too. It doesn't have to be backported, as I originally wrote the fix for that branch. Here is the PR, it just needs to be merged. (It contains the first 2 commits of this PR.)

…today's date and a format is specified

The issue was that the time part was bleeding through from the current timestamp in `DateTime::createFromFormat`.
Test case:
* Validation rule: 'birth_date' => ['required', 'date_format:Y-m-d', 'before_or_equal:today']
* '2017-08-27' was parsed as '2017-08-27 22:41:37'.
* 'today' was parsed as '2017-08-27 00:00:00'.
* Validation failed.
This shows that the fix in 1b651e6 is not going to break applications, people can still use the time part.
This should address the comment of @taylorotwell at laravel#20789 .
Also added a number of test cases.
The issue was that `===` always returns false for the two separate DateTime instances returned by `getDateTimeWithOptionalFormat`.
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.

2 participants