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
Summary: ConsiderLibraryHoursInCirculation set to "shorten the loan period and set the...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-01-21 16:40 UTC by Sara Brown
Modified: 2025-11-06 11:01 UTC (History)
9 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38940: Fix ConsiderLibraryHoursInCirculation when next day has no hours (10.80 KB, patch)
2025-10-30 10:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38940: Fix ConsiderLibraryHoursInCirculation when next day has no hours (12.50 KB, patch)
2025-11-06 10:59 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38940: Consider calendar holidays when extending loans to next opening time (18.94 KB, patch)
2025-11-06 10:59 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Sara Brown 2025-01-21 16:40:44 UTC
If I have ConsiderLibraryHoursInCirculation set to "shorten the loan period and set the checkout to be due at the library’s close time" and check out an item whose loan period should be shortened to account for closing hours, the loan period isn't being shortened as expected. To replicate:

-Set ConsiderLibraryHoursInCirculation to "shorten the loan period and set the checkout to be due at the library’s close time"
-Set Library A's hours from any opening time (I used 8a) to 12p
-Set a circ rule for all patron types/test item type for hourly loans using the number of hours that would go past your closing time (test: 3 hours)
-Check out the test item type at a time when the loan period should be shortened (test: 11.30a)
-Note that the loan period is not shortened (test: 2.30p) despite the closing time and system preference
Comment 1 Katrin Fischer 2025-01-21 16:47:46 UTC
I wonder if circulation library / home library /patron library or similar could be at play here. Can you share if they have all been the same?
Comment 2 Koha collecto 2025-02-07 15:37:57 UTC
Hi ! Are all of your open time et close time filled ? 

I was having the same issue but my saturday and sunday were "null" since the library is closed on the weekend. I then filled the hours for the two days and I was able to make it work.

Tho there's still a problem with that because I've put 00:00 in both the open time and close time thinking that Koha would understand that the library is not open on that day, but when I tried the "extend the loan period and set the checkout to be due at the library's open time" option my loan period was extended to saturday 00:00 wich is not wat I want.
Comment 3 Raina 2025-03-11 16:54:09 UTC
We are seeing this work (loan period shortens successfully to closing time) for the most part but can replicate a scenario in which it doesn't work, if the next day's hours are null for the library. Before testing, I confirmed that there was a corresponding circ rule for this library, item type, and patron category.

- Set ConsiderLibraryHoursInCirculation to shorten 
- Set library's hours on current day (e.g. Monday, 8 a.m. to 5 p.m.), but leave the next day with null hours (e.g. Tuesday, leave blank)
- Check out an item with hourly loan period where the loan period should shorten (e.g. check out 4 hour loan at 4:00 p.m.).
- Loan did not shorten, due at 8 p.m.

When I repeated the steps above but entered hours for the current day and the next day (e.g. Monday AND Tuesday), the 4-hour loan checked out at 4:00 p.m. shortened as expected and was due at closing time, 5 p.m.
Comment 4 Ray Delahunty 2025-06-10 13:03:49 UTC
We are extensively testing loan shortening in 24.11 to try to get it to work with how we have to configure circcontrol (to keep max loans entitlement under control in our multi-site setup given Bug 27834) and are replicating what Raina reports. And while an attempt to trick Koha by adding in a 5 minute opening time at 03:00 on the closed day tomorrow appears to work (the loan period is shortened correctly today) Koha requires the staff member to manually approve the loan, so I don't see our laptops loans units and self issue kiosks tolerating that.
Comment 5 Martin Renvoize (ashimema) 2025-10-30 10:54:56 UTC Comment hidden (obsolete)
Comment 6 David Nind 2025-11-02 03:57:53 UTC
At a loss on how to do step 5. Tried setting server date and time, but I couldn't get it to change.
Comment 7 Martin Renvoize (ashimema) 2025-11-06 10:59:04 UTC
Created attachment 189154 [details] [review]
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>
Comment 8 Martin Renvoize (ashimema) 2025-11-06 10:59:07 UTC
Created attachment 189155 [details] [review]
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.
Comment 9 Martin Renvoize (ashimema) 2025-11-06 11:01:22 UTC
Hi David, 

Thanks for testing. I've reworked the test plan to not require changes to system time.. that was a little bit too much of an ask for testing, not sure where my brain was at.

Hopefully the updated test plan is a little clearer.

I've also added a further patch that is my own take on how I think we should also be handling calendar closed days.. it goes some way toward answering my own question in the original patch.