8000 bug #15061 [Form] Fixed handling of choices passed in choice groups (… · symfony/symfony@b16fc6c · GitHub
[go: up one dir, main page]
Skip to content

Commit b16fc6c

Browse files
fabpotfabpot
committed
bug #15061 [Form] Fixed handling of choices passed in choice groups (webmozart)
This PR was merged into the 2.7 branch. Discussion ---------- [Form] Fixed handling of choices passed in choice groups | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | **yes** | Deprecations? | no | Tests pass? | yes | Fixed tickets | #14915 | License | MIT | Doc PR | - I introduced a bug in the 2.7 ChoiceList implementation when choices are passed as groups: ``` $form->add('response', 'choice', array( 'choices' => array( 'Decided' => array($yesObj, $noObj), 'Undecided' => array($maybeObj), ), // use getName() for the labels 'choice_label' => 'name', 'choices_as_values' => true, )); ``` In this example, since the choices `$yesObj` and `$maybeObj` have the same array index `0`, the same label is displayed for the two options. The problem is that we rely on the keys passed in the "choices" option to identify choices in a choice list (which are, as you see, not guaranteed to be free of duplicates). This PR changes the new choice list implementation to identify choices by values instead. We already have the guarantee that choices can be identified uniquely by their string values. This PR should be included in 2.7.2 to fix the regression. Unfortunately, a few BC breaks in the new implementation are necessary to make this fix: * The legacy `ChoiceListInterface` was reverted to how it was in 2.6 and does *not* extend the new `ChoiceListInterface` anymore. * As a consequence, legacy choice lists need to be wrapped into a `LegacyChoiceListAdapter` when they are passed to any place in the framework where a new choice list is expected. * The new `ChoiceListInterface` has two additional methods `getStructuredValues()` and `getOriginalKeys()` now. * `ArrayKeyChoiceList::toArrayKey()` was marked as internal. * `ChoiceListFactoryInterface::createView()` does not accept arrays and Traversables anymore for the `$groupBy` parameter (for simplicity). @fabpot Where should we document the upgrade path for 2.7.1 => 2.7.2? Commits ------- 7623dc8 [Form] Fixed handling of choices passed in choice groups
2 parents bfc174a + 7623dc8 commit b16fc6c

20 files changed

+816
-351
lines changed

‎src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php

Lines changed: 96 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,21 @@ class ArrayChoiceList implements ChoiceListInterface
3030
*
3131
* @var array
3232
*/
33-
protected $choices = array();
33+
protected $choices;
3434

3535
/**
36-
* The values of the choices.
36+
* The values indexed by the original keys.
3737
*
38-
* @var string[]
38+
* @var array
39+
*/
40+
protected $structuredValues;
41+
42+
/**
43+
* The original keys of the choices array.
44+
*
45+
* @var int[]|string[]
3946
*/
40-
protected $values = array();
47+
protected $originalKeys;
4148

4249
/**
4350
* The callback for creating the value for a choice.
@@ -51,31 +58,41 @@ class ArrayChoiceList implements ChoiceListInterface
5158
*
5259
* The given choice array must have the same array keys as the value array.
5360
*
54-
* @param array $choices The selectable choices
55-
* @param callable|null $value The callable for creating the value for a
56-
* choice. If `null` is passed, incrementing
57-
* integers are used as values
61+
* @param array|\Traversable $choices The selectable choices
62+
* @param callable|null $value The callable for creating the value
63+
* for a choice. If `null` is passed,
64+
* incrementing integers are used as
65+
* values
5866
*/
59-
public function __construct(array $choices, $value = null)
67+
public function __construct($choices, $value = null)
6068
{
6169
if (null !== $value && !is_callable($value)) {
6270
throw new UnexpectedTypeException($value, 'null or callable');
6371
}
6472

65-
$this->choices = $choices;
66-
$this->values = array();
67-
$this->valueCallback = $value;
73+
if ($choices instanceof \Traversable) {
74+
$choices = iterator_to_array($choices);
75+
}
6876

69-
if (null === $value) {
70-
$i = 0;
71-
foreach ($this->choices as $key => $choice) {
72-
$this->values[$key] = (string) $i++;
73-
}
77+
if (null !== $value) {
78+
// If a deterministic value generator was passed, use it later
79+
$this->valueCallback = $value;
7480
} else {
75-
foreach ($choices as $key => $choice) {
76-
$this->values[$key] = (string) call_user_func($value, $choice);
77-
}
81+
// Otherwise simply generate incrementing integers as values
82+
$i = 0;
83+
$value = function () use (&$i) {
84+
return $i++;
85+
};
7886
}
87+
88+
// If the choices are given as recursive array (i.e. with explicit
89+
// choice groups), flatten the array. The grouping information is needed
90+
// in the view only.
91+
$this->flatten($choices, $value, $choicesByValues, $keysByValues, $structuredValues);
92+
93+
$this->choices = $choicesByValues;
94+
$this->originalKeys = $keysByValues;
95+
$this->structuredValues = $structuredValues;
7996
}
8097

8198
/**
@@ -91,7 +108,23 @@ public function getChoices()
91108
*/
92109
public function getValues()
93110
{
94-
return $this->values;
111+
return array_map('strval', array_keys($this->choices));
112+
}
113+
114+
/**
115+
* {@inheritdoc}
116+
*/
117+
public function getStructuredValues()
118+
{
119+
return $this->structuredValues;
120+
}
121+
122+
/**
123+
* {@inheritdoc}
124+
*/
125+
public function getOriginalKeys()
126+
{
127+
return $this->originalKeys;
95128
}
96129

97130
/**
@@ -102,17 +135,8 @@ public function getChoicesForValues(array $values)
102135
$choices = array();
103136

104137
foreach ($values as $i => $givenValue) {
105-
foreach ($this->values as $j => $value) {
106-
if ($value !== (string) $givenValue) {
107-
continue;
108-
}
109-
110-
$choices[$i] = $this->choices[$j];
111-
unset($values[$i]);
112-
113-
if (0 === count($values)) {
114-
break 2;
115-
}
138+
if (isset($this->choices[$givenValue])) {
139+
$choices[$i] = $this->choices[$givenValue];
116140
}
117141
}
118142

@@ -131,28 +155,56 @@ public function getValuesForChoices(array $choices)
131155
$givenValues = array();
132156

133157
foreach ($choices as $i => $givenChoice) {
134-
$givenValues[$i] = (string) call_user_func($this->valueCallback, $givenChoice);
158+
$givenValues[$i] = call_user_func($this->valueCallback, $givenChoice);
135159
}
136160

137-
return array_intersect($givenValues, $this->values);
161+
return array_intersect($givenValues, array_keys($this->choices));
138162
}
139163

140164
// Otherwise compare choices by identity
141165
foreach ($choices as $i => $givenChoice) {
142-
foreach ($this->choices as $j => $choice) {
143-
if ($choice !== $givenChoice) {
144-
continue;
145-
}
146-
147-
$values[$i] = $this->values[$j];
148-
unset($choices[$i]);
149-
150-
if (0 === count($choices)) {
151-
break 2;
166+
foreach ($this->choices as $value => $choice) {
167+
if ($choice === $givenChoice) {
168+
$values[$i] = (string) $value;
169+
break;
152170
}
153171
}
154172
}
155173

156174
return $values;
157175
}
176+
177+
/**
178+
* Flattens an array into the given output variables.
179+
*
180+
* @param array $choices The array to flatten
181+
* @param callable $value The callable for generating choice values
182+
* @param array $choicesByValues The flattened choices indexed by the
183+
* corresponding values
184+
* @param array $keysByValues The original keys indexed by the
185+
* corresponding values
186+
*
187+
* @internal Must not be used by user-land code
188+
*/
189+
protected function flatten(array $choices, $value, &$choicesByValues, &$keysByValues, &$structuredValues)
190+
{
191+
if (null === $choicesByValues) {
192+
$choicesByValues = array();
193+
$keysByValues = array();
194+
$structuredValues = array();
195+
}
196+
197+
foreach ($choices as $key => $choice) {
198+
if (is_array($choice)) {
199+
$this->flatten($choice, $value, $choicesByValues, $keysByValues, $structuredValues[$key]);
200+
201+
continue;
202+
}
203+
204+
$choiceValue = (string) call_user_func($value, $choice);
205+
$choicesByValues[$choiceValue] = $choice;
206+
$keysByValues[$choiceValue] = $key;
207+
$structuredValues[$key] = $choiceValue;
208+
}
209+
}
158210
}

‎src/Symfony/Component/Form/ChoiceList/ArrayKeyChoiceList.php

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ class ArrayKeyChoiceList extends ArrayChoiceList
6262
* @return int|string The choice as PHP array key
6363
*
6464
* @throws InvalidArgumentException If the choice is not scalar
65+
*
66+
* @internal Must not be used outside this class
6567
*/
6668
public static function toArrayKey($choice)
6769
{
@@ -89,23 +91,27 @@ public static function toArrayKey($choice)
8991
* If no values are given, the choices are cast to strings and used as
9092
* values.
9193
*
92-
* @param array $choices The selectable choices
93-
* @param callable $value The callable for creating the value for a
94-
* choice. If `null` is passed, the choices are
95-
* cast to strings and used as values
94+
* @param array|\Traversable $choices The selectable choices
95+
* @param callable $value The callable for creating the value
96+
* for a choice. If `null` is passed, the
97+
* choices are cast to strings and used
98+
* as values
9699
*
97100
* @throws InvalidArgumentException If the keys of the choices don't match
98101
* the keys of the values or if any of the
99102
* choices is not scalar
100103
*/
101-
public function __construct(array $choices, $value = null)
104+
public function __construct($choices, $value = null)
102105
{
103-
$choices = array_map(array(__CLASS__, 'toArrayKey'), $choices);
104-
106+
// If no values are given, use the choices as values
107+
// Since the choices are stored in the collection keys, i.e. they are
108+
// strings or integers, we are guaranteed to be able to convert them
109+
// to strings
105110
if (null === $value) {
106111
$value = function ($choice) {
107112
return (string) $choice;
108113
};
114+
109115
$this->useChoicesAsValues = true;
110116
}
111117

@@ -122,7 +128,7 @@ public function getChoicesForValues(array $values)
122128

123129
// If the values are identical to the choices, so we can just return
124130
// them to improve performance a little bit
125-
return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, $this->values));
131+
return array_map(array(__CLASS__, 'toArrayKey'), array_intersect($values, array_keys($this->choices)));
126132
}
127133

128134
return parent::getChoicesForValues($values);
@@ -143,4 +149,38 @@ public function getValuesForChoices(array $choices)
143149

144150
return parent::getValuesForChoices($choices);
145151
}
152+
153+
/**
154+
* Flattens and flips an array into the given output variable.
155+
*
156+
* @param array $choices The array to flatten
157+
* @param callable $value The callable for generating choice values
158+
* @param array $choicesByValues The flattened choices indexed by the
159+
* corresponding values
160+
* @param array $keysByValues The original keys indexed by the
161+
* corresponding values
162+
*
163+
* @internal Must not be used by user-land code
164+
*/
165+
protected function flatten(array $choices, $value, &$choicesByValues, &$keysByValues, &$structuredValues)
166+
{
167+
if (null === $choicesByValues) {
168+
$choicesByValues = array();
169+
$keysByValues = array();
170+
$structuredValues = array();
171+
}
172+
173+
foreach ($choices as $choice => $key) {
174+
if (is_array($key)) {
175+
$this->flatten($key, $value, $choicesByValues, $keysByValues, $structuredValues[$choice]);
176+
177+
continue;
178+
}
179+
180+
$choiceValue = (string) call_user_func($value, $choice);
181+
$choicesByValues[$choiceValue] = $choice;
182+
$keysByValues[$choiceValue] = $key;
183+
$structuredValues[$key] = $choiceValue;
184+
}
185+
}
146186
}

‎src/Symfony/Component/Form/ChoiceList/ChoiceListInterface.php

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,40 +14,80 @@
1414
/**
1515
* A list of choices that can be selected in a choice field.
1616
*
17-
* A choice list assigns string values to each of a list of choices. These
18-
* string values are displayed in the "value" attributes in HTML and submitted
19-
* back to the server.
17+
* A choice list assigns unique string values to each of a list of choices.
18+
* These string values are displayed in the "value" attributes in HTML and
19+
* submitted back to the server.
2020
*
2121
* The acceptable data types for the choices depend on the implementation.
2222
* Values must always be strings and (within the list) free of duplicates.
2323
*
24-
* The choices returned by {@link getChoices()} and the values returned by
25-
* {@link getValues()} must have the same array indices.
26-
*
2724
* @author Bernhard Schussek <bschussek@gmail.com>
2825
*/
2926
interface ChoiceListInterface
3027
{
3128
/**
3229
* Returns all selectable choices.
3330
*
34-
* The keys of the choices correspond to the keys of the values returned by
35-
* {@link getValues()}.
36-
*
37-
* @return array The selectable choices
31+
* @return array The selectable choices indexed by the corresponding values
3832
*/
3933
public function getChoices();
4034

4135
/**
4236
* Returns the values for the choices.
4337
*
44-
* The keys of the values correspond to the keys of the choices returned by
45-
* {@link getChoices()}.
38+
* The values are strings that do not contain duplicates.
4639
*
4740
* @return string[] The choice values
4841
*/
4942
public function getValues();
5043

44+
/**
45+
* Returns the values in the structure originally passed to the list.
46+
*
47+
* Contrary to {@link getValues()}, the result is indexed by the original
48+
* keys of the choices. If the original array contained nested arrays, these
49+
* nested arrays are represented here as well:
50+
*
51+
* $form->add('field', 'choice', array(
52+
* 'choices' => array(
53+
* 'Decided' => array('Yes' => true, 'No' => false),
54+
* 'Undecided' => array('Maybe' => null),
55+
* ),
56+
* ));
57+
*
58+
* In this example, the result of this method is:
59+
*
741A
60+
* array(
61+
* 'Decided' => array('Yes' => '0', 'No' => '1'),
62+
* 'Undecided' => array('Maybe' => '2'),
63+
* )
64+
*
65+
* @return string[] The choice values
66+
*/
67+
public function getStructuredValues();
68+
69+
/**
70+
* Returns the original keys of the choices.
71+
*
72+
* The original keys are the keys of the choice array that was passed in the
73+
* "choice" option of the choice type. Note that this array may contain
74+
* duplicates if the "choice" option contained choice groups:
75+
*
76+
* $form->add('field', 'choice', array(
77+
* 'choices' => array(
78+
* 'Decided' => array(true, false),
79+
* 'Undecided' => array(null),
80+
* ),
81+
* ));
82+
*
83+
* In this example, the original key 0 appears twice, once for `true` and
84+
* once for `null`.
85+
*
86+
* @return int[]|string[] The original choice keys indexed by the
87+
* corresponding choice values
88+
*/
89+
public function getOriginalKeys();
90+
5191
/**
5292
* Returns the choices corresponding to the given values.
5393
*

0 commit comments

Comments
 (0)
0