From 9a4a9415ab2a695ebede35375bd32e2046a97cce Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 6 Jul 2026 10:10:42 +0100 Subject: [PATCH] Bug 42997: Warn staff when a due date override runs into the next booking When checking out a booked item, staff may override the suggested due date. If that override runs into the next booking for the item, the previous patch would silently keep the booking's original end date - staff had no way of knowing their override crossed another booking. This patch adds an exclude_booking_id option to Koha::Item->find_booking so a lookup can skip a specific booking (here: the patron's own booking being collected), and uses it in CanBookBeIssued to detect when an explicitly requested due date would run into the next booking for the item. When it would, a new BOOKED_DUE_DATE_CLASH confirmation is raised and displayed on the checkout screen, telling staff the linked booking will keep its original end date if they continue. Only explicitly requested due dates trigger the confirmation - calculated due dates are clamped to the booking end date later in the staff checkout flow, so checking them would produce false positives. SIP is unaffected as it never passes a requested due date. Test plan: 1) Run t/db_dependent/Circulation.t and t/db_dependent/Koha/Item.t 2) Create two consecutive bookings on the same item for two patrons 3) Check the item out to the first patron specifying a due date beyond the second booking's start date; confirm the new confirmation message appears and checkout only proceeds once confirmed Sponsored-by: OpenFifth Sponsored-by: Westminster City Council Signed-off-by: Hannah Dunne-Howrie Signed-off-by: Paul Derscheid Signed-off-by: Pedro Amorim --- C4/Circulation.pm | 16 +++++++ Koha/Item.pm | 17 +++++-- .../prog/en/modules/circ/circulation.tt | 7 +++ t/db_dependent/Circulation.t | 46 ++++++++++++++++++- t/db_dependent/Koha/Item.t | 27 ++++++++++- 5 files changed, 106 insertions(+), 7 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e4c76fb5f8..211de86d21 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -864,6 +864,7 @@ sub CanBookBeIssued { my $now = dt_from_string(); + my $requested_duedate = $duedate ? 1 : 0; $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron ); if ( DateTime->compare( $duedate, $now ) == -1 ) { # duedate cannot be before now @@ -1327,6 +1328,21 @@ sub CanBookBeIssued { } else { $alerts{'BOOKED'} = $booking; } + + # An explicitly requested due date that runs into the next booking + # for this item, regardless of whose booking that is, needs staff + # confirmation; the booking end date will not be extended past that + # booking (see AddIssue) + if ($requested_duedate) { + my $next_booking = $item_object->find_booking( + { + checkout_date => $now, + due_date => $duedate, + exclude_booking_id => $booking->booking_id, + } + ); + $needsconfirmation{'BOOKED_DUE_DATE_CLASH'} = $next_booking if $next_booking; + } } # Booked to another patron diff --git a/Koha/Item.pm b/Koha/Item.pm index eb19468653..a5599caab6 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -629,12 +629,17 @@ sub bookings { =head3 find_booking - my $booking = $item->find_booking( { checkout_date => $now, due_date => $future_date } ); + my $booking = $item->find_booking( + { checkout_date => $now, due_date => $future_date, [ exclude_booking_id => $booking_id ] } ); Find the first booking that would conflict with the passed checkout dates for this item. If a booking lead period is configured for the itemtype we will also take that into account here, counting bookings that fall in that lead period as conflicts too. +Optionally, you may pass an exclude_booking_id to leave a specific booking out of the checks; this is +helpful when looking for bookings that would clash with an extension of the checkout that is already +satisfying that booking. + FIXME: This can be simplified, it was originally intended to iterate all biblio level bookings to catch cases where this item may be the last available to satisfy a biblio level only booking. However, we dropped the biblio level functionality prior to push as bugs were found in it's @@ -645,9 +650,10 @@ implementation. sub find_booking { my ( $self, $params ) = @_; - my $start_date = $params->{checkout_date}; - my $end_date = $params->{due_date}; - my $biblio = $self->biblio; + my $start_date = $params->{checkout_date}; + my $end_date = $params->{due_date}; + my $exclude_booking_id = $params->{exclude_booking_id}; + my $biblio = $self->biblio; my $rule = Koha::CirculationRules->get_effective_rule( { @@ -689,7 +695,8 @@ sub find_booking { } ] }, - { status => { '-not_in' => [ 'cancelled', 'completed' ] } } + { status => { '-not_in' => [ 'cancelled', 'completed' ] } }, + ( $exclude_booking_id ? { booking_id => { '!=' => $exclude_booking_id } } : () ), ] }, { order_by => { '-asc' => 'start_date' } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt index 4a74a2b403..39f1c61ec8 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt @@ -317,6 +317,13 @@ [% IF ( BOOKED_EARLY ) %]
  • Patron has this item booked for checkout on [% BOOKED_EARLY.start_date | $KohaDates %]
    • [% END %] + + [% IF ( BOOKED_DUE_DATE_CLASH ) %] +
  • + The requested due date falls within the next booking for this item (booked from [% BOOKED_DUE_DATE_CLASH.start_date | $KohaDates %]). If you continue, the booking linked to this checkout will keep its original end + date. +
    • + [% END %] [% IF CAN_user_circulate_force_checkout or HIGHHOLDS %] diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 1974c2efa8..7564d18d90 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -5711,7 +5711,7 @@ subtest 'CanBookBeIssued | recalls' => sub { }; subtest 'CanBookBeIssued | bookings' => sub { - plan tests => 4; + plan tests => 7; my $schema = Koha::Database->schema; $schema->storage->txn_begin; @@ -5780,6 +5780,50 @@ subtest 'CanBookBeIssued | bookings' => sub { "Booked to this user, but they're collecting early" ); + # Requested due date runs into the next booking for this item + $booking->start_date( dt_from_string()->subtract( days => 1 ) )->store(); + + my $subsequent_booking = Koha::Booking->new( + { + patron_id => $patron2->borrowernumber, + pickup_library_id => $pickup_library->branchcode, + item_id => $item->itemnumber, + biblio_id => $item->biblio->biblionumber, + start_date => dt_from_string()->add( days => 8 ), + end_date => dt_from_string()->add( days => 12 ), + } + )->store(); + + ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) = CanBookBeIssued( + $patron1, $item->barcode, + dt_from_string()->add( days => 9 ), + undef, undef, undef + ); + is( + $needsconfirmation->{BOOKED_DUE_DATE_CLASH}->booking_id, + $subsequent_booking->booking_id, + "Requested due date running into the next booking needs confirmation" + ); + + ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) = CanBookBeIssued( + $patron1, $item->barcode, + dt_from_string()->add( days => 5 ), + undef, undef, undef + ); + is( + $needsconfirmation->{BOOKED_DUE_DATE_CLASH}, undef, + "No confirmation needed when the requested due date stays clear of the next booking" + ); + + ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) = CanBookBeIssued( + $patron1, $item->barcode, + undef, undef, undef, undef + ); + is( + $needsconfirmation->{BOOKED_DUE_DATE_CLASH}, undef, + "No confirmation triggered by a calculated due date; only explicit overrides are checked" + ); + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 3f1dc11c97..12f9409e15 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -3499,7 +3499,7 @@ subtest 'bookings' => sub { }; subtest 'find_booking' => sub { - plan tests => 9; + plan tests => 11; $schema->storage->txn_begin; @@ -3606,6 +3606,31 @@ subtest 'find_booking' => sub { "Koha::Item->find_booking returns the current booking not a future one" ); + # Exclusion of a specific booking + $found_booking = $item->find_booking( + { + checkout_date => dt_from_string(), + due_date => dt_from_string()->add( days => 7 ), + exclude_booking_id => $booking2->booking_id, + } + ); + is( + $found_booking, undef, + "Koha::Item->find_booking skips the excluded booking when nothing else clashes" + ); + + $found_booking = $item->find_booking( + { + checkout_date => dt_from_string(), + due_date => dt_from_string()->add( days => 9 ), + exclude_booking_id => $booking2->booking_id, + } + ); + is( + $found_booking->booking_id, $booking3->booking_id, + "Koha::Item->find_booking returns the next clashing booking when the excluded one is skipped" + ); + # Cancelled booking $booking2->status('cancelled')->store(); $found_booking = $item->find_booking(