Skip to content

Minor UA cleanup #1115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mvorisek merged 4 commits into from
Aug 17, 2023
Merged

Minor UA cleanup #1115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c81d9b7
improve UA tests
mvorisek Aug 15, 2023
54ca9ec
improve UA exception messages
mvorisek Aug 15, 2023
84ef169
unify UA execute/preview
mvorisek Aug 15, 2023
e117425
add full UA support for model
mvorisek Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 54 additions & 35 deletions src/Model/UserAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ class UserAction
public const APPLIES_TO_MULTIPLE_RECORDS = 'multiple'; // e.g. delete
public const APPLIES_TO_ALL_RECORDS = 'all'; // e.g. truncate

/** @var string by default action is for a single record */
public $appliesTo = self::APPLIES_TO_SINGLE_RECORD;

/** 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;

Expand Down Expand Up @@ -74,26 +74,28 @@ class UserAction
/** @var bool Atomic action will automatically begin transaction before and commit it after completing. */
public $atomic = true;

private function _getOwner(): Model
{
return $this->getOwner(); // @phpstan-ignore-line;
}

public function isOwnerEntity(): bool
{
/** @var Model */
$owner = $this->getOwner(); // @phpstan-ignore-line
$owner = $this->_getOwner();

return $owner->isEntity();
}

public function getModel(): Model
{
/** @var Model */
$owner = $this->getOwner(); // @phpstan-ignore-line
$owner = $this->_getOwner();

return $owner->getModel(true);
}

public function getEntity(): Model
{
/** @var Model */
$owner = $this->getOwner(); // @phpstan-ignore-line
$owner = $this->_getOwner();
$owner->assertIsEntity();

return $owner;
Expand All @@ -104,8 +106,7 @@ public function getEntity(): Model
*/
public function getActionForEntity(Model $entity): self
{
/** @var Model */
$owner = $this->getOwner(); // @phpstan-ignore-line
$owner = $this->_getOwner();

$entity->assertIsEntity($owner);
foreach ($owner->getUserActions() as $name => $action) {
Expand All @@ -126,12 +127,13 @@ public function getActionForEntity(Model $entity): self
*/
public function execute(...$args)
{
$passOwner = false;
if ($this->callback === null) {
$fx = \Closure::fromCallable([$this->getEntity(), $this->shortName]);
$fx = \Closure::fromCallable([$this->_getOwner(), $this->shortName]);
} elseif (is_string($this->callback)) {
$fx = \Closure::fromCallable([$this->getEntity(), $this->callback]);
$fx = \Closure::fromCallable([$this->_getOwner(), $this->callback]);
} else {
array_unshift($args, $this->getEntity());
$passOwner = true;
$fx = $this->callback;
}

Expand All @@ -140,9 +142,13 @@ public function execute(...$args)
try {
$this->validateBeforeExecute();

if ($passOwner) {
array_unshift($args, $this->_getOwner());
}

return $this->atomic === false
? $fx(...$args)
: $this->getModel()->atomic(static fn () => $fx(...$args));
: $this->_getOwner()->atomic(static fn () => $fx(...$args));
} catch (CoreException $e) {
$e->addMoreInfo('action', $this);

Expand All @@ -152,39 +158,39 @@ public function execute(...$args)

protected function validateBeforeExecute(): void
{
if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->getEntity()) === false)) {
throw new Exception('This action is disabled');
if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->_getOwner()) === false)) {
throw new Exception('User action is disabled');
}

// Verify that model fields wouldn't be too dirty
if (is_array($this->fields)) {
$tooDirty = array_diff(array_keys($this->getEntity()->getDirtyRef()), $this->fields);
if (!is_bool($this->fields) && $this->fields !== []) {
$dirtyFields = array_keys($this->getEntity()->getDirtyRef());
$tooDirtyFields = array_diff($dirtyFields, $this->fields);

if ($tooDirty) {
throw (new Exception('Calling user action on a Model with dirty fields that are not allowed by this action'))
->addMoreInfo('too_dirty', $tooDirty)
->addMoreInfo('dirty', array_keys($this->getEntity()->getDirtyRef()))
->addMoreInfo('permitted', $this->fields);
if ($tooDirtyFields !== []) {
throw (new Exception('User action cannot be executed as unrelated fields are dirty'))
->addMoreInfo('tooDirtyFields', $tooDirtyFields)
->addMoreInfo('otherDirtyFields', array_diff($dirtyFields, $tooDirtyFields));
}
} elseif (!is_bool($this->fields)) { // @phpstan-ignore-line
throw (new Exception('Argument `fields` for the user action must be either array or boolean'))
->addMoreInfo('fields', $this->fields);
}

// Verify some records scope cases
switch ($this->appliesTo) {
case self::APPLIES_TO_NO_RECORDS:
if ($this->getEntity()->isLoaded()) {
throw (new Exception('This user action can be executed on non-existing record only'))
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('This user action requires you to load existing record first');
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;
}
}
Expand All @@ -198,14 +204,27 @@ protected function validateBeforeExecute(): void
*/
public function preview(...$args)
{
$passOwner = false;
if (is_string($this->preview)) {
$fx = \Closure::fromCallable([$this->getEntity(), $this->preview]);
$fx = \Closure::fromCallable([$this->_getOwner(), $this->preview]);
} else {
array_unshift($args, $this->getEntity());
$passOwner = true;
$fx = $this->preview;
}

return $fx(...$args);
try {
$this->validateBeforeExecute();

if ($passOwner) {
array_unshift($args, $this->_getOwner());
}

return $fx(...$args);
} catch (CoreException $e) {
$e->addMoreInfo('action', $this);

throw $e;
}
}

/**
Expand All @@ -232,7 +251,7 @@ public function getConfirmation()
} elseif ($this->confirmation === true) {
$confirmation = 'Are you sure you wish to execute '
. $this->getCaption()
. ($this->getEntity()->getTitle() ? ' using ' . $this->getEntity()->getTitle() : '')
. ($this->isOwnerEntity() && $this->getEntity()->getTitle() ? ' using ' . $this->getEntity()->getTitle() : '')
. '?';

return $confirmation;
Expand Down
85 changes: 49 additions & 36 deletions tests/UserActionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Atk4\Data\Tests;

use Atk4\Core\Exception as CoreException;
use Atk4\Data\Exception;
use Atk4\Data\Model;
use Atk4\Data\Persistence;
use Atk4\Data\Schema\TestCase;
Expand Down Expand Up @@ -85,7 +85,7 @@ public function testBasic(): void
$client->unload();

// test system action
$act2 = $client->getUserAction('backupClients');
$act2 = $client->getModel()->getUserAction('backupClients');

// action takes no arguments. If it would, we should be able to find info about those
self::assertSame([], $act2->args);
Expand All @@ -108,6 +108,17 @@ public function testCustomSeedClass(): void
self::assertSame($customClass, get_class($client->getUserAction('foo')));
}

public function testExecuteUndefinedMethodException(): void
{
$client = new UaClient($this->pers);
$client->addUserAction('new_client');
$client = $client->load(1);

$this->expectException(\Error::class);
$this->expectExceptionMessage('Call to undefined method');
$client->executeUserAction('new_client');
}

public function testPreview(): void
{
$client = new UaClient($this->pers);
Expand All @@ -116,12 +127,13 @@ public function testPreview(): void
});

$client = $client->load(1);

self::assertSame('John', $client->getUserAction('say_name')->execute());

$client->getUserAction('say_name')->preview = function (UaClient $m, string $arg) {
$client->getUserAction('say_name')->preview = function (UaClient $m) {
return 'will say ' . $m->get('name');
};
self::assertSame('will say John', $client->getUserAction('say_name')->preview('x'));
self::assertSame('will say John', $client->getUserAction('say_name')->preview());

$client->getModel()->addUserAction('also_backup', ['callback' => 'backupClients']);
self::assertSame('backs up all clients', $client->getUserAction('also_backup')->execute());
Expand All @@ -132,55 +144,68 @@ public function testPreview(): void
self::assertSame('Also Backup UaClient', $client->getUserAction('also_backup')->getDescription());
}

public function testAppliesTo1(): void
public function testAppliesToSingleRecordNotEntityException(): void
{
$client = new UaClient($this->pers);
$client = $client->createEntity();

$this->expectExceptionMessage('load existing record');
$this->expectException(\TypeError::class);
$this->expectExceptionMessage('Expected entity, but instance is a model');
$client->executeUserAction('sendReminder');
}

public function testAppliesTo2(): void
public function testAppliesToAllRecordsEntityException(): void
{
$client = new UaClient($this->pers);
$client->addUserAction('new_client', ['appliesTo' => Model\UserAction::APPLIES_TO_NO_RECORDS]);
$client = $client->load(1);

$this->expectExceptionMessage('can be executed on non-existing record');
$client->executeUserAction('new_client');
$this->expectException(\TypeError::class);
$this->expectExceptionMessage('Expected model, but instance is an entity');
$client->executeUserAction('backupClients');
}

public function testAppliesTo3(): void
public function testAppliesToSingleRecordNotLoadedException(): void
{
$client = new UaClient($this->pers);
$client->addUserAction('new_client', ['appliesTo' => Model\UserAction::APPLIES_TO_NO_RECORDS, 'atomic' => false]);
$client = $client->createEntity();

$this->expectExceptionMessage('undefined method');
$this->expectException(Exception::class);
$this->expectExceptionMessage('User action can be executed on loaded entity only');
$client->executeUserAction('sendReminder');
}

public function testAppliesToNoRecordsLoadedRecordException(): void
{
$client = new UaClient($this->pers);
$client->addUserAction('new_client', ['appliesTo' => Model\UserAction::APPLIES_TO_NO_RECORDS]);
$client = $client->load(1);

$this->expectException(Exception::class);
$this->expectExceptionMessage('User action can be executed on new entity only');
$client->executeUserAction('new_client');
}

public function testException1(): void
public function testNotDefinedException(): void
{
$client = new UaClient($this->pers);

$this->expectException(CoreException::class);
$this->expectException(Exception::class);
$this->expectExceptionMessage('User action is not defined');
$client->getUserAction('non_existent_action');
}

public function testDisabled1(): void
public function testDisabledBoolException(): void
{
$client = new UaClient($this->pers);
$client = $client->load(1);

$client->getUserAction('sendReminder')->enabled = false;

$this->expectExceptionMessage('disabled');
$this->expectException(Exception::class);
$this->expectExceptionMessage('User action is disabled');
$client->getUserAction('sendReminder')->execute();
}

public function testDisabled2(): void
public function testDisabledClosureException(): void
{
$client = new UaClient($this->pers);
$client = $client->load(1);
Expand All @@ -194,7 +219,8 @@ public function testDisabled2(): void
return false;
};

$this->expectExceptionMessage('disabled');
$this->expectException(Exception::class);
$this->expectExceptionMessage('User action is disabled');
$client->getUserAction('sendReminder')->execute();
}

Expand All @@ -211,7 +237,7 @@ public function testFields(): void
self::assertSame('Peter', $client->get('name'));
}

public function testFieldsTooDirty1(): void
public function testFieldsTooDirtyException(): void
{
$client = new UaClient($this->pers);
$client->addUserAction('change_details', ['callback' => 'save', 'fields' => ['name']]);
Expand All @@ -222,21 +248,8 @@ public function testFieldsTooDirty1(): void
$client->set('name', 'Peter');
$client->set('reminder_sent', true);

$this->expectExceptionMessage('dirty fields');
$client->getUserAction('change_details')->execute();
}

public function testFieldsIncorrect(): void
{
$client = new UaClient($this->pers);
$client->addUserAction('change_details', ['callback' => 'save', 'fields' => 'whops_forgot_brackets']);

$client = $client->load(1);

self::assertNotSame('Peter', $client->get('name'));
$client->set('name', 'Peter');

$this->expectExceptionMessage('must be either array or boolean');
$this->expectException(Exception::class);
$this->expectExceptionMessage('User action cannot be executed as unrelated fields are dirty');
$client->getUserAction('change_details')->execute();
}

Expand Down