From 7bd89316bcc92642af6c17c9d6e7b89ad6807055 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Tue, 11 Feb 2025 15:29:16 +0000
Subject: [PATCH] Sorting: Updated sort set command, Changed sort timestamp
handling
- Renamed AssignSortSetCommand to AssignSortRuleCommand, updated
contents and testing.
- Updated sorting operations to not update timestamps if only priority
is changed.
---
...tCommand.php => AssignSortRuleCommand.php} | 46 +++----
app/Sorting/BookSorter.php | 7 +-
lang/en/activities.php | 14 +--
tests/Commands/AssignSortRuleCommandTest.php | 112 ++++++++++++++++++
tests/Commands/AssignSortSetCommandTest.php | 112 ------------------
tests/Sorting/BookSortTest.php | 26 ++++
tests/Sorting/SortRuleTest.php | 21 ++++
7 files changed, 193 insertions(+), 145 deletions(-)
rename app/Console/Commands/{AssignSortSetCommand.php => AssignSortRuleCommand.php} (60%)
create mode 100644 tests/Commands/AssignSortRuleCommandTest.php
delete mode 100644 tests/Commands/AssignSortSetCommandTest.php
diff --git a/app/Console/Commands/AssignSortSetCommand.php b/app/Console/Commands/AssignSortRuleCommand.php
similarity index 60%
rename from app/Console/Commands/AssignSortSetCommand.php
rename to app/Console/Commands/AssignSortRuleCommand.php
index 6c9d3f764..c438d0783 100644
--- a/app/Console/Commands/AssignSortSetCommand.php
+++ b/app/Console/Commands/AssignSortRuleCommand.php
@@ -7,60 +7,60 @@ use BookStack\Sorting\BookSorter;
use BookStack\Sorting\SortRule;
use Illuminate\Console\Command;
-class AssignSortSetCommand extends Command
+class AssignSortRuleCommand extends Command
{
/**
* The name and signature of the console command.
*
* @var string
*/
- protected $signature = 'bookstack:assign-sort-set
- {sort-set=0: ID of the sort set to apply}
+ protected $signature = 'bookstack:assign-sort-rule
+ {sort-rule=0: ID of the sort rule to apply}
{--all-books : Apply to all books in the system}
- {--books-without-sort : Apply to only books without a sort set already assigned}
- {--books-with-sort= : Apply to only books with the sort of given id}';
+ {--books-without-sort : Apply to only books without a sort rule already assigned}
+ {--books-with-sort= : Apply to only books with the sort rule of given id}';
/**
* The console command description.
*
* @var string
*/
- protected $description = 'Assign a sort set to content in the system';
+ protected $description = 'Assign a sort rule to content in the system';
/**
* Execute the console command.
*/
public function handle(BookSorter $sorter): int
{
- $sortSetId = intval($this->argument('sort-set')) ?? 0;
- if ($sortSetId === 0) {
- return $this->listSortSets();
+ $sortRuleId = intval($this->argument('sort-rule')) ?? 0;
+ if ($sortRuleId === 0) {
+ return $this->listSortRules();
}
- $set = SortRule::query()->find($sortSetId);
+ $rule = SortRule::query()->find($sortRuleId);
if ($this->option('all-books')) {
$query = Book::query();
} else if ($this->option('books-without-sort')) {
- $query = Book::query()->whereNull('sort_set_id');
+ $query = Book::query()->whereNull('sort_rule_id');
} else if ($this->option('books-with-sort')) {
$sortId = intval($this->option('books-with-sort')) ?: 0;
if (!$sortId) {
$this->error("Provided --books-with-sort option value is invalid");
return 1;
}
- $query = Book::query()->where('sort_set_id', $sortId);
+ $query = Book::query()->where('sort_rule_id', $sortId);
} else {
$this->error("No option provided to specify target. Run with the -h option to see all available options.");
return 1;
}
- if (!$set) {
- $this->error("Sort set of provided id {$sortSetId} not found!");
+ if (!$rule) {
+ $this->error("Sort rule of provided id {$sortRuleId} not found!");
return 1;
}
$count = $query->clone()->count();
- $this->warn("This will apply sort set [{$set->id}: {$set->name}] to {$count} book(s) and run the sort on each.");
+ $this->warn("This will apply sort rule [{$rule->id}: {$rule->name}] to {$count} book(s) and run the sort on each.");
$confirmed = $this->confirm("Are you sure you want to continue?");
if (!$confirmed) {
@@ -68,11 +68,11 @@ class AssignSortSetCommand extends Command
}
$processed = 0;
- $query->chunkById(10, function ($books) use ($set, $sorter, $count, &$processed) {
+ $query->chunkById(10, function ($books) use ($rule, $sorter, $count, &$processed) {
$max = min($count, ($processed + 10));
$this->info("Applying to {$processed}-{$max} of {$count} books");
foreach ($books as $book) {
- $book->sort_set_id = $set->id;
+ $book->sort_rule_id = $rule->id;
$book->save();
$sorter->runBookAutoSort($book);
}
@@ -84,14 +84,14 @@ class AssignSortSetCommand extends Command
return 0;
}
- protected function listSortSets(): int
+ protected function listSortRules(): int
{
- $sets = SortRule::query()->orderBy('id', 'asc')->get();
- $this->error("Sort set ID required!");
- $this->warn("\nAvailable sort sets:");
- foreach ($sets as $set) {
- $this->info("{$set->id}: {$set->name}");
+ $rules = SortRule::query()->orderBy('id', 'asc')->get();
+ $this->error("Sort rule ID required!");
+ $this->warn("\nAvailable sort rules:");
+ foreach ($rules as $rule) {
+ $this->info("{$rule->id}: {$rule->name}");
}
return 1;
diff --git a/app/Sorting/BookSorter.php b/app/Sorting/BookSorter.php
index 7bf1b63f4..6710f070a 100644
--- a/app/Sorting/BookSorter.php
+++ b/app/Sorting/BookSorter.php
@@ -2,6 +2,7 @@
namespace BookStack\Sorting;
+use BookStack\App\Model;
use BookStack\Entities\Models\Book;
use BookStack\Entities\Models\BookChild;
use BookStack\Entities\Models\Chapter;
@@ -57,7 +58,7 @@ class BookSorter
foreach ($topItems as $index => $topItem) {
$topItem->priority = $index + 1;
- $topItem->save();
+ $topItem::withoutTimestamps(fn () => $topItem->save());
}
foreach ($chapters as $chapter) {
@@ -68,7 +69,7 @@ class BookSorter
foreach ($pages as $index => $page) {
$page->priority = $index + 1;
- $page->save();
+ $page::withoutTimestamps(fn () => $page->save());
}
}
}
@@ -166,7 +167,7 @@ class BookSorter
}
if ($chapterChanged || $priorityChanged) {
- $model->save();
+ $model::withoutTimestamps(fn () => $model->save());
}
}
diff --git a/lang/en/activities.php b/lang/en/activities.php
index 7db872c0c..67df53e36 100644
--- a/lang/en/activities.php
+++ b/lang/en/activities.php
@@ -127,13 +127,13 @@ return [
'comment_update' => 'updated comment',
'comment_delete' => 'deleted comment',
- // Sort Sets
- 'sort_set_create' => 'created sort set',
- 'sort_set_create_notification' => 'Sort set successfully created',
- 'sort_set_update' => 'updated sort set',
- 'sort_set_update_notification' => 'Sort set successfully update',
- 'sort_set_delete' => 'deleted sort set',
- 'sort_set_delete_notification' => 'Sort set successfully deleted',
+ // Sort Rules
+ 'sort_rule_create' => 'created sort rule',
+ 'sort_rule_create_notification' => 'Sort rule successfully created',
+ 'sort_rule_update' => 'updated sort rule',
+ 'sort_rule_update_notification' => 'Sort rule successfully update',
+ 'sort_rule_delete' => 'deleted sort rule',
+ 'sort_rule_delete_notification' => 'Sort rule successfully deleted',
// Other
'permissions_update' => 'updated permissions',
diff --git a/tests/Commands/AssignSortRuleCommandTest.php b/tests/Commands/AssignSortRuleCommandTest.php
new file mode 100644
index 000000000..5b308cd7c
--- /dev/null
+++ b/tests/Commands/AssignSortRuleCommandTest.php
@@ -0,0 +1,112 @@
+<?php
+
+namespace Commands;
+
+use BookStack\Entities\Models\Book;
+use BookStack\Sorting\SortRule;
+use Tests\TestCase;
+
+class AssignSortRuleCommandTest extends TestCase
+{
+ public function test_no_given_sort_rule_lists_options()
+ {
+ $sortRules = SortRule::factory()->createMany(10);
+
+ $commandRun = $this->artisan('bookstack:assign-sort-rule')
+ ->expectsOutputToContain('Sort rule ID required!')
+ ->assertExitCode(1);
+
+ foreach ($sortRules as $sortRule) {
+ $commandRun->expectsOutputToContain("{$sortRule->id}: {$sortRule->name}");
+ }
+ }
+
+ public function test_run_without_options_advises_help()
+ {
+ $this->artisan("bookstack:assign-sort-rule 100")
+ ->expectsOutput("No option provided to specify target. Run with the -h option to see all available options.")
+ ->assertExitCode(1);
+ }
+
+ public function test_run_without_valid_sort_advises_help()
+ {
+ $this->artisan("bookstack:assign-sort-rule 100342 --all-books")
+ ->expectsOutput("Sort rule of provided id 100342 not found!")
+ ->assertExitCode(1);
+ }
+
+ public function test_confirmation_required()
+ {
+ $sortRule = SortRule::factory()->create();
+
+ $this->artisan("bookstack:assign-sort-rule {$sortRule->id} --all-books")
+ ->expectsConfirmation('Are you sure you want to continue?', 'no')
+ ->assertExitCode(1);
+
+ $booksWithSort = Book::query()->whereNotNull('sort_rule_id')->count();
+ $this->assertEquals(0, $booksWithSort);
+ }
+
+ public function test_assign_to_all_books()
+ {
+ $sortRule = SortRule::factory()->create();
+ $booksWithoutSort = Book::query()->whereNull('sort_rule_id')->count();
+ $this->assertGreaterThan(0, $booksWithoutSort);
+
+ $this->artisan("bookstack:assign-sort-rule {$sortRule->id} --all-books")
+ ->expectsOutputToContain("This will apply sort rule [{$sortRule->id}: {$sortRule->name}] to {$booksWithoutSort} book(s)")
+ ->expectsConfirmation('Are you sure you want to continue?', 'yes')
+ ->expectsOutputToContain("Sort applied to {$booksWithoutSort} book(s)")
+ ->assertExitCode(0);
+
+ $booksWithoutSort = Book::query()->whereNull('sort_rule_id')->count();
+ $this->assertEquals(0, $booksWithoutSort);
+ }
+
+ public function test_assign_to_all_books_without_sort()
+ {
+ $totalBooks = Book::query()->count();
+ $book = $this->entities->book();
+ $sortRuleA = SortRule::factory()->create();
+ $sortRuleB = SortRule::factory()->create();
+ $book->sort_rule_id = $sortRuleA->id;
+ $book->save();
+
+ $booksWithoutSort = Book::query()->whereNull('sort_rule_id')->count();
+ $this->assertEquals($totalBooks, $booksWithoutSort + 1);
+
+ $this->artisan("bookstack:assign-sort-rule {$sortRuleB->id} --books-without-sort")
+ ->expectsConfirmation('Are you sure you want to continue?', 'yes')
+ ->expectsOutputToContain("Sort applied to {$booksWithoutSort} book(s)")
+ ->assertExitCode(0);
+
+ $booksWithoutSort = Book::query()->whereNull('sort_rule_id')->count();
+ $this->assertEquals(0, $booksWithoutSort);
+ $this->assertEquals($totalBooks, $sortRuleB->books()->count() + 1);
+ }
+
+ public function test_assign_to_all_books_with_sort()
+ {
+ $book = $this->entities->book();
+ $sortRuleA = SortRule::factory()->create();
+ $sortRuleB = SortRule::factory()->create();
+ $book->sort_rule_id = $sortRuleA->id;
+ $book->save();
+
+ $this->artisan("bookstack:assign-sort-rule {$sortRuleB->id} --books-with-sort={$sortRuleA->id}")
+ ->expectsConfirmation('Are you sure you want to continue?', 'yes')
+ ->expectsOutputToContain("Sort applied to 1 book(s)")
+ ->assertExitCode(0);
+
+ $book->refresh();
+ $this->assertEquals($sortRuleB->id, $book->sort_rule_id);
+ $this->assertEquals(1, $sortRuleB->books()->count());
+ }
+
+ public function test_assign_to_all_books_with_sort_id_is_validated()
+ {
+ $this->artisan("bookstack:assign-sort-rule 50 --books-with-sort=beans")
+ ->expectsOutputToContain("Provided --books-with-sort option value is invalid")
+ ->assertExitCode(1);
+ }
+}
diff --git a/tests/Commands/AssignSortSetCommandTest.php b/tests/Commands/AssignSortSetCommandTest.php
deleted file mode 100644
index 659840a9b..000000000
--- a/tests/Commands/AssignSortSetCommandTest.php
+++ /dev/null
@@ -1,112 +0,0 @@
-<?php
-
-namespace Commands;
-
-use BookStack\Entities\Models\Book;
-use BookStack\Sorting\SortRule;
-use Tests\TestCase;
-
-class AssignSortSetCommandTest extends TestCase
-{
- public function test_no_given_sort_set_lists_options()
- {
- $sortSets = SortRule::factory()->createMany(10);
-
- $commandRun = $this->artisan('bookstack:assign-sort-set')
- ->expectsOutputToContain('Sort set ID required!')
- ->assertExitCode(1);
-
- foreach ($sortSets as $sortSet) {
- $commandRun->expectsOutputToContain("{$sortSet->id}: {$sortSet->name}");
- }
- }
-
- public function test_run_without_options_advises_help()
- {
- $this->artisan("bookstack:assign-sort-set 100")
- ->expectsOutput("No option provided to specify target. Run with the -h option to see all available options.")
- ->assertExitCode(1);
- }
-
- public function test_run_without_valid_sort_advises_help()
- {
- $this->artisan("bookstack:assign-sort-set 100342 --all-books")
- ->expectsOutput("Sort set of provided id 100342 not found!")
- ->assertExitCode(1);
- }
-
- public function test_confirmation_required()
- {
- $sortSet = SortRule::factory()->create();
-
- $this->artisan("bookstack:assign-sort-set {$sortSet->id} --all-books")
- ->expectsConfirmation('Are you sure you want to continue?', 'no')
- ->assertExitCode(1);
-
- $booksWithSort = Book::query()->whereNotNull('sort_set_id')->count();
- $this->assertEquals(0, $booksWithSort);
- }
-
- public function test_assign_to_all_books()
- {
- $sortSet = SortRule::factory()->create();
- $booksWithoutSort = Book::query()->whereNull('sort_set_id')->count();
- $this->assertGreaterThan(0, $booksWithoutSort);
-
- $this->artisan("bookstack:assign-sort-set {$sortSet->id} --all-books")
- ->expectsOutputToContain("This will apply sort set [{$sortSet->id}: {$sortSet->name}] to {$booksWithoutSort} book(s)")
- ->expectsConfirmation('Are you sure you want to continue?', 'yes')
- ->expectsOutputToContain("Sort applied to {$booksWithoutSort} book(s)")
- ->assertExitCode(0);
-
- $booksWithoutSort = Book::query()->whereNull('sort_set_id')->count();
- $this->assertEquals(0, $booksWithoutSort);
- }
-
- public function test_assign_to_all_books_without_sort()
- {
- $totalBooks = Book::query()->count();
- $book = $this->entities->book();
- $sortSetA = SortRule::factory()->create();
- $sortSetB = SortRule::factory()->create();
- $book->sort_rule_id = $sortSetA->id;
- $book->save();
-
- $booksWithoutSort = Book::query()->whereNull('sort_set_id')->count();
- $this->assertEquals($totalBooks, $booksWithoutSort + 1);
-
- $this->artisan("bookstack:assign-sort-set {$sortSetB->id} --books-without-sort")
- ->expectsConfirmation('Are you sure you want to continue?', 'yes')
- ->expectsOutputToContain("Sort applied to {$booksWithoutSort} book(s)")
- ->assertExitCode(0);
-
- $booksWithoutSort = Book::query()->whereNull('sort_set_id')->count();
- $this->assertEquals(0, $booksWithoutSort);
- $this->assertEquals($totalBooks, $sortSetB->books()->count() + 1);
- }
-
- public function test_assign_to_all_books_with_sort()
- {
- $book = $this->entities->book();
- $sortSetA = SortRule::factory()->create();
- $sortSetB = SortRule::factory()->create();
- $book->sort_rule_id = $sortSetA->id;
- $book->save();
-
- $this->artisan("bookstack:assign-sort-set {$sortSetB->id} --books-with-sort={$sortSetA->id}")
- ->expectsConfirmation('Are you sure you want to continue?', 'yes')
- ->expectsOutputToContain("Sort applied to 1 book(s)")
- ->assertExitCode(0);
-
- $book->refresh();
- $this->assertEquals($sortSetB->id, $book->sort_rule_id);
- $this->assertEquals(1, $sortSetB->books()->count());
- }
-
- public function test_assign_to_all_books_with_sort_id_is_validated()
- {
- $this->artisan("bookstack:assign-sort-set 50 --books-with-sort=beans")
- ->expectsOutputToContain("Provided --books-with-sort option value is invalid")
- ->assertExitCode(1);
- }
-}
diff --git a/tests/Sorting/BookSortTest.php b/tests/Sorting/BookSortTest.php
index b30cec8b6..c4217a4cc 100644
--- a/tests/Sorting/BookSortTest.php
+++ b/tests/Sorting/BookSortTest.php
@@ -207,6 +207,32 @@ class BookSortTest extends TestCase
]);
}
+ public function test_book_sort_does_not_change_timestamps_on_just_order_changes()
+ {
+ $book = $this->entities->bookHasChaptersAndPages();
+ $chapter = $book->chapters()->first();
+ \DB::table('chapters')->where('id', '=', $chapter->id)->update([
+ 'priority' => 10001,
+ 'updated_at' => \Carbon\Carbon::now()->subYear(5),
+ ]);
+
+ $chapter->refresh();
+ $oldUpdatedAt = $chapter->updated_at->unix();
+
+ $sortData = [
+ 'id' => $chapter->id,
+ 'sort' => 0,
+ 'parentChapter' => false,
+ 'type' => 'chapter',
+ 'book' => $book->id,
+ ];
+ $this->asEditor()->put($book->getUrl('/sort'), ['sort-tree' => json_encode([$sortData])])->assertRedirect();
+
+ $chapter->refresh();
+ $this->assertNotEquals(10001, $chapter->priority);
+ $this->assertEquals($oldUpdatedAt, $chapter->updated_at->unix());
+ }
+
public function test_book_sort_item_returns_book_content()
{
$bookToSort = $this->entities->book();
diff --git a/tests/Sorting/SortRuleTest.php b/tests/Sorting/SortRuleTest.php
index d0ff263df..0f6f43cee 100644
--- a/tests/Sorting/SortRuleTest.php
+++ b/tests/Sorting/SortRuleTest.php
@@ -165,6 +165,27 @@ class SortRuleTest extends TestCase
]);
}
+ public function test_auto_book_sort_does_not_touch_timestamps()
+ {
+ $book = $this->entities->bookHasChaptersAndPages();
+ $rule = SortRule::factory()->create(['sequence' => 'name_asc,chapters_first']);
+ $book->sort_rule_id = $rule->id;
+ $book->save();
+ $page = $book->pages()->first();
+ $chapter = $book->chapters()->first();
+
+ $resp = $this->actingAsApiEditor()->put("/api/pages/{$page->id}", [
+ 'name' => '1111 page',
+ ]);
+ $resp->assertOk();
+
+ $oldTime = $chapter->updated_at->unix();
+ $oldPriority = $chapter->priority;
+ $chapter->refresh();
+ $this->assertEquals($oldTime, $chapter->updated_at->unix());
+ $this->assertNotEquals($oldPriority, $chapter->priority);
+ }
+
public function test_name_numeric_ordering()
{
$book = Book::factory()->create();