From b00b995cbb39e0160f2f286af83267ae2a06e77f Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 15 Mar 2022 15:18:22 -0300 Subject: [PATCH] Bug 30165: (follow-up) Fix GET /acquisitions/orders This patch fixes the particular use case of the orders route, which has a slightly modified version of the objects.search helped, embeded in the controller itself. This controller gets adjusted to the fact q will now be an array. Because of the latter, we end up requiring more code duplication regarding the query fix, so I moved it to an internal sub that gets reused. To test: 1. Apply the previous patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/* => FAIL: It t/db_dependent/api/v1/acquisitions_orders.t fails! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart --- Koha/REST/V1/Acquisitions/Orders.pm | 71 +++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/Koha/REST/V1/Acquisitions/Orders.pm b/Koha/REST/V1/Acquisitions/Orders.pm index f46f4cfd7f4..c91353a0708 100644 --- a/Koha/REST/V1/Acquisitions/Orders.pm +++ b/Koha/REST/V1/Acquisitions/Orders.pm @@ -22,6 +22,7 @@ use Mojo::Base 'Mojolicious::Controller'; use Koha::Acquisition::Orders; use Clone qw( clone ); +use JSON; use Scalar::Util qw( blessed ); use Try::Tiny qw( catch try ); @@ -78,8 +79,7 @@ sub list { if ( exists $reserved_params->{_order_by} ) { # _order_by passed, fix if required for my $p ( @{$reserved_params->{_order_by}} ) { - $p =~ s|biblio\.|biblio\.biblioitem\.|g - if $p =~ m/.*(isbn|ean|publisher).*/; + $p = $c->table_name_fixer($p); } } @@ -132,24 +132,43 @@ sub list { } } - if ( defined $reserved_params->{q} || defined $reserved_params->{query} || defined $reserved_params->{'x-koha-query'}) { + if ( defined $reserved_params->{q} + || defined $reserved_params->{query} + || defined $reserved_params->{'x-koha-query'} ) + { + $filtered_params //={}; + my @query_params_array; - my $query_params; - # FIXME The following lines are an ugly fix to deal with isbn and ean searches - # This must NOT be reused or extended - # Instead we need a better and global solution in a Koha::*Biblio method - for my $q ( qw( q query x-koha-query ) ) { - next unless $reserved_params->{$q}; - for my $f ( qw( isbn ean publisher ) ) { - $reserved_params->{$q} =~ s|"biblio.$f":|"biblio.biblioitem.$f":|g; - } - push @query_params_array, $reserved_params->{$q}; + my $json = JSON->new; + + # q is defined as multi => JSON::Validator generates an array + # containing the string + foreach my $q ( @{ $reserved_params->{q} } ) { + push @query_params_array, + $json->decode( $c->table_name_fixer($q) ) + if $q; # skip if exists but is empty } - if(scalar(@query_params_array) > 1) { - $query_params = {'-and' => \@query_params_array}; + # x-koha-query contains a string + push @query_params_array, + $json->decode( + $c->table_name_fixer( $reserved_params->{'x-koha-query'} ) ) + if $reserved_params->{'x-koha-query'}; + + # query is already decoded by JSON::Validator at this point + push @query_params_array, + $json->decode( + $c->table_name_fixer( + $json->encode( $reserved_params->{query} ) + ) + ) if $reserved_params->{query}; + + my $query_params; + + if ( scalar(@query_params_array) > 1 ) { + $query_params = { '-and' => \@query_params_array }; } else { $query_params = $query_params_array[0]; @@ -309,4 +328,26 @@ sub delete { }; } +=head2 Internal methods + +=head3 table_name_fixer + + $q = $c->table_name_fixer( $q ); + +The Koha::Biblio representation includes the biblioitem.* attributes. This is handy +for API consumers but as they are different tables, converting the queries that mention +biblioitem columns can be tricky. This method renames known column names as used on Koha's +UI. + +=cut + +sub table_name_fixer { + my ( $self, $q ) = @_; + + $q =~ s|biblio\.|biblio\.biblioitem\.|g + if $q =~ m/.*(isbn|ean|publisher).*/; + + return $q; +} + 1; -- 2.25.1