Fix #62: Fix Sort not taking defaults into account

Co-authored-by: Alexander Makarov <samdark@users.noreply.github.com> Co-authored-by: roxblnfk <roxblnfk@ya.ru>
yiisoft · Dec 10, 2020 · 8dc315b · 8dc315b
1 parent 899988d
commit 8dc315b
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 26 deletions.
diff --git a/src/Reader/Sort.php b/src/Reader/Sort.php
@@ -4,14 +4,33 @@
 
 namespace Yiisoft\Data\Reader;
 
+use function array_key_exists;
+use function is_array;
+use function is_int;
+use function is_string;
+
 /**
  * Sort represents information relevant to sorting according to one or multiple item fields.
  *
+ * @template TSortFieldItem as array<string, int>
+ * @template TConfigItem as array{asc: TSortFieldItem, desc: TSortFieldItem, default: string, label: string}
+ * @template TConfig as array<string, TConfigItem>
  * @psalm-immutable
  */
 final class Sort
 {
+    /** @psalm-var TConfig */
+    private array $config;
+
+    /**
+     * @var array Field names to order by as keys, direction as values.
+     */
+    private array $currentOrder = [];
+
     /**
+     * @var array $config A list of sortable fields along with their
+     * configuration.
+     *
      * ```php
      * [
      *     'age', // means will be sorted as is
@@ -35,42 +54,41 @@ final class Sort
      *     'label' => Inflector::camel2words('age'),
      * ]
      * ```
+     *
+     * The name field is a virtual field name that consists of two real fields, `first_name` amd `last_name`. Virtual
+     * field name is used in order string or order array while real fields are used in final sorting criteria.
+     *
+     * Each configuration has the following options:
+     *
+     * - `asc` - criteria for ascending sorting.
+     * - `desc` - criteria for descending sorting.
+     * - `default` - default sorting. Could be either `asc` or `desc`. If not specified, `asc` is used.
+     * - `label` -
+     * @psalm-var array<int, string>|array<string, array<string, int|string>> $config
      */
-    private array $config;
-
-    /**
-     * @var array field names to order by as keys, direction as values
-     */
-    private array $currentOrder = [];
-
     public function __construct(array $config)
     {
         $normalizedConfig = [];
         foreach ($config as $fieldName => $fieldConfig) {
-            if (
-                !(is_int($fieldName) && is_string($fieldConfig))
-                && !(is_string($fieldName) && is_array($fieldConfig))
-            ) {
-                throw new \InvalidArgumentException('Invalid config format');
+            if (!(is_int($fieldName) && is_string($fieldConfig) || is_string($fieldName) && is_array($fieldConfig))) {
+                throw new \InvalidArgumentException('Invalid config format.');
             }
 
             if (is_string($fieldConfig)) {
                 $fieldName = $fieldConfig;
                 $fieldConfig = [];
             }
 
-            if (!isset($fieldConfig['asc'], $fieldConfig['desc'])) {
-                $normalizedConfig[$fieldName] = array_merge([
-                    'asc' => [$fieldName => SORT_ASC],
-                    'desc' => [$fieldName => SORT_DESC],
-                    'default' => 'asc',
-                    'label' => $fieldName,
-                ], $fieldConfig);
-            } else {
-                $normalizedConfig[$fieldName] = $fieldConfig;
-            }
+            /** @psalm-var TConfig $fieldConfig */
+            $normalizedConfig[$fieldName] = array_merge([
+                'asc' => [$fieldName => SORT_ASC],
+                'desc' => [$fieldName => SORT_DESC],
+                'default' => 'asc',
+                'label' => $fieldName,
+            ], $fieldConfig);
         }
 
+        /** @psalm-var TConfig $normalizedConfig */
         $this->config = $normalizedConfig;
     }
 
@@ -99,7 +117,7 @@ public function withOrderString(string $orderString): self
     }
 
     /**
-     * @param array $order field names to order by as keys, direction as values
+     * @param array $order Field names to order by as keys, direction as values.
      *
      * @return $this
      */
@@ -124,14 +142,30 @@ public function getOrderAsString(): string
         return implode(',', $parts);
     }
 
+    /**
+     * Final sorting criteria to apply.
+     */
     public function getCriteria(): array
     {
         $criteria = [];
-        foreach ($this->getOrder() as $field => $direction) {
-            if (isset($this->config[$field][$direction])) {
-                $criteria = array_merge($criteria, $this->config[$field][$direction]);
+        $order = $this->getOrder();
+
+        $config = $this->config;
+
+        foreach ($order as $field => $direction) {
+            if (!array_key_exists($field, $config)) {
+                continue;
             }
+
+            $criteria += $config[$field][$direction];
+
+            unset($config[$field]);
         }
+
+        foreach ($config as $field => $fieldConfig) {
+            $criteria += $fieldConfig[$fieldConfig['default']];
+        }
+
         return $criteria;
     }
 }
diff --git a/tests/Reader/SortTest.php b/tests/Reader/SortTest.php
@@ -135,4 +135,63 @@ public function testGetCriteria(): void
             'bee' => SORT_ASC,
         ], $sort->getCriteria());
     }
+
+    public function testGetCriteriaDefaults(): void
+    {
+        $sort = (new Sort([
+            'b' => [
+                'asc' => ['bee' => SORT_ASC],
+                'desc' => ['bee' => SORT_DESC],
+                'default' => 'desc',
+                'label' => 'B',
+            ],
+        ]))
+            ->withOrder([]);
+
+        $this->assertSame([
+            'bee' => SORT_DESC,
+        ], $sort->getCriteria());
+    }
+
+    public function testGetCriteriaOrder(): void
+    {
+        $sort = (new Sort([
+            'b',
+            'c',
+        ]))
+            ->withOrder(['c' => 'desc']);
+
+        $this->assertSame([
+            'c' => SORT_DESC,
+            'b' => SORT_ASC,
+        ], $sort->getCriteria());
+    }
+
+    public function testGetCriteriaDefaultsWhenConfigIsNotComplete(): void
+    {
+        $sort = (new Sort([
+            'b' => [
+                'asc' => ['bee' => SORT_ASC],
+                'desc' => ['bee' => SORT_DESC],
+            ],
+        ]))
+            ->withOrder([]);
+
+        $this->assertSame([
+            'bee' => SORT_ASC,
+        ], $sort->getCriteria());
+    }
+
+    public function testGetCriteriaWithShortFieldSyntax(): void
+    {
+        $sort = (new Sort([
+            'id',
+            'name',
+        ]))->withOrder(['name' => 'desc']);
+
+        $this->assertSame([
+            'name' => SORT_DESC,
+            'id' => SORT_ASC,
+        ], $sort->getCriteria());
+    }
 }