From f05a647c1cd4173a48a278e23696ae17ef2f22f9 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Fri, 26 Feb 2021 14:44:23 -0300
Subject: [PATCH] Bug 27680: Add support for param[] syntax

While not that common nowadays, it is the syntax PHP uses, and
DataTables also generates such thing in 'traditional' mode. We should
support it as well.

This patch adds support for that. It does so by adding _order_by[] to
the reserved param names, and proper handling on the dbic_merge_sorting
helper.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Tests pass!
3- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 Koha/REST/Plugin/Query.pm                 | 35 +++++++++++++++++------
 t/db_dependent/Koha/REST/Plugin/Objects.t | 25 +++++++++-------
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm
index 93ce2c2c494..25e66fc4725 100644
--- a/Koha/REST/Plugin/Query.pm
+++ b/Koha/REST/Plugin/Query.pm
@@ -90,19 +90,38 @@ Generates the DBIC order_by attributes based on I<$params>, and merges into I<$a
             my $attributes = $args->{attributes};
             my $result_set = $args->{result_set};
 
-            if ( defined $args->{params}->{_order_by} ) {
-                my $order_by = $args->{params}->{_order_by};
-                $order_by = [ split(/,/, $order_by) ] if ( !reftype($order_by) && index(',',$order_by) == -1);
+            my @order_by_styles = (
+                '_order_by',
+                '_order_by[]'
+            );
+            my @order_by_params;
+
+            foreach my $order_by_style ( @order_by_styles ) {
+                if ( defined $args->{params}->{$order_by_style} and ref($args->{params}->{$order_by_style}) eq 'ARRAY' )  {
+                    push( @order_by_params, @{$args->{params}->{$order_by_style} });
+                }
+                else {
+                    push @order_by_params, $args->{params}->{$order_by_style}
+                        if defined $args->{params}->{$order_by_style};
+                }
+            }
+
+            my @THE_order_by;
+
+            foreach my $order_by_param ( @order_by_params ) {
+                my $order_by = [ split(/,/, $order_by_param) ] if ( !reftype($order_by_param) && index(',',$order_by_param) == -1);
                 if ( reftype($order_by) and reftype($order_by) eq 'ARRAY' ) {
-                    my @order_by = map { _build_order_atom({ string => $_, result_set => $result_set }) }
-                                @{ $order_by };
-                    $attributes->{order_by} = \@order_by;
+                    my @order_by = map { _build_order_atom({ string => $_, result_set => $result_set }) } @{ $order_by };
+                    push( @THE_order_by, @order_by);
                 }
                 else {
-                    $attributes->{order_by} = _build_order_atom({ string => $order_by, result_set => $result_set });
+                    push @THE_order_by, _build_order_atom({ string => $order_by, result_set => $result_set });
                 }
             }
 
+            $attributes->{order_by} = \@THE_order_by
+                if scalar @THE_order_by > 0;
+
             return $attributes;
         }
     );
@@ -253,7 +272,7 @@ Merges parameters from $q_params into $filtered_params.
 
 sub _reserved_words {
 
-    my @reserved_words = qw( _match _order_by _page _per_page q query x-koha-query);
+    my @reserved_words = qw( _match _order_by _order_by[] _page _per_page q query x-koha-query);
     return \@reserved_words;
 }
 
diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t
index f6d9af83a4b..299e6b1b204 100755
--- a/t/db_dependent/Koha/REST/Plugin/Objects.t
+++ b/t/db_dependent/Koha/REST/Plugin/Objects.t
@@ -196,7 +196,7 @@ subtest 'objects.search helper' => sub {
 
 subtest 'objects.search helper, sorting on mapped column' => sub {
 
-    plan tests => 35;
+    plan tests => 42;
 
     $schema->storage->txn_begin;
 
@@ -209,7 +209,7 @@ subtest 'objects.search helper, sorting on mapped column' => sub {
     $builder->build_object({ class => 'Koha::Cities', value => { city_name => 'C', city_country => 'Belarus' } });
 
     my $t = Test::Mojo->new;
-    diag("CSV-param");
+    # CSV-param
     $t->get_ok('/cities?_order_by=%2Bname,-country')
       ->status_is(200)
       ->json_has('/0')
@@ -222,7 +222,7 @@ subtest 'objects.search helper, sorting on mapped column' => sub {
       ->json_is('/3/country' => 'Argentina')
       ->json_hasnt('/4');
 
-    diag("Multi-param: traditional");
+    # Multi-param: traditional
     $t->get_ok('/cities?_order_by=%2Bname&_order_by=-country')
       ->status_is(200)
       ->json_has('/0')
@@ -235,15 +235,20 @@ subtest 'objects.search helper, sorting on mapped column' => sub {
       ->json_is('/3/country' => 'Argentina')
       ->json_hasnt('/4');
 
-    diag("Pipe-param: Passes validation (treated as a 'single value array or one string), subsequently explodes");
-    $t->get_ok('/cities?_order_by=%2Bname|-country')
-      ->status_is(500);
-
-    diag("Multi-param: PHP Style, Passes validation as above, subsequntly explodes");
+    # Multi-param: PHP Style, Passes validation as above, subsequntly explodes
     $t->get_ok('/cities?_order_by[]=%2Bname&_order_by[]=-country')
-      ->status_is(500);
+      ->status_is(200)
+      ->json_has('/0')
+      ->json_has('/1')
+      ->json_is('/0/name' => 'A')
+      ->json_is('/1/name' => 'B')
+      ->json_is('/2/name' => 'C')
+      ->json_is('/2/country' => 'Belarus')
+      ->json_is('/3/name' => 'C')
+      ->json_is('/3/country' => 'Argentina')
+      ->json_hasnt('/4');
 
-    diag("Single-param");
+    # Single-param
     $t->get_ok('/cities?_order_by=-name')
       ->status_is(200)
       ->json_has('/0')
-- 
2.30.1