Skip to content
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

[Form] ChoiceType error after upgrade to 3.0.4 #18461

Closed
walczakmac opened this issue Apr 6, 2016 · 7 comments
Closed

[Form] ChoiceType error after upgrade to 3.0.4 #18461

walczakmac opened this issue Apr 6, 2016 · 7 comments

Comments

@walczakmac
Copy link

I just upgraded to Symfony 3.0.4 and I can't set integers as choice values in ChoiceType field. Validation gives me "This value is not valid." message for an integer value. In my case it was default value passed through "empty_data" option.

Here is example field I use to reproduce this:

$builder->add('emailNotificationsEnabled', ChoiceType::class, [
    'required' => false,
    'multiple' => false,
    'choices'  => [0, 1],
    'empty_data' => 1,
]);

I found out that this is caused by this change: 3eac469#diff-81585954baecb0c458557b7969974093L42

@HeahDude
Copy link
Contributor

HeahDude commented Apr 6, 2016

Hi @walczakmac, thank you for reporting this.
In fact empty_data of the ChoiceType should always be the string value of the choice.

This option stands for the submitted data (the string selected from the html form) and will be matched in the ChoiceToValueTransformer who needs a string to get to the right choice which in that case is an integer.

This can be a "BC break", however this is the way it is supposed to happen, as far as I understand.

If I'm wrong, please submit a PR to fix this regression.

In any case sorry for the trouble

@HeahDude
Copy link
Contributor

HeahDude commented Apr 6, 2016

In fact that the casting was done at this line 3eac469#diff-81585954baecb0c458557b7969974093R46 and is useless with that change.

Because it should always be a string.

In contrary, to pre select the choice, the option data needs to be the "real" choice value, in your case an integer.

So I suggest to open a PR anyway, either to remove the useless (string) casting, or to revert that change.

ping @webmozart

@walczakmac Would you like to handle it ?

@HeahDude
Copy link
Contributor

HeahDude commented Apr 6, 2016

ping @Tobion :)

@walczakmac
Copy link
Author

@HeahDude Yes i can open a PR to fix that. I'll revert that change becase it's a BC break IMO and shouldn't be introduced in minor version.

@HeahDude
Copy link
Contributor

HeahDude commented Apr 6, 2016

Should be closed by symfony/symfony-docs#6340.

@walczakmac
Copy link
Author

As mentioned in #18462 I'm closing this.

@ameenross
Copy link

This cost me upwards of 2 hours today. Whatever was wrong with PHP type juggling I wonder... if I'd thought it to be a problem, I'd be using a strongly typed language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants