Bug 16547: Do not display "multi holds" view if only one is selected
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Mon, 13 Jan 2020 09:48:11 +0000 (10:48 +0100)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Mon, 6 Apr 2020 09:41:02 +0000 (10:41 +0100)
If a hold is selected from the result list, we should let the ability to
select an item-level hold.

Test plan:
I. Detail page
1/ Go to a bibliographic record detail page
2/ Click "Place hold"
3/ Select a patron
=> No change expected, you can select an item

II. Search result, multiple holds
1/ Search for an item with more than one search result
2/ Select several items, click 'Place hold'
3/ Enter a patron card number
=> No change expected, item level holds are not available.

III. Search result, single hold
1/ Search for an item with more than one search result
2/ Select only one item, click 'Place hold'
3/ Enter a patron card number
=> With this patch applied, item level hold is available. The screen is the same
as when you place a hold from the bibliographic record detail page
=> Without this patch you cannot place an item-level hold

QA notes: We could go a bit further and remove the 2 biblionumbers and
biblionumber from hold script, as well as remove the checkMultiHold in
request.tt. We should not have a biblionumbers param that contain a list
of biblionumber separated by '/' but several biblionumber parameters
instead.

QA notes 2: About placerequest.pl, see bug 19618 comment 27.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt
reserve/modrequest.pl
reserve/placerequest.pl
reserve/request.pl

index 78ac884..c24843e 100644 (file)
                     <input id="hold_form_biblios" type="hidden" name="biblionumbers" value="" />
                     <input type="hidden" name="findborrower" id="holdFor" value="" />
                     <input type="hidden" name="club" id="holdForClub" value="" />
-                    <input type="hidden" name="multi_hold" value="1"/>
                 </form>
 
                 <form id="list_form" method="get" action="/cgi-bin/koha/reserve/request.pl">
index dfe26e2..acb9bd9 100644 (file)
                                 <input type="text" size="40" id="patron" class="focus" name="findborrower" autocomplete="off" />
                                 <input type="submit" value="Search" />
                                 [% IF multi_hold %]
-                                    <input type="hidden" name="multi_hold" value="[% multi_hold | html %]"/>
                                     <input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
                                 [% ELSE %]
                                     <input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
                                 [% END %]
 
-                                [% IF ( multi_hold ) %]
-                                    <input type="hidden" name="multi_hold" value="[% multi_hold | html %]"/>
-                                    <input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
-                                [% END %]
                             </form> <!-- /#holds_patronsearch -->
                             [% IF borrowers %]
                                 [% INCLUDE 'circ-patron-search-results.inc' destination = "holds" %]
                                 <input type="text" size="40" id="club" class="focus" name="findclub" autocomplete="off" />
                                 <input type="submit" value="Search" />
                                 [% IF multi_hold %]
-                                    <input type="hidden" name="multi_hold" value="[% multi_hold | html %]"/>
                                     <input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
                                 [% ELSE %]
                                     <input type="hidden" name="biblionumber" value="[% biblionumber | html %]" />
                                 [% END %]
 
-                                [% IF ( multi_hold ) %]
-                                    <input type="hidden" name="multi_hold" value="[% multi_hold | html %]"/>
-                                    <input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
-                                [% END %]
                             </form> <!-- /#holds_patronsearch -->
                             [% IF clubs %]
                                 [% INCLUDE 'clubs-table.inc' destination = "holds" %]
                     <form action="/api/v1/clubs/[% club.id | html %]/holds" method="post" name="form" id="club-request-form">
 
                         [% IF ( multi_hold ) %]
-                            <input type="hidden" name="multi_hold" value="[% multi_hold | html %]"/>
                             <input type="hidden" name="biblionumbers" id="multi_hold_bibs" value="[% biblionumbers | html %]"/>
                             <input type="hidden" name="bad_bibs" id="bad_bibs" value=""/>
                             <input type="hidden" name="request" value="any"/>
                         <input type="hidden" name="type" value="str8" />
 
                         [% IF ( multi_hold ) %]
-                            <input type="hidden" name="multi_hold" value="[% multi_hold | html %]"/>
                             <input type="hidden" name="biblionumbers" id="multi_hold_bibs" value="[% biblionumbers | html %]"/>
                             <input type="hidden" name="bad_bibs" id="bad_bibs" value=""/>
                             <input type="hidden" name="request" value="any"/>
index 72420dd..5beada8 100644 (file)
 <form id="hold_form" method="get" action="/cgi-bin/koha/reserve/request.pl">
     <!-- Value will be set here by placeHold() -->
     <input id="hold_form_biblios" type="hidden" name="biblionumbers" value="" />
-    <input type="hidden" name="multi_hold" value="1"/>
 </form>
 
             </main>
index 40c3902..a1f9733 100755 (executable)
@@ -49,7 +49,6 @@ my @reservedates = $query->multi_param('reservedate');
 my @expirationdates = $query->multi_param('expirationdate');
 my @branch = $query->multi_param('pickup');
 my @itemnumber = $query->multi_param('itemnumber');
-my $multi_hold = $query->param('multi_hold');
 my $biblionumbers = $query->param('biblionumbers');
 my $count=@rank;
 
@@ -94,7 +93,7 @@ if ( $from eq 'borrower'){
     print $query->redirect("/cgi-bin/koha/circ/circulation.pl?borrowernumber=$borrower[0]");
 } else {
      my $url = "/cgi-bin/koha/reserve/request.pl?";
-     if ($multi_hold) {
+     if (@biblionumber > 1) {
          $url .= "multi_hold=1&biblionumbers=$biblionumbers";
      } else {
          $url .= "biblionumber=$biblionumber[0]";
index 264ba96..88fa593 100755 (executable)
@@ -56,8 +56,9 @@ my $itemtype       = $input->param('itemtype') || undef;
 my $borrower = Koha::Patrons->find( $borrowernumber );
 $borrower = $borrower->unblessed if $borrower;
 
-my $multi_hold = $input->param('multi_hold');
-my $biblionumbers = $multi_hold ? $input->param('biblionumbers') : ($biblionumber . '/');
+my $biblionumbers = $input->param('biblionumbers');
+$biblionumbers ||= $biblionumber . '/';
+
 my $bad_bibs = $input->param('bad_bibs');
 my $holds_to_place_count = $input->param('holds_to_place_count') || 1;
 
@@ -118,7 +119,7 @@ if ( $type eq 'str8' && $borrower ) {
 
             }
 
-        } elsif ($multi_hold) {
+        } elsif (@biblionumbers > 1) {
             my $bibinfo = $bibinfos{$biblionumber};
             if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) {
                 AddReserve(
@@ -161,15 +162,10 @@ if ( $type eq 'str8' && $borrower ) {
         }
     }
 
-    if ($multi_hold) {
-        if ($bad_bibs) {
-            $biblionumbers .= $bad_bibs;
-        }
-        print $input->redirect("request.pl?biblionumbers=$biblionumbers&multi_hold=1");
-    }
-    else {
-        print $input->redirect("request.pl?biblionumber=$biblionumber");
+    if ($bad_bibs) {
+        $biblionumbers .= $bad_bibs;
     }
+    print $input->redirect("request.pl?biblionumber=$biblionumber");
 }
 elsif ( $borrowernumber eq '' ) {
     print $input->header();
index 23bd88e..61f853d 100755 (executable)
@@ -168,9 +168,8 @@ if ( $biblionumbers ) {
     push @biblionumbers, $input->multi_param('biblionumber');
 }
 
-my $multihold = scalar $input->param('multi_hold');
-# FIXME multi_hold should not be a variable but depends on the number of elements in @biblionumbers
-$template->param(multi_hold => scalar $input->param('multi_hold'));
+my $multi_hold = @biblionumbers > 1;
+$template->param(multi_hold => $multi_hold);
 
 # If we have the borrowernumber because we've performed an action, then we
 # don't want to try to place another reserve.
@@ -461,7 +460,7 @@ foreach my $biblionumber (@biblionumbers) {
                 $do_check = $patron->do_check_for_previous_checkout($item) if $wants_check;
                 if ( $do_check && $wants_check ) {
                     $item->{checked_previously} = $do_check;
-                    if ( $multihold ) {
+                    if ( $multi_hold ) {
                         $biblioloopiter{checked_previously} = $do_check;
                     } else {
                         $template->param( checked_previously => $do_check );