From 79064a88639ba725cc898cb940c85ff3fb1d4bee Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Bug 27680: (QA follow-up) Minor perlcritic issue Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart Signed-off-by: Jonathan Druart --- Koha/REST/Plugin/Query.pm | 42 ++++++++++++++++++----- t/db_dependent/Koha/REST/Plugin/Objects.t | 25 ++++++++------ 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm index 93ce2c2c49..ef70c9b55a 100644 --- a/Koha/REST/Plugin/Query.pm +++ b/Koha/REST/Plugin/Query.pm @@ -90,19 +90,43 @@ 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); - 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_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 { - $attributes->{order_by} = _build_order_atom({ string => $order_by, result_set => $result_set }); + 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; + $order_by = [ split(/,/, $order_by_param) ] + if ( !reftype($order_by_param) && index(',',$order_by_param) == -1); + + if ($order_by) { + if ( reftype($order_by) and reftype($order_by) eq 'ARRAY' ) { + my @order_by = map { _build_order_atom({ string => $_, result_set => $result_set }) } @{ $order_by }; + push( @THE_order_by, @order_by); + } + else { + 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 +277,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 f6d9af83a4..299e6b1b20 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.39.5