Bugzilla – Attachment 74798 Details for
Bug 19204
Fines in days restriction calculation is not taking calendar into account
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19204: Make the debarment date calculation depends on finesCalendar
Bug-19204-Make-the-debarment-date-calculation-depe.patch (text/plain), 7.36 KB, created by
Tomás Cohen Arazi (tcohen)
on 2018-04-24 12:41:36 UTC
(
hide
)
Description:
Bug 19204: Make the debarment date calculation depends on finesCalendar
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2018-04-24 12:41:36 UTC
Size:
7.36 KB
patch
obsolete
>From 836f9becd8065399a9d24155e24eafd17b9e256a Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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 <claire.gravely@bsz-bw.de> > >Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > 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 c6dc6afacd..2deb3678a1 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 c66ab45092..fc63572b28 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 468fdfac37..b7f9d4fba9 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.17.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 19204
:
74726
|
74727
|
74728
|
74730
|
74733
|
74734
|
74735
|
74736
|
74777
|
74778
|
74779
|
74780
| 74798 |
74799
|
74800
|
74801