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.

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;

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());
         }
     }
 

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',

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);
+    }
+}

tests/Commands/AssignSortSetCommandTest.php

@@ -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);
-    }
-}

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();

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();