From f073994bc3d290d88aefca623ccdd2d2b51d5060 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 25 Nov 2025 12:36:33 +0000 Subject: [PATCH 1/5] Testing: Extracted copy tests to their own class --- tests/Entity/BookTest.php | 104 ------------- tests/Entity/ChapterTest.php | 83 ---------- tests/Entity/CopyTest.php | 294 +++++++++++++++++++++++++++++++++++ tests/Entity/PageTest.php | 91 ----------- 4 files changed, 294 insertions(+), 278 deletions(-) create mode 100644 tests/Entity/CopyTest.php diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php index 545d6b30578..a7142f03736 100644 --- a/tests/Entity/BookTest.php +++ b/tests/Entity/BookTest.php @@ -264,108 +264,4 @@ public function test_show_view_displays_description_if_no_description_html_set() $resp = $this->asEditor()->get($book->getUrl()); $resp->assertSee("

My great
\ndescription
\n
\nwith newlines

", false); } - - public function test_show_view_has_copy_button() - { - $book = $this->entities->book(); - $resp = $this->asEditor()->get($book->getUrl()); - - $this->withHtml($resp)->assertElementContains("a[href=\"{$book->getUrl('/copy')}\"]", 'Copy'); - } - - public function test_copy_view() - { - $book = $this->entities->book(); - $resp = $this->asEditor()->get($book->getUrl('/copy')); - - $resp->assertOk(); - $resp->assertSee('Copy Book'); - $this->withHtml($resp)->assertElementExists("input[name=\"name\"][value=\"{$book->name}\"]"); - } - - public function test_copy() - { - /** @var Book $book */ - $book = Book::query()->whereHas('chapters')->whereHas('pages')->first(); - $resp = $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']); - - /** @var Book $copy */ - $copy = Book::query()->where('name', '=', 'My copy book')->first(); - - $resp->assertRedirect($copy->getUrl()); - $this->assertEquals($book->getDirectVisibleChildren()->count(), $copy->getDirectVisibleChildren()->count()); - - $this->get($copy->getUrl())->assertSee($book->description_html, false); - } - - public function test_copy_does_not_copy_non_visible_content() - { - /** @var Book $book */ - $book = Book::query()->whereHas('chapters')->whereHas('pages')->first(); - - // Hide child content - /** @var BookChild $page */ - foreach ($book->getDirectVisibleChildren() as $child) { - $this->permissions->setEntityPermissions($child, [], []); - } - - $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']); - /** @var Book $copy */ - $copy = Book::query()->where('name', '=', 'My copy book')->first(); - - $this->assertEquals(0, $copy->getDirectVisibleChildren()->count()); - } - - public function test_copy_does_not_copy_pages_or_chapters_if_user_cant_create() - { - /** @var Book $book */ - $book = Book::query()->whereHas('chapters')->whereHas('directPages')->whereHas('chapters')->first(); - $viewer = $this->users->viewer(); - $this->permissions->grantUserRolePermissions($viewer, ['book-create-all']); - - $this->actingAs($viewer)->post($book->getUrl('/copy'), ['name' => 'My copy book']); - /** @var Book $copy */ - $copy = Book::query()->where('name', '=', 'My copy book')->first(); - - $this->assertEquals(0, $copy->pages()->count()); - $this->assertEquals(0, $copy->chapters()->count()); - } - - public function test_copy_clones_cover_image_if_existing() - { - $book = $this->entities->book(); - $bookRepo = $this->app->make(BookRepo::class); - $coverImageFile = $this->files->uploadedImage('cover.png'); - $bookRepo->updateCoverImage($book, $coverImageFile); - - $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book'])->assertRedirect(); - /** @var Book $copy */ - $copy = Book::query()->where('name', '=', 'My copy book')->first(); - - $this->assertNotNull($copy->coverInfo()->getImage()); - $this->assertNotEquals($book->coverInfo()->getImage()->id, $copy->coverInfo()->getImage()->id); - } - - public function test_copy_adds_book_to_shelves_if_edit_permissions_allows() - { - /** @var Bookshelf $shelfA */ - /** @var Bookshelf $shelfB */ - [$shelfA, $shelfB] = Bookshelf::query()->take(2)->get(); - $book = $this->entities->book(); - - $shelfA->appendBook($book); - $shelfB->appendBook($book); - - $viewer = $this->users->viewer(); - $this->permissions->grantUserRolePermissions($viewer, ['book-update-all', 'book-create-all', 'bookshelf-update-all']); - $this->permissions->setEntityPermissions($shelfB); - - - $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']); - /** @var Book $copy */ - $copy = Book::query()->where('name', '=', 'My copy book')->first(); - - $this->assertTrue($copy->shelves()->where('id', '=', $shelfA->id)->exists()); - $this->assertFalse($copy->shelves()->where('id', '=', $shelfB->id)->exists()); - } } diff --git a/tests/Entity/ChapterTest.php b/tests/Entity/ChapterTest.php index 1577cee76d8..0c0ec784135 100644 --- a/tests/Entity/ChapterTest.php +++ b/tests/Entity/ChapterTest.php @@ -66,90 +66,7 @@ public function test_delete() $this->assertNotificationContains($redirectReq, 'Chapter Successfully Deleted'); } - public function test_show_view_has_copy_button() - { - $chapter = $this->entities->chapter(); - - $resp = $this->asEditor()->get($chapter->getUrl()); - $this->withHtml($resp)->assertElementContains("a[href$=\"{$chapter->getUrl('/copy')}\"]", 'Copy'); - } - - public function test_copy_view() - { - $chapter = $this->entities->chapter(); - - $resp = $this->asEditor()->get($chapter->getUrl('/copy')); - $resp->assertOk(); - $resp->assertSee('Copy Chapter'); - $this->withHtml($resp)->assertElementExists("input[name=\"name\"][value=\"{$chapter->name}\"]"); - $this->withHtml($resp)->assertElementExists('input[name="entity_selection"]'); - } - - public function test_copy() - { - /** @var Chapter $chapter */ - $chapter = Chapter::query()->whereHas('pages')->first(); - /** @var Book $otherBook */ - $otherBook = Book::query()->where('id', '!=', $chapter->book_id)->first(); - - $resp = $this->asEditor()->post($chapter->getUrl('/copy'), [ - 'name' => 'My copied chapter', - 'entity_selection' => 'book:' . $otherBook->id, - ]); - - /** @var Chapter $newChapter */ - $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); - - $resp->assertRedirect($newChapter->getUrl()); - $this->assertEquals($otherBook->id, $newChapter->book_id); - $this->assertEquals($chapter->pages->count(), $newChapter->pages->count()); - } - - public function test_copy_does_not_copy_non_visible_pages() - { - $chapter = $this->entities->chapterHasPages(); - // Hide pages to all non-admin roles - /** @var Page $page */ - foreach ($chapter->pages as $page) { - $this->permissions->setEntityPermissions($page, [], []); - } - - $this->asEditor()->post($chapter->getUrl('/copy'), [ - 'name' => 'My copied chapter', - ]); - - /** @var Chapter $newChapter */ - $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); - $this->assertEquals(0, $newChapter->pages()->count()); - } - - public function test_copy_does_not_copy_pages_if_user_cant_page_create() - { - $chapter = $this->entities->chapterHasPages(); - $viewer = $this->users->viewer(); - $this->permissions->grantUserRolePermissions($viewer, ['chapter-create-all']); - - // Lacking permission results in no copied pages - $this->actingAs($viewer)->post($chapter->getUrl('/copy'), [ - 'name' => 'My copied chapter', - ]); - - /** @var Chapter $newChapter */ - $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); - $this->assertEquals(0, $newChapter->pages()->count()); - - $this->permissions->grantUserRolePermissions($viewer, ['page-create-all']); - - // Having permission rules in copied pages - $this->actingAs($viewer)->post($chapter->getUrl('/copy'), [ - 'name' => 'My copied again chapter', - ]); - - /** @var Chapter $newChapter2 */ - $newChapter2 = Chapter::query()->where('name', '=', 'My copied again chapter')->first(); - $this->assertEquals($chapter->pages()->count(), $newChapter2->pages()->count()); - } public function test_sort_book_action_visible_if_permissions_allow() { diff --git a/tests/Entity/CopyTest.php b/tests/Entity/CopyTest.php new file mode 100644 index 00000000000..353808beacb --- /dev/null +++ b/tests/Entity/CopyTest.php @@ -0,0 +1,294 @@ +entities->book(); + $resp = $this->asEditor()->get($book->getUrl()); + + $this->withHtml($resp)->assertElementContains("a[href=\"{$book->getUrl('/copy')}\"]", 'Copy'); + } + + public function test_book_copy_view() + { + $book = $this->entities->book(); + $resp = $this->asEditor()->get($book->getUrl('/copy')); + + $resp->assertOk(); + $resp->assertSee('Copy Book'); + $this->withHtml($resp)->assertElementExists("input[name=\"name\"][value=\"{$book->name}\"]"); + } + + public function test_book_copy() + { + /** @var Book $book */ + $book = Book::query()->whereHas('chapters')->whereHas('pages')->first(); + $resp = $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']); + + /** @var Book $copy */ + $copy = Book::query()->where('name', '=', 'My copy book')->first(); + + $resp->assertRedirect($copy->getUrl()); + $this->assertEquals($book->getDirectVisibleChildren()->count(), $copy->getDirectVisibleChildren()->count()); + + $this->get($copy->getUrl())->assertSee($book->description_html, false); + } + + public function test_book_copy_does_not_copy_non_visible_content() + { + /** @var Book $book */ + $book = Book::query()->whereHas('chapters')->whereHas('pages')->first(); + + // Hide child content + /** @var BookChild $page */ + foreach ($book->getDirectVisibleChildren() as $child) { + $this->permissions->setEntityPermissions($child, [], []); + } + + $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']); + /** @var Book $copy */ + $copy = Book::query()->where('name', '=', 'My copy book')->first(); + + $this->assertEquals(0, $copy->getDirectVisibleChildren()->count()); + } + + public function test_book_copy_does_not_copy_pages_or_chapters_if_user_cant_create() + { + /** @var Book $book */ + $book = Book::query()->whereHas('chapters')->whereHas('directPages')->whereHas('chapters')->first(); + $viewer = $this->users->viewer(); + $this->permissions->grantUserRolePermissions($viewer, ['book-create-all']); + + $this->actingAs($viewer)->post($book->getUrl('/copy'), ['name' => 'My copy book']); + /** @var Book $copy */ + $copy = Book::query()->where('name', '=', 'My copy book')->first(); + + $this->assertEquals(0, $copy->pages()->count()); + $this->assertEquals(0, $copy->chapters()->count()); + } + + public function test_book_copy_clones_cover_image_if_existing() + { + $book = $this->entities->book(); + $bookRepo = $this->app->make(BookRepo::class); + $coverImageFile = $this->files->uploadedImage('cover.png'); + $bookRepo->updateCoverImage($book, $coverImageFile); + + $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book'])->assertRedirect(); + /** @var Book $copy */ + $copy = Book::query()->where('name', '=', 'My copy book')->first(); + + $this->assertNotNull($copy->coverInfo()->getImage()); + $this->assertNotEquals($book->coverInfo()->getImage()->id, $copy->coverInfo()->getImage()->id); + } + + public function test_book_copy_adds_book_to_shelves_if_edit_permissions_allows() + { + /** @var Bookshelf $shelfA */ + /** @var Bookshelf $shelfB */ + [$shelfA, $shelfB] = Bookshelf::query()->take(2)->get(); + $book = $this->entities->book(); + + $shelfA->appendBook($book); + $shelfB->appendBook($book); + + $viewer = $this->users->viewer(); + $this->permissions->grantUserRolePermissions($viewer, ['book-update-all', 'book-create-all', 'bookshelf-update-all']); + $this->permissions->setEntityPermissions($shelfB); + + + $this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']); + /** @var Book $copy */ + $copy = Book::query()->where('name', '=', 'My copy book')->first(); + + $this->assertTrue($copy->shelves()->where('id', '=', $shelfA->id)->exists()); + $this->assertFalse($copy->shelves()->where('id', '=', $shelfB->id)->exists()); + } + + public function test_chapter_show_view_has_copy_button() + { + $chapter = $this->entities->chapter(); + + $resp = $this->asEditor()->get($chapter->getUrl()); + $this->withHtml($resp)->assertElementContains("a[href$=\"{$chapter->getUrl('/copy')}\"]", 'Copy'); + } + + public function test_chapter_copy_view() + { + $chapter = $this->entities->chapter(); + + $resp = $this->asEditor()->get($chapter->getUrl('/copy')); + $resp->assertOk(); + $resp->assertSee('Copy Chapter'); + $this->withHtml($resp)->assertElementExists("input[name=\"name\"][value=\"{$chapter->name}\"]"); + $this->withHtml($resp)->assertElementExists('input[name="entity_selection"]'); + } + + public function test_chapter_copy() + { + /** @var Chapter $chapter */ + $chapter = Chapter::query()->whereHas('pages')->first(); + /** @var Book $otherBook */ + $otherBook = Book::query()->where('id', '!=', $chapter->book_id)->first(); + + $resp = $this->asEditor()->post($chapter->getUrl('/copy'), [ + 'name' => 'My copied chapter', + 'entity_selection' => 'book:' . $otherBook->id, + ]); + + /** @var Chapter $newChapter */ + $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); + + $resp->assertRedirect($newChapter->getUrl()); + $this->assertEquals($otherBook->id, $newChapter->book_id); + $this->assertEquals($chapter->pages->count(), $newChapter->pages->count()); + } + + public function test_chapter_copy_does_not_copy_non_visible_pages() + { + $chapter = $this->entities->chapterHasPages(); + + // Hide pages to all non-admin roles + /** @var Page $page */ + foreach ($chapter->pages as $page) { + $this->permissions->setEntityPermissions($page, [], []); + } + + $this->asEditor()->post($chapter->getUrl('/copy'), [ + 'name' => 'My copied chapter', + ]); + + /** @var Chapter $newChapter */ + $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); + $this->assertEquals(0, $newChapter->pages()->count()); + } + + public function test_chapter_copy_does_not_copy_pages_if_user_cant_page_create() + { + $chapter = $this->entities->chapterHasPages(); + $viewer = $this->users->viewer(); + $this->permissions->grantUserRolePermissions($viewer, ['chapter-create-all']); + + // Lacking permission results in no copied pages + $this->actingAs($viewer)->post($chapter->getUrl('/copy'), [ + 'name' => 'My copied chapter', + ]); + + /** @var Chapter $newChapter */ + $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); + $this->assertEquals(0, $newChapter->pages()->count()); + + $this->permissions->grantUserRolePermissions($viewer, ['page-create-all']); + + // Having permission rules in copied pages + $this->actingAs($viewer)->post($chapter->getUrl('/copy'), [ + 'name' => 'My copied again chapter', + ]); + + /** @var Chapter $newChapter2 */ + $newChapter2 = Chapter::query()->where('name', '=', 'My copied again chapter')->first(); + $this->assertEquals($chapter->pages()->count(), $newChapter2->pages()->count()); + } + + public function test_page_copy() + { + $page = $this->entities->page(); + $page->html = '

This is some test content

'; + $page->save(); + + $currentBook = $page->book; + $newBook = Book::where('id', '!=', $currentBook->id)->first(); + + $resp = $this->asEditor()->get($page->getUrl('/copy')); + $resp->assertSee('Copy Page'); + + $movePageResp = $this->post($page->getUrl('/copy'), [ + 'entity_selection' => 'book:' . $newBook->id, + 'name' => 'My copied test page', + ]); + $pageCopy = Page::where('name', '=', 'My copied test page')->first(); + + $movePageResp->assertRedirect($pageCopy->getUrl()); + $this->assertTrue($pageCopy->book->id == $newBook->id, 'Page was copied to correct book'); + $this->assertStringContainsString('This is some test content', $pageCopy->html); + } + + public function test_page_copy_with_markdown_has_both_html_and_markdown() + { + $page = $this->entities->page(); + $page->html = '

This is some test content

'; + $page->markdown = '# This is some test content'; + $page->save(); + $newBook = Book::where('id', '!=', $page->book->id)->first(); + + $this->asEditor()->post($page->getUrl('/copy'), [ + 'entity_selection' => 'book:' . $newBook->id, + 'name' => 'My copied test page', + ]); + $pageCopy = Page::where('name', '=', 'My copied test page')->first(); + + $this->assertStringContainsString('This is some test content', $pageCopy->html); + $this->assertEquals('# This is some test content', $pageCopy->markdown); + } + + public function test_page_copy_with_no_destination() + { + $page = $this->entities->page(); + $currentBook = $page->book; + + $resp = $this->asEditor()->get($page->getUrl('/copy')); + $resp->assertSee('Copy Page'); + + $movePageResp = $this->post($page->getUrl('/copy'), [ + 'name' => 'My copied test page', + ]); + + $pageCopy = Page::where('name', '=', 'My copied test page')->first(); + + $movePageResp->assertRedirect($pageCopy->getUrl()); + $this->assertTrue($pageCopy->book->id == $currentBook->id, 'Page was copied to correct book'); + $this->assertTrue($pageCopy->id !== $page->id, 'Page copy is not the same instance'); + } + + public function test_page_can_be_copied_without_edit_permission() + { + $page = $this->entities->page(); + $currentBook = $page->book; + $newBook = Book::where('id', '!=', $currentBook->id)->first(); + $viewer = $this->users->viewer(); + + $resp = $this->actingAs($viewer)->get($page->getUrl()); + $resp->assertDontSee($page->getUrl('/copy')); + + $newBook->owned_by = $viewer->id; + $newBook->save(); + $this->permissions->grantUserRolePermissions($viewer, ['page-create-own']); + $this->permissions->regenerateForEntity($newBook); + + $resp = $this->actingAs($viewer)->get($page->getUrl()); + $resp->assertSee($page->getUrl('/copy')); + + $movePageResp = $this->post($page->getUrl('/copy'), [ + 'entity_selection' => 'book:' . $newBook->id, + 'name' => 'My copied test page', + ]); + $movePageResp->assertRedirect(); + + $this->assertDatabaseHasEntityData('page', [ + 'name' => 'My copied test page', + 'created_by' => $viewer->id, + 'book_id' => $newBook->id, + ]); + } +} diff --git a/tests/Entity/PageTest.php b/tests/Entity/PageTest.php index afe15f4d4a3..1b2f3c9fe5a 100644 --- a/tests/Entity/PageTest.php +++ b/tests/Entity/PageTest.php @@ -178,97 +178,6 @@ public function test_page_full_delete_nulls_related_images() ]); } - public function test_page_copy() - { - $page = $this->entities->page(); - $page->html = '

This is some test content

'; - $page->save(); - - $currentBook = $page->book; - $newBook = Book::where('id', '!=', $currentBook->id)->first(); - - $resp = $this->asEditor()->get($page->getUrl('/copy')); - $resp->assertSee('Copy Page'); - - $movePageResp = $this->post($page->getUrl('/copy'), [ - 'entity_selection' => 'book:' . $newBook->id, - 'name' => 'My copied test page', - ]); - $pageCopy = Page::where('name', '=', 'My copied test page')->first(); - - $movePageResp->assertRedirect($pageCopy->getUrl()); - $this->assertTrue($pageCopy->book->id == $newBook->id, 'Page was copied to correct book'); - $this->assertStringContainsString('This is some test content', $pageCopy->html); - } - - public function test_page_copy_with_markdown_has_both_html_and_markdown() - { - $page = $this->entities->page(); - $page->html = '

This is some test content

'; - $page->markdown = '# This is some test content'; - $page->save(); - $newBook = Book::where('id', '!=', $page->book->id)->first(); - - $this->asEditor()->post($page->getUrl('/copy'), [ - 'entity_selection' => 'book:' . $newBook->id, - 'name' => 'My copied test page', - ]); - $pageCopy = Page::where('name', '=', 'My copied test page')->first(); - - $this->assertStringContainsString('This is some test content', $pageCopy->html); - $this->assertEquals('# This is some test content', $pageCopy->markdown); - } - - public function test_page_copy_with_no_destination() - { - $page = $this->entities->page(); - $currentBook = $page->book; - - $resp = $this->asEditor()->get($page->getUrl('/copy')); - $resp->assertSee('Copy Page'); - - $movePageResp = $this->post($page->getUrl('/copy'), [ - 'name' => 'My copied test page', - ]); - - $pageCopy = Page::where('name', '=', 'My copied test page')->first(); - - $movePageResp->assertRedirect($pageCopy->getUrl()); - $this->assertTrue($pageCopy->book->id == $currentBook->id, 'Page was copied to correct book'); - $this->assertTrue($pageCopy->id !== $page->id, 'Page copy is not the same instance'); - } - - public function test_page_can_be_copied_without_edit_permission() - { - $page = $this->entities->page(); - $currentBook = $page->book; - $newBook = Book::where('id', '!=', $currentBook->id)->first(); - $viewer = $this->users->viewer(); - - $resp = $this->actingAs($viewer)->get($page->getUrl()); - $resp->assertDontSee($page->getUrl('/copy')); - - $newBook->owned_by = $viewer->id; - $newBook->save(); - $this->permissions->grantUserRolePermissions($viewer, ['page-create-own']); - $this->permissions->regenerateForEntity($newBook); - - $resp = $this->actingAs($viewer)->get($page->getUrl()); - $resp->assertSee($page->getUrl('/copy')); - - $movePageResp = $this->post($page->getUrl('/copy'), [ - 'entity_selection' => 'book:' . $newBook->id, - 'name' => 'My copied test page', - ]); - $movePageResp->assertRedirect(); - - $this->assertDatabaseHasEntityData('page', [ - 'name' => 'My copied test page', - 'created_by' => $viewer->id, - 'book_id' => $newBook->id, - ]); - } - public function test_page_within_chapter_deletion_returns_to_chapter() { $chapter = $this->entities->chapter(); From ba675b6349f3e9fd464dba02557c33441a5e3a96 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 25 Nov 2025 13:52:36 +0000 Subject: [PATCH 2/5] Copying: Added tests to cover copy self-references Logic to make tests pass to follow --- tests/Entity/CopyTest.php | 86 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/Entity/CopyTest.php b/tests/Entity/CopyTest.php index 353808beacb..258b538f5aa 100644 --- a/tests/Entity/CopyTest.php +++ b/tests/Entity/CopyTest.php @@ -201,6 +201,92 @@ public function test_chapter_copy_does_not_copy_pages_if_user_cant_page_create() $this->assertEquals($chapter->pages()->count(), $newChapter2->pages()->count()); } + public function test_book_copy_updates_internal_references() + { + $book = $this->entities->bookHasChaptersAndPages(); + /** @var Chapter $chapter */ + $chapter = $book->chapters()->first(); + /** @var Page $page */ + $page = $chapter->pages()->first(); + $this->asEditor(); + $this->entities->updatePage($page, [ + 'name' => 'reference test page', + 'html' => '

This is a test book link

', + ]); + + $html = '

This is a test page link

'; + + // Quick pre-update to get stable slug + $this->put($book->getUrl(), ['name' => 'Internal ref test']); + $book->refresh(); + + $this->put($book->getUrl(), ['name' => 'Internal ref test', 'description_html' => $html]); + + $this->post($book->getUrl('/copy'), ['name' => 'My copied book']); + + $newBook = Book::query()->where('name', '=', 'My copied book')->first(); + $newPage = $newBook->pages()->where('name', '=', 'reference test page')->first(); + + $this->assertStringContainsString($newBook->getUrl(), $newPage->html); + $this->assertStringContainsString($newPage->getUrl(), $newBook->description_html); + + $this->assertStringNotContainsString($book->getUrl(), $newPage->html); + $this->assertStringNotContainsString($page->getUrl(), $newBook->description_html); + } + + public function test_chapter_copy_updates_internal_references() + { + $chapter = $this->entities->chapterHasPages(); + /** @var Page $page */ + $page = $chapter->pages()->first(); + $this->asEditor(); + $this->entities->updatePage($page, [ + 'name' => 'reference test page', + 'html' => '

This is a test chapter link

', + ]); + + $html = '

This is a test page link

'; + + // Quick pre-update to get stable slug + $this->put($chapter->getUrl(), ['name' => 'Internal ref test']); + $chapter->refresh(); + + $this->put($chapter->getUrl(), ['name' => 'Internal ref test', 'description_html' => $html]); + + $this->post($chapter->getUrl('/copy'), ['name' => 'My copied chapter']); + + $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); + $newPage = $newChapter->pages()->where('name', '=', 'reference test page')->first(); + + $this->assertStringContainsString($newChapter->getUrl(), $newPage->html); + $this->assertStringContainsString($newPage->getUrl(), $newChapter->description_html); + + $this->assertStringNotContainsString($chapter->getUrl(), $newPage->html); + $this->assertStringNotContainsString($page->getUrl(), $newChapter->description_html); + } + + public function test_page_copy_updates_internal_self_references() + { + $page = $this->entities->page(); + $this->asEditor(); + + // Initial update to get stable slug + $this->entities->updatePage($page, ['name' => 'reference test page']); + + $page->refresh(); + $this->entities->updatePage($page, [ + 'name' => 'reference test page', + 'html' => '

This is a test page link

', + ]); + + $this->post($page->getUrl('/copy'), ['name' => 'My copied page']); + $newPage = Page::query()->where('name', '=', 'My copied page')->first(); + $this->assertNotNull($newPage); + + $this->assertStringContainsString($newPage->getUrl(), $newPage->html); + $this->assertStringNotContainsString($page->getUrl(), $newPage->html); + } + public function test_page_copy() { $page = $this->entities->page(); From 674bb84fac98a73820217a0a43c3fb732919899a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 25 Nov 2025 14:46:36 +0000 Subject: [PATCH 3/5] Copying: Added reference change context tracking Added core wiring in the cloning logic, just need to implement core logic in the updater now. --- app/Entities/Tools/Cloner.php | 41 ++++++++++++++++++++++- app/References/ReferenceChangeContext.php | 19 +++++++++++ app/References/ReferenceUpdater.php | 8 +++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 app/References/ReferenceChangeContext.php diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php index ff42ae6e41b..916ce42c572 100644 --- a/app/Entities/Tools/Cloner.php +++ b/app/Entities/Tools/Cloner.php @@ -13,30 +13,47 @@ use BookStack\Entities\Repos\ChapterRepo; use BookStack\Entities\Repos\PageRepo; use BookStack\Permissions\Permission; +use BookStack\References\ReferenceChangeContext; +use BookStack\References\ReferenceUpdater; use BookStack\Uploads\Image; use BookStack\Uploads\ImageService; use Illuminate\Http\UploadedFile; class Cloner { + protected ReferenceChangeContext $referenceChangeContext; + public function __construct( protected PageRepo $pageRepo, protected ChapterRepo $chapterRepo, protected BookRepo $bookRepo, protected ImageService $imageService, + protected ReferenceUpdater $referenceUpdater, ) { + $this->referenceChangeContext = new ReferenceChangeContext(); } /** * Clone the given page into the given parent using the provided name. */ public function clonePage(Page $original, Entity $parent, string $newName): Page + { + $context = $this->newReferenceChangeContext(); + $page = $this->createPageClone($original, $parent, $newName); + $this->referenceUpdater->changeReferencesUsingContext($context); + return $page; + } + + protected function createPageClone(Page $original, Entity $parent, string $newName): Page { $copyPage = $this->pageRepo->getNewDraftPage($parent); $pageData = $this->entityToInputData($original); $pageData['name'] = $newName; - return $this->pageRepo->publishDraft($copyPage, $pageData); + $newPage = $this->pageRepo->publishDraft($copyPage, $pageData); + $this->referenceChangeContext->add($original, $newPage); + + return $newPage; } /** @@ -44,6 +61,14 @@ public function clonePage(Page $original, Entity $parent, string $newName): Page * Clones all child pages. */ public function cloneChapter(Chapter $original, Book $parent, string $newName): Chapter + { + $context = $this->newReferenceChangeContext(); + $chapter = $this->createChapterClone($original, $parent, $newName); + $this->referenceUpdater->changeReferencesUsingContext($context); + return $chapter; + } + + protected function createChapterClone(Chapter $original, Book $parent, string $newName): Chapter { $chapterDetails = $this->entityToInputData($original); $chapterDetails['name'] = $newName; @@ -65,6 +90,14 @@ public function cloneChapter(Chapter $original, Book $parent, string $newName): * Clones all child chapters and pages. */ public function cloneBook(Book $original, string $newName): Book + { + $context = $this->newReferenceChangeContext(); + $book = $this->createBookClone($original, $newName); + $this->referenceUpdater->changeReferencesUsingContext($context); + return $book; + } + + protected function createBookClone(Book $original, string $newName): Book { $bookDetails = $this->entityToInputData($original); $bookDetails['name'] = $newName; @@ -155,4 +188,10 @@ protected function entityTagsToInputArray(Entity $entity): array return $tags; } + + protected function newReferenceChangeContext(): ReferenceChangeContext + { + $this->referenceChangeContext = new ReferenceChangeContext(); + return $this->referenceChangeContext; + } } diff --git a/app/References/ReferenceChangeContext.php b/app/References/ReferenceChangeContext.php new file mode 100644 index 00000000000..df3028b931a --- /dev/null +++ b/app/References/ReferenceChangeContext.php @@ -0,0 +1,19 @@ + + */ + protected array $changes = []; + + public function add(Entity $oldEntity, Entity $newEntity): void + { + $this->changes[] = [$oldEntity, $newEntity]; + } +} diff --git a/app/References/ReferenceUpdater.php b/app/References/ReferenceUpdater.php index 06b3389bae5..3a6db05ef39 100644 --- a/app/References/ReferenceUpdater.php +++ b/app/References/ReferenceUpdater.php @@ -30,6 +30,14 @@ public function updateEntityReferences(Entity $entity, string $oldLink): void } } + public function changeReferencesUsingContext(ReferenceChangeContext $context): void + { + // TODO + + // We should probably have references by this point, so we could use those for efficient + // discovery instead of scanning each item within the context. + } + /** * @return Reference[] */ From 959981a676e222aa9a7a3d0f163e1a21509086f2 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 25 Nov 2025 17:52:26 +0000 Subject: [PATCH 4/5] Copying: Added logic to find & update references --- app/Entities/Models/Page.php | 8 +++++ app/Entities/Tools/Cloner.php | 10 ++++-- app/References/ReferenceChangeContext.php | 37 +++++++++++++++++++++ app/References/ReferenceUpdater.php | 40 ++++++++++++++++++++--- tests/Entity/CopyTest.php | 16 ++++----- 5 files changed, 96 insertions(+), 15 deletions(-) diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php index 88c59bd1bd0..a1d3fc7b40d 100644 --- a/app/Entities/Models/Page.php +++ b/app/Entities/Models/Page.php @@ -124,6 +124,14 @@ public function getUrl(string $path = ''): string return url('/' . implode('/', $parts)); } + /** + * Get the ID-based permalink for this page. + */ + public function getPermalink(): string + { + return url("/link/{$this->id}"); + } + /** * Get this page for JSON display. */ diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php index 916ce42c572..64c48c351ae 100644 --- a/app/Entities/Tools/Cloner.php +++ b/app/Entities/Tools/Cloner.php @@ -78,10 +78,12 @@ protected function createChapterClone(Chapter $original, Book $parent, string $n if (userCan(Permission::PageCreate, $copyChapter)) { /** @var Page $page */ foreach ($original->getVisiblePages() as $page) { - $this->clonePage($page, $copyChapter, $page->name); + $this->createPageClone($page, $copyChapter, $page->name); } } + $this->referenceChangeContext->add($original, $copyChapter); + return $copyChapter; } @@ -109,11 +111,11 @@ protected function createBookClone(Book $original, string $newName): Book $directChildren = $original->getDirectVisibleChildren(); foreach ($directChildren as $child) { if ($child instanceof Chapter && userCan(Permission::ChapterCreate, $copyBook)) { - $this->cloneChapter($child, $copyBook, $child->name); + $this->createChapterClone($child, $copyBook, $child->name); } if ($child instanceof Page && !$child->draft && userCan(Permission::PageCreate, $copyBook)) { - $this->clonePage($child, $copyBook, $child->name); + $this->createPageClone($child, $copyBook, $child->name); } } @@ -125,6 +127,8 @@ protected function createBookClone(Book $original, string $newName): Book } } + $this->referenceChangeContext->add($original, $copyBook); + return $copyBook; } diff --git a/app/References/ReferenceChangeContext.php b/app/References/ReferenceChangeContext.php index df3028b931a..f1161981369 100644 --- a/app/References/ReferenceChangeContext.php +++ b/app/References/ReferenceChangeContext.php @@ -16,4 +16,41 @@ public function add(Entity $oldEntity, Entity $newEntity): void { $this->changes[] = [$oldEntity, $newEntity]; } + + /** + * Get all the change pairs. + * Returned array is an array of pairs, where the first item is the old entity + * and the second is the new entity. + * @return array + */ + public function getChanges(): array + { + return $this->changes; + } + + /** + * Get all the new entities from the changes. + */ + public function getNewEntities(): array + { + return array_column($this->changes, 1); + } + + /** + * Get all the old entities from the changes. + */ + public function getOldEntities(): array + { + return array_column($this->changes, 0); + } + + public function getNewForOld(Entity $oldEntity): ?Entity + { + foreach ($this->changes as [$old, $new]) { + if ($old->id === $oldEntity->id && $old->type === $oldEntity->type) { + return $new; + } + } + return null; + } } diff --git a/app/References/ReferenceUpdater.php b/app/References/ReferenceUpdater.php index 3a6db05ef39..b811fe868ab 100644 --- a/app/References/ReferenceUpdater.php +++ b/app/References/ReferenceUpdater.php @@ -5,7 +5,6 @@ use BookStack\Entities\Models\Book; use BookStack\Entities\Models\HasDescriptionInterface; use BookStack\Entities\Models\Entity; -use BookStack\Entities\Models\EntityContainerData; use BookStack\Entities\Models\Page; use BookStack\Entities\Repos\RevisionRepo; use BookStack\Util\HtmlDocument; @@ -30,12 +29,45 @@ public function updateEntityReferences(Entity $entity, string $oldLink): void } } + /** + * Change existing references for a range of entities using the given context. + */ public function changeReferencesUsingContext(ReferenceChangeContext $context): void { - // TODO + $bindings = []; + foreach ($context->getOldEntities() as $old) { + $bindings[] = $old->getMorphClass(); + $bindings[] = $old->id; + } - // We should probably have references by this point, so we could use those for efficient - // discovery instead of scanning each item within the context. + // No targets to update within the context, so no need to continue. + if (count($bindings) < 2) { + return; + } + + $toReferenceQuery = '(to_type, to_id) IN (' . rtrim(str_repeat('(?,?),', count($bindings) / 2), ',') . ')'; + + // Cycle each new entity in the context + foreach ($context->getNewEntities() as $new) { + // For each, get all references from it which lead to other items within the context of the change + $newReferencesInContext = $new->referencesFrom()->whereRaw($toReferenceQuery, $bindings)->get(); + // For each reference, update the URL and the reference entry + foreach ($newReferencesInContext as $reference) { + $oldToEntity = $reference->to; + $newToEntity = $context->getNewForOld($oldToEntity); + if ($newToEntity === null) { + continue; + } + + $this->updateReferencesWithinEntity($new, $oldToEntity->getUrl(), $newToEntity->getUrl()); + if ($newToEntity instanceof Page && $oldToEntity instanceof Page) { + $this->updateReferencesWithinPage($newToEntity, $oldToEntity->getPermalink(), $newToEntity->getPermalink()); + } + $reference->to_id = $newToEntity->id; + $reference->to_type = $newToEntity->getMorphClass(); + $reference->save(); + } + } } /** diff --git a/tests/Entity/CopyTest.php b/tests/Entity/CopyTest.php index 258b538f5aa..43fe120836f 100644 --- a/tests/Entity/CopyTest.php +++ b/tests/Entity/CopyTest.php @@ -214,12 +214,12 @@ public function test_book_copy_updates_internal_references() 'html' => '

This is a test book link

', ]); - $html = '

This is a test page link

'; - // Quick pre-update to get stable slug $this->put($book->getUrl(), ['name' => 'Internal ref test']); $book->refresh(); + $page->refresh(); + $html = '

This is a test page link

'; $this->put($book->getUrl(), ['name' => 'Internal ref test', 'description_html' => $html]); $this->post($book->getUrl('/copy'), ['name' => 'My copied book']); @@ -245,12 +245,12 @@ public function test_chapter_copy_updates_internal_references() 'html' => '

This is a test chapter link

', ]); - $html = '

This is a test page link

'; - // Quick pre-update to get stable slug $this->put($chapter->getUrl(), ['name' => 'Internal ref test']); $chapter->refresh(); + $page->refresh(); + $html = '

This is a test page link

'; $this->put($chapter->getUrl(), ['name' => 'Internal ref test', 'description_html' => $html]); $this->post($chapter->getUrl('/copy'), ['name' => 'My copied chapter']); @@ -258,11 +258,11 @@ public function test_chapter_copy_updates_internal_references() $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); $newPage = $newChapter->pages()->where('name', '=', 'reference test page')->first(); - $this->assertStringContainsString($newChapter->getUrl(), $newPage->html); - $this->assertStringContainsString($newPage->getUrl(), $newChapter->description_html); + $this->assertStringContainsString($newChapter->getUrl() . '"', $newPage->html); + $this->assertStringContainsString($newPage->getUrl() . '"', $newChapter->description_html); - $this->assertStringNotContainsString($chapter->getUrl(), $newPage->html); - $this->assertStringNotContainsString($page->getUrl(), $newChapter->description_html); + $this->assertStringNotContainsString($chapter->getUrl() . '"', $newPage->html); + $this->assertStringNotContainsString($page->getUrl() . '"', $newChapter->description_html); } public function test_page_copy_updates_internal_self_references() From 3cd3e73f607dc7513c1bd7dc9d458808267eae4c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 29 Nov 2025 20:35:16 +0000 Subject: [PATCH 5/5] Copying: Fixed issue with non-page links to page permalinks Found during manual testing. Added test case to cover. --- app/References/ReferenceChangeContext.php | 11 ----------- app/References/ReferenceUpdater.php | 2 +- tests/Entity/CopyTest.php | 19 +++++++++++++++++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/References/ReferenceChangeContext.php b/app/References/ReferenceChangeContext.php index f1161981369..27de0e2d24c 100644 --- a/app/References/ReferenceChangeContext.php +++ b/app/References/ReferenceChangeContext.php @@ -17,17 +17,6 @@ public function add(Entity $oldEntity, Entity $newEntity): void $this->changes[] = [$oldEntity, $newEntity]; } - /** - * Get all the change pairs. - * Returned array is an array of pairs, where the first item is the old entity - * and the second is the new entity. - * @return array - */ - public function getChanges(): array - { - return $this->changes; - } - /** * Get all the new entities from the changes. */ diff --git a/app/References/ReferenceUpdater.php b/app/References/ReferenceUpdater.php index b811fe868ab..42de72fde04 100644 --- a/app/References/ReferenceUpdater.php +++ b/app/References/ReferenceUpdater.php @@ -61,7 +61,7 @@ public function changeReferencesUsingContext(ReferenceChangeContext $context): v $this->updateReferencesWithinEntity($new, $oldToEntity->getUrl(), $newToEntity->getUrl()); if ($newToEntity instanceof Page && $oldToEntity instanceof Page) { - $this->updateReferencesWithinPage($newToEntity, $oldToEntity->getPermalink(), $newToEntity->getPermalink()); + $this->updateReferencesWithinEntity($new, $oldToEntity->getPermalink(), $newToEntity->getPermalink()); } $reference->to_id = $newToEntity->id; $reference->to_type = $newToEntity->getMorphClass(); diff --git a/tests/Entity/CopyTest.php b/tests/Entity/CopyTest.php index 43fe120836f..d4b6d54cf88 100644 --- a/tests/Entity/CopyTest.php +++ b/tests/Entity/CopyTest.php @@ -265,6 +265,25 @@ public function test_chapter_copy_updates_internal_references() $this->assertStringNotContainsString($page->getUrl() . '"', $newChapter->description_html); } + public function test_chapter_copy_updates_internal_permalink_references_in_its_description() + { + $chapter = $this->entities->chapterHasPages(); + /** @var Page $page */ + $page = $chapter->pages()->first(); + + $this->asEditor()->put($chapter->getUrl(), [ + 'name' => 'Internal ref test', + 'description_html' => '

This is a test page link

', + ]); + $chapter->refresh(); + + $this->post($chapter->getUrl('/copy'), ['name' => 'My copied chapter']); + $newChapter = Chapter::query()->where('name', '=', 'My copied chapter')->first(); + + $this->assertStringContainsString('/link/', $newChapter->description_html); + $this->assertStringNotContainsString($page->getPermalink() . '"', $newChapter->description_html); + } + public function test_page_copy_updates_internal_self_references() { $page = $this->entities->page();