From 8380935ef5e19c3549d79df4b2cc397a2a4cf1db 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. Note the current time and day of week for your testing. You will configure Koha's library operating hours and circulation rules based on the current time to trigger the bug conditions. 3. Configure library operating hours at Administration > Libraries > [Select your library] > Hours Set operating hours based on today's day of week: - 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: Leave completely blank (no hours defined at all) This is the key configuration that triggers the bug! - Other days: Can be set normally or left blank (doesn't affect this test) Example if testing on Tuesday at 14:30: - Tuesday: 09:00-17:00 (must include current time of 14:30) - Wednesday: Leave completely blank (tomorrow has no hours - this triggers the bug) - Monday, Thursday-Sunday: Set normally or leave blank (doesn't matter) 4. Set system preferences: - ConsiderLibraryHoursInCirculation: "shorten the loan period to end when the library closes" - useDaysMode: Can be set to either "Days" or "Calendar" (both code paths are affected) 5. 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 so the loan would normally extend past 17:00 6. Test the 'close' behavior (shorten to closing time): - Check out an item using the hourly loan rule you created Without patch: Due date/time would extend well past closing time (ignoring library hours because tomorrow has no hours defined) Example: 14:30 + 4 hours = 18:30 (ignoring 17:00 close) With patch: Due date/time should be shortened to today's closing time Example: 14:30 + 4 hour loan = 18:30, but shortened to 17:00 7. Verify the behavior is consistent: - The due date/time should match today's closing time exactly - Try checking out multiple items - all should have the same due time (today's closing time) 8. Test with ConsiderLibraryHoursInCirculation set to "extend to next opening time": - Change the system preference to "extend the loan period to the library's next opening time" - Check out an item with the same hourly loan rule - Since tomorrow has no defined operating hours, the system cannot determine when to extend the loan to - The checkout should fall back to standard loan duration (ignoring library hours) - Example: 14:30 + 4 hour loan = 18:30 (ignoring the 17:00 closing time, since we cannot extend to tomorrow's 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