gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 42997: (QA follow-up) Don't invert the booking period when syncing end dates 

Collecting a booking early (within the bookings lead period) with a
due date override before the booking start syncs end_date to a date
that precedes start_date. Koha::Booking->store has no interval
validation and the clash checks' date windows never see the inverted
period, so the broken row is stored silently - reproduced via AddIssue
on this patchset.

- pull start_date back to the checkout date whenever the synced due
  date precedes it, in both AddIssue and AddRenewal, so the booking
  window follows the loan instead of inverting
- cover both paths in Circulation.t; the AddRenewal case needs the
  Biblio::check_booking exemption from the previous patch, as the
  synced window no longer overlaps the stored one

Test plan:
1. prove t/db_dependent/Circulation.t
2. Set bookings_lead_period to e.g. 5 for an itemtype and enable
   SpecifyDueDate
3. Book an item of that itemtype for a patron starting in 3 days
4. Check it out to them today with an explicit due date of tomorrow
5. Without this patch the booking is stored with end_date before
   start_date; with it both dates follow the checkout period

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Paul Derscheid 2026-07-24 11:26:41 +02:00 committed by Pedro Amorim
parent 8bd9790e47
commit 90281d5a1f
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
2 changed files with 85 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
C4/Circulation.pm
View File

@ -1877,7 +1877,15 @@ sub AddIssue {
# Sync any due date override back to the booking end_date so the
# booking and checkout agree from the moment of issue
my $booking_end = dt_from_string( $booking->end_date );
$booking->end_date($datedue) if $datedue->compare($booking_end) != 0;
if ( $datedue->compare($booking_end) != 0 ) {
$booking->end_date($datedue);
# An early collection can be due back before the booked period
# even starts; pull the start_date back to the checkout date
# rather than storing an inverted booking period
my $booking_start = dt_from_string( $booking->start_date );
$booking->start_date($issuedate) if $datedue->compare($booking_start) < 0;
}
try {
$booking->store;
@ -3750,7 +3758,14 @@ sub AddRenewal {
my $booking_end = dt_from_string( $booking->end_date );
if ( $datedue->compare($booking_end) != 0 ) {
try {
$booking->end_date($datedue)->store;
$booking->end_date($datedue);
# As at issue time, guard against inverting the booking period
# when the new due date precedes the booking start
my $booking_start = dt_from_string( $booking->start_date );
$booking->start_date( dt_from_string( $issue->issuedate ) )
if $datedue->compare($booking_start) < 0;
$booking->store;
} catch {
my $error = $_;
if ( blessed $error && $error->isa('Koha::Exceptions::Booking::Clash') ) {

70
t/db_dependent/Circulation.t
View File

@ -7539,7 +7539,7 @@ subtest 'AddIssue/AddReturn | booking status handling' => sub {
};
subtest 'AddIssue | booking end_date sync with due date override' => sub {
plan tests => 5;
plan tests => 8;
my $schema = Koha::Database->schema;
$schema->storage->txn_begin;
@ -7627,6 +7627,45 @@ subtest 'AddIssue | booking end_date sync with due date override' => sub {
"Booking end_date kept when override would clash with a subsequent booking"
);
AddReturn( $item->barcode, $pickup_library->branchcode );
$booking->delete();
$subsequent_booking->delete();
# Case 4: Early collection due back before the booking starts - the
# start_date follows the checkout so the period is not inverted
Koha::CirculationRules->set_rule(
{
branchcode => '*',
itemtype => $item->effective_itemtype,
rule_name => 'bookings_lead_period',
rule_value => 5,
}
);
$booking = Koha::Booking->new(
{
patron_id => $patron->borrowernumber,
pickup_library_id => $pickup_library->branchcode,
item_id => $item->itemnumber,
biblio_id => $item->biblio->biblionumber,
start_date => dt_from_string()->add( days => 3 ),
end_date => dt_from_string()->add( days => 10 ),
}
)->store();
my $early_due = dt_from_string()->add( days => 1 )->truncate( to => 'minute' );
my $early_issue = AddIssue( $patron, $item->barcode, $early_due->clone );
$booking->discard_changes();
is( $booking->status, 'issued', "Booking marked issued when collected early" );
is(
dt_from_string( $booking->end_date )->compare($early_due), 0,
"Booking end_date synced to an override before the booking start"
);
is(
$booking->start_date, $early_issue->issuedate,
"Booking start_date pulled back to the checkout date instead of storing an inverted period"
);
$schema->storage->txn_rollback;
};
@ -7841,7 +7880,7 @@ subtest 'AddRenewal | booking_id preservation' => sub {
};
subtest 'AddRenewal | booking end_date sync' => sub {
plan tests => 4;
plan tests => 6;
my $schema = Koha::Database->schema;
$schema->storage->txn_begin;
@ -7938,6 +7977,33 @@ subtest 'AddRenewal | booking end_date sync' => sub {
"Booking end_date kept when the renewal clashes with the next booking"
);
# A renewal due before the booking start pulls the start_date back too
$booking->set(
{
start_date => dt_from_string()->add( days => 5 ),
end_date => dt_from_string()->add( days => 8 ),
}
)->store;
my $short_due_date = dt_from_string()->add( days => 2 )->truncate( to => 'minute' );
AddRenewal(
{
borrowernumber => $patron->borrowernumber,
itemnumber => $item->itemnumber,
branch => $library->branchcode,
datedue => $short_due_date->clone,
}
);
$booking->discard_changes;
is(
dt_from_string( $booking->end_date )->compare($short_due_date), 0,
"Booking end_date synced to a renewal due date before the booking start"
);
is(
$booking->start_date, $issue->get_from_storage->issuedate,
"Booking start_date pulled back to the checkout date instead of storing an inverted period"
);
$schema->storage->txn_rollback;
};