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 #41325

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

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented May 20, 2021

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

@carsonbot
Copy link

Hey!

I think @przemyslaw-bogusz has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@alexandre-daubois alexandre-daubois changed the base branch from 5.2 to 5.3 June 9, 2021 08:13
@alexandre-daubois alexandre-daubois force-pushed the fix-atleastoneof-message-translation branch from 9bacb1b to ee02a6d Compare June 9, 2021 08:15
@alexandre-daubois alexandre-daubois force-pushed the fix-atleastoneof-message-translation branch from ee02a6d to 044f223 Compare July 5, 2021 11:55
@fabpot fabpot modified the milestones: 5.2, 5.3 Jul 26, 2021
@OskarStark OskarStark changed the title [Validator] Fix translation of AtLeastOneOf constraint message [Validator] Fix translation of AtLeastOneOf constraint message Aug 1, 2021
@alexandre-daubois alexandre-daubois force-pushed the fix-atleastoneof-message-translation branch from 044f223 to a08c00b Compare August 2, 2021 06:57
@@ -26,7 +26,7 @@ class AtLeastOneOf extends Composite
];

public $constraints = [];
public $message = 'This value should satisfy at least one of the following constraints:';
public $message = 'This value should satisfy at least one of the following constraints: {{ child_messages }}';
Copy link
Member

Choose a reason for hiding this comment

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

As of today, translations are the same in all maintained branches, which simplifies things a lot. That would break this pattern as this message would be different depending on the version. I would love to find another way to fix this issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, thank you for your feedback. I'll try to get back with another solution! 👍

Copy link
Member

Choose a reason for hiding this comment

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

can't we concatenate the other messages after translating the current message?
that would allow not changing the translation tables

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I forgot about this one. I'll have a look at it soon and give it another try with your additional info!

@alexandre-daubois alexandre-daubois force-pushed the fix-atleastoneof-message-translation branch from a08c00b to 38b1aef Compare November 18, 2021 16:08
@TimoBakx
Copy link
Member

I ran into this same problem today. Is there anything I can do to help?

@alexandre-daubois
Copy link
Member Author

Closed in favor of #49484

@alexandre-daubois alexandre-daubois deleted the fix-atleastoneof-message-translation branch February 21, 2023 20:51
nicolas-grekas added a commit 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 #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:

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

Commits
-------

a687c9a [Validator] Fix translation of AtLeastOneOf constraint message
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
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.

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