From e872b68d245b7dc101fd2c73be33ac2f59285353 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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