Bugzilla – Attachment 185898 Details for
Bug 40665
Add booking_id field to issues to link checkouts to bookings that were fulfilled by them
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40665: Link checkouts to bookings
Bug-40665-Link-checkouts-to-bookings.patch (text/plain), 11.47 KB, created by
Martin Renvoize (ashimema)
on 2025-08-29 10:32:20 UTC
(
hide
)
Description:
Bug 40665: Link checkouts to bookings
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-08-29 10:32:20 UTC
Size:
11.47 KB
patch
obsolete
>From f320098daecad0ae5d8c65e3bd3db36673fb58af Mon Sep 17 00:00:00 2001 >From: Paul Derscheid <paul.derscheid@lmscloud.de> >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 <martin.renvoize@openfifth.co.uk> >--- > 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
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 40665
:
185516
|
185517
|
185518
|
185519
|
185520
|
185895
|
185896
|
185897
| 185898 |
185899
|
186250
|
186369
|
187589