From: Dan Brown Date: Mon, 24 Nov 2025 10:46:24 +0000 (+0000) Subject: Slugs: Fixed storage bugs, added testing coverage X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/dd5375f480bc7f8ee48e376d8fff1bee9f6bc542 Slugs: Fixed storage bugs, added testing coverage --- diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 1a8985ad4..717e9c9f8 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -165,10 +165,7 @@ class BaseRepo */ public function refreshSlug(Entity $entity): void { - if ($entity->id) { - $this->slugHistory->recordForEntity($entity); - } - + $this->slugHistory->recordForEntity($entity); $this->slugGenerator->regenerateForEntity($entity); } } diff --git a/app/Entities/Tools/SlugHistory.php b/app/Entities/Tools/SlugHistory.php index 5114d67c7..dc4527a00 100644 --- a/app/Entities/Tools/SlugHistory.php +++ b/app/Entities/Tools/SlugHistory.php @@ -2,6 +2,7 @@ namespace BookStack\Entities\Tools; +use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Entity; use Illuminate\Support\Facades\DB; @@ -12,16 +13,25 @@ class SlugHistory */ public function recordForEntity(Entity $entity): void { + if (!$entity->id || !$entity->slug) { + return; + } + $latest = $this->getLatestEntryForEntity($entity); if ($latest && $latest->slug === $entity->slug && $latest->parent_slug === $entity->getParent()?->slug) { return; } + $parentSlug = null; + if ($entity instanceof BookChild) { + $parentSlug = $entity->book()->first()?->slug; + } + $info = [ 'sluggable_type' => $entity->getMorphClass(), 'sluggable_id' => $entity->id, 'slug' => $entity->slug, - 'parent_slug' => $entity->getParent()?->slug, + 'parent_slug' => $parentSlug, 'created_at' => now(), 'updated_at' => now(), ]; diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php index 543c4e8bb..545d6b305 100644 --- a/tests/Entity/BookTest.php +++ b/tests/Entity/BookTest.php @@ -238,30 +238,6 @@ class BookTest extends TestCase $this->assertEquals('list', setting()->getUser($editor, 'books_view_type')); } - public function test_slug_multi_byte_url_safe() - { - $book = $this->entities->newBook([ - 'name' => 'информация', - ]); - - $this->assertEquals('informaciia', $book->slug); - - $book = $this->entities->newBook([ - 'name' => '¿Qué?', - ]); - - $this->assertEquals('que', $book->slug); - } - - public function test_slug_format() - { - $book = $this->entities->newBook([ - 'name' => 'PartA / PartB / PartC', - ]); - - $this->assertEquals('parta-partb-partc', $book->slug); - } - public function test_description_limited_to_specific_html() { $book = $this->entities->book(); diff --git a/tests/Entity/PageTest.php b/tests/Entity/PageTest.php index b9e1294e0..afe15f4d4 100644 --- a/tests/Entity/PageTest.php +++ b/tests/Entity/PageTest.php @@ -269,28 +269,6 @@ class PageTest extends TestCase ]); } - public function test_old_page_slugs_redirect_to_new_pages() - { - $page = $this->entities->page(); - - // Need to save twice since revisions are not generated in seeder. - $this->asAdmin()->put($page->getUrl(), [ - 'name' => 'super test', - 'html' => '

', - ]); - - $page->refresh(); - $pageUrl = $page->getUrl(); - - $this->put($pageUrl, [ - 'name' => 'super test page', - 'html' => '

', - ]); - - $this->get($pageUrl) - ->assertRedirect("/books/{$page->book->slug}/page/super-test-page"); - } - public function test_page_within_chapter_deletion_returns_to_chapter() { $chapter = $this->entities->chapter(); diff --git a/tests/Entity/SlugTest.php b/tests/Entity/SlugTest.php new file mode 100644 index 000000000..d60daff24 --- /dev/null +++ b/tests/Entity/SlugTest.php @@ -0,0 +1,131 @@ +entities->newBook([ + 'name' => 'информация', + ]); + + $this->assertEquals('informaciia', $book->slug); + + $book = $this->entities->newBook([ + 'name' => '¿Qué?', + ]); + + $this->assertEquals('que', $book->slug); + } + + public function test_slug_format() + { + $book = $this->entities->newBook([ + 'name' => 'PartA / PartB / PartC', + ]); + + $this->assertEquals('parta-partb-partc', $book->slug); + } + + public function test_old_page_slugs_redirect_to_new_pages() + { + $page = $this->entities->page(); + + // Need to save twice since revisions are not generated in seeder. + $this->asAdmin()->put($page->getUrl(), [ + 'name' => 'super test', + 'html' => '

', + ]); + + $page->refresh(); + $pageUrl = $page->getUrl(); + + $this->put($pageUrl, [ + 'name' => 'super test page', + 'html' => '

', + ]); + + $this->get($pageUrl) + ->assertRedirect("/books/{$page->book->slug}/page/super-test-page"); + } + + public function test_slugs_recorded_in_history_on_page_update() + { + $page = $this->entities->page(); + $this->asAdmin()->put($page->getUrl(), [ + 'name' => 'new slug', + 'html' => '

', + ]); + + $oldSlug = $page->slug; + $page->refresh(); + $this->assertNotEquals($oldSlug, $page->slug); + + $this->assertDatabaseHas('slug_history', [ + 'sluggable_id' => $page->id, + 'sluggable_type' => 'page', + 'slug' => $oldSlug, + 'parent_slug' => $page->book->slug, + ]); + } + + public function test_slugs_recorded_in_history_on_chapter_update() + { + $chapter = $this->entities->chapter(); + $this->asAdmin()->put($chapter->getUrl(), [ + 'name' => 'new slug', + ]); + + $oldSlug = $chapter->slug; + $chapter->refresh(); + $this->assertNotEquals($oldSlug, $chapter->slug); + + $this->assertDatabaseHas('slug_history', [ + 'sluggable_id' => $chapter->id, + 'sluggable_type' => 'chapter', + 'slug' => $oldSlug, + 'parent_slug' => $chapter->book->slug, + ]); + } + + public function test_slugs_recorded_in_history_on_book_update() + { + $book = $this->entities->book(); + $this->asAdmin()->put($book->getUrl(), [ + 'name' => 'new slug', + ]); + + $oldSlug = $book->slug; + $book->refresh(); + $this->assertNotEquals($oldSlug, $book->slug); + + $this->assertDatabaseHas('slug_history', [ + 'sluggable_id' => $book->id, + 'sluggable_type' => 'book', + 'slug' => $oldSlug, + 'parent_slug' => null, + ]); + } + + public function test_slugs_recorded_in_history_on_shelf_update() + { + $shelf = $this->entities->shelf(); + $this->asAdmin()->put($shelf->getUrl(), [ + 'name' => 'new slug', + ]); + + $oldSlug = $shelf->slug; + $shelf->refresh(); + $this->assertNotEquals($oldSlug, $shelf->slug); + + $this->assertDatabaseHas('slug_history', [ + 'sluggable_id' => $shelf->id, + 'sluggable_type' => 'bookshelf', + 'slug' => $oldSlug, + 'parent_slug' => null, + ]); + } +}