Skip to content

Commit b783f27

Browse files
committed
Fixing edge bug where validatedFields would not clear if that data was lost
This fixed a problem with CollectionType. If you added a new, embedded form with, for example, index 3 and then modified one of its fields, validatedFields would now contain, for example, form_name.comments.4.content. but then, if you removed this embedded form, but immediately added another, because its index would also be "4", its "content" field would be validated immediately.
1 parent c7a708a commit b783f27

File tree

5 files changed

+147
-5
lines changed

5 files changed

+147
-5
lines changed

src/LiveComponent/src/ComponentWithFormTrait.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,17 @@ private function submitForm(bool $validateAll = true): void
139139
// we only want to validate fields in validatedFields
140140
// but really, everything is validated at this point, which
141141
// means we need to clear validation on non-matching fields
142-
$this->clearErrorsForNonValidatedFields($form, $this->getFormName());
142+
$this->clearErrorsForNonValidatedFields($form, $form->getName());
143143
}
144144

145145
// re-extract the "view" values in case the submitted data
146146
// changed the underlying data or structure of the form
147147
$this->formValues = $this->extractFormValues($this->getForm());
148+
// remove any validatedFields that do not exist in data anymore
149+
$this->validatedFields = ArrayUtil::removePathsNotInData(
150+
$this->validatedFields ?? [],
151+
[$form->getName() => $this->formValues],
152+
);
148153

149154
if (!$form->isValid()) {
150155
throw new UnprocessableEntityHttpException('Form validation failed in component');
@@ -189,7 +194,7 @@ private function getFormInstance(): FormInterface
189194
return $this->formInstance;
190195
}
191196

192-
private function clearErrorsForNonValidatedFields(Form $form, $currentPath = ''): void
197+
private function clearErrorsForNonValidatedFields(Form $form, string $currentPath = ''): void
193198
{
194199
if (!$currentPath || !\in_array($currentPath, $this->validatedFields, true)) {
195200
$form->clearErrors();
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\UX\LiveComponent\Util;
13+
14+
final class ArrayUtil
15+
{
16+
/**
17+
* Removes the "paths" not present in the $data array.
18+
*
19+
* Given an array of paths - ['name', 'post.title', 'post.description'] -
20+
* and a $data array - ['name' => 'Ryan', 'post' => ['title' => 'Hi there!']] -
21+
* this removes any "paths" not present in the array.
22+
*/
23+
public static function removePathsNotInData(array $paths, array $data): array
24+
{
25+
return array_values(array_filter($paths, static function($path) use ($data) {
26+
$parts = explode('.', $path);
27+
while (count($parts) > 0) {
28+
$part = $parts[0];
29+
if (!array_key_exists($part, $data)) {
30+
return false;
31+
}
32+
33+
// reset $parts and go to the next level
34+
unset($parts[0]);
35+
$parts = array_values($parts);
36+
$data = $data[$part];
37+
}
38+
39+
// key was found at all levels
40+
return true;
41+
}));
42+
}
43+
}

src/LiveComponent/tests/Fixture/Component/FormWithCollectionTypeComponent.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\Form\FormInterface;
1616
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
1717
use Symfony\UX\LiveComponent\Attribute\LiveAction;
18+
use Symfony\UX\LiveComponent\Attribute\LiveArg;
1819
use Symfony\UX\LiveComponent\ComponentWithFormTrait;
1920
use Symfony\UX\LiveComponent\DefaultActionTrait;
2021
use Symfony\UX\LiveComponent\Tests\Fixture\Dto\BlogPost;
@@ -46,4 +47,10 @@ public function addComment()
4647
{
4748
$this->formValues['comments'][] = [];
4849
}
50+
51+
#[LiveAction]
52+
public function removeComment(#[LiveArg] int $index)
53+
{
54+
unset($this->formValues['comments'][$index]);
55+
}
4956
}

src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ public function testFormValuesRebuildAfterFormChanges(): void
5050
$dehydrated['validatedFields'] = ['blog_post_form.content'];
5151
$token = $response->crawler()->filter('div')->first()->attr('data-live-csrf-value');
5252
})
53-
// post to action, which will change the "name" field in the form
54-
->post('/_components/form_with_collection_type/addComment?'.http_build_query($dehydrated), [
53+
54+
// post to action, which will add a new embedded comment
55+
->post('/_components/form_with_collection_type/addComment', [
56+
'body' => $dehydrated,
5557
'headers' => ['X-CSRF-TOKEN' => $token],
5658
])
5759
->assertStatus(422)
@@ -65,7 +67,7 @@ public function testFormValuesRebuildAfterFormChanges(): void
6567
->assertContains('The content field is too short')
6668
// make sure the title field did not suddenly become validated
6769
->assertNotContains('The title field should not be blank')
68-
->use(function (Crawler $crawler) {
70+
->use(function (Crawler $crawler) use (&$dehydrated, &$token) {
6971
$div = $crawler->filter('[data-controller="live"]');
7072
$liveData = json_decode($div->attr('data-live-data-value'), true);
7173
// make sure the 2nd collection type was initialized, that it didn't
@@ -77,6 +79,33 @@ public function testFormValuesRebuildAfterFormChanges(): void
7779
],
7880
$liveData['blog_post_form']['comments']
7981
);
82+
83+
// grab the latest live data
84+
$dehydrated = $liveData;
85+
// fake that this field was being validated
86+
$dehydrated['validatedFields'][] = 'blog_post_form.0.comments.content';
87+
$token = $div->attr('data-live-csrf-value');
88+
})
89+
90+
// post to action, which will remove the original embedded comment
91+
->post('/_components/form_with_collection_type/removeComment?'.http_build_query(['args' => 'index=0']), [
92+
'body' => $dehydrated,
93+
'headers' => ['X-CSRF-TOKEN' => $token],
94+
])
95+
->assertStatus(422)
96+
->dump()
97+
// the original embedded form should be gone
98+
->assertNotContains('<textarea id="blog_post_form_comments_0_content"')
99+
// the added one should still be present
100+
->assertContains('<textarea id="blog_post_form_comments_1_content"')
101+
->use(function (Crawler $crawler) {
102+
$div = $crawler->filter('[data-controller="live"]');
103+
$liveData = json_decode($div->attr('data-live-data-value'), true);
104+
// the embedded validated field should be gone, since its data is gone
105+
$this->assertEquals(
106+
['blog_post_form.content'],
107+
$liveData['validatedFields']
108+
);
80109
})
81110
;
82111
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\UX\LiveComponent\Tests\Unit\Util;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\UX\LiveComponent\Util\ArrayUtil;
16+
17+
final class ArrayUtilTest extends TestCase
18+
{
19+
/**
20+
* @dataProvider getPathsTests
21+
*/
22+
public function testRemovePathsNotInData(array $inputPaths, array $inputData, array $expectedPaths)
23+
{
24+
$this->assertEquals($expectedPaths, ArrayUtil::removePathsNotInData($inputPaths, $inputData));
25+
}
26+
27+
public function getPathsTests(): iterable
28+
{
29+
yield 'it_leaves_everything_with_simple_paths' => [
30+
['name', 'price'],
31+
['name' => null, 'price' => 20],
32+
['name', 'price'],
33+
];
34+
35+
yield 'it_leaves_removes_simple_path_not_present' => [
36+
['name', 'price'],
37+
['price' => 20],
38+
['price'],
39+
];
40+
41+
yield 'it_removes_missing_deeper_path' => [
42+
['name', 'post.title'],
43+
['name' => 'Ryan'],
44+
['name'],
45+
];
46+
47+
yield 'it_removes_missing_deeper_indexed_path' => [
48+
['name', 'posts.0.title', 'posts.1.title'],
49+
[
50+
'name' => 'Ryan',
51+
'posts' => [
52+
1 => ['title' => '1 index post']
53+
]
54+
],
55+
['name', 'posts.1.title'],
56+
];
57+
}
58+
}

0 commit comments

Comments
 (0)