From 014a6bbe1a91d1c047f0c83ac5263a587db6d330 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 23 Oct 2019 11:32:28 +0100 Subject: [PATCH] Bug 23091: Add handling for new lostreturn rules This patch adds handing for the new values available for the lostreturn policy settings. * undef - Do nothing, leave fees and fines as they were at the point of lose. * refund - Refund the lost item fee only * charge - Refund the lost item fee and charge a fresh overdue fine dated for a return on the date the item is 'found' * restore - Refund the lost item fee and restore the original overdue fine (dated for a 'return' on the date the item was 'lost' Test plan 1/ apply patch 2/ updatedatabase, restart_all 3/ verify finesmode and CalculateFinesOnReturn and WhenLostChargeReplacementFee are on 4/ verify WhenLostForgiveFine is set to "Forgive" 5/ verify circ rules include fines 6/ set Default lost item fee refund on return policy to "Refund lost item charge" 7/ create 4 overdue checkouts that will incur fines 8/ run fines.pl 9/ confirm 4 items checked out with accruing fines 10/ confirm all 4 items have a replacement price Item 1 11/ mark the first item lost 12/ verify that fine is gone and lost fee has been charged 13/ check item in 14/ verify that lost fee is gone and overdue charge has not returned Item 2 15/ set Default lost item fee refund on return policy to "Refund lost item charge and charge new overdue fine" 16/ mark second item lost 17/ verify that fine is gone and lost fee has been charged 18/ check item in 19/ verify that lost fee is gone and a new overdue charge has been made Item 3 20/ set Default lost item fee refund on return policy to "Refund lost item charge and restore overdue fine" 21/ mark third item lost 22/ verify that fine is gone and lost fee has been charged 23/ check item in 24/ verify that lost fee is gone and the old overdue charge has been restored Item 4 25/ set Default lost item fee refund on return policy to "Leave lost item charge" 26/ mark fourth item lost 27/ verify that fine is gone and lost fee has been charged 28/ check item in 29/ verify that lost fee remains and the overdue charge is still gone Signed-off-by: Andrew Fuerste-Henry --- C4/Circulation.pm | 154 +++++++++++++++++- Koha/CirculationRules.pm | 19 ++- admin/smart-rules.pl | 16 +- .../prog/en/modules/admin/smart-rules.tt | 87 ++++++---- 4 files changed, 227 insertions(+), 49 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 2f91511dbd..ac4a7f1045 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1519,6 +1519,9 @@ sub AddIssue { UpdateTotalIssues( $item_object->biblionumber, 1 ); } + # Record if item was lost + my $was_lost = $item_object->itemlost; + $item_object->issues( ( $item_object->issues || 0 ) + 1); $item_object->holdingbranch(C4::Context->userenv->{'branch'}); $item_object->itemlost(0); @@ -1527,6 +1530,52 @@ sub AddIssue { $item_object->store({log_action => 0}); ModDateLastSeen( $item_object->itemnumber ); + # If the item was lost, it has now been found, restore/charge the overdue if necessary + if ($was_lost) { + my $lostreturn_policy = + Koha::CirculationRules->get_lostreturn_policy( + { + return_branch => C4::Context->userenv->{branch}, + item => $item_object + } + ); + + if ($lostreturn_policy) { + if ( $lostreturn_policy eq 'charge' ) { + $actualissue //= Koha::Old::Checkouts->search( + { itemnumber => $item_unblessed->{itemnumber} }, + { + order_by => { '-desc' => 'returndate' }, + rows => 1 + } + )->single; + unless ( exists( $borrower->{branchcode} ) ) { + my $patron = $actualissue->patron; + $borrower = $patron->unblessed; + } + _CalculateAndUpdateFine( + { + issue => $actualissue, + item => $item_unblessed, + borrower => $borrower, + return_date => $issuedate + } + ); + _FixOverduesOnReturn( $borrower->{borrowernumber}, + $item_object->itemnumber, undef, 'RENEWED' ); + } + elsif ( $lostreturn_policy eq 'restore' ) { + _RestoreOverdueForLostAndFound( + $item_object->itemnumber ); + } + + if ( C4::Context->preference('AccountAutoReconcile') ) { + $account->reconcile_balance; + } + } + + } + # If it costs to borrow this book, charge it to the patron's account. my ( $charge, $itemtype ) = GetIssuingCharges( $item_object->itemnumber, $borrower->{'borrowernumber'} ); if ( $charge && $charge > 0 ) { @@ -2032,6 +2081,42 @@ sub AddReturn { $messages->{'WasLost'} = 1; unless ( C4::Context->preference("BlockReturnOfLostItems") ) { $messages->{'LostItemFeeRefunded'} = $updated_item->{_refunded}; + + my $lostreturn_policy = + Koha::CirculationRules->get_lostreturn_policy( + { + return_branch => C4::Context->userenv->{branch}, + item => $updated_item + } + ); + + if ($lostreturn_policy) { + if ( $lostreturn_policy eq 'charge' ) { + $issue //= Koha::Old::Checkouts->search( + { itemnumber => $item->itemnumber }, + { order_by => { '-desc' => 'returndate' }, rows => 1 } + )->single; + unless (exists($patron_unblessed->{branchcode})) { + my $patron = $issue->patron; + $patron_unblessed = $patron->unblessed; + } + _CalculateAndUpdateFine( + { + issue => $issue, + item => $item->unblessed, + borrower => $patron_unblessed, + return_date => $return_date + } + ); + _FixOverduesOnReturn( $patron_unblessed->{borrowernumber}, + $item->itemnumber, undef, 'RETURNED' ); + $messages->{'LostItemFeeCharged'} = 1; + } + elsif ( $lostreturn_policy eq 'restore' ) { + _RestoreOverdueForLostAndFound( $item->itemnumber ); + $messages->{'LostItemFeeRestored'} = 1; + } + } } } @@ -2455,6 +2540,7 @@ sub _FixOverduesOnReturn { my $amountoutstanding = $accountline->amountoutstanding; if ( $accountline->amount == 0 && $payments->count == 0 ) { $accountline->delete; + return 0; # no warning, we've just removed a zero value fine (backdated return) } elsif ($exemptfine && ($amountoutstanding != 0)) { my $account = Koha::Account->new({patron_id => $borrowernumber}); my $credit = $account->add_credit( @@ -2473,14 +2559,70 @@ sub _FixOverduesOnReturn { if (C4::Context->preference("FinesLog")) { &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item"); } + } - $accountline->status('FORGIVEN'); - $accountline->store(); - } else { - $accountline->status($status); - $accountline->store(); + $accountline->status($status); + return $accountline->store(); + } + ); - } + return $result; +} + +=head2 _RestoreOverdueForLostAndFound + + &_RestoreOverdueForLostAndFound( $itemnumber ); + +C<$itemnumber> itemnumber + +Internal function + +=cut + +sub _RestoreOverdueForLostAndFound { + my ( $item ) = @_; + + unless( $item ) { + warn "_RestoreOverdueForLostAndFound() not supplied valid itemnumber"; + return; + } + + my $schema = Koha::Database->schema; + + my $result = $schema->txn_do( + sub { + # check for lost overdue fine + my $accountlines = Koha::Account::Lines->search( + { + itemnumber => $item, + debit_type_code => 'OVERDUE', + status => 'LOST' + }, + { + order_by => { '-desc' => 'date' }, + rows => 1 + } + ); + return 0 unless $accountlines->count; # no warning, there's just nothing to fix + + # Update status of fine + my $overdue = $accountlines->next; + $overdue->status('RETURNED')->store(); + + # Find related forgive credit + my $refunds = $overdue->credits( + { + credit_type_code => 'FORGIVEN', + itemnumber => $item, + status => [ { '!=' => 'VOID' }, undef ] + }, + { order_by => { '-desc' => 'date' }, rows => 1 } + ); + return 0 unless $refunds->count; # no warning, there's just nothing to fix + + # Revert the forgive credit + my $refund = $refunds->next; + return $refund->void(); } ); diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 5b63998a8b..78fda01da5 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -46,7 +46,7 @@ Any attempt to set a rule with a nonsensical scope (for instance, setting the C< =cut our $RULE_KINDS = { - refund => { + lostreturn => { scope => [ 'branchcode' ], }, @@ -433,7 +433,18 @@ sub get_onshelfholds_policy { =head3 get_lostreturn_policy - my $refund = Koha::CirculationRules->get_lostreturn_policy( { return_branch => $return_branch, item => $item } ); + my $lostrefund_policy = Koha::CirculationRules->get_lostreturn_policy( { return_branch => $return_branch, item => $item } ); + +Return values are: + +=over 2 + +=item 0 - Do not refund +=item refund - Refund the lost item charge +=item restore - Refund the lost item charge and restore the original overdue fine +=item charge - Refund the lost item charge and charge a new overdue fine + +=back =cut @@ -454,11 +465,11 @@ sub get_lostreturn_policy { my $rule = Koha::CirculationRules->get_effective_rule( { branchcode => $branch, - rule_name => 'refund', + rule_name => 'lostreturn', } ); - return $rule ? $rule->rule_value : 1; + return $rule ? $rule->rule_value : 'refund'; } =head3 article_requestable_rules diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index bdae9b5c27..2b962c1366 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -517,16 +517,16 @@ elsif ($op eq "add-branch-item") { } elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { - my $refund = $input->param('refund'); + my $lostreturn = $input->param('lostreturn'); - if ( $refund eq '*' ) { + if ( $lostreturn eq '*' ) { if ( $branch ne '*' ) { - # only do something for $refund eq '*' if branch-specific + # only do something for $lostreturn eq '*' if branch-specific Koha::CirculationRules->set_rules( { branchcode => $branch, rules => { - refund => undef + lostreturn => undef } } ); @@ -536,18 +536,18 @@ elsif ( $op eq 'mod-refund-lost-item-fee-rule' ) { { branchcode => $branch, rules => { - refund => $refund + lostreturn => $lostreturn } } ); } } -my $refundLostItemFeeRule = Koha::CirculationRules->find({ branchcode => ($branch eq '*') ? undef : $branch, rule_name => 'refund' }); -my $defaultLostItemFeeRule = Koha::CirculationRules->find({ branchcode => undef, rule_name => 'refund' }); +my $refundLostItemFeeRule = Koha::CirculationRules->find({ branchcode => ($branch eq '*') ? undef : $branch, rule_name => 'lostreturn' }); +my $defaultLostItemFeeRule = Koha::CirculationRules->find({ branchcode => undef, rule_name => 'lostreturn' }); $template->param( refundLostItemFeeRule => $refundLostItemFeeRule, - defaultRefundRule => $defaultLostItemFeeRule ? $defaultLostItemFeeRule->rule_value : 1 + defaultRefundRule => $defaultLostItemFeeRule ? $defaultLostItemFeeRule->rule_value : 'refund' ); my $patron_categories = Koha::Patron::Categories->search({}, { order_by => ['description'] }); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index b34d02e2fa..9b580dfa54 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -762,23 +762,35 @@ - [%# Default branch %] [% IF ( current_branch == '*' ) %] - [% IF ( defaultRefundRule ) %] - + + + + [% ELSIF ( defaultRefundRule == 'charge' ) %] + + + + + [% ELSIF ( defaultRefundRule == 'restore' ) %] + + + + + [% ELSIF ( defaultRefundRule == 0 ) %] + + + + [% ELSE %] - + + + [% END %] - Yes - - [% IF ( not defaultRefundRule ) %] - [% ELSE %] [%# Branch-specific %] [% IF ( not refundLostItemFeeRule ) %] @@ -786,30 +798,43 @@ [% ELSE %] [% IF ( not refundLostItemFeeRule ) %] - - + + + + [% ELSE %] - [% IF ( refundLostItemFeeRule.rule_value ) %] - + + + + [% ELSIF ( refundLostItemFeeRule.rule_value == 'charge' ) %] + + + + + [% ELSIF ( refundLostItemFeeRule.rule_value == 'restore' ) %] + + + + + [% ELSIF ( refundLostItemFeeRule.rule_value == 0 ) %] + + + + [% END %] - Yes - - [% IF ( not refundLostItemFeeRule.rule_value ) %] - [% END %] [% END %] -- 2.20.1