From 8d91f4369b418aa922e0246fdc57e4755936077d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 10 Jan 2022 17:04:01 +0000 Subject: [PATCH] Improved custom homepage check on item deletion Custom homepage usage will now be checked before any actioning of deletion rather than potentially causing an exception acting during the deletion. Previously a deletion could still be created, within the recycle bin, for the parent which may lead to the page being deleted anyway. For #3150 --- app/Entities/Models/Entity.php | 1 + app/Entities/Tools/TrashCan.php | 46 ++++++++++++++++++++++++++++----- tests/HomepageTest.php | 18 +++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php index b55334295..7ad78f1d1 100644 --- a/app/Entities/Models/Entity.php +++ b/app/Entities/Models/Entity.php @@ -36,6 +36,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @property string $slug * @property Carbon $created_at * @property Carbon $updated_at + * @property Carbon $deleted_at * @property int $created_by * @property int $updated_by * @property bool $restricted diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index ab62165af..015610e71 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -22,9 +22,11 @@ class TrashCan { /** * Send a shelf to the recycle bin. + * @throws NotifyException */ public function softDestroyShelf(Bookshelf $shelf) { + $this->ensureDeletable($shelf); Deletion::createForEntity($shelf); $shelf->delete(); } @@ -36,6 +38,7 @@ class TrashCan */ public function softDestroyBook(Book $book) { + $this->ensureDeletable($book); Deletion::createForEntity($book); foreach ($book->pages as $page) { @@ -57,6 +60,7 @@ class TrashCan public function softDestroyChapter(Chapter $chapter, bool $recordDelete = true) { if ($recordDelete) { + $this->ensureDeletable($chapter); Deletion::createForEntity($chapter); } @@ -77,19 +81,47 @@ class TrashCan public function softDestroyPage(Page $page, bool $recordDelete = true) { if ($recordDelete) { + $this->ensureDeletable($page); Deletion::createForEntity($page); } - // Check if set as custom homepage & remove setting if not used or throw error if active - $customHome = setting('app-homepage', '0:'); - if (intval($page->id) === intval(explode(':', $customHome)[0])) { - if (setting('app-homepage-type') === 'page') { - throw new NotifyException(trans('errors.page_custom_home_deletion'), $page->getUrl()); + $page->delete(); + } + + /** + * Ensure the given entity is deletable. + * Is not for permissions, but logical conditions within the application. + * Will throw if not deletable. + * + * @throws NotifyException + */ + protected function ensureDeletable(Entity $entity): void + { + $customHomeId = intval(explode(':', setting('app-homepage', '0:'))[0]); + $customHomeActive = setting('app-homepage-type') === 'page'; + $removeCustomHome = false; + + // Check custom homepage usage for pages + if ($entity instanceof Page && $entity->id === $customHomeId) { + if ($customHomeActive) { + throw new NotifyException(trans('errors.page_custom_home_deletion'), $entity->getUrl()); } - setting()->remove('app-homepage'); + $removeCustomHome = true; } - $page->delete(); + // Check custom homepage usage within chapters or books + if ($entity instanceof Chapter || $entity instanceof Book) { + if ($entity->pages()->where('id', '=', $customHomeId)->exists()) { + if ($customHomeActive) { + throw new NotifyException(trans('errors.page_custom_home_deletion'), $entity->getUrl()); + } + $removeCustomHome = true; + } + } + + if ($removeCustomHome) { + setting()->remove('app-homepage'); + } } /** diff --git a/tests/HomepageTest.php b/tests/HomepageTest.php index dc1b22779..900650a70 100644 --- a/tests/HomepageTest.php +++ b/tests/HomepageTest.php @@ -79,6 +79,24 @@ class HomepageTest extends TestCase $pageDeleteReq->assertSessionMissing('error'); } + public function test_custom_homepage_cannot_be_deleted_from_parent_deletion() + { + /** @var Page $page */ + $page = Page::query()->first(); + $this->setSettings([ + 'app-homepage' => $page->id, + 'app-homepage-type' => 'page', + ]); + + $this->asEditor()->delete($page->book->getUrl()); + $this->assertSessionError('Cannot delete a page while it is set as a homepage'); + $this->assertDatabaseMissing('deletions', ['deletable_id' => $page->book->id]); + + $page->refresh(); + $this->assertNull($page->deleted_at); + $this->assertNull($page->book->deleted_at); + } + public function test_custom_homepage_renders_includes() { $this->asEditor();