refactor: extract HasResourceFields trait (closes #135 after merge)

src/Resources/BuildoraResource.php

@@ -5,6 +5,7 @@
 use Ginkelsoft\Buildora\Actions\BulkAction;
 use Ginkelsoft\Buildora\Exceptions\BuildoraException;
 use Ginkelsoft\Buildora\Resources\Concerns\HasResourceActions;
+use Ginkelsoft\Buildora\Resources\Concerns\HasResourceFields;
 use Ginkelsoft\Buildora\Resources\Concerns\HasResourceNavigation;
 use Ginkelsoft\Buildora\Resources\Concerns\HasResourceQuery;
 use Illuminate\Database\Eloquent\Model;
@@ -20,6 +21,7 @@
 abstract class BuildoraResource
 {
     use HasResourceActions;
+    use HasResourceFields;
     use HasResourceNavigation;
     use HasResourceQuery;
 
@@ -58,50 +60,8 @@ public static function make(): self
         return new static();
     }
 
-    /**
-     * Fill the resource fields with values from the given model instance.
-     *
-     * @param Model $model
-     * @return $this
-     */
-    public function fill(Model $model): self
-    {
-        $this->parentModel = $model;
-
-        foreach ($this->fields as $field) {
-            if (method_exists($field, 'setParentModel')) {
-                $field->setParentModel($model);
-            }
-
-            if (method_exists($field, 'setValue')) {
-                $field->setValue($model);
-            } else {
-                $field->value = $model->{$field->name} ?? null;
-            }
-
-            if (method_exists($field, 'getDisplayValue')) {
-                $field->displayValue = $field->getDisplayValue($model);
-            } else {
-                $field->displayValue = $field->value;
-            }
-        }
-
-        return $this;
-    }
-
-    public function setFields(array $fields): void
-    {
-        foreach ($fields as $field) {
-            if (! $field instanceof Field) {
-                $type = is_object($field) ? get_class($field) : gettype($field);
-                throw new BuildoraException(
-                    "Ongeldig veld in " . static::class . ": verwacht een Field-object, kreeg {$type}"
-                );
-            }
-        }
-
-        $this->fields = $fields;
-    }
+    // fill(), setFields(), getFields(), resolveFields() live in
+    // HasResourceFields (Resources\Concerns\HasResourceFields).
 
     /**
      * Define the widgets for this resource (used on the dashboard).
@@ -124,32 +84,7 @@ public function defineWidgets(): array
      */
     abstract public function defineFields(): array;
 
-    public function getFields(): array
-    {
-        $fields = $this->fields ?? [];
-
-        foreach ($fields as $field) {
-            if (! $field instanceof \Ginkelsoft\Buildora\Fields\Field) {
-                $type = is_object($field) ? get_class($field) : gettype($field);
-                throw new BuildoraException(
-                    "Ongeldig veld in " . static::class . ": verwacht een Field-object, kreeg {$type}"
-                );
-            }
-        }
-
-        return $fields;
-    }
-
-    /**
-     * Prepare the fields with data from a specific model.
-     *
-     * @param Model $model
-     * @return Field[]
-     */
-    public function resolveFields($model): array
-    {
-        return FieldManager::prepare($this->fields, $model);
-    }
+    // (getFields() / resolveFields() moved to HasResourceFields trait.)
 
     /**
      * Return a new instance of the underlying model.

src/Resources/Concerns/HasResourceFields.php

@@ -0,0 +1,115 @@
+<?php
+
+namespace Ginkelsoft\Buildora\Resources\Concerns;
+
+use Ginkelsoft\Buildora\Exceptions\BuildoraException;
+use Ginkelsoft\Buildora\Fields\Field;
+use Ginkelsoft\Buildora\Resources\FieldManager;
+use Illuminate\Database\Eloquent\Model;
+
+/**
+ * Field-management surface, extracted from BuildoraResource for #135.
+ *
+ * The trait covers the four non-abstract methods that read/write the
+ * resource's field collection. The abstract defineFields() hook itself
+ * stays on BuildoraResource — subclasses must implement it, and an
+ * abstract method declared via a trait is harder to discover (PHPStorm
+ * doesn't surface it as "implement" candidate when you extend the class).
+ *
+ * State touched by this trait:
+ *   - $this->fields            (collection of Field instances)
+ *   - $this->parentModel       (set by fill())
+ *
+ * Both are protected properties declared on BuildoraResource. PHP allows
+ * trait code to access them on the using class; nothing on this trait
+ * requires re-declaration.
+ */
+trait HasResourceFields
+{
+    /**
+     * Fill every field's value/displayValue from the given model. Used
+     * before rendering a row in a datatable or detail view.
+     */
+    public function fill(Model $model): self
+    {
+        $this->parentModel = $model;
+
+        foreach ($this->fields as $field) {
+            if (method_exists($field, 'setParentModel')) {
+                $field->setParentModel($model);
+            }
+
+            if (method_exists($field, 'setValue')) {
+                $field->setValue($model);
+            } else {
+                $field->value = $model->{$field->name} ?? null;
+            }
+
+            if (method_exists($field, 'getDisplayValue')) {
+                $field->displayValue = $field->getDisplayValue($model);
+            } else {
+                $field->displayValue = $field->value;
+            }
+        }
+
+        return $this;
+    }
+
+    /**
+     * Replace the resource's fields. Validates that every entry is a
+     * Field instance before assigning — guarantees no stray array/string
+     * sneaks into the field pipeline.
+     *
+     * @param array<int, Field> $fields
+     * @throws BuildoraException
+     */
+    public function setFields(array $fields): void
+    {
+        foreach ($fields as $field) {
+            if (! $field instanceof Field) {
+                $type = is_object($field) ? get_class($field) : gettype($field);
+                throw new BuildoraException(
+                    "Ongeldig veld in " . static::class . ": verwacht een Field-object, kreeg {$type}"
+                );
+            }
+        }
+
+        $this->fields = $fields;
+    }
+
+    /**
+     * Return the current field collection. Performs the same Field-type
+     * validation as setFields() — defensive, but the cost is negligible
+     * compared with how often callers iterate the result.
+     *
+     * @return array<int, Field>
+     * @throws BuildoraException
+     */
+    public function getFields(): array
+    {
+        $fields = $this->fields ?? [];
+
+        foreach ($fields as $field) {
+            if (! $field instanceof Field) {
+                $type = is_object($field) ? get_class($field) : gettype($field);
+                throw new BuildoraException(
+                    "Ongeldig veld in " . static::class . ": verwacht een Field-object, kreeg {$type}"
+                );
+            }
+        }
+
+        return $fields;
+    }
+
+    /**
+     * Re-prepare the field collection for a specific model. Delegates to
+     * FieldManager::prepare so the field-type-specific setValue logic
+     * stays in one place.
+     *
+     * @return array<int, Field>
+     */
+    public function resolveFields($model): array
+    {
+        return FieldManager::prepare($this->fields, $model);
+    }
+}

tests/Unit/Resources/Concerns/HasResourceFieldsTest.php

@@ -0,0 +1,141 @@
+<?php
+
+namespace Ginkelsoft\Buildora\Tests\Unit\Resources\Concerns;
+
+use Ginkelsoft\Buildora\Exceptions\BuildoraException;
+use Ginkelsoft\Buildora\Fields\Types\TextField;
+use Ginkelsoft\Buildora\Resources\BuildoraResource;
+use Ginkelsoft\Buildora\Resources\Concerns\HasResourceFields;
+use Ginkelsoft\Buildora\Tests\TestCase;
+use Ginkelsoft\Buildora\Traits\HasBuildora;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Support\Facades\Schema;
+use PHPUnit\Framework\Attributes\Test;
+use ReflectionClass;
+
+class RFItem extends Model
+{
+    use HasBuildora;
+    protected $table = 'rf_items';
+    protected $guarded = [];
+    public $timestamps = false;
+}
+
+class RFItemBuildora extends BuildoraResource
+{
+    public static function modelClass(): string
+    {
+        return RFItem::class;
+    }
+
+    public function defineFields(): array
+    {
+        return [
+            TextField::make('title', 'Titel'),
+        ];
+    }
+}
+
+class HasResourceFieldsTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        Schema::create('rf_items', function ($t) {
+            $t->increments('id');
+            $t->string('title')->nullable();
+        });
+    }
+
+    #[Test]
+    public function get_fields_returns_the_resource_field_collection(): void
+    {
+        $resource = new RFItemBuildora();
+
+        $fields = $resource->getFields();
+
+        $this->assertCount(1, $fields);
+        $this->assertSame('title', $fields[0]->name);
+    }
+
+    #[Test]
+    public function fill_sets_the_field_value_from_the_model_attribute(): void
+    {
+        $item = RFItem::create(['title' => 'Hallo']);
+        $resource = new RFItemBuildora();
+
+        $resource->fill($item);
+
+        $fields = $resource->getFields();
+        $this->assertSame('Hallo', $fields[0]->value);
+    }
+
+    #[Test]
+    public function set_fields_rejects_non_field_entries(): void
+    {
+        $resource = new RFItemBuildora();
+
+        $this->expectException(BuildoraException::class);
+        $resource->setFields(['not a field']);
+    }
+
+    #[Test]
+    public function set_fields_accepts_a_valid_field_array(): void
+    {
+        $resource = new RFItemBuildora();
+
+        $resource->setFields([TextField::make('alt')]);
+
+        $this->assertSame('alt', $resource->getFields()[0]->name);
+    }
+
+    #[Test]
+    public function resolve_fields_re_prepares_fields_for_a_given_model(): void
+    {
+        $item = RFItem::create(['title' => 'Resolved']);
+        $resource = new RFItemBuildora();
+
+        $resolved = $resource->resolveFields($item);
+
+        $this->assertCount(1, $resolved);
+        $this->assertSame('Resolved', $resolved[0]->value);
+    }
+
+    #[Test]
+    public function buildora_resource_uses_the_fields_trait(): void
+    {
+        $traits = (new ReflectionClass(BuildoraResource::class))->getTraitNames();
+
+        $this->assertContains(HasResourceFields::class, $traits);
+    }
+
+    #[Test]
+    public function field_methods_are_no_longer_inlined_in_buildora_resource(): void
+    {
+        $resourceSource = file_get_contents(
+            (new ReflectionClass(BuildoraResource::class))->getFileName()
+        );
+
+        // defineFields stays abstract on BuildoraResource itself, so it is
+        // *not* part of the extraction check.
+        foreach (['fill', 'setFields', 'getFields', 'resolveFields'] as $movedMethod) {
+            $this->assertStringNotContainsString(
+                "public function {$movedMethod}(",
+                $resourceSource,
+                "Method '{$movedMethod}' has been re-inlined into BuildoraResource.php — it should live in HasResourceFields trait."
+            );
+        }
+
+        $traitSource = file_get_contents(
+            (new ReflectionClass(HasResourceFields::class))->getFileName()
+        );
+
+        foreach (['fill', 'setFields', 'getFields', 'resolveFields'] as $movedMethod) {
+            $this->assertStringContainsString(
+                "function {$movedMethod}(",
+                $traitSource,
+            );
+        }
+    }
+}