Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Validator] Support \DateInterval in comparison constraints#33401

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@fancyweb
Copy link
Contributor

@fancywebfancyweb commentedAug 30, 2019•
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PRTODO

That feature allows to use theRange and all comparisons constraints on\DateInterval values.

/** * @GreaterThan("+30 seconds") */private$foo;/** * @Range(min="6 months", max="10 years") */private$bar;

TODO :

  • Update CHANGELOG
  • TestConstraintValidator::formatValue() withPRETTY_DATE_INTERVAL
  • TestComparisonValidatorDateIntervalHelper
  • CheckDivisibleBy - this constraint is a comparison and there is no test with dates, so I guess a proper exception needs to be thrown. - see[Validator] Only handle numeric values in DivisibleBy #33435

Edit : waiting for other related PRs to be treated before resynchronizing everything on this one.

seb-jean and maxhelias reacted with thumbs up emoji
@fancywebfancywebforce-pushed thevalidator-compare-date-interval branch from9f86290 to2554641CompareAugust 30, 2019 16:27
@ycerutoyceruto added this to thenext milestoneAug 30, 2019
@fancywebfancyweb changed the title[Validator] Support \DateInterval in comparison constraints[WIP][Validator] Support \DateInterval in comparison constraintsSep 3, 2019
fabpot added a commit that referenced this pull requestSep 3, 2019
… (fancyweb)This PR was squashed before being merged into the 3.4 branch (closes#33434).Discussion----------[Validator] Add ConstraintValidator::formatValue() tests| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -So#33401 tests can be built on top of this.Commits-------b688aa3 [Validator] Add ConstraintValidator::formatValue() tests
symfony-splitter pushed a commit to symfony/validator that referenced this pull requestSep 3, 2019
… (fancyweb)This PR was squashed before being merged into the 3.4 branch (closes #33434).Discussion----------[Validator] Add ConstraintValidator::formatValue() tests| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -Sosymfony/symfony#33401 tests can be built on top of this.Commits-------b688aa31ec [Validator] Add ConstraintValidator::formatValue() tests
@fancywebfancywebforce-pushed thevalidator-compare-date-interval branch 2 times, most recently fromaf34089 to9a19842CompareSeptember 4, 2019 14:53
@fancywebfancyweb changed the title[WIP][Validator] Support \DateInterval in comparison constraints[Validator] Support \DateInterval in comparison constraintsSep 4, 2019
@fancywebfancywebforce-pushed thevalidator-compare-date-interval branch from9a19842 to1b0654aCompareSeptember 10, 2019 13:26
@fancywebfancywebforce-pushed thevalidator-compare-date-interval branch from1b0654a to0c7342dCompareSeptember 27, 2019 09:31
@fancywebfancywebforce-pushed thevalidator-compare-date-interval branch from0c7342d to1664890CompareSeptember 27, 2019 14:59
@xabbuh
Copy link
Member

I find the PR description a bit confusing. Is the goal of this PR to support specifying intervals as an input of the constraints or do you really want to be able to have your properties being intervals? From reading I understand the latter, but I don't really see a use case for that.

@fancyweb
Copy link
ContributorAuthor

The latter. A class property holds a\DateInterval instance and I want to validate that it is between 1 and 10 min for example. Basically, it's the same feature we did for dates but for intervals.\DateInterval is in PHP core, let's support it!

'h' =>'hour',
'i' =>'minute',
's' =>'second',
'f' =>'microsecond',
Copy link
Member

Choose a reason for hiding this comment

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

this formatting logic is English-only, which looks bad.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

As we did for dates just above, the format is hardcoded here except in this case there are words.

@fancywebfancyweb changed the base branch from4.4 tomasterFebruary 3, 2020 17:34
@fancyweb
Copy link
ContributorAuthor

I just rebased this one on master.

@fancyweb
Copy link
ContributorAuthor

I must admit there is a problem on this one with the hardcoded English formatting that does not follow any standard (contrary to the hardcoded RFC-3339 date format). How could we deal with this?

seb-jean reacted with thumbs up emoji

@fancywebfancywebforce-pushed thevalidator-compare-date-interval branch from3c5df46 tof4b9bf6CompareFebruary 10, 2020 09:37
@seb-jean
Copy link
Contributor

This feature would be very interesting :)

@fancyweb
Copy link
ContributorAuthor

@fabpot I am closing this one as it cannot be merged with a fixed format. I am still working on the format customisation sometimes but it is not ready yet. I will reopen a new PR once everything is ready.

xabbuh reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

@fabpotfabpotfabpot requested changes

@dunglasdunglasAwaiting requested review from dunglas

@lyrixxlyrixxAwaiting requested review from lyrixx

@srozesrozeAwaiting requested review from sroze

@xabbuhxabbuhAwaiting requested review from xabbuh

+2 more reviewers

@a-menshchikova-menshchikova-menshchikov left review comments

@ktheragektheragektherage left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

10 participants

@fancyweb@xabbuh@seb-jean@fabpot@nicolas-grekas@stof@a-menshchikov@ktherage@yceruto@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp