Bug 39584 - Booking post-processing time cuts into circulation period
Summary: Booking post-processing time cuts into circulation period
Status: BLOCKED
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: 39916 37354
Blocks: 37707 39390
  Show dependency treegraph
 
Reported: 2025-04-08 17:38 UTC by Kristi
Modified: 2025-06-06 06:59 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function: Bookings


Attachments
Bug 39584: Use startDate instead of hoverDate when calculating loan period on mouseover (1.91 KB, patch)
2025-04-22 20:32 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
BookingPostProcessing_39584 (28.42 KB, image/png)
2025-04-23 16:58 UTC, Kristi
Details
Bug 39584: Include trailDays when calculating maxDate (2.16 KB, patch)
2025-05-14 13:37 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 39584: Include trailDays when calculating maxDate (5.11 KB, patch)
2025-05-14 14:36 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
BookingWithPostProcessing_NoRenewals (27.40 KB, image/png)
2025-05-20 15:19 UTC, Kristi
Details
BookingWithPostProcessing_WithRenewals (40.22 KB, image/png)
2025-05-20 15:20 UTC, Kristi
Details
BookingsWithDayAddedInBooking (21.46 KB, image/png)
2025-05-20 15:20 UTC, Kristi
Details
Bug 39584: Include trailDays when calculating maxDate (5.16 KB, patch)
2025-06-02 16:14 UTC, Eric Garcia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kristi 2025-04-08 17:38:22 UTC
Enforcing max circ rule loan periods on bookings (37354), cuts into the circ period of a booking when post-processing days are added.

To replicate:
1. Set up two days of post-processing time in the circ rules for an item type.
2. Make that item type bookable.
3. Place a booking for an item from that item type. Note that the 2-days of post-processing time are factored into the total number of days allowed for the booking. For a 7-day circ period, the booking cannot be set for 7 days; it is enforced to end at 5.
Comment 1 Lucas Gass (lukeg) 2025-04-22 20:29:59 UTC
I think this is a problem in the mouseover function AFTER selecting a startDate where it is incorrectly using hoverDate instead of startDate
Comment 2 Lucas Gass (lukeg) 2025-04-22 20:32:57 UTC
Created attachment 181312 [details] [review]
Bug 39584: Use startDate instead of hoverDate when calculating loan period on mouseover

To test:
1. Set up two days of post-processing time in the circ rules for an item type.
2. Make that item type bookable.
3. Place a booking for an item from that item type. Note that the 2-days of post-processing time are factored into the total number of days allowed for the booking. For a 7-day circ period, the booking cannot be set for 7 days; it is enforced to end at 5.
4. APPLY PATCH
5. Try the booking again, this time you should be able to select the full loan period for that item.
Comment 3 Kristi 2025-04-23 16:57:07 UTC
I looked at this in a sandbox. While I am able to add 2 days of post-processing and get the full 7-day circ period, it looks like the date picker is marking the post-processing days within the booking now. 

That is, if I pick a booking for a 7-day circ period between May 5-12, the calendar is marking May 6 and 7 as the post-processing days, not May 13-14. Please see the attached screenshot.
Comment 4 Kristi 2025-04-23 16:58:04 UTC
Created attachment 181394 [details]
BookingPostProcessing_39584

Shows booking post-processing days assigned within the booking period instead of after.
Comment 5 Martin Renvoize (ashimema) 2025-05-14 13:37:21 UTC
Created attachment 182448 [details] [review]
Bug 39584: Include trailDays when calculating maxDate

When enforcing the max booking period from the circulation rules, we
should not include booking post processing days.

Our pre and post processing days handling looks for 'flatpickr-disabled'
and ensure processing days cannot overlap. When setting maxDate,
flatpickr will apply the flatpickr-disabled class to the next day to
prevent selection past the max date and as such our post-processing acts
in an inclusive manor.

This patch updates our maxDate handler to increase then maximum target
by the trailDays setting to allow a full loan period.

To test:
1. Set up two days of post-processing time in the circ rules for an item type.
2. Make that item type bookable.
3. Place a booking for an item from that item type. Note that the 2-days of
   post-processing time are factored into the total number of days allowed
   for the booking. For a 7-day circ period, the booking cannot be set for
   7 days; it is enforced to end at 5.
4. APPLY PATCH
5. Try the booking again, this time you should be able to select the full loan period for that item.
Comment 6 Martin Renvoize (ashimema) 2025-05-14 13:38:22 UTC
I've got a slightly cleaner approach.. but I'm still refining a bit as I don't like that because the maxDate is extended in my version it appears later than it really is due to the ungreyed out dates.
Comment 7 Martin Renvoize (ashimema) 2025-05-14 14:36:39 UTC
Created attachment 182451 [details] [review]
Bug 39584: Include trailDays when calculating maxDate

When enforcing the max booking period from the circulation rules, we
should not include booking post processing days.

Our pre and post processing days handling looks for 'flatpickr-disabled'
and ensure processing days cannot overlap. When setting maxDate,
flatpickr will apply the flatpickr-disabled class to the next day to
prevent selection past the max date and as such our post-processing acts
in an inclusive manor.

This patch updates our maxDate handler to increase then maximum target
by the trailDays setting to allow a full loan period and includes
styling to match flatpickr-disabled for the exclusive trailDays.

To test:
1. Set up two days of post-processing time in the circ rules for an item type.
2. Make that item type bookable.
3. Place a booking for an item from that item type. Note that the 2-days of
   post-processing time are factored into the total number of days allowed
   for the booking. For a 7-day circ period, the booking cannot be set for
   7 days; it is enforced to end at 5.
4. APPLY PATCH
5. Try the booking again, this time you should be able to select the full loan period for that item.
Comment 8 Martin Renvoize (ashimema) 2025-05-14 14:39:20 UTC
OK.. I'm done.. could someone please try out my alternative patch.. I believe it more accurately allows for all the existing cases.

So.. the basic test plan just checks that the bug in question is resolved.. but what would be even better is to add a set of bookings in and ensure end date handling is still properly handled for those bookings with a start date before one and around the expected maximum date.

Many thanks,
Comment 9 Kristi 2025-05-20 15:19:13 UTC
I looked at this in a sandbox with a 7-day circ period and no renewals allowed. I got the full circ period with the 2 post-processing factored in at the end (39584_20250520.png) 

That addresses the bug as described, post-processing cutting into the circ period. 

If there are renewals allowed, the post-processing also does not cut into the circ peiod (39584_WithRenewals_20250520.png) It does look like it adds another day on the end date in this case though. A booking for 7 days with 2 renewals starting on 6/02 would end on 6/23 in the date picker, but it ends on 6/24 when booked (39584_Bookings.PNG)

I'm not sure I fully understand "what would be even better is to add a set of bookings in and ensure end date handling is still properly handled for those bookings with a start date before one and around the expected maximum date." Happy to discuss that further. 

Otherwise, the patch addresses the problem as described "Enforcing max circ rule loan periods on bookings (37354), cuts into the circ period of a booking when post-processing days are added."
Comment 10 Kristi 2025-05-20 15:19:45 UTC
Created attachment 182678 [details]
BookingWithPostProcessing_NoRenewals
Comment 11 Kristi 2025-05-20 15:20:11 UTC
Created attachment 182679 [details]
BookingWithPostProcessing_WithRenewals
Comment 12 Kristi 2025-05-20 15:20:45 UTC
Created attachment 182680 [details]
BookingsWithDayAddedInBooking
Comment 13 Martin Renvoize (ashimema) 2025-05-21 06:58:55 UTC
Thanks for testing Kristi, it's very much appreciated.


(In reply to Kristi from comment #9)
> If there are renewals allowed, the post-processing also does not cut into
> the circ peiod (39584_WithRenewals_20250520.png) It does look like it adds
> another day on the end date in this case though. A booking for 7 days with 2
> renewals starting on 6/02 would end on 6/23 in the date picker, but it ends
> on 6/24 when booked (39584_Bookings.PNG)

As discussed, I think this is pre-existing and it's actually a day "added" at to the initial issue if I'm understanding it correctly.

We count the first day of the booking as the "pickup day" and under normal circulation we would set the due date to today at midnight + issue period.. so if I booked from the first, the due date would be midnight on the 8th.. 1st + 7 days = 8th.

For renewals, we currently add a renewal period for each renewal allowed, so it's exclusive of the 'renewal' day.. it presumed you would be returning the item on the last day of your issue and adds renewal period to that day.

> I'm not sure I fully understand "what would be even better is to add a set
> of bookings in and ensure end date handling is still properly handled for
> those bookings with a start date before one and around the expected maximum
> date." Happy to discuss that further.

So what I meant by that was that I just wanted to verify that we're still handling bookings clashes as expected. i.e that the lead and trail periods still interact with other bookings, ensuring one cannot overlap with another booking (I know I also have another bug to work on to ensure we don't overlap lead/trail periods with each other too)
Comment 14 Kristi 2025-05-21 13:25:14 UTC
Thanks for the explanations on both points. 

Bookings do still overlap with pre- and post-processing time. I set circ rules for 2 days of post-processing and a 7-day circ period, no renewals. 

I placed a booking for 6/9-6/16, which would have the 2 post-processing days on 6/17 and 6/18. 

I was able to place a booking for 6/18- 6/25. That is, I was allowed to place a booking that started on one of the post-processing days for the first booking. 

This is bib 194 in the PTFS-hosted sandbox for this bug if it helps. 

Would this issue be a new bug report?
Comment 15 Eric Garcia 2025-06-02 16:14:54 UTC
Created attachment 182910 [details] [review]
Bug 39584: Include trailDays when calculating maxDate

When enforcing the max booking period from the circulation rules, we
should not include booking post processing days.

Our pre and post processing days handling looks for 'flatpickr-disabled'
and ensure processing days cannot overlap. When setting maxDate,
flatpickr will apply the flatpickr-disabled class to the next day to
prevent selection past the max date and as such our post-processing acts
in an inclusive manor.

This patch updates our maxDate handler to increase then maximum target
by the trailDays setting to allow a full loan period and includes
styling to match flatpickr-disabled for the exclusive trailDays.

To test:
1. Set up two days of post-processing time in the circ rules for an item type.
2. Make that item type bookable.
3. Place a booking for an item from that item type. Note that the 2-days of
   post-processing time are factored into the total number of days allowed
   for the booking. For a 7-day circ period, the booking cannot be set for
   7 days; it is enforced to end at 5.
4. APPLY PATCH
5. Try the booking again, this time you should be able to select the full loan period for that item.

Signed-off-by: Eric Garcia <cubingguy714@gmail.com>
Comment 16 Marcel de Rooy 2025-06-06 06:59:20 UTC
Blocked currently by 39916