Bug 24502: object.search also filter by prefetched columns
authorAgustin Moyano <agustinmoyano@theke.io>
Sat, 25 Jan 2020 02:59:32 +0000 (23:59 -0300)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Tue, 3 Mar 2020 09:16:55 +0000 (09:16 +0000)
This patch adds the possibility to object.search helper, to also filter by prefetched columns.

In order to dynamically add filter parameters, they must be coded as json and placed in the body of the request, coded as string in 'q' query parameter or as string in 'x-koha-query' header.

The coded json, is in fact dbix syntax.

To test:
1. apply this patch
2. prove t/Koha/REST/Plugin/Query.t t/db_dependent/Koha/REST/Plugin/Objects.t
3. Sign off

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Koha/REST/Plugin/Objects.pm
Koha/REST/Plugin/Query.pm
t/Koha/REST/Plugin/Query.t
t/db_dependent/Koha/REST/Plugin/Objects.t

index 7e6347a..81cc976 100644 (file)
@@ -19,6 +19,8 @@ use Modern::Perl;
 
 use Mojo::Base 'Mojolicious::Plugin';
 
+use JSON qw(decode_json);
+
 =head1 NAME
 
 Koha::REST::Plugin::Objects
@@ -97,6 +99,22 @@ sub register {
                 }
             }
 
+            if( defined $reserved_params->{q} || defined $reserved_params->{query} || defined $reserved_params->{'x-koha-query'}) {
+                $filtered_params //={};
+                my @query_params_array;
+                my $query_params;
+                push @query_params_array, $reserved_params->{query} if defined $reserved_params->{query};
+                push @query_params_array, decode_json($reserved_params->{q}) if defined $reserved_params->{q};
+                push @query_params_array, decode_json($reserved_params->{'x-koha-query'}) if defined $reserved_params->{'x-koha-query'};
+
+                if(scalar(@query_params_array) > 1) {
+                    $query_params = {'-and' => \@query_params_array};
+                } else {
+                    $query_params = $query_params_array[0];
+                }
+
+                $filtered_params = $c->merge_q_params( $filtered_params, $query_params, $result_set );
+            }
             # Perform search
             my $objects = $result_set->search( $filtered_params, $attributes );
 
index 4bdbb90..9f6263e 100644 (file)
@@ -20,6 +20,7 @@ use Modern::Perl;
 use Mojo::Base 'Mojolicious::Plugin';
 use List::MoreUtils qw(any);
 use Scalar::Util qw(reftype);
+use JSON qw(decode_json);
 
 use Koha::Exceptions;
 
@@ -179,6 +180,28 @@ is raised.
         }
     );
 
+=head3 merge_q_params
+
+    $c->merge_q_params( $filtered_params, $q_params, $result_set );
+
+Merges parameters from $q_params into $filtered_params.
+
+=cut
+
+    $app->helper(
+        'merge_q_params' => sub {
+
+            my ( $c, $filtered_params, $q_params, $result_set ) = @_;
+
+            $q_params = decode_json($q_params) unless reftype $q_params;
+
+            my $params = _parse_dbic_query($q_params, $result_set);
+
+            return $params unless scalar(keys %{$filtered_params});
+            return {'-and' => [$params, $filtered_params ]};
+        }
+    );
+
 =head3 stash_embed
 
     $c->stash_embed( $c->match->endpoint->pattern->defaults->{'openapi.op_spec'} );
@@ -229,7 +252,7 @@ is raised.
 
 sub _reserved_words {
 
-    my @reserved_words = qw( _match _order_by _page _per_page );
+    my @reserved_words = qw( _match _order_by _page _per_page q query x-koha-query);
     return \@reserved_words;
 }
 
@@ -346,4 +369,53 @@ sub _parse_prefetch {
     return $prefetch;
 }
 
+sub _from_api_param {
+    my ($key, $result_set) = @_;
+
+    if($key =~ /\./) {
+
+        my ($curr, $next) = split /\s*\.\s*/, $key, 2;
+
+        return $curr.'.'._from_api_param($next, $result_set) if $curr eq 'me';
+
+        my $ko_class = $result_set->prefetch_whitelist->{$curr};
+
+        Koha::Exceptions::BadParameter->throw("Cannot find Koha::Object class for $curr")
+            unless defined $ko_class;
+
+        $result_set = $ko_class->new;
+
+        if ($next =~ /\./) {
+            return _from_api_param($next, $result_set);
+        } else {
+            return $curr.'.'.($result_set->from_api_mapping && defined $result_set->from_api_mapping->{$next} ? $result_set->from_api_mapping->{$next}:$next);
+        }
+    } else {
+        return defined $result_set->from_api_mapping->{$key} ? $result_set->from_api_mapping->{$key} : $key;
+    }
+}
+
+sub _parse_dbic_query {
+    my ($q_params, $result_set) = @_;
+
+    if(reftype($q_params) && reftype($q_params) eq 'HASH') {
+        my $parsed_hash;
+        foreach my $key (keys %{$q_params}) {
+            if($key =~ /-?(not_?)?bool/i ) {
+                $parsed_hash->{$key} = _from_api_param($q_params->{$key}, $result_set);
+                next;
+            }
+            my $k = _from_api_param($key, $result_set);
+            $parsed_hash->{$k} = _parse_dbic_query($q_params->{$key}, $result_set);
+        }
+        return $parsed_hash;
+    } elsif (reftype($q_params) && reftype($q_params) eq 'ARRAY') {
+        my @mapped = map{ _parse_dbic_query($_, $result_set) } @$q_params;
+        return \@mapped;
+    } else {
+        return $q_params;
+    }
+
+}
+
 1;
index a54c314..0ddd6fc 100644 (file)
@@ -23,6 +23,7 @@ use Try::Tiny;
 
 use Koha::Cities;
 use Koha::Holds;
+use Koha::Biblios;
 
 app->log->level('error');
 
@@ -134,6 +135,15 @@ get '/dbic_merge_prefetch' => sub {
     $c->render( json => $attributes, status => 200 );
 };
 
+get '/merge_q_params' => sub {
+  my $c = shift;
+  my $filtered_params = {'biblio_id' => 1};
+  my $result_set = Koha::Biblios->new;
+  $filtered_params = $c->merge_q_params($filtered_params, $c->req->json->{q}, $result_set);
+
+  $c->render( json => $filtered_params, status => 200 );
+};
+
 get '/build_query' => sub {
     my $c = shift;
     my ( $filtered_params, $reserved_params ) =
@@ -209,7 +219,7 @@ sub to_model {
 
 # The tests
 
-use Test::More tests => 5;
+use Test::More tests => 6;
 use Test::Mojo;
 
 subtest 'extract_reserved_params() tests' => sub {
@@ -297,6 +307,55 @@ subtest '/dbic_merge_prefetch' => sub {
       ->json_like( '/prefetch/1' => qr/item|biblio/ );
 };
 
+subtest '/merge_q_params' => sub {
+  plan tests => 3;
+  my $t = Test::Mojo->new;
+
+  $t->get_ok('/merge_q_params' => json => {
+    q => {
+      "-not_bool" => "suggestions.suggester.patron_card_lost",
+      "-or" => [
+        {
+          "creation_date" => {
+            "!=" => ["fff", "zzz", "xxx"]
+          }
+        },
+        { "suggestions.suggester.housebound_profile.frequency" => "123" },
+        {
+          "suggestions.suggester.library_id" => {"like" => "%CPL%"}
+        }
+      ]
+    }
+  })->status_is(200)
+    ->json_is( '/-and' => [
+        {
+          "-not_bool" => "suggester.lost",
+          "-or" => [
+            {
+              "datecreated" => {
+                "!=" => [
+                  "fff",
+                  "zzz",
+                  "xxx"
+                ]
+              }
+            },
+            {
+              "housebound_profile.frequency" => 123
+            },
+            {
+              "suggester.branchcode" => {
+                "like" => "\%CPL\%"
+              }
+            }
+          ]
+        },
+        {
+          "biblio_id" => 1
+        }
+      ]);
+};
+
 subtest '_build_query_params_from_api' => sub {
 
     plan tests => 16;
index 519467f..d92333e 100644 (file)
@@ -20,6 +20,7 @@ use Modern::Perl;
 use Koha::Acquisition::Orders;
 use Koha::Cities;
 use Koha::Holds;
+use Koha::Biblios;
 
 # Dummy app for testing the plugin
 use Mojolicious::Lite;
@@ -55,8 +56,28 @@ get '/patrons/:patron_id/holds' => sub {
     $c->render( status => 200, json => {count => scalar(@$holds)} );
 };
 
+get '/biblios' => sub {
+    my $c = shift;
+    my $output = $c->req->params->to_hash;
+    $output->{query} = $c->req->json if defined $c->req->json;
+    my $headers = $c->req->headers->to_hash;
+    $output->{'x-koha-query'} = $headers->{'x-koha-query'} if defined $headers->{'x-koha-query'};
+    $c->validation->output($output);
+    my $biblios_set = Koha::Biblios->new;
+    $c->stash("koha.embed", {
+        "suggestions" => {
+            children => {
+                "suggester" => {}
+            }
+        }
+    });
+    my $biblios = $c->objects->search($biblios_set);
+
+    $c->render( status => 200, json => {count => scalar(@$biblios)} );
+};
+
 # The tests
-use Test::More tests => 4;
+use Test::More tests => 8;
 use Test::Mojo;
 
 use t::lib::TestBuilder;
@@ -231,3 +252,98 @@ subtest 'objects.search helper, with path parameters and _match' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'object.search helper with query parameter' => sub {
+    plan tests => 4;
+
+    $schema->storage->txn_begin;
+
+    my $patron1 = $builder->build_object( { class => "Koha::Patrons" } );
+    my $patron2 = $builder->build_object( { class => "Koha::Patrons" } );
+    my $biblio1 = $builder->build_sample_biblio;
+    my $biblio2 = $builder->build_sample_biblio;
+    my $biblio3 = $builder->build_sample_biblio;
+    my $suggestion1 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron1->borrowernumber, biblionumber => $biblio1->biblionumber} } );
+    my $suggestion2 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio2->biblionumber} } );
+    my $suggestion3 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio3->biblionumber} } );
+
+    $t->get_ok('/biblios' => json => {"suggestions.suggester.patron_id" => $patron1->borrowernumber })
+      ->json_is('/count' => 1, 'there should be 1 biblio with suggestions of patron 1');
+
+    $t->get_ok('/biblios' => json => {"suggestions.suggester.patron_id" => $patron2->borrowernumber })
+      ->json_is('/count' => 2, 'there should be 2 biblios with suggestions of patron 2');
+
+    $schema->storage->txn_rollback;
+};
+
+subtest 'object.search helper with q parameter' => sub {
+    plan tests => 4;
+
+    $schema->storage->txn_begin;
+
+    my $patron1 = $builder->build_object( { class => "Koha::Patrons" } );
+    my $patron2 = $builder->build_object( { class => "Koha::Patrons" } );
+    my $biblio1 = $builder->build_sample_biblio;
+    my $biblio2 = $builder->build_sample_biblio;
+    my $biblio3 = $builder->build_sample_biblio;
+    my $suggestion1 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron1->borrowernumber, biblionumber => $biblio1->biblionumber} } );
+    my $suggestion2 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio2->biblionumber} } );
+    my $suggestion3 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio3->biblionumber} } );
+
+    $t->get_ok('/biblios?q={"suggestions.suggester.patron_id": "'.$patron1->borrowernumber.'"}')
+      ->json_is('/count' => 1, 'there should be 1 biblio with suggestions of patron 1');
+
+    $t->get_ok('/biblios?q={"suggestions.suggester.patron_id": "'.$patron2->borrowernumber.'"}')
+      ->json_is('/count' => 2, 'there should be 2 biblios with suggestions of patron 2');
+
+    $schema->storage->txn_rollback;
+};
+
+subtest 'object.search helper with x-koha-query header' => sub {
+    plan tests => 4;
+
+    $schema->storage->txn_begin;
+
+    my $patron1 = $builder->build_object( { class => "Koha::Patrons" } );
+    my $patron2 = $builder->build_object( { class => "Koha::Patrons" } );
+    my $biblio1 = $builder->build_sample_biblio;
+    my $biblio2 = $builder->build_sample_biblio;
+    my $biblio3 = $builder->build_sample_biblio;
+    my $suggestion1 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron1->borrowernumber, biblionumber => $biblio1->biblionumber} } );
+    my $suggestion2 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio2->biblionumber} } );
+    my $suggestion3 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio3->biblionumber} } );
+
+    $t->get_ok('/biblios' => {'x-koha-query' => '{"suggestions.suggester.patron_id": "'.$patron1->borrowernumber.'"}'})
+      ->json_is('/count' => 1, 'there should be 1 biblio with suggestions of patron 1');
+
+    $t->get_ok('/biblios' => {'x-koha-query' => '{"suggestions.suggester.patron_id": "'.$patron2->borrowernumber.'"}'})
+      ->json_is('/count' => 2, 'there should be 2 biblios with suggestions of patron 2');
+
+    $schema->storage->txn_rollback;
+};
+
+subtest 'object.search helper with all query methods' => sub {
+    plan tests => 6;
+
+    $schema->storage->txn_begin;
+
+    my $patron1 = $builder->build_object( { class => "Koha::Patrons" , value => {cardnumber => 'cardpatron1', firstname=>'patron1'} } );
+    my $patron2 = $builder->build_object( { class => "Koha::Patrons" , value => {cardnumber => 'cardpatron2', firstname=>'patron2'} } );
+    my $biblio1 = $builder->build_sample_biblio;
+    my $biblio2 = $builder->build_sample_biblio;
+    my $biblio3 = $builder->build_sample_biblio;
+    my $suggestion1 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron1->borrowernumber, biblionumber => $biblio1->biblionumber} } );
+    my $suggestion2 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio2->biblionumber} } );
+    my $suggestion3 = $builder->build_object( { class => "Koha::Suggestions", value => { suggestedby => $patron2->borrowernumber, biblionumber => $biblio3->biblionumber} } );
+
+    $t->get_ok('/biblios?q={"suggestions.suggester.firstname": "'.$patron1->firstname.'"}' => {'x-koha-query' => '{"suggestions.suggester.patron_id": "'.$patron1->borrowernumber.'"}'} => json => {"suggestions.suggester.cardnumber" => $patron1->cardnumber})
+      ->json_is('/count' => 1, 'there should be 1 biblio with suggestions of patron 1');
+
+    $t->get_ok('/biblios?q={"suggestions.suggester.firstname": "'.$patron2->firstname.'"}' => {'x-koha-query' => '{"suggestions.suggester.patron_id": "'.$patron2->borrowernumber.'"}'} => json => {"suggestions.suggester.cardnumber" => $patron2->cardnumber})
+      ->json_is('/count' => 2, 'there should be 2 biblios with suggestions of patron 2');
+
+    $t->get_ok('/biblios?q={"suggestions.suggester.firstname": "'.$patron1->firstname.'"}' => {'x-koha-query' => '{"suggestions.suggester.patron_id": "'.$patron2->borrowernumber.'"}'} => json => {"suggestions.suggester.cardnumber" => $patron2->cardnumber})
+      ->json_is('/count' => 0, 'there shouldn\'t be biblios where suggester has patron1 fistname and patron2 id');
+
+    $schema->storage->txn_rollback;
+};
\ No newline at end of file