From 66a9f731b53e81b4c8c59de85ee8f6f950106b07 Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Fri, 24 Jan 2020 23:59:32 -0300 Subject: [PATCH] Bug 24502: object.search also filter by prefetched columns 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 string, and sent within the q parameter. 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 --- Koha/REST/Plugin/Objects.pm | 6 +- Koha/REST/Plugin/Query.pm | 72 ++++++++++++++++++++++- t/Koha/REST/Plugin/Query.t | 47 ++++++++++++++- t/db_dependent/Koha/REST/Plugin/Objects.t | 42 ++++++++++++- 4 files changed, 163 insertions(+), 4 deletions(-) diff --git a/Koha/REST/Plugin/Objects.pm b/Koha/REST/Plugin/Objects.pm index b120f4842c..701af91465 100644 --- a/Koha/REST/Plugin/Objects.pm +++ b/Koha/REST/Plugin/Objects.pm @@ -97,6 +97,10 @@ sub register { } } + if( defined $reserved_params->{q} ) { + $filtered_params //={}; + $filtered_params = $c->merge_q_params( $filtered_params, $reserved_params->{q}, $result_set ); + } # Perform search my $objects = $result_set->search( $filtered_params, $attributes ); @@ -106,7 +110,7 @@ sub register { params => $args, }); } - + return $objects->to_api({ embed => $embed }); } ); diff --git a/Koha/REST/Plugin/Query.pm b/Koha/REST/Plugin/Query.pm index 54a710ec29..243f51a1a1 100644 --- a/Koha/REST/Plugin/Query.pm +++ b/Koha/REST/Plugin/Query.pm @@ -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); + + 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 ); return \@reserved_words; } @@ -340,4 +363,51 @@ sub _parse_prefetch { return $prefetch; } +sub _from_api_param { + my ($key, $result_set) = @_; + + if($key =~ /\./) { + + my ($curr, $next) = split /\s*\.\s*/, $key, 2; + + 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.'.'.(defined $result_set->from_api_mapping ? $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; diff --git a/t/Koha/REST/Plugin/Query.t b/t/Koha/REST/Plugin/Query.t index 46e4760c68..1747dec7bb 100644 --- a/t/Koha/REST/Plugin/Query.t +++ b/t/Koha/REST/Plugin/Query.t @@ -23,6 +23,7 @@ use Try::Tiny; use Koha::Cities; use Koha::Holds; +use Koha::Biblios; app->log->level('error'); @@ -134,6 +135,16 @@ 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->param('q'), $result_set); + + $c->render( json => $filtered_params, status => 200 ); +}; + get '/build_query' => sub { my $c = shift; my ( $filtered_params, $reserved_params ) = @@ -208,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 { @@ -296,6 +307,40 @@ 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?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; diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t index 5d9d8c7b12..05c647f12f 100644 --- a/t/db_dependent/Koha/REST/Plugin/Objects.t +++ b/t/db_dependent/Koha/REST/Plugin/Objects.t @@ -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,24 @@ get '/patrons/:patron_id/holds' => sub { $c->render( status => 200, json => {count => scalar(@$holds)} ); }; +get '/biblios' => sub { + my $c = shift; + $c->validation->output($c->req->params->to_hash); + 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 => 5; use Test::Mojo; use t::lib::TestBuilder; @@ -220,3 +237,26 @@ subtest 'objects.search helper, with path parameters and _match' => sub { $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; +} -- 2.17.1