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

[Form] AddLocaleTypeTest::testInvalidLocaleMessage()#29828

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

Closed
phansys wants to merge1 commit intosymfony:3.4fromphansys:form_not_synchronized

Conversation

@phansys
Copy link
Contributor

@phansysphansys commentedJan 9, 2019
edited
Loading

QA
Branch3.4
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

Since passing a invalid locale causes the form to be not synchronized, the validation fails because this reason instead of the locale validation.
Since I don't know if this is the expected result, this PR provides a test covering the described behavior in order to clarify the situation.

@phansysphansys changed the titleAddLocaleTypeTest::testInvalidLocaleMessage()[Form] AddLocaleTypeTest::testInvalidLocaleMessage()Jan 9, 2019
@xabbuh
Copy link
Member

The current behaviour is expected. TheLocaleType is a specialisedChoiceType and theChoiceType is automatically invalid in case an invalid choice is submitted (there have been bugfixes recently to ensure this behaviour).

@phansys
Copy link
ContributorAuthor

Thank you for the response.
IIUC, based on that premise, when I submit a invalid locale through this form type I can't get the violation fromLocaleValidator anymore?

@xabbuh
Copy link
Member

Yes, that's right. But you can use theinvalid_message option of the form type to reuse the message from the validator.

@phansys
Copy link
ContributorAuthor

In that case, I should instantiate manually theLocale constraint just to obtain the$message property and pass it as the "invalid_message" option. Moreover, I think this solution doesn't cover (in an easy way) the scenario where the validation constraint has multiple possible messages or when one form field has more than one validation constraint associated to the property which the field represents.

@nicolas-grekasnicolas-grekas added this to the3.4 milestoneJan 24, 2019
@nicolas-grekas
Copy link
Member

Should we merge this PR? If yes, please rebase to account for short arrays. If needed, please open an issue if there is anything to keep track of.

@xabbuh
Copy link
Member

The added test is failing as expected. I think for cases where the current behaviour is not sufficient enough, having to configure theinvalid_message option yourself is acceptable. You even do not need to instantiate the constraint. Just take a look at the message being used and optionally copy its translations to themessages domain.

@phansys
Copy link
ContributorAuthor

Thank for your response. I think this behavior was changed between3.4.20 and3.4.21. I detected this change thanks to a test that I have in a own project, so I think the change, although minor, could be considered a BC break.
Is there any entry at changelog or upgrade files? If not, should I add a note for this change?
Please, let me know.

@xabbuh
Copy link
Member

I think that's because before#29500 was merged, theLocaleType mistakenly was not marked as invalid (not synchronised). Though I do not think we should add a dedicated changelog entry. That PR was just a regular bugfix.

@phansys
Copy link
ContributorAuthor

Closing based on the given explanation. Thank you!

xabbuh reacted with thumbs up emoji

@phansysphansys closed thisFeb 2, 2019
@phansysphansys deleted the form_not_synchronized branchFebruary 2, 2019 12:33
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

4 participants

@phansys@xabbuh@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp