Bugzilla – Attachment 110722 Details for
Bug 23091
Add options to charge new or restore forgiven overdues when a lost item is returned
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23091: Add handling for new lostreturn rules
Bug-23091-Add-handling-for-new-lostreturn-rules.patch (text/plain), 21.58 KB, created by
Martin Renvoize (ashimema)
on 2020-09-25 09:32:50 UTC
(
hide
)
Description:
Bug 23091: Add handling for new lostreturn rules
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-09-25 09:32:50 UTC
Size:
21.58 KB
patch
obsolete
>From 9d0c3eb9f45190bb6b2b458a01581551336f8019 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >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 <andrew@bywatersolutions.com> >--- > 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 3571051e84..027dcea264 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1508,6 +1508,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); >@@ -1516,6 +1519,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 ) { >@@ -2021,6 +2070,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; >+ } >+ } > } > } > >@@ -2444,6 +2529,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( >@@ -2462,14 +2548,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 87062f541a..2a770cebc0 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 4a4727e5c2..3eb23a029f 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -522,16 +522,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 > } > } > ); >@@ -541,18 +541,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 @@ > </tr> > <tr> > <td> >- <select name="refund"> >+ <select name="lostreturn"> > [%# Default branch %] > [% IF ( current_branch == '*' ) %] >- [% IF ( defaultRefundRule ) %] >- <option value="1" selected="selected"> >+ [% IF ( defaultRefundRule == 'refund' ) %] >+ <option value="refund" selected="selected">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> >+ [% ELSIF ( defaultRefundRule == 'charge' ) %] >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge" selected="selected">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> >+ [% ELSIF ( defaultRefundRule == 'restore' ) %] >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore" selected="selected">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> >+ [% ELSIF ( defaultRefundRule == 0 ) %] >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0" selected="selected">Leave lost item charge</option> > [% ELSE %] >- <option value="1"> >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> > [% END %] >- Yes >- </option> >- [% IF ( not defaultRefundRule ) %] >- <option value="0" selected="selected"> >- [% ELSE %] >- <option value="0"> >- [% END %] >- No >- </option> > [% ELSE %] > [%# Branch-specific %] > [% IF ( not refundLostItemFeeRule ) %] >@@ -786,30 +798,43 @@ > [% ELSE %] > <option value="*"> > [% END %] >- [% IF defaultRefundRule %] >- Use default (Yes) >+ [% IF defaultRefundRule == 'refund' %] >+ Use default (Refund lost item charge) >+ [% ELSIF defaultRefundRule == 'charge' %] >+ Use default (Refund lost item charge and restore overdue fine) >+ [% ELSIF defaultRefundRule == 'restore' %] >+ Use default (Refund lost item charge and charge new overdue fine) > [% ELSE %] >- Use default (No) >+ Use default (Leave lost item charge) > [% END %] > </option> > [% IF ( not refundLostItemFeeRule ) %] >- <option value="1">Yes</option> >- <option value="0">No</option> >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> > [% ELSE %] >- [% IF ( refundLostItemFeeRule.rule_value ) %] >- <option value="1" selected="selected"> >- [% ELSE %] >- <option value="1"> >+ [% IF ( refundLostItemFeeRule.rule_value == 'refund' ) %] >+ <option value="refund" selected="selected">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> >+ [% ELSIF ( refundLostItemFeeRule.rule_value == 'charge' ) %] >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge" selected="selected">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> >+ [% ELSIF ( refundLostItemFeeRule.rule_value == 'restore' ) %] >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore" selected="selected">Refund lost item charge and restore overdue fine</option> >+ <option value="0">Leave lost item charge</option> >+ [% ELSIF ( refundLostItemFeeRule.rule_value == 0 ) %] >+ <option value="refund">Refund lost item charge</option> >+ <option value="charge">Refund lost item charge and charge new overdue fine</option> >+ <option value="restore">Refund lost item charge and restore overdue fine</option> >+ <option value="0" selected="selected">Leave lost item charge</option> > [% END %] >- Yes >- </option> >- [% IF ( not refundLostItemFeeRule.rule_value ) %] >- <option value="0" selected="selected"> >- [% ELSE %] >- <option value="0"> >- [% END %] >- No >- </option> > [% END %] > [% END %] > </select> >-- >2.20.1
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 23091
:
96217
|
105529
|
105530
|
105531
|
105544
|
105545
|
105546
|
105547
|
105567
|
105649
|
106871
|
106872
|
106873
|
106874
|
106875
|
106876
|
106906
|
106907
|
106908
|
106909
|
106910
|
106911
|
106912
|
106961
|
107026
|
107027
|
107028
|
107029
|
107030
|
107031
|
107032
|
107033
|
107034
|
107100
|
107101
|
107102
|
107103
|
107104
|
107105
|
107106
|
107107
|
107108
|
107335
|
107336
|
107337
|
107338
|
107339
|
107340
|
107341
|
107342
|
107343
|
108718
|
108719
|
108720
|
108721
|
108722
|
108723
|
108724
|
109571
|
109572
|
109573
|
109574
|
109575
|
109576
|
109577
|
110660
|
110661
|
110662
|
110663
|
110664
|
110665
|
110666
|
110667
|
110720
|
110721
|
110722
|
110723
|
110724
|
110725
|
110726
|
110727
|
111501
|
111502
|
111503
|
111504
|
111505
|
111506
|
111507
|
111508
|
111935
|
111936
|
111965
|
111979
|
111980
|
111981
|
111982
|
111983
|
111984
|
111985
|
111986
|
111987
|
111988
|
111989
|
113081
|
113082
|
113083
|
113084
|
113085
|
113086
|
113087
|
113088
|
113089
|
113090
|
113091