Attachment 135002 Details for Bug 30742 – Bug 30742: Prevent placing holds on items/records where all items notforloan

[patch] Bug 30742: Prevent placing holds on items/records where all items notforloan

Bug-30742-Prevent-placing-holds-on-itemsrecords-wh.patch (text/plain), 11.80 KB, created by David Nind on 2022-05-14 23:07:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2022-05-14 23:07:35 UTC

Size: 11.80 KB

patch

obsolete

>From dcca3fafc4072f6dd98f945f02c39778810deceb Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Fri, 13 May 2022 14:01:48 +0000
>Subject: [PATCH] Bug 30742: Prevent placing holds on items/records where all
> items notforloan
>
>This patch does a few things:
>1 - Adds itemtype not for loan status to display
>2 - Adds a conditional to display notforlaon status as the reason a hold cannot be placed
>3 - Seperates the lower 'Place hold(s)' buttons for single and multi holds into two template sections
>4 - Handles the case where all bibs in a multi hold have no items available
>5 - Disables the button for single hlds when all items are unavailable
>
>To test:
> 1 - Find or create a record with all items of itemtype marked 'notforloan'
> 2 - Attempt to place single hold on this record from staff client
> 3 - See one disab;ed button, one enabled 'Place holds' button
> 4 - Click 'Place holds' - hold placed
> 5 - Cancel hold
> 6 - Place multiple holds with some bibs that can be held, and this one that cannot
> 7 - Notice message that 'Cannot place hold on some items'
> 8 - Click 'Place holds' - hold is generated for the notforloan bib
> 9 - Apply patch
>10 - Place single hold
>11 - Note you now see not for loan status on items
>12 - Note the red x also includes message abnout not for loan status
>13 - Note the 'Place hold' button is disabled
>14 - Attempt multi hold
>15 - Message now includes "No items available: One or more records have no items that can be held"
>16 - Click 'Place holds'
>17 - Above still places the hold - this is for a followup patch
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> .../prog/en/modules/reserve/request.tt        | 26 ++++++++++++++-----
> reserve/request.pl                            | 21 ++++++++++-----
> 2 files changed, 35 insertions(+), 12 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 59f215ebd2..f4812ad94e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -399,6 +399,8 @@
>                                         <li><strong>Too many holds for <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblioloo.biblionumber | uri %]"> [% biblioloo.title | html %]</a>: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% patron.borrowernumber | html %]">[% patron.firstname | html %] [% patron.surname | html %] </a> can only place a maximum of [% max_holds_for_record | html %] hold(s) on this record.</li>
>                                     [% END %]
>                                 [% END %]
>+                            [% ELSIF ( none_available ) %]
>+                                <li><strong>No items available: </strong>One or more records have no items that can be held</li>
>                             [% END # /IF exceeded_maxreserves %]
>                         [% END # /UNLESS multi_hold %]
>                     </div>
>@@ -663,6 +665,8 @@
>                                                                     <span>Library is not a pickup location</span>
>                                                                 [% ELSIF itemloo.not_holdable == 'no_valid_pickup_location' %]
>                                                                     <span>No valid pickup location</span>
>+                                                                [% ELSIF itemloo.not_holdable == 'notforloan' %]
>+                                                                    <span>Not for loan</span>
>                                                                 [% ELSE %]
>                                                                     <span>[% itemloo.not_holdable | html %]</span>
>                                                                 [% END %]
>@@ -769,6 +773,8 @@
> 
>                                                     [% IF ( itemloo.notforloan ) %]
>                                                        <span class="nfl">Not for loan ([% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => itemloo.notforloan ) | html %])</span>
>+                                                    [% ELSIF ( itemloo.notforloanitype ) %]
>+                                                       <span class="nfl">Not for loan (Itemtype not for loan)</span>
>                                                     [% END %]
>                                                 </td>
>                                             </tr>
>@@ -883,13 +889,21 @@
> 
>                         <fieldset class="action">
>                             [% IF ( patron AND patron.borrowernumber ) %]
>-                                [% IF ( override_required ) %]
>-                                    <button type="submit" class="btn btn-default warning"><i class="fa fa-exclamation-triangle "></i> Place holds</button>
>-                                [% ELSIF ( none_available ) %]
>-                                    <button class="btn btn-default" type="submit">Place holds</button>
>-                                [% ELSE %]
>-                                    [% IF ( multi_hold ) %]
>+                                [% IF ( multi_hold ) %]
>+                                    [% IF ( override_required ) %]
>+                                        <button type="submit" class="btn btn-default warning"><i class="fa fa-exclamation-triangle "></i> Place holds</button>
>+                                    [% ELSIF ( no_bibs_available ) %]
>+                                        <button type="submit" class="btn btn-default btn-disabled" disabled="disabled">Place holds</button>
>+                                    [% ELSIF ( none_available ) %]
>+                                        <button type="submit" class="btn btn-default">Place holds</button>
>+                                    [% ELSE %]
>                                         <button type="submit" class="btn btn-default" id="multi_hold_submit">Place holds</button>
>+                                    [% END %]
>+                                [% ELSE %]
>+                                    [% IF ( override_required ) %]
>+                                        <button type="submit" class="btn btn-default warning"><i class="fa fa-exclamation-triangle "></i> Place hold</button>
>+                                    [% ELSIF ( none_available ) %]
>+                                        <button type="submit" class="btn btn-default btn-disabled" disabled="disabled">Place hold</button>
>                                     [% ELSE %]
>                                         <button type="submit" class="btn btn-default">Place hold</button>
>                                     [% END %]
>diff --git a/reserve/request.pl b/reserve/request.pl
>index c89a89ee90..8c1f107e00 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -71,7 +71,7 @@ my $pickup = $input->param('pickup');
> my $itemtypes = {
>     map {
>         $_->itemtype =>
>-          { %{ $_->unblessed }, image_location => $_->image_location }
>+          { %{ $_->unblessed }, image_location => $_->image_location, notforloan => $_->notforloan }
>     } Koha::ItemTypes->search_with_localization->as_list
> };
> 
>@@ -294,6 +294,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>     my $itemdata_ccode = 0;
>     my @biblioloop = ();
>     my $no_reserves_allowed = 0;
>+    my $num_bibs_available = 0;
>     foreach my $biblionumber (@biblionumbers) {
>         next unless $biblionumber =~ m|^\d+$|;
> 
>@@ -382,7 +383,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
> 
>         if ( $club_hold or $borrowernumber_hold ) {
>             my @available_itemtypes;
>-            my $num_available = 0;
>+            my $num_items_available = 0;
>             my $num_override  = 0;
>             my $hiddencount   = 0;
>             my $num_alreadyheld = 0;
>@@ -462,6 +463,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>                 }
> 
>                 # If there is no loan, return and transfer, we show a checkbox.
>+                $item->{notforloanitype} = $item->{itemtype}->{notforloan};
>                 $item->{notforloan} ||= 0;
> 
>                 # if independent branches is on we need to check if the person can reserve
>@@ -486,6 +488,8 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
> 
>                     my $can_item_be_reserved = CanItemBeReserved( $patron, $item_object )->{status};
>                     $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' );
>+                    $item->{not_holdable} ||= 'notforloan' if ( $item->{notforloanitype} || $item->{notforloan} > 0 );
>+
> 
>                     $item->{item_level_holds} = Koha::CirculationRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } );
> 
>@@ -502,7 +506,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>                         my $pickup_locations = $item_object->pickup_locations({ patron => $patron });
>                         $item->{pickup_locations_count} = $pickup_locations->count;
>                         if ( $item->{pickup_locations_count} > 0 ) {
>-                            $num_available++;
>+                            $num_items_available++;
>                             $item->{available} = 1;
>                             # pass the holding branch for use as default
>                             my $default_pickup_location = $pickup_locations->search({ branchcode => $item->{holdingbranch} })->next;
>@@ -524,7 +528,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>                             $item->{pickup_locations_count} = scalar @pickup_locations;
> 
>                             if ( @pickup_locations ) {
>-                                $num_available++;
>+                                $num_items_available++;
>                                 $item->{available} = 1;
> 
>                                 my $default_pickup_location;
>@@ -543,9 +547,11 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>                         } else { $num_alreadyheld++ }
> 
>                         push( @available_itemtypes, $item->{itype} );
>+                    } else {
>+                        # If none of the conditions hold true, then neither override nor available is set and the item cannot be checked
>+                        $item->{available} = 0;
>                     }
> 
>-                    # If none of the conditions hold true, then neither override nor available is set and the item cannot be checked
> 
>                     # Show serial enumeration when needed
>                     if ($item->{enumchron}) {
>@@ -567,7 +573,7 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>             if ( $num_override > 0 && ($num_override + $num_alreadyheld) == scalar( @{ $biblioloopiter{itemloop} } ) ) {
>             # That is, if all items require an override
>                 $template->param( override_required => 1 );
>-            } elsif ( $num_available == 0 ) {
>+            } elsif ( $num_items_available == 0 ) {
>                 $template->param( none_available => 1 );
>                 $biblioloopiter{warn} = 1;
>                 $biblioloopiter{none_avail} = 1;
>@@ -675,9 +681,12 @@ if (   ( $findborrower && $borrowernumber_hold || $findclub && $club_hold )
>             $biblioloopiter{pickup_locations_codes} = [ map { $_->branchcode } @pickup_locations ];
>         }
> 
>+        $num_bibs_available++ unless $biblioloopiter{none_avail};
>         push @biblioloop, \%biblioloopiter;
>     }
> 
>+    $template->param( no_bibs_available => 1 ) unless $num_bibs_available > 0;
>+
>     $template->param( biblioloop => \@biblioloop );
>     $template->param( no_reserves_allowed => $no_reserves_allowed );
>     $template->param( exceeded_maxreserves => $exceeded_maxreserves );
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 30742: 134982 | 134983 | 135002 | 135003 | 135255 | 135256 | 135906