Attachment 152984 Details for Bug 33974 – Bug 33974: Make biblioitem columns searchable in a generic way

[patch] Bug 33974: Make biblioitem columns searchable in a generic way

Bug-33974-Make-biblioitem-columns-searchable-in-a-.patch (text/plain), 9.35 KB, created by Martin Renvoize (ashimema) on 2023-07-03 15:27:17 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2023-07-03 15:27:17 UTC

Size: 9.35 KB

patch

obsolete

>From a13c4e9ff2eb4b921490f2974f20cf790b3ede58 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 16 Jun 2023 12:50:07 -0300
>Subject: [PATCH] Bug 33974: Make biblioitem columns searchable in a generic
> way
>
>This patch makes biblioitem attributes be searchable on the biblios
>endpoint. It does so by using the new method in Koha::Biblios, and by
>adjusting objects.search(_rs) to accept a $query_fixer arrayref of
>functions to be applied to each query or order_by parameters.
>
>The result is cleaner code to write, but complex internals.
>
>To test:
>1. Apply this patch
>2. Run:
>   $ ktd --shell
>  k$ prove t/db_dependent/api/v1/biblios.t
>=> SUCCESS: Tests pass! Searching for biblioitem attributes works on the
>API!
>3. Sign off :-D
>
>Signed-off-by: Sam Lau <samalau@gmail.com>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> Koha/REST/Plugin/Objects.pm     | 68 +++++++++++++++++++++++++--------
> Koha/REST/V1/Biblios.pm         |  9 +++--
> t/db_dependent/api/v1/biblios.t | 20 ++++++++--
> 3 files changed, 73 insertions(+), 24 deletions(-)
>
>diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm
>index 94baf03753..6313b912c6 100644
>--- a/Koha/REST/Plugin/Objects.pm
>+++ b/Koha/REST/Plugin/Objects.pm
>@@ -112,10 +112,10 @@ shouldn't be called twice in it.
> 
>     $app->helper(
>         'objects.search' => sub {
>-            my ( $c, $result_set ) = @_;
>+            my ( $c, $result_set, $query_fixers ) = @_;
> 
>             # Generate the resultset using the HTTP request information
>-            my $objects_rs = $c->objects->search_rs($result_set);
>+            my $objects_rs = $c->objects->search_rs( $result_set, $query_fixers );
> 
>             # Add pagination headers
>             $c->add_pagination_headers();
>@@ -127,13 +127,21 @@ shouldn't be called twice in it.
> =head3 objects.search_rs
> 
>     my $patrons_object = Koha::Patrons->new;
>-    my $patrons_rs = $c->objects->search_rs( $patrons_object );
>+    my $patrons_rs = $c->objects->search_rs( $patrons_object [, $query_fixers ] );
>     . . .
>     return $c->render({ status => 200, openapi => $patrons_rs->to_api });
> 
> Returns the passed Koha::Objects resultset filtered as requested by the api query
> parameters and with requested embeds applied.
> 
>+The optional I<$query_fixers> parameter is expected to be a reference to a list of
>+function references. This functions need to accept two parameters: ( $query, $no_quotes ).
>+
>+The I<$query> is a string to be adapted. A modified version will be returned. The
>+I<$no_quotes> parameter controls if quotes need to be added for matching inside the string.
>+Quoting should be used by default, for replacing JSON keys e.g "biblio.isbn" would match
>+and biblio.isbn wouldn't.
>+
> Warning: this helper stashes base values for the pagination headers to the calling
> controller, and thus shouldn't be called twice in it.
> 
>@@ -141,15 +149,26 @@ controller, and thus shouldn't be called twice in it.
> 
>     $app->helper(
>         'objects.search_rs' => sub {
>-            my ( $c, $result_set ) = @_;
>+            my ( $c, $result_set, $query_fixers ) = @_;
> 
>             my $args       = $c->validation->output;
>             my $attributes = {};
> 
>+            $query_fixers //= [];
>+
>             # Extract reserved params
>             my ( $filtered_params, $reserved_params, $path_params ) =
>               $c->extract_reserved_params($args);
> 
>+            if ( exists $reserved_params->{_order_by} ) {
>+                # _order_by passed, fix if required
>+                for my $p ( @{$reserved_params->{_order_by}} ) {
>+                    foreach my $qf ( @{$query_fixers} ) {
>+                        $p = $qf->($p, 1); # 1 => no quotes on matching
>+                    }
>+                }
>+            }
>+
>             # Merge sorting into query attributes
>             $c->dbic_merge_sorting(
>                 {
>@@ -204,31 +223,48 @@ controller, and thus shouldn't be called twice in it.
> 
>                 my @query_params_array;
> 
>-                # query in request body, JSON::Validator already decoded it
>-                push @query_params_array, $reserved_params->{query}
>-                  if defined $reserved_params->{query};
>-
>                 my $json = JSON->new;
> 
>+                # query in request body, JSON::Validator already decoded it
>+                if ( $reserved_params->{query} ) {
>+                    my $query = $json->encode( $reserved_params->{query} );
>+                    foreach my $qf ( @{$query_fixers} ) {
>+                        $query = $qf->($query);
>+                    }
>+                    push @query_params_array, $json->decode($query);
>+                }
>+
>                 if ( ref( $reserved_params->{q} ) eq 'ARRAY' ) {
> 
>                    # q is defined as multi => JSON::Validator generates an array
>                     foreach my $q ( @{ $reserved_params->{q} } ) {
>-                        push @query_params_array, $json->decode($q)
>-                          if $q;    # skip if exists but is empty
>+                        if ( $q ) { # skip if exists but is empty
>+                            foreach my $qf (@{$query_fixers}) {
>+                                $q = $qf->($q);
>+                            }
>+                            push @query_params_array, $json->decode($q);
>+                        }
>                     }
>                 }
>                 else {
>                     # objects.search called outside OpenAPI context
>                     # might be a hashref
>-                    push @query_params_array,
>-                      $json->decode( $reserved_params->{q} )
>-                      if $reserved_params->{q};
>+                    if ( $reserved_params->{q} ) {
>+                        my $q = $reserved_params->{q};
>+                        foreach my $qf (@{$query_fixers}) {
>+                            $q = $qf->($q);
>+                        }
>+                        push @query_params_array, $json->decode( $q );
>+                    }
>                 }
> 
>-                push @query_params_array,
>-                  $json->decode( $reserved_params->{'x-koha-query'} )
>-                  if defined $reserved_params->{'x-koha-query'};
>+                if ( $reserved_params->{'x-koha-query'} ) {
>+                    my $q = $reserved_params->{'x-koha-query'};
>+                    foreach my $qf (@{$query_fixers}) {
>+                        $q = $qf->($q);
>+                    }
>+                    push @query_params_array, $json->decode( $q );
>+                }
> 
>                 my $query_params;
> 
>diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm
>index 6457d49d7a..bbfec9c1c7 100644
>--- a/Koha/REST/V1/Biblios.pm
>+++ b/Koha/REST/V1/Biblios.pm
>@@ -31,6 +31,7 @@ use C4::Context;
> 
> use Koha::Items;
> 
>+use Clone qw( clone );
> use List::MoreUtils qw( any );
> use MARC::Record::MiJ;
> 
>@@ -797,12 +798,12 @@ Controller function that handles retrieving a single biblio object
> sub list {
>     my $c = shift->openapi->valid_input or return;
> 
>-    my $attributes;
>-    $attributes =
>-      { prefetch => ['metadata'] }    # don't prefetch metadata if not needed
>+    my @prefetch = qw(biblioitem);
>+    push @prefetch, 'metadata'    # don't prefetch metadata if not needed
>       unless $c->req->headers->accept =~ m/application\/json/;
> 
>-    my $biblios = $c->objects->search_rs( Koha::Biblios->new );
>+    my $rs = Koha::Biblios->search( undef, { prefetch => \@prefetch });
>+    my $biblios = $c->objects->search_rs( $rs, [(sub{ $rs->api_query_fixer( $_[0], '', $_[1] ) })] );
> 
>     return try {
> 
>diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t
>index eb6dcaa78b..2fd0477fd5 100755
>--- a/t/db_dependent/api/v1/biblios.t
>+++ b/t/db_dependent/api/v1/biblios.t
>@@ -36,6 +36,8 @@ use Koha::Database;
> use Koha::Checkouts;
> use Koha::Old::Checkouts;
> 
>+use Mojo::JSON qw(encode_json);
>+
> my $schema  = Koha::Database->new->schema;
> my $builder = t::lib::TestBuilder->new;
> 
>@@ -1617,7 +1619,7 @@ subtest 'put() tests' => sub {
> 
> subtest 'list() tests' => sub {
> 
>-    plan tests => 15;
>+    plan tests => 17;
> 
>     $schema->storage->txn_begin;
> 
>@@ -1651,8 +1653,7 @@ subtest 'list() tests' => sub {
>     t::lib::Mocks::mock_preference('marcflavour', 'MARC21');
>     my $biblionumber2 = $builder->build_sample_biblio->biblionumber;
> 
>-    my $search =
>-"[{\"biblionumber\": \"$biblionumber1\"}, {\"biblionumber\": \"$biblionumber2\"}]";
>+    my $search = encode_json( [ { 'me.biblio_id' => $biblionumber1 }, { 'me.biblio_id' => $biblionumber2 } ] );
>     $t->get_ok(
>         "//$userid:$password@/api/v1/biblios/" => { 'x-koha-query' => $search }
>     )->status_is(403);
>@@ -1686,6 +1687,17 @@ subtest 'list() tests' => sub {
>           { Accept => 'text/plain', 'x-koha-query' => $search } )
>       ->status_is(200);
> 
>+    # DELETE any biblio with ISBN = TOMAS
>+    Koha::Biblios->search({ 'biblioitem.isbn' => 'TOMAS' }, { join => [ 'biblioitem' ] })
>+                 ->delete;
>+
>+
>+    my $isbn_query = encode_json({ isbn => 'TOMAS' });
>+    $biblio->biblioitem->set({ isbn => 'TOMAS' })->store;
>+    $t->get_ok( "//$userid:$password@/api/v1/biblios?q=$isbn_query" =>
>+          { Accept => 'text/plain' } )
>+      ->status_is(200);
>+
>     $schema->storage->txn_rollback;
> };
> 
>@@ -1801,4 +1813,4 @@ subtest 'update_item() tests' => sub {
>     ->json_is('/replacement_price', 30);
> 
>   $schema->storage->txn_rollback;
>-};
>\ No newline at end of file
>+};
>-- 
>2.41.0

Actions: View | Diff | Splinter Review

Attachments on bug 33974: 152433 | 152434 | 152435 | 152487 | 152488 | 152489 | 152765 | 152766 | 152767 | 152983 | 152984 | 152985 | 152986 | 152988 | 153104