Attachment 131823 Details for Bug 30165 – Bug 30165: (follow-up) Fix GET /acquisitions/orders

[patch] Bug 30165: (follow-up) Fix GET /acquisitions/orders

Bug-30165-follow-up-Fix-GET-acquisitionsorders.patch (text/plain), 4.79 KB, created by Jonathan Druart on 2022-03-17 08:35:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2022-03-17 08:35:09 UTC

Size: 4.79 KB

patch

obsolete

>From b00b995cbb39e0160f2f286af83267ae2a06e77f Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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 <tomascohen@theke.io>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 30165: 131111 | 131706 | 131707 | 131759 | 131760 | 131761 | 131803 | 131804 | 131805 | 131821 | 131822 | 131823 | 132138 | 132139