From 05ebdc7e6a534678e3e6457e26f23bcadae39367 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 17 Jul 2025 15:24:44 +0200 Subject: [PATCH] Bug 37651: (follow-up) Still skip future holds for renewals We need a skip_future_holds flag in item->current_holds to make renewals act as they did before. There is no need to add it to biblio->current_holds. Test plan: Run t/db_dependent/Holds.t Interface test: Issue a book, place hold on tomorrow, set ConfirmFutureHolds to 2 days. Verify that renew is not blocked by this hold. Signed-off-by: Marcel de Rooy Signed-off-by: Nick Clemens --- C4/Circulation.pm | 3 ++- Koha/Item.pm | 10 ++++++---- t/db_dependent/Holds.t | 15 ++++++++++++++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 48185eb0224..d03e6090edc 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3205,8 +3205,9 @@ sub CanBookBeRenewed { } # There is an item level hold on this item, no other item can fill the hold + # The flag skip_future_holds may be removed in the future. See bug 40435. return ( 0, "on_reserve" ) - if ( $item->current_holds->search( { non_priority => 0 } )->count ); + if ( $item->current_holds( { skip_future_holds => 1 } )->search( { non_priority => 0 } )->count ); my ( $status, $matched_reserve, $possible_holds ) = C4::Reserves::CheckReserves($item); my @fillable_holds = (); diff --git a/Koha/Item.pm b/Koha/Item.pm index 8b4f1585afd..df6a4a52d9d 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1174,11 +1174,13 @@ Respects the lookahead days in ConfirmFutureHolds pref. =cut sub current_holds { - my ($self) = @_; + my ( $self, $params ) = @_; + my $attributes = { order_by => 'priority' }; my $dtf = Koha::Database->new->schema->storage->datetime_parser; - my $dt = dt_from_string()->add( days => C4::Context->preference('ConfirmFutureHolds') || 0 ); - my $params = { + my $days = $params->{skip_future_holds} ? 0 : C4::Context->preference('ConfirmFutureHolds') || 0; + my $dt = dt_from_string()->add( days => $days ); + my $s_params = { itemnumber => $self->itemnumber, suspend => 0, -or => [ @@ -1186,7 +1188,7 @@ sub current_holds { waitingdate => { '!=' => undef }, ], }; - my $hold_rs = $self->_result->reserves->search( $params, $attributes ); + my $hold_rs = $self->_result->reserves->search( $s_params, $attributes ); return Koha::Holds->_new_from_dbic($hold_rs); } diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 197ded172c4..52a5158d2e1 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -2111,7 +2111,7 @@ subtest 'EmailPatronWhenHoldIsPlaced tests' => sub { }; subtest 'current_holds with future holds' => sub { - plan tests => 2; + plan tests => 5; $schema->storage->txn_begin; t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 2 ); @@ -2130,5 +2130,18 @@ subtest 'current_holds with future holds' => sub { t::lib::Mocks::mock_preference( 'ConfirmFutureHolds', 0 ); is( $item->biblio->current_holds->count, 0, 'No future hold was counted when ConfirmFutureHolds is zero' ); + # Test if renewal ignores future holds (until we decide otherwise; see bug 40435) + my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); + t::lib::Mocks::mock_userenv( { branchcode => $patron2->branchcode } ); + my $issue = C4::Circulation::AddIssue( $patron2, $item->barcode, dt_from_string() ); + my ( $ok, $err ) = CanBookBeRenewed( $patron2, $issue ); + is( $ok, 1, 'Hold does not block renewal' ); + + # Changing the reservedate should block the renewal + Koha::Holds->find($hold_id)->reservedate( dt_from_string() )->store; + ( $ok, $err ) = CanBookBeRenewed( $patron2, $issue ); + is( $ok, 0, 'Hold now blocks renewal' ); + is( $err, 'on_reserve', 'Code for blocking renewal' ); + $schema->storage->txn_rollback; }; -- 2.39.5