diff --git a/README.md b/README.md index 43ccff2..60d420f 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,11 @@ form to its usual endpoint (or any endpoint that will submit the form), take the HTML response, extract the parts that need to be re-rendered and then replace the HTML on the page. +Make sure to also replace the hidden `__dynamic_state` field: it holds the values +of the dependencies as they were rendered. If it keeps the state of the first +render, the validation errors of the dependent fields will be hidden again on +the next submit. + This is a non-trivial task and there may be room for improvement in this library to make this easier. If you have ideas, please open an issue! diff --git a/composer.json b/composer.json index 29c3962..0a36f65 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,8 @@ "symfony/twig-bundle": "^5.4|^6.4|^7.0|^8.0", "twig/twig": "^2.15|^3.0", "symfony/options-resolver": "^5.4|^6.4|^7.0|^8.0", - "phpunit/phpunit": "^9.6" + "phpunit/phpunit": "^9.6", + "symfony/validator": "^5.4|^6.4|^7.0|^8.0" }, "minimum-stability": "dev", "prefer-stable": true, diff --git a/src/DynamicFormBuilder.php b/src/DynamicFormBuilder.php index adf2a73..7fa5381 100644 --- a/src/DynamicFormBuilder.php +++ b/src/DynamicFormBuilder.php @@ -46,11 +46,20 @@ class DynamicFormBuilder implements FormBuilderInterface, \IteratorAggregate private array $preSetDataDependencyData = []; private array $postSubmitDependencyData = []; + /** + * Names of the dependencies whose submitted value differs from the value + * rendered on the previous request. + * + * @var string[] + */ + private array $changedDependencies = []; + public function __construct(private FormBuilderInterface $builder) { $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) { $this->form = $event->getForm(); $this->preSetDataDependencyData = []; + $this->changedDependencies = []; $this->initializeListeners(); // A fake hidden field where we can "store" an error if a dependent form @@ -66,14 +75,32 @@ public function __construct(private FormBuilderInterface $builder) 'error_bubbling' => false, ]); } + + // A fake hidden field carrying the values of the dependencies as + // they were rendered, so the next submit can tell which of them + // the user actually changed since the previous render. + if (!$this->form->has('__dynamic_state')) { + $this->form->add('__dynamic_state', HiddenType::class, [ + 'mapped' => false, + ]); + } }, 100); + $builder->addEventListener(FormEvents::POST_SET_DATA, function (FormEvent $event) { + $this->storeRenderedDependencyState($event); + }); + + $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) { + $this->detectChangedDependencies($event); + }); + $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) { $this->postSubmitDependencyData = []; }); // guarantee later than core ValidationListener $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) { $this->clearDataOnTransformationError($event); + $this->clearErrorsOnEmptiedDependentFields($event); }, -1); } @@ -123,10 +150,151 @@ public function clearDataOnTransformationError(FormEvent $event): void // We need to make sure that the form doesn't submit successfully, // but we also don't want to render a validation error on any field. // So, we jam the error into a hidden field, which doesn't render errors. - if ($form->get('__dynamic_error')->isValid()) { - $form->get('__dynamic_error')->addError(new FormError('Some dynamic fields have errors.')); + $this->addHiddenError($form); + } + } + + /** + * Hides the validation errors of a dependent field that was emptied because + * a field it depends on just changed (e.g. user selected "Pizza" as the main + * food, then changed "Meal" back and forth: "mainFood" is re-rendered empty, + * so a NotBlank-style violation on it is only noise). Like for transformation + * failures above, the error is moved to the hidden field: nothing is + * rendered, but the form remains invalid. + */ + private function clearErrorsOnEmptiedDependentFields(FormEvent $event): void + { + if (!$this->changedDependencies) { + return; + } + + $form = $event->getForm(); + $errorsCleared = false; + foreach ($this->dependentFieldConfigs as $dependentFieldConfig) { + if (!array_intersect($dependentFieldConfig->dependencies, $this->changedDependencies)) { + continue; + } + + if (!$form->has($dependentFieldConfig->name)) { + continue; + } + + $subForm = $form->get($dependentFieldConfig->name); + if (!$subForm instanceof ClearableErrorsInterface || !$subForm->isEmpty()) { + continue; + } + + if (0 === \count($subForm->getErrors(false))) { + continue; + } + + $subForm->clearErrors(); + $errorsCleared = true; + } + + if ($errorsCleared) { + $this->addHiddenError($form); + } + } + + private function storeRenderedDependencyState(FormEvent $event): void + { + $form = $event->getForm(); + $state = []; + foreach ($this->getDependencyNames() as $dependency) { + if ($form->has($dependency)) { + $state[$dependency] = $this->normalizeDependencyValue($form->get($dependency)->getViewData()); } } + + $form->get('__dynamic_state')->setData(json_encode($state)); + } + + /** + * Compares the submitted dependency values with the ones rendered on the + * previous request, then replaces the submitted state with the current + * values, so the next render carries them. + * + * Without a usable state (e.g. the field was not rendered), no dependency + * is considered changed: rendering an extra error is safer than silently + * blocking the submit. + */ + private function detectChangedDependencies(FormEvent $event): void + { + $this->changedDependencies = []; + + $data = $event->getData(); + if (!\is_array($data)) { + return; + } + + $previousState = \is_string($data['__dynamic_state'] ?? null) ? json_decode($data['__dynamic_state'], true) : null; + + $currentState = []; + foreach ($this->getDependencyNames() as $dependency) { + if (!$event->getForm()->has($dependency)) { + continue; + } + + // browsers do not submit disabled fields, and Symfony ignores + // their submitted data anyway: such a dependency cannot change + $dependencyForm = $event->getForm()->get($dependency); + if ($dependencyForm->isDisabled()) { + $currentState[$dependency] = $this->normalizeDependencyValue($dependencyForm->getViewData()); + + continue; + } + + $currentState[$dependency] = $this->normalizeDependencyValue($data[$dependency] ?? null); + + if (\is_array($previousState) + && \array_key_exists($dependency, $previousState) + && $this->normalizeDependencyValue($previousState[$dependency]) !== $currentState[$dependency] + ) { + $this->changedDependencies[] = $dependency; + } + } + + $data['__dynamic_state'] = json_encode($currentState); + $event->setData($data); + } + + /** + * @return string[] + */ + private function getDependencyNames(): array + { + $dependencies = []; + foreach ($this->dependentFieldConfigs as $dependentFieldConfig) { + $dependencies = [...$dependencies, ...$dependentFieldConfig->dependencies]; + } + + return array_values(array_unique($dependencies)); + } + + /** + * Makes a rendered view value comparable with a raw submitted one: a + * missing or null value is submitted as an empty string, and so is a + * multiple choice with nothing selected, which browsers omit entirely. + */ + private function normalizeDependencyValue(mixed $value): mixed + { + if (null === $value || [] === $value) { + return ''; + } + + if (\is_array($value)) { + return array_map(fn (mixed $item): mixed => $this->normalizeDependencyValue($item), $value); + } + + return \is_scalar($value) ? (string) $value : $value; + } + + private function addHiddenError(FormInterface $form): void + { + if ($form->get('__dynamic_error')->isValid()) { + $form->get('__dynamic_error')->addError(new FormError('Some dynamic fields have errors.')); + } } private function executeReadyCallbacks(array $availableDependencyData, string $eventName): void diff --git a/tests/ConstrainedDependentFieldTest.php b/tests/ConstrainedDependentFieldTest.php new file mode 100644 index 0000000..b4204e8 --- /dev/null +++ b/tests/ConstrainedDependentFieldTest.php @@ -0,0 +1,161 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfonycasts\DynamicForms\Tests; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\Form\Extension\Core\Type\ChoiceType; +use Symfony\Component\Form\Extension\Core\Type\FormType; +use Symfony\Component\Form\Extension\Core\Type\TextType; +use Symfony\Component\Form\FormInterface; +use Symfony\Component\Validator\Constraints\NotBlank; +use Symfonycasts\DynamicForms\DependentField; +use Symfonycasts\DynamicForms\DynamicFormBuilder; +use Symfonycasts\DynamicForms\Tests\fixtures\DynamicFormsTestKernel; +use Symfonycasts\DynamicForms\Tests\fixtures\Enum\DynamicTestMeal; +use Symfonycasts\DynamicForms\Tests\fixtures\TestConstrainedDynamicForm; +use Zenstruck\Browser\Test\HasBrowser; + +/** + * A dependent field is emptied by the UI whenever one of its dependencies + * changes. Its validation errors (e.g. NotBlank) must not be rendered in + * that case, in the same way transformation failures are already hidden. + * + * @see https://github.com/SymfonyCasts/dynamic-forms/issues/50 + */ +class ConstrainedDependentFieldTest extends KernelTestCase +{ + use HasBrowser; + + public function testNoErrorIsRenderedWhenReturningToThePreviousOption(): void + { + $browser = $this->browser(); + // pick Dinner, then Pizza + $browser->visit('/constrained-form') + ->selectFieldOption('Meal', 'Dinner') + ->click('Submit Form') + ->selectFieldOption('Main food', 'Pizza') + ->click('Submit Form') + ->assertSee('Is Form Valid: yes') + ; + + // change the meal: the submitted "pizza" is not a valid choice anymore, + // the transformation failure is hidden but the form is invalid + $browser->selectFieldOption('Meal', 'Breakfast') + ->click('Submit Form') + ->assertSee('Is Form Valid: no') + ->assertNotSee('This food is not served at this meal.') + ; + + // change the meal back: "mainFood" is submitted empty, its NotBlank + // violation must be hidden too, but the form must remain invalid + $browser->selectFieldOption('Meal', 'Dinner') + ->click('Submit Form') + ->assertSee('Is Form Valid: no') + ->assertNotSee('This value should not be blank.') + ; + + // selecting a food again makes the form valid + $browser->selectFieldOption('Main food', 'Pasta') + ->click('Submit Form') + ->assertSee('Is Form Valid: yes') + ; + } + + public function testErrorIsRenderedWhenTheDependencyDidNotChange(): void + { + $browser = $this->browser(); + // "Breakfast" is the initial data: submitting it unchanged with an + // empty mainFood is a real mistake, the NotBlank violation must render + $browser->visit('/constrained-form') + ->click('Submit Form') + ->assertSee('Is Form Valid: no') + ->assertSee('This value should not be blank.') + ; + } + + public function testErrorIsRenderedOnSubmitAfterDependencyChangedInPreviousRender(): void + { + $browser = $this->browser() + ->visit('/constrained-form') + ->selectFieldOption('Meal', 'Dinner') + ->click('Submit Form') // re-render after the dependency changed: fine to hide + ->assertNotSee('This value should not be blank.') + ; + + // Meal is still "Dinner", the user submits without picking a food + $browser->click('Submit Form') + ->assertSee('Is Form Valid: no') + ->assertSee('This value should not be blank.') + ; + } + + public function testErrorIsRenderedWhenTheRenderedStateIsMissing(): void + { + $form = self::getContainer()->get('form.factory')->create(TestConstrainedDynamicForm::class, [ + 'meal' => DynamicTestMeal::Breakfast, + ]); + + // "meal" changed, but without the rendered state there is no way to + // know it: the error must not be hidden + $form->submit(['meal' => 'dinner', 'mainFood' => '']); + + $this->assertFalse($form->isValid()); + $this->assertCount(1, $form->get('mainFood')->getErrors()); + } + + public function testErrorIsRenderedWhenADisabledDependencyIsNotSubmitted(): void + { + $depOptions = ['choices' => ['A' => 'a', 'B' => 'b'], 'disabled' => true]; + $renderedState = $this->renderDynamicState('b', $depOptions); + + // browsers do not submit disabled fields, Symfony keeps "b" + $form = $this->createFormWithDependency('b', $depOptions); + $form->submit(['notes' => '', '__dynamic_state' => $renderedState]); + + $this->assertFalse($form->isValid()); + $this->assertCount(1, $form->get('notes')->getErrors()); + } + + public function testErrorIsRenderedWhenAnEmptyMultipleDependencyIsNotSubmitted(): void + { + $depOptions = ['choices' => ['A' => 'a', 'B' => 'b'], 'multiple' => true]; + $renderedState = $this->renderDynamicState([], $depOptions); + + // browsers omit a multiple choice field with nothing selected + $form = $this->createFormWithDependency([], $depOptions); + $form->submit(['notes' => '', '__dynamic_state' => $renderedState]); + + $this->assertFalse($form->isValid()); + $this->assertCount(1, $form->get('notes')->getErrors()); + } + + private function renderDynamicState(mixed $depData, array $depOptions): string + { + return $this->createFormWithDependency($depData, $depOptions)->createView()['__dynamic_state']->vars['value']; + } + + private function createFormWithDependency(mixed $depData, array $depOptions): FormInterface + { + $builder = new DynamicFormBuilder( + self::getContainer()->get('form.factory')->createBuilder(FormType::class, ['dep' => $depData]) + ); + $builder->add('dep', ChoiceType::class, $depOptions); + $builder->addDependent('notes', ['dep'], static function (DependentField $field) { + $field->add(TextType::class, ['constraints' => [new NotBlank()]]); + }); + + return $builder->getForm(); + } + + protected static function getKernelClass(): string + { + return DynamicFormsTestKernel::class; + } +} diff --git a/tests/fixtures/DynamicFormsTestKernel.php b/tests/fixtures/DynamicFormsTestKernel.php index 70ebc59..3405b85 100644 --- a/tests/fixtures/DynamicFormsTestKernel.php +++ b/tests/fixtures/DynamicFormsTestKernel.php @@ -40,6 +40,19 @@ public function form(Environment $twig, FormFactoryInterface $formFactory, Reque ])); } + public function constrainedForm(Environment $twig, FormFactoryInterface $formFactory, Request $request): Response + { + $form = $formFactory->create(TestConstrainedDynamicForm::class, [ + 'meal' => DynamicTestMeal::Breakfast, + ]); + $form->handleRequest($request); + + return new Response($twig->render('form.html.twig', [ + 'form' => $form->createView(), + 'isFormValid' => $form->isSubmitted() && $form->isValid(), + ])); + } + public function registerBundles(): iterable { return [ @@ -83,5 +96,6 @@ protected function build(ContainerBuilder $container): void protected function configureRoutes(RoutingConfigurator $routes): void { $routes->add('form', '/form')->controller('kernel::form'); + $routes->add('constrained_form', '/constrained-form')->controller('kernel::constrainedForm'); } } diff --git a/tests/fixtures/TestConstrainedDynamicForm.php b/tests/fixtures/TestConstrainedDynamicForm.php new file mode 100644 index 0000000..fd20e37 --- /dev/null +++ b/tests/fixtures/TestConstrainedDynamicForm.php @@ -0,0 +1,45 @@ + + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfonycasts\DynamicForms\Tests\fixtures; + +use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\Extension\Core\Type\EnumType; +use Symfony\Component\Form\FormBuilderInterface; +use Symfony\Component\Validator\Constraints\NotBlank; +use Symfonycasts\DynamicForms\DependentField; +use Symfonycasts\DynamicForms\DynamicFormBuilder; +use Symfonycasts\DynamicForms\Tests\fixtures\Enum\DynamicTestFood; +use Symfonycasts\DynamicForms\Tests\fixtures\Enum\DynamicTestMeal; + +class TestConstrainedDynamicForm extends AbstractType +{ + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $builder = new DynamicFormBuilder($builder); + + $builder->add('meal', EnumType::class, [ + 'class' => DynamicTestMeal::class, + 'choice_label' => static fn (DynamicTestMeal $meal): string => $meal->getReadable(), + 'placeholder' => 'Which meal is it?', + ]); + + $builder->addDependent('mainFood', ['meal'], static function (DependentField $field, ?DynamicTestMeal $meal) { + $field->add(EnumType::class, [ + 'class' => DynamicTestFood::class, + 'placeholder' => null === $meal ? 'Select a meal first' : \sprintf('What is for %s?', $meal->getReadable()), + 'choices' => $meal?->getFoodChoices(), + 'choice_label' => static fn (DynamicTestFood $food): string => $food->getReadable(), + 'disabled' => null === $meal, + 'invalid_message' => 'This food is not served at this meal.', + 'constraints' => [new NotBlank()], + ]); + }); + } +}