Fix hookables overwriting

Sylius · Feb 6, 2024 · 7d0ab09 · 7d0ab09
1 parent bcfa48d
commit 7d0ab09
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 19 deletions.
diff --git a/src/TwigHooks/src/DependencyInjection/Configuration.php b/src/TwigHooks/src/DependencyInjection/Configuration.php
@@ -88,7 +88,7 @@ private function addHooksConfiguration(ArrayNodeDefinition $rootNode): void
                                     ->useAttributeAsKey('name')
                                     ->prototype('variable')->end()
                                 ->end()
-                                ->integerNode('priority')->defaultValue(0)->end()
+                                ->integerNode('priority')->defaultNull()->end()
                             ->end()
                         ->end()
                     ->end()

diff --git a/src/TwigHooks/src/Hookable/AbstractHookable.php b/src/TwigHooks/src/Hookable/AbstractHookable.php
@@ -21,8 +21,8 @@ public function __construct (
         protected string $name,
         protected string $type,
         protected string $target,
-        protected ?array $data = null,
-        protected ?array $configuration = null,
+        protected array $data = [],
+        protected array $configuration = [],
         protected ?int $priority = null,
         protected ?bool $enabled = null,
     ) {
@@ -101,8 +101,8 @@ public function overwriteWith(self $hookable): self
             $hookable->getName(),
             $hookable->getType(),
             $hookable->getTarget(),
-            $hookable->data ?? $this->getData(),
-            $hookable->configuration ?? $this->getConfiguration(),
+            array_merge($this->getData(), $hookable->data),
+            array_merge($this->getConfiguration(), $hookable->configuration),
             $hookable->priority ?? $this->getPriority(),
             $hookable->enabled ?? $this->isEnabled(),
         );

diff --git a/src/TwigHooks/tests/Integration/DependencyInjection/ConfigurationTest.php b/src/TwigHooks/tests/Integration/DependencyInjection/ConfigurationTest.php
@@ -45,7 +45,7 @@ public function testItSetsDefaultValuesForHookable(): void
                             'target' => 'some_target.html.twig',
                             'data' => [],
                             'configuration' => [],
-                            'priority' => 0,
+                            'priority' => null,
                             'enabled' => true,
                             'component' => null,
                             'template' => null,
@@ -100,7 +100,7 @@ public function testItAllowsToUseComponentShortcut(): void
                             'target' => 'MyAwesomeComponent',
                             'data' => [],
                             'configuration' => [],
-                            'priority' => 0,
+                            'priority' => null,
                             'enabled' => true,
                             'component' => 'MyAwesomeComponent',
                             'template' => null,
@@ -134,7 +134,7 @@ public function testItAllowsToUseTemplateShortcut(): void
                             'target' => 'some_target.html.twig',
                             'data' => [],
                             'configuration' => [],
-                            'priority' => 0,
+                            'priority' => null,
                             'enabled' => true,
                             'component' => null,
                             'template' => 'some_target.html.twig',

diff --git a/src/TwigHooks/tests/Unit/Hookable/BaseHookableTest.php b/src/TwigHooks/tests/Unit/Hookable/BaseHookableTest.php
@@ -71,17 +71,37 @@ public function testItThrowsAnExceptionWhenTryingToOverrideHookableWithDifferent
 
     public function testItOverwritesHookableWithGivenHookable(): void
     {
-        $testSubject = $this->getTestSubject();
-
-        $overwrittenTestSubject = $testSubject->overwriteWith(BaseHookableMotherObject::withTarget('some_other_target'));
-
-        $this->assertSame('some_hook', $overwrittenTestSubject->getHookName());
-        $this->assertSame('some_name', $overwrittenTestSubject->getName());
-        $this->assertSame('some_other_target', $overwrittenTestSubject->getTarget());
-        $this->assertSame([], $overwrittenTestSubject->getData());
-        $this->assertSame([], $overwrittenTestSubject->getConfiguration());
-        $this->assertSame(0, $overwrittenTestSubject->getPriority());
-        $this->assertTrue($overwrittenTestSubject->isEnabled());
+        $hookableToBeOverwritten = new BaseHookable(
+            'some_hook',
+            'some_name',
+            'template',
+            'some_target',
+            ['some_data' => 'yes', 'another_data' => 'no'],
+            ['title' => 'King', 'name' => 'Arthur'],
+            50,
+            true,
+        );
+        $hookableToOverwrite = new BaseHookable(
+            'some_hook',
+            'some_name',
+            'component',
+            'some_other_target',
+            ['another_data' => 'yes', 'another_other_data' => 'no'],
+            ['title' => 'Queen'],
+            100,
+            false,
+        );
+
+        $overwrittenHookable = $hookableToBeOverwritten->overwriteWith($hookableToOverwrite);
+
+        $this->assertSame('some_hook', $overwrittenHookable->getHookName());
+        $this->assertSame('some_name', $overwrittenHookable->getName());
+        $this->assertSame('component', $overwrittenHookable->getType());
+        $this->assertSame('some_other_target', $overwrittenHookable->getTarget());
+        $this->assertSame(['some_data' => 'yes', 'another_data' => 'yes', 'another_other_data' => 'no'], $overwrittenHookable->getData());
+        $this->assertSame(['title' => 'Queen', 'name' => 'Arthur'], $overwrittenHookable->getConfiguration());
+        $this->assertSame(100, $overwrittenHookable->getPriority());
+        $this->assertFalse($overwrittenHookable->isEnabled());
     }
 
     public function testItAllowsToOverwriteHookableWithAnotherType(): void