Skip to content

Commit dca14fe

Browse files
committed
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.
1 parent d7ccb3c commit dca14fe

File tree

3 files changed

+20
-5
lines changed

3 files changed

+20
-5
lines changed

app/Sorting/SortRuleOperation.php

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ enum SortRuleOperation: string
1010
case NameAsc = 'name_asc';
1111
case NameDesc = 'name_desc';
1212
case NameNumericAsc = 'name_numeric_asc';
13+
case NameNumericDesc = 'name_numeric_desc';
1314
case CreatedDateAsc = 'created_date_asc';
1415
case CreatedDateDesc = 'created_date_desc';
1516
case UpdateDateAsc = 'updated_date_asc';

app/Sorting/SortSetOperationComparisons.php

+6-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
/**
99
* Sort comparison function for each of the possible SortSetOperation values.
1010
* Method names should be camelCase names for the SortSetOperation enum value.
11-
* TODO - Test to cover each SortSetOperation enum value is covered.
1211
*/
1312
class SortSetOperationComparisons
1413
{
@@ -27,9 +26,12 @@ public static function nameNumericAsc(Entity $a, Entity $b): int
2726
$numRegex = '/^\d+(\.\d+)?/';
2827
$aMatches = [];
2928
$bMatches = [];
30-
preg_match($numRegex, $a, $aMatches);
31-
preg_match($numRegex, $b, $bMatches);
32-
return ($aMatches[0] ?? 0) <=> ($bMatches[0] ?? 0);
29+
preg_match($numRegex, $a->name, $aMatches);
30+
preg_match($numRegex, $b->name, $bMatches);
31+
$aVal = floatval(($aMatches[0] ?? 0));
32+
$bVal = floatval(($bMatches[0] ?? 0));
33+
34+
return $aVal <=> $bVal;
3335
}
3436

3537
public static function nameNumericDesc(Entity $a, Entity $b): int

tests/Sorting/SortRuleTest.php

+13-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use BookStack\Activity\ActivityType;
66
use BookStack\Entities\Models\Book;
77
use BookStack\Sorting\SortRule;
8+
use BookStack\Sorting\SortRuleOperation;
89
use Tests\Api\TestsApi;
910
use Tests\TestCase;
1011

@@ -202,7 +203,8 @@ public function test_name_numeric_ordering()
202203
"20 - Milk",
203204
];
204205

205-
foreach ($namesToAdd as $name) {
206+
$reverseNamesToAdd = array_reverse($namesToAdd);
207+
foreach ($reverseNamesToAdd as $name) {
206208
$this->actingAsApiEditor()->post("/api/pages", [
207209
'book_id' => $book->id,
208210
'name' => $name,
@@ -218,4 +220,14 @@ public function test_name_numeric_ordering()
218220
]);
219221
}
220222
}
223+
224+
public function test_each_sort_rule_operation_has_a_comparison_function()
225+
{
226+
$operations = SortRuleOperation::cases();
227+
228+
foreach ($operations as $operation) {
229+
$comparisonFunc = $operation->getSortFunction();
230+
$this->assertIsCallable($comparisonFunc);
231+
}
232+
}
221233
}

0 commit comments

Comments
 (0)