Slugs: Fixed storage bugs, added testing coverage

diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php
index 1a8985ad4f9d9e3cb67b866db244dcd84a5d7323..717e9c9f82a09012245d7f3025f7ce2648da9f60 100644 (file)
--- 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 5114d67c7a097d74431b955840aed3257eec1445..dc4527a0007add495e5197ae8229b0841925f24a 100644 (file)
--- 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 543c4e8bbdb032d910577b103d06a20e0b7c8b1b..545d6b305784bf2c80dc6e772323cbd2535b945a 100644 (file)
--- 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 b9e1294e0ec44ca7b911e4363f51109f676ba511..afe15f4d4a355adef55c41439873e65d3b6e6ad1 100644 (file)
--- 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' => '<p></p>',
-        ]);
-
-        $page->refresh();
-        $pageUrl = $page->getUrl();
-
-        $this->put($pageUrl, [
-            'name' => 'super test page',
-            'html' => '<p></p>',
-        ]);
-
-        $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 (file)
index 0000000..d60daff
--- /dev/null
+++ b/tests/Entity/SlugTest.php
@@ -0,0 +1,131 @@
+<?php
+
+namespace Tests\Entity;
+
+use Tests\TestCase;
+
+class SlugTest extends TestCase
+{
+    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_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' => '<p></p>',
+        ]);
+
+        $page->refresh();
+        $pageUrl = $page->getUrl();
+
+        $this->put($pageUrl, [
+            'name' => 'super test page',
+            'html' => '<p></p>',
+        ]);
+
+        $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' => '<p></p>',
+        ]);
+
+        $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,
+        ]);
+    }
+}