Bugzilla – Attachment 189155 Details for
Bug 38940
ConsiderLibraryHoursInCirculation set to "shorten the loan period and set the checkout to be due at the library’s close time" is not working
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 38940: Consider calendar holidays when extending loans to next opening time
Bug-38940-Consider-calendar-holidays-when-extendin.patch (text/plain), 18.94 KB, created by
Martin Renvoize (ashimema)
on 2025-11-06 10:59:07 UTC
(
hide
)
Description:
Bug 38940: Consider calendar holidays when extending loans to next opening time
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-11-06 10:59:07 UTC
Size:
18.94 KB
patch
obsolete
>From e872b68d245b7dc101fd2c73be33ac2f59285353 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Thu, 6 Nov 2025 10:51:36 +0000 >Subject: [PATCH] Bug 38940: Consider calendar holidays when extending loans to > next opening time > >This follow-up patch addresses a limitation in the previous fix where >ConsiderLibraryHoursInCirculation='open' mode would extend loans to >tomorrow's opening time even when tomorrow was marked as a holiday in >the calendar. > >The issue: A library might have operating hours defined for every day of >the week (e.g., Monday-Sunday 09:00-17:00), but have specific days >marked as holidays in the calendar (e.g., public holidays, special >closures). The previous code only checked if operating hours were >defined, not whether the library was actually open according to the >calendar. > >This patch modifies the logic to: >1. When in 'open' mode with Calendar daysmode, check if tomorrow is a > holiday in the calendar >2. If tomorrow is a holiday, look forward up to 7 days to find the next > day that is: > - Not marked as a holiday in the calendar AND > - Has defined operating hours >3. Extend the loan to that day's opening time >4. If no suitable day is found within 7 days, fall back to standard > loan duration (ignoring library hours) > >Note: 'close' mode is unaffected since we only need today's closing >time, and checkouts can only happen on open days. > >Test plan: >1. Apply patch and run tests: > prove t/db_dependent/Circulation/CalcDateDue.t > >2. Note the current time and day of week for your testing. > Configure Koha based on the current time to test the functionality. > >3. Configure library operating hours at Administration > Libraries > [Select your library] > Hours > > Set operating hours for multiple consecutive days: > - TODAY (your current day): Set hours that include the current time > Example: If it's currently 14:30, set hours like 09:00-17:00 > - TOMORROW: Set hours (e.g., 10:00-18:00) > - DAY AFTER TOMORROW: Set hours (e.g., 10:00-18:00) > - THIRD DAY: Set hours (e.g., 10:00-18:00) > > Example if testing on Friday at 14:30: > - Friday: 09:00-17:00 (must include current time) > - Saturday: 10:00-18:00 > - Sunday: 10:00-18:00 > - Monday: 10:00-18:00 > >4. Mark tomorrow as a holiday at Tools > Calendar > [Select your library] > - Click "New holiday" or "Holidays repeating weekly" > - If testing on Friday, mark Saturday as a single-day holiday > - Save the holiday > > Note: This creates a scenario where tomorrow HAS operating hours > defined, but is marked as closed in the calendar (e.g., a public > holiday that falls on a normally-open day) > >5. Set system preferences: > - ConsiderLibraryHoursInCirculation: "extend the loan period to the library's next opening time" > - useDaysMode: "Use the calendar to skip days the library is closed" > >6. Create a circulation rule with hourly loan period: > - Go to Administration > Circulation and fine rules > - Set loan period in hours that would extend past today's closing time > - Calculate: (Closing time - Current time) + 1 hour minimum > Example: If it's 14:30 now and library closes at 17:00, set loan > period to 4 or 5 hours > >7. Test the 'open' behavior with calendar holidays: > - Check out an item using the hourly loan rule > > Without patch: Due date/time would extend to tomorrow's opening time > (e.g., Saturday 10:00), even though Saturday is a holiday > > With patch: Due date/time should extend to the next open day's opening > time (e.g., if Saturday is a holiday, should extend to > Sunday 10:00, or if Sunday is also a holiday, to Monday 10:00) > >8. Verify the behavior: > - The due date should skip over the calendar holiday > - Check out multiple items - they should all have the same due time > (next open day's opening time) > >9. Test with multiple consecutive holidays: > - Mark both TOMORROW and DAY AFTER as holidays in the calendar > - Check out an item > - Should extend to THIRD DAY's opening time (first non-holiday day > with defined hours) > >10. Test fallback behavior: > - Configure library with NO operating hours for any day after today > - Mark tomorrow as a holiday > - Check out should fall back to standard loan duration since there's > no suitable day to extend to > >11. Test with 'close' mode to verify it's unaffected: > - Change ConsiderLibraryHoursInCirculation to "shorten the loan > period to end when the library closes" > - Check out an item > - Should still shorten to today's closing time (calendar holidays > don't affect this mode since we only need today's hours) > >NOTES FOR REVIEW: > >This patch looks ahead up to 7 days to find a suitable open day. This >limit prevents infinite loops and matches typical library closure >patterns (weekend + public holiday = 3-4 days max). > >When no suitable day is found within 7 days, the system falls back to >standard loan duration, which then uses the calendar's normal day- >skipping logic when the loan comes due. > >This patch only affects Calendar daysmode. When using Days mode (ignore >calendar), the behavior is unchanged from the previous patch. >--- > C4/Circulation.pm | 68 ++++++++- > t/db_dependent/Circulation/CalcDateDue.t | 175 ++++++++++++++++++++++- > 2 files changed, 235 insertions(+), 8 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 42bfeaf5b04..2fd437038bf 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -4166,13 +4166,49 @@ sub CalcDateDue { > $considerlibraryhours = 'ignore'; > } > >+ # For 'open' mode, if tomorrow is a calendar holiday, find the next open day >+ my $next_open_day; >+ my $next_open_hours; >+ if ( $considerlibraryhours eq 'open' && $daysmode ne 'Days' ) { >+ my $calendar = Koha::Calendar->new( branchcode => $branch, days_mode => $daysmode ); >+ my $check_day = $datedue->clone->add( days => 1 ); >+ my $days_forward = 1; >+ >+ # Look ahead up to 7 days to find next open day with defined hours >+ while ( $days_forward <= 7 ) { >+ if ( !$calendar->is_holiday($check_day) ) { >+ >+ # This day is not a holiday, check if it has operating hours >+ my $check_dayofweek = $check_day->day_of_week; >+ if ( $check_dayofweek == 7 ) { >+ $check_dayofweek = 0; >+ } >+ my $check_hours = Koha::Library::Hours->find( { library_id => $branch, day => $check_dayofweek } ); >+ if ( $check_hours && $check_hours->open_time ) { >+ $next_open_day = $check_day; >+ $next_open_hours = $check_hours; >+ last; >+ } >+ } >+ $check_day->add( days => 1 ); >+ $days_forward++; >+ } >+ >+ # If we couldn't find a suitable day, fall back to ignoring library hours >+ if ( !$next_open_day ) { >+ $considerlibraryhours = 'ignore'; >+ } >+ } >+ > my @open = undef; > if ( $considerlibraryhours ne 'ignore' and $todayhours and $todayhours->close_time ) { > @close = split( ":", $todayhours->close_time ); > $library_close = $library_close->set( hour => $close[0], minute => $close[1] ); > $potential_datedue = $potential_datedue->add( hours => $loanlength->{$length_key} ) > ; # datedue without consideration for open hours >- if ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { >+ if ( $considerlibraryhours eq 'open' and $next_open_hours and $next_open_hours->open_time ) { >+ @open = split( ":", $next_open_hours->open_time ); >+ } elsif ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { > @open = split( ":", $tomorrowhours->open_time ); > } > } >@@ -4189,11 +4225,20 @@ sub CalcDateDue { > # datedue will be after the library closes on that day > # shorten loan period to end when library closes > $datedue->set( hour => $close[0], minute => $close[1] ); >- } elsif ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { >+ } elsif ( $considerlibraryhours eq 'open' and @open ) { > > # datedue will be after the library closes on that day >- # extend loan period to when library opens following day >- $datedue->add( days => 1 )->set( hour => $open[0], minute => $open[1] ); >+ # extend loan period to when library opens on next open day >+ if ($next_open_day) { >+ >+ # Use the calculated next open day (accounting for calendar holidays) >+ my $days_to_add = $next_open_day->delta_days($datedue)->in_units('days'); >+ $datedue->add( days => $days_to_add )->set( hour => $open[0], minute => $open[1] ); >+ } else { >+ >+ # Tomorrow is open and not a holiday >+ $datedue->add( days => 1 )->set( hour => $open[0], minute => $open[1] ); >+ } > } else { > > # ignore library open hours or can't extend to tomorrow (no tomorrow hours) >@@ -4221,11 +4266,20 @@ sub CalcDateDue { > # datedue will be after the library closes on that day > # shorten loan period to end when library closes by hardcoding due time > $datedue->set( hour => $close[0], minute => $close[1] ); >- } elsif ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { >+ } elsif ( $considerlibraryhours eq 'open' and @open ) { > > # datedue will be after the library closes on that day >- # extend loan period to when library opens following day by hardcoding due time for next open day >- $dur = DateTime::Duration->new( days => 1 ); >+ # extend loan period to when library opens on next open day by hardcoding due time for next open day >+ if ($next_open_day) { >+ >+ # Use the calculated next open day (accounting for calendar holidays) >+ my $days_to_add = $next_open_day->delta_days($datedue)->in_units('days'); >+ $dur = DateTime::Duration->new( days => $days_to_add ); >+ } else { >+ >+ # Tomorrow is open and not a holiday >+ $dur = DateTime::Duration->new( days => 1 ); >+ } > $datedue->set( hour => $open[0], minute => $open[1] ); > } else { > >diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t >index 2a1b917327d..0cdc4a60d0b 100755 >--- a/t/db_dependent/Circulation/CalcDateDue.t >+++ b/t/db_dependent/Circulation/CalcDateDue.t >@@ -3,7 +3,7 @@ > use Modern::Perl; > > use Test::NoWarnings; >-use Test::More tests => 27; >+use Test::More tests => 32; > use Test::MockModule; > use DBI; > use DateTime; >@@ -552,5 +552,178 @@ is( > "Bug 38940: Loan period shortened with Calendar mode when next day has null hours (ConsiderLibraryHoursInCirculation='close')" > ); > >+# Bug 38940 follow-up - Test that ConsiderLibraryHoursInCirculation='open' extends to next open day >+# when tomorrow is marked as a holiday in the calendar (even though it has operating hours defined) >+ >+my $library3 = $builder->build( { source => 'Branch' } ); >+Koha::CirculationRules->set_rules( >+ { >+ categorycode => $borrower->categorycode, >+ itemtype => $itemtype, >+ branchcode => $library3->{branchcode}, >+ rules => { >+ issuelength => 4, # loan period is 4 hours >+ lengthunit => 'hours', >+ daysmode => '', >+ } >+ } >+); >+ >+# Set up hours for Friday (day 5), Saturday (day 6), and Monday (day 1) >+# Friday: 09:00-17:00, Saturday: 09:00-17:00, Monday: 09:00-17:00 >+my $fri_open = DateTime->new( year => 2023, month => 5, day => 5, hour => 10 )->hms; >+my $fri_close = DateTime->new( year => 2023, month => 5, day => 5, hour => 17 )->hms; >+my $sat_open = DateTime->new( year => 2023, month => 5, day => 6, hour => 10 )->hms; >+my $sat_close = DateTime->new( year => 2023, month => 5, day => 6, hour => 17 )->hms; >+my $mon_open = DateTime->new( year => 2023, month => 5, day => 1, hour => 10 )->hms; >+my $mon_close = DateTime->new( year => 2023, month => 5, day => 1, hour => 17 )->hms; >+my $friday_14 = DateTime->new( year => 2023, month => 5, day => 5, hour => 14 ); # 2PM on Friday >+ >+# Set operating hours for Friday (day 5), Saturday (day 6), and Monday (day 1) >+Koha::Library::Hour->new( >+ { day => 5, library_id => $library3->{branchcode}, open_time => $fri_open, close_time => $fri_close } )->store; >+Koha::Library::Hour->new( >+ { day => 6, library_id => $library3->{branchcode}, open_time => $sat_open, close_time => $sat_close } )->store; >+Koha::Library::Hour->new( >+ { day => 1, library_id => $library3->{branchcode}, open_time => $mon_open, close_time => $mon_close } )->store; >+ >+# Mark Saturday and Sunday as holidays in the calendar >+my $calendar3 = C4::Calendar->new( branchcode => $library3->{branchcode} ); >+$calendar3->insert_week_day_holiday( >+ weekday => 6, >+ title => 'Saturday', >+ description => 'Saturday closure' >+); >+$calendar3->insert_week_day_holiday( >+ weekday => 0, >+ title => 'Sunday', >+ description => 'Sunday closure' >+); >+ >+t::lib::Mocks::mock_preference( 'useDaysMode', 'Calendar' ); >+t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); >+ >+# Test with 'open' mode - should extend to Monday's opening time (skipping Saturday and Sunday holidays) >+$date = C4::Circulation::CalcDateDue( $friday_14, $itemtype, $library3->{branchcode}, $borrower ); >+$expected_duetime = DateTime->new( year => 2023, month => 5, day => 8, hour => 10, minute => 0 ); >+is( >+ $date, $expected_duetime, >+ "Bug 38940 follow-up: Loan period extended to Monday opening when Saturday/Sunday are calendar holidays (ConsiderLibraryHoursInCirculation='open')" >+); >+ >+# Test with 'close' mode - should still shorten to Friday's closing time (calendar holidays don't affect today) >+t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'close' ); >+$date = C4::Circulation::CalcDateDue( $friday_14, $itemtype, $library3->{branchcode}, $borrower ); >+$expected_duetime = $friday_14->clone->set( hour => 17, minute => 0 ); >+is( >+ $date, $expected_duetime, >+ "Bug 38940 follow-up: Loan period shortened to closing time, unaffected by weekend holidays (ConsiderLibraryHoursInCirculation='close')" >+); >+ >+# Test the same with Days mode (ignoring calendar) - should use tomorrow's hours directly >+t::lib::Mocks::mock_preference( 'useDaysMode', 'Days' ); >+t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); >+$date = C4::Circulation::CalcDateDue( $friday_14, $itemtype, $library3->{branchcode}, $borrower ); >+$expected_duetime = DateTime->new( year => 2023, month => 5, day => 6, hour => 10, minute => 0 ); >+is( >+ $date, $expected_duetime, >+ "Bug 38940 follow-up: In Days mode, extends to Saturday (ignoring calendar holidays)" >+); >+ >+# Test scenario where next open day has no operating hours - should fall back to ignoring library hours >+my $library4 = $builder->build( { source => 'Branch' } ); >+Koha::CirculationRules->set_rules( >+ { >+ categorycode => $borrower->categorycode, >+ itemtype => $itemtype, >+ branchcode => $library4->{branchcode}, >+ rules => { >+ issuelength => 4, >+ lengthunit => 'hours', >+ daysmode => '', >+ } >+ } >+); >+ >+# Set up hours only for Friday (day 5) >+Koha::Library::Hour->new( >+ { day => 5, library_id => $library4->{branchcode}, open_time => $fri_open, close_time => $fri_close } )->store; >+ >+# Mark Saturday and Sunday as holidays, and don't define hours for Monday >+my $calendar4 = C4::Calendar->new( branchcode => $library4->{branchcode} ); >+$calendar4->insert_week_day_holiday( >+ weekday => 6, >+ title => 'Saturday', >+ description => 'Saturday closure' >+); >+$calendar4->insert_week_day_holiday( >+ weekday => 0, >+ title => 'Sunday', >+ description => 'Sunday closure' >+); >+ >+t::lib::Mocks::mock_preference( 'useDaysMode', 'Calendar' ); >+t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); >+ >+# Should fall back to standard loan since no suitable open day with hours found >+$date = C4::Circulation::CalcDateDue( $friday_14, $itemtype, $library4->{branchcode}, $borrower ); >+$expected_duetime = $friday_14->clone->add( hours => 4 ); >+is( >+ $date, $expected_duetime, >+ "Bug 38940 follow-up: Falls back to standard loan when next open day has no operating hours" >+); >+ >+# Test with a single day holiday (not weekly repeating) >+my $library5 = $builder->build( { source => 'Branch' } ); >+Koha::CirculationRules->set_rules( >+ { >+ categorycode => $borrower->categorycode, >+ itemtype => $itemtype, >+ branchcode => $library5->{branchcode}, >+ rules => { >+ issuelength => 4, >+ lengthunit => 'hours', >+ daysmode => '', >+ } >+ } >+); >+ >+# Set hours for all days of the week (0-6) >+for my $day ( 0 .. 6 ) { >+ Koha::Library::Hour->new( >+ { day => $day, library_id => $library5->{branchcode}, open_time => $fri_open, close_time => $fri_close } ) >+ ->store; >+} >+ >+# Mark Saturday May 6, 2023 and Sunday May 7, 2023 as single day holidays >+my $calendar5 = C4::Calendar->new( branchcode => $library5->{branchcode} ); >+my $specific_saturday = DateTime->new( year => 2023, month => 5, day => 6 ); >+$calendar5->insert_single_holiday( >+ day => $specific_saturday->day, >+ month => $specific_saturday->month, >+ year => $specific_saturday->year, >+ title => 'Public Holiday', >+ description => 'Special bank holiday' >+); >+my $specific_sunday = DateTime->new( year => 2023, month => 5, day => 7 ); >+$calendar5->insert_single_holiday( >+ day => $specific_sunday->day, >+ month => $specific_sunday->month, >+ year => $specific_sunday->year, >+ title => 'Sunday', >+ description => 'Sunday closure' >+); >+ >+t::lib::Mocks::mock_preference( 'useDaysMode', 'Calendar' ); >+t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); >+ >+# Should extend to Monday (skipping Saturday public holiday and Sunday which has no hours) >+$date = C4::Circulation::CalcDateDue( $friday_14, $itemtype, $library5->{branchcode}, $borrower ); >+$expected_duetime = DateTime->new( year => 2023, month => 5, day => 8, hour => 10, minute => 0 ); >+is( >+ $date, $expected_duetime, >+ "Bug 38940 follow-up: Extends to Monday when Saturday is single-day holiday" >+); >+ > $cache->clear_from_cache($key); > $schema->storage->txn_rollback; >-- >2.51.1
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 38940
:
188622
|
189154
| 189155