Attachment 106606 Details for Bug 20815 – Bug 20815: Add ability to choose if lost fee is refunded based on length of time item has been lost

[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.61 KB, created by Katrin Fischer on 2020-07-06 21:04:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Katrin Fischer

Created: 2020-07-06 21:04:35 UTC

Size: 16.61 KB

patch

obsolete

>From 03fa734ea2b77e52f1d97c8121c12bea94c10d52 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>
>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>---
> C4/Circulation.pm                                  |  32 ++-
> installer/data/mysql/atomicupdate/bug_20815.perl   |  10 +
> installer/data/mysql/sysprefs.sql                  |   1 +
> .../en/modules/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 67a986b417..e1d6ff41e8 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1456,11 +1456,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,
>                         }
>                     )
>@@ -2040,7 +2053,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 d67a02b367..2bebabc5e2 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 f2379e797e..897db1c6c1 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
>@@ -937,6 +937,11 @@ Circulation:
>                   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:
>                   yes: Charge
>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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 20815: 98772 | 98929 | 99038 | 105141 | 106575 | 106606 | 106607 | 106608 | 107165 | 107177