Drop UserAction::$modifier property (#1238)

atk4 · Nov 23, 2024 · ede9fa8 · ede9fa8
1 parent 70104d9
commit ede9fa8
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 77 deletions.
diff --git a/src/Model/UserAction.php b/src/Model/UserAction.php
@@ -27,27 +27,17 @@ class UserAction
     use TrackableTrait;
 
     /** Defining records scope of the action */
-    public const APPLIES_TO_NO_RECORDS = 'none'; // e.g. add
-    public const APPLIES_TO_SINGLE_RECORD = 'single'; // e.g. archive
-    public const APPLIES_TO_MULTIPLE_RECORDS = 'multiple'; // e.g. delete
+    public const APPLIES_TO_NO_RECORD = 'none'; // e.g. add
+    public const APPLIES_TO_SINGLE_RECORD = 'single'; // e.g. edit, delete, archive
     public const APPLIES_TO_ALL_RECORDS = 'all'; // e.g. truncate
 
-    /** Defining action modifier */
-    public const MODIFIER_CREATE = 'create'; // create new record(s)
-    public const MODIFIER_UPDATE = 'update'; // update existing record(s)
-    public const MODIFIER_DELETE = 'delete'; // delete record(s)
-    public const MODIFIER_READ = 'read'; // just read, does not modify record(s)
-
     /** @var string by default action is for a single record */
     public $appliesTo = self::APPLIES_TO_SINGLE_RECORD;
 
-    /** @var string How this action interact with record */
-    public $modifier;
-
-    /** @var \Closure<T of Model>(T, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed): mixed|string code to execute. By default will call entity method with same name */
+    /** @var \Closure<T of Model>(T, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed): mixed|string|null code to execute. By default will call entity method with same name */
     public $callback;
 
-    /** @var \Closure<T of Model>(T, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed): mixed|string identical to callback, but would generate preview of action without permanent effect */
+    /** @var \Closure<T of Model>(T, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed, mixed): mixed|string|null identical to callback, but would generate preview of action without permanent effect */
     public $preview;
 
     /** @var string|null caption to put on the button */
@@ -62,17 +52,17 @@ class UserAction
     /** @var bool|\Closure<T of Model>(T): bool setting this to false will disable action. */
     public $enabled = true;
 
-    /** @var bool system action will be hidden from UI, but can still be explicitly triggered */
-    public $system = false;
+    /** System action will be hidden from UI, but can still be explicitly triggered */
+    public bool $system = false;
 
-    /** @var array<string, array<string, mixed>> Argument definition. */
+    /** @var array<string, array<string, mixed>> Arguments definition. */
     public $args = [];
 
-    /** @var list<string>|bool Specify which fields may be dirty when invoking action. APPLIES_TO_NO_RECORDS|APPLIES_TO_SINGLE_RECORD scopes for adding/modifying */
+    /** @var list<string>|bool Specify which fields may be dirty when invoking action. APPLIES_TO_NO_RECORD|APPLIES_TO_SINGLE_RECORD scopes for adding/modifying */
     public $fields = [];
 
-    /** @var bool Atomic action will automatically begin transaction before and commit it after completing. */
-    public $atomic = true;
+    /** Atomic action will automatically begin transaction before and commit it after completing. */
+    public bool $atomic = true;
 
     private function _getOwner(): Model
     {
@@ -118,8 +108,46 @@ public function getActionForEntity(Model $entity): self
         throw new Exception('Action instance not found in model');
     }
 
+    public function validateBeforeExecute(): void
+    {
+        if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->_getOwner()) === false)) {
+            throw new Exception('User action is disabled');
+        }
+
+        switch ($this->appliesTo) {
+            case self::APPLIES_TO_NO_RECORD:
+                if ($this->getEntity()->isLoaded()) {
+                    throw (new Exception('User action can be executed on new entity only'))
+                        ->addMoreInfo('id', $this->getEntity()->getId());
+                }
+
+                break;
+            case self::APPLIES_TO_SINGLE_RECORD:
+                if (!$this->getEntity()->isLoaded()) {
+                    throw new Exception('User action can be executed on loaded entity only');
+                }
+
+                break;
+            case self::APPLIES_TO_ALL_RECORDS:
+                $this->_getOwner()->assertIsModel();
+
+                break;
+        }
+
+        if (!is_bool($this->fields) && $this->isOwnerEntity()) {
+            $dirtyFields = array_keys($this->getEntity()->getDirtyRef());
+            $tooDirtyFields = array_diff($dirtyFields, $this->fields);
+
+            if ($tooDirtyFields !== []) {
+                throw (new Exception('User action cannot be executed when unrelated fields are dirty'))
+                    ->addMoreInfo('tooDirtyFields', $tooDirtyFields)
+                    ->addMoreInfo('otherDirtyFields', array_diff($dirtyFields, $tooDirtyFields));
+            }
+        }
+    }
+
     /**
-     * Attempt to execute callback of the action.
+     * Execute callback of the action.
      *
      * @param mixed ...$args
      *
@@ -137,8 +165,6 @@ public function execute(...$args)
             $fx = $this->callback;
         }
 
-        // todo - ACL tests must allow
-
         try {
             $this->validateBeforeExecute();
 
@@ -156,45 +182,6 @@ public function execute(...$args)
         }
     }
 
-    public function validateBeforeExecute(): void
-    {
-        if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->_getOwner()) === false)) {
-            throw new Exception('User action is disabled');
-        }
-
-        if (!is_bool($this->fields) && $this->isOwnerEntity()) {
-            $dirtyFields = array_keys($this->getEntity()->getDirtyRef());
-            $tooDirtyFields = array_diff($dirtyFields, $this->fields);
-
-            if ($tooDirtyFields !== []) {
-                throw (new Exception('User action cannot be executed when unrelated fields are dirty'))
-                    ->addMoreInfo('tooDirtyFields', $tooDirtyFields)
-                    ->addMoreInfo('otherDirtyFields', array_diff($dirtyFields, $tooDirtyFields));
-            }
-        }
-
-        switch ($this->appliesTo) {
-            case self::APPLIES_TO_NO_RECORDS:
-                if ($this->getEntity()->isLoaded()) {
-                    throw (new Exception('User action can be executed on new entity only'))
-                        ->addMoreInfo('id', $this->getEntity()->getId());
-                }
-
-                break;
-            case self::APPLIES_TO_SINGLE_RECORD:
-                if (!$this->getEntity()->isLoaded()) {
-                    throw new Exception('User action can be executed on loaded entity only');
-                }
-
-                break;
-            case self::APPLIES_TO_MULTIPLE_RECORDS:
-            case self::APPLIES_TO_ALL_RECORDS:
-                $this->_getOwner()->assertIsModel();
-
-                break;
-        }
-    }
-
     /**
      * Identical to Execute but display a preview of what will happen.
      *
@@ -227,6 +214,11 @@ public function preview(...$args)
         }
     }
 
+    public function getCaption(): string
+    {
+        return $this->caption ?? $this->getModel()->readableCaption($this->shortName);
+    }
+
     /**
      * Get description of this current action in a user-understandable language.
      */
@@ -249,7 +241,7 @@ public function getConfirmation()
         if ($this->confirmation instanceof \Closure) {
             return ($this->confirmation)($this);
         } elseif ($this->confirmation === true) {
-            $confirmation = 'Are you sure you wish to execute '
+            $confirmation = 'Are you sure to execute '
                 . $this->getCaption()
                 . ($this->isOwnerEntity() && $this->getEntity()->getTitle() ? ' using ' . $this->getEntity()->getTitle() : '')
                 . '?';
@@ -259,9 +251,4 @@ public function getConfirmation()
 
         return $this->confirmation;
     }
-
-    public function getCaption(): string
-    {
-        return $this->caption ?? $this->getModel()->readableCaption($this->shortName);
-    }
 }
diff --git a/src/Model/UserActionsTrait.php b/src/Model/UserActionsTrait.php
@@ -133,15 +133,13 @@ protected function initUserActions(): void
         // declare our basic CRUD actions for the model
         $this->addUserAction('add', [
             'fields' => true,
-            'modifier' => UserAction::MODIFIER_CREATE,
-            'appliesTo' => UserAction::APPLIES_TO_NO_RECORDS,
+            'appliesTo' => UserAction::APPLIES_TO_NO_RECORD,
             'callback' => 'save',
             'description' => 'Add ' . $this->getModelCaption(),
         ]);
 
         $this->addUserAction('edit', [
             'fields' => true,
-            'modifier' => UserAction::MODIFIER_UPDATE,
             'appliesTo' => UserAction::APPLIES_TO_SINGLE_RECORD,
             'callback' => static function (Model $entity) {
                 $entity->assertIsLoaded();
@@ -152,7 +150,6 @@ protected function initUserActions(): void
 
         $this->addUserAction('delete', [
             'appliesTo' => UserAction::APPLIES_TO_SINGLE_RECORD,
-            'modifier' => UserAction::MODIFIER_DELETE,
             'callback' => static function (Model $entity) {
                 return $entity->delete();
             },
@@ -161,7 +158,6 @@ protected function initUserActions(): void
         $this->addUserAction('validate', [
             // 'appliesTo' => any entity!
             'description' => 'Provided with modified values will validate them but will not save',
-            'modifier' => UserAction::MODIFIER_READ,
             'fields' => true,
             'system' => true, // don't show by default
             'args' => [

diff --git a/tests/ConditionSqlTest.php b/tests/ConditionSqlTest.php
@@ -373,12 +373,12 @@ public function testArrayCondition(): void
 
         $m = new Model($this->db, ['table' => 'user']);
         $m->addField('name');
-        $m->addCondition('name', []); // this should not fail, should be always false
+        $m->addCondition('name', []); // always false condition
         self::assertCount(0, $m->export());
 
         $m = new Model($this->db, ['table' => 'user']);
         $m->addField('name');
-        $m->addCondition('name', 'not in', []); // this should not fail, should be always true
+        $m->addCondition('name', 'not in', []); // always true condition
         self::assertCount(3, $m->export());
     }
 

diff --git a/tests/UserActionTest.php b/tests/UserActionTest.php
@@ -201,7 +201,7 @@ public function testAppliesToSingleRecordNotLoadedException(): void
     public function testAppliesToNoRecordsLoadedRecordException(): void
     {
         $client = new UaClient($this->pers);
-        $client->addUserAction('new_client', ['appliesTo' => Model\UserAction::APPLIES_TO_NO_RECORDS]);
+        $client->addUserAction('new_client', ['appliesTo' => Model\UserAction::APPLIES_TO_NO_RECORD]);
         $client = $client->load(1);
 
         $this->expectException(Exception::class);
@@ -288,7 +288,7 @@ public function testConfirmation(): void
         self::assertFalse($action->getConfirmation());
 
         $action->confirmation = true;
-        self::assertSame('Are you sure you wish to execute Test using John?', $action->getConfirmation());
+        self::assertSame('Are you sure to execute Test using John?', $action->getConfirmation());
 
         $action->confirmation = 'Are you sure?';
         self::assertSame('Are you sure?', $action->getConfirmation());