From a89c0423282f4badee85de8f1631bb1ead4b8c16 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 30 Oct 2025 10:51:20 +0000 Subject: [PATCH] Bug 38940: Fix ConsiderLibraryHoursInCirculation when next day has no hours When ConsiderLibraryHoursInCirculation is set to "shorten the loan period and set the checkout to be due at the library's close time", the feature fails to work if the next day has null/blank library hours (e.g., weekends or closed days). The code was incorrectly forcing the system to ignore library hours entirely whenever EITHER today's hours OR tomorrow's hours were undefined. This was too restrictive because: - The 'close' option only needs today's closing time - The 'open' option needs tomorrow's opening time This patch modifies the logic to: 1. Only ignore library hours if today's hours are missing 2. For 'open' mode, require tomorrow's hours to extend the loan 3. For 'close' mode, allow it to work with just today's hours 4. Gracefully handle null tomorrow hours in both code paths Test plan: 1. Apply patch and run tests: prove t/db_dependent/Circulation/CalcDateDue.t prove t/db_dependent/Circulation.t 2. Set up a library with operating hours in Tools > Libraries and hours > [Your library] - Monday-Friday: 09:00-17:00 - Saturday-Sunday: Leave blank (no hours defined) 3. Set system preferences: - ConsiderLibraryHoursInCirculation: shorten the loan period - useDaysMode: Use the calendar to skip days the library is closed 4. Create a circulation rule with hourly loan: - Go to Administration > Circulation and fine rules - Set loan period to 4 hours for a patron category/itemtype 5. On a Friday at 2:00 PM, check out an item with the 4-hour loan rule - Without patch: Due date would be Friday 6:00 PM (ignoring library hours because Saturday has no hours) - With patch: Due date should be Friday 5:00 PM (correctly shortened to library closing time) 6. Verify that checking out at 3:00 PM still shortens to 5:00 PM close 7. Test with ConsiderLibraryHoursInCirculation set to "extend to next opening time" - Checkout should fall back to standard loan duration when next day has no hours (since we can't extend to an undefined opening time) NOTES FOR REVIEW: When ConsiderLibraryHoursInCirculation is set to "extend the loan period to the library's next opening time" and the next day has no defined hours, this patch falls back to using the standard loan duration (ignoring library hours). Question for reviewers: Should we instead roll forward to find the next day that HAS defined opening hours? For example, if checking out Friday evening with the library closed Saturday/Sunday, should we extend the due date to Monday's opening time rather than falling back to ignoring library hours? Sponsored-by: Open Fifth --- C4/Circulation.pm | 28 ++++++----- t/db_dependent/Circulation/CalcDateDue.t | 62 +++++++++++++++++++++++- 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 0a8df342638..42bfeaf5b04 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -4158,15 +4158,23 @@ sub CalcDateDue { ; # get open hours of next day # Defend against missing library hours definitions - if ( !$todayhours || !$tomorrowhours ) { $considerlibraryhours = 'ignore' } + # For 'close' mode, we only need today's hours + # For 'open' mode, we need both today's and tomorrow's hours + if ( !$todayhours ) { + $considerlibraryhours = 'ignore'; + } elsif ( $considerlibraryhours eq 'open' && !$tomorrowhours ) { + $considerlibraryhours = 'ignore'; + } my @open = undef; - if ( $considerlibraryhours ne 'ignore' and $todayhours->close_time and $tomorrowhours->open_time ) { + 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 - @open = split( ":", $tomorrowhours->open_time ); + if ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { + @open = split( ":", $tomorrowhours->open_time ); + } } # calculate the datedue as normal @@ -4174,22 +4182,21 @@ sub CalcDateDue { if ( $loanlength->{lengthunit} eq 'hours' ) { if ( $considerlibraryhours ne 'ignore' and $potential_datedue > $library_close - and $todayhours->close_time - and $tomorrowhours->open_time ) + and $todayhours->close_time ) { if ( $considerlibraryhours eq 'close' ) { # 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' ) { + } elsif ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { # 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] ); } else { - # ignore library open hours + # ignore library open hours or can't extend to tomorrow (no tomorrow hours) $datedue->add( hours => $loanlength->{$length_key} ); } } else { @@ -4207,15 +4214,14 @@ sub CalcDateDue { if ( $loanlength->{lengthunit} eq 'hours' ) { if ( $considerlibraryhours ne 'ignore' and $potential_datedue > $library_close - and $todayhours->close_time - and $tomorrowhours->open_time ) + and $todayhours->close_time ) { if ( $considerlibraryhours eq 'close' ) { # 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' ) { + } elsif ( $considerlibraryhours eq 'open' and $tomorrowhours and $tomorrowhours->open_time ) { # 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 @@ -4223,7 +4229,7 @@ sub CalcDateDue { $datedue->set( hour => $open[0], minute => $open[1] ); } else { - # ignore library open hours + # ignore library open hours or can't extend to tomorrow (no tomorrow hours) $dur = DateTime::Duration->new( hours => $loanlength->{$length_key} ); } } else { diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t index 00e3c99a2ca..2a1b917327d 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 => 24; +use Test::More tests => 27; use Test::MockModule; use DBI; use DateTime; @@ -492,5 +492,65 @@ is( "Loan period was extended (but considers the holiday) because ConsiderLibraryHoursInCirculation is set to open time" ); +# Bug 38940 - Test that ConsiderLibraryHoursInCirculation='close' works when tomorrow has no hours +# This tests the scenario where a library is closed the next day (e.g., weekend) +# but should still shorten loans to today's closing time + +my $library2 = $builder->build( { source => 'Branch' } ); +Koha::CirculationRules->set_rules( + { + categorycode => $borrower->categorycode, + itemtype => $itemtype, + branchcode => $library2->{branchcode}, + rules => { + issuelength => 3, # loan period is 3 hours + lengthunit => 'hours', + daysmode => '', + } + } +); + +# Set up hours for Friday (day 5): open 10:00, close 16:00 +# Saturday (day 6) will have NO hours (null), simulating a weekend +my $friday_open = DateTime->new( year => 2023, month => 5, day => 5, hour => 10 )->hms; +my $friday_close = DateTime->new( year => 2023, month => 5, day => 5, hour => 16 )->hms; +my $friday_now = DateTime->new( year => 2023, month => 5, day => 5, hour => 14 ); # 2PM on Friday + +# Only set hours for Friday (day 5), leaving Saturday (day 6) with null hours +Koha::Library::Hour->new( + { day => 5, library_id => $library2->{branchcode}, open_time => $friday_open, close_time => $friday_close } ) + ->store; + +t::lib::Mocks::mock_preference( 'useDaysMode', 'Days' ); +t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'close' ); + +# Test with 'close' mode - should shorten to 4PM today even though tomorrow has no hours +$date = C4::Circulation::CalcDateDue( $friday_now, $itemtype, $library2->{branchcode}, $borrower ); +$expected_duetime = $friday_now->clone->set( hour => 16, minute => 0 ); +is( + $date, $expected_duetime, + "Bug 38940: Loan period shortened to closing time when next day has null hours (ConsiderLibraryHoursInCirculation='close')" +); + +# Test with 'open' mode - should fall back to regular 3-hour loan since tomorrow has no hours +t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); +$date = C4::Circulation::CalcDateDue( $friday_now, $itemtype, $library2->{branchcode}, $borrower ); +$expected_duetime = $friday_now->clone->add( hours => 3 ); +is( + $date, $expected_duetime, + "Bug 38940: Loan period not extended when next day has null hours (ConsiderLibraryHoursInCirculation='open' falls back to standard loan)" +); + +# Test the same scenario with Calendar daysmode +t::lib::Mocks::mock_preference( 'useDaysMode', 'Calendar' ); +t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'close' ); + +$date = C4::Circulation::CalcDateDue( $friday_now, $itemtype, $library2->{branchcode}, $borrower ); +$expected_duetime = $friday_now->clone->set( hour => 16, minute => 0 ); +is( + $date, $expected_duetime, + "Bug 38940: Loan period shortened with Calendar mode when next day has null hours (ConsiderLibraryHoursInCirculation='close')" +); + $cache->clear_from_cache($key); $schema->storage->txn_rollback; -- 2.51.1