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.

Sign up for GitHub

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

Jump to bottom

[5.3] Fix date_format validation for all formats #16692

Merged
merged 2 commits into from
Dec 7, 2016
Merged

[5.3] Fix date_format validation for all formats #16692

merged 2 commits into from
Dec 7, 2016

Conversation

rohitsubedi
Copy link
Contributor

@rohitsubedi rohitsubedi commented Dec 7, 2016 •
edited
Loading

Fix date_format validation for all format

This PR fixes the case when validation rule is date_format:Y-m-d, It passes the format 16-01-01. But the year should be digit 4.

@samundra
Copy link

samundra commented Dec 7, 2016

👍 Currently as it looks like the validateDateFormat only checks for the valid date entry and doesn't strictly compares the date format. This PR fixes that issue.

@ankitpokhrel
Copy link
Contributor

👍

@rohitsubedi rohitsubedi changed the title Fix date_format validation for all formats [5.3] Fix date_format validation for all formats Dec 7, 2016
@themsaid
Copy link
Member

themsaid commented Dec 7, 2016

Can you guys share a validation that passes while it should fail?

@samundra
Copy link

samundra commented Dec 7, 2016

@themsaid

validation rule date_format:Y-m-d

for input param: 16-12-10
expected output: validation should fail
actual output: validation passed

@taylorotwell taylorotwell merged commit 4c70ab0 into laravel:5.3 Dec 7, 2016
@taylorotwell
Copy link
Member

Thanks

@gkunwar
Copy link

gkunwar commented Dec 8, 2016

Great! @rohitsubedi

@oriceon oriceon mentioned this pull request Dec 16, 2016
Closed
@mfn mfn mentioned this pull request Dec 17, 2016
Closed
mfn added a commit to mfn/laravel-framework that referenced this pull request Dec 17, 2016
@mfn
Revert "Merge pull request laravel#16692 from rohitsubedi/rohitsubedi…
9a2117a 
…-patch-1"

This reverts commit 4c70ab0, reversing
changes made to e91f04b.
@mfn mfn mentioned this pull request Dec 17, 2016
Merged
taylorotwell pushed a commit that referenced this pull request Dec 17, 2016
@mfn @taylorotwell
[5.3] Revert #16692 and make date_format work with ISO8601 again (#16845
359b7c3 
)

* Revert "Merge pull request #16692 from rohitsubedi/rohitsubedi-patch-1"

This reverts commit 4c70ab0, reversing
changes made to e91f04b.

* validation: add a few more date_format tests

Showing off how Y-m-d\TH:i:sP matches multple valid ISO 8601 date formats.

* validation: add test showing 00-00-01 passes for Y-m-d

It's unexpected for Y to match two-digit dates
@themsaid themsaid mentioned this pull request Dec 19, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rohitsubedi @samundra @ankitpokhrel @themsaid @taylorotwell @gkunwar