Recursive dependent fields fix

.php-cs-fixer.dist.php

@@ -6,6 +6,7 @@
 
 $finder = (new \PhpCsFixer\Finder())
     ->in([__DIR__.'/src', __DIR__.'/tests'])
+    ->exclude('tests/fixtures/var')
 ;
 
 return (new \PhpCsFixer\Config())

composer.json

@@ -21,7 +21,9 @@
         "symfony/twig-bundle": "^5.4|^6.3|^7.0",
         "twig/twig": "^2.15|^3.0",
         "symfony/options-resolver": "^5.4|^6.3|^7.0",
-        "phpunit/phpunit": "^9.6"
+        "phpunit/phpunit": "^9.6",
+        "symfony/panther": "^2.2",
+        "dbrekelmans/bdi": "dev-main"
     },
     "minimum-stability": "dev",
     "autoload": {

phpunit.xml.dist

@@ -17,6 +17,12 @@
         <server name="SYMFONY_PHPUNIT_REMOVE" value="" />
         <server name="SYMFONY_PHPUNIT_VERSION" value="9.6" />
         <server name="SYMFONY_DEPRECATIONS_HELPER" value="max[indirect]=11" />
+        <server name="PANTHER_APP_ENV" value="1" force="true" />
+        <server name="PANTHER_WEB_SERVER_DIR" value="./tests/fixtures/public" force="true" />
+        <server name="BROWSER_SOURCE_DIR" value="./tests/fixtures/var/browser/source" force="true" />
+        <server name="BROWSER_SCREENSHOT_DIR" value="./tests/fixtures/var/browser/screenshots" force="true" />
+        <server name="BROWSER_CONSOLE_LOG_DIR" value="./tests/fixtures/var/browser/console-logs" force="true" />
+        <server name="PANTHER_NO_HEADLESS" value="1" force="true" />
     </php>
 
     <testsuites>
@@ -34,4 +40,8 @@
     <listeners>
         <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener" />
     </listeners>
+
+    <extensions>
+        <extension class="Zenstruck\Browser\Test\BrowserExtension" />
+    </extensions>
 </phpunit>

src/DynamicFormBuilder.php

@@ -130,24 +130,101 @@ public function clearDataOnTransformationError(FormEvent $event): void
     }
 
     private function executeReadyCallbacks(array $availableDependencyData, string $eventName): void
+    {
+        $hasChanges = true;
+        $maxIterations = 10;
+        $iteration = 0;
+
+        while ($hasChanges && $iteration < $maxIterations) {
+            $hasChanges = false;
+            ++$iteration;
+
+            // First pass: handle removals and reset dependent callbacks
+            foreach ($this->dependentFieldConfigs as $dependentFieldConfig) {
+                if ($dependentFieldConfig->isReady($availableDependencyData, $eventName)) {
+                    $dynamicField = $dependentFieldConfig->execute($availableDependencyData, $eventName);
+                    $name = $dependentFieldConfig->name;
+                    $fieldExisted = $this->form->has($name);
+
+                    if (!$dynamicField->shouldBeAdded()) {
+                        if ($fieldExisted) {
+                            $this->form->remove($name);
+                            $hasChanges = true;
+
+                            // Reset callbacks for fields that depend on this removed field
+                            $this->resetDependentCallbacks($name, $eventName);
+                        }
+                        continue;
+                    }
+
+                    // Field should be added - handle both new and existing fields
+                    if ($fieldExisted) {
+                        // Field exists but may need to be updated with new options
+                        // Remove and re-add to ensure proper configuration
+                        $this->form->remove($name);
+                        $hasChanges = true;
+                    }
+
+                    // Add/re-add the field with current configuration
+                    $this->builder->add($name, $dynamicField->getType(), $dynamicField->getOptions());
+                    $this->initializeListeners([$name]);
+                    // auto initialize mimics FormBuilder::getForm() behavior
+                    $field = $this->builder->get($name)->setAutoInitialize(false)->getForm();
+                    $this->form->add($field);
+
+                    if (!$fieldExisted) {
+                        $hasChanges = true;
+                    }
+                }
+            }
+
+            // If we had changes, we need to re-evaluate all dependencies that might now be invalid
+            if ($hasChanges) {
+                $this->validateAndRemoveOrphanedFields($eventName);
+            }
+        }
+    }
+
+    /**
+     * Remove fields that should no longer exist because their dependencies are missing.
+     */
+    private function validateAndRemoveOrphanedFields(string $eventName): void
     {
         foreach ($this->dependentFieldConfigs as $dependentFieldConfig) {
-            if ($dependentFieldConfig->isReady($availableDependencyData, $eventName)) {
-                $dynamicField = $dependentFieldConfig->execute($availableDependencyData, $eventName);
-                $name = $dependentFieldConfig->name;
+            $name = $dependentFieldConfig->name;
+
+            // If the field exists in the form, check if it should still exist
+            if ($this->form->has($name)) {
+                $hasAllDependencies = true;
+
+                foreach ($dependentFieldConfig->dependencies as $dependency) {
+                    // Check if the dependency field exists and has appropriate data
+                    if (!$this->form->has($dependency)) {
+                        $hasAllDependencies = false;
+                        break;
+                    }
+                }
 
-                if (!$dynamicField->shouldBeAdded()) {
+                // If dependencies are missing, remove the field and reset its callback
+                if (!$hasAllDependencies) {
                     $this->form->remove($name);
+                    $dependentFieldConfig->callbackExecuted[$eventName] = false;
 
-                    continue;
+                    // Reset callbacks for fields that depend on this removed field
+                    $this->resetDependentCallbacks($name, $eventName);
                 }
+            }
+        }
+    }
 
-                $this->builder->add($name, $dynamicField->getType(), $dynamicField->getOptions());
-
-                $this->initializeListeners([$name]);
-                // auto initialize mimics FormBuilder::getForm() behavior
-                $field = $this->builder->get($name)->setAutoInitialize(false)->getForm();
-                $this->form->add($field);
+    /**
+     * Reset callback execution status for fields that depend on a removed field.
+     */
+    private function resetDependentCallbacks(string $removedFieldName, string $eventName): void
+    {
+        foreach ($this->dependentFieldConfigs as $dependentFieldConfig) {
+            if (\in_array($removedFieldName, $dependentFieldConfig->dependencies)) {
+                $dependentFieldConfig->callbackExecuted[$eventName] = false;
             }
         }
     }

tests/E2ETest.php

@@ -0,0 +1,62 @@
+<?php
+
+/*
+ * This file is part of the SymfonyCasts DynamicForms package.
+ * Copyright (c) SymfonyCasts <https://symfonycasts.com/>
+ * 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\Component\Panther\PantherTestCase;
+use Symfonycasts\DynamicForms\Tests\fixtures\DynamicFormsTestKernel;
+use Zenstruck\Browser\Test\HasBrowser;
+
+class E2ETest extends PantherTestCase
+{
+    use HasBrowser;
+
+    public function testRecursiveDynamicFields()
+    {
+        $browser = $this->pantherBrowser();
+        $browser->visit('/form-pizza-selected')
+            // check for the hidden field
+            ->waitUntilSeeIn('//html', 'Is Form Valid: no')
+            ->assertSeeElement('#test_dynamic_form___dynamic_error')
+            ->assertSee('Pizza 🍕')
+            ->assertNotContains('<option value="bacon">')
+            ->assertContains('<option value="pizza" selected="selected">')
+            ->assertContains('What size pizza?')
+        ;
+
+        // now change the meal to breakfast
+        $browser->selectFieldOption('Meal', 'Breakfast')
+            ->click('Submit Form')
+            // form is not valid: the mainFood submitted an invalid value
+            ->waitUntilSeeIn('//html', 'Is Form Valid: no')
+            ->assertContains('<option value="bacon">')
+            ->assertNotContains('<option value="pizza"')
+            ->assertNotContains('What size pizza?')
+        ;
+
+        // select a valid food for breakfast
+        $browser->selectFieldOption('Main food', 'Bacon')
+            ->click('Submit Form')
+            // form is valid again
+            ->waitUntilSeeIn('//html', 'Is Form Valid: yes')
+        ;
+
+        // change the meal again
+        $browser->selectFieldOption('Meal', 'Lunch')
+            ->click('Submit Form')
+            // form is not valid: the mainFood=bacon is invalid for lunch
+            ->waitUntilSeeIn('//html', 'Is Form Valid: no')
+        ;
+    }
+
+    protected static function getKernelClass(): string
+    {
+        return DynamicFormsTestKernel::class;
+    }
+}

tests/fixtures/DynamicFormsTestKernel.php

@@ -20,6 +20,7 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
+use Symfonycasts\DynamicForms\Tests\fixtures\Enum\DynamicTestFood;
 use Symfonycasts\DynamicForms\Tests\fixtures\Enum\DynamicTestMeal;
 use Twig\Environment;
 
@@ -40,6 +41,23 @@ public function form(Environment $twig, FormFactoryInterface $formFactory, Reque
         ]));
     }
 
+    /**
+     * Verify that recursive dependencies are checked.
+     */
+    public function formPizzaSelected(Environment $twig, FormFactoryInterface $formFactory, Request $request): Response
+    {
+        $form = $formFactory->create(TestDynamicForm::class, [
+            'meal' => DynamicTestMeal::Dinner,
+            'mainFood' => DynamicTestFood::Pizza,
+        ]);
+        $form->handleRequest($request);
+
+        return new Response($twig->render('form.html.twig', [
+            'form' => $form->createView(),
+            'isFormValid' => $form->isSubmitted() && $form->isValid(),
+        ]));
+    }
+
     public function registerBundles(): iterable
     {
         return [
@@ -78,5 +96,6 @@ protected function build(ContainerBuilder $container): void
     protected function configureRoutes(RoutingConfigurator $routes): void
     {
         $routes->add('form', '/form')->controller('kernel::form');
+        $routes->add('form-pizza-selected', '/form-pizza-selected')->controller('kernel::formPizzaSelected');
     }
 }

tests/fixtures/public/index.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the SymfonyCasts DynamicForms package.
+ * Copyright (c) SymfonyCasts <https://symfonycasts.com/>
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfonycasts\DynamicForms\Tests\fixtures\DynamicFormsTestKernel;
+
+require dirname(__DIR__, 3).'/vendor/autoload.php';
+
+$kernel = new DynamicFormsTestKernel($_SERVER['APP_ENV'] ?? 'dev', true);
+$kernel->boot();
+
+$request = Request::createFromGlobals();
+
+$response = $kernel->handle($request);
+$response->send();
+
+$kernel->terminate($request, $response);

tests/fixtures/templates/form.html.twig

@@ -1,6 +1,6 @@
 Is Form Valid: {{ isFormValid ? 'yes' : 'no' }}
 
-{{ form_start(form) }}
+{{ form_start(form, {attr: {'novalidate': ''}}) }}
     {{ form_widget(form) }}
 
     <button type="submit">Submit Form</button>