Skip to content

Commit dd5375f

Browse files
ssddanbrownssddanbrown
committed
Slugs: Fixed storage bugs, added testing coverage
1 parent 291a807 commit dd5375f

File tree

5 files changed

+143
-51
lines changed

5 files changed

+143
-51
lines changed

‎app/Entities/Repos/BaseRepo.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,7 @@ protected function updateDescription(Entity $entity, array $input): void
165165
*/
166166
public function refreshSlug(Entity $entity): void
167167
{
168-
if ($entity->id) {
169-
$this->slugHistory->recordForEntity($entity);
170-
}
171-
168+
$this->slugHistory->recordForEntity($entity);
172169
$this->slugGenerator->regenerateForEntity($entity);
173170
}
174171
}

‎app/Entities/Tools/SlugHistory.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace BookStack\Entities\Tools;
44

5+
use BookStack\Entities\Models\BookChild;
56
use BookStack\Entities\Models\Entity;
67
use Illuminate\Support\Facades\DB;
78

@@ -12,16 +13,25 @@ class SlugHistory
1213
*/
1314
public function recordForEntity(Entity $entity): void
1415
{
16+
if (!$entity->id || !$entity->slug) {
17+
return;
18+
}
19+
1520
$latest = $this->getLatestEntryForEntity($entity);
1621
if ($latest && $latest->slug === $entity->slug && $latest->parent_slug === $entity->getParent()?->slug) {
1722
return;
1823
}
1924

25+
$parentSlug = null;
26+
if ($entity instanceof BookChild) {
27+
$parentSlug = $entity->book()->first()?->slug;
28+
}
29+
2030
$info = [
2131
'sluggable_type' => $entity->getMorphClass(),
2232
'sluggable_id' => $entity->id,
2333
'slug' => $entity->slug,
24-
'parent_slug' => $entity->getParent()?->slug,
34+
'parent_slug' => $parentSlug,
2535
'created_at' => now(),
2636
'updated_at' => now(),
2737
];

‎tests/Entity/BookTest.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -238,30 +238,6 @@ public function test_books_view_shows_view_toggle_option()
238238
$this->assertEquals('list', setting()->getUser($editor, 'books_view_type'));
239239
}
240240

241-
public function test_slug_multi_byte_url_safe()
242-
{
243-
$book = $this->entities->newBook([
244-
'name' => 'информация',
245-
]);
246-
247-
$this->assertEquals('informaciia', $book->slug);
248-
249-
$book = $this->entities->newBook([
250-
'name' => '¿Qué?',
251-
]);
252-
253-
$this->assertEquals('que', $book->slug);
254-
}
255-
256-
public function test_slug_format()
257-
{
258-
$book = $this->entities->newBook([
259-
'name' => 'PartA / PartB / PartC',
260-
]);
261-
262-
$this->assertEquals('parta-partb-partc', $book->slug);
263-
}
264-
265241
public function test_description_limited_to_specific_html()
266242
{
267243
$book = $this->entities->book();

‎tests/Entity/PageTest.php‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -269,28 +269,6 @@ public function test_page_can_be_copied_without_edit_permission()
269269
]);
270270
}
271271

272-
public function test_old_page_slugs_redirect_to_new_pages()
273-
{
274-
$page = $this->entities->page();
275-
276-
// Need to save twice since revisions are not generated in seeder.
277-
$this->asAdmin()->put($page->getUrl(), [
278-
'name' => 'super test',
279-
'html' => '<p></p>',
280-
]);
281-
282-
$page->refresh();
283-
$pageUrl = $page->getUrl();
284-
285-
$this->put($pageUrl, [
286-
'name' => 'super test page',
287-
'html' => '<p></p>',
288-
]);
289-
290-
$this->get($pageUrl)
291-
->assertRedirect("/books/{$page->book->slug}/page/super-test-page");
292-
}
293-
294272
public function test_page_within_chapter_deletion_returns_to_chapter()
295273
{
296274
$chapter = $this->entities->chapter();

‎tests/Entity/SlugTest.php‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
<?php
2+
3+
namespace Tests\Entity;
4+
5+
use Tests\TestCase;
6+
7+
class SlugTest extends TestCase
8+
{
9+
public function test_slug_multi_byte_url_safe()
10+
{
11+
$book = $this->entities->newBook([
12+
'name' => 'информация',
13+
]);
14+
15+
$this->assertEquals('informaciia', $book->slug);
16+
17+
$book = $this->entities->newBook([
18+
'name' => '¿Qué?',
19+
]);
20+
21+
$this->assertEquals('que', $book->slug);
22+
}
23+
24+
public function test_slug_format()
25+
{
26+
$book = $this->entities->newBook([
27+
'name' => 'PartA / PartB / PartC',
28+
]);
29+
30+
$this->assertEquals('parta-partb-partc', $book->slug);
31+
}
32+
33+
public function test_old_page_slugs_redirect_to_new_pages()
34+
{
35+
$page = $this->entities->page();
36+
37+
// Need to save twice since revisions are not generated in seeder.
38+
$this->asAdmin()->put($page->getUrl(), [
39+
'name' => 'super test',
40+
'html' => '<p></p>',
41+
]);
42+
43+
$page->refresh();
44+
$pageUrl = $page->getUrl();
45+
46+
$this->put($pageUrl, [
47+
'name' => 'super test page',
48+
'html' => '<p></p>',
49+
]);
50+
51+
$this->get($pageUrl)
52+
->assertRedirect("/books/{$page->book->slug}/page/super-test-page");
53+
}
54+
55+
public function test_slugs_recorded_in_history_on_page_update()
56+
{
57+
$page = $this->entities->page();
58+
$this->asAdmin()->put($page->getUrl(), [
59+
'name' => 'new slug',
60+
'html' => '<p></p>',
61+
]);
62+
63+
$oldSlug = $page->slug;
64+
$page->refresh();
65+
$this->assertNotEquals($oldSlug, $page->slug);
66+
67+
$this->assertDatabaseHas('slug_history', [
68+
'sluggable_id' => $page->id,
69+
'sluggable_type' => 'page',
70+
'slug' => $oldSlug,
71+
'parent_slug' => $page->book->slug,
72+
]);
73+
}
74+
75+
public function test_slugs_recorded_in_history_on_chapter_update()
76+
{
77+
$chapter = $this->entities->chapter();
78+
$this->asAdmin()->put($chapter->getUrl(), [
79+
'name' => 'new slug',
80+
]);
81+
82+
$oldSlug = $chapter->slug;
83+
$chapter->refresh();
84+
$this->assertNotEquals($oldSlug, $chapter->slug);
85+
86+
$this->assertDatabaseHas('slug_history', [
87+
'sluggable_id' => $chapter->id,
88+
'sluggable_type' => 'chapter',
89+
'slug' => $oldSlug,
90+
'parent_slug' => $chapter->book->slug,
91+
]);
92+
}
93+
94+
public function test_slugs_recorded_in_history_on_book_update()
95+
{
96+
$book = $this->entities->book();
97+
$this->asAdmin()->put($book->getUrl(), [
98+
'name' => 'new slug',
99+
]);
100+
101+
$oldSlug = $book->slug;
102+
$book->refresh();
103+
$this->assertNotEquals($oldSlug, $book->slug);
104+
105+
$this->assertDatabaseHas('slug_history', [
106+
'sluggable_id' => $book->id,
107+
'sluggable_type' => 'book',
108+
'slug' => $oldSlug,
109+
'parent_slug' => null,
110+
]);
111+
}
112+
113+
public function test_slugs_recorded_in_history_on_shelf_update()
114+
{
115+
$shelf = $this->entities->shelf();
116+
$this->asAdmin()->put($shelf->getUrl(), [
117+
'name' => 'new slug',
118+
]);
119+
120+
$oldSlug = $shelf->slug;
121+
$shelf->refresh();
122+
$this->assertNotEquals($oldSlug, $shelf->slug);
123+
124+
$this->assertDatabaseHas('slug_history', [
125+
'sluggable_id' => $shelf->id,
126+
'sluggable_type' => 'bookshelf',
127+
'slug' => $oldSlug,
128+
'parent_slug' => null,
129+
]);
130+
}
131+
}

0 commit comments

Comments
 (0)