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] Choice type int values (BC Fix) #21957

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
7 participants
@mcfedr
Copy link
Contributor

commented Mar 10, 2017

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #21952
License MIT

#21267 unnecessarily forced all choice values to be strings, this was a BC braking change.

Everything can work fine if they are strings or ints. specifically the issue was because array_flip is used on the choices array, but this function will work fine with ints as well as strings.

Normally, when using HTML forms all data comes as strings, but the symfony/form component has use cases beyond this, specifically, I use it with JSON data, where valid values for a ChoiceType might not be strings.

@mcfedr mcfedr force-pushed the mcfedr:choice-type-int-values branch from 480d18c to 4953631 Mar 10, 2017

@mcfedr mcfedr force-pushed the mcfedr:choice-type-int-values branch from 4953631 to be1c3a8 Mar 10, 2017

@mcfedr mcfedr changed the title Choice type int values Choice type int values (BC Fix) Mar 10, 2017

@@ -171,7 +171,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
}
foreach ($data as $v) {
if (null !== $v && !is_string($v)) {
if (null !== $v && !is_string($v) && !is_int($v)) {
throw new TransformationFailedException('All choices submitted must be NULL or strings.');

This comment has been minimized.

Copy link
@stof

stof Mar 10, 2017

Member

the message should be updated too

This comment has been minimized.

Copy link
@mcfedr

mcfedr Mar 10, 2017

Author Contributor

Changed it.

@mcfedr mcfedr force-pushed the mcfedr:choice-type-int-values branch from be1c3a8 to 56e3974 Mar 10, 2017

@nicolas-grekas nicolas-grekas modified the milestones: 2.8, 2.7 Mar 10, 2017

@mcfedr mcfedr force-pushed the mcfedr:choice-type-int-values branch from 56e3974 to dad87f8 Mar 10, 2017

@mcfedr mcfedr changed the title Choice type int values (BC Fix) [Form] Choice type int values (BC Fix) Mar 10, 2017

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 10, 2017

Thank you @mcfedr.

fabpot added a commit that referenced this pull request Mar 10, 2017

bug #21957 [Form] Choice type int values (BC Fix) (mcfedr)
This PR was squashed before being merged into the 2.7 branch (closes #21957).

Discussion
----------

[Form] Choice type int values (BC Fix)

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21952
| License       | MIT

#21267 unnecessarily forced all choice values to be strings, this was a BC braking change.

Everything can work fine if they are strings or ints. specifically the issue was because `array_flip` is used on the `choices` array, but this function will work fine with ints as well as strings.

Normally, when using HTML forms all data comes as strings, but the `symfony/form` component has use cases beyond this, specifically, I use it with JSON data, where valid values for a `ChoiceType` might not be strings.

Commits
-------

ed211e9 [Form] Choice type int values (BC Fix)

@fabpot fabpot closed this Mar 10, 2017

@mcfedr mcfedr deleted the mcfedr:choice-type-int-values branch Mar 11, 2017

@mcfedr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2017

Thank you @fabpot for Symfony :)

@mcfedr

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2017

This needs to land it 2.8 and 3.2 branches as well.

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 12, 2017

Merged into 2.8 as well. For 3.2, I will let @nicolas-grekas do the merge

chihiro-adachi added a commit to chihiro-adachi/ec-cube that referenced this pull request Mar 14, 2017

テストに失敗するため修正
- symfony 3.2.5/6で、choice typeでint値を受け付けなくなっていたため(symfony/symfony#21267)
- 21267はbc break扱いで再度修正が入っている(symfony/symfony#21957)
- symfony3.2.7で解消されると思われるが、テスト通したいので直しておく
@superhaggis

This comment has been minimized.

Copy link

commented Mar 29, 2017

Ping @nicolas-grekas - any ETA on the 3.2 merge? We're still locked to 3.2.4 here as a result of the issue. 😢

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

@superhaggis It's been merged a while ago.

@fabpot

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

Or are you asking for a new release? In that case, that should be done very soon.

@superhaggis

This comment has been minimized.

Copy link

commented Mar 29, 2017

@fabpot Yeah, I guess I'm asking for a new release. Thanks for the update. 😄

@fabpot fabpot referenced this pull request Apr 4, 2017

Merged

Release v2.7.26 #22263

@PhilippSchreiber

This comment has been minimized.

Copy link

commented Apr 4, 2017

When will this fix be released in 3.2.*? Downgrading right now to 3.2.4 :(

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 4, 2017

A new 3.2 version will probably be released today.

@fabpot fabpot referenced this pull request Apr 5, 2017

Merged

Release v2.8.19 #22284

chihiro-adachi added a commit to chihiro-adachi/ec-cube that referenced this pull request Apr 5, 2017

テストに失敗するため修正 - symfony 3.2.5/6で、choice typeでint値を受け付けなくなっていたため(symfon…
…y/symfony#21267) - 21267はbc break扱いで再度修正が入っている(symfony/symfony#21957) - symfony3.2.7で解消されると思われるが、テスト通したいので直しておく

@fabpot fabpot referenced this pull request Apr 5, 2017

Merged

Release v3.2.7 #22293

fabpot added a commit that referenced this pull request May 9, 2017

minor #22673 [Form] Minor: Fix comment in ChoiceType (issei-m)
This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Minor: Fix comment in ChoiceType

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | n/a
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Forgotten to be updated in: #21957

Commits
-------

a49d79c [Form] Minor: Fix comment in ChoiceType
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.