From 2f0aaff7136db9b596a39a6408378a18b38151e1 Mon Sep 17 00:00:00 2001
From: Agustin Moyano <agustinmoyano@theke.io>
Date: Sun, 12 Jan 2020 01:11:52 -0300
Subject: [PATCH] Bug 24350: (follow-up) Clear
 Koha::Libraries->pickup_locations() and move logic to
 Koha::Template::Plugin::Branches->pickup_locations().

---
 Koha/Libraries.pm                             | 30 ++++++++++++-------
 Koha/Template/Plugin/Branches.pm              | 29 ++++++++++++++----
 .../prog/en/includes/holds_table.inc          |  2 +-
 3 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/Koha/Libraries.pm b/Koha/Libraries.pm
index c3464ee95b..13f377939d 100644
--- a/Koha/Libraries.pm
+++ b/Koha/Libraries.pm
@@ -67,17 +67,12 @@ sub pickup_locations {
 
     my $item = $params->{'item'};
     my $biblio = $params->{'biblio'};
-    my $patron = $params->{'patron'};
-
     if ($biblio && $item) {
         Koha::Exceptions::BadParameter->throw(
             error => "Koha::Libraries->pickup_locations takes either 'biblio' or "
             ." 'item' as parameter, but not both."
         );
     }
-    unless (! defined $patron || ref($patron) eq 'Koha::Patron') {
-        $patron = Koha::Patrons->find($patron);
-    }
 
     # Select libraries that are configured as pickup locations
     my $libraries = $self->search({
@@ -86,20 +81,33 @@ sub pickup_locations {
         order_by => ['branchname']
     });
 
-    return $libraries unless $item or $biblio;
-    if($item) {
+    return $libraries->unblessed unless $item or $biblio;
+    return $libraries->unblessed
+        unless C4::Context->preference('UseBranchTransferLimits');
+    my $limittype = C4::Context->preference('BranchTransferLimitsType');
+
+    if ($item) {
         unless (ref($item) eq 'Koha::Item') {
             $item = Koha::Items->find($item);
-            return $libraries unless $item;
+            return $libraries->unblessed unless $item;
         }
-        return $item->pickup_locations( {patron => $patron} );
     } else {
         unless (ref($biblio) eq 'Koha::Biblio') {
             $biblio = Koha::Biblios->find($biblio);
-            return $libraries unless $biblio;
+            return $libraries->unblessed unless $biblio;
         }
-        return $biblio->pickup_locations( {patron => $patron} );
     }
+
+    my @pickup_locations;
+    foreach my $library ($libraries->as_list) {
+        if ($item && $item->can_be_transferred({ to => $library })) {
+            push @pickup_locations, $library->unblessed;
+        } elsif ($biblio && $biblio->can_be_transferred({ to => $library })) {
+            push @pickup_locations, $library->unblessed;
+        }
+    }
+
+    return wantarray ? @pickup_locations : \@pickup_locations;
 }
 
 =head3 search_filtered
diff --git a/Koha/Template/Plugin/Branches.pm b/Koha/Template/Plugin/Branches.pm
index 6ee4254402..08dbcb5c1a 100644
--- a/Koha/Template/Plugin/Branches.pm
+++ b/Koha/Template/Plugin/Branches.pm
@@ -87,13 +87,32 @@ sub InIndependentBranchesMode {
 
 sub pickup_locations {
     my ( $self, $params ) = @_;
-
     my $search_params = $params->{search_params} || {};
     my $selected      = $params->{selected};
-    my $libraries     = Koha::Libraries->pickup_locations($search_params);
+    my @libraries;
+
+    if(defined $search_params->{item} || defined $search_params->{biblio}) {
+        my $item = $search_params->{'item'};
+        my $biblio = $search_params->{'biblio'};
+        my $patron = $search_params->{'patron'};
+
+        unless (! defined $patron || ref($patron) eq 'Koha::Patron') {
+            $patron = Koha::Patrons->find($patron);
+        }
+
+        if($item) {
+            $item = Koha::Items->find($item) unless ref($item) eq 'Koha::Item';
+            @libraries = map { $_->unblessed } $item->pickup_locations( {patron => $patron} ) if defined $item;
+        } elsif($biblio) {
+            $biblio = Koha::Biblios->find($biblio) unless ref($biblio) eq 'Koha::Biblio';
+            @libraries = map { $_->unblessed } $biblio->pickup_locations( {patron => $patron} ) if defined $biblio;
+        }
+
+    }
 
-    for my $l (@$libraries) {
-        $l = $l->unblessed;
+    @libraries = Koha::Libraries->pickup_locations() unless @libraries;
+
+    for my $l (@libraries) {
         if ( defined $selected and $l->{branchcode} eq $selected
             or not defined $selected
             and C4::Context->userenv
@@ -103,7 +122,7 @@ sub pickup_locations {
         }
     }
 
-    return $libraries;
+    return \@libraries;
 }
 
 1;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
index 6e66ffe06c..bb3cf7ce30 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
@@ -131,7 +131,7 @@
                         [% Branches.GetName(hold.branchcode) | html %] <input type="hidden" name="pickup" value="[% hold.branchcode | html %]" />
                     [% ELSE %]
                         <select name="pickup">
-                            [% PROCESS options_for_libraries libraries => Branches.pickup_locations( { search_params => { item => hold.itemnumber, patron => hold.patron }, selected => hold.branchcode }) %]
+                            [% PROCESS options_for_libraries libraries => Branches.pickup_locations( { search_params => { item => hold.itemnumber, biblio => hold.biblionumber, patron => hold.patron }, selected => hold.branchcode }) %]
                         </select>
                     [% END %]
                 [% END %]
-- 
2.17.1