Skip to content

Commit 8898647

Browse files
authored
Merge pull request #5846 from BookStackApp/page_image_nullification
Images: Added nulling of image page relation on page delete
2 parents 0bfd799 + ea63448 commit 8898647

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed

app/Entities/Tools/TrashCan.php

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use BookStack\Exceptions\NotifyException;
1616
use BookStack\Facades\Activity;
1717
use BookStack\Uploads\AttachmentService;
18+
use BookStack\Uploads\Image;
1819
use BookStack\Uploads\ImageService;
1920
use BookStack\Util\DatabaseTransaction;
2021
use Exception;
@@ -217,14 +218,11 @@ protected function destroyPage(Page $page): int
217218
->where('default_template_id', '=', $page->id)
218219
->update(['default_template_id' => null]);
219220

220-
// TODO - Handle related images (uploaded_to for gallery/drawings).
221-
// Should maybe reset to null
222-
// But does that present visibility/permission issues if they used to retain their old
223-
// unused ID?
224-
// If so, might be better to leave them as-is like before, but ensure the maintenance
225-
// cleanup command/action can find these "orphaned" images and delete them.
226-
// But that would leave potential attachment to new pages on increment reset scenarios.
227-
// Need to review permission scenarios for null field values relative to storage options.
221+
// Nullify uploaded image relations
222+
Image::query()
223+
->whereIn('type', ['gallery', 'drawio'])
224+
->where('uploaded_to', '=', $page->id)
225+
->update(['uploaded_to' => null]);
228226

229227
$page->forceDelete();
230228

@@ -275,8 +273,8 @@ public function destroyFromDeletion(Deletion $deletion): int
275273
// exists in the event it has already been destroyed during this request.
276274
$entity = $deletion->deletable()->first();
277275
$count = 0;
278-
if ($entity) {
279-
$count = $this->destroyEntity($deletion->deletable);
276+
if ($entity instanceof Entity) {
277+
$count = $this->destroyEntity($entity);
280278
}
281279
$deletion->delete();
282280

database/migrations/2025_09_15_134751_update_entity_relation_columns.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use BookStack\Permissions\JointPermissionBuilder;
44
use Illuminate\Database\Migrations\Migration;
5+
use Illuminate\Database\Query\Builder;
56
use Illuminate\Database\Schema\Blueprint;
67
use Illuminate\Support\Facades\Schema;
78

@@ -66,6 +67,15 @@ public function up(): void
6667
DB::table('images')->where('uploaded_to', '=', 0)->update(['uploaded_to' => null]);
6768
DB::table('activities')->where('loggable_id', '=', 0)->update(['loggable_id' => null]);
6869

70+
// Clean up any orphaned gallery/drawio images to nullify their page relation
71+
DB::table('images')
72+
->whereIn('type', ['gallery', 'drawio'])
73+
->whereNotIn('uploaded_to', function (Builder $query) {
74+
$query->select('id')
75+
->from('entities')
76+
->where('type', '=', 'page');
77+
})->update(['uploaded_to' => null]);
78+
6979
// Rebuild joint permissions if needed
7080
// This was moved here from 2023_01_24_104625_refactor_joint_permissions_storage since the changes
7181
// made for this release would mean our current logic would not be compatible with

tests/Entity/PageTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use BookStack\Entities\Models\Book;
66
use BookStack\Entities\Models\Page;
7+
use BookStack\Uploads\Image;
78
use Carbon\Carbon;
89
use Tests\TestCase;
910

@@ -158,6 +159,25 @@ public function test_page_full_delete_removes_all_revisions()
158159
]);
159160
}
160161

162+
public function test_page_full_delete_nulls_related_images()
163+
{
164+
$page = $this->entities->page();
165+
$image = Image::factory()->create(['type' => 'gallery', 'uploaded_to' => $page->id]);
166+
167+
$this->asEditor()->delete($page->getUrl());
168+
$this->asAdmin()->post('/settings/recycle-bin/empty');
169+
170+
$this->assertDatabaseMissing('images', [
171+
'type' => 'gallery',
172+
'uploaded_to' => $page->id,
173+
]);
174+
175+
$this->assertDatabaseHas('images', [
176+
'id' => $image->id,
177+
'uploaded_to' => null,
178+
]);
179+
}
180+
161181
public function test_page_copy()
162182
{
163183
$page = $this->entities->page();

0 commit comments

Comments
 (0)