8000 [Bugfix] Ensure Eloquent delete events fire on soft-delete · Hy0320/laravel-json-api@2085435 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2085435

Browse files
lindyhopchrislindyhopchris
committed
[Bugfix] Ensure Eloquent delete events fire on soft-delete
Closes cloudcreativity#371
1 parent cfc7298 commit 2085435

File tree

4 files changed

+88
-8
lines changed

4 files changed

+88
-8
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ Can now set an API's default controller name to the singular version of the reso
1212
Can now change the JSON API controller's default connection and whether it uses transactions
1313
via an API's config.
1414

15+
### Fixed
16+
- [#371](https://github.com/cloudcreativity/laravel-json-api/issues/371)
17+
Ensure Eloquent delete/deleting events are fired when soft-deleting a resource.
18+
- Eloquent adapter hooks will now be invoked when force-deleting a resource that uses
19+
the `SoftDeletesModels` trait.
20+
1521
## [1.2.0] - 2019-06-20
1622

1723
### Added

‎composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
"require-dev": {
4141
"ext-sqlite3": "*",
4242
"cloudcreativity/json-api-testing": "^1.0.0",
43+
"composer/semver": "^1.5",
4344
"guzzlehttp/guzzle": "^6.3",
4445
"mockery/mockery": "^1.1",
4546
"orchestra/testbench": "3.5.*|3.6.*|3.7.*|3.8.*",

‎src/Eloquent/Concerns/SoftDeletesModels.php

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use Illuminate\Database\Schema\Builder;
2424
use Illuminate\Support\Arr;
2525
use Illuminate\Support\Collection;
26-
use Neomerx\JsonApi\Contracts\Encoder\Parameters\EncodingParametersInterface;
2726

2827
/**
2928
* Trait SoftDeletesModels
@@ -35,10 +34,9 @@ trait SoftDeletesModels
3534

3635
/**
3736
* @param Model $record
38-
* @param EncodingParametersInterface $params
3937
* @return bool
4038
*/
41-
public function delete($record, EncodingParametersInterface $params)
39+
protected function destroy($record)
4240
{
4341
return (bool) $record->forceDelete();
4442
}
@@ -166,14 +164,34 @@ protected function persist($record)
166164

167165
/**
168166
* @param Model $record
167+
* @return void
169168
*/
170169
protected function saveOrRestore(Model $record)
171170
{
172171
if ($this->willRestore($record)) {
173172
$record->restore();
174-
} else {
175-
$record->save();
173+
return;
174+
}
175+
176+
/**
177+
* To ensure Laravel still executes its soft-delete logic (e.g. firing events)
178+
* we need to delete before a save when we are soft-deleting. Although this
179+
* may result in two database calls in this scenario, it means we can guarantee
180+
* that standard Laravel soft-delete logic is executed.
181+
*
182+
* @see https://github.com/cloudcreativity/laravel-json-api/issues/371
183+
*/
184+
if ($this->willSoftDelete($record)) {
185+
$key = $this->getSoftDeleteKey($record);
186+
// save the original date so we can put it back later on
187+
$deletedAt = $record{$key};
188+
// delete on the record so that deleting and deleted events get fired
189+
$record->delete();
190+
// apply the original soft deleting date back before saving
191+
$record{$key} = $deletedAt;
176192
}
193+
194+
$record->save();
177195
}
178196

179197
/**
@@ -190,7 +208,30 @@ protected function willRestore(Model $record)
190208
return false;
191209
}
192210

211+
/**
212+
* The use of `trashed()` here looks the wrong way round, but it is
213+
* because that method checks the current value on the model. I.e.
214+
* as we have filled the model by this point, it will think that it
215+
* is not trashed even though that has not been persisted to the database.
216+
*/
193217
return $record->isDirty($key) && !$record->trashed();
194218
}
195219

220+
/**
221+
* @param Model $record
222+
* @return bool
223+
*/
224+
protected function willSoftDelete(Model $record)
225+
{
226+
if (!$record->exists) {
227+
return false;
228+
}
229+
230+
if (!$key = $this->getSoftDeleteKey($record)) {
231+
return false;
232+
}
233+
234+
return null === $record->getOriginal($key) && $record->trashed();
235+
}
236+
196237
}

‎tests/lib/Integration/Eloquent/ResourceTest.php

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
use Carbon\Carbon;
2121
use CloudCreativity\LaravelJsonApi\Tests\Integration\TestCase;
22+
use Composer\Semver\Semver;
2223
use DummyApp\Comment;
2324
use DummyApp\Post;
2425
use DummyApp\Tag;
@@ -497,6 +498,8 @@ public function testInvalidDateTime()
497498

498499
public function testSoftDelete()
499500
{
501+
Event::fake();
502+
500503
$post = factory(Post::class)->create();
501504

502505
$data = [
@@ -509,6 +512,16 @@ public function testSoftDelete()
509512

510513
$this->doUpdate($data)->assertUpdated($data);
511514
$this->assertSoftDeleted('posts', [$post->getKeyName() => $post->getKey()]);
515+
516+
Event::assertDispatched("eloquent.deleting: " . Post::class, function ($name, $actual) use ($post) {
517+
return $post->is($actual);
518+
});
519+
520+
Event::assertDispatched("eloquent.deleted: " . Post::class, function ($name, $actual) use ($post) {
521+
return $post->is($actual);
522+
});
523+
524+
Event::assertNotDispatched("eloquent.forceDeleted: " . Post::class);
512525
}
513526

514527
public function testSoftDeleteWithBoolean()
@@ -645,10 +658,29 @@ public function testUpdateAndRestore()
645658
*/
646659
public function testDelete()
647660
{
648-
$model = $this->createPost();
661+
Event::fake();
662+
663+
$post = $this->createPost();
664+
665+
$this->doDelete($post)->assertDeleted();
666+
$this->assertDatabaseMissing('posts', [$post->getKeyName() => $post->getKey()]);
667+
668+
Event::assertDispatched("eloquent.deleting: " . Post::class, function ($name, $actual) use ($post) {
669+
return $post->is($actual);
670+
});
671+
672+
Event::assertDispatched("eloquent.deleted: " . Post::class, function ($name, $actual) use ($post) {
673+
return $post->is($actual);
674+
});
649675

650-
$this->doDelete($model)->assertDeleted();
651-
$this->assertDatabaseMissing('posts', [$model->getKeyName() => $model->getKey()]);
676+
/**
677+
* Force deleted event was added in Laravel 5.6.
678+
*/
679+
if (Semver::satisfies($this->app->version(), '>=5.6')) {
680+
Event::assertDispatched("eloquent.forceDeleted: " . Post::class, function ($name, $actual) use ($post) {
681+
return $post->is($actual);
682+
4B95 });
683+
}
652684
}
653685

654686
/**

0 commit comments

Comments
 (0)
0