Bugzilla – Attachment 189154 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: Fix ConsiderLibraryHoursInCirculation when next day has no hours
Bug-38940-Fix-ConsiderLibraryHoursInCirculation-wh.patch (text/plain), 12.50 KB, created by
Martin Renvoize (ashimema)
on 2025-11-06 10:59:04 UTC
(
hide
)
Description:
Bug 38940: Fix ConsiderLibraryHoursInCirculation when next day has no hours
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-11-06 10:59:04 UTC
Size:
12.50 KB
patch
obsolete
>From 8380935ef5e19c3549d79df4b2cc397a2a4cf1db Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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 <https://openfith.co.uk> >--- > 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
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