bug #310 [Core] Fix ordering not properly handled (sstok)

This PR was merged into the 2.0-dev branch.

Discussion
----------

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tickets       | 
| License       | MIT

- Ordering direction was not included in exported conditions
- String (view format) didn't support localized labels
- The OrderStructureBuilder didn't use the correct transformer when changing fields; as this is configurable per field

**IMPORTANT**

```PHP
            $fieldSet->add('@Date', OrderFieldType::class, ['case' => OrderTransformer::CASE_LOWERCASE, 'alias' => ['up' => 'ASC', 'down' => 'DESC'], 'default' => 'down']);
            $fieldSet->add('@id', OrderFieldType::class, ['default' => 'ASC']);
```

Previously the `@id` would contain the same transformer as the `@date` field (due to a bug in the `OrderStructureBuilder`), but this is incorrect behavior as each field has it's own configuration. 
If your current configuration stops working this is why.

Commits
-------

7371077 [Core] Fix ordering not properly handled

lib/Core/Exporter/JsonExporter.php

@@ -32,7 +32,32 @@ final class JsonExporter extends AbstractExporter
 {
     public function exportCondition(SearchCondition $condition): string
     {
-        return (string) json_encode($this->exportGroup($condition->getValuesGroup(), $condition->getFieldSet(), true));
+        $fieldSet = $condition->getFieldSet();
+
+        return (string) json_encode(
+            array_merge(
+                $this->exportOrder($condition, $fieldSet),
+                $this->exportGroup($condition->getValuesGroup(), $fieldSet, true)
+            ),
+            \JSON_THROW_ON_ERROR
+        );
+    }
+
+    protected function exportOrder(SearchCondition $condition, FieldSet $fieldSet): array
+    {
+        $order = $condition->getOrder();
+
+        if ($order === null) {
+            return [];
+        }
+
+        $result = [];
+
+        foreach ($order->getFields() as $name => $direction) {
+            $result[mb_substr($name, 1)] = $this->modelToNorm($direction, $fieldSet->get($name));
+        }
+
+        return $result ? ['order' => $result] : [];
     }
 
     protected function exportGroup(ValuesGroup $valuesGroup, FieldSet $fieldSet, bool $isRoot = false): array

lib/Core/Exporter/StringExporter.php

@@ -37,13 +37,31 @@ abstract class StringExporter extends AbstractExporter
 
     public function exportCondition(SearchCondition $condition): string
     {
-        $this->fields = $this->resolveLabels($condition->getFieldSet());
+        $this->fields = $this->resolveLabels($fieldSet = $condition->getFieldSet());
 
-        return $this->exportGroup($condition->getValuesGroup(), $condition->getFieldSet(), true);
+        return $this->exportOrder($condition, $fieldSet) . $this->exportGroup($condition->getValuesGroup(), $fieldSet, true);
     }
 
     abstract protected function resolveLabels(FieldSet $fieldSet): array;
 
+    protected function exportOrder(SearchCondition $condition, FieldSet $fieldSet): string
+    {
+        $order = $condition->getOrder();
+
+        if ($order === null) {
+            return '';
+        }
+
+        $result = '';
+
+        foreach ($order->getFields() as $name => $direction) {
+            $result .= $this->getFieldLabel($name);
+            $result .= ': ' . $this->modelToExported($direction, $fieldSet->get($name)) . '; ';
+        }
+
+        return trim($result);
+    }
+
     protected function exportGroup(ValuesGroup $valuesGroup, FieldSet $fieldSet, bool $isRoot = false): string
     {
         $result = '';

lib/Core/Extension/Core/DataTransformer/OrderToLocalizedTransformer.php

@@ -0,0 +1,99 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the RollerworksSearch package.
+ *
+ * (c) Sebastiaan Stok <[email protected]>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Rollerworks\Component\Search\Extension\Core\DataTransformer;
+
+use Rollerworks\Component\Search\DataTransformer;
+use Rollerworks\Component\Search\Exception\TransformationFailedException;
+
+final class OrderToLocalizedTransformer implements DataTransformer
+{
+    private array $alias;
+    private array $viewLabel;
+    private string $case;
+
+    public function __construct(array $alias, array $viewLabel, string $case = OrderTransformer::CASE_UPPERCASE)
+    {
+        $this->case = $case;
+        $this->alias = $alias;
+        $this->viewLabel = $viewLabel;
+    }
+
+    public function transform($value)
+    {
+        if ($value === null) {
+            return '';
+        }
+
+        if (! \is_string($value)) {
+            throw new TransformationFailedException('Expected a string or null.');
+        }
+
+        switch ($this->case) {
+            case OrderTransformer::CASE_LOWERCASE:
+                $value = mb_strtolower($value);
+
+                break;
+
+            case OrderTransformer::CASE_UPPERCASE:
+                $value = mb_strtoupper($value);
+
+                break;
+        }
+
+        if (! isset($this->viewLabel[$value])) {
+            throw new TransformationFailedException(sprintf('No localized label configured for "%s".', $value));
+        }
+
+        return $this->viewLabel[$value];
+    }
+
+    public function reverseTransform($value)
+    {
+        if ($value !== null && ! \is_string($value)) {
+            throw new TransformationFailedException('Expected a string or null.');
+        }
+
+        if ($value === '') {
+            return null;
+        }
+
+        switch ($this->case) {
+            case OrderTransformer::CASE_LOWERCASE:
+                $value = mb_strtolower($value);
+
+                break;
+
+            case OrderTransformer::CASE_UPPERCASE:
+                $value = mb_strtoupper($value);
+
+                break;
+        }
+
+        if (! isset($this->alias[$value])) {
+            throw new TransformationFailedException(
+                sprintf(
+                    'Invalid sort direction "%1$s" specified, expected one of: "%2$s"',
+                    $value,
+                    implode('", "', array_keys($this->alias))
+                ),
+                0,
+                null,
+                'This value is not a valid sorting direction. Accepted directions are "{{ directions }}".',
+                ['{{ directions }}' => mb_strtolower(implode('", "', array_unique(array_keys($this->alias))))]
+            );
+        }
+
+        return $this->alias[$value];
+    }
+}

lib/Core/Extension/Core/DataTransformer/OrderTransformer.php

@@ -34,16 +34,10 @@ final class OrderTransformer implements DataTransformer
      */
     private $case;
 
-    /**
-     * @var string|null
-     */
-    private $default;
-
-    public function __construct(array $alias, string $case = self::CASE_UPPERCASE, ?string $default = null)
+    public function __construct(array $alias, string $case = self::CASE_UPPERCASE)
     {
         $this->alias = $alias;
         $this->case = $case;
-        $this->default = $default;
     }
 
     public function transform($value)
@@ -87,7 +81,11 @@ public function reverseTransform($value)
                     'Invalid sort direction "%1$s" specified, expected one of: "%2$s"',
                     $value,
                     implode('", "', array_keys($this->alias))
-                )
+                ),
+                0,
+                null,
+                'This value is not a valid sorting direction. Accepted directions are "{{ directions }}".',
+                ['{{ directions }}' => mb_strtolower(implode('", "', array_unique(array_keys($this->alias))))]
             );
         }
 

lib/Core/Field/OrderFieldType.php

@@ -13,7 +13,9 @@
 
 namespace Rollerworks\Component\Search\Field;
 
+use Rollerworks\Component\Search\Extension\Core\DataTransformer\OrderToLocalizedTransformer;
 use Rollerworks\Component\Search\Extension\Core\DataTransformer\OrderTransformer;
+use Symfony\Component\OptionsResolver\Options;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 /**
@@ -32,6 +34,7 @@ public function configureOptions(OptionsResolver $resolver): void
             'default' => null,
             'case' => OrderTransformer::CASE_UPPERCASE,
             'alias' => ['ASC' => 'ASC', 'DESC' => 'DESC'],
+            'view_label' => ['ASC' => 'asc', 'DESC' => 'desc'],
             'type' => null,
             'type_options' => [],
         ]);
@@ -41,17 +44,46 @@ public function configureOptions(OptionsResolver $resolver): void
             OrderTransformer::CASE_UPPERCASE,
         ]);
         $resolver->setAllowedTypes('alias', 'array');
+        $resolver->setAllowedTypes('view_label', ['array']);
         $resolver->setAllowedTypes('default', ['null', 'string']);
         $resolver->setAllowedTypes('type', ['string', 'null']);
         $resolver->setAllowedTypes('type_options', ['array']);
+
+        // Ensure view-labels are part of the alias list.
+        $resolver->addNormalizer('alias', static function (Options $options, array $value): mixed {
+            // Must always exist for interoperability, but it's still possible to overwrite.
+            $value = array_merge($value,
+                $options['case'] === OrderTransformer::CASE_LOWERCASE ? ['asc' => 'ASC', 'desc' => 'DESC'] : ['ASC' => 'ASC', 'DESC' => 'DESC']
+            );
+
+            foreach ($options['view_label'] as $direction => $label) {
+                switch ($options['case']) {
+                    case OrderTransformer::CASE_LOWERCASE:
+                        $label = mb_strtolower($label);
+
+                        break;
+
+                    case OrderTransformer::CASE_UPPERCASE:
+                        $label = mb_strtoupper($label);
+
+                        break;
+                }
+
+                if (isset($value[$label])) {
+                    continue;
+                }
+
+                $value[$label] = $direction;
+            }
+
+            return $value;
+        });
     }
 
     public function buildType(FieldConfig $config, array $options): void
     {
-        $transformer = new OrderTransformer($options['alias'], $options['case'], $options['default']);
-
-        $config->setNormTransformer($transformer);
-        $config->setViewTransformer($transformer);
+        $config->setNormTransformer(new OrderTransformer($options['alias'], $options['case']));
+        $config->setViewTransformer(new OrderToLocalizedTransformer($options['alias'], $options['view_label'], $options['case']));
     }
 
     public function buildView(SearchFieldView $view, FieldConfig $config, array $options): void

lib/Core/Input/JsonInput.php

@@ -35,6 +35,8 @@
  * Each entry must contain an array with 'fields' and/or 'groups' structures.
  * Optionally the array can contain 'logical-case' => 'OR' to make it OR-cased.
  *
+ * The 'order' setting can only be applied at root level, and must NOT begin with the @-sign.
+ *
  * The 'groups' array contains groups with the keys as described above ('fields' and/or 'groups').
  *
  * The fields array is an hash-map where each key is the field-name

lib/Core/Input/OrderStructureBuilder.php

@@ -59,13 +59,16 @@ final class OrderStructureBuilder implements StructureBuilder
      */
     private $inputTransformer;
 
-    public function __construct(ProcessorConfig $config, Validator $validator, ErrorList $errorList, string $path = '')
+    private bool $viewFormat;
+
+    public function __construct(ProcessorConfig $config, Validator $validator, ErrorList $errorList, string $path = '', bool $viewFormat = false)
     {
         $this->fieldSet = $config->getFieldSet();
         $this->validator = $validator;
         $this->path = $path ?: 'order';
         $this->errorList = $errorList;
         $this->valuesGroup = new ValuesGroup();
+        $this->viewFormat = $viewFormat;
     }
 
     public function getErrors(): ErrorList
@@ -100,6 +103,7 @@ public function field(string $name, string $path): void
         }
 
         $this->fieldConfig = $this->fieldSet->get($name);
+        $this->inputTransformer = ($this->viewFormat ? $this->fieldConfig->getViewTransformer() : $this->fieldConfig->getNormTransformer()) ?? false;
 
         $this->valuesBag = $this->valuesGroup->getField($name);
 
@@ -182,10 +186,6 @@ private function addError(ConditionErrorMessage $error): void
      */
     private function inputToNorm($value, string $path)
     {
-        if ($this->inputTransformer === null) {
-            $this->inputTransformer = $this->fieldConfig->getNormTransformer() ?? false;
-        }
-
         if ($this->inputTransformer === false) {
             if ($value !== null && ! \is_scalar($value)) {
                 $e = new \RuntimeException(

lib/Core/Input/ProcessorConfig.php

@@ -134,7 +134,6 @@ public function getCacheTTL(): \DateInterval|int|null
         return $this->cacheTTL;
     }
 
-
     public function getDefaultField(bool $error = false): ?string
     {
         if ($this->defaultField === null && $error) {

lib/Core/Input/StringQueryInput.php

@@ -71,7 +71,9 @@ protected function initForProcess(ProcessorConfig $config): void
         $this->orderStructureBuilder = new OrderStructureBuilder(
             $this->config,
             $this->validator,
-            $this->errors
+            $this->errors,
+            '',
+            true
         );
     }
 }

lib/Core/Resources/translations/validators.en.xlf

@@ -42,6 +42,10 @@
                 <source>This value is not a valid datetime.</source>
                 <target>This value is not a valid datetime.</target>
             </trans-unit>
+            <trans-unit id="11">
+                <source>This value is not a valid sorting direction. Accepted directions are "{{ directions }}".</source>
+                <target>This value is not a valid sorting direction. Accepted directions are "{{ directions }}".</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>

lib/Core/Resources/translations/validators.nl.xlf

@@ -43,6 +43,10 @@
                 <source>This value is not a valid datetime.</source>
                 <target>Deze waarde is geen geldige datum en tijd.</target>
             </trans-unit>
+            <trans-unit id="11">
+                <source>This value is not a valid sorting direction. Accepted directions are "{{ directions }}".</source>
+                <target>Deze waarde is geen geldige sorteer richting. De geaccepteerde richtingen zijn : "{{ directions }}".</target>
+            </trans-unit>
         </body>
     </file>
 </xliff>