Attachment 109861 Details for Bug 20212 – Bug 20212: Add more embeddable objects to orders

[patch] Bug 20212: Add more embeddable objects to orders

Bug-20212-Add-more-embeddable-objects-to-orders.patch (text/plain), 11.31 KB, created by Tomás Cohen Arazi (tcohen) on 2020-09-10 18:58:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-09-10 18:58:25 UTC

Size: 11.31 KB

patch

obsolete

>From cf322d52aa176aa5bb49003c6a227c3366aee5b8 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 17 Jan 2020 13:05:24 -0300
>Subject: [PATCH] Bug 20212: Add more embeddable objects to orders
>
>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.<biblioitem
>field> 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.
>---
> 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 a9ed5123f9..3fb9b28b22 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 f3d07d3735..482decb1f1 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 b9e00200a0..3812f2cf66 100644
>--- a/api/v1/swagger/paths/acquisitions_orders.json
>+++ b/api/v1/swagger/paths/acquisitions_orders.json
>@@ -39,6 +39,13 @@
>                     "required": false,
>                     "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.28.0

Actions: View | Diff | Splinter Review

Attachments on bug 20212: 97545 | 97546 | 97547 | 97629 | 97630 | 97633 | 97634 | 97635 | 97745 | 98226 | 98230 | 98241 | 99724 | 99725 | 99727 | 100142 | 100475 | 100476 | 100477 | 100478 | 100552 | 100553 | 100554 | 100555 | 100600 | 100601 | 100602 | 100603 | 100605 | 100606 | 100643 | 101320 | 103684 | 103685 | 103686 | 103687 | 103688 | 109861 | 109862 | 109863 | 109864 | 109866 | 109867 | 109868 | 109869 | 109870 | 109936 | 110229 | 110234 | 110478 | 110482 | 111568 | 111569 | 112310 | 112521 | 112522 | 112523 | 112524 | 112686 | 112695 | 112698 | 112699 | 112700 | 112702 | 112703 | 112704 | 112705 | 112706 | 112868 | 112882 | 112987 | 113278 | 113279 | 114607 | 114608 | 114609 | 114610 | 114611 | 114612 | 114613 | 114614 | 114615 | 114616 | 114617 | 114942 | 114953 | 114954 | 114956 | 114959 | 114963 | 115050 | 115056 | 115057 | 115060 | 115062 | 115107 | 115108 | 115109 | 115817 | 115818 | 115819 | 115820 | 115821 | 115822 | 115886 | 115887 | 115888 | 115889 | 115890 | 115891 | 115892 | 115893 | 115894 | 115895 | 115896 | 115897 | 115898 | 115899 | 115900 | 115901 | 115902 | 115903 | 115904 | 115905 | 115906 | 115920 | 115921 | 115940 | 115941 | 115943 | 115944 | 115945 | 115946 | 115947 | 115948 | 115949 | 115950 | 115951 | 115952 | 115953 | 115954 | 115955 | 115956 | 115957 | 115958 | 115959 | 115960 | 115961 | 115962 | 115963 | 115964 | 115965 | 116139