From dca14feaaad686bfbe9acb59f5eb11b815501e5b Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 24 Feb 2025 16:58:59 +0000
Subject: [PATCH] Sorting: Fixes during testing of sort rules
- Fixed name numeric sorting not working as expected due to bad
comparison.
- Added name numeric desc operation option.
- Added test to ensure each operating has a comparison function.
---
app/Sorting/SortRuleOperation.php | 1 +
app/Sorting/SortSetOperationComparisons.php | 10 ++++++----
tests/Sorting/SortRuleTest.php | 14 +++++++++++++-
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/app/Sorting/SortRuleOperation.php b/app/Sorting/SortRuleOperation.php
index 0d8ff239f..27bb9e3c0 100644
--- a/app/Sorting/SortRuleOperation.php
+++ b/app/Sorting/SortRuleOperation.php
@@ -10,6 +10,7 @@ enum SortRuleOperation: string
case NameAsc = 'name_asc';
case NameDesc = 'name_desc';
case NameNumericAsc = 'name_numeric_asc';
+ case NameNumericDesc = 'name_numeric_desc';
case CreatedDateAsc = 'created_date_asc';
case CreatedDateDesc = 'created_date_desc';
case UpdateDateAsc = 'updated_date_asc';
diff --git a/app/Sorting/SortSetOperationComparisons.php b/app/Sorting/SortSetOperationComparisons.php
index e1c3e625f..0d8e58264 100644
--- a/app/Sorting/SortSetOperationComparisons.php
+++ b/app/Sorting/SortSetOperationComparisons.php
@@ -8,7 +8,6 @@ use BookStack\Entities\Models\Entity;
/**
* Sort comparison function for each of the possible SortSetOperation values.
* Method names should be camelCase names for the SortSetOperation enum value.
- * TODO - Test to cover each SortSetOperation enum value is covered.
*/
class SortSetOperationComparisons
{
@@ -27,9 +26,12 @@ class SortSetOperationComparisons
$numRegex = '/^\d+(\.\d+)?/';
$aMatches = [];
$bMatches = [];
- preg_match($numRegex, $a, $aMatches);
- preg_match($numRegex, $b, $bMatches);
- return ($aMatches[0] ?? 0) <=> ($bMatches[0] ?? 0);
+ preg_match($numRegex, $a->name, $aMatches);
+ preg_match($numRegex, $b->name, $bMatches);
+ $aVal = floatval(($aMatches[0] ?? 0));
+ $bVal = floatval(($bMatches[0] ?? 0));
+
+ return $aVal <=> $bVal;
}
public static function nameNumericDesc(Entity $a, Entity $b): int
diff --git a/tests/Sorting/SortRuleTest.php b/tests/Sorting/SortRuleTest.php
index 10c7bc17d..85b3d7991 100644
--- a/tests/Sorting/SortRuleTest.php
+++ b/tests/Sorting/SortRuleTest.php
@@ -5,6 +5,7 @@ namespace Tests\Sorting;
use BookStack\Activity\ActivityType;
use BookStack\Entities\Models\Book;
use BookStack\Sorting\SortRule;
+use BookStack\Sorting\SortRuleOperation;
use Tests\Api\TestsApi;
use Tests\TestCase;
@@ -202,7 +203,8 @@ class SortRuleTest extends TestCase
"20 - Milk",
];
- foreach ($namesToAdd as $name) {
+ $reverseNamesToAdd = array_reverse($namesToAdd);
+ foreach ($reverseNamesToAdd as $name) {
$this->actingAsApiEditor()->post("/api/pages", [
'book_id' => $book->id,
'name' => $name,
@@ -218,4 +220,14 @@ class SortRuleTest extends TestCase
]);
}
}
+
+ public function test_each_sort_rule_operation_has_a_comparison_function()
+ {
+ $operations = SortRuleOperation::cases();
+
+ foreach ($operations as $operation) {
+ $comparisonFunc = $operation->getSortFunction();
+ $this->assertIsCallable($comparisonFunc);
+ }
+ }
}