From bb6277ffcc593685554112d770ac273c9efeda33 Mon Sep 17 00:00:00 2001 From: Jesse Weaver Date: Tue, 9 Jun 2015 16:22:05 -0600 Subject: [PATCH] Bug 14464: Add ability to cancel waiting holds from checkin screen Test plan: 1. Ensure that ExpireReservesMaxPickUpDelayCharge is set to 0. 2. Place a hold (doesn't matter whether it's a bib/item-level hold), then confirm the hold by checking it in. 3. Check in the item again, and hit Cancel. 4. The reserve in question should be cancelled. 5. Repeat steps 2-4 twice, once after setting ExpireReservesMaxPickUpDelayCharge to a nonzero value and again after clicking the "Forgive fees for manually expired holds" checkbox. A fine should only be applied when the syspref is enabled and the checkbox is not checked. Also, the checkbox should only appear after enabling the syspref. And finally, the checkbox should remember whether it is checked across multiple checkins, same as the "Forgive overdue charges" and "Book drop mode" checkboxes. Signed-off-by: Jason Burds Signed-off-by: Jonathan Druart Amended patch: Removed 2 debugging lines. Signed-off-by: Tomas Cohen Arazi --- C4/Reserves.pm | 21 +++++++------ circ/returns.pl | 19 +++++++++--- .../prog/en/modules/circ/returns.tt | 30 +++++++++++++++++++ 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 6252b86158..96570df58f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1077,7 +1077,6 @@ sub CancelExpiredReserves { # Cancel reserves that have been waiting too long if ( C4::Context->preference("ExpireReservesMaxPickUpDelay") ) { my $max_pickup_delay = C4::Context->preference("ReservesMaxPickUpDelay"); - my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); my $today = dt_from_string(); @@ -1098,11 +1097,7 @@ sub CancelExpiredReserves { } if ( $do_cancel ) { - if ( $charge ) { - manualinvoice($res->{'borrowernumber'}, $res->{'itemnumber'}, 'Hold waiting too long', 'F', $charge); - } - - CancelReserve({ reserve_id => $res->{'reserve_id'} }); + CancelReserve({ reserve_id => $res->{'reserve_id'}, charge_cancel_fee => 1 }); } } } @@ -1129,9 +1124,9 @@ sub AutoUnsuspendReserves { =head2 CancelReserve - CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber ] }); + CancelReserve({ reserve_id => $reserve_id, [ biblionumber => $biblionumber, borrowernumber => $borrrowernumber, itemnumber => $itemnumber, ] [ charge_cancel_fee => 1 ] }); -Cancels a reserve. +Cancels a reserve. If C is passed and the C syspref is set, charge that fee to the patron's account. =cut @@ -1139,7 +1134,9 @@ sub CancelReserve { my ( $params ) = @_; my $reserve_id = $params->{'reserve_id'}; - $reserve_id = GetReserveId( $params ) unless ( $reserve_id ); + # Filter out only the desired keys; this will insert undefined values for elements missing in + # \%params, but GetReserveId filters them out anyway. + $reserve_id = GetReserveId( { biblionumber => $params->{'biblionumber'}, borrowernumber => $params->{'borrowernumber'}, itemnumber => $params->{'itemnumber'} } ) unless ( $reserve_id ); return unless ( $reserve_id ); @@ -1174,6 +1171,12 @@ sub CancelReserve { # now fix the priority on the others.... _FixPriority({ biblionumber => $reserve->{biblionumber} }); + + # and, if desired, charge a cancel fee + my $charge = C4::Context->preference("ExpireReservesMaxPickUpDelayCharge"); + if ( $charge && $params->{'charge_cancel_fee'} ) { + manualinvoice($reserve->{'borrowernumber'}, $reserve->{'itemnumber'}, 'Hold waiting too long', 'F', $charge); + } } return $reserve; diff --git a/circ/returns.pl b/circ/returns.pl index 8084727e0b..4a13711da5 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -30,6 +30,9 @@ script to execute returns of books use strict; use warnings; +use Carp 'verbose'; +$SIG{ __DIE__ } = sub { Carp::confess( @_ ) }; + use CGI qw ( -utf8 ); use DateTime; use C4::Context; @@ -84,6 +87,7 @@ my $printers = GetPrinters(); my $userenv = C4::Context->userenv; my $userenv_branch = $userenv->{'branch'} // ''; my $printer = $userenv->{'branchprinter'} // ''; +my $forgivemanualholdsexpire = $query->param('forgivemanualholdsexpire'); my $overduecharges = (C4::Context->preference('finesMode') && C4::Context->preference('finesMode') ne 'off'); # @@ -144,12 +148,18 @@ if ( $query->param('resbarcode') ) { my $resbarcode = $query->param('resbarcode'); my $diffBranchReturned = $query->param('diffBranch'); my $iteminfo = GetBiblioFromItemNumber($item); + my $cancel_reserve = $query->param('cancel_reserve'); # fix up item type for display $iteminfo->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $iteminfo->{'itype'} : $iteminfo->{'itemtype'}; - my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef; -# diffBranchSend tells ModReserveAffect whether document is expected in this library or not, -# i.e., whether to apply waiting status - ModReserveAffect( $item, $borrowernumber, $diffBranchSend); + + if ( $cancel_reserve ) { + CancelReserve({ borrowernumber => $borrowernumber, itemnumber => $item, charge_cancel_fee => !$forgivemanualholdsexpire }); + } else { + my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef; + # diffBranchSend tells ModReserveAffect whether document is expected in this library or not, + # i.e., whether to apply waiting status + ModReserveAffect( $item, $borrowernumber, $diffBranchSend); + } # check if we have other reserves for this document, if we have a return send the message of transfer my ( $messages, $nextreservinfo ) = GetOtherReserves($item); @@ -613,6 +623,7 @@ $template->param( exemptfine => $exemptfine, dropboxmode => $dropboxmode, dropboxdate => output_pref($dropboxdate), + forgivemanualholdsexpire => $forgivemanualholdsexpire, overduecharges => $overduecharges, soundon => C4::Context->preference("SoundOn"), BlockReturnOfWithdrawnItems => C4::Context->preference("BlockReturnOfWithdrawnItems"), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt index f710cdbba7..769e4c0032 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt @@ -79,6 +79,16 @@ $(document).ready(function () { } $("#barcode").focus(); }); + $("#forgivemanualholdsexpire").change(function () { + if (this.checked == true) { + $("#barcode").addClass("alert"); + $("#forgivemanualholdsexpire-alert").show(); + } else { + $("#barcode").removeClass("alert"); + $("#forgivemanualholdsexpire-alert").hide(); + } + $("#barcode").focus(); + }); [% IF(overduecharges) %] $("#barcode").focus(function () { if (($("#exemptcheck").attr("checked") == true) || ($("#dropboxcheck").attr("checked") == true)) { $("#barcode").addClass("alert"); @@ -211,6 +221,9 @@ $(document).ready(function () {

Hold at [% destbranchname %]

[% END %]
+ + + @@ -227,6 +240,7 @@ $(document).ready(function () { + @@ -272,6 +286,7 @@ $(document).ready(function () { + @@ -307,6 +322,7 @@ $(document).ready(function () { + [% FOREACH inputloo IN inputloop %] @@ -398,6 +414,7 @@ $(document).ready(function () { +
@@ -488,6 +505,9 @@ $(document).ready(function () { + @@ -553,6 +573,16 @@ $(document).ready(function () { [% END %]

+ [% IF Koha.Preference('ExpireReservesMaxPickUpDelayCharge') %] +

+ [% IF ( forgivemanualholdsexpire ) %] + + [% ELSE %] + + [% END %] + +

+ [% END %] -- 2.39.5