From e93d0c629b3b20591581620a0c624b290b7a6339 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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