From 3bfe2d21a07c558be5a6bdc4022ffa5978dc7451 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 --- C4/Circulation.pm | 157 +++++++++++++++--- Koha/CirculationRules.pm | 19 ++- admin/smart-rules.pl | 16 +- .../prog/en/modules/admin/smart-rules.tt | 87 ++++++---- 4 files changed, 213 insertions(+), 66 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 1dcfad42ea..1ab54d99b2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1453,19 +1453,45 @@ sub AddIssue { UpdateTotalIssues( $item_object->biblionumber, 1 ); } - ## If item was lost, it has now been found, reverse any list item charges if necessary. + ## If item was lost, it has now been found, reverse any lost item charges if necessary. if ( $item_object->itemlost ) { - if ( - Koha::CirculationRules->get_lostreturn_policy( - { - return_branch => C4::Context->userenv->{branch}, - item => $item_object - } - ) - ) - { + my $lostreturn_policy = + Koha::CirculationRules->get_lostreturn_policy( + { + return_branch => C4::Context->userenv->{branch}, + item => $item_object + } + ); + + if ($lostreturn_policy) { _FixAccountForLostAndFound( $item_object->itemnumber, undef, $item_object->barcode ); + + if ( $lostreturn_policy eq 'charge' ) { + $actualissue //= Koha::Old::Checkouts->search( + { itemnumber => $item_unblessed->{itemnumber} }, + { + order_by => { '-desc' => 'returndate' }, + rows => 1 + } + )->single; + + _CalculateAndUpdateFine( + { + issue => $actualissue, + item => $item_unblessed, + borrower => $borrower + } + ); + _FixOverduesOnReturn( $borrower->{borrowernumber}, + $item_object->itemnumber, undef, 'RENEWED' ); + } + elsif ( $lostreturn_policy eq 'restore' ) { + _RestoreOverdueForLostAndFound( + $borrower->{'borrowernumber'}, + $item_object->itemnumber + ); + } } } @@ -2038,18 +2064,39 @@ sub AddReturn { if ( $item->itemlost ) { $messages->{'WasLost'} = 1; unless ( C4::Context->preference("BlockReturnOfLostItems") ) { - if ( - Koha::CirculationRules->get_lostreturn_policy( - { - return_branch => C4::Context->userenv->{branch}, - item => $item, - } - ) - ) - { + my $lostreturn_policy = + Koha::CirculationRules->get_lostreturn_policy( + { + return_branch => C4::Context->userenv->{branch}, + item => $item + } + ); + + if ($lostreturn_policy) { _FixAccountForLostAndFound( $item->itemnumber, $borrowernumber, $barcode ); $messages->{'LostItemFeeRefunded'} = 1; + + if ( $lostreturn_policy eq 'charge' ) { + $issue //= Koha::Old::Checkouts->search( + { itemnumber => $item->itemnumber }, + { order_by => { '-desc' => 'returndate' }, rows => 1 } + )->single; + _CalculateAndUpdateFine( + { + issue => $issue, + item => $item->unblessed, + borrower => $patron_unblessed, + return_date => $return_date + } + ); + $messages->{'LostItemFeeCharged'} = 1; + } + elsif ( $lostreturn_policy eq 'restore' ) { + _RestoreOverdueForLostAndFound( $borrowernumber, + $item->itemnumber ); + $messages->{'LostItemFeeRestored'} = 1; + } } } } @@ -2454,15 +2501,12 @@ sub _FixOverduesOnReturn { $credit->apply({ debits => [ $accountline ], offset_type => 'Forgiven' }); - $accountline->status('FORGIVEN'); - if (C4::Context->preference("FinesLog")) { &logaction("FINES", 'MODIFY',$borrowernumber,"Overdue forgiven: item $item"); } - } else { - $accountline->status($status); } + $accountline->status($status); return $accountline->store(); } ); @@ -2559,6 +2603,73 @@ sub _FixAccountForLostAndFound { return ($credit) ? $credit->id : undef; } +=head2 _RestoreOverdueForLostAndFound + + &_RestoreOverdueForLostAndFound($borrowernumber, $itemnumber ); + +C<$borrowernumber> borrowernumber + +C<$itemnumber> itemnumber + +Internal function + +=cut + +sub _RestoreOverdueForLostAndFound { + my ( $borrowernumber, $item ) = @_; + + unless( $borrowernumber ) { + warn "_RestoreOverdueForLostAndFound() not supplied valid borrowernumber"; + return; + } + 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( + { + borrowernumber => $borrowernumber, + 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(); + } + ); + + return $result; +} + =head2 _GetCircControlBranch my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower); diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 9bf1ab860e..5e92674a60 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' ], }, @@ -428,7 +428,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 @@ -449,11 +460,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 04ddb2698a..1c17758dc1 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -523,16 +523,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 } } ); @@ -542,18 +542,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 cfea9b6cec..c64838e3f0 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 @@ -738,23 +738,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 ) %] @@ -762,30 +774,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