Bugzilla – Attachment 188622 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), 10.80 KB, created by
Martin Renvoize (ashimema)
on 2025-10-30 10:54:56 UTC
(
hide
)
Description:
Bug 38940: Fix ConsiderLibraryHoursInCirculation when next day has no hours
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-10-30 10:54:56 UTC
Size:
10.80 KB
patch
obsolete
>From a89c0423282f4badee85de8f1631bb1ead4b8c16 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. 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 <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