Bugzilla – Attachment 135906 Details for
Bug 30742
Confusion when placing hold on record with no items available because of not for loan
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[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
Bug-30742-2111x-Prevent-placing-holds-on-itemsreco.patch (text/plain), 16.45 KB, created by
Nick Clemens (kidclamp)
on 2022-06-09 15:30:18 UTC
(
hide
)
Description:
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
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-06-09 15:30:18 UTC
Size:
16.45 KB
patch
obsolete
>From c0d84b0b87aea330d9de005a786d1a7ff2a0f390 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 ) %] > <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"/> > [% FOREACH biblioloo IN biblioloop %] >- <input type="hidden" name="title_[% biblioloo.biblionumber | html %]" value="[% biblioloo.title | html %]"/> >- <input type="hidden" name="rank_[% biblioloo.biblionumber | html %]" value="[% biblioloo.rank | html %]"/> >+ [% UNLESS biblioloo.none_avail %] >+ <input type="hidden" name="holdable_bibs" id="holdable_bibs" value="[% biblioloo.biblionumber | html %]"/> >+ <input type="hidden" name="title_[% biblioloo.biblionumber | html %]" value="[% biblioloo.title | html %]"/> >+ <input type="hidden" name="rank_[% biblioloo.biblionumber | html %]" value="[% biblioloo.rank | html %]"/> >+ [% END %] > [% END %] > [% ELSE %] > <input type="hidden" name="biblionumber" value="[% biblionumber | html %]" /> >+ <input type="hidden" name="holdable_bibs" id="holdable_bibs" value="[% biblio.biblionumber | html %]"/> > <input type="hidden" name="title" value="[% biblio.title | html %]" /> > <input type="hidden" name="rank-request" value="[% fixedRank | html %]" /> > [% END # /IF multi_hold %] >@@ -403,6 +406,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> >@@ -450,14 +455,17 @@ > [% 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 %] >- <input type="hidden" name="title_[% biblioloo.biblionumber | html %]" value="[% biblioloo.title | html %]"/> >- <input type="hidden" name="rank_[% biblioloo.biblionumber | html %]" value="[% biblioloo.rank | html %]"/> >+ [% UNLESS biblioloo.none_avail %] >+ <input type="hidden" name="holdable_bibs" id="holdable_bibs" value="[% biblioloo.biblionumber | html %]"/> >+ <input type="hidden" name="title_[% biblioloo.biblionumber | html %]" value="[% biblioloo.title | html %]"/> >+ <input type="hidden" name="rank_[% biblioloo.biblionumber | html %]" value="[% biblioloo.rank | html %]"/> >+ [% END %] > [% END %] > [% ELSE %] > <input type="hidden" name="biblionumber" value="[% biblionumber | html %]" /> >+ <input type="hidden" name="holdable_bibs" id="holdable_bibs" value="[% biblio.biblionumber | html %]"/> > <input type="hidden" name="title" value="[% biblio.title | html %]" /> > <input type="hidden" name="rank-request" value="[% fixedRank | html %]" /> > [% 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 ) %] > <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> > <td> >@@ -886,13 +898,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 %] >@@ -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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=135906">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30742
:
134982
|
134983
|
135002
|
135003
|
135255
|
135256
| 135906