Bug 37707 - Lead/Trail times should work in combination
Summary: Lead/Trail times should work in combination
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
: 39390 (view as bug list)
Depends on: 39584
Blocks: 40134 41514
  Show dependency treegraph
 
Reported: 2024-08-22 09:21 UTC by Martin Renvoize (ashimema)
Modified: 2026-01-07 21:38 UTC (History)
5 users (show)

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


Attachments
Bug 37707: First draft (17.33 KB, patch)
2025-05-22 06:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: Add Cypress tests for bidirectional lead/trail period conflicts (40.41 KB, patch)
2026-01-03 07:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: Improve lead/trail period conflict detection (16.57 KB, patch)
2026-01-03 07:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: (follow-up) Add visual refinements for existing booking lead/trail periods (78.60 KB, patch)
2026-01-03 07:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: (follow-up) Add textual feedback for booking lead/trail period conflicts (8.77 KB, patch)
2026-01-03 07:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: Add Cypress tests for bidirectional lead/trail period conflicts (40.41 KB, patch)
2026-01-07 17:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: Improve lead/trail period conflict detection (16.57 KB, patch)
2026-01-07 17:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: (follow-up) Add visual refinements for existing booking lead/trail periods (78.60 KB, patch)
2026-01-07 17:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37707: (follow-up) Add textual feedback for booking lead/trail period conflicts (33.32 KB, patch)
2026-01-07 17:57 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 Martin Renvoize (ashimema) 2024-08-22 09:21:24 UTC
Whilst working on the lead/trail time addition to bookings we discussed how they should affect things.

In the end we decided that they should work in combination when placing bookings so the window of unbookable time before a bookings should be the previous bookings 'trail' period + the current bookings 'lead' period.

I've run out of capacity to make that change in the aforementioned bug, so I'm splitting it out here to work on that as a expedient follow-up.

Lets get the core functionality in and then continue to polish it.

A- | TA >|< LB | -B- | TB >|< LC | -C
Comment 1 Martin Renvoize (ashimema) 2025-05-21 13:26:31 UTC
*** Bug 39390 has been marked as a duplicate of this bug. ***
Comment 2 Martin Renvoize (ashimema) 2025-05-22 06:48:35 UTC Comment hidden (obsolete)
Comment 3 Andrew Fuerste-Henry 2025-09-15 20:51:57 UTC
I can confirm this works as described -- bookings cannot be created such that the trailing time of one booking intersects with the leading time of the next. The interface grays out dates that cannot be selected and changes the mouse icon to a crossed-out-circle. Martin, what do you still feel is missing in the display here? Our libraries would love to see this functionality in Koha sooner rather than later.
Comment 4 Kristi Krueger 2025-10-01 19:54:59 UTC
We are eager to see this one move forward.
Comment 5 Martin Renvoize (ashimema) 2025-10-24 06:54:46 UTC
I'm still targeting 25.11 for this. There's been a few holdups in terms of getting consistent Cypress tests working upstream of this (I don't want to break existing functionality whilst working on this), and the test infrastructure has been changing multiple times whilst I've been working on that.  There's also a Vue refactor of the current Modal code to make extending it simpler, which Paul has ready to submit.

In short.. It's all just around the corner, but every time I hit it of late, I find the tides have shifted beneath my feet.
Comment 6 Martin Renvoize (ashimema) 2026-01-03 07:54:36 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize (ashimema) 2026-01-03 07:54:38 UTC Comment hidden (obsolete)
Comment 8 Martin Renvoize (ashimema) 2026-01-03 07:54:39 UTC Comment hidden (obsolete)
Comment 9 Martin Renvoize (ashimema) 2026-01-03 07:54:41 UTC Comment hidden (obsolete)
Comment 10 Kristi Krueger 2026-01-05 13:50:42 UTC
Is this something that could be tested in a sandbox?
Comment 11 Andrew Fuerste-Henry 2026-01-05 20:05:16 UTC
(In reply to Martin Renvoize (ashimema) from comment #9)
> Created attachment 190862 [details] [review] [review]
> Bug 37707: (follow-up) Add textual feedback for booking lead/trail period
> conflicts

This seems to be enforcing allowable dates correctly, but the messaging doesn't match what's given in the test plan and is not entirely accurate.

Additionally, some messages use "lead period" and "trail period" which other use  "preparation time" and "processing period." We should use "lead period" and "trail period" consistently.

> 5. Hover over Jan 12 (part of existing booking)
>    - Verify message: "This date is part of an existing booking"

Message I receive here is "Cannot select: Lead period (2 days before start) conflicts with existing booking." 
This is accurate in that the whole of the lead period for the new booking would be within the date range of the existing booking. But the test plan says I should just get told that the date I'm trying to select is within an existing booking and that would be a simpler message.

> 6. Hover over Jan 8 (existing booking's lead period)
>    - Verify message explains it's part of the lead period

Message I receive here is "Cannot select: Processing period (3 days after return) conflicts with existing booking's preparation time." 
This is potentially accurate -- if I'm trying to make a booking that starts on 1/8 and ends on 1/9, then the trail period of the new booking would include 1/9, which is in the lead period for my existing booking. But it would be more clear and true to tell me I can't start a booking on 1/8 because 1/8 is within the lead time for the existing booking.

> 7. Try to select Jan 19-20 as booking dates (2-day lead = Jan 17-18)
>    - Hover over Jan 19 when it appears disabled
>    - Verify error message: "conflicts with existing booking's trail period"

Hovering over 1/19 or 1/20, I get the message "Cannot select: Lead period (2 days before start) conflicts with existing booking."
This is inaccurate. I should be told the new lead period would conflict with the existing trail period.

Hovering over 1/16, 1/17, or 1/18, I get the message "Cannot select: Lead period (2 days before start) conflicts with existing item's processing time."
This is inaccurate for 1/16; the lead time for a booking starting 1/16 would be fully within the existing booking rather than in its trail period.

> 8. Hover over Jan 21 (no conflicts)
>    - Verify info message about lead/trail requirements

These messages get confusing if I edit my circ rules so that I'm only setting a lead time *or* a trail time. If I put a 2 in "Booking pre-processing" and leave "Booking post-processing" blank, the circ rules interface actually saves a blank  booking_trail_period value and then the message when placing a booking reads "Selecting end date. Processing period: days after return" It should either not mention lead periods or trail periods at all if the given value is blank. In fact, it should probably not mention them for values of 0, either.

Additionally, if I have a lead period of 2 days defined and try to place a booking starting fewer than 2 days in the future, I get the message "Cannot select: Lead period (2 days before start) conflicts with existing booking." I get this message even on a record with no existing bookings at all. If I'm trying to select a date in the past, it should tell me I cannot select a date in the past. If I'm trying to select a date that would put my lead period in the past, it should tell me that the date cannot be selected because it would provide insufficient lead time.
Comment 12 Martin Renvoize (ashimema) 2026-01-07 17:57:20 UTC
Created attachment 191000 [details] [review]
Bug 37707: Add Cypress tests for bidirectional lead/trail period conflicts

This enhances the bookings lead/trail period tests to validate that:
- New booking's lead period respects existing bookings' trail periods
- New booking's trail period respects existing bookings' lead periods

Previously, we only checked:
- New booking lead vs existing booking actual dates
- New booking trail vs existing booking actual dates

Now we also validate the full "protected period" concept where each
existing booking reserves: lead + actual + trail period.

Test timeline updated:
- Booking A: Days 20-25 (protected: 18-28)
- Booking B: Days 60-65 (protected: 58-68)

New test coverage:
- TEST 2B: New lead conflicts with existing trail (Days 27-30)
- TEST 6B: New trail conflicts with existing lead (Days 55-56)
- TEST 7: Valid booking window (Days 31-54) respecting both

Current status: Tests FAIL as expected, revealing the bug that
bidirectional checking is not yet implemented.
Comment 13 Martin Renvoize (ashimema) 2026-01-07 17:57:22 UTC
Created attachment 191001 [details] [review]
Bug 37707: Improve lead/trail period conflict detection

This implements the full "protected period" concept for bookings, where
each existing booking reserves: lead period + actual dates + trail period.

Previously, we only checked:
- New booking lead vs existing booking actual dates
- New booking trail vs existing booking actual dates

Now we ALSO check:
- New booking lead vs existing booking trail periods
- New booking trail vs existing booking lead periods

This ensures librarians cannot create bookings too close together,
respecting both item preparation time (lead) and processing time (trail).

Test plan:
1. Apply all patches and run: yarn build
2. Configure a bookable item with 2-day lead and 3-day trail periods
3. Create booking A: January 5-9
   - This reserves Jan 3-4 (lead), Jan 5-9 (actual), Jan 10-12 (trail)
4. Attempt to create booking B: January 12-15 with 2-day lead
   - Lead would be Jan 10-11, which conflicts with booking A's trail
5. Verify days 10-11 are visually disabled with striped pattern
6. Verify hovering shows "conflicts with existing booking's trail period"
7. Verify you cannot select January 12 as start date
8. Verify January 13 IS selectable (no conflict)
9. Run Cypress tests to confirm:
   npx cypress run --spec "t/cypress/integration/Circulation/bookingsModalDatePicker_spec.ts"
Comment 14 Martin Renvoize (ashimema) 2026-01-07 17:57:23 UTC
Created attachment 191002 [details] [review]
Bug 37707: (follow-up) Add visual refinements for existing booking lead/trail periods

This patch enhances the visual presentation of existing bookings'
protected periods (lead/trail) to match the polished appearance of
new booking lead/trail period indicators.

Test plan:
1. Apply all patches and run: yarn build
2. Configure a bookable item with 2-day lead and 2-day trail
3. Create an existing booking for January 10-15
4. Open the booking calendar for that item
5. Verify visual styling of protected periods:
   - Jan 8-9 (lead period): diagonal striped pattern, rounded left edge on Jan 8
   - Jan 10-15 (booking): solid color fill
   - Jan 16-17 (trail period): diagonal striped pattern, rounded right edge on Jan 17
6. Verify no visual gaps between consecutive protected period dates
7. Verify the striped pattern is clearly distinguishable from solid booking dates
8. Hover over different date ranges and verify lead/trail styling updates correctly
Comment 15 Martin Renvoize (ashimema) 2026-01-07 17:57:24 UTC
Created attachment 191003 [details] [review]
Bug 37707: (follow-up) Add textual feedback for booking lead/trail period conflicts

This patch adds a feedback message container below the calendar that
provides contextual information about lead/trail period requirements
and conflicts, improving accessibility and user understanding.

The feedback updates dynamically on hover, showing:
- Info messages when hovering selectable dates
- Error messages when hovering dates disabled due to conflicts
- Specific explanations distinguishing between conflict types
  (e.g., new lead vs existing trail, new trail vs existing lead)
- Multiple conflict messages when both lead and trail periods conflict
- Specific messages for past dates and insufficient lead time

Key improvements:
- Standardized terminology consistently uses "lead period" and "trail period"
- Proper internationalization using __() for all user-facing strings
- DRY code with detectPeriodConflict() helper function
- Styling moved from inline JavaScript to SCSS using Bootstrap alert classes
- Empty/zero periods are handled gracefully (not mentioned in messages)
- Accurate conflict detection prioritizes what the hovered date IS over
  what it conflicts with

This complements the visual color indicators by providing explicit
textual explanations, improving accessibility and user understanding.

Test plan:
1. Apply all patches and run: yarn build
2. Configure a bookable item with 2-day lead and 3-day trail periods
3. Create an existing booking for January 10-15 (protected: Jan 8-18)
4. Open the booking calendar and observe feedback area below calendar

5. Test past date and insufficient lead time messages:
   - Hover on today's date
     * Verify message: "Cannot select: Date is in the past"
   - Hover on tomorrow (with 2-day lead period)
     * Verify message: "Cannot select: Insufficient lead time (2 days required before start)"
   - Hover on the day after tomorrow
     * Verify message: "Cannot select: Insufficient lead time (2 days required before start)"
     * Should NOT show "conflicts with existing booking" if no booking exists

6. Test existing booking conflict messages:
   - Hover over Jan 12 (part of existing booking)
     * Verify message: "Cannot select: This date is part of an existing booking"
   - Hover over Jan 8 (existing booking's lead period)
     * Verify message: "Cannot select: This date is part of an existing booking's lead period"
   - Hover over Jan 16 (existing booking's trail period)
     * Verify message: "Cannot select: This date is part of an existing booking's trail period"

7. Test lead/trail period conflict messages:
   - Try to select Jan 19-20 as booking dates (2-day lead = Jan 17-18)
   - Hover over Jan 19 when it appears disabled
     * Verify error message: "Cannot select: Lead period (2 days before start) conflicts with existing booking's trail period"

8. Test info messages for valid dates:
   - Hover over Jan 21 (no conflicts, with lead period configured)
     * Verify info message: "Selecting start date. Lead period: 2 days before start"
   - Configure booking with NO lead period (0 or blank)
   - Hover over a valid date
     * Verify message: "Selecting start date" (no mention of lead period)

9. Test multiple conflict messages:
   - Position calendar to hover between two bookings where both lead and trail conflict
     * Verify message shows both conflicts separated by semicolon
     * Example: "Cannot select: lead period conflicts with existing booking's trail period; trail period conflicts with existing booking's lead period"

10. Verify all feedback messages:
    - Are properly translated (check .po files if applicable)
    - Use consistent "lead period" and "trail period" terminology
    - Are clear and helpful for librarians
    - Update color coding (red for errors, blue for info)
Comment 16 Martin Renvoize (ashimema) 2026-01-07 17:59:03 UTC
Thanks for the feedback Andrew, hopefully my replaced last patch makes the messages clearer.

Kristi, yes you should be able to test this using a sandbox I believe.
Comment 17 Andrew Fuerste-Henry 2026-01-07 21:38:00 UTC
Following the test plan in comment 15, I've run into some issues.
I've got a bib with just 1 item on it. That item is bookable.

After creating a booking for January 10-15, the calendar to create a new booking has January 10-16 greyed out, as if my booking were 10-16.

In the calendar, mousing over days in the past gives error messages as if all past dates are part of an existing booking.

When mousing over a date to select the start of a booking, the system gives feedback about the potential booking's trail period. I would suggest we should only give feedback about the potential booking's trail period when selecting the end of the potential booking. Until the user is proposing a booking end date, we cannot know what that booking's trail will conflict with.

The datepicker and message area generally appeared above the Booking Dates input field. However, sometimes the datepicker and message area appeared below the input field instead. When this happened, the bottom of the message field pushed out of my window and could not be seen without zooming my browswer out. I was not able to scroll down to see the rest of the message.

The messages contain inconsistent capitalization. Errors all start "Cannot select:" but the first word following that colon is sometimes capitalized and sometimes not.

 (In reply to Martin Renvoize (ashimema) from comment #15)
> 6. Test existing booking conflict messages:
>    - Hover over Jan 12 (part of existing booking)
>      * Verify message: "Cannot select: This date is part of an existing
> booking"

This gives me the message "lead period conflicts with existing booking; trail period conflicts with an existing booking." The first statement will be true if I select 1/12 as the start of my booking. The second statement will be true if I select 1/12 as the end of my booking. In either case, the proposed message of "This date is part of an existing booking" would be more clear.

>    - Hover over Jan 8 (existing booking's lead period)
>      * Verify message: "Cannot select: This date is part of an existing
> booking's lead period"

This gives me the message "lead period conflicts with existing booking; trail period conflicts with existing booking's lead period." The first statement is *not* true, the lead period for a booking starting 1/8 would need to start 1/6, which is in the past and does not have a booking.

>    - Hover over Jan 16 (existing booking's trail period)
>      * Verify message: "Cannot select: This date is part of an existing
> booking's trail period"

This gives me the message "This date is part of an exiting booking." This is *not* true, 1/16 is part of the trail period of my existing booking.