Skip to content

Commit edd55c9

Browse files
mvorisekmvorisek
authored
Minor UA cleanup (#1115)
1 parent 58d0e8c commit edd55c9

2 files changed

Lines changed: 103 additions & 71 deletions

File tree

‎src/Model/UserAction.php‎

Lines changed: 54 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ class UserAction
3232
public const APPLIES_TO_MULTIPLE_RECORDS = 'multiple'; // e.g. delete
3333
public const APPLIES_TO_ALL_RECORDS = 'all'; // e.g. truncate
3434

35-
/** @var string by default action is for a single record */
36-
public $appliesTo = self::APPLIES_TO_SINGLE_RECORD;
37-
3835
/** Defining action modifier */
3936
public const MODIFIER_CREATE = 'create'; // create new record(s)
4037
public const MODIFIER_UPDATE = 'update'; // update existing record(s)
4138
public const MODIFIER_DELETE = 'delete'; // delete record(s)
4239
public const MODIFIER_READ = 'read'; // just read, does not modify record(s)
4340

41+
/** @var string by default action is for a single record */
42+
public $appliesTo = self::APPLIES_TO_SINGLE_RECORD;
43+
4444
/** @var string How this action interact with record */
4545
public $modifier;
4646

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

77+
private function _getOwner(): Model
78+
{
79+
return $this->getOwner(); // @phpstan-ignore-line;
80+
}
81+
7782
public function isOwnerEntity(): bool
7883
{
79-
/** @var Model */
80-
$owner = $this->getOwner(); // @phpstan-ignore-line
84+
$owner = $this->_getOwner();
8185

8286
return $owner->isEntity();
8387
}
8488

8589
public function getModel(): Model
8690
{
87-
/** @var Model */
88-
$owner = $this->getOwner(); // @phpstan-ignore-line
91+
$owner = $this->_getOwner();
8992

9093
return $owner->getModel(true);
9194
}
9295

9396
public function getEntity(): Model
9497
{
95-
/** @var Model */
96-
$owner = $this->getOwner(); // @phpstan-ignore-line
98+
$owner = $this->_getOwner();
9799
$owner->assertIsEntity();
98100

99101
return $owner;
@@ -104,8 +106,7 @@ public function getEntity(): Model
104106
*/
105107
public function getActionForEntity(Model $entity): self
106108
{
107-
/** @var Model */
108-
$owner = $this->getOwner(); // @phpstan-ignore-line
109+
$owner = $this->_getOwner();
109110

110111
$entity->assertIsEntity($owner);
111112
foreach ($owner->getUserActions() as $name => $action) {
@@ -126,12 +127,13 @@ public function getActionForEntity(Model $entity): self
126127
*/
127128
public function execute(...$args)
128129
{
130+
$passOwner = false;
129131
if ($this->callback === null) {
130-
$fx = \Closure::fromCallable([$this->getEntity(), $this->shortName]);
132+
$fx = \Closure::fromCallable([$this->_getOwner(), $this->shortName]);
131133
} elseif (is_string($this->callback)) {
132-
$fx = \Closure::fromCallable([$this->getEntity(), $this->callback]);
134+
$fx = \Closure::fromCallable([$this->_getOwner(), $this->callback]);
133135
} else {
134-
array_unshift($args, $this->getEntity());
136+
$passOwner = true;
135137
$fx = $this->callback;
136138
}
137139

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

145+
if ($passOwner) {
146+
array_unshift($args, $this->_getOwner());
147+
}
148+
143149
return $this->atomic === false
144150
? $fx(...$args)
145-
: $this->getModel()->atomic(static fn () => $fx(...$args));
151+
: $this->_getOwner()->atomic(static fn () => $fx(...$args));
146152
} catch (CoreException $e) {
147153
$e->addMoreInfo('action', $this);
148154

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

153159
protected function validateBeforeExecute(): void
154160
{
155-
if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->getEntity()) === false)) {
156-
throw new Exception('This action is disabled');
161+
if ($this->enabled === false || ($this->enabled instanceof \Closure && ($this->enabled)($this->_getOwner()) === false)) {
162+
throw new Exception('User action is disabled');
157163
}
158164

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

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

174-
// Verify some records scope cases
175176
switch ($this->appliesTo) {
176177
case self::APPLIES_TO_NO_RECORDS:
177178
if ($this->getEntity()->isLoaded()) {
178-
throw (new Exception('This user action can be executed on non-existing record only'))
179+
throw (new Exception('User action can be executed on new entity only'))
179180
->addMoreInfo('id', $this->getEntity()->getId());
180181
}
181182

182183
break;
183184
case self::APPLIES_TO_SINGLE_RECORD:
184185
if (!$this->getEntity()->isLoaded()) {
185-
throw new Exception('This user action requires you to load existing record first');
186+
throw new Exception('User action can be executed on loaded entity only');
186187
}
187188

189+
break;
190+
case self::APPLIES_TO_MULTIPLE_RECORDS:
191+
case self::APPLIES_TO_ALL_RECORDS:
192+
$this->_getOwner()->assertIsModel();
193+
188194
break;
189195
}
190196
}
@@ -198,14 +204,27 @@ protected function validateBeforeExecute(): void
198204
*/
199205
public function preview(...$args)
200206
{
207+
$passOwner = false;
201208
if (is_string($this->preview)) {
202-
$fx = \Closure::fromCallable([$this->getEntity(), $this->preview]);
209+
$fx = \Closure::fromCallable([$this->_getOwner(), $this->preview]);
203210
} else {
204-
array_unshift($args, $this->getEntity());
211+
$passOwner = true;
205212
$fx = $this->preview;
206213
}
207214

208-
return $fx(...$args);
215+
try {
216+
$this->validateBeforeExecute();
217+
218+
if ($passOwner) {
219+
array_unshift($args, $this->_getOwner());
220+
}
221+
222+
return $fx(...$args);
223+
} catch (CoreException $e) {
224+
$e->addMoreInfo('action', $this);
225+
226+
throw $e;
227+
}
209228
}
210229

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

238257
return $confirmation;

‎tests/UserActionTest.php‎

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace Atk4\Data\Tests;
66

7-
use Atk4\Core\Exception as CoreException;
7+
use Atk4\Data\Exception;
88
use Atk4\Data\Model;
99
use Atk4\Data\Persistence;
1010
use Atk4\Data\Schema\TestCase;
@@ -85,7 +85,7 @@ public function testBasic(): void
8585
$client->unload();
8686

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

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

111+
public function testExecuteUndefinedMethodException(): void
112+
{
113+
$client = new UaClient($this->pers);
114+
$client->addUserAction('new_client');
115+
$client = $client->load(1);
116+
117+
$this->expectException(\Error::class);
118+
$this->expectExceptionMessage('Call to undefined method');
119+
$client->executeUserAction('new_client');
120+
}
121+
111122
public function testPreview(): void
112123
{
113124
$client = new UaClient($this->pers);
@@ -116,12 +127,13 @@ public function testPreview(): void
116127
});
117128

118129
$client = $client->load(1);
130+
119131
self::assertSame('John', $client->getUserAction('say_name')->execute());
120132

121-
$client->getUserAction('say_name')->preview = function (UaClient $m, string $arg) {
133+
$client->getUserAction('say_name')->preview = function (UaClient $m) {
122134
return 'will say ' . $m->get('name');
123135
};
124-
self::assertSame('will say John', $client->getUserAction('say_name')->preview('x'));
136+
self::assertSame('will say John', $client->getUserAction('say_name')->preview());
125137

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

135-
public function testAppliesTo1(): void
147+
public function testAppliesToSingleRecordNotEntityException(): void
136148
{
137149
$client = new UaClient($this->pers);
138-
$client = $client->createEntity();
139150

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

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

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

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

160-
$this->expectExceptionMessage('undefined method');
171+
$this->expectException(Exception::class);
172+
$this->expectExceptionMessage('User action can be executed on loaded entity only');
173+
$client->executeUserAction('sendReminder');
174+
}
175+
176+
public function testAppliesToNoRecordsLoadedRecordException(): void
177+
{
178+
$client = new UaClient($this->pers);
179+
$client->addUserAction('new_client', ['appliesTo' => Model\UserAction::APPLIES_TO_NO_RECORDS]);
180+
$client = $client->load(1);
181+
182+
$this->expectException(Exception::class);
183+
$this->expectExceptionMessage('User action can be executed on new entity only');
161184
$client->executeUserAction('new_client');
162185
}
163186

164-
public function testException1(): void
187+
public function testNotDefinedException(): void
165188
{
166189
$client = new UaClient($this->pers);
167190

168-
$this->expectException(CoreException::class);
191+
$this->expectException(Exception::class);
192+
$this->expectExceptionMessage('User action is not defined');
169193
$client->getUserAction('non_existent_action');
170194
}
171195

172-
public function testDisabled1(): void
196+
public function testDisabledBoolException(): void
173197
{
174198
$client = new UaClient($this->pers);
175199
$client = $client->load(1);
176200

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

179-
$this->expectExceptionMessage('disabled');
203+
$this->expectException(Exception::class);
204+
$this->expectExceptionMessage('User action is disabled');
180205
$client->getUserAction('sendReminder')->execute();
181206
}
182207

183-
public function testDisabled2(): void
208+
public function testDisabledClosureException(): void
184209
{
185210
$client = new UaClient($this->pers);
186211
$client = $client->load(1);
@@ -194,7 +219,8 @@ public function testDisabled2(): void
194219
return false;
195220
};
196221

197-
$this->expectExceptionMessage('disabled');
222+
$this->expectException(Exception::class);
223+
$this->expectExceptionMessage('User action is disabled');
198224
$client->getUserAction('sendReminder')->execute();
199225
}
200226

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

214-
public function testFieldsTooDirty1(): void
240+
public function testFieldsTooDirtyException(): void
215241
{
216242
$client = new UaClient($this->pers);
217243
$client->addUserAction('change_details', ['callback' => 'save', 'fields' => ['name']]);
@@ -222,21 +248,8 @@ public function testFieldsTooDirty1(): void
222248
$client->set('name', 'Peter');
223249
$client->set('reminder_sent', true);
224250

225-
$this->expectExceptionMessage('dirty fields');
226-
$client->getUserAction('change_details')->execute();
227-
}
228-
229-
public function testFieldsIncorrect(): void
230-
{
231-
$client = new UaClient($this->pers);
232-
$client->addUserAction('change_details', ['callback' => 'save', 'fields' => 'whops_forgot_brackets']);
233-
234-
$client = $client->load(1);
235-
236-
self::assertNotSame('Peter', $client->get('name'));
237-
$client->set('name', 'Peter');
238-
239-
$this->expectExceptionMessage('must be either array or boolean');
251+
$this->expectException(Exception::class);
252+
$this->expectExceptionMessage('User action cannot be executed as unrelated fields are dirty');
240253
$client->getUserAction('change_details')->execute();
241254
}
242255

0 commit comments

Comments
 (0)