Bugzilla – Attachment 183744 Details for
Bug 40296
Bookings that are checked out do not have status updated to completed
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40296: (follow-up) Fix booking patron ownership and cancellation logic
Bug-40296-follow-up-Fix-booking-patron-ownership-a.patch (text/plain), 4.37 KB, created by
Martin Renvoize (ashimema)
on 2025-07-03 10:55:34 UTC
(
hide
)
Description:
Bug 40296: (follow-up) Fix booking patron ownership and cancellation logic
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-07-03 10:55:34 UTC
Size:
4.37 KB
patch
obsolete
>From e93d0c629b3b20591581620a0c624b290b7a6339 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Thu, 3 Jul 2025 11:18:40 +0100 >Subject: [PATCH] Bug 40296: (follow-up) Fix booking patron ownership and > cancellation logic > >This follow-up addresses critical issues in the initial implementation: > >1. **Patron Ownership Check**: Only mark bookings as 'completed' when > the patron checking out the item is the same patron who made the booking > >2. **Smart Cancellation**: When another patron checks out an item that > conflicts with an existing booking: > - Cancel the booking only if checkout overlaps with actual booking period > - Respect lead/trail periods - don't cancel if conflict is only during > preparation periods (allows librarian override scenarios) > >3. **Date Logic**: Proper overlap detection using start_date and end_date > rather than periods extended by lead time > >This prevents incorrect booking completion and handles librarian override >scenarios appropriately. > >Test plan: >1. Apply previous patch and this follow-up >2. Enable bookings and create a bookable item > >Test A - Patron's own booking completion: >3. Create booking for Patron A: start = today, end = +7 days >4. Check out item to Patron A (due date = +5 days) >5. Verify booking status = 'completed' > >Test B - Another patron's booking cancellation: >6. Return the item, create new booking for Patron A: start = +2 days, end = +6 days >7. Check out item to Patron B (due date = +4 days) - may need override >8. Verify Patron A's booking status = 'cancelled' > >Test C - Lead period override scenario: >9. Return item, set bookings_lead_period circulation rule = 3 days >10. Create booking for Patron A: start = +5 days, end = +8 days >11. Check out item to Patron B (due date = +4 days) - conflicts with lead period >12. Override the booking conflict warning >13. Verify Patron A's booking status remains 'new' (not cancelled) >14. This respects librarian's decision that item will return in time > >Test D - No booking scenario: >15. Return item, ensure no active bookings exist >16. Check out item to any patron >17. Verify checkout works normally without errors >--- > C4/Circulation.pm | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 60c8c209acc..b742d6b78f8 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1767,14 +1767,29 @@ sub AddIssue { > # and SwitchOnSiteCheckouts is enabled this converts it to a regular checkout > $issue = Koha::Checkouts->find( { itemnumber => $item_object->itemnumber } ); > >- #if this checkout is a booking mark it as completed >- if ( >- my $booking = $item_object->find_booking( >- { checkout_date => $issuedate, due_date => $datedue, patron_id => $patron->borrowernumber } >- ) >- ) >- { >- $booking->status('completed')->store; >+ # If this checkout relates to a booking, handle booking status appropriately >+ if ( my $booking = $item_object->find_booking( { checkout_date => $issuedate, due_date => $datedue } ) ) { >+ if ( $booking->patron_id == $patron->borrowernumber ) { >+ >+ # Patron's own booking - mark as completed >+ $booking->status('completed')->store; >+ } else { >+ >+ # Another patron's booking - only cancel if checkout period overlaps with actual booking period >+ my $booking_start = dt_from_string( $booking->start_date ); >+ my $booking_end = dt_from_string( $booking->end_date ); >+ >+ # Check if checkout period overlaps with actual booking period (not just lead/trail) >+ if ( ( $issuedate >= $booking_start && $issuedate <= $booking_end ) >+ || ( $datedue >= $booking_start && $datedue <= $booking_end ) >+ || ( $issuedate <= $booking_start && $datedue >= $booking_end ) ) >+ { >+ # Checkout overlaps with actual booking period - cancel the booking >+ $booking->status('cancelled')->store; >+ } >+ >+ # If only in lead/trail period, do nothing - librarian has made informed decision >+ } > } > > if ($issue) { >-- >2.50.0
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 40296
:
183717
|
183743
| 183744 |
183745