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] Allow NumberToLocalizedStringTransformer::reverseTransform to accept int/float#12470

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

Conversation

@boekkooi
Copy link
Contributor

QA
Bug fix?not sure
New feature?not sure
BC breaks?yes
Deprecations?no
Tests pass?yes
Fixed tickets#12499
LicenseMIT
Doc PR-

When adding a form typeinteger with aempty_data => 0 (aka with a int or float value) the form field will never validate correctly because theNumberToLocalizedStringTransformer will throw aTransformationFailedException because the value is not a string.

This PR fixes this problem by allowing int and float types as values inreverseTransform. Because the string only behavior was enforced by the transformer tests this is a BC change. (Personally I think that this is not a BC break that would cause problems).

Another possible fix is to add a normalizer for theempty_data option and transform a int or float to a LocalizedString but that looks/feels like a bad patch job.

I hope this PR will have less people going through there validation schema just to figure out that it's theNumberToLocalizedStringTransformer causing the problem.

@boekkooi
Copy link
ContributorAuthor

@fabpot Is there any chance of getting this merged?

@hpatoio
Copy link
Contributor

👍

@webmozart
Copy link
Contributor

Hi@boekkooi, thank you for taking the time to submit this PR! :) Unfortunately, I think that this is kind of a hack. The underlying problem is#4715, which cannot be fixed without BC breaks. Hopefully, we can fix#4715 in 3.0+, so that this issue will be fixed as well.

@boekkooi
Copy link
ContributorAuthor

@webmozart Thanks for having a look at this PR. I'm personally not convinced at all that this is a hack, so if you have the time could you maybe explain why you think that it is? (I tried to figure out what you mean based on#4715 but I'm not sure).

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@boekkooi@hpatoio@webmozart@jakzal

[8]ページ先頭

©2009-2025 Movatter.jp