Attachment 96645 Details for Bug 24228 – Bug 24228: Make ->to_api params a hashref

[patch] Bug 24228: Make ->to_api params a hashref

Bug-24228-Make--toapi-params-a-hashref.patch (text/plain), 9.54 KB, created by Agustín Moyano on 2019-12-26 21:22:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Agustín Moyano

Created: 2019-12-26 21:22:59 UTC

Size: 9.54 KB

patch

obsolete

>From dd502dd098fbf784133ddfc57e9b58f4623d5bc4 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Mon, 23 Dec 2019 12:03:12 -0300
>Subject: [PATCH] Bug 24228: Make ->to_api params a hashref
>
>This patch makes the parameters for Koha::Object(s)->to_api a hashref
>preparing the ground for new parameters. The proposed syntax is:
>
>$object->to_api(
>    {
>        embed => {
>            'items' => {
>                'children' => {
>                    'homebranch' => {}
>                }
>            }
>        }
>    }
>);
>
>Tests are added for the Koha::Objects implementation and tests for
>Koha::Object are adjusted to the new syntax, and a test for the single
>result accessor is added as well.
>
>To test:
>1. Apply this patches
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/Koha/Object.t t/db_dependent/Koha/Objects.t
>=> SUCCESS: Tests pass!
>3. Sign off :-D
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
>---
> Koha/Object.pm                | 37 +++++++++++++++-----
> Koha/Objects.pm               |  4 +--
> t/db_dependent/Koha/Object.t  | 47 ++++++++++++-------------
> t/db_dependent/Koha/Objects.t | 65 ++++++++++++++++++++++++++++++++++-
> 4 files changed, 118 insertions(+), 35 deletions(-)
>
>diff --git a/Koha/Object.pm b/Koha/Object.pm
>index 553e53d28b..c55016a800 100644
>--- a/Koha/Object.pm
>+++ b/Koha/Object.pm
>@@ -373,16 +373,37 @@ sub _numeric_column_type {
> 
> =head3 to_api
> 
>-    my $object_for_api = $object->to_api;
>+    my $object_for_api = $object->to_api(
>+        {
>+          [ embed => {
>+                items => {
>+                    children => {
>+                        holds => {,
>+                            children => {
>+                              ...
>+                            }
>+                        }
>+                    }
>+                },
>+                library => {
>+                    ...
>+                }
>+            },
>+            ...
>+         ]
>+        }
>+    );
> 
> Returns a representation of the object, suitable for API output.
> 
> =cut
> 
> sub to_api {
>-    my ( $self, $embeds ) = @_;
>+    my ( $self, $params ) = @_;
>     my $json_object = $self->TO_JSON;
> 
>+    my $embeds = $params->{embed};
>+
>     # Rename attributes if there's a mapping
>     if ( $self->can('to_api_mapping') ) {
>         foreach my $column ( keys %{ $self->to_api_mapping } ) {
>@@ -403,18 +424,16 @@ sub to_api {
>     }
> 
>     if ($embeds) {
>-        foreach my $embed (@$embeds) {
>-            my ( $curr, $next ) = split /\s*\.\s*/, $embed, 2;
>-            my @nxembeds;
>-
>-            @nxembeds = ($next) if $next;
>+        foreach my $embed ( keys %{ $embeds } ) {
>+            my $curr = $embed;
>+            my $next = $embeds->{$curr}->{children};
> 
>             my $children = $self->$curr;
>             if ( ref $children eq 'ARRAY' ) {
>                 my @list;
>                 my $pos = 0;
>                 foreach my $child (@$children) {
>-                    my $res = $child->to_api( \@nxembeds );
>+                    my $res = $child->to_api({ embed => $next });
>                     $res = { $json_object->{$curr}->[$pos], $res }
>                       if defined $json_object->{$curr}
>                       && defined $json_object->{$curr}->[$pos];
>@@ -424,7 +443,7 @@ sub to_api {
>                 $json_object->{$curr} = \@list;
>             }
>             else {
>-                my $res = $children->to_api( \@nxembeds );
>+                my $res = $children->to_api({ embed => $next });
>                 $res = { $json_object->{$curr}, $res }
>                   if defined $json_object->{$curr};
>                 $json_object->{$curr} = $res;
>diff --git a/Koha/Objects.pm b/Koha/Objects.pm
>index 05b1de467d..82df97b687 100644
>--- a/Koha/Objects.pm
>+++ b/Koha/Objects.pm
>@@ -315,9 +315,9 @@ Returns a representation of the objects, suitable for API output .
> =cut
> 
> sub to_api {
>-    my ($self, $embeds) = @_;
>+    my ($self, $params) = @_;
> 
>-    return [ map { $_->to_api($embeds) } $self->as_list ];
>+    return [ map { $_->to_api($params) } $self->as_list ];
> }
> 
> =head3 Koha::Objects->_wrap
>diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t
>index af97834a7d..cacaeb1cbe 100755
>--- a/t/db_dependent/Koha/Object.t
>+++ b/t/db_dependent/Koha/Object.t
>@@ -215,7 +215,7 @@ subtest 'TO_JSON tests' => sub {
> 
> subtest "to_api() tests" => sub {
> 
>-    plan tests => 18;
>+    plan tests => 21;
> 
>     $schema->storage->txn_begin;
> 
>@@ -262,43 +262,44 @@ subtest "to_api() tests" => sub {
>     my $illrequest = $builder->build_object({ class => 'Koha::Illrequests' });
>     is_deeply( $illrequest->to_api, $illrequest->TO_JSON, 'If no to_api_method present, return TO_JSON' );
> 
>-    my $item_class = Test::MockModule->new('Koha::Item');
>-    $item_class->mock( 'to_api_mapping',
>-        sub {
>-            return {
>-                itemnumber       => 'item_id'
>-            };
>-        }
>-    );
>-
>-    my $hold_class = Test::MockModule->new('Koha::Hold');
>-    $hold_class->mock( 'to_api_mapping',
>-        sub {
>-            return {
>-                reserve_id       => 'hold_id'
>-            };
>-        }
>-    );
>-
>     my $biblio = $builder->build_sample_biblio();
>     my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
>     my $hold = $builder->build_object({ class => 'Koha::Holds', value => { itemnumber => $item->itemnumber } });
> 
>-    my @embeds = ('items');
>+    my $embeds = { 'items' => {} };
> 
>-    my $biblio_api = $biblio->to_api(\@embeds);
>+    my $biblio_api = $biblio->to_api({ embed => $embeds });
> 
>     ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>     is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item matches');
>     ok(!exists $biblio_api->{items}->[0]->{holds}, 'No holds info should be embedded yet');
> 
>-    @embeds = ('items.holds');
>-    $biblio_api = $biblio->to_api(\@embeds);
>+    $embeds = (
>+        {
>+            'items' => {
>+                'children' => {
>+                    'holds' => {}
>+                }
>+            },
>+            'biblioitem' => {}
>+        }
>+    );
>+    $biblio_api = $biblio->to_api({ embed => $embeds });
> 
>     ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>     is($biblio_api->{items}->[0]->{item_id}, $item->itemnumber, 'Item still matches');
>     ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded');
>     is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $hold->reserve_id, 'Hold matches');
>+    is_deeply($biblio_api->{biblioitem}, $biblio->biblioitem->to_api, 'More than one root');
>+
>+    my $hold_api = $hold->to_api(
>+        {
>+            embed => { 'item' => {} }
>+        }
>+    );
>+
>+    is( ref($hold_api->{item}), 'HASH', 'Single nested object works correctly' );
>+    is( $hold_api->{item}->{item_id}, $item->itemnumber, 'Object embedded correctly' );
> 
>     $schema->storage->txn_rollback;
> };
>diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
>index 592ab40866..e5bd6aa41e 100644
>--- a/t/db_dependent/Koha/Objects.t
>+++ b/t/db_dependent/Koha/Objects.t
>@@ -283,7 +283,7 @@ subtest '->search() tests' => sub {
> 
> subtest "to_api() tests" => sub {
> 
>-    plan tests => 4;
>+    plan tests => 18;
> 
>     $schema->storage->txn_begin;
> 
>@@ -303,6 +303,69 @@ subtest "to_api() tests" => sub {
>     is_deeply( $cities_api->[0], $city_1->to_api, 'to_api returns the individual objects with ->to_api' );
>     is_deeply( $cities_api->[1], $city_2->to_api, 'to_api returns the individual objects with ->to_api' );
> 
>+    my $biblio_1 = $builder->build_sample_biblio();
>+    my $item_1   = $builder->build_sample_item({ biblionumber => $biblio_1->biblionumber });
>+    my $hold_1   = $builder->build_object(
>+        {
>+            class => 'Koha::Holds',
>+            value => { itemnumber => $item_1->itemnumber }
>+        }
>+    );
>+
>+    my $biblio_2 = $builder->build_sample_biblio();
>+    my $item_2   = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber });
>+    my $hold_2   = $builder->build_object(
>+        {
>+            class => 'Koha::Holds',
>+            value => { itemnumber => $item_2->itemnumber }
>+        }
>+    );
>+
>+    my $embed = { 'items' => {} };
>+
>+    my $i = 0;
>+    my @items = ( $item_1, $item_2 );
>+    my @holds = ( $hold_1, $hold_2 );
>+
>+    my $biblios_api = Koha::Biblios->search(
>+        {
>+            biblionumber => [ $biblio_1->biblionumber, $biblio_2->biblionumber ]
>+        }
>+    )->to_api( { embed => $embed } );
>+
>+    foreach my $biblio_api ( @{ $biblios_api } ) {
>+        ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>+        is($biblio_api->{items}->[0]->{item_id}, $items[$i]->itemnumber, 'Item matches');
>+        ok(!exists $biblio_api->{items}->[0]->{holds}, 'No holds info should be embedded yet');
>+
>+        $i++;
>+    }
>+
>+    # One more level
>+    $embed = {
>+        'items' => {
>+            children => { 'holds' => {} }
>+        }
>+    };
>+
>+    $i = 0;
>+
>+    $biblios_api = Koha::Biblios->search(
>+        {
>+            biblionumber => [ $biblio_1->biblionumber, $biblio_2->biblionumber ]
>+        }
>+    )->to_api( { embed => $embed } );
>+
>+    foreach my $biblio_api ( @{ $biblios_api } ) {
>+
>+        ok(exists $biblio_api->{items}, 'Items where embedded in biblio results');
>+        is($biblio_api->{items}->[0]->{item_id}, $items[$i]->itemnumber, 'Item still matches');
>+        ok(exists $biblio_api->{items}->[0]->{holds}, 'Holds info should be embedded');
>+        is($biblio_api->{items}->[0]->{holds}->[0]->{hold_id}, $holds[$i]->reserve_id, 'Hold matches');
>+
>+        $i++;
>+    }
>+
>     $schema->storage->txn_rollback;
> };
> 
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 24228: 96543 | 96612 | 96613 | 96615 | 96622 | 96644 | 96645 | 96646 | 96685 | 96686 | 96687 | 96954 | 96955