From 124c263eb3641e934216213afb55314c1545c8cf Mon Sep 17 00:00:00 2001
From: David Cook <dcook@prosentient.com.au>
Date: Wed, 23 Dec 2020 06:35:19 +0000
Subject: [PATCH] Bug 27301: Pass biblionumbers to opac-reserve.pl in one style
 only

This patch forces biblionumbers to be passed to opac-reserve.pl as
individual "biblionumber" parameters instead of sending a concatenated
string as a "biblionumbers" parameter.

Test plan:

_Search Results_
1. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
2. Checkbox # 1 and # 2
3. Click "Place hold"
4. Click "Confirm hold"
5. Note that holds are made
6. Cancel holds to prepare for next test...

_Cart_
1. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
2. Checkbox # 1 and # 2
3. Add to cart
4. Click on "Cart"
5. Click "Select all"
6. Click "Place hold"
7. Click "Confirm hold"
8. Note that holds are made
9. Cancel holds to prepare for next test...

_List_
1. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=test&weight_search=1
2. Checkbox # 1 and # 2
3. Add to new list
4. Click on list
5. Click "Select all"
6. Click "Place hold"
7. Click "Confirm hold"
8. Note that holds are made
9. Cancel holds to prepare for next test...

_Detail page_
1. Go to http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=29
2. Click "Place hold"
3. Click "Confirm hold"
4. Note that holds are made
5. Cancel hold (because you may as well at this point)
---
 .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt |  5 +----
 .../opac-tmpl/bootstrap/en/modules/opac-results.tt | 12 +++++-----
 .../opac-tmpl/bootstrap/en/modules/opac-shelves.tt | 11 ++++++---
 koha-tmpl/opac-tmpl/bootstrap/js/basket.js         | 15 +++++++++++--
 opac/opac-reserve.pl                               | 26 ++--------------------
 5 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
index ec4aaf5b03..545c7d96c9 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
@@ -159,7 +159,6 @@
                 <form action="/cgi-bin/koha/opac-reserve.pl" method="post" id="hold-request-form">
                     <input type="hidden" name="place_reserve" value="1"/>
                     <!-- These values are set dynamically by js -->
-                    <input type="hidden" name="biblionumbers" id="biblionumbers"/>
                     <input type="hidden" name="selecteditems" id="selections"/>
                     <div id="bigloop">
 
@@ -178,6 +177,7 @@
                                 [% END %]
                                 <p>
                                     [% IF ( bibitemloo.holdable ) %]
+                                        <input name="biblionumber" type="hidden" value="[% bibitemloo.biblionumber | html %]"/>
                                         <input class="reserve_mode" name="reserve_mode" type="hidden" value="single"/>
                                         <input class="single_bib" name="single_bib" type="hidden" value="[% bibitemloo.biblionumber | html %]"/>
                                         <span class="confirmjs_hold" title="[% bibitemloo.biblionumber | html %]" style="padding:.3em"></span>
@@ -620,7 +620,6 @@
 
         // When 'Place Hold' button is clicked
         $(".placehold").click(function(){
-            var biblionumbers = "";
             var selections = "";
 
             [% IF new_reserves_allowed %]
@@ -639,7 +638,6 @@
             var badBib = null;
             $(".confirmjs:checked").each(function() {
                 var biblioNum = $(this).val();
-                biblionumbers += biblioNum + "/";
                 selections += biblioNum + "/";
 
                 // If required hold note is empty, make it visible
@@ -677,7 +675,6 @@
             }
 
             $("#selections").val(selections);
-            $("#biblionumbers").val(biblionumbers);
 
             return true;
         });
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
index 26d3db9f8a..1909679f9c 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
@@ -590,7 +590,6 @@
 
                             <form id="hold_form" name="hold_form" method="get" action="/cgi-bin/koha/opac-reserve.pl">
                                 <!-- The value will be set by holdBiblioNums() in basket.js -->
-                                <input id="hold_form_biblios" type="hidden" name="biblionumbers" value=""/>
                             </form>
                         </div> <!-- / .searchresults -->
 
@@ -641,12 +640,12 @@
     <script>
         [% IF ( Koha.Preference( 'opacuserlogin' ) == 1 ) && ( Koha.Preference( 'RequestOnOpac' ) == 1 ) %]
             function holdMultiple() {
-                var checkedBiblioNums = ""; // Separated by "/"
+                var checkedBiblioNums = [];
                 var checkedCount = 0;
                 if(document.bookbag_form.biblionumber.length > 0) {
                     for (var i=0; i < document.bookbag_form.biblionumber.length; i++) {
                         if (document.bookbag_form.biblionumber[i].checked) {
-                            checkedBiblioNums += (document.bookbag_form.biblionumber[i].value + "/");
+                            checkedBiblioNums.push(document.bookbag_form.biblionumber[i].value);
                             checkedCount++;
                         }
                     }
@@ -660,8 +659,11 @@
             }
 
             function holdBiblioNums(numList) {
-                // numList: biblio numbers separated by "/"
-                $("#hold_form_biblios").attr("value", numList);
+                if (numList.length){
+                    numList.forEach(function(list_item){
+                        $("#hold_form").append('<input type="hidden" name="biblionumber" value="' + list_item + '"/>');
+                    });
+                }
                 $("#hold_form").submit();
             }
         [% END # /IF opacuserlogin && RequestOnOpac %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
index 4ee6813fc7..c05bfb0f2b 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
@@ -823,11 +823,16 @@ var MSG_CONFIRM_REMOVE_SHARE = _("Are you sure you want to remove this share?");
         if ($(checkedBoxes).size() == 0) {
             alert( __("No item was selected") );
         } else {
-            var bibs = "";
+            var bibs;
+            var records = [];
             $(checkedBoxes).each(function(){
-                bibs += $(this).val() + "/";
+                records.push( $(this).val() );
             });
-            document.location = "/cgi-bin/koha/opac-reserve.pl?biblionumbers="+bibs;
+            if (records.length){
+                var params = { "biblionumber": records };
+                bibs = $.param(params, true); //Use true so don't suffix params with []
+            }
+            document.location = "/cgi-bin/koha/opac-reserve.pl?"+bibs;
         }
     }
 [% END %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/basket.js b/koha-tmpl/opac-tmpl/bootstrap/js/basket.js
index caefe626aa..3272fec4ea 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/js/basket.js
+++ b/koha-tmpl/opac-tmpl/bootstrap/js/basket.js
@@ -383,9 +383,20 @@ function showLess() {
 }
 
 function holdSel() {
-    var items = document.getElementById('records').value;
+    var items;
+    var inputs = $("form#bookbag_form input:checked");
+    if (inputs.length){
+        var records = []
+        inputs.each(function(){
+            records.push( $(this).val() );
+        });
+        if (records.length){
+            var params = { "biblionumber": records };
+            items = $.param(params, true); //Use true so don't suffix params with []
+        }
+    }
     if (items) {
-        parent.opener.document.location = "/cgi-bin/koha/opac-reserve.pl?biblionumbers=" + items;
+        parent.opener.document.location = "/cgi-bin/koha/opac-reserve.pl?" + items;
         window.close();
     } else {
         alert( __p("Bibliographic record", "No item was selected") );
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index b0ed088689..a8c7663b88 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -101,31 +101,9 @@ if ( $reservefee > 0){
 
 my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } };
 
-# There are two ways of calling this script, with a single biblio num
-# or multiple biblio nums.
-my $biblionumbers = $query->param('biblionumbers');
-my $reserveMode = $query->param('reserve_mode');
-if ($reserveMode && ($reserveMode eq 'single')) {
-    my $bib = $query->param('single_bib');
-    $biblionumbers = "$bib/";
-}
-if (! $biblionumbers) {
-    $biblionumbers = $query->param('biblionumber');
-}
-
-if ((! $biblionumbers) && (! $query->param('place_reserve'))) {
-    $template->param(message=>1, no_biblionumber=>1);
-    output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 };
-    exit;
-}
-
-# Pass the numbers to the page so they can be fed back
-# when the hold is confirmed. TODO: Not necessary?
-$template->param( biblionumbers => $biblionumbers );
+my @biblionumbers = $query->multi_param('biblionumber');
 
-# Each biblio number is suffixed with '/', e.g. "1/2/3/"
-my @biblionumbers = split /\//, $biblionumbers;
-if (($#biblionumbers < 0) && (! $query->param('place_reserve'))) {
+if ( ! @biblionumbers && (! $query->param('place_reserve'))) {
     # TODO: New message?
     $template->param(message=>1, no_biblionumber=>1);
     output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 };
-- 
2.11.0