controller cleanup

composer.json

@@ -6,7 +6,8 @@
     "minimum-stability": "dev",
     "autoload": {
         "psr-4": {
-            "markhuot\\keystone\\": "src/"
+            "markhuot\\keystone\\": "src/",
+            "markhuot\\keystone\\tests\\": "tests/"
         },
         "files": [
             "src/helpers/event.php",

src/actions/MakeModelFromArray.php

@@ -6,6 +6,7 @@
 use craft\base\FieldInterface;
 use craft\base\Model;
 use markhuot\keystone\db\ActiveRecord;
+use markhuot\keystone\tests\models\ElementToElementIdTest;
 use yii\base\ModelEvent;
 
 use function markhuot\keystone\helpers\base\app;
@@ -89,14 +90,22 @@ protected function load(\yii\base\Model $model, mixed $data): void
 
         $reflect = new \ReflectionClass($model);
 
-        foreach ($data as $key => &$value) {
+        foreach ($data as $key => $value) {
+            // If we were passed something like elementId over the wire, but want to deal with
+            // a property of `->element` then check for that here and swap the keys out.
+            if (str_ends_with($key, 'Id') && ! $reflect->hasProperty($key)) {
+                unset($data[$key]);
+                $key = substr($key, 0, -2);
+            }
+
             if ($reflect->hasProperty($key)) {
                 $property = $reflect->getProperty($key);
                 $type = $property->getType()->getName();
 
                 if (enum_exists($type)) {
                     $value = $type::from($value);
-                } elseif (class_exists($type) || interface_exists($type)) {
+                }
+                elseif (class_exists($type) || interface_exists($type)) {
                     $value = (new static)
                         ->handle(
                             className: $type,
@@ -107,8 +116,17 @@ className: $type,
                         );
                 }
             }
+
+            $data[$key] = $value;
         }
 
+        // We can't set a non-null property to null, or it would error out here during the load
+        // phase instead of during the validation phase. So, if a value is null but the model doesn't
+        // support that remove the null value from the data array. This will leave the value unset
+        // in the model and it will later fail validation.
+        //
+        // Basically, we're punting null errors further down in the processing so it can catch during
+        // validation and not throw a runtime error here during load.
         $reflect = new \ReflectionClass($model);
         foreach ($reflect->getProperties(\ReflectionProperty::IS_PUBLIC) as $property) {
             if (! $property->getType()?->allowsNull() && ($data[$property->getName()] ?? null) === null) {

src/base/Model.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace markhuot\keystone\base;
+
+use markhuot\keystone\validators\Required;
+use markhuot\keystone\validators\Safe;
+
+class Model extends \craft\base\Model
+{
+    public function safeAttributes()
+    {
+        $reflect = new \ReflectionClass($this);
+        $properties = collect($reflect->getProperties(\ReflectionProperty::IS_PUBLIC))
+            ->filter(fn (\ReflectionProperty $property) => $property->getAttributes(Safe::class))
+            ->map(fn (\ReflectionProperty $property) => $property->getName())
+            ->all();
+
+        return array_merge(parent::safeAttributes(), $properties);
+    }
+
+    public function rules(): array
+    {
+        $reflect = new \ReflectionClass($this);
+        $required = collect($reflect->getProperties(\ReflectionProperty::IS_PUBLIC))
+            ->filter(fn (\ReflectionProperty $property) => $property->getAttributes(Required::class))
+            ->map(fn (\ReflectionProperty $property) => $property->getName())
+            ->all();
+
+        return [
+            [$required, 'required'],
+        ];
+    }
+}

src/behaviors/BodyParamObjectBehavior.php

@@ -40,6 +40,34 @@ public function getBodyParamObject(string $class, string $formName = '')
         // Get the post data
         $data = $this->owner->getBodyParams();
 
+        return $this->handleData($data, $class, $formName);
+    }
+
+    /**
+     * @template T
+     *
+     * @param  class-string<T>  $class
+     * @return T
+     */
+    public function getQueryParamObject(string $class, string $formName = '')
+    {
+        return $this->handleData($this->owner->getQueryParams(), $class, $formName);
+    }
+
+    public function getQueryParamObjectOrFail(string $class, string $formName = '')
+    {
+        return $this->handleData($this->owner->getQueryParams(), $class, $formName, true, false);
+    }
+
+    /**
+     * @template T
+     *
+     * @param array<mixed> $data
+     * @param class-string<T>  $class
+     * @return T
+     */
+    protected function handleData(array $data, string $class, string $formName = '', bool $errorOnMissing = false, bool $createOnMissing = true)
+    {
         // Yii doesn't support nested form names so manually pull out
         // the right data using Laravel's data_get() and then drop the
         // form name from the Yii call
@@ -48,7 +76,7 @@ public function getBodyParamObject(string $class, string $formName = '')
         }
 
         // Create our model
-        $model = (new MakeModelFromArray)->handle($class, $data);
+        $model = (new MakeModelFromArray)->handle($class, $data, $errorOnMissing, $createOnMissing);
 
         // Validate the model
         if ($model->hasErrors()) {

src/controllers/ComponentsController.php

@@ -13,6 +13,7 @@
 use markhuot\keystone\behaviors\BodyParamObjectBehavior;
 use markhuot\keystone\models\Component;
 use markhuot\keystone\models\http\AddComponentRequest;
+use markhuot\keystone\models\http\StoreComponentRequest;
 use markhuot\keystone\models\http\MoveComponentRequest;
 use yii\web\Request;
 
@@ -23,32 +24,26 @@ class ComponentsController extends Controller
 {
     public function actionAdd()
     {
-        $elementId = $this->request->getRequiredQueryParam('elementId');
-        $element = Craft::$app->elements->getElementById($elementId);
-        $fieldId = $this->request->getRequiredQueryParam('fieldId');
-        $field = Craft::$app->fields->getFieldById($fieldId);
-        $path = $this->request->getQueryParam('path');
-        $slot = $this->request->getQueryParam('slot');
-        $sortOrder = $this->request->getRequiredQueryParam('sortOrder');
-        $parent = (new GetParentFromPath)->handle($elementId, $fieldId, $path);
+        $data = $this->request->getQueryParamObject(AddComponentRequest::class);
+        $parent = (new GetParentFromPath)->handle($data->element->id, $data->field->id, $data->path);
 
         return $this->asCpScreen()
             ->title('Add component')
             ->action('keystone/components/store')
             ->contentTemplate('keystone/select', [
-                'element' => $element,
-                'field' => $field,
-                'path' => $path,
-                'slot' => $slot,
+                'element' => $data->element,
+                'field' => $data->field,
+                'path' => $data->path,
+                'slot' => $data->slot,
                 'parent' => $parent,
                 'groups' => (new GetComponentType())->all()->groupBy(fn ($t) => $t->getCategory()),
-                'sortOrder' => $sortOrder,
+                'sortOrder' => $data->sortOrder,
             ]);
     }
 
     public function actionStore()
     {
-        $data = $this->request->getBodyParamObject(AddComponentRequest::class);
+        $data = $this->request->getBodyParamObject(StoreComponentRequest::class);
 
         (new AddComponent)->handle(
             elementId: $data->element->id,
@@ -66,10 +61,7 @@ public function actionStore()
 
     public function actionEdit()
     {
-        $id = $this->request->getRequiredQueryParam('id');
-        $elementId = $this->request->getRequiredQueryParam('elementId');
-        $fieldId = $this->request->getRequiredQueryParam('fieldId');
-        $component = Component::findOne(['id' => $id, 'elementId' => $elementId, 'fieldId' => $fieldId]);
+        $component = $this->request->getQueryParamObjectOrFail(Component::class);
         $hasContentFields = $component->getType()->getFieldDefinitions()->isNotEmpty();
 
         return $this->asCpScreen()

src/models/http/AddComponentRequest.php

@@ -5,36 +5,28 @@
 use craft\base\ElementInterface;
 use craft\base\FieldInterface;
 use craft\base\Model;
+use markhuot\keystone\validators\Required;
+use markhuot\keystone\validators\Safe;
 
 class AddComponentRequest extends Model
 {
+    #[Required]
     public ElementInterface $element;
 
+    #[Required]
     public FieldInterface $field;
 
-    public ?string $path;
+    #[Safe]
+    public ?string $path=null;
 
-    public ?string $slot;
+    #[Safe]
+    public ?string $slot=null;
 
+    #[Required]
     public int $sortOrder;
 
-    public string $type;
-
-    public function safeAttributes()
+    public function getParent()
     {
-        return [...parent::safeAttributes(), 'path', 'slot'];
-    }
 
-    /**
-     * @return array<mixed>
-     */
-    public function rules(): array
-    {
-        return [
-            ['element', 'required'],
-            ['field', 'required'],
-            ['sortOrder', 'required'],
-            ['type', 'required'],
-        ];
     }
 }

src/models/http/EditComponentRequest.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace markhuot\keystone\models\http;
+
+use markhuot\keystone\base\Model;
+use markhuot\keystone\models\Component;
+
+class EditComponentRequest extends Model
+{
+    public Component $component;
+}

src/models/http/StoreComponentRequest.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace markhuot\keystone\models\http;
+
+use craft\base\ElementInterface;
+use craft\base\FieldInterface;
+use markhuot\keystone\base\Model;
+use markhuot\keystone\validators\Required;
+use markhuot\keystone\validators\Safe;
+
+class StoreComponentRequest extends Model
+{
+    #[Required]
+    public ElementInterface $element;
+
+    #[Required]
+    public FieldInterface $field;
+
+    #[Safe]
+    public ?string $path=null;
+
+    #[Safe]
+    public ?string $slot=null;
+
+    #[Required]
+    public int $sortOrder;
+
+    #[Required]
+    public string $type;
+}

src/validators/Required.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace markhuot\keystone\validators;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_PROPERTY)]
+class Required
+{
+
+}

src/validators/Safe.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace markhuot\keystone\validators;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_PROPERTY)]
+class Safe
+{
+
+}

tests/RouteModelBindingTest.php

@@ -5,8 +5,11 @@
 use markhuot\keystone\actions\MakeModelFromArray;
 use markhuot\keystone\enums\MoveComponentPosition;
 use markhuot\keystone\models\Component;
-use markhuot\keystone\models\http\AddComponentRequest;
+use markhuot\keystone\models\http\StoreComponentRequest;
 use markhuot\keystone\models\http\MoveComponentRequest;
+use markhuot\keystone\tests\models\ElementToElementIdTest;
+use markhuot\keystone\tests\models\OptionalFieldsTest;
+use markhuot\keystone\tests\models\RequiredFieldsTest;
 
 it('loads top level models', function () {
     $createdComponent = Component::factory()->create();
@@ -25,7 +28,7 @@
 
 it('gets elementinterface properties', function () {
     $entry = Entry::factory()->create();
-    $formData = (new MakeModelFromArray)->handle(AddComponentRequest::class, ['element' => $entry->id]);
+    $formData = (new MakeModelFromArray)->handle(StoreComponentRequest::class, ['element' => $entry->id]);
 
     expect($formData)->element->id->toBe($entry->id);
 });
@@ -36,3 +39,24 @@
 
     expect($formData)->id->toBe($entry->id);
 });
+
+it('automatically translates element to elementId', function () {
+    $entry = Entry::factory()->create();
+    $formData = (new MakeModelFromArray)->handle(ElementToElementIdTest::class, ['elementId' => $entry->id]);
+
+    expect($formData)->element->id->toBe($entry->id);
+});
+
+it('supports #[Required] attribute', function () {
+    $rules = (new RequiredFieldsTest)->rules();
+
+    expect($rules)->toHaveCount(1);
+    expect($rules[0][0])->toBe(['foo']);
+    expect($rules[0][1])->toBe('required');
+});
+
+it('supports #[Safe] attribute', function () {
+    $formData = (new MakeModelFromArray)->handle(OptionalFieldsTest::class, ['foo' => 'foo', 'bar' => 'bar']);
+
+    expect($formData)->foo->toBe('foo')->bar->toBe('bar');
+});

tests/models/ElementToElementIdTest.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace markhuot\keystone\tests\models;
+
+use craft\base\ElementInterface;
+use craft\base\Model;
+
+class ElementToElementIdTest extends Model
+{
+    public ElementInterface $element;
+
+    public function rules(): array
+    {
+        return [
+            [['element'], 'required'],
+        ];
+    }
+}