Attachment 129285 Details for Bug 29844 – Bug 29844: Remove use of wantarray from Koha::Objects

[patch] Bug 29844: Remove use of wantarray from Koha::Objects

Bug-29844-Remove-use-of-wantarray-from-KohaObjects.patch (text/plain), 6.20 KB, created by Tomás Cohen Arazi (tcohen) on 2022-01-10 20:19:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2022-01-10 20:19:59 UTC

Size: 6.20 KB

patch

obsolete

>From 0150a9fdfb18807e982e0aa078596523c02d5b85 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Mon, 10 Jan 2022 16:18:57 -0300
>Subject: [PATCH] Bug 29844: Remove use of wantarray from Koha::Objects
>
>This patch removes the use of `wantarray` from the following methods in
>the Koha::Objects class:
>- search
>- search_related
>
>In both cases, the change is trivial. And the tests get the 'list
>context' portion removed as well.
>
>To test:
>1. Apply this patch
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/Koha/Objects.t
>=> SUCCESS: Tests pass!
>3. Sign off :-D
>
>Caveat: we broke many things by removing the feature. Check follow-up
>patches as well.
>---
> Koha/Objects.pm               | 39 ++++++-------------------
> t/db_dependent/Koha/Objects.t | 54 ++++-------------------------------
> 2 files changed, 13 insertions(+), 80 deletions(-)
>
>diff --git a/Koha/Objects.pm b/Koha/Objects.pm
>index 3ee0a55742..afd6096119 100644
>--- a/Koha/Objects.pm
>+++ b/Koha/Objects.pm
>@@ -122,8 +122,6 @@ sub find_or_create {
> 
> =head3 search
> 
>-    # list context
>-    my @objects = Koha::Objects->search([$params, $attributes]);
>     # scalar context
>     my $objects = Koha::Objects->search([$params, $attributes]);
>     while (my $object = $objects->next) {
>@@ -133,31 +131,19 @@ sub find_or_create {
> This B<instantiates> the I<Koha::Objects> class, and generates a resultset
> based on the query I<$params> and I<$attributes> that are passed (like in DBIC).
> 
>-In B<list context> it returns an array of I<Koha::Object> objects.
>-In B<scalar context> it returns an iterator.
>-
> =cut
> 
> sub search {
>     my ( $self, $params, $attributes ) = @_;
> 
>-    if (wantarray) {
>-        my @dbic_rows = $self->_resultset()->search($params, $attributes);
>-
>-        return $self->_wrap(@dbic_rows);
>+    my $class = ref($self) ? ref($self) : $self;
>+    my $rs = $self->_resultset()->search($params, $attributes);
> 
>-    }
>-    else {
>-        my $class = ref($self) ? ref($self) : $self;
>-        my $rs = $self->_resultset()->search($params, $attributes);
>-
>-        return $class->_new_from_dbic($rs);
>-    }
>+    return $class->_new_from_dbic($rs);
> }
> 
> =head3 search_related
> 
>-    my @objects = Koha::Objects->search_related( $rel_name, $cond?, \%attrs? );
>     my $objects = Koha::Objects->search_related( $rel_name, $cond?, \%attrs? );
> 
> Searches the specified relationship, optionally specifying a condition and attributes for matching records.
>@@ -168,22 +154,13 @@ sub search_related {
>     my ( $self, $rel_name, @params ) = @_;
> 
>     return if !$rel_name;
>-    if (wantarray) {
>-        my @dbic_rows = $self->_resultset()->search_related($rel_name, @params);
>-        return if !@dbic_rows;
>-        my $object_class = _get_objects_class( $dbic_rows[0]->result_class );
> 
>-        eval "require $object_class";
>-        return _wrap( $object_class, @dbic_rows );
>+    my $rs = $self->_resultset()->search_related($rel_name, @params);
>+    return if !$rs;
>+    my $object_class = _get_objects_class( $rs->result_class );
> 
>-    } else {
>-        my $rs = $self->_resultset()->search_related($rel_name, @params);
>-        return if !$rs;
>-        my $object_class = _get_objects_class( $rs->result_class );
>-
>-        eval "require $object_class";
>-        return _new_from_dbic( $object_class, $rs );
>-    }
>+    eval "require $object_class";
>+    return _new_from_dbic( $object_class, $rs );
> }
> 
> =head3 delete
>diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
>index 34385dc7f4..a64f9007eb 100755
>--- a/t/db_dependent/Koha/Objects.t
>+++ b/t/db_dependent/Koha/Objects.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 24;
>+use Test::More tests => 23;
> use Test::Exception;
> use Test::MockModule;
> use Test::Warn;
>@@ -141,7 +141,9 @@ subtest 'find' => sub {
> };
> 
> subtest 'search_related' => sub {
>-    plan tests => 6;
>+
>+    plan tests => 3;
>+
>     my $builder   = t::lib::TestBuilder->new;
>     my $patron_1  = $builder->build( { source => 'Borrower' } );
>     my $patron_2  = $builder->build( { source => 'Borrower' } );
>@@ -163,26 +165,6 @@ subtest 'search_related' => sub {
>         [ $patron_1->{branchcode}, $patron_2->{branchcode} ] ),
>         'Koha::Objects->search_related should work as expected'
>     );
>-
>-    my @libraries = Koha::Patrons->search(
>-        {
>-            -or => {
>-                borrowernumber =>
>-                  [ $patron_1->{borrowernumber}, $patron_2->{borrowernumber} ]
>-            }
>-        }
>-    )->search_related('branchcode');
>-    is(
>-        ref( $libraries[0] ), 'Koha::Library',
>-        'Koha::Objects->search_related should return a list of Koha::Object-based objects'
>-    );
>-    is( scalar(@libraries), 2,
>-        'Koha::Objects->search_related should work as expected' );
>-    ok( eq_array(
>-        [ map { $_->branchcode } @libraries ],
>-        [ $patron_1->{branchcode}, $patron_2->{branchcode} ] ),
>-        'Koha::Objects->search_related should work as expected'
>-    );
> };
> 
> subtest 'single' => sub {
>@@ -211,7 +193,7 @@ subtest 'last' => sub {
> 
> subtest 'get_column' => sub {
>     plan tests => 1;
>-    my @cities = Koha::Cities->search;
>+    my @cities = Koha::Cities->search->as_list;
>     my @city_names = map { $_->city_name } @cities;
>     is_deeply( [ Koha::Cities->search->get_column('city_name') ], \@city_names, 'Koha::Objects->get_column should be allowed' );
> };
>@@ -287,32 +269,6 @@ subtest '->is_paged and ->pager tests' => sub {
>     $schema->storage->txn_rollback;
> };
> 
>-subtest '->search() tests' => sub {
>-
>-    plan tests => 12;
>-
>-    $schema->storage->txn_begin;
>-
>-    my $count = Koha::Patrons->search->count;
>-
>-    # Create 10 patrons
>-    foreach (1..10) {
>-        $builder->build_object({ class => 'Koha::Patrons' });
>-    }
>-
>-    my $patrons = Koha::Patrons->search();
>-    is( ref($patrons), 'Koha::Patrons', 'search in scalar context returns the Koha::Object-based type' );
>-    my @patrons = Koha::Patrons->search();
>-    is( scalar @patrons, $count + 10, 'search in list context returns a list of objects' );
>-    my $i = 0;
>-    foreach (1..10) {
>-        is( ref($patrons[$i]), 'Koha::Patron', 'Objects in the list have the singular type' );
>-        $i++;
>-    }
>-
>-    $schema->storage->txn_rollback;
>-};
>-
> subtest "to_api() tests" => sub {
> 
>     plan tests => 19;
>-- 
>2.32.0

Actions: View | Diff | Splinter Review

Attachments on bug 29844: 129285 | 129286 | 129287 | 129290 | 129291 | 129292 | 129293 | 129295 | 129996 | 129997 | 129998 | 129999 | 130098 | 130099 | 130100 | 130101 | 130129 | 130130 | 130131 | 130132 | 130133 | 130134 | 130135 | 130136 | 130137 | 130138 | 130139 | 130143 | 130145 | 130146 | 130481 | 130947 | 131032