Bugzilla – Attachment 105141 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), 16.54 KB, created by
Kyle M Hall (khall)
on 2020-05-20 11:10:15 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-05-20 11:10:15 UTC
Size:
16.54 KB
patch
obsolete
>From b0427c0430743354325e868b0b107e4f98a9db26 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 > >Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us> >--- > C4/Circulation.pm | 32 +- > .../data/mysql/atomicupdate/bug_20815.perl | 10 + > installer/data/mysql/sysprefs.sql | 1 + > .../admin/preferences/circulation.pref | 5 + > t/db_dependent/Circulation.t | 302 +++++++++++++++++- > 5 files changed, 346 insertions(+), 4 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_20815.perl > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index e439250933..f8b356bb72 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1447,11 +1447,24 @@ 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, > } > ) >@@ -2031,7 +2044,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/atomicupdate/bug_20815.perl b/installer/data/mysql/atomicupdate/bug_20815.perl >new file mode 100644 >index 0000000000..50c7ef54ce >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_20815.perl >@@ -0,0 +1,10 @@ >+$DBversion = 'XXX'; # will be replaced by the RM >+if( CheckVersion( $DBversion ) ) { >+ $dbh->do(q{ >+ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES >+ ('NoRefundOnLostReturnedItemsAge','','','Do not refund lost item fees if item is lost for more than this number of days','Integer') >+ }); >+ >+ SetVersion( $DBversion ); >+ print "Upgrade to $DBversion done (Bug XXXXX - description)\n"; >+} >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index 4ad73ef827..08d193b8aa 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -332,6 +332,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 itemtype images in the staff interface','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 92137896dc..b34cacf89b 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 >@@ -930,6 +930,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 e9f28b55d9..667a97f10f 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 => 48; >+use Test::More tests => 52; > use Test::MockModule; > use Test::Deep qw( cmp_deeply ); > >@@ -3926,6 +3926,306 @@ subtest 'Filling a hold should cancel existing transfer' => sub { > is( Koha::Item::Transfers->search({ itemnumber => $item->itemnumber, datearrived => undef })->count, 0, "No outstanding transfers when hold is waiting"); > }; > >+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->_result->itemlost(1); >+ $item->_result->itemlost_on( $lost_on ); >+ $item->_result->update(); >+ >+ 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->_result->itemlost(1); >+ $item->_result->itemlost_on( $lost_on ); >+ $item->_result->update(); >+ >+ 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->_result->itemlost(1); >+ $item->_result->itemlost_on( $lost_on ); >+ $item->_result->update(); >+ >+ 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->_result->itemlost(1); >+ $item->_result->itemlost_on( $lost_on ); >+ $item->_result->update(); >+ >+ 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.24.2 (Apple Git-127)
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=105141">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