Bugzilla – Attachment 94099 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.78 KB, created by
Jonathan Druart
on 2019-10-14 13:56:38 UTC
(
hide
)
Description:
Bug 20262: Add ability to refund lost item fees without creating credits
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2019-10-14 13:56:38 UTC
Size:
14.78 KB
patch
obsolete
>From 087f0ee4a9a7cf9552d193d9810738f9decaa7d4 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. > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> > >Signed-off-by: Martha Fuerst <mfuerst@hmcpl.org> >--- > C4/Circulation.pm | 45 ++++++++------- > .../prog/en/modules/admin/smart-rules.tt | 29 ++++++++-- > t/db_dependent/Circulation.t | 67 +++++++++++++++++++++- > t/db_dependent/RefundLostItemFeeRule.t | 8 +-- > 4 files changed, 117 insertions(+), 32 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 014c018628..839c4d693d 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1436,19 +1436,18 @@ sub AddIssue { > > ## If item was lost, it has now been found, reverse any list item charges if necessary. > if ( $item_object->itemlost ) { >- if ( >- Koha::RefundLostItemFeeRules->should_refund( >+ my $should_refund = Koha::RefundLostItemFeeRules->should_refund( > { >- current_branch => C4::Context->userenv->{branch}, >- item_home_branch => $item_object->homebranch, >+ current_branch => C4::Context->userenv->{branch}, >+ item_home_branch => $item_object->homebranch, > item_holding_branch => $item_object->holdingbranch, > } >- ) >- ) >- { >- _FixAccountForLostAndReturned( $item_object->itemnumber, undef, >- $item_object->barcode ); >- } >+ ); >+ >+ if ($should_refund) { >+ _FixAccountForLostAndReturned( $item_object->itemnumber, >+ undef , $item_object->barcode, $should_refund eq '2' ); >+ } > } > > ModItem( >@@ -2008,18 +2007,17 @@ 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 >- } >- ) >- ) >- { >+ 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 ); >+ $borrowernumber, $barcode, $should_refund eq '2' ); > $messages->{'LostItemFeeRefunded'} = 1; > } > } >@@ -2405,6 +2403,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; > > my $credit; > >@@ -2424,6 +2423,8 @@ sub _FixAccountForLostAndReturned { > my $accountline = $accountlines->next; > my $total_to_refund = 0; > >+ return if $skip_paid && $accountline->amountoutstanding <= 0; >+ > return unless $accountline->borrowernumber; > my $patron = Koha::Patrons->find( $accountline->borrowernumber ); > return unless $patron; # Patron has been deleted, nobody to credit the return to >@@ -2447,7 +2448,7 @@ sub _FixAccountForLostAndReturned { > : 0; > } > >- my $credit_total = $accountline->amountoutstanding + $total_to_refund; >+ my $credit_total = $accountline->amountoutstanding + ( $skip_paid ? 0 : $total_to_refund ); > > if ( $credit_total > 0 ) { > my $branchcode = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; >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 ed8bc3034e..af257d15e3 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 >@@ -639,20 +639,28 @@ > <select name="refund"> > [#% Default branch %#] > [% IF ( current_branch == '*' ) %] >- [% IF ( defaultRefundRule ) %] >+ [% IF ( defaultRefundRule == 1 ) %] > <option value="1" selected="selected"> > [% ELSE %] > <option value="1"> > [% END %] > Yes > </option> >- [% IF ( not defaultRefundRule ) %] >+ [% IF ( defaultRefundRule == 0 ) %] > <option value="0" selected="selected"> > [% ELSE %] > <option value="0"> > [% END %] > No > </option> >+ >+ [% IF ( defaultRefundRule == 2 ) %] >+ <option value="2" selected="selected"> >+ [% ELSE %] >+ <option value="2"> >+ [% END %] >+ Only if unpaid >+ </option> > [% ELSE %] > [#% Branch-specific %#] > [% IF ( not refundLostItemFeeRule ) %] >@@ -660,8 +668,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 %] >@@ -669,21 +679,30 @@ > [% IF ( not refundLostItemFeeRule ) %] > <option value="1">Yes</option> > <option value="0">No</option> >+ <option value="2">Only if unpaid</option> > [% ELSE %] >- [% IF ( refundLostItemFeeRule.rule_value ) %] >+ [% IF ( refundLostItemFeeRule.rule_value == 1 ) %] > <option value="1" selected="selected"> > [% ELSE %] > <option value="1"> > [% END %] > Yes > </option> >- [% IF ( not refundLostItemFeeRule.rule_value ) %] >+ [% IF ( refundLostItemFeeRule.rule_value == 0 ) %] > <option value="0" selected="selected"> > [% ELSE %] > <option value="0"> > [% END %] > No > </option> >+ >+ [% IF ( refundLostItemFeeRule.rule_value == 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 b7dca93851..0b29e3e770 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -2078,7 +2078,7 @@ subtest 'AddReturn | is_overdue' => sub { > > subtest '_FixAccountForLostAndReturned' => sub { > >- plan tests => 5; >+ plan tests => 14; > > t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); > t::lib::Mocks::mock_preference( 'WhenLostForgiveFine', 0 ); >@@ -2464,6 +2464,71 @@ subtest '_FixAccountForLostAndReturned' => sub { > my $manual_debit = Koha::Account::Lines->search({ borrowernumber => $patron->id, accounttype => 'OVERDUE', status => 'UNRETURNED' })->next; > is( $manual_debit->amountoutstanding + 0, $manual_debit_amount - $payment_amount, 'reconcile_balance was called' ); > }; >+ >+ >+ 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( $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 4599165961..548afcd8ee 100755 >--- a/t/db_dependent/RefundLostItemFeeRule.t >+++ b/t/db_dependent/RefundLostItemFeeRule.t >@@ -181,7 +181,7 @@ subtest 'Koha::RefundLostItemFeeRules::_effective_branch_rule() tests' => sub { > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >- rule_value => 1, >+ rule_value => 2, > } > } > ); >@@ -215,7 +215,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; >@@ -319,7 +319,7 @@ subtest 'Koha::RefundLostItemFeeRules::should_refund() tests' => sub { > categorycode => undef, > itemtype => undef, > rule_name => 'refund', >- rule_value => 1 >+ rule_value => 2 > } > } > ); >@@ -356,7 +356,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.11.0
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