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] Only handle numeric values in DivisibleBy#33435

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

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

Currently it probably breaks becauseabs throws a notice on objects.

@fancywebfancywebforce-pushed thevalidator-divisible-by-non-numeric-values branch 2 times, most recently from6afa33e to13e8b44CompareSeptember 3, 2019 07:41
@fancyweb
Copy link
ContributorAuthor

Now the only problem with it is that the values can be transformed in theAbstractComparisonValidator class, meaning that the message can be incoherent with what is actually passed by the developer to the constraint. I wonder if we should not add aprotected function supportsValue($value): bool in the abstract class to throw from here with the real value?

@ro0NL
Copy link
Contributor

@fancyweb not sure i follow, the passed message still applies to numeric values. We get a different message/violation for type errors, like any other constraint does.

@fancyweb
Copy link
ContributorAuthor

What I mean is that if my constraint is@DivisibleBy("+10 days") on a\DateTime value (totally not logical but still possible), the error message will beExpected argument of type "numeric", "\DateTime" given while I never provided a\DateTime instance in the first place. It was converted in theAbstractComparisonValidator class. The goal would be to have a better message with the initial passed value. Might not worth it though.

@ro0NL
Copy link
Contributor

i see, IMHO that's a side effect we can live with yes.

@fancywebfancywebforce-pushed thevalidator-divisible-by-non-numeric-values branch from13e8b44 tof974addCompareSeptember 3, 2019 09:18
@fabpot
Copy link
Member

Thank you@fancyweb.

fabpot added a commit that referenced this pull requestSep 3, 2019
…cyweb)This PR was merged into the 4.3 branch.Discussion----------[Validator] Only handle numeric values in DivisibleBy| Q             | A| ------------- | ---| Branch?       | 4.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -Currently it probably breaks because `abs` throws a notice on objects.Commits-------f974add [Validator] Only handle numeric values in DivisibleBy
@fabpotfabpot merged commitf974add intosymfony:4.3Sep 3, 2019
@fancywebfancyweb deleted the validator-divisible-by-non-numeric-values branchSeptember 3, 2019 17:35
@fabpotfabpot mentioned this pull requestOct 7, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoyceruto approved these changes

+1 more reviewer

@ro0NLro0NLro0NL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

5 participants

@fancyweb@ro0NL@fabpot@yceruto@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp