From 71868d0b4558dcc0bcc6de33822b391aa1187374 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 makes the helper handling _order_by params expect a list of values instead of a (to-be-splitted) string. The idea is that the OpenAPI plugin will take care of splitting pipe-delimited values if the spec is correctly defined. 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 --- Koha/REST/Plugin/Query.pm | 7 ++++--- t/Koha/REST/Plugin/Query.t | 52 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm index 45cfe14e7c..13ef5e2222 100644 --- a/Koha/REST/Plugin/Query.pm +++ b/Koha/REST/Plugin/Query.pm @@ -81,7 +81,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; } @@ -120,9 +120,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/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index 28519622a6..ac126a17ae 100644 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -73,7 +73,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 ); @@ -117,9 +117,51 @@ 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; + + my $t = Test::Mojo->new; + + # _match => contains + $t->get_ok('/build_query?_match=contains&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => + { author => { like => '%Orson%' }, title => { like => '%Ender%' } } ); + + # _match => starts_with + $t->get_ok('/build_query?_match=starts_with&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => + { author => { like => 'Orson%' }, title => { like => 'Ender%' } } ); + + # _match => ends_with + $t->get_ok('/build_query?_match=ends_with&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => + { author => { like => '%Orson' }, title => { like => '%Ender' } } ); + + # _match => exact + $t->get_ok('/build_query?_match=exact&title=Ender&author=Orson') + ->status_is(200) + ->json_is( '/query' => { author => 'Orson', title => 'Ender' } ); + + # _match => blah + $t->get_ok('/build_query?_match=blah&title=Ender&author=Orson') + ->status_is(400) + ->json_is( '/exception_msg' => 'Invalid value for _match param (blah)' ) + ->json_is( '/exception_type' => 'Koha::Exceptions::WrongParameter' ); + }; -- 2.15.0