From 2062293054dc582c5ca9378344c5416464925725 Mon Sep 17 00:00:00 2001 From: matt Date: Sat, 24 Feb 2024 10:53:16 +0000 Subject: [PATCH 1/2] Add failing test for input ordering --- test/Form/Asset/InputFilterFieldset.php | 29 +++++++++++++++++++++++++ test/Form/FormVisitorTest.php | 19 ++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/Form/Asset/InputFilterFieldset.php diff --git a/test/Form/Asset/InputFilterFieldset.php b/test/Form/Asset/InputFilterFieldset.php new file mode 100644 index 0000000..d8f2b7e --- /dev/null +++ b/test/Form/Asset/InputFilterFieldset.php @@ -0,0 +1,29 @@ +add(new Text('first')); + $this->add(new Text('second')); + } + + public function getInputFilterSpecification(): array + { + return [ + 'second' => [ + 'required' => true, + ], + ]; + } +} \ No newline at end of file diff --git a/test/Form/FormVisitorTest.php b/test/Form/FormVisitorTest.php index 2bec10e..d4617f7 100644 --- a/test/Form/FormVisitorTest.php +++ b/test/Form/FormVisitorTest.php @@ -9,8 +9,10 @@ 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\Hidden; use Laminas\Form\Element\Text; use Laminas\Form\Fieldset; use Laminas\Form\Form; @@ -242,4 +244,21 @@ 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; + $fooArray = $foo->getSingleAtomic(); + self::assertInstanceOf(TKeyedArray::class, $fooArray); + + $actual = array_keys($fooArray->properties); + self::assertSame($expected, $actual); + } } From b030a874bf7359f8681bb752712dbfe6b55b010c Mon Sep 17 00:00:00 2001 From: matt Date: Sat, 24 Feb 2024 11:43:42 +0000 Subject: [PATCH 2/2] Preserve order of inputs in generated array shape --- src/Form/FormVisitor.php | 26 +++++++++++++++---------- test/Form/Asset/InputFilterFieldset.php | 5 ++++- test/Form/FormVisitorTest.php | 4 +++- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/Form/FormVisitor.php b/src/Form/FormVisitor.php index 17ab830..2096c09 100644 --- 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 index d8f2b7e..64feb28 100644 --- a/test/Form/Asset/InputFilterFieldset.php +++ b/test/Form/Asset/InputFilterFieldset.php @@ -10,6 +10,9 @@ final class InputFilterFieldset extends Fieldset implements InputFilterProviderInterface { + /** + * @param string|null $name + */ public function __construct($name = null, array $options = []) { parent::__construct($name, $options); @@ -26,4 +29,4 @@ public function getInputFilterSpecification(): array ], ]; } -} \ No newline at end of file +} diff --git a/test/Form/FormVisitorTest.php b/test/Form/FormVisitorTest.php index d4617f7..6be1113 100644 --- a/test/Form/FormVisitorTest.php +++ b/test/Form/FormVisitorTest.php @@ -12,7 +12,6 @@ use KynxTest\Laminas\FormShape\Form\Asset\InputFilterFieldset; use Laminas\Form\Element\Collection; use Laminas\Form\Element\Email; -use Laminas\Form\Element\Hidden; use Laminas\Form\Element\Text; use Laminas\Form\Fieldset; use Laminas\Form\Form; @@ -28,6 +27,8 @@ use Psalm\Type\Atomic\TTypeAlias; use Psalm\Type\Union; +use function array_keys; + #[CoversClass(FormVisitor::class)] final class FormVisitorTest extends TestCase { @@ -255,6 +256,7 @@ public function testVisitPreservesInputOrderWhenInputIsRequired(): void $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);