diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index cc43b9096..c298169c3 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -15,6 +15,7 @@ use BookStack\Entities\Queries\EntityQueries; use BookStack\Exceptions\NotifyException; use BookStack\Facades\Activity; use BookStack\Uploads\AttachmentService; +use BookStack\Uploads\Image; use BookStack\Uploads\ImageService; use BookStack\Util\DatabaseTransaction; use Exception; @@ -217,14 +218,11 @@ class TrashCan ->where('default_template_id', '=', $page->id) ->update(['default_template_id' => null]); - // TODO - Handle related images (uploaded_to for gallery/drawings). - // Should maybe reset to null - // But does that present visibility/permission issues if they used to retain their old - // unused ID? - // If so, might be better to leave them as-is like before, but ensure the maintenance - // cleanup command/action can find these "orphaned" images and delete them. - // But that would leave potential attachment to new pages on increment reset scenarios. - // Need to review permission scenarios for null field values relative to storage options. + // Nullify uploaded image relations + Image::query() + ->whereIn('type', ['gallery', 'drawio']) + ->where('uploaded_to', '=', $page->id) + ->update(['uploaded_to' => null]); $page->forceDelete(); @@ -275,8 +273,8 @@ class TrashCan // exists in the event it has already been destroyed during this request. $entity = $deletion->deletable()->first(); $count = 0; - if ($entity) { - $count = $this->destroyEntity($deletion->deletable); + if ($entity instanceof Entity) { + $count = $this->destroyEntity($entity); } $deletion->delete(); diff --git a/database/migrations/2025_09_15_134751_update_entity_relation_columns.php b/database/migrations/2025_09_15_134751_update_entity_relation_columns.php index 6fbeb1dd1..f8622d7c6 100644 --- a/database/migrations/2025_09_15_134751_update_entity_relation_columns.php +++ b/database/migrations/2025_09_15_134751_update_entity_relation_columns.php @@ -2,6 +2,7 @@ use BookStack\Permissions\JointPermissionBuilder; use Illuminate\Database\Migrations\Migration; +use Illuminate\Database\Query\Builder; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; @@ -66,6 +67,15 @@ return new class extends Migration DB::table('images')->where('uploaded_to', '=', 0)->update(['uploaded_to' => null]); DB::table('activities')->where('loggable_id', '=', 0)->update(['loggable_id' => null]); + // Clean up any orphaned gallery/drawio images to nullify their page relation + DB::table('images') + ->whereIn('type', ['gallery', 'drawio']) + ->whereNotIn('uploaded_to', function (Builder $query) { + $query->select('id') + ->from('entities') + ->where('type', '=', 'page'); + })->update(['uploaded_to' => null]); + // Rebuild joint permissions if needed // This was moved here from 2023_01_24_104625_refactor_joint_permissions_storage since the changes // made for this release would mean our current logic would not be compatible with diff --git a/tests/Entity/PageTest.php b/tests/Entity/PageTest.php index 699414462..b9e1294e0 100644 --- a/tests/Entity/PageTest.php +++ b/tests/Entity/PageTest.php @@ -4,6 +4,7 @@ namespace Tests\Entity; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\Page; +use BookStack\Uploads\Image; use Carbon\Carbon; use Tests\TestCase; @@ -158,6 +159,25 @@ class PageTest extends TestCase ]); } + public function test_page_full_delete_nulls_related_images() + { + $page = $this->entities->page(); + $image = Image::factory()->create(['type' => 'gallery', 'uploaded_to' => $page->id]); + + $this->asEditor()->delete($page->getUrl()); + $this->asAdmin()->post('/settings/recycle-bin/empty'); + + $this->assertDatabaseMissing('images', [ + 'type' => 'gallery', + 'uploaded_to' => $page->id, + ]); + + $this->assertDatabaseHas('images', [ + 'id' => $image->id, + 'uploaded_to' => null, + ]); + } + public function test_page_copy() { $page = $this->entities->page();