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] Add support for theotherwise option in theWhen constraint#59634

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

Merged
fabpot merged 1 commit intosymfony:7.3fromalexandre-daubois:negate-when
Feb 7, 2025

Conversation

alexandre-daubois
Copy link
Member

@alexandre-dauboisalexandre-daubois commentedJan 28, 2025
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#57370
LicenseMIT

This feature allows to give constraints toWhen when the evaluated expression returns false:

useSymfony\Component\Validator\Constraints\IsNull;useSymfony\Component\Validator\Constraints\Length;useSymfony\Component\Validator\Constraints\When;class BlogPost{public ?int$id =null;        #[When(        expression:'this.id !== null',         constraints:newLength(min:10),        otherwise:newIsNull(),    )]public ?string$content =null;}

valtzu and Methraen reacted with thumbs up emoji
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedJan 28, 2025
edited
Loading

I'm not sure this is what the issue is about. From what I can read, they need a ternary style instead:

when foo ? then-this : else-that.

#[When('expression', otherwise: new Bar)] would be closer to what's described in the issue.

(please take time to give example in PR descriptions for new feature so that it's easy to bootstrap the doc / write a blog post.)

@alexandre-daubois
Copy link
MemberAuthor

alexandre-daubois commentedJan 28, 2025
edited
Loading

I'm not sure this is what the issue is about. From what I can read, they need a ternary style instead

I did so to be consistent with the Expression constraint, taking the last issue comment example as reference which seemed more in line with what already exists. But if we don't care about this point, let's do as you say, works for me as well 👍

(please take time to give example in PR descriptions for new feature so that it's easy to bootstrap the doc / write a blog post.)

My bad indeed, will do!

@nicolas-grekas
Copy link
Member

I don't see the point of a negate option here sorry.

@Kocal
Copy link
Member

Kocal commentedJan 28, 2025
edited
Loading

Hey!

Given your proposal, the example from#57370 will become:

#[Assert\When('this.getProduct()?.getNature()?.value === "SESSION"',newAssert\IsNull())]#[Assert\When('this.getProduct()?.getNature()?.value === "SESSION"',newAssert\NotNull(), negate:true)]

right?

What about this syntax suggested in the issue and by Nicolas above?

#[Assert\When('this.getProduct()?.getNature()?.value === "SESSION"',newAssert\IsNull(),     otherwise:newAssert\NotNull())]

or maybe aWhenNot assertion?

@alexandre-daubois
Copy link
MemberAuthor

otherwise solution is nice, lets implement this one! I've got one last thing to figure with parent groups propagation in nested constraints and it should be good 👍

@alexandre-dauboisalexandre-dauboisforce-pushed thenegate-when branch 2 times, most recently from7dcfac3 to5439efbCompareJanuary 28, 2025 19:29
@alexandre-daubois
Copy link
MemberAuthor

It's been more complicated than expected because of how composite constraints work. Their internals has to be updated to accept multiple fields that contains nested constraints (and propagate groups). For this,Composer::getCompositeOption() return type has to be widen toarray|string. If I'm right, this does not break BC as child classes are allowed to narrow the return type.

So here's a new implementation, tell me what you think 👍

@alexandre-dauboisalexandre-daubois changed the title[Validator] Add support for thenegate option in theWhen constraint[Validator] Add support for theotherwise option in theWhen constraintJan 29, 2025
@alexandre-daubois
Copy link
MemberAuthor

alexandre-daubois commentedFeb 5, 2025
edited
Loading

All comments addressed, thanks!

Should a PR be sent to update toget_debug_type($this) all calls tostatic::class? There are a lot to them after the deprecation of constraints arguments as array (73 occurrences it seems in Validator)

@nicolas-grekas
Copy link
Member

get_debug_type is to be used in exception messages. that's better, but not critical either.

alexandre-daubois reacted with thumbs up emoji

@fabpot
Copy link
Member

Thank you@alexandre-daubois.

@fabpotfabpot merged commitbc08cb4 intosymfony:7.3Feb 7, 2025
9 of 11 checks passed
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

"When" Constraint -> Possibility to apply validations if expression(s) not matched (same way as if/else)
5 participants
@alexandre-daubois@nicolas-grekas@Kocal@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp