From 6f1e9fe75080b9a02c49730cbdb341cf09ca9826 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Fri, 8 May 2020 11:40:15 -0300
Subject: [PATCH] Bug 25421: Make ->pickup_locations only return an arrayref

This patch makes Koha::Item->pickup_locations and
Koha::Biblio->pickup_locations always return an arrayref.

Test are adjusted to reflect this:

1. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t
=> SUCCESS: Tests pass!
2. Apply this patch and repeat 1
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/Biblio.pm               | 11 ++++++-----
 Koha/Item.pm                 |  6 +++---
 t/db_dependent/Koha/Biblio.t |  2 +-
 t/db_dependent/Koha/Item.t   |  2 +-
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm
index 4fe30113e3..830b9c4d73 100644
--- a/Koha/Biblio.pm
+++ b/Koha/Biblio.pm
@@ -205,10 +205,11 @@ sub can_be_transferred {
 
 =head3 pickup_locations
 
-@pickup_locations = $biblio->pickup_locations( {patron => $patron } )
+    my $pickup_locations = $biblio->pickup_locations( {patron => $patron } );
 
-Returns possible pickup locations for this biblio items, according to patron's home library (if patron is defined and holds are allowed only from hold groups)
-and if item can be transferred to each pickup location.
+Returns an I<arrayref> of possible pickup locations for this biblio's items,
+according to patron's home library (if patron is defined and holds are allowed
+only from hold groups) and if item can be transferred to each pickup location.
 
 =cut
 
@@ -219,14 +220,14 @@ sub pickup_locations {
 
     my @pickup_locations;
     foreach my $item_of_bib ($self->items->as_list) {
-        push @pickup_locations, $item_of_bib->pickup_locations( {patron => $patron} );
+        push @pickup_locations, @{ $item_of_bib->pickup_locations( {patron => $patron} ) };
     }
 
     my %seen;
     @pickup_locations =
       grep { !$seen{ $_->branchcode }++ } @pickup_locations;
 
-    return wantarray ? @pickup_locations : \@pickup_locations;
+    return \@pickup_locations;
 }
 
 =head3 hidden_in_opac
diff --git a/Koha/Item.pm b/Koha/Item.pm
index 57fe8f1b20..51a7725a03 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -551,8 +551,8 @@ sub pickup_locations {
 
     my @libs;
     if(defined $patron) {
-        return @libs if $branchitemrule->{holdallowed} == 3 && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} );
-        return @libs if $branchitemrule->{holdallowed} == 1 && $self->home_branch->branchcode ne $patron->branchcode;
+        return \@libs if $branchitemrule->{holdallowed} == 3 && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} );
+        return \@libs if $branchitemrule->{holdallowed} == 1 && $self->home_branch->branchcode ne $patron->branchcode;
     }
 
     if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup') {
@@ -581,7 +581,7 @@ sub pickup_locations {
         }
     }
 
-    return wantarray ? @pickup_locations : \@pickup_locations;
+    return \@pickup_locations;
 }
 
 =head3 article_request_type
diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t
index 790b623aad..59a0895d62 100644
--- a/t/db_dependent/Koha/Biblio.t
+++ b/t/db_dependent/Koha/Biblio.t
@@ -398,7 +398,7 @@ subtest 'pickup_locations' => sub {
         my ( $cbranch, $biblio, $patron, $results ) = @_;
         t::lib::Mocks::mock_preference('ReservesControlBranch', $cbranch);
 
-        my @pl = $biblio->pickup_locations( { patron => $patron} );
+        my @pl = @{ $biblio->pickup_locations( { patron => $patron} ) };
 
         foreach my $pickup_location (@pl) {
             is( ref($pickup_location), 'Koha::Library', 'Object type is correct' );
diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t
index 581c96d776..20547d71b4 100644
--- a/t/db_dependent/Koha/Item.t
+++ b/t/db_dependent/Koha/Item.t
@@ -292,7 +292,7 @@ subtest 'pickup_locations' => sub {
                 }
             }
         );
-        my @pl = $item->pickup_locations( { patron => $patron} );
+        my @pl = @{ $item->pickup_locations( { patron => $patron} ) };
         my $ha_value=$ha==3?'holdgroup':($ha==2?'any':'homebranch');
 
         foreach my $pickup_location (@pl) {
-- 
2.39.5