diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a925aac..c0b01b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ the data reader (@vjik) - New #153: Add `Compare::withValue()` method (@vjik) - Chg #154: Raise minimum required PHP version to 8.1 (@vjik) +- Bug #155: Fix `Sort` configuration preparation (@vjik) +- Bug #155: Fix same named order fields in `Sort` were not overriding previous ones (@vjik) ## 1.0.1 January 25, 2023 diff --git a/src/Reader/Sort.php b/src/Reader/Sort.php index 7827c606..d71d79bf 100644 --- a/src/Reader/Sort.php +++ b/src/Reader/Sort.php @@ -41,7 +41,12 @@ * @psalm-type TSortFieldItem = array * @psalm-type TConfigItem = array{asc: TSortFieldItem, desc: TSortFieldItem, default: "asc"|"desc"} * @psalm-type TConfig = array - * @psalm-type TUserConfig = array|array> + * @psalm-type TUserConfigItem = array{ + * asc?: int|"asc"|"desc"|array, + * desc?: int|"asc"|"desc"|array, + * default?: "asc"|"desc" + * } + * @psalm-type TUserConfig = array|array */ final class Sort { @@ -81,17 +86,43 @@ private function __construct(private bool $ignoreExtraFields, array $config) throw new InvalidArgumentException('Invalid config format.'); } - if (is_string($fieldConfig)) { + if (is_int($fieldName)) { + /** @var string $fieldConfig */ $fieldName = $fieldConfig; $fieldConfig = []; + } else { + /** @psalm-var TUserConfigItem $fieldConfig */ + foreach ($fieldConfig as $key => &$criteria) { + // 'default' => 'asc' or 'desc' + if ($key === 'default') { + continue; + } + // 'asc'/'desc' => SORT_* + if (is_int($criteria)) { + continue; + } + // 'asc'/'desc' => 'asc' or 'asc'/'desc' => 'desc' + if (is_string($criteria)) { + $criteria = [$fieldName => $criteria === 'desc' ? SORT_DESC : SORT_ASC]; + continue; + } + // 'asc'/'desc' => ['field' => SORT_*|'asc'|'desc'] + foreach ($criteria as &$subCriteria) { + if (is_string($subCriteria)) { + $subCriteria = $subCriteria === 'desc' ? SORT_DESC : SORT_ASC; + } + } + } } - /** @psalm-var TConfig $fieldConfig */ - $normalizedConfig[$fieldName] = array_merge([ - 'asc' => [$fieldName => SORT_ASC], - 'desc' => [$fieldName => SORT_DESC], - 'default' => 'asc', - ], $fieldConfig); + $normalizedConfig[$fieldName] = array_merge( + [ + 'asc' => [$fieldName => SORT_ASC], + 'desc' => [$fieldName => SORT_DESC], + 'default' => 'asc', + ], + $fieldConfig, + ); } /** @psalm-var TConfig $normalizedConfig */ @@ -272,7 +303,7 @@ public function getOrderAsString(): string * when obtaining the data i.e. a list of real fields along with their order directions. * * @return array Sorting criteria. - * @psalm-return array + * @psalm-return array */ public function getCriteria(): array { @@ -281,13 +312,13 @@ public function getCriteria(): array foreach ($this->currentOrder as $field => $direction) { if (array_key_exists($field, $config)) { - $criteria += $config[$field][$direction]; + $criteria = array_merge($criteria, $config[$field][$direction]); unset($config[$field]); } else { if ($this->ignoreExtraFields) { continue; } - $criteria += [$field => $direction]; + $criteria = array_merge($criteria, [$field => $direction === 'desc' ? SORT_DESC : SORT_ASC]); } } diff --git a/tests/Reader/SortTest.php b/tests/Reader/SortTest.php index ffe69288..d680f6b7 100644 --- a/tests/Reader/SortTest.php +++ b/tests/Reader/SortTest.php @@ -5,6 +5,7 @@ namespace Yiisoft\Data\Tests\Reader; use InvalidArgumentException; +use PHPUnit\Framework\Attributes\DataProvider; use Yiisoft\Data\Reader\Sort; use Yiisoft\Data\Tests\TestCase; @@ -117,12 +118,18 @@ public function testOnlyModeGetCriteriaWithEmptyConfig(): void public function testAnyModeGetCriteriaWithEmptyConfig(): void { - $sort = Sort::any([])->withOrder([ + $sort = Sort::any()->withOrder([ 'a' => 'desc', 'b' => 'asc', ]); - $this->assertSame(['a' => 'desc', 'b' => 'asc'], $sort->getCriteria()); + $this->assertSame( + [ + 'a' => SORT_DESC, + 'b' => SORT_ASC, + ], + $sort->getCriteria(), + ); } public function testGetCriteria(): void @@ -158,10 +165,13 @@ public function testAnyModeGetCriteriaWhenAnyFieldConflictsWithConfig(): void ], ])->withOrderString('-bee,b,a,-foo'); - $this->assertSame([ - 'bee' => 'desc', - 'foo' => 'asc', - ], $sort->getCriteria()); + $this->assertSame( + [ + 'bee' => SORT_ASC, + 'foo' => SORT_DESC, + ], + $sort->getCriteria(), + ); } public function testGetCriteriaDefaults(): void @@ -265,4 +275,62 @@ public function testIgnoreExtraFields(): void $sort->getCriteria() ); } + + public static function dataPrepareConfig(): array + { + return [ + [ + [ + 'a' => SORT_ASC, + 'b' => SORT_DESC, + ], + [ + 'a', + 'b' => ['asc' => 'desc'], + ], + ], + [ + [ + 'a' => SORT_ASC, + ], + [ + 'a' => [ + 'asc' => 'desc', + 'desc' => 'asc', + 'default' => 'desc', + ], + ], + ], + [ + [ + 'a' => SORT_ASC, + ], + [ + 'a' => [ + 'asc' => SORT_DESC, + 'desc' => 'asc', + 'default' => 'desc', + ], + ], + ], + [ + [ + 'x' => SORT_ASC, + ], + [ + 'a' => [ + 'default' => 'desc', + 'desc' => ['x' => 'asc'], + ], + ], + ], + ]; + } + + #[DataProvider('dataPrepareConfig')] + public function testPrepareConfig(array $expected, array $config): void + { + $sort = Sort::only($config); + $this->assertSame($expected, $sort->getCriteria()); + } }