Merge pull request #11971 from creative-commoners/pulls/5.4/listbox-mixed-values

FIX ListboxField discarding mixed-type values

Author: ChloeSartorelli

src/Forms/ListboxField.php

@@ -247,27 +247,34 @@ public function getValueArray()
             return [];
         }
 
-        $canary = reset($validValues);
-        $targetType = gettype($canary);
-        if (is_array($value) && count($value) > 0) {
-            $first = reset($value);
-            // sanity check the values - make sure strings get strings, ints get ints etc
-            if ($targetType !== gettype($first)) {
-                $replaced = [];
-                foreach ($value as $item) {
-                    if (!is_array($item)) {
-                        $item = json_decode($item, true);
-                    }
+        if (!is_array($value) && ($value === 0 || $value === '0')) {
+            $value = [$value];
+        }
 
-                    if ($targetType === gettype($item)) {
-                        $replaced[] = $item;
-                    } elseif (isset($item['Value'])) {
-                        $replaced[] = $item['Value'];
+        if (is_array($value) && count($value) > 0) {
+            $replaced = [];
+            foreach ($value as $item) {
+                if (!is_array($item) && is_string($item)) {
+                    $trimmed = trim($item);
+                    if ($trimmed !== '') {
+                        $firstChar = substr($trimmed, 0, 1);
+                        if ($firstChar === '{' || $firstChar === '[') {
+                            $decoded = json_decode($item, true);
+                            if (is_array($decoded)) {
+                                $item = $decoded;
+                            }
+                        }
                     }
                 }
 
-                $value = $replaced;
+                if (is_array($item) && array_key_exists('Value', $item)) {
+                    $replaced[] = $item['Value'];
+                } else {
+                    $replaced[] = $item;
+                }
             }
+
+            $value = $replaced;
         }
 
         return $this->getListValues($value);

tests/php/Forms/ListboxFieldTest.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Forms\Tests;
 
+use PHPUnit\Framework\Attributes\DataProvider;
 use SilverStripe\Forms\Tests\ListboxFieldTest\Article;
 use SilverStripe\Forms\Tests\ListboxFieldTest\Tag;
 use SilverStripe\Forms\Tests\ListboxFieldTest\TestObject;
@@ -120,6 +121,60 @@ public function testSaveIntoMultiple()
         $this->assertEquals('["a","c"]', $obj2->Choices);
     }
 
+    public static function provideSaveIntoValueScenarios(): array
+    {
+        return [
+            'mixed-values' => [
+                'input' => [null, 0, false, '123', -123, '', '0', 'ABC123'],
+                'expected' => [null, 0, false, '123', -123, '', '0', 'ABC123'],
+            ],
+            'string-values' => [
+                'input' => ['123', '0', '-123', '', 'ABC123'],
+                'expected' => ['123', '0', '-123', '', 'ABC123'],
+            ],
+            'integer-values' => [
+                'input' => [0, 123, -123],
+                'expected' => [0, 123, -123],
+            ],
+            'boolean-values' => [
+                'input' => [false, true],
+                'expected' => [false, true],
+            ],
+            'null-only' => [
+                'input' => [null],
+                'expected' => [null],
+            ],
+            'scalar-zero' => [
+                'input' => 0,
+                'expected' => [0],
+            ],
+            'scalar-string-zero' => [
+                'input' => '0',
+                'expected' => ['0'],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideSaveIntoValueScenarios
+     */
+    public function testSaveIntoPreservesValueTypes(mixed $input, array $expected): void
+    {
+        $choices = [
+            '' => 'Empty string',
+            0 => 'Zero',
+            '123' => 'String integer',
+            -123 => 'Negative integer',
+            'ABC123' => 'Alpha-numeric string',
+            'false' => 'False string',
+        ];
+        $field = new ListboxField('Choices', 'Choices', $choices);
+        $obj = new TestObject();
+        $field->setValue($input);
+        $field->saveInto($obj);
+        $this->assertSame($expected, json_decode($obj->Choices, true));
+    }
+
     public function testSaveIntoManyManyRelation()
     {
         $article = $this->objFromFixture(Article::class, 'articlewithouttags');