From 26b6b10d34153c3fac8537706789f9873ff1f289 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Mon, 3 May 2021 11:42:47 -0300
Subject: [PATCH] Bug 28273: Multi-hold should not offer invalid pickup
 locations

This patch makes the multi-hold page offer only valid pickup locations
for the selected biblios. Prior to this, all system-wide pickup
locations were offered.

To test:
1. Set 'Hold pickup library match' to 'Item's home branch' so we put a
   constraint on the valid pickup locations for easier testing.
2. Choose two or more biblios from a search, which contain  in total 2
   or 3 item home branches.
3. Click 'Place hold'
4. Choose a patron
=> FAIL: The dropdown offers all system's pickup locations
5. Apply this patches
6. Reload the page
=> SUCCESS: Only valid pickup locations are offered
7. Sign off :-D

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 .../prog/en/modules/reserve/request.tt        | 44 +++++++++++++++++--
 reserve/placerequest.pl                       |  7 +--
 reserve/request.pl                            | 14 ++++++
 3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
index 27218671ff..27861b5093 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -1,6 +1,7 @@
 [% USE raw %]
 [% USE To %]
 [% USE Asset %]
+[% USE JSON.Escape %]
 [% USE Koha %]
 [% USE KohaDates %]
 [% USE Branches %]
@@ -414,6 +415,7 @@
 
                         [% IF ( multi_hold ) %]
                             <input type="hidden" name="biblionumbers" id="multi_hold_bibs" value="[% biblionumbers | html %]"/>
+                            <input type="hidden" name="multi_holds" id="multi_holds" value="1" />
                             <input type="hidden" name="bad_bibs" id="bad_bibs" value=""/>
                             <input type="hidden" name="request" value="any"/>
                             [% FOREACH biblioloo IN biblioloop %]
@@ -454,7 +456,10 @@
                                     [% PROCESS options_for_libraries libraries => Branches.pickup_locations({ search_params => { biblio => biblionumber, patron => patron }, selected => pickup }) %]
                             [% ELSE %]
                                 <select name="pickup" id="pickup_multi" data-patron-id="[% patron.borrowernumber | html %]">
-                                    [% PROCESS options_for_libraries libraries => Branches.all({ selected => pickup, search_params => { pickup_location => 1 } }) %]
+                                    <option value="" selected="selected"></option>
+                                [% FOREACH pickup_location IN multi_pickup_locations %]
+                                    <option value="[% pickup_location.branchcode | html %]">[% pickup_location.branchname | html %]</option>
+                                [% END %]
                             [% END %]
                                 </select>
                             </li>
@@ -753,6 +758,7 @@
                                     [% END %]
                                     <th>Priority</th>
                                     <th>Information</th>
+                                    <th>Pickup location</th>
                                 </tr>
                                 [% FOREACH biblioloo IN biblioloop %]
                                     [% IF ( biblioloo.warn ) %]
@@ -816,6 +822,18 @@
                                                 [% END %]
                                             [% END %]
                                         </td>
+                                        <td>
+                                            <select name="pickup_[% biblioloo.biblionumber | html %]"
+                                                    class="multi_pickup_select"
+                                                    data-biblio-id="[% biblioloo.biblionumber | html %]"
+                                                    data-patron-id="[% patron.borrowernumber | html %]"
+                                                    data-pickup-locations='[% biblioloo.pickup_locations_codes.json | $raw %]'>
+                                                <option value=""></option>
+                                            [% FOREACH pickup_location IN biblioloo.pickup_locations %]
+                                                <option value="[% pickup_location.branchcode | html %]">[% pickup_location.branchname | html %]</option>
+                                            [% END %]
+                                            </select>
+                                        </td>
                                     </tr>
                                 [% END # /FOREACH biblioloo %]
                             </table> <!-- /#requesttitles -->
@@ -1246,10 +1264,30 @@
                     templateResult: display_pickup_location
                 });
             });
+
             $("#pickup_multi").select2({
-                width: 'style',
-                allowClear: false
+                width: '30%',
+                allowClear: true
+            });
+
+            $('.multi_pickup_select').select2({
+                width: '100%',
+                allowClear: true
             });
+
+            $("#pickup_multi").on("change", function() {
+                var selection = $(this).val();
+                if ( selection != '' ) {
+                    $(".multi_pickup_select").each(function() {
+                        var valid_pickup_locations = $(this).data('pickup-locations');
+                        if ( valid_pickup_locations.includes(selection) ) {
+                            $(this).val(selection);
+                            $(this).trigger("change");
+                        }
+                    });
+                }
+            });
+
             $("#pickup").each( function () {
                 var this_dropdown = $(this);
                 var patron_id = $(this).data('patron-id');
diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl
index ea81592ef0..05b5c7f3c9 100755
--- a/reserve/placerequest.pl
+++ b/reserve/placerequest.pl
@@ -67,8 +67,9 @@ my %bibinfos = ();
 my @biblionumbers = split '/', $biblionumbers;
 foreach my $bibnum (@biblionumbers) {
     my %bibinfo = ();
-    $bibinfo{title} = $input->param("title_$bibnum");
-    $bibinfo{rank} = $input->param("rank_$bibnum");
+    $bibinfo{title}  = $input->param("title_$bibnum");
+    $bibinfo{rank}   = $input->param("rank_$bibnum");
+    $bibinfo{pickup} = $input->param("pickup_$bibnum");
     $bibinfos{$bibnum} = \%bibinfo;
 }
 
@@ -126,7 +127,7 @@ if ( $type eq 'str8' && $borrower ) {
             if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) {
                 AddReserve(
                     {
-                        branchcode       => $branch,
+                        branchcode       => $bibinfo->{pickup},
                         borrowernumber   => $borrower->{'borrowernumber'},
                         biblionumber     => $biblionumber,
                         priority         => $bibinfo->{rank},
diff --git a/reserve/request.pl b/reserve/request.pl
index 9c83b617cf..7637692c99 100755
--- a/reserve/request.pl
+++ b/reserve/request.pl
@@ -286,6 +286,13 @@ $template->param(
 # FIXME launch another time GetMember perhaps until (Joubu: Why?)
 my $patron = Koha::Patrons->find( $borrowernumber_hold );
 
+if ( $patron && $multi_hold ) {
+    my @multi_pickup_locations =
+      Koha::Biblios->search( { biblionumber => \@biblionumbers } )
+      ->pickup_locations( { patron => $patron } );
+    $template->param( multi_pickup_locations => \@multi_pickup_locations );
+}
+
 my $logged_in_patron = Koha::Patrons->find( $borrowernumber );
 
 my $wants_check;
@@ -736,6 +743,13 @@ foreach my $biblionumber (@biblionumbers) {
         $template->param( reserveloop => \@reserveloop );
     }
 
+    if ( $patron ) {
+        # Add the valid pickup locations
+        my @pickup_locations = $biblio->pickup_locations({ patron => $patron });
+        $biblioloopiter{pickup_locations} = \@pickup_locations;
+        $biblioloopiter{pickup_locations_codes} = [ map { $_->branchcode } @pickup_locations ];
+    }
+
     push @biblioloop, \%biblioloopiter;
 }
 
-- 
2.39.5