From 11800c3055014b9d67faa56dbb13475d8a86a6ae Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 23 Apr 2018 15:23:00 -0300 Subject: [PATCH] Bug 19204: Make the debarment date calculation depends on finesCalendar This patchs adds the ability to calculate the end of the suspension date (debarment date) using the finesCalendar syspref. Prior to this patch it was never calculating without taking into account the calendar. calculated without taking holidays into account. This was a problem because the restriction could end in the middle of a period the library is closed. Test plan: - Set finescalendar to 'not including days the library is closed' - Set a circulation condition with no fine/maxfine, but fine days and max fine days instead - Check out an item with a due date in the past - Check the item in and verify the restriction date - Clean the restriction - Add holidays to your calendar on the calculated restriction date - Check the item out again with the same due date in the past - Check in the item again - Verify the calculated restriction end date has changed, it's set to the day after the holiday. Fines in days restriction calculation is correctly taking calendar into account. Signed-off-by: Claire Gravely --- C4/Circulation.pm | 14 ++++++-- Koha/Calendar.pm | 2 +- t/db_dependent/Circulation.t | 77 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index c6dc6af..2deb367 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2246,8 +2246,18 @@ sub _debar_user_on_return { ); } - my $new_debar_dt = - $return_date->clone()->add_duration( $suspension_days ); + my $new_debar_dt; + # Use the calendar or not to calculate the debarment date + if ( C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed' ) { + my $calendar = Koha::Calendar->new( + branchcode => $branchcode, + days_mode => 'Calendar' + ); + $new_debar_dt = $calendar->addDate( $return_date, $suspension_days ); + } + else { + $new_debar_dt = $return_date->clone()->add_duration($suspension_days); + } Koha::Patron::Debarments::AddUniqueDebarment({ borrowernumber => $borrower->{borrowernumber}, diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index c66ab45..fc63572 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -47,7 +47,7 @@ sub _init { 1; } - $self->{days_mode} = C4::Context->preference('useDaysMode'); + $self->{days_mode} ||= C4::Context->preference('useDaysMode'); $self->{test} = 0; return; } diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 468fdfa..b7f9d4f 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -24,6 +24,7 @@ use POSIX qw( floor ); use t::lib::Mocks; use t::lib::TestBuilder; +use C4::Calendar; use C4::Circulation; use C4::Biblio; use C4::Items; @@ -48,6 +49,11 @@ my $dbh = C4::Context->dbh; # Start transaction $dbh->{RaiseError} = 1; +my $cache = Koha::Caches->get_instance(); +$dbh->do(q|DELETE FROM special_holidays|); +$dbh->do(q|DELETE FROM repeatable_holidays|); +$cache->clear_from_cache('single_holidays'); + # Start with a clean slate $dbh->do('DELETE FROM issues'); $dbh->do('DELETE FROM borrowers'); @@ -1758,7 +1764,7 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { }; subtest 'AddReturn + suspension_chargeperiod' => sub { - plan tests => 6; + plan tests => 8; my $library = $builder->build( { source => 'Branch' } ); my $patron = $builder->build( { source => 'Borrower', value => { categorycode => $patron_category->{categorycode} } } ); @@ -1863,9 +1869,72 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { Koha::Patron::Debarments::DelUniqueDebarment( { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); -}; + + # We want to charge 2 days every days, with 0 day of grace (to not burn brains) + $rule->finedays(2)->store; + $rule->suspension_chargeperiod(1)->store; + $rule->firstremind(0)->store; + t::lib::Mocks::mock_preference('finesCalendar', 'noFinesWhenClosed'); + + # Adding a holiday 2 days ago + my $calendar = C4::Calendar->new(branchcode => $library->{branchcode}); + my $two_days_ago = dt_from_string->subtract( days => 2 ); + $calendar->insert_single_holiday( + day => $two_days_ago->day, + month => $two_days_ago->month, + year => $two_days_ago->year, + title => 'holidayTest-2d', + description => 'holidayDesc 2 days ago' + ); + + # With 5 days of overdue, only 4 (x finedays=2) days must charged (one was an holiday) + AddIssue( $patron, $item_1->{barcode}, $five_days_ago ); # Add an overdue + + AddReturn( $item_1->{barcode}, $library->{branchcode}, + undef, undef, dt_from_string ); + $debarments = Koha::Patron::Debarments::GetDebarments( + { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); + $expected_expiration = output_pref( + { + dt => dt_from_string->add( days => floor( ( ( 5 - 0 - 1 ) / 1 ) * 2 ) ), + dateformat => 'sql', + dateonly => 1 + } + ); + is( $debarments->[0]->{expiration}, $expected_expiration ); + Koha::Patron::Debarments::DelUniqueDebarment( + { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); + my $two_days_ahead = dt_from_string->add( days => 2 ); + $calendar->insert_single_holiday( + day => $two_days_ahead->day, + month => $two_days_ahead->month, + year => $two_days_ahead->year, + title => 'holidayTest+2d', + description => 'holidayDesc 2 days ahead' + ); + + # Same as above, but we should skip D+2 + AddIssue( $patron, $item_1->{barcode}, $five_days_ago ); # Add an overdue + + AddReturn( $item_1->{barcode}, $library->{branchcode}, + undef, undef, dt_from_string ); + $debarments = Koha::Patron::Debarments::GetDebarments( + { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); + $expected_expiration = output_pref( + { + dt => dt_from_string->add( days => floor( ( ( 5 - 0 - 1 ) / 1 ) * 2 ) + 1 ), + dateformat => 'sql', + dateonly => 1 + } + ); + is( $debarments->[0]->{expiration}, $expected_expiration ); + Koha::Patron::Debarments::DelUniqueDebarment( + { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); + +}; + subtest 'AddReturn | is_overdue' => sub { plan tests => 5; @@ -2163,6 +2232,10 @@ subtest 'CanBookBeIssued | is_overdue' => sub { }; + +$schema->storage->txn_rollback; +$cache->clear_from_cache('single_holidays'); + sub set_userenv { my ( $library ) = @_; C4::Context->set_userenv(0,0,0,'firstname','surname', $library->{branchcode}, $library->{branchname}, '', '', ''); -- 2.1.4