From 5a62462cd139c479c73de05cc5ce11174e7e7031 Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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. Signed-off-by: Kyle M Hall Signed-off-by: Martha Fuerst --- C4/Circulation.pm | 48 +++++++-------- .../prog/en/modules/admin/smart-rules.tt | 31 ++++++++-- t/db_dependent/Circulation.t | 71 ++++++++++++++++++++-- t/db_dependent/RefundLostItemFeeRule.t | 8 +-- 4 files changed, 119 insertions(+), 39 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 2deb3678a1..2df9a81cee 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 eq '2' ); } } @@ -1961,18 +1959,16 @@ sub AddReturn { if ( $item->{'itemlost'} ) { $messages->{'WasLost'} = 1; unless ( C4::Context->preference("BlockReturnOfLostItems") ) { - if ( - Koha::RefundLostItemFeeRules->should_refund( - { - current_branch => C4::Context->userenv->{branch}, - item_home_branch => $item->{homebranch}, - item_holding_branch => $item_holding_branch - } - ) - ) - { - _FixAccountForLostAndReturned( $item->{'itemnumber'}, - $borrowernumber, $barcode ); + my $should_refund = Koha::RefundLostItemFeeRules->should_refund( + { + current_branch => C4::Context->userenv->{branch}, + item_home_branch => $item->{homebranch}, + item_holding_branch => $item_holding_branch + } + ); + + if ($should_refund) { + _FixAccountForLostAndReturned( $item->{itemnumber}, $borrowernumber, $barcode, $should_refund eq '2' ); $messages->{'LostItemFeeRefunded'} = 1; } } @@ -2384,6 +2380,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 +2395,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 +2403,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 @@ 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)