From: Dan Brown Date: Mon, 10 Jan 2022 17:04:01 +0000 (+0000) Subject: Improved custom homepage check on item deletion X-Git-Tag: v21.12.2~1^2~5 X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/8d91f4369b418aa922e0246fdc57e4755936077d?ds=inline 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 --- 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();