Bugzilla – Attachment 148896 Details for
Bug 33302
Placing item level holds in OPAC allows to pick forbidden pick-up locations, but then places no hold
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC
Bug-33302-Send-and-display-errors-when-a-hold-cann.patch (text/plain), 7.74 KB, created by
Biblibre Sandboxes
on 2023-03-29 10:02:17 UTC
(
hide
)
Description:
Bug 33302: Send and display errors when a hold cannot be placed on the OPAC
Filename:
MIME Type:
Creator:
Biblibre Sandboxes
Created:
2023-03-29 10:02:17 UTC
Size:
7.74 KB
patch
obsolete
>From 213dc1cbc813d127549b5623eea25ec36afcb6cd Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 28 Mar 2023 11:31:42 +0000 >Subject: [PATCH] Bug 33302: Send and display errors when a hold cannot be > placed on the OPAC > >This patch changes opac-reserve.pl to return the error(s) when placing >a hold as a pipe delimited list which is then translated to a message for the >user > >To test: >1 - Find a record with items available on the opac >2 - Click 'place hold' and set things up, but do not confirm >3 - In staff client, do something to make hold invalid: > - Make item damaged > - Make library not a pickup location > - Place other holds for patron up to limit > - etc. >4 - Confirm hold on OPAC >5 - You are sent to patron's account, hold is not placed >6 - There is little or no message to explain why >7 - Apply patch >8 - Repeat >9 - Now errors are clear > >Signed-off-by: Andrew Auld <andrew.auld@ptfs-europe.com> >--- > .../bootstrap/en/modules/opac-user.tt | 46 ++++++++++++++++++- > opac/opac-reserve.pl | 31 +++++++++---- > 2 files changed, 68 insertions(+), 9 deletions(-) > >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >index 37d36399ea..0c77bb27f6 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt >@@ -67,7 +67,51 @@ > [% IF failed_holds %] > <div class="alert alert-info"> > <h2>Notice:</h2> >- <p>One or more holds were not placed due to existing holds.</p> >+ <p>One or more holds were not placed due to following errors:</p> >+ <ul> >+ [% FOREACH fail IN failed_holds.split('\|') %] >+ <li> >+ [% SWITCH fail %] >+ [% CASE 'damaged' %] >+ <span>Item damaged</span> >+ [% CASE 'ageRestricted' %] >+ <span>Age restricted</span> >+ [% CASE 'tooManyHoldsForThisRecord' %] >+ <span>Exceeded max holds per record</span> >+ [% CASE 'tooManyReservesToday' %] >+ <span>Daily hold limit reached for patron</span> >+ [% CASE 'tooManyReserves' %] >+ <span>Too many holds</span> >+ [% CASE 'notReservable' %] >+ <span>Not holdable</span> >+ [% CASE 'cannotReserveFromOtherBranches' %] >+ <span>Patron is from different library</span> >+ [% CASE 'branchNotInHoldGroup' %] >+ <span>Cannot place hold from patron's library</span> >+ [% CASE 'itemAlreadyOnHold' %] >+ <span>Patron already has hold for this item</span> >+ [% CASE 'cannotBeTransferred' %] >+ <span>Cannot be transferred to pickup library</span> >+ [% CASE 'pickupNotInHoldGroup' %] >+ <span>Only pickup locations within the same hold group are allowed</span> >+ [% CASE 'noReservesAllowed' %] >+ <span>No holds are allowed on this item</span> >+ [% CASE 'libraryNotPickupLocation' %] >+ <span>Library is not a pickup location</span> >+ [% CASE 'no_valid_pickup_location' %] >+ <span>No valid pickup location</span> >+ [% CASE 'notforloan' %] >+ <span>Not for loan</span> >+ [% CASE 'items_available' %] >+ <span>There are items available in the library, please visit to check them out</span> >+ [% CASE 'not_placed' %] >+ <span>Error when placing hold, please report this to the library</span> >+ [% CASE %] >+ <span>Error: [% fail | html %]</span> >+ [% END %] >+ </li> >+ [% END %] >+ </ul> > </div> > [% END %] > >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 07440efa6a..0778ac5d04 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -212,7 +212,7 @@ if ( $query->param('place_reserve') ) { > exit; > } > >- my $failed_holds = 0; >+ my @failed_holds; > while (@selectedItems) { > my $biblioNum = shift(@selectedItems); > my $itemNum = shift(@selectedItems); >@@ -264,11 +264,21 @@ if ( $query->param('place_reserve') ) { > my $biblio = Koha::Biblios->find($biblioNum); > my $rank = $biblio->holds->search( { found => [ { "!=" => "W" }, undef ] } )->count + 1; > if ( $item ) { >- $canreserve = 1 if CanItemBeReserved( $patron, $item, $branch )->{status} eq 'OK'; >+ my $status = CanItemBeReserved( $patron, $item, $branch )->{status}; >+ if( $status eq 'OK' ){ >+ $canreserve = 1; >+ } else { >+ push @failed_holds, $status; >+ } >+ > } > else { >- $canreserve = 1 >- if CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $itemtype } )->{status} eq 'OK'; >+ my $status = CanBookBeReserved( $borrowernumber, $biblioNum, $branch, { itemtype => $itemtype } )->{status}; >+ if( $status eq 'OK'){ >+ $canreserve = 1; >+ } else { >+ push @failed_holds, $status; >+ } > > # Inserts a null into the 'itemnumber' field of 'reserves' table. > $itemNum = undef; >@@ -279,6 +289,7 @@ if ( $query->param('place_reserve') ) { > if ( $maxreserves > && $reserve_cnt >= $maxreserves ) > { >+ push @failed_holds, 'tooManyReserves'; > $canreserve = 0; > } > >@@ -287,7 +298,8 @@ if ( $query->param('place_reserve') ) { > my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count; > my $nb_of_items_unavailable = $items_in_this_library->search({ -or => { lost => { '!=' => 0 }, damaged => { '!=' => 0 }, } }); > if ( $items_in_this_library->count > $nb_of_items_issued + $nb_of_items_unavailable ) { >- $canreserve = 0 >+ $canreserve = 0; >+ push @failed_holds, 'items_available'; > } > } > >@@ -309,12 +321,15 @@ if ( $query->param('place_reserve') ) { > item_group_id => $item_group_id, > } > ); >- $failed_holds++ unless $reserve_id; >- ++$reserve_cnt; >+ if( $reserve_id ){ >+ ++$reserve_cnt; >+ } else { >+ push @failed_holds, 'not_placed'; >+ } > } > } > >- print $query->redirect("/cgi-bin/koha/opac-user.pl?" . ( $failed_holds ? "failed_holds=$failed_holds" : q|| ) . "#opac-user-holds"); >+ print $query->redirect("/cgi-bin/koha/opac-user.pl?" . ( @failed_holds ? "failed_holds=" . join('|',@failed_holds) : q|| ) . "#opac-user-holds"); > exit; > } > >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 33302
:
148813
|
148896
|
149123
|
150667
|
150668