Merge pull request #23 from kynx/preserve-input-order

Preserve input order in generated array shape
kynx · Feb 24, 2024 · 3ce2452 · 3ce2452
2 parents 789c4fa + b030a87
commit 3ce2452
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 10 deletions.
diff --git a/src/Form/FormVisitor.php b/src/Form/FormVisitor.php
@@ -13,6 +13,7 @@
 use Laminas\Form\FieldsetInterface;
 use Laminas\Form\FormInterface;
 use Laminas\InputFilter\CollectionInputFilter;
+use Laminas\InputFilter\InputFilter;
 use Laminas\InputFilter\InputFilterInterface;
 use Laminas\InputFilter\InputInterface;
 use Psalm\Type\Union;
@@ -88,29 +89,34 @@ private function convertCollectionFilters(
         FieldsetInterface $fieldset,
         InputFilterInterface $inputFilter
     ): InputFilterInterface {
-        foreach ($fieldset->getFieldsets() as $childFieldset) {
-            $name = (string) $childFieldset->getName();
+        $newFilter = new InputFilter();
+
+        foreach ($fieldset->getIterator() as $elementOrFieldset) {
+            $name = (string) $elementOrFieldset->getName();
             if (! $inputFilter->has($name)) {
                 continue;
             }
 
             $inputOrFilter = $inputFilter->get($name);
-            if (! $inputOrFilter instanceof InputFilterInterface) {
+            if (! ($inputOrFilter instanceof InputFilterInterface && $elementOrFieldset instanceof FieldsetInterface)) {
+                $newFilter->add($inputOrFilter, $name);
                 continue;
             }
 
-            $childFilter = $this->convertCollectionFilters($childFieldset, $inputOrFilter);
-            if (! $childFieldset instanceof Collection || $childFilter instanceof CollectionInputFilter) {
+            $childFilter = $this->convertCollectionFilters($elementOrFieldset, $inputOrFilter);
+            if (! $elementOrFieldset instanceof Collection || $childFilter instanceof CollectionInputFilter) {
+                $newFilter->add($childFilter, $name);
                 continue;
             }
 
             if (! $childFilter->has(0)) {
+                $newFilter->add($childFilter, $name);
                 continue;
             }
 
             $target   = $childFilter->get(0);
-            $required = ! $childFieldset->allowRemove();
-            $count    = $required ? $childFieldset->getCount() : 0;
+            $required = ! $elementOrFieldset->allowRemove();
+            $count    = $required ? $elementOrFieldset->getCount() : 0;
 
             if ($target instanceof InputInterface) {
                 $inputOrFilter = CollectionInput::fromInput($target, $count, ! $required);
@@ -122,11 +128,11 @@ private function convertCollectionFilters(
                 }
             }
 
-            $inputFilter->remove($name);
-            $inputFilter->add($inputOrFilter, $name);
+            $newFilter->add($inputOrFilter, $name);
         }
 
-        return $inputFilter;
+        $newFilter->setData($inputFilter->getRawValues());
+        return $newFilter;
     }
 
     /**

diff --git a/test/Form/Asset/InputFilterFieldset.php b/test/Form/Asset/InputFilterFieldset.php
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace KynxTest\Laminas\FormShape\Form\Asset;
+
+use Laminas\Form\Element\Text;
+use Laminas\Form\Fieldset;
+use Laminas\InputFilter\InputFilterProviderInterface;
+
+final class InputFilterFieldset extends Fieldset implements InputFilterProviderInterface
+{
+    /**
+     * @param string|null $name
+     */
+    public function __construct($name = null, array $options = [])
+    {
+        parent::__construct($name, $options);
+
+        $this->add(new Text('first'));
+        $this->add(new Text('second'));
+    }
+
+    public function getInputFilterSpecification(): array
+    {
+        return [
+            'second' => [
+                'required' => true,
+            ],
+        ];
+    }
+}
diff --git a/test/Form/FormVisitorTest.php b/test/Form/FormVisitorTest.php
@@ -9,6 +9,7 @@
 use Kynx\Laminas\FormShape\InputFilter\ImportType;
 use Kynx\Laminas\FormShape\InputFilter\InputFilterVisitor;
 use Kynx\Laminas\FormShape\InputFilter\InputVisitor;
+use KynxTest\Laminas\FormShape\Form\Asset\InputFilterFieldset;
 use Laminas\Form\Element\Collection;
 use Laminas\Form\Element\Email;
 use Laminas\Form\Element\Text;
@@ -26,6 +27,8 @@
 use Psalm\Type\Atomic\TTypeAlias;
 use Psalm\Type\Union;
 
+use function array_keys;
+
 #[CoversClass(FormVisitor::class)]
 final class FormVisitorTest extends TestCase
 {
@@ -242,4 +245,22 @@ public function testVisitCollectionWithImportTypes(): void
         $actual = $this->visitor->visit($form, [Fieldset::class => $importType]);
         self::assertEquals($expected, $actual);
     }
+
+    public function testVisitPreservesInputOrderWhenInputIsRequired(): void
+    {
+        $expected = ['first', 'second'];
+
+        $form = new Form();
+        $form->add(new InputFilterFieldset('foo'));
+
+        $formArray = $this->visitor->visit($form, [])->getSingleAtomic();
+        self::assertInstanceOf(TKeyedArray::class, $formArray);
+        $foo = $formArray->properties['foo'] ?? null;
+        self::assertInstanceOf(Union::class, $foo);
+        $fooArray = $foo->getSingleAtomic();
+        self::assertInstanceOf(TKeyedArray::class, $fooArray);
+
+        $actual = array_keys($fooArray->properties);
+        self::assertSame($expected, $actual);
+    }
 }