From c0d84b0b87aea330d9de005a786d1a7ff2a0f390 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 9 Jun 2022 15:27:56 +0000 Subject: [PATCH] Bug 30742: [21.11.x] Prevent placing holds on items/records where all items notforloan Bug 30742: Remove 'bad_bibs' and send a list of holdable bibs Bug 30892: (bug 30742 follow-up) Send single bib as a holdable bib Rebeased/rewritten patch - includes fix from 30892 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 Currently place request gets a list of bad_bibs that is created via javascript on the template. It ignores this list Ths patch instead doesn't add info for bad bibs, and provides a list of the bibs that can be held To test: 1 - Attempt multi hold with some items that can be held, and one that cannot due to notforloan 2 - Fill in pickup locations and place hold 3 - Note hold is place on bib with no avilable items and hsows twice in results 4 - Apply patch 5 - repeat with another patron 6 - Note no aidditonal hold on record with notforloan items 7 - Note with with not for loan items appears only once in results Bug 30892 To test: 1. Try placing hold 2. Everything seems to work but no hold gets placed. 3. Apply patch 4. Verify holds are no placed correctly. --- .../prog/en/modules/reserve/request.tt | 52 ++++++++++++------- reserve/placerequest.pl | 5 +- reserve/request.pl | 11 ++-- 3 files changed, 41 insertions(+), 27 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 13285130c1..b450007989 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -206,14 +206,17 @@ [% IF ( multi_hold ) %] - [% FOREACH biblioloo IN biblioloop %] - - + [% UNLESS biblioloo.none_avail %] + + + + [% END %] [% END %] [% ELSE %] + [% END # /IF multi_hold %] @@ -403,6 +406,8 @@
  • Too many holds for [% biblioloo.title | html %]: [% patron.firstname | html %] [% patron.surname | html %] can only place a maximum of [% max_holds_for_record | html %] hold(s) on this record.
    • [% END %] [% END %] + [% ELSIF ( none_available ) %] +
  • No items available: One or more records have no items that can be held
    • [% END # /IF exceeded_maxreserves %] [% END # /UNLESS multi_hold %] @@ -450,14 +455,17 @@ [% IF ( multi_hold ) %] - [% FOREACH biblioloo IN biblioloop %] - - + [% UNLESS biblioloo.none_avail %] + + + + [% END %] [% END %] [% ELSE %] + [% END # /IF multi_hold %] @@ -666,6 +674,8 @@ Library is not a pickup location [% ELSIF itemloo.not_holdable == 'no_valid_pickup_location' %] No valid pickup location + [% ELSIF itemloo.not_holdable == 'notforloan' %] + Not for loan [% ELSE %] [% itemloo.not_holdable | html %] [% END %] @@ -760,6 +770,8 @@ [% IF ( itemloo.notforloan ) %] Not for loan ([% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => itemloo.notforloan ) | html %]) + [% ELSIF ( itemloo.notforloanitype ) %] + Not for loan (Itemtype not for loan) [% END %] @@ -886,13 +898,21 @@
    [% IF ( patron AND patron.borrowernumber ) %] - [% IF ( override_required ) %] - - [% ELSIF ( none_available ) %] - - [% ELSE %] - [% IF ( multi_hold ) %] + [% IF ( multi_hold ) %] + [% IF ( override_required ) %] + + [% ELSIF ( no_bibs_available ) %] + + [% ELSIF ( none_available ) %] + + [% ELSE %] + [% END %] + [% ELSE %] + [% IF ( override_required ) %] + + [% ELSIF ( none_available ) %] + [% ELSE %] [% END %] @@ -1383,15 +1403,7 @@ return false; } - var badSpans = $(".not_holdable"); - var badBibs = ""; - $(badSpans).each(function() { - var bibnum = $(this).attr("title"); - badBibs += bibnum + "/"; - }); - $("#multi_hold_bibs").val(biblionumbers); - $("#bad_bibs").val(badBibs); $('#hold-request-form').preventDoubleFormSubmit(); diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 5ee8dce2f6..5f320fdb89 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -36,6 +36,7 @@ checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); my @bibitems = $input->multi_param('biblioitem'); my @reqbib = $input->multi_param('reqbib'); +my @holdable_bibs = $input->multi_param('holdable_bibs'); my $biblionumber = $input->param('biblionumber'); my $borrowernumber = $input->param('borrowernumber'); my $notes = $input->param('notes'); @@ -55,7 +56,6 @@ $borrower = $borrower->unblessed if $borrower; 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; my %bibinfos = (); @@ -164,9 +164,6 @@ if ( $type eq 'str8' && $borrower ) { } } - if ($bad_bibs) { - $biblionumbers .= $bad_bibs; - } print $input->redirect("request.pl?biblionumbers=$biblionumbers"); } elsif ( $borrowernumber eq '' ) { diff --git a/reserve/request.pl b/reserve/request.pl index 9e785c0552..36daf35dcd 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -320,6 +320,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+$|; @@ -443,7 +444,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) my @available_itemtypes; foreach my $biblioitemnumber (@biblioitemnumbers) { my $biblioitem = $biblioiteminfos_of->{$biblioitemnumber}; - my $num_available = 0; + my $num_items_available = 0; my $num_override = 0; my $hiddencount = 0; my $num_alreadyheld = 0; @@ -556,6 +557,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) } # If there is no loan, return and transfer, we show a checkbox. + $item->{notforloanitype} = $itemtypes->{$item->{itype}}{notforloan}; $item->{notforloan} ||= 0; # if independent branches is on we need to check if the person can reserve @@ -580,6 +582,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber )->{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 } ); @@ -597,7 +600,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; @@ -659,7 +662,7 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) if ( $num_override > 0 && ($num_override + $num_alreadyheld) == scalar( @{ $biblioitem->{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; @@ -767,9 +770,11 @@ 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