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

Appearance settings

[Validator] Fix translation of AtLeastOneOf constraint message #49484

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Feb 21, 2023

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #41317
License MIT
Doc PR NA

New try after Nicolas' hint on #41325.
This is a much simpler solution using execution context to get the base message of the constraint translated.

I tried to create the relevant test case, but other locales doesn't seem to be available in validators test cases. I'm having a look at a test case in a dummy locale after #49484 (comment)

When using another locale, the base message of the constraint is actually translated now:

image

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 21, 2023

Can't we have a test with dummy translations in any locale?

@alexandre-daubois
Copy link
Member Author

I'll have a look at it 👍

@alexandre-daubois
Copy link
Member Author

The test has been added with a dummy translator 👍

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

see fabbot :)


public function trans(?string $id, array $parameters = [], string $domain = null, string $locale = null): string
{
if ($id === 'This value should satisfy at least one of the following constraints:') {
Copy link
Member

Choose a reason for hiding this comment

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

let's add another case to also translate "This value should be null."?

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be it! Also renamed the test case to be more relevant 🙂

@nicolas-grekas
Copy link
Member

Thank you @alexandre-daubois.

@nicolas-grekas nicolas-grekas merged commit 01a222b into symfony:5.4 Feb 22, 2023
@alexandre-daubois alexandre-daubois deleted the fix-atleastoneof branch February 22, 2023 19:19
symfony-splitter pushed a commit to symfony/validator that referenced this pull request Feb 22, 2023
…sage (alexandre-daubois)

This PR was merged into the 5.4 branch.

Discussion
----------

[Validator] Fix translation of AtLeastOneOf constraint message

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #41317
| License       | MIT
| Doc PR        | _NA_

New try after Nicolas' hint on symfony/symfony#41325.
This is a much simpler solution using execution context to get the base message of the constraint translated.

~I tried to create the relevant test case, but other locales doesn't seem to be available in validators test cases.~ I'm having a look at a test case in a dummy locale after symfony/symfony#49484 (comment)

When using another locale, the base message of the constraint is actually translated now:

<img width="1238" alt="image" src="https://user-images.githubusercontent.com/2144837/220455120-c17e7ac6-81c5-4dd0-a2fc-b100a6a00688.png">

Commits
-------

a687c9a2a0 [Validator] Fix translation of AtLeastOneOf constraint message
This was referenced Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.