From 112d6d08e64d5c66e2b0e65fe024f57fc72b9472 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 17 Jan 2020 13:05:24 -0300 Subject: [PATCH] Bug 20212: Add more embeddable objects to orders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds options to embed more related objects based on the needs by parcel.tt. For filtering by biblioitems fields (ISBN and EAN) I had to make the 'list' method a modified version of the objects->search helper. I thought of doing it in a more generic way but I didn't find any other use cases and it would certainly make an already complex piece of code even more complex. So this is quite similar, but at some steps the biblio. gets translated into the proper relation names, and the same happens for prefetching. A new parameter is also added: only_active. It makes the controller use Koha::Acquisition::Orders->filter_by_active, avoiding the need to build complex queries in the UI. The same handling is done when the order_id parameter is passed (outside the q= parameters). In this case using Koha::Acquisition::Orders->filter_by_id_including_transfers This is all respecting the C4::Acquisitions::SearchOrders behaviour. TL;DR: This patch adapts the code from the list() sub so it manipulates the query parameters and the embed header so: - the biblioitem relationship is prefetch - any queries on biblio.isbn and biblio.ean are correctly translated into search on the biblioitems table. - Adds an only_active parameter to the /acquisitions/orders route to easily request only the active orders. Signed-off-by: Séverine QUEUNE --- Koha/REST/V1/Acquisitions/Orders.pm | 163 +++++++++++++++++++++++++- api/v1/swagger/definitions/order.json | 9 +- api/v1/swagger/paths/acquisitions_orders.json | 38 +++++- 3 files changed, 203 insertions(+), 7 deletions(-) diff --git a/Koha/REST/V1/Acquisitions/Orders.pm b/Koha/REST/V1/Acquisitions/Orders.pm index a9ed512..3fb9b28 100644 --- a/Koha/REST/V1/Acquisitions/Orders.pm +++ b/Koha/REST/V1/Acquisitions/Orders.pm @@ -22,6 +22,8 @@ use Mojo::Base 'Mojolicious::Controller'; use Koha::Acquisition::Orders; use Koha::DateUtils; +use Clone 'clone'; +use JSON qw(decode_json); use Scalar::Util qw( blessed ); use Try::Tiny; @@ -44,11 +46,120 @@ sub list { my $c = shift->openapi->valid_input or return; return try { - my $orders = $c->objects->search( Koha::Acquisition::Orders->new ); + + my $only_active = delete $c->validation->output->{only_active}; + my $order_id = delete $c->validation->output->{order_id}; + + my $orders_rs; + + if ( $only_active ) { + $orders_rs = Koha::Acquisition::Orders->filter_by_active; + } + else { + $orders_rs = Koha::Acquisition::Orders->new; + } + + $orders_rs = $orders_rs->filter_by_id_including_transfers({ ordernumber => $order_id }) + if $order_id; + + my $args = $c->validation->output; + my $attributes = {}; + + # Extract reserved params + my ( $filtered_params, $reserved_params, $path_params ) = $c->extract_reserved_params($args); + # Look for embeds + my $embed = $c->stash('koha.embed'); + my $fixed_embed = clone($embed); + if ( exists $fixed_embed->{biblio} ) { + # Add biblioitems to prefetch + # FIXME remove if we merge biblio + biblioitems + $fixed_embed->{biblio}->{children}->{biblioitem} = {}; + $c->stash('koha.embed', $fixed_embed); + } + + # If no pagination parameters are passed, default + $reserved_params->{_per_page} //= C4::Context->preference('RESTdefaultPageSize'); + $reserved_params->{_page} //= 1; + + unless ( $reserved_params->{_per_page} == -1 ) { + # Merge pagination into query attributes + $c->dbic_merge_pagination( + { + filter => $attributes, + params => $reserved_params + } + ); + } + + # Generate prefetches for embedded stuff + $c->dbic_merge_prefetch( + { + attributes => $attributes, + result_set => $orders_rs + } + ); + + # Call the to_model function by reference, if defined + if ( defined $filtered_params ) { + + # Apply the mapping function to the passed params + $filtered_params = $orders_rs->attributes_from_api($filtered_params); + $filtered_params = $c->build_query_params( $filtered_params, $reserved_params ); + } + + if ( defined $path_params ) { + + # Apply the mapping function to the passed params + $filtered_params //= {}; + $path_params = $orders_rs->attributes_from_api($path_params); + foreach my $param (keys %{$path_params}) { + $filtered_params->{$param} = $path_params->{$param}; + } + } + + if ( defined $reserved_params->{q} || defined $reserved_params->{query} || defined $reserved_params->{'x-koha-query'}) { + $filtered_params //={}; + my @query_params_array; + my $query_params; + if ( exists $reserved_params->{query} and defined $reserved_params->{query} ) { + push @query_params_array, fix_query({ query => $reserved_params->{query} }); + } + if ( exists $reserved_params->{q} and defined $reserved_params->{q}) { + push @query_params_array, fix_query({ query => decode_json($reserved_params->{q}) }); + } + if ( exists $reserved_params->{'x-koha-query'} and defined $reserved_params->{'x-koha-query'} ) { + push @query_params_array, fix_query({ query => decode_json($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, $orders_rs ); + } + + # Perform search + my $orders = $orders_rs->search( $filtered_params, $attributes ); + + if ($orders->is_paged) { + $c->add_pagination_headers({ + total => $orders->pager->total_entries, + params => $args, + }); + } + else { + $c->add_pagination_headers({ + total => $orders->count, + params => $args, + }); + } return $c->render( status => 200, - openapi => $orders + openapi => $orders->to_api({ embed => $embed }) ); } catch { @@ -185,4 +296,52 @@ sub delete { }; } +=head2 Internal methods + +=head3 fix_query + + my $query = fix_query($query); + +This method takes care of recursively fixing queries that should be done +against biblioitems (instead if biblio as exposed on the API) + +=cut + +sub fix_query { + my ($args) = @_; + + my $query = $args->{query}; + my $biblioitem_fields = { + 'biblio.isbn' => 'biblio.biblioitem.isbn', + 'biblio.ean' => 'biblio.biblioitem.ean' + }; + + if ( ref($query) eq 'HASH' ) { + foreach my $key (keys %{$query}) { + if ( exists $biblioitem_fields->{$key}) { + my $subq = delete $query->{$key}; + $query->{$biblioitem_fields->{$key}} = (ref($subq) eq 'HASH') + ? fix_query({ query => $subq }) + : $subq; + } + else { + $query->{$key} = fix_query({ query => $query->{$key} }); + } + } + } + elsif ( ref($query) eq 'ARRAY' ) { + my @accum; + foreach my $item (@{$query}) { + push @accum, fix_query({ query => $item }); + } + $query = \@accum; + } + else { # scalar + $query = $biblioitem_fields->{$query} + if exists $biblioitem_fields->{$query}; + } + + return $query; +} + 1; diff --git a/api/v1/swagger/definitions/order.json b/api/v1/swagger/definitions/order.json index f3d07d3..482decb 100644 --- a/api/v1/swagger/definitions/order.json +++ b/api/v1/swagger/definitions/order.json @@ -13,7 +13,10 @@ "description": "Identifier for the linked bibliographic record" }, "created_by": { - "type": "integer", + "type": [ + "integer", + "null" + ], "description": "Interal patron identifier of the order line creator" }, "entry_date": { @@ -302,6 +305,10 @@ "null" ] }, + "current_item_level_holds_count": { + "type": "integer", + "description": "Current holds count for associated items" + }, "fund": { "type": [ "object", diff --git a/api/v1/swagger/paths/acquisitions_orders.json b/api/v1/swagger/paths/acquisitions_orders.json index b9e0020..3812f2c 100644 --- a/api/v1/swagger/paths/acquisitions_orders.json +++ b/api/v1/swagger/paths/acquisitions_orders.json @@ -40,6 +40,13 @@ "type": "string" }, { + "name": "only_active", + "in": "query", + "description": "If only active orders should be listed", + "required": false, + "type": "boolean" + }, + { "$ref": "../parameters.json#/match" }, { @@ -50,6 +57,15 @@ }, { "$ref": "../parameters.json#/per_page" + }, + { + "$ref": "../parameters.json#/q_param" + }, + { + "$ref": "../parameters.json#/q_body" + }, + { + "$ref": "../parameters.json#/q_header" } ], "responses": { @@ -100,11 +116,18 @@ }, "x-koha-embed": [ "basket", + "basket.basket_group", + "basket.creator", + "biblio", + "biblio.active_orders+count", + "biblio.holds+count", + "biblio.items+count", + "biblio.suggestions.suggester", "fund", + "current_item_level_holds+count", "invoice", - "subscription", "items", - "biblio" + "subscription" ] }, "post": { @@ -240,11 +263,18 @@ }, "x-koha-embed": [ "basket", + "basket.basket_group", + "basket.creator", + "biblio", + "biblio.active_orders+count", + "biblio.holds+count", + "biblio.items+count", + "biblio.suggestions.suggester", "fund", + "current_item_level_holds+count", "invoice", - "subscription", "items", - "biblio" + "subscription" ] }, "put": { -- 2.1.4