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 b5533429517e12f8263cd02f8dfae328b5403c36..7ad78f1d1475d03c6131146335949bb29b29be82 100644 (file)
--- 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 ab62165af1583e146298652e60a984571e12f9ff..015610e717b0571649ced0331b9acc7821a8a076 100644 (file)
--- 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 dc1b2277959fef2437218e9372c34c3c437b5706..900650a70537eaddd6da5b4d68a8d28dc91b1155 100644 (file)
--- 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();