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

Bug 42997: Prevent renewals running into the next booking 

CanBookBeRenewed checked bookings by asking Koha::Item->find_booking
for the first booking that clashes with the renewal period. When the
renewing patron's own booking satisfies this checkout, that booking is
returned first and renewal is allowed - subsequent bookings for the
item were never considered. A patron could therefore renew a booked
item straight through the start of the next patron's booking.

Additionally, when staff renew with an explicit due date (the renewal
due date input on the checkouts screen, or the hard due date on
circ/renew.pl), CanBookBeRenewed validated a *calculated* due date
whilst AddRenewal then applied the *requested* one, so the requested
date was never checked against bookings at all.

This patch:

* repeats the booking lookup with the patron's own booking excluded,
  refusing the renewal ('booked') when it would run into the next
  booking for the item
* adds an optional $date_due parameter to CanBookBeRenewed, used in
  place of the calculated due date for the bookings check, and threads
  the requested date through from svc/renew and circ/renew.pl

Test plan:
1) Run t/db_dependent/Circulation.t - the extended
   'CanBookBeRenewed | bookings' subtest covers both the calculated
   and requested due date cases
2) Check out a booked item to its patron, add a subsequent booking for
   another patron, and attempt to renew past the second booking's
   start date from the checkouts screen; confirm the renewal is
   refused as 'booked'

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:14:04 +01:00 committed by Pedro Amorim
parent 9a4a9415ab
commit 2da3248b41
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
4 changed files with 87 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
C4/Circulation.pm
View File

@ -3331,7 +3331,7 @@ sub GetUpcomingDueIssues {
=head2 CanBookBeRenewed
($ok,$error,$info) = &CanBookBeRenewed($patron, $issue, $override_limit, $cron);
($ok,$error,$info) = &CanBookBeRenewed($patron, $issue, $override_limit, $cron, $date_due);
Find out whether a borrowed item may be renewed.
@ -3348,6 +3348,11 @@ the limit on the number of times that the loan can be renewed
to renew sooner than "No renewal before" and to manually renew loans
that are automatically renewed.
C<$date_due>, optional, is the requested due date for the renewal when
the caller intends to renew to a specific date rather than let the
circulation rules calculate one. It is used in place of the calculated
due date when checking the renewal against bookings for the item.
C<$CanBookBeRenewed> returns a true value if the item may be renewed. The
item must currently be on loan to the specified borrower; renewals
must be allowed for the item's type; and the borrower must not have
@ -3359,7 +3364,7 @@ already renewed the loan.
=cut
sub CanBookBeRenewed {
my ( $patron, $issue, $override_limit, $cron ) = @_;
my ( $patron, $issue, $override_limit, $cron, $date_due ) = @_;
my $auto_renew = "no";
my $soonest;
@ -3524,7 +3529,10 @@ sub CanBookBeRenewed {
( C4::Context->preference('ManualRenewalPeriodBase') eq 'date_due' )
? dt_from_string( $issue->date_due, 'sql' )
: dt_from_string();
my $datedue = CalcDateDue( $startdate, $item->effective_itemtype, $branchcode, $patron, 'is a renewal' );
my $datedue =
$date_due
? dt_from_string($date_due)
: CalcDateDue( $startdate, $item->effective_itemtype, $branchcode, $patron, 'is a renewal' );
if (
my $booking = $item->find_booking(
{ checkout_date => $startdate, due_date => $datedue, patron_id => $patron->borrowernumber }
@ -3532,6 +3540,19 @@ sub CanBookBeRenewed {
)
{
return ( 0, 'booked' ) unless ( $booking->patron_id == $patron->borrowernumber );
# The renewal must not run into the next booking for this item,
# regardless of whose booking that is; the patron's own booking
# hides any subsequent ones from the lookup above, so repeat it
# with their booking excluded
return ( 0, 'booked' )
if $item->find_booking(
{
checkout_date => $startdate,
due_date => $datedue,
exclude_booking_id => $booking->booking_id,
}
);
}
}

7
circ/renew.pl
View File

@ -72,7 +72,12 @@ if ( $op eq 'cud-renew' && $barcode ) {
my $confirmations;
my $can_renew;
my $info;
( $can_renew, $error, $info ) = CanBookBeRenewed( $patron, $checkout, $override_limit );
my $requested_due_date =
( C4::Context->preference('SpecifyDueDate') && $hard_due_date )
? $hard_due_date
: undef;
( $can_renew, $error, $info ) =
CanBookBeRenewed( $patron, $checkout, $override_limit, undef, $requested_due_date );
push @{$confirmations}, 'RENEWAL_LIMIT' if $override_limit;
if ( $error && ( $error eq 'on_reserve' ) ) {

2
svc/renew
View File

@ -66,7 +66,7 @@ my $item = Koha::Items->find($itemnumber);
my $confirmations;
( $data->{renew_okay}, $data->{error} ) =
CanBookBeRenewed( $patron, $item->checkout, $override_limit );
CanBookBeRenewed( $patron, $item->checkout, $override_limit, undef, $date_due );
# If we're allowing reserved items to be renewed...
if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference('AllowRenewalOnHoldOverride') ) {

57
t/db_dependent/Circulation.t
View File

@ -2009,7 +2009,7 @@ subtest "CanBookBeRenewed tests" => sub {
};
subtest "CanBookBeRenewed | bookings" => sub {
plan tests => 3;
plan tests => 8;
my $schema = Koha::Database->schema;
$schema->storage->txn_begin;
@ -2044,6 +2044,61 @@ subtest "CanBookBeRenewed | bookings" => sub {
is( $renewok, 0, "Renewal not allowed as it would mean the item was not returned before the next booking" );
is( $error, 'booked', "Error is 'booked'" );
# Renewing patron has their own booking satisfied by this checkout;
# the renewal still must not run into the next booking for the item
$booking->delete();
my $own_booking = $builder->build_object(
{
class => 'Koha::Bookings',
value => {
patron_id => $renewing_patron->borrowernumber,
pickup_library_id => $pickup_library->branchcode,
item_id => $item->itemnumber,
biblio_id => $item->biblio->biblionumber,
start_date => dt_from_string()->subtract( days => 1 ),
end_date => $datedue->clone(),
status => 'issued',
}
}
);
$issue->booking_id( $own_booking->booking_id )->store();
$issue->discard_changes;
my $next_booking = $builder->build_object(
{
class => 'Koha::Bookings',
value => {
patron_id => $booked_patron->borrowernumber,
pickup_library_id => $pickup_library->branchcode,
item_id => $item->itemnumber,
biblio_id => $item->biblio->biblionumber,
start_date => $datedue->clone()->add( days => 2 ),
end_date => $datedue->clone()->add( days => 10 ),
status => 'new',
}
}
);
# Calculated renewal due date (date_due + renewal period) runs into the next booking
( $renewok, $error ) = CanBookBeRenewed( $renewing_patron, $issue, 0 );
is( $renewok, 0, "Renewal with own booking not allowed when it would run into the next booking" );
is( $error, 'booked', "Error is 'booked'" );
# An explicitly requested due date is checked instead of the calculated one
( $renewok, $error ) = CanBookBeRenewed(
$renewing_patron, $issue, 0, undef,
$datedue->clone()->add( days => 3 )
);
is( $renewok, 0, "Requested due date running into the next booking is refused" );
is( $error, 'booked', "Error is 'booked'" );
( $renewok, $error ) = CanBookBeRenewed(
$renewing_patron, $issue, 0, undef,
$datedue->clone()->add( days => 1 )
);
is( $renewok, 1, "Requested due date clear of the next booking is allowed" );
$schema->storage->txn_rollback;
};