Skip to content

Commit 02216bc

Browse files
committed
FIX ListboxField discarding mixed-type values
1 parent a8f508e commit 02216bc

File tree

2 files changed

+78
-16
lines changed

2 files changed

+78
-16
lines changed

src/Forms/ListboxField.php

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -247,27 +247,34 @@ public function getValueArray()
247247
return [];
248248
}
249249

250-
$canary = reset($validValues);
251-
$targetType = gettype($canary);
252-
if (is_array($value) && count($value) > 0) {
253-
$first = reset($value);
254-
// sanity check the values - make sure strings get strings, ints get ints etc
255-
if ($targetType !== gettype($first)) {
256-
$replaced = [];
257-
foreach ($value as $item) {
258-
if (!is_array($item)) {
259-
$item = json_decode($item, true);
260-
}
250+
if (!is_array($value) && ($value === 0 || $value === '0')) {
251+
$value = [$value];
252+
}
261253

262-
if ($targetType === gettype($item)) {
263-
$replaced[] = $item;
264-
} elseif (isset($item['Value'])) {
265-
$replaced[] = $item['Value'];
254+
if (is_array($value) && count($value) > 0) {
255+
$replaced = [];
256+
foreach ($value as $item) {
257+
if (!is_array($item) && is_string($item)) {
258+
$trimmed = trim($item);
259+
if ($trimmed !== '') {
260+
$firstChar = substr($trimmed, 0, 1);
261+
if ($firstChar === '{' || $firstChar === '[') {
262+
$decoded = json_decode($item, true);
263+
if (is_array($decoded)) {
264+
$item = $decoded;
265+
}
266+
}
266267
}
267268
}
268269

269-
$value = $replaced;
270+
if (is_array($item) && array_key_exists('Value', $item)) {
271+
$replaced[] = $item['Value'];
272+
} else {
273+
$replaced[] = $item;
274+
}
270275
}
276+
277+
$value = $replaced;
271278
}
272279

273280
return $this->getListValues($value);

tests/php/Forms/ListboxFieldTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace SilverStripe\Forms\Tests;
44

5+
use PHPUnit\Framework\Attributes\DataProvider;
56
use SilverStripe\Forms\Tests\ListboxFieldTest\Article;
67
use SilverStripe\Forms\Tests\ListboxFieldTest\Tag;
78
use SilverStripe\Forms\Tests\ListboxFieldTest\TestObject;
@@ -120,6 +121,60 @@ public function testSaveIntoMultiple()
120121
$this->assertEquals('["a","c"]', $obj2->Choices);
121122
}
122123

124+
public static function provideSaveIntoValueScenarios(): array
125+
{
126+
return [
127+
'mixed-values' => [
128+
'input' => [null, 0, false, '123', -123, '', '0', 'ABC123'],
129+
'expected' => [null, 0, false, '123', -123, '', '0', 'ABC123'],
130+
],
131+
'string-values' => [
132+
'input' => ['123', '0', '-123', '', 'ABC123'],
133+
'expected' => ['123', '0', '-123', '', 'ABC123'],
134+
],
135+
'integer-values' => [
136+
'input' => [0, 123, -123],
137+
'expected' => [0, 123, -123],
138+
],
139+
'boolean-values' => [
140+
'input' => [false, true],
141+
'expected' => [false, true],
142+
],
143+
'null-only' => [
144+
'input' => [null],
145+
'expected' => [null],
146+
],
147+
'scalar-zero' => [
148+
'input' => 0,
149+
'expected' => [0],
150+
],
151+
'scalar-string-zero' => [
152+
'input' => '0',
153+
'expected' => ['0'],
154+
],
155+
];
156+
}
157+
158+
/**
159+
* @dataProvider provideSaveIntoValueScenarios
160+
*/
161+
public function testSaveIntoPreservesValueTypes(mixed $input, array $expected): void
162+
{
163+
$choices = [
164+
'' => 'Empty string',
165+
0 => 'Zero',
166+
'123' => 'String integer',
167+
-123 => 'Negative integer',
168+
'ABC123' => 'Alpha-numeric string',
169+
'false' => 'False string',
170+
];
171+
$field = new ListboxField('Choices', 'Choices', $choices);
172+
$obj = new TestObject();
173+
$field->setValue($input);
174+
$field->saveInto($obj);
175+
$this->assertSame($expected, json_decode($obj->Choices, true));
176+
}
177+
123178
public function testSaveIntoManyManyRelation()
124179
{
125180
$article = $this->objFromFixture(Article::class, 'articlewithouttags');

0 commit comments

Comments
 (0)