From dae3b8a52ffee6031e75c7823511c2647b82e6a7 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.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 Koha/Objects.pm               | 41 ++++++--------------------
 t/db_dependent/Koha/Objects.t | 54 ++++-------------------------------
 2 files changed, 14 insertions(+), 81 deletions(-)

diff --git a/Koha/Objects.pm b/Koha/Objects.pm
index 3ee0a55742..133d0cb08e 100644
--- a/Koha/Objects.pm
+++ b/Koha/Objects.pm
@@ -34,7 +34,7 @@ Koha::Objects - Koha Object set base class
 =head1 SYNOPSIS
 
     use Koha::Objects;
-    my @objects = Koha::Objects->search({ borrowernumber => $borrowernumber});
+    my $objects = Koha::Objects->search({ borrowernumber => $borrowernumber});
 
 =head1 DESCRIPTION
 
@@ -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