Skip to content

[Validator] Add min and max in both error messages of LengthValidator #60805

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
Merged
fabpot merged 1 commit into from
Jul 26, 2025
Merged

[Validator] Add min and max in both error messages of LengthValidator #60805

fabpot merged 1 commit into from
Jul 26, 2025

Conversation

@VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Jun 16, 2025 •
edited by OskarStark
Loading

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues --
License MIT

This would allow to use the same message for a Length constraint which say
"The length needs to be between {{ min }} and {{ max }}".

@carsonbot carsonbot added this to the 7.4 milestone Jun 16, 2025
@OskarStark OskarStark changed the title [Validator] Add min and max in both error message of LengthValidator [Validator] Add min and max in both error messages of LengthValidator Jun 16, 2025
@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jun 16, 2025

The Tests on 8.2 high-deps are failing because this PR
fc7d413
changes the tests and the CI is trying to run on the branch 6.4
image

How do I solve this ?

OskarStark
OskarStark reviewed Jun 16, 2025
View reviewed changes
@VincentLanglet VincentLanglet force-pushed the lengthValidator branch 2 times, most recently from f51245e to 8df3197 Compare June 20, 2025 21:14
@Spomky
Copy link
Contributor

Spomky commented Jul 17, 2025 •
edited
Loading

Looks good.

I noticed some failures are related to these changes.
Would you mind to rebase (solve the conflicts) and fix the tests?
Regards

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jul 17, 2025

I noticed some failures are related to these changes. Would you ming to rebase (solve the conflicts) and fix the tests? Regards

Hi ; I rebased but I dunno how to fix the test.

See https://github.com/symfony/symfony/actions/runs/16356617489/job/46216379960?pr=60805

The Unit test 8.2 high-deps is checking out the code on Symfony 6.4

Checking out Symfony 6.4 and running tests with patched components as deps

So this file changes are lost
https://github.com/symfony/symfony/pull/60805/files#diff-927362e8296b98f998ae0368419317fd75a34054f520cc354c691ddc22cb5b0c

And of course tests are failing since they don't have the {{ min }} value I added then...

Any clue how to fix this @Spomky ?

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jul 24, 2025

Friendly ping @nicolas-grekas you might know how to solve the failure #60805 (comment)

Test are failing but I feel like I cannot fix them since it runs the 6.4 version and I'm on the 7.4 branch...

@xabbuh
Copy link
Member

xabbuh commented Jul 25, 2025

I suggest that we update the ApiAttributesTest in the 6.4 after this PR is merged and relax the assertion there.

@fabpot fabpot force-pushed the lengthValidator branch from 9ec2a5e to c465978 Compare July 26, 2025 12:32
@fabpot
Copy link
Member

fabpot commented Jul 26, 2025

Thank you @VincentLanglet.

@fabpot fabpot merged commit 2663b3b into symfony:7.4 Jul 26, 2025
9 of 11 checks passed
@fabpot fabpot mentioned this pull request Jul 26, 2025
Closed
@MatTheCat MatTheCat mentioned this pull request Jul 28, 2025
Merged
@xabbuh xabbuh mentioned this pull request Jul 30, 2025
Merged
nicolas-grekas added a commit that referenced this pull request Jul 30, 2025
@nicolas-grekas
minor #61264 [FrameworkBundle] relax assertions for forward compatibi…
92727bd 
…lity with Symfony 7.4 (xabbuh)

This PR was merged into the 6.4 branch.

Discussion
----------

[FrameworkBundle] relax assertions for forward compatibility with Symfony 7.4

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | see #60805 (comment) and my response to it
| License       | MIT

Commits
-------

0e9136e relax assertions for forward compatibility with Symfony 7.4
@fabpot fabpot mentioned this pull request Oct 27, 2025
Merged
@fabpot fabpot mentioned this pull request Oct 27, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@Spomky Spomky Spomky approved these changes

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees

No one assigned

Labels

Feature Status: Reviewed Validator

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

6 participants

@VincentLanglet @Spomky @xabbuh @fabpot @OskarStark @carsonbot