From c6b8c98a3e9abf4678ff471a0af2492e8125cd52 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 23 Nov 2017 10:45:59 -0300
Subject: [PATCH] Bug 19370: (QA followup) Use OpenAPI's handling of pipe
 separated values

This patch changes the _order_by definition so it's values are handled
correctly by the Mojolicious::Plugin::OpenAPI plugin.

Code is adjusted to expect a list instead of a (to-be-splitted) string.

Tests are adjusted too.

Note: In the process I noticed + on the URL represents a space, so the
helper function is updated to handle both + and %2B as ascending.

To test:
- Run:
  $ kshell
 k$ prove t/Koha/REST/Plugin/Query.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 Koha/REST/Plugin/Query.pm      |  7 ++++---
 api/v1/swagger/parameters.json |  6 +++++-
 t/Koha/REST/Plugin/Query.t     | 16 +++++++++++-----
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm
index 77d2fadd6a..090f02fe0b 100644
--- a/Koha/REST/Plugin/Query.pm
+++ b/Koha/REST/Plugin/Query.pm
@@ -83,7 +83,7 @@ Generates the DBIC order_by attributes based on I<$params>, and merges into I<$a
 
             if ( defined $args->{params}->{_order_by} ) {
                 my @order_by = map { _build_order_atom($_) }
-                                split( /\|/, $args->{params}->{_order_by} );
+                               @{ $args->{params}->{_order_by} };
                 $attributes->{order_by} = \@order_by;
             }
 
@@ -167,9 +167,10 @@ according to the following rules:
 sub _build_order_atom {
     my $string = shift;
 
-    if ( $string =~ m/^\+/ ) {
+    if ( $string =~ m/^\+/ or
+         $string =~ m/^\s/ ) {
         # asc order operator present
-        $string =~ s/^\+//;
+        $string =~ s/^(\+|\s)//;
         return { -asc => $string };
     }
     elsif ( $string =~ m/^\-/ ) {
diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json
index 482f7e8c53..b5a2b36207 100644
--- a/api/v1/swagger/parameters.json
+++ b/api/v1/swagger/parameters.json
@@ -32,7 +32,11 @@
     "in": "query",
     "required": false,
     "description": "Sorting criteria",
-    "type": "string"
+    "type": "array",
+    "collectionFormat": "pipes",
+    "items": {
+        "type": "string"
+    }
   },
   "page": {
     "name": "_page",
diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t
index a4c6684ba5..cdcdb4ca92 100644
--- a/t/Koha/REST/Plugin/Query.t
+++ b/t/Koha/REST/Plugin/Query.t
@@ -74,7 +74,7 @@ get '/dbic_merge_sorting' => sub {
     $attributes = $c->dbic_merge_sorting(
         {
             attributes => $attributes,
-            params     => { _match => 'exact', _order_by => 'uno|-dos|+tres' }
+            params     => { _match => 'exact', _order_by => [ 'uno', '-dos', '+tres', ' cuatro' ] }
         }
     );
     $c->render( json => $attributes, status => 200 );
@@ -135,13 +135,19 @@ subtest 'dbic_merge_sorting() tests' => sub {
 
     my $t = Test::Mojo->new;
 
-    $t->get_ok('/dbic_merge_sorting')
-      ->status_is(200)
+    $t->get_ok('/dbic_merge_sorting')->status_is(200)
       ->json_is( '/a' => 'a', 'Existing values are kept (a)' )
-      ->json_is( '/b' => 'b', 'Existing values are kept (b)' )
-      ->json_is( '/order_by' => [ 'uno', { -desc => 'dos' }, { -asc => 'tres' } ] );
+      ->json_is( '/b' => 'b', 'Existing values are kept (b)' )->json_is(
+        '/order_by' => [
+            'uno',
+            { -desc => 'dos' },
+            { -asc  => 'tres' },
+            { -asc  => 'cuatro' }
+        ]
+      );
 };
 
+
 subtest '_build_query_params_from_api' => sub {
 
     plan tests => 16;
-- 
2.15.0