From f320098daecad0ae5d8c65e3bd3db36673fb58af Mon Sep 17 00:00:00 2001 From: Paul Derscheid Date: Mon, 18 Aug 2025 15:43:00 +0000 Subject: [PATCH] Bug 40665: Link checkouts to bookings This patch adds a booking_id field to the issues and old_issues tables to create a linkage between checkouts and the bookings that generated them. When a patron with a booking checks out the reserved item, the checkout is linked to their booking and the booking status is updated to 'completed'. The booking_id is preserved during renewals and when checkouts are moved to old_issues upon return. Database changes: - Add booking_id column to issues and old_issues tables - Add foreign key constraints to maintain referential integrity Test plan: 1. Apply the database update 2. Restart services 3. Create a booking for a patron 4. When the patron checks out the booked item: a. Verify the checkout links to the booking (booking_id is set) b. Verify the booking status changes to 'completed' 5. Renew the checkout a. Verify the booking_id is preserved after renewal b. Check the database: SELECT booking_id FROM issues WHERE issue_id = X 6. Return the item a. Verify the checkout moves to old_issues b. Verify the booking_id is preserved in old_issues 7. Test the new Koha::Booking methods: a. For a booking with a checkout, verify $booking->checkout returns the correct Koha::Checkout object b. For a booking with an old checkout, verify $booking->old_checkout returns the correct Koha::Old::Checkout object 8. Test the API: a. Verify checkout endpoints include booking_id in responses b. Verify the booking relationship can be embedded using x-koha-embed: booking 9. Run the test suite: prove t/db_dependent/Circulation.t prove t/db_dependent/Koha/Booking.t prove t/db_dependent/Koha/Checkout.t NOTE: I also thought about the inverse: linking to a checkout from the booking, but as Koha is a circulation centric system, this approach makes more sense in my opinion. I also thought about bidirectional linking via FKs but that is too much maintenance overhead (and potential error source) that we don't need. The inverse lookups should still be performant through the FKs. Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 80 ++++++++++++++++++++++++-------------------- Koha/Booking.pm | 28 ++++++++++++++++ Koha/Checkout.pm | 17 ++++++++++ Koha/Old/Checkout.pm | 17 ++++++++++ 4 files changed, 106 insertions(+), 36 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 514eb3ae97a..87926ed20b2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1768,47 +1768,55 @@ sub AddIssue { # and SwitchOnSiteCheckouts is enabled this converts it to a regular checkout $issue = Koha::Checkouts->find( { itemnumber => $item_object->itemnumber } ); - # 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 ) { + my $schema = Koha::Database->schema; + $schema->txn_do( + sub { + # 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 { + # Patron's own booking - mark as completed and link checkout to booking + $booking->status('completed')->store; + $issue_attributes->{'booking_id'} = $booking->booking_id; + } 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 ); + # 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; + # 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 only in lead/trail period, do nothing - librarian has made informed decision - } - } + if ($issue) { + $issue->set($issue_attributes)->store; + } else { + $issue = Koha::Checkout->new( + { + itemnumber => $item_object->itemnumber, + %$issue_attributes, + } + )->store; - if ($issue) { - $issue->set($issue_attributes)->store; - } else { - $issue = Koha::Checkout->new( - { - itemnumber => $item_object->itemnumber, - %$issue_attributes, + # Ensure item is no longer listed in the holds queue + $dbh->do( + q{DELETE tmp_holdsqueue, hold_fill_targets FROM tmp_holdsqueue LEFT JOIN hold_fill_targets USING ( itemnumber ) WHERE itemnumber = ?}, + undef, $item_object->id + ); } - )->store; - - # Ensure item is no longer listed in the holds queue - $dbh->do( - q{DELETE tmp_holdsqueue, hold_fill_targets FROM tmp_holdsqueue LEFT JOIN hold_fill_targets USING ( itemnumber ) WHERE itemnumber = ?}, - undef, $item_object->id - ); - } + } + ); $issue->discard_changes; #Update borrowers.lastseen @@ -3475,13 +3483,13 @@ sub AddRenewal { # of how many times it has been renewed. my $renews = ( $issue->renewals_count || 0 ) + 1; my $sth = $dbh->prepare( - "UPDATE issues SET date_due = ?, renewals_count = ?, unseen_renewals = ?, lastreneweddate = ? WHERE issue_id = ?" + "UPDATE issues SET date_due = ?, renewals_count = ?, unseen_renewals = ?, lastreneweddate = ?, booking_id = ? WHERE issue_id = ?" ); eval { $sth->execute( $datedue->strftime('%Y-%m-%d %H:%M'), $renews, $unseen_renewals, $lastreneweddate, - $issue->issue_id + $issue->booking_id, $issue->issue_id ); }; if ( $sth->err ) { diff --git a/Koha/Booking.pm b/Koha/Booking.pm index ab5a4180470..2aaeee8d51f 100644 --- a/Koha/Booking.pm +++ b/Koha/Booking.pm @@ -92,6 +92,34 @@ sub item { return Koha::Item->_new_from_dbic($item_rs); } +=head3 checkout + +Returns the related Koha::Checkout object for this booking, if any + +=cut + +sub checkout { + my ($self) = @_; + + my $checkout_rs = $self->_result->issues->first; + return unless $checkout_rs; + return Koha::Checkout->_new_from_dbic($checkout_rs); +} + +=head3 old_checkout + +Returns the related Koha::Old::Checkout object for this booking, if any + +=cut + +sub old_checkout { + my ($self) = @_; + + my $old_checkout_rs = $self->_result->old_issues->first; + return unless $old_checkout_rs; + return Koha::Old::Checkout->_new_from_dbic($old_checkout_rs); +} + =head3 store Booking specific store method to catch booking clashes and ensure we have an item assigned diff --git a/Koha/Checkout.pm b/Koha/Checkout.pm index 3c8218cea0f..3839935ebbf 100644 --- a/Koha/Checkout.pm +++ b/Koha/Checkout.pm @@ -24,6 +24,7 @@ use DateTime; use Try::Tiny qw( catch try ); use C4::Circulation qw( AddRenewal CanBookBeRenewed LostItem MarkIssueReturned ); +use Koha::Booking; use Koha::Checkouts::Renewals; use Koha::Checkouts::ReturnClaims; use Koha::Database; @@ -165,6 +166,21 @@ sub renewals { return Koha::Checkouts::Renewals->_new_from_dbic($renewals_rs); } +=head3 booking + +my $booking = $checkout->booking; + +Return the linked booking + +=cut + +sub booking { + my ($self) = @_; + my $booking_rs = $self->_result->booking; + return unless $booking_rs; + return Koha::Booking->_new_from_dbic($booking_rs); +} + =head3 attempt_auto_renew my ($success, $error, $updated) = $checkout->auto_renew({ confirm => 1 }); @@ -227,6 +243,7 @@ sub to_api_mapping { issue_id => 'checkout_id', borrowernumber => 'patron_id', itemnumber => 'item_id', + booking_id => 'booking_id', date_due => 'due_date', branchcode => 'library_id', returndate => 'checkin_date', diff --git a/Koha/Old/Checkout.pm b/Koha/Old/Checkout.pm index b2187ef6500..d7171000683 100644 --- a/Koha/Old/Checkout.pm +++ b/Koha/Old/Checkout.pm @@ -17,6 +17,7 @@ package Koha::Old::Checkout; use Modern::Perl; +use Koha::Booking; use Koha::Database; use Koha::Exceptions::SysPref; use Koha::Libraries; @@ -120,6 +121,21 @@ sub renewals { return Koha::Checkouts::Renewals->_new_from_dbic($renewals_rs); } +=head3 booking + +my $booking = $checkout->booking; + +Return the linked booking + +=cut + +sub booking { + my ($self) = @_; + my $booking_rs = $self->_result->booking; + return unless $booking_rs; + return Koha::Booking->_new_from_dbic($booking_rs); +} + =head3 anonymize $checkout->anonymize(); @@ -155,6 +171,7 @@ sub to_api_mapping { issue_id => 'checkout_id', borrowernumber => 'patron_id', itemnumber => 'item_id', + booking_id => 'booking_id', date_due => 'due_date', branchcode => 'library_id', returndate => 'checkin_date', -- 2.51.0