From eb64028804e63e36f78fe4a0da4ea4039f76fc55 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Thu, 21 Mar 2024 01:15:14 +0100 Subject: [PATCH 1/3] Multiplier::validate(): Filter out non-validatable components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 45ed76a7ed22c9b97048441eccd4a7fb2f8f2d6f started filtering components to `Control`s, to appease PHPStan, since `Container::validate()` only accepted `Control[]`. But `Multiplier` does not actually have `Control`s as direct children (other than the ‘Add’ `Submitter`s), so it would stop validating and filtering multiplied controls. a5a7348fdb1046275e83fa47d73a444695cf21b4 reverted that part but kept the incorrect phpdoc type cast. Now, it works without the filter because `Container::validate()` already ignores non-validatable components but we should still respect its contract. Let’s filter the components before passing them down. This will also allow us to drop the lying phpdoc type cast. nette/forms 3.2.2 updated its phpdoc param type to allow that: https://github.com/nette/forms/commit/64376718de7c40494d5cc88c83332f59fa1e8086 --- phpstan.neon | 2 +- src/Multiplier.php | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 5eda96f..9e4a8b0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -22,5 +22,5 @@ parameters: - message: '#^Parameter \#1 \$array of function array_filter expects array, Iterator\ given\.$#' identifier: argument.type - count: 3 + count: 4 path: src/Multiplier.php diff --git a/src/Multiplier.php b/src/Multiplier.php index 2fe7c42..9706992 100644 --- a/src/Multiplier.php +++ b/src/Multiplier.php @@ -173,8 +173,7 @@ public function addCreateButton(?string $caption = null, int $copyCount = 1): Cr */ public function validate(?array $controls = null): void { - /** @var Control[] $components */ - $components = $controls ?? $this->getComponents(); + $components = $controls ?? array_filter($this->getComponents(), fn ($component) => $component instanceof Control || $component instanceof Container); foreach ($components as $index => $control) { foreach ($this->noValidate as $item) { From 7d72a68c2b3abbb3c22cb405046ad7dee81b3632 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 17 Apr 2024 09:52:59 +0200 Subject: [PATCH 2/3] Add regression test for filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filters are used for converting an input value (usually a string) into a domain specific PHP type and they are bound to the validation scope: https://doc.nette.org/en/forms/validation#toc-modifying-input-values 45ed76a7ed22c9b97048441eccd4a7fb2f8f2d6f started filtering components passed to `Container::validate()` to `Control`s, to appease PHPStan. But `Multiplier` does not actually have `Control`s as direct children (other than the ‘Add’ `Submitter`s), so it would stop validating and filtering multiplied controls. It has been since fixed in a5a7348fdb1046275e83fa47d73a444695cf21b4, and more properly in the parent commit but let’s add a test so this does not happen again. It can be reproduced by removing `|| $component instanceof Container` from the parent commit. --- tests/Unit/MultiplierTest.php | 44 +++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/Unit/MultiplierTest.php b/tests/Unit/MultiplierTest.php index bf04ca4..0a6f55e 100644 --- a/tests/Unit/MultiplierTest.php +++ b/tests/Unit/MultiplierTest.php @@ -330,6 +330,50 @@ public function testSendNestedInnerWithDefault() ); } + /** + * Ensure filters work on submit, since they are dependent on properly set valdation scope. + * Regression test for https://github.com/contributte/forms-multiplier/issues/68 + */ + public function testSubmitFilter() + { + $response = $this->services->form->createRequest( + MultiplierBuilder::create() + ->fields([]) + ->beforeFormModifier(function (Form $form) { + $form->addInteger('num'); + }) + ->multiplierModifier(function (Multiplier $multiplier) { + $multiplier->onCreate[] = function (Container $container) { + $container->addInteger('mnum'); + }; + }) + ->formModifier(function (Form $form) { + $form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () { + }; + }) + ->createForm() + ) + ->setPost([ + 'num' => '11', + 'm' => [ + ['mnum' => '49'], + ], + ])->send(); + + $this->assertTrue($response->isSuccess()); + $this->assertSame([ + 'num' => 11, + 'm' => [ + ['mnum' => 49], + ], + ], $response->getValues()); + + $dom = $response->toDomQuery(); + + $this->assertDomHas($dom, 'input[name="m[0][mnum]"][value="49"]'); + $this->assertDomNotHas($dom, 'input[name="m[1][mnum]"]'); + } + public function testGroup() { $request = $this->services->form->createRequest( From 71b44f424f7a2c9be0991d6bd88b440d8d5ee72a Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sat, 18 May 2024 17:06:07 +0200 Subject: [PATCH 3/3] Remove redundant cast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was changed to array in bde2503bdfb2a59d5bbeb9fcfe536305362bb376 to override incorrect return value from phpstan-nette’s stub. But with upgrade to PHPStan 2.0 in 07a1009a4897039db9369badf5c28e228a0c0a60, it is no longer helping – we had to resort to ignoring the error in 95335308e14b6be64341bc7dffee0c705753e01e. --- src/Multiplier.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Multiplier.php b/src/Multiplier.php index 9706992..8158e6f 100644 --- a/src/Multiplier.php +++ b/src/Multiplier.php @@ -315,7 +315,6 @@ public function getContainers(): iterable { $this->createCopies(); - /** @var array $containers */ $containers = array_filter($this->getComponents(), fn ($component) => $component instanceof Container); return $containers;