Bugzilla – Attachment 75470 Details for
Bug 20262
Add ability to refund lost item fees without creating credits
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20262: Add ability to refund lost item fees without creating credits
Bug-20262-Add-ability-to-refund-lost-item-fees-wit.patch (text/plain), 14.09 KB, created by
Kyle M Hall
on 2018-05-22 14:21:11 UTC
(
hide
)
Description:
Bug 20262: Add ability to refund lost item fees without creating credits
Filename:
MIME Type:
Creator:
Kyle M Hall
Created:
2018-05-22 14:21:11 UTC
Size:
14.09 KB
patch
obsolete
>From 859136292cb8bc9a52cb1c7657d3641694155c9f Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatetsolutions.com> >Date: Wed, 21 Feb 2018 11:10:02 -0500 >Subject: [PATCH] Bug 20262: Add ability to refund lost item fees without > creating credits > >Some libraries handle refunding of paid lost fees at a financial office and not within Koha. To facilitate this, it would be good for Koha to have the option to not generate lost returned credits by skipping fully paid lost items, and only refunding the outstanding balance on partially paid lost fees. > >Test Plan: >1) Apply this patch >2) Set your lost item refund on return policies to 'Only if unpaid' >3) Mark an item lost, charging the lost fee >4) Return the item, a full refund should ocurr >5) Mark another item lost, charging the lost fee >6) Make a partial payment on this lost fee >7) Return the item >8) The remaining balance of that fee should be 0, but the patron should > not recieve a credit for the already paid balance >8) Mark another item lost, charging the lost fee >9) Fully pay the lost fee >10) Return the item. The paid lost fee should be unaffected. >--- > C4/Circulation.pm | 26 ++++---- > .../prog/en/modules/admin/smart-rules.tt | 31 ++++++++-- > t/db_dependent/Circulation.t | 71 ++++++++++++++++++++-- > t/db_dependent/RefundLostItemFeeRule.t | 8 +-- > 4 files changed, 109 insertions(+), 27 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 2deb3678a1..ac8a28669a 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1374,18 +1374,16 @@ sub AddIssue { > > ## If item was lost, it has now been found, reverse any list item charges if necessary. > if ( $item->{'itemlost'} ) { >- if ( >- Koha::RefundLostItemFeeRules->should_refund( >- { >- current_branch => C4::Context->userenv->{branch}, >- item_home_branch => $item->{homebranch}, >- item_holding_branch => $item->{holdingbranch} >- } >- ) >- ) >- { >- _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, >- $item->{'barcode'} ); >+ my $should_refund = Koha::RefundLostItemFeeRules->should_refund( >+ { >+ current_branch => C4::Context->userenv->{branch}, >+ item_home_branch => $item->{homebranch}, >+ item_holding_branch => $item->{holdingbranch} >+ } >+ ); >+ >+ if ($should_refund) { >+ _FixAccountForLostAndReturned( $item->{'itemnumber'}, undef, $item->{'barcode'}, $should_refund == 2 ); > } > } > >@@ -2384,6 +2382,7 @@ sub _FixAccountForLostAndReturned { > my $itemnumber = shift or return; > my $borrowernumber = @_ ? shift : undef; > my $item_id = @_ ? shift : $itemnumber; # Send the barcode if you want that logged in the description >+ my $skip_paid = @_ ? shift : 0; > > # check for charge made for lost book > my $accountline = Koha::Account::Lines->search( >@@ -2398,6 +2397,7 @@ sub _FixAccountForLostAndReturned { > > return unless $accountline; > return if $accountline->accounttype eq 'W'; # Written off >+ return if $skip_paid && $accountline->amountoutstanding <= 0; > > $accountline->accounttype('LR'); > $accountline->store(); >@@ -2405,7 +2405,7 @@ sub _FixAccountForLostAndReturned { > my $account = Koha::Account->new( { patron_id => $accountline->borrowernumber } ); > my $credit_id = $account->pay( > { >- amount => $accountline->amount, >+ amount => $skip_paid ? $accountline->amountoutstanding : $accountline->amount, > description => "Item Returned " . $item_id, > account_type => 'CR', > offset_type => 'Lost Item Return', >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 86226cb7d3..18d93c34e7 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 >@@ -554,20 +554,29 @@ > <select name="refund"> > [#% Default branch %#] > [% IF ( current_branch == '*' ) %] >- [% IF ( refundLostItemFeeRule.refund ) %] >+ [% IF ( refundLostItemFeeRule.refund == 1 ) %] > <option value="1" selected="selected"> > [% ELSE %] > <option value="1"> > [% END %] > Yes > </option> >- [% IF ( not refundLostItemFeeRule.refund ) %] >+ >+ [% IF ( refundLostItemFeeRule.refund == 0 ) %] > <option value="0" selected="selected"> > [% ELSE %] > <option value="0"> > [% END %] > No > </option> >+ >+ [% IF ( refundLostItemFeeRule.refund == 2 ) %] >+ <option value="2" selected="selected"> >+ [% ELSE %] >+ <option value="2"> >+ [% END %] >+ Only if unpaid >+ </option> > [% ELSE %] > [#% Branch-specific %#] > [% IF ( not refundLostItemFeeRule ) %] >@@ -575,8 +584,10 @@ > [% ELSE %] > <option value="*"> > [% END %] >- [% IF defaultRefundRule %] >+ [% IF defaultRefundRule == 1 %] > Use default (Yes) >+ [% ELSIF defaultRefundRule == 2 %] >+ Use default (Only if unpaid) > [% ELSE %] > Use default (No) > [% END %] >@@ -584,21 +595,31 @@ > [% IF ( not refundLostItemFeeRule ) %] > <option value="1">Yes</option> > <option value="0">No</option> >+ <option value="2">Only if unpaid</option> > [% ELSE %] >- [% IF ( refundLostItemFeeRule.refund ) %] >+ [% IF ( refundLostItemFeeRule.refund == 1 ) %] > <option value="1" selected="selected"> > [% ELSE %] > <option value="1"> > [% END %] > Yes > </option> >- [% IF ( not refundLostItemFeeRule.refund ) %] >+ >+ [% IF ( refundLostItemFeeRule.refund == 0 ) %] > <option value="0" selected="selected"> > [% ELSE %] > <option value="0"> > [% END %] > No > </option> >+ >+ [% IF ( refundLostItemFeeRule.refund == 3 ) %] >+ <option value="3" selected="selected"> >+ [% ELSE %] >+ <option value="3"> >+ [% END %] >+ Only if unpaid >+ </option> > [% END %] > [% END %] > </select> >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index e93b019c5b..04602ef493 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -1972,7 +1972,7 @@ subtest 'AddReturn | is_overdue' => sub { > }; > > subtest '_FixAccountForLostAndReturned' => sub { >- plan tests => 2; >+ plan tests => 11; > > # Generate test biblio > my $title = 'Koha for Dummies'; >@@ -1992,11 +1992,11 @@ subtest '_FixAccountForLostAndReturned' => sub { > $biblionumber > ); > >- my $patron = $builder->build( { source => 'Borrower' } ); >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); > > my $accountline = Koha::Account::Line->new( > { >- borrowernumber => $patron->{borrowernumber}, >+ borrowernumber => $patron->borrowernumber, > accounttype => 'L', > itemnumber => $itemnumber, > amount => 99.00, >@@ -2004,12 +2004,73 @@ subtest '_FixAccountForLostAndReturned' => sub { > } > )->store(); > >- C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->{borrowernumber} ); >+ C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber ); > > $accountline->_result()->discard_changes(); > > is( $accountline->amountoutstanding, '0.000000', 'Lost fee has no outstanding amount' ); >- is( $accountline->accounttype, 'LR', 'Lost fee now has account type of LR ( Lost Returned )'); >+ is( $accountline->accounttype, 'LR', 'Lost fee now has account type of LR ( Lost Returned )' ); >+ is( $patron->account->balance, 0, 'Account balance should be 0 after return of lost item' ); >+ >+ # Tests for only if unpaid >+ ## Unpaid >+ $accountline->delete(); >+ $accountline = Koha::Account::Line->new( >+ { >+ borrowernumber => $patron->borrowernumber, >+ accounttype => 'L', >+ itemnumber => $itemnumber, >+ amount => 99.00, >+ amountoutstanding => 99.00, >+ } >+ )->store(); >+ >+ C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber, undef, 1 ); >+ >+ $accountline->_result()->discard_changes(); >+ >+ is( $accountline->amountoutstanding, '0.000000', 'Lost fee has no outstanding amount, with skip_paid = 1 and unpaid' ); >+ is( $accountline->accounttype, 'LR', 'Lost fee now has account type of LR ( Lost Returned ), with skip_paid = 1 and unpaid'); >+ is( $patron->account->balance, 0, 'Account balance should be 0 after return of unpaid lost item' ); >+ >+ ## Paid >+ $accountline->delete(); >+ $accountline = Koha::Account::Line->new( >+ { >+ borrowernumber => $patron->borrowernumber, >+ accounttype => 'L', >+ itemnumber => $itemnumber, >+ amount => 99.00, >+ amountoutstanding => 0.00, >+ } >+ )->store(); >+ >+ C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber, undef, 1 ); >+ >+ $accountline->_result()->discard_changes(); >+ >+ is( $accountline->amountoutstanding, '0.000000', 'Lost fee still has outstanding amount of 0, with skip_paid = 1 and already paid' ); >+ is( $accountline->accounttype, 'L', 'Lost fee now has account type of L ( Lost ), with skip_paid = 1 and already paid'); >+ is( $patron->account->balance, 0, 'Account balance should be 0 after return of partially paid lost item' ); >+ >+ ## Partially paid >+ $accountline->delete(); >+ $accountline = Koha::Account::Line->new( >+ { >+ borrowernumber => $patron->borrowernumber, >+ accounttype => 'L', >+ itemnumber => $itemnumber, >+ amount => 99.00, >+ amountoutstanding => 33.00, >+ } >+ )->store(); >+ >+ C4::Circulation::_FixAccountForLostAndReturned( $itemnumber, $patron->borrowernumber, undef, 1 ); >+ >+ $accountline->_result()->discard_changes(); >+ >+ is( $accountline->amountoutstanding, '0.000000', 'Lost fee has no outstanding amount, with skip_paid = 1 and partially paid' ); >+ is( $accountline->accounttype, 'LR', 'Lost fee now has account type of L ( Lost ), with skip_paid = 1 and partially paid'); > }; > > subtest '_FixOverduesOnReturn' => sub { >diff --git a/t/db_dependent/RefundLostItemFeeRule.t b/t/db_dependent/RefundLostItemFeeRule.t >index 84a373a008..c6057bd456 100755 >--- a/t/db_dependent/RefundLostItemFeeRule.t >+++ b/t/db_dependent/RefundLostItemFeeRule.t >@@ -130,7 +130,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > source => 'RefundLostItemFeeRule', > value => { > branchcode => '*', >- refund => 1 >+ refund => 2 > } > }); > my $specific_rule_false = $builder->build({ >@@ -153,7 +153,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > # Delete specific rules > Koha::RefundLostItemFeeRules->find({ branchcode => $specific_rule_false->{ branchcode } })->delete; > is( Koha::RefundLostItemFeeRules->_effective_branch_rule( $specific_rule_false->{ branchcode } ), >- 1,'No specific rule defined, fallback to global (true)'); >+ 2,'No specific rule defined, fallback to global (true)'); > > # Rollback transaction > $schema->storage->txn_rollback; >@@ -243,7 +243,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > my $specific_rule_true = $builder->build({ > source => 'RefundLostItemFeeRule', > value => { >- refund => 1 >+ refund => 2 > } > }); > # Make sure we have an unused branchcode >@@ -264,7 +264,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > > t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'CheckinLibrary' ); > is( Koha::RefundLostItemFeeRules->should_refund( $params ), >- 1,'Specific rule is applied (true)'); >+ 2,'Specific rule is applied (true)'); > > t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl', 'ItemHomeBranch' ); > is( Koha::RefundLostItemFeeRules->should_refund( $params ), >-- >2.15.1 (Apple Git-101)
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 20262
:
72029
|
72030
|
75470
|
75503
|
75532
|
94099
|
127018
|
127285
|
127286
|
134555
|
136006
|
136007
|
139543
|
139544
|
139545