Refactor form services to avoid repeating code (#2553)

* refactor: DRY on CanHaveSubFields * Add tests
area17 · May 28, 2024 · 2768019 · 2768019
1 parent 9a75e13
commit 2768019
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 71 deletions.
diff --git a/src/Services/Forms/Columns.php b/src/Services/Forms/Columns.php
@@ -4,6 +4,7 @@
 
 use A17\Twill\Services\Forms\Contracts\CanHaveSubfields;
 use A17\Twill\Services\Forms\Contracts\CanRenderForBlocks;
+use A17\Twill\Services\Forms\Traits\HasSubFields;
 use A17\Twill\Services\Forms\Traits\RenderForBlocks;
 use Illuminate\Contracts\View\View;
 use Illuminate\Support\Collection;
@@ -12,6 +13,7 @@
 class Columns implements CanHaveSubfields, CanRenderForBlocks
 {
     use RenderForBlocks;
+    use HasSubFields;
 
     protected function __construct(
         public ?Collection $left = null,
@@ -66,37 +68,8 @@ public function render(): View
 
     public function registerDynamicRepeaters(): void
     {
-        if ($this->left) {
-            foreach ($this->left as $field) {
-                if ($field instanceof InlineRepeater) {
-                    $field->register();
-                }
-                if ($field instanceof CanHaveSubfields) {
-                    $field->registerDynamicRepeaters();
-                }
-            }
-        }
-
-        if ($this->middle) {
-            foreach ($this->middle as $field) {
-                if ($field instanceof InlineRepeater) {
-                    $field->register();
-                }
-                if ($field instanceof CanHaveSubfields) {
-                    $field->registerDynamicRepeaters();
-                }
-            }
-        }
-
-        if ($this->right) {
-            foreach ($this->right as $field) {
-                if ($field instanceof InlineRepeater) {
-                    $field->register();
-                }
-                if ($field instanceof CanHaveSubfields) {
-                    $field->registerDynamicRepeaters();
-                }
-            }
-        }
+        $this->registerDynamicRepeatersFor($this->left);
+        $this->registerDynamicRepeatersFor($this->middle);
+        $this->registerDynamicRepeatersFor($this->right);
     }
 }
diff --git a/src/Services/Forms/Fieldset.php b/src/Services/Forms/Fieldset.php
@@ -3,11 +3,14 @@
 namespace A17\Twill\Services\Forms;
 
 use A17\Twill\Services\Forms\Contracts\CanHaveSubfields;
+use A17\Twill\Services\Forms\Traits\HasSubFields;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
 
 class Fieldset implements CanHaveSubfields
 {
+    use HasSubFields;
+
     protected function __construct(
         public ?string $title = null,
         public ?Collection $fields = null,
@@ -77,13 +80,6 @@ public function fields(array $fields): static
 
     public function registerDynamicRepeaters(): void
     {
-        foreach ($this as $field) {
-            if ($field instanceof InlineRepeater) {
-                $field->register();
-            }
-            if ($field instanceof CanHaveSubfields) {
-                $field->registerDynamicRepeaters();
-            }
-        }
+        $this->registerDynamicRepeatersFor($this->fields);
     }
 }
diff --git a/src/Services/Forms/Fieldsets.php b/src/Services/Forms/Fieldsets.php
@@ -2,8 +2,11 @@
 
 namespace A17\Twill\Services\Forms;
 
+use A17\Twill\Services\Forms\Contracts\CanHaveSubfields;
+use A17\Twill\Services\Forms\Traits\HasSubFields;
 use Illuminate\Support\Collection;
 
-class Fieldsets extends Collection
+class Fieldsets extends Collection implements CanHaveSubfields
 {
+    use HasSubFields;
 }
diff --git a/src/Services/Forms/Form.php b/src/Services/Forms/Form.php
@@ -3,11 +3,14 @@
 namespace A17\Twill\Services\Forms;
 
 use A17\Twill\Services\Forms\Contracts\CanHaveSubfields;
+use A17\Twill\Services\Forms\Traits\HasSubFields;
 use Illuminate\Contracts\View\View;
 use Illuminate\Support\Collection;
 
 class Form extends Collection implements CanHaveSubfields
 {
+    use HasSubFields;
+
     public ?Fieldsets $fieldsets = null;
 
     private ?Form $sideForm = null;
@@ -111,27 +114,7 @@ public function renderSideForm(): View
 
     public function registerDynamicRepeaters(): void
     {
-        // We have to loop over all of the fields/fieldsets.
-        foreach ($this as $field) {
-            if ($field instanceof InlineRepeater) {
-                $field->register();
-            }
-            if ($field instanceof CanHaveSubfields) {
-                $field->registerDynamicRepeaters();
-            }
-        }
-
-        if ($this->fieldsets) {
-            foreach ($this->fieldsets as $fieldset) {
-                foreach ($fieldset->fields as $field) {
-                    if ($field instanceof InlineRepeater) {
-                        $field->register();
-                    }
-                    if ($field instanceof CanHaveSubfields) {
-                        $field->registerDynamicRepeaters();
-                    }
-                }
-            }
-        }
+        $this->registerDynamicRepeatersFor($this);
+        $this->fieldsets?->registerDynamicRepeaters();
     }
 }
diff --git a/src/Services/Forms/InlineRepeater.php b/src/Services/Forms/InlineRepeater.php
@@ -7,6 +7,7 @@
 use A17\Twill\Services\Forms\Contracts\CanHaveSubfields;
 use A17\Twill\Services\Forms\Contracts\CanRenderForBlocks;
 use A17\Twill\Services\Forms\Fields\Repeater;
+use A17\Twill\Services\Forms\Traits\HasSubFields;
 use A17\Twill\Services\Forms\Traits\RenderForBlocks;
 use Illuminate\Contracts\View\View;
 use Illuminate\Support\Collection;
@@ -15,6 +16,7 @@
 class InlineRepeater implements CanHaveSubfields, CanRenderForBlocks
 {
     use RenderForBlocks;
+    use HasSubFields;
 
     protected function __construct(
         private ?string $name = null,
@@ -225,14 +227,8 @@ public function render(): View
 
     public function registerDynamicRepeaters(): void
     {
-        foreach ($this->fields as $field) {
-            if ($field instanceof self) {
-                $field->register();
-            }
-            if ($field instanceof CanHaveSubfields) {
-                $field->registerDynamicRepeaters();
-            }
-        }
+        $this->register();
+        $this->registerDynamicRepeatersFor($this->fields);
     }
 
 

diff --git a/src/Services/Forms/Traits/HasSubFields.php b/src/Services/Forms/Traits/HasSubFields.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace A17\Twill\Services\Forms\Traits;
+
+use A17\Twill\Services\Forms\Contracts\CanHaveSubfields;
+use Illuminate\Support\Collection;
+
+trait HasSubFields
+{
+    public function registerDynamicRepeaters(): void
+    {
+        if ($this instanceof Collection) {
+            $this->registerDynamicRepeatersFor($this);
+        }
+    }
+
+    protected function registerDynamicRepeatersFor(?iterable $fields): void
+    {
+        if ($fields) {
+            foreach ($fields as $field) {
+                if ($field instanceof CanHaveSubfields) {
+                    $field->registerDynamicRepeaters();
+                }
+            }
+        }
+    }
+}
diff --git a/tests/unit/Services/Forms/NestingTest.php b/tests/unit/Services/Forms/NestingTest.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace A17\Twill\Tests\Unit\Services\Forms;
+
+use A17\Twill\Services\Forms\Columns;
+use A17\Twill\Services\Forms\Fieldset;
+use A17\Twill\Services\Forms\Form;
+use A17\Twill\Services\Forms\InlineRepeater;
+use A17\Twill\Tests\Unit\TestCase;
+use A17\Twill\TwillBlocks;
+
+class NestingTest extends TestCase
+{
+    public function testRegisterDynamicRepeaters()
+    {
+        TwillBlocks::$dynamicRepeaters = [];
+        $form = Form::make([
+            Columns::make()->middle([
+                InlineRepeater::make()->name('repeater1')
+            ]),
+            InlineRepeater::make()->name('repeater2'),
+        ])->addFieldset(
+            Fieldset::make()->fields([
+                Columns::make()->left([
+                    InlineRepeater::make()->name('set-repeater1')->fields([
+                        InlineRepeater::make()->name('nested1')
+                    ])
+                ])->right([
+                    InlineRepeater::make()->name('set-repeater2')
+                ])
+            ])
+        );
+        $form->registerDynamicRepeaters();
+        $this->assertArrayHasKey('repeater1', TwillBlocks::$dynamicRepeaters);
+        $this->assertArrayHasKey('repeater2', TwillBlocks::$dynamicRepeaters);
+        $this->assertArrayHasKey('set-repeater1', TwillBlocks::$dynamicRepeaters);
+        $this->assertArrayHasKey('nested1', TwillBlocks::$dynamicRepeaters);
+        $this->assertArrayHasKey('set-repeater2', TwillBlocks::$dynamicRepeaters);
+    }
+}