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>
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit ee318d20f5)
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
This commit is contained in:
parent
4aa36507f4
commit
28db837abb
2 changed files with 68 additions and 9 deletions
|
|
@ -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 %]
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue