From dedee2926d4dee3cf4da9deeb11d9fe5e7b9e8f5 Mon Sep 17 00:00:00 2001 From: Damien ALEXANDRE Date: Tue, 18 Aug 2026 17:42:02 +0200 Subject: [PATCH 1/3] fix(queue): Fix js_validation option inheritance from global config - Change FormExtension default for js_validation from true to null - SubscriberToQueue now treats null as "inherit from global" - This allows global js_validation: false to actually disable validation unless explicitly enabled per form type - Add comprehensive tests for all combination cases Fixes #56 --- Tests/Unit/SubscriberToQueueTest.php | 104 ++++++++++++++++++++++ src/Factory/JsFormValidatorFactory.php | 3 +- src/Form/Extension/FormExtension.php | 2 +- src/Form/Subscriber/SubscriberToQueue.php | 9 +- 4 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 Tests/Unit/SubscriberToQueueTest.php diff --git a/Tests/Unit/SubscriberToQueueTest.php b/Tests/Unit/SubscriberToQueueTest.php new file mode 100644 index 00000000..925ed01e --- /dev/null +++ b/Tests/Unit/SubscriberToQueueTest.php @@ -0,0 +1,104 @@ +createFactory(array('js_validation' => false)); + $formFactory = $this->createFormFactory($factory); + $subscriber = new SubscriberToQueue($factory); + + $form = $formFactory + ->createNamedBuilder('test_form', FormType::class, null, array('js_validation' => true)) + ->getForm() + ; + + $subscriber->onFormSetData(new FormEvent($form, null)); + + $this->assertTrue($factory->inQueue($form)); + } + + public function testDoesNotAddToQueueWhenGlobalDisabledAndLocalNotSet() + { + $factory = $this->createFactory(array('js_validation' => false)); + $formFactory = $this->createFormFactory($factory); + $subscriber = new SubscriberToQueue($factory); + + // Form with no explicit js_validation option (defaults to null) + $form = $formFactory + ->createNamedBuilder('test_form', FormType::class) + ->getForm() + ; + + $subscriber->onFormSetData(new FormEvent($form, null)); + + $this->assertFalse($factory->inQueue($form)); + } + + public function testAddToQueueWhenGlobalEnabledAndLocalNotSet() + { + $factory = $this->createFactory(array('js_validation' => true)); + $formFactory = $this->createFormFactory($factory); + $subscriber = new SubscriberToQueue($factory); + + // Form with no explicit js_validation option (defaults to null, inherits global) + $form = $formFactory + ->createNamedBuilder('test_form', FormType::class) + ->getForm() + ; + + $subscriber->onFormSetData(new FormEvent($form, null)); + + $this->assertTrue($factory->inQueue($form)); + } + + public function testDoesNotAddToQueueWhenLocalExplicitlyDisabled() + { + $factory = $this->createFactory(array('js_validation' => true)); + $formFactory = $this->createFormFactory($factory); + $subscriber = new SubscriberToQueue($factory); + + $form = $formFactory + ->createNamedBuilder('test_form', FormType::class, null, array('js_validation' => false)) + ->getForm() + ; + + $subscriber->onFormSetData(new FormEvent($form, null)); + + $this->assertFalse($factory->inQueue($form)); + } + + private function createFactory(array $config = array()) + { + return new JsFormValidatorFactory( + Validation::createValidator(), + $this->createStub(TranslatorInterface::class), + $this->createStub(UrlGeneratorInterface::class), + $config, + 'validators' + ); + } + + private function createFormFactory(JsFormValidatorFactory $factory) + { + return Forms::createFormFactoryBuilder() + ->addExtension(new ValidatorExtension(Validation::createValidator())) + ->addTypeExtension(new FormExtension($factory)) + ->getFormFactory() + ; + } +} diff --git a/src/Factory/JsFormValidatorFactory.php b/src/Factory/JsFormValidatorFactory.php index 48d345e2..cd72bee5 100755 --- a/src/Factory/JsFormValidatorFactory.php +++ b/src/Factory/JsFormValidatorFactory.php @@ -235,7 +235,8 @@ public function createJsModel(FormInterface $form) $this->currentElement = $form; $conf = $form->getConfig(); - // If field is disabled or has no any validations + // If field is explicitly disabled, skip it + // null means "inherit" which is treated as enabled (same as true) if (false === $conf->getOption('js_validation')) { return null; } diff --git a/src/Form/Extension/FormExtension.php b/src/Form/Extension/FormExtension.php index 5545ee28..02e217b0 100644 --- a/src/Form/Extension/FormExtension.php +++ b/src/Form/Extension/FormExtension.php @@ -42,7 +42,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void */ public function configureOptions(OptionsResolver $resolver): void { - $resolver->setDefaults(array('js_validation' => true)); + $resolver->setDefaults(array('js_validation' => null)); } /** diff --git a/src/Form/Subscriber/SubscriberToQueue.php b/src/Form/Subscriber/SubscriberToQueue.php index 3edc5e19..4f2d1cdf 100755 --- a/src/Form/Subscriber/SubscriberToQueue.php +++ b/src/Form/Subscriber/SubscriberToQueue.php @@ -44,11 +44,14 @@ public function onFormSetData(FormEvent $event): void $globalSwitch = $this->factory->getConfig('js_validation'); $localSwitch = $form->getConfig()->getOption('js_validation'); - // Add only parent forms which are not disabled - if ($globalSwitch && $localSwitch) { + // If local option is null (not explicitly set), inherit from global + $enabled = null === $localSwitch ? $globalSwitch : $localSwitch; + + // Add only parent forms which are enabled + if ($enabled) { $parent = $this->getParent($form); if (!$this->factory->inQueue($parent)) { - $this->factory->addToQueue($this->getParent($form)); + $this->factory->addToQueue($parent); } } } From 2de2e65913f49c1bead564df5e2a501dd61dbb58 Mon Sep 17 00:00:00 2001 From: Damien ALEXANDRE Date: Tue, 18 Aug 2026 17:52:21 +0200 Subject: [PATCH 2/3] chore(tests): refactor the factory thing / use a Trait --- Tests/Unit/FactoryTestTrait.php | 61 +++++++++++++++++++++ Tests/Unit/JsFormValidatorFactoryTest.php | 65 ++++------------------- Tests/Unit/SubscriberToQueueTest.php | 34 ++---------- 3 files changed, 75 insertions(+), 85 deletions(-) create mode 100644 Tests/Unit/FactoryTestTrait.php diff --git a/Tests/Unit/FactoryTestTrait.php b/Tests/Unit/FactoryTestTrait.php new file mode 100644 index 00000000..c366ce8e --- /dev/null +++ b/Tests/Unit/FactoryTestTrait.php @@ -0,0 +1,61 @@ + true) + ): JsFormValidatorFactory { + if (!$router) { + $router = $this->createStub(UrlGeneratorInterface::class); + $router + ->method('generate') + ->willReturn('/generated-route') + ; + } + + return new JsFormValidatorFactory( + $validator ?: Validation::createValidator(), + new IdentityTranslator(), + $router, + $config, + 'validators' + ); + } + + private function createFormFactory(JsFormValidatorFactory $factory, ?ValidatorInterface $validator = null) + { + $validator = $validator ?: Validation::createValidator(); + + return Forms::createFormFactoryBuilder() + ->addExtension(new ValidatorExtension($validator)) + ->addTypeExtension(new FormExtension($factory)) + ->getFormFactory() + ; + } +} + +class IdentityTranslator implements TranslatorInterface +{ + public function trans(string $id, array $parameters = array(), ?string $domain = null, ?string $locale = null): string + { + return strtr($id, $parameters); + } + + public function getLocale(): string + { + return 'en'; + } +} diff --git a/Tests/Unit/JsFormValidatorFactoryTest.php b/Tests/Unit/JsFormValidatorFactoryTest.php index ccfaa7a7..6318996f 100644 --- a/Tests/Unit/JsFormValidatorFactoryTest.php +++ b/Tests/Unit/JsFormValidatorFactoryTest.php @@ -2,25 +2,19 @@ namespace Fp\JsFormValidatorBundle\Tests\Unit; -use Fp\JsFormValidatorBundle\Factory\JsFormValidatorFactory; -use Fp\JsFormValidatorBundle\Form\Extension\FormExtension; use Fp\JsFormValidatorBundle\Form\Constraint\UniqueEntity as JsUniqueEntity; use PHPUnit\Framework\TestCase; use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity as SymfonyUniqueEntity; use Symfony\Component\Form\Extension\Core\Type\FormType; use Symfony\Component\Form\Extension\Core\Type\TextType; -use Symfony\Component\Form\Extension\Validator\ValidatorExtension; -use Symfony\Component\Form\Forms; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Validator\Constraints\NotBlank; -use Symfony\Component\Validator\Validation; -use Symfony\Contracts\Translation\TranslatorInterface; class JsFormValidatorFactoryTest extends TestCase { + use FactoryTestTrait; public function testCreatesModelFromModernSymfonyForm() { - $validator = Validation::createValidator(); $router = $this->createMock(UrlGeneratorInterface::class); $router ->method('generate') @@ -28,24 +22,13 @@ public function testCreatesModelFromModernSymfonyForm() ->willReturn('/fp_js_form_validator/check_unique_entity') ; - $factory = new JsFormValidatorFactory( - $validator, - new IdentityTranslator(), - $router, - array( - 'js_validation' => true, - 'routing' => array( - 'check_unique_entity' => 'fp_js_form_validator.check_unique_entity', - ), + $factory = $this->createFactory(null, $router, array( + 'js_validation' => true, + 'routing' => array( + 'check_unique_entity' => 'fp_js_form_validator.check_unique_entity', ), - 'validators' - ); - - $formFactory = Forms::createFormFactoryBuilder() - ->addExtension(new ValidatorExtension($validator)) - ->addTypeExtension(new FormExtension($factory)) - ->getFormFactory() - ; + )); + $formFactory = $this->createFormFactory($factory); $form = $formFactory ->createBuilder(FormType::class, null, array('validation_groups' => array('Default'))) @@ -68,21 +51,8 @@ public function testCreatesModelFromModernSymfonyForm() public function testUniqueEntityConstraintIncludesBoundEntityId() { - $validator = Validation::createValidator(); - $router = $this->createMock(UrlGeneratorInterface::class); - $factory = new JsFormValidatorFactory( - $validator, - new IdentityTranslator(), - $router, - array('js_validation' => true), - 'validators' - ); - - $formFactory = Forms::createFormFactoryBuilder() - ->addExtension(new ValidatorExtension($validator)) - ->addTypeExtension(new FormExtension($factory)) - ->getFormFactory() - ; + $factory = $this->createFactory(); + $formFactory = $this->createFormFactory($factory); $form = $formFactory ->createBuilder( @@ -106,23 +76,6 @@ public function testUniqueEntityConstraintIncludesBoundEntityId() } } -class IdentityTranslator implements TranslatorInterface -{ - public function trans( - string $id, - array $parameters = array(), - ?string $domain = null, - ?string $locale = null - ): string { - return strtr($id, $parameters); - } - - public function getLocale(): string - { - return 'en'; - } -} - class UniqueEntityUser { public $email; diff --git a/Tests/Unit/SubscriberToQueueTest.php b/Tests/Unit/SubscriberToQueueTest.php index 925ed01e..e54ea657 100644 --- a/Tests/Unit/SubscriberToQueueTest.php +++ b/Tests/Unit/SubscriberToQueueTest.php @@ -3,22 +3,18 @@ namespace Fp\JsFormValidatorBundle\Tests\Unit; use Fp\JsFormValidatorBundle\Factory\JsFormValidatorFactory; -use Fp\JsFormValidatorBundle\Form\Extension\FormExtension; use Fp\JsFormValidatorBundle\Form\Subscriber\SubscriberToQueue; use PHPUnit\Framework\TestCase; use Symfony\Component\Form\Extension\Core\Type\FormType; -use Symfony\Component\Form\Extension\Validator\ValidatorExtension; use Symfony\Component\Form\FormEvent; -use Symfony\Component\Form\Forms; -use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -use Symfony\Component\Validator\Validation; use Symfony\Contracts\Translation\TranslatorInterface; class SubscriberToQueueTest extends TestCase { + use FactoryTestTrait; public function testAddToQueueWhenGlobalDisabledButLocalExplicitlyEnabled() { - $factory = $this->createFactory(array('js_validation' => false)); + $factory = $this->createFactory(null, null, array('js_validation' => false)); $formFactory = $this->createFormFactory($factory); $subscriber = new SubscriberToQueue($factory); @@ -34,7 +30,7 @@ public function testAddToQueueWhenGlobalDisabledButLocalExplicitlyEnabled() public function testDoesNotAddToQueueWhenGlobalDisabledAndLocalNotSet() { - $factory = $this->createFactory(array('js_validation' => false)); + $factory = $this->createFactory(null, null, array('js_validation' => false)); $formFactory = $this->createFormFactory($factory); $subscriber = new SubscriberToQueue($factory); @@ -51,7 +47,7 @@ public function testDoesNotAddToQueueWhenGlobalDisabledAndLocalNotSet() public function testAddToQueueWhenGlobalEnabledAndLocalNotSet() { - $factory = $this->createFactory(array('js_validation' => true)); + $factory = $this->createFactory(null, null, array('js_validation' => true)); $formFactory = $this->createFormFactory($factory); $subscriber = new SubscriberToQueue($factory); @@ -68,7 +64,7 @@ public function testAddToQueueWhenGlobalEnabledAndLocalNotSet() public function testDoesNotAddToQueueWhenLocalExplicitlyDisabled() { - $factory = $this->createFactory(array('js_validation' => true)); + $factory = $this->createFactory(null, null, array('js_validation' => true)); $formFactory = $this->createFormFactory($factory); $subscriber = new SubscriberToQueue($factory); @@ -81,24 +77,4 @@ public function testDoesNotAddToQueueWhenLocalExplicitlyDisabled() $this->assertFalse($factory->inQueue($form)); } - - private function createFactory(array $config = array()) - { - return new JsFormValidatorFactory( - Validation::createValidator(), - $this->createStub(TranslatorInterface::class), - $this->createStub(UrlGeneratorInterface::class), - $config, - 'validators' - ); - } - - private function createFormFactory(JsFormValidatorFactory $factory) - { - return Forms::createFormFactoryBuilder() - ->addExtension(new ValidatorExtension(Validation::createValidator())) - ->addTypeExtension(new FormExtension($factory)) - ->getFormFactory() - ; - } } From 74d1554977d8b337c8ee3b0eac59d824c6a281c3 Mon Sep 17 00:00:00 2001 From: Damien ALEXANDRE Date: Tue, 18 Aug 2026 18:02:12 +0200 Subject: [PATCH 3/3] fix(doc): Add documentation and test --- Tests/Unit/SubscriberToQueueTest.php | 22 ++++++++++++++++++++++ src/Resources/doc/2_1.md | 19 ++++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/Tests/Unit/SubscriberToQueueTest.php b/Tests/Unit/SubscriberToQueueTest.php index e54ea657..78e0428c 100644 --- a/Tests/Unit/SubscriberToQueueTest.php +++ b/Tests/Unit/SubscriberToQueueTest.php @@ -77,4 +77,26 @@ public function testDoesNotAddToQueueWhenLocalExplicitlyDisabled() $this->assertFalse($factory->inQueue($form)); } + + public function testChildFieldOptInAddsEntireFormToQueue() + { + // Global disabled, but a child field explicitly opts in + $factory = $this->createFactory(null, null, array('js_validation' => false)); + $formFactory = $this->createFormFactory($factory); + $subscriber = new SubscriberToQueue($factory); + + $form = $formFactory + ->createNamedBuilder('test_form', FormType::class) + ->add('email', FormType::class, array('js_validation' => true)) + ->getForm() + ; + + // Trigger subscriber on the child field that opts in + $child = $form->get('email'); + $subscriber->onFormSetData(new FormEvent($child, null)); + + // The entire parent form should be in the queue + $this->assertTrue($factory->inQueue($form)); + $this->assertArrayHasKey('test_form', $factory->getQueue()); + } } diff --git a/src/Resources/doc/2_1.md b/src/Resources/doc/2_1.md index 464134d9..8318b876 100755 --- a/src/Resources/doc/2_1.md +++ b/src/Resources/doc/2_1.md @@ -1,9 +1,12 @@ -### 2.1 Disabling validation +### 2.1 Enabling and Disabling validation -You can disable JavaScript validation in three ways. +JavaScript validation is **enabled by default** for all forms. +The `js_validation` option can be set at three levels: #### Globally +Disable validation for all forms: + ```yaml # config/packages/fp_js_form_validator.yaml fp_js_form_validator: @@ -12,6 +15,8 @@ fp_js_form_validator: #### For a form type +Override the global setting for a specific form type: + ```php setDefaults([ - 'js_validation' => false, + 'js_validation' => false, // Disable for this form only ]); } } ``` +When the form option is not explicitly set (i.e., it is `null`), the form +inherits the global configuration value. This allows you to: + +- Disable validation globally (`js_validation: false`) and then enable it + **only for selected forms** by setting `'js_validation' => true` on those forms. +- Enable validation globally (the default) and disable it for specific forms + by setting `'js_validation' => false` on those forms. + #### For a field See [disable validation for a specified field](3_1.md).