Bugzilla – Attachment 98772 Details for
Bug 20815
Add ability to choose if lost fee is refunded based on length of time item has been lost
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost
Bug-20815-Add-ability-to-choose-if-lost-fee-is-ref.patch (text/plain), 15.77 KB, created by
Kyle M Hall (khall)
on 2020-02-12 19:31:28 UTC
(
hide
)
Description:
Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2020-02-12 19:31:28 UTC
Size:
15.77 KB
patch
obsolete
>From de617144c5a35f4c6b91ab20f24c003714564a40 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Wed, 12 Feb 2020 12:55:51 -0500 >Subject: [PATCH] Bug 20815: Add ability to choose if lost fee is refunded > based on length of time item has been lost > >This adds the ability to not refund lost item fees on return if the item >has been lost for more than a given number of days. > >Test Plan: >1) Set the new system preference NoRefundOnLostReturnedItemsAge to a number of days >2) Find a lost item that has been lost longer than that NoRefundOnLostReturnedItemsAge days which would have otherwise been refunded >3) Return the item >4) Note no refund on the lost item fee was processed, the fee remains unchanged >5) prove t/db_dependent/Circulation.t >--- > C4/Circulation.pm | 36 ++- > installer/data/mysql/sysprefs.sql | 1 + > .../admin/preferences/circulation.pref | 5 + > t/db_dependent/Circulation.t | 302 +++++++++++++++++- > 4 files changed, 338 insertions(+), 6 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 379f1f46b5..c213977fb9 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1445,18 +1445,31 @@ sub AddIssue { > > ## If item was lost, it has now been found, reverse any list item charges if necessary. > if ( $item_object->itemlost ) { >+ my $refund = 1; >+ my $no_refund_after_days = C4::Context->preference('NoRefundOnLostReturnedItemsAge'); >+ if ($no_refund_after_days) { >+ my $today = dt_from_string(); >+ my $lost_age_in_days = >+ dt_from_string( $item_object->itemlost_on ) >+ ->delta_days($today) >+ ->in_units('days'); >+ >+ $refund = 0 unless ( $lost_age_in_days < $no_refund_after_days ); >+ } >+ > if ( >- Koha::RefundLostItemFeeRules->should_refund( >+ $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 ); >+ _FixAccountForLostAndReturned( $item_object->itemnumber, >+ undef, $item_object->barcode ); > } > } > >@@ -2027,7 +2040,20 @@ sub AddReturn { > if ( $item->itemlost ) { > $messages->{'WasLost'} = 1; > unless ( C4::Context->preference("BlockReturnOfLostItems") ) { >+ my $refund = 1; >+ my $no_refund_after_days = C4::Context->preference('NoRefundOnLostReturnedItemsAge'); >+ if ($no_refund_after_days) { >+ my $today = dt_from_string(); >+ my $lost_age_in_days = >+ dt_from_string( $item->itemlost_on ) >+ ->delta_days($today) >+ ->in_units('days'); >+ >+ $refund = 0 unless ( $lost_age_in_days < $no_refund_after_days ); >+ } >+ > if ( >+ $refund && > Koha::RefundLostItemFeeRules->should_refund( > { > current_branch => C4::Context->userenv->{branch}, >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index cfa295e197..410a212252 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -327,6 +327,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('noissuescharge','5','','Define maximum amount withstanding before check outs are blocked','Integer'), > ('NoIssuesChargeGuarantees','','','Define maximum amount withstanding before check outs are blocked','Integer'), > ('noItemTypeImages','0',NULL,'If ON, disables item-type images','YesNo'), >+('NoRefundOnLostReturnedItemsAge','','','Do not refund lost item fees if item is lost for more than this number of days','Integer'), > ('NoRenewalBeforePrecision','exact_time','date|exact_time','Calculate "No renewal before" based on date only or exact time of due date','Choice'), > ('NotesBlacklist','',NULL,'List of notes fields that should not appear in the title notes/description separator of details','free'), > ('NotHighlightedWords','and|or|not',NULL,'List of words to NOT highlight when OpacHitHighlight is enabled','free'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >index 6eb5af0660..a86b02a499 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >@@ -878,6 +878,11 @@ Circulation: > yes: Forgive > no: "Don't Forgive" > - the fines on an item when it is lost. >+ - >+ - "Don't refund lost fees if a lost item is returned more than" >+ - pref: NoRefundOnLostReturnedItemsAge >+ class: integer >+ - days after it was marked lost. > - > - pref: WhenLostChargeReplacementFee > choices: >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 38502941dd..80c579de34 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > use utf8; > >-use Test::More tests => 46; >+use Test::More tests => 50; > use Test::MockModule; > use Test::Deep qw( cmp_deeply ); > >@@ -3721,6 +3721,306 @@ subtest "Test Backdating of Returns" => sub { > is( $accountline->amount+0, 0, 'Fee amount was reduced to 0' ); > }; > >+subtest 'Tests for NoRefundOnLostReturnedItemsAge = undef' => sub { >+ plan tests => 3; >+ >+ t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); >+ t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', undef ); >+ >+ my $lost_on = dt_from_string->subtract( days => 7 )->date; >+ >+ my $library = $builder->build( { source => 'Branch' } ); >+ my $patron = $builder->build( >+ { >+ source => 'Borrower', >+ value => { categorycode => $patron_category->{categorycode} } >+ } >+ ); >+ >+ my $biblionumber = $builder->build_sample_biblio( >+ { >+ branchcode => $library->{branchcode}, >+ } >+ )->biblionumber; >+ my $item = $builder->build_sample_item( >+ { >+ biblionumber => $biblionumber, >+ library => $library->{branchcode}, >+ replacementprice => '42.00', >+ } >+ ); >+ >+ # And the circulation rule >+ Koha::CirculationRules->search->delete; >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ issuelength => 14, >+ lengthunit => 'days', >+ } >+ } >+ ); >+ $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1 >+ } >+ } >+ ); >+ >+ # Test with NoRefundOnLostReturnedItemsAge disabled >+ my $issue = AddIssue( $patron, $item->barcode ); >+ LostItem( $item->itemnumber, 'cli', 0 ); >+ $item->itemlost(1); >+ $item->itemlost_on( $lost_on ); >+ $item->store(); >+ >+ my $a = Koha::Account::Lines->find( >+ { >+ itemnumber => $item->id, >+ borrowernumber => $patron->{borrowernumber} >+ } >+ ); >+ ok( $a, "Found accountline for lost fee" ); >+ is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); >+ my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); >+ $a = Koha::Account::Lines->find( $a->id ); >+ is( $a->amountoutstanding, '0.000000', "Lost fee was refunded" ); >+}; >+ >+subtest 'Tests for NoRefundOnLostReturnedItemsAge > length of days item has been lost' => sub { >+ plan tests => 3; >+ >+ t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); >+ t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); >+ >+ my $lost_on = dt_from_string->subtract( days => 6 )->date; >+ >+ my $library = $builder->build( { source => 'Branch' } ); >+ my $patron = $builder->build( >+ { >+ source => 'Borrower', >+ value => { categorycode => $patron_category->{categorycode} } >+ } >+ ); >+ >+ my $biblionumber = $builder->build_sample_biblio( >+ { >+ branchcode => $library->{branchcode}, >+ } >+ )->biblionumber; >+ my $item = $builder->build_sample_item( >+ { >+ biblionumber => $biblionumber, >+ library => $library->{branchcode}, >+ replacementprice => '42.00', >+ } >+ ); >+ >+ # And the circulation rule >+ Koha::CirculationRules->search->delete; >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ issuelength => 14, >+ lengthunit => 'days', >+ } >+ } >+ ); >+ $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1 >+ } >+ } >+ ); >+ >+ # Test with NoRefundOnLostReturnedItemsAge disabled >+ my $issue = AddIssue( $patron, $item->barcode ); >+ LostItem( $item->itemnumber, 'cli', 0 ); >+ $item->itemlost(1); >+ $item->itemlost_on( $lost_on ); >+ $item->store(); >+ >+ my $a = Koha::Account::Lines->find( >+ { >+ itemnumber => $item->id, >+ borrowernumber => $patron->{borrowernumber} >+ } >+ ); >+ ok( $a, "Found accountline for lost fee" ); >+ is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); >+ my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); >+ $a = Koha::Account::Lines->find( $a->id ); >+ is( $a->amountoutstanding, '0.000000', "Lost fee was refunded" ); >+}; >+ >+subtest 'Tests for NoRefundOnLostReturnedItemsAge = length of days item has been lost' => sub { >+ plan tests => 3; >+ >+ t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); >+ t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); >+ >+ my $lost_on = dt_from_string->subtract( days => 7 )->date; >+ >+ my $library = $builder->build( { source => 'Branch' } ); >+ my $patron = $builder->build( >+ { >+ source => 'Borrower', >+ value => { categorycode => $patron_category->{categorycode} } >+ } >+ ); >+ >+ my $biblionumber = $builder->build_sample_biblio( >+ { >+ branchcode => $library->{branchcode}, >+ } >+ )->biblionumber; >+ my $item = $builder->build_sample_item( >+ { >+ biblionumber => $biblionumber, >+ library => $library->{branchcode}, >+ replacementprice => '42.00', >+ } >+ ); >+ >+ # And the circulation rule >+ Koha::CirculationRules->search->delete; >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ issuelength => 14, >+ lengthunit => 'days', >+ } >+ } >+ ); >+ $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1 >+ } >+ } >+ ); >+ >+ # Test with NoRefundOnLostReturnedItemsAge disabled >+ my $issue = AddIssue( $patron, $item->barcode ); >+ LostItem( $item->itemnumber, 'cli', 0 ); >+ $item->itemlost(1); >+ $item->itemlost_on( $lost_on ); >+ $item->store(); >+ >+ my $a = Koha::Account::Lines->find( >+ { >+ itemnumber => $item->id, >+ borrowernumber => $patron->{borrowernumber} >+ } >+ ); >+ ok( $a, "Found accountline for lost fee" ); >+ is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); >+ my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); >+ $a = Koha::Account::Lines->find( $a->id ); >+ is( $a->amountoutstanding, '42.000000', "Lost fee was not refunded" ); >+}; >+ >+subtest 'Tests for NoRefundOnLostReturnedItemsAge < length of days item has been lost' => sub { >+ plan tests => 3; >+ >+ t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); >+ t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); >+ >+ my $lost_on = dt_from_string->subtract( days => 8 )->date; >+ >+ my $library = $builder->build( { source => 'Branch' } ); >+ my $patron = $builder->build( >+ { >+ source => 'Borrower', >+ value => { categorycode => $patron_category->{categorycode} } >+ } >+ ); >+ >+ my $biblionumber = $builder->build_sample_biblio( >+ { >+ branchcode => $library->{branchcode}, >+ } >+ )->biblionumber; >+ my $item = $builder->build_sample_item( >+ { >+ biblionumber => $biblionumber, >+ library => $library->{branchcode}, >+ replacementprice => '42.00', >+ } >+ ); >+ >+ # And the circulation rule >+ Koha::CirculationRules->search->delete; >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ issuelength => 14, >+ lengthunit => 'days', >+ } >+ } >+ ); >+ $builder->build( >+ { >+ source => 'CirculationRule', >+ value => { >+ branchcode => undef, >+ categorycode => undef, >+ itemtype => undef, >+ rule_name => 'refund', >+ rule_value => 1 >+ } >+ } >+ ); >+ >+ # Test with NoRefundOnLostReturnedItemsAge disabled >+ my $issue = AddIssue( $patron, $item->barcode ); >+ LostItem( $item->itemnumber, 'cli', 0 ); >+ $item->itemlost(1); >+ $item->itemlost_on( $lost_on ); >+ $item->store(); >+ >+ my $a = Koha::Account::Lines->find( >+ { >+ itemnumber => $item->id, >+ borrowernumber => $patron->{borrowernumber} >+ } >+ ); >+ ok( $a, "Found accountline for lost fee" ); >+ is( $a->amountoutstanding, '42.000000', "Lost fee charged correctly" ); >+ my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->{branchcode}, undef, dt_from_string ); >+ $a = Koha::Account::Lines->find( $a->id ); >+ is( $a->amountoutstanding, '42.000000', "Lost fee was not refunded" ); >+}; >+ > $schema->storage->txn_rollback; > C4::Context->clear_syspref_cache(); > $cache->clear_from_cache('single_holidays'); >-- >2.21.1 (Apple Git-122.3)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=98772">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20815
:
98772
|
98929
|
99038
|
105141
|
106575
|
106606
|
106607
|
106608
|
107165
|
107177