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 <https://openfifth.co.uk/\>
Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/\>
Signed-off-by: Hannah Dunne-Howrie <hdunne-howrie@westminster.gov.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-07-06 10:10:42 +01:00 committed by Pedro Amorim
parent 41c453eff9
commit 9a4a9415ab
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
5 changed files with 106 additions and 7 deletions

View File

@ -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

View File

@ -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' } }

View File

@ -317,6 +317,13 @@
[% IF ( BOOKED_EARLY ) %]
<li class="needsconfirm booked_early"> Patron has this item booked for checkout on [% BOOKED_EARLY.start_date | $KohaDates %] </li>
[% END %]
[% IF ( BOOKED_DUE_DATE_CLASH ) %]
<li class="needsconfirm 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.
</li>
[% END %]
</ul>
[% IF CAN_user_circulate_force_checkout or HIGHHOLDS %]

View File

@ -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;
};

View File

@ -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(