From f2f6a3ebcfd7f606f66c2302e7d1835ca1eb4ada Mon Sep 17 00:00:00 2001 From: Emily Lamancusa Date: Mon, 11 Aug 2025 15:13:11 -0400 Subject: [PATCH] Bug 21572: Update CanBookBeIssued to ignore each hold status separately Adjust CanBookBeIssued parameter $ignore_reserves from a scalar to a hashref, to allow granular override of each hold status. To test: prove t/db_dependent/Circulation.t --- C4/Circulation.pm | 18 ++++++--- Koha/REST/V1/Checkouts.pm | 2 +- t/db_dependent/Circulation.t | 71 +++++++++++++++++++++++++++++++++--- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 51438ac9b6..17f4a02ceb 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -699,7 +699,13 @@ data is keyed in lower case! =item C<$inprocess> boolean switch -=item C<$ignore_reserves> boolean switch +=item C<$ignore_reserves> Hashref of boolean switches + +Available keys: + pending - ignore pending holds + waiting - ignore waiting holds + processing - ignore holds in processing + transferred - ignore holds in transit =item C<$params> Hashref of additional parameters @@ -1211,7 +1217,7 @@ sub CanBookBeIssued { } } - unless ( $ignore_reserves || $recall ) { + unless ($recall) { # See if the item is on reserve. my ( $restype, $res ) = C4::Reserves::CheckReserves($item_object); @@ -1219,7 +1225,7 @@ sub CanBookBeIssued { my $resbor = $res->{'borrowernumber'}; if ( $resbor ne $patron->borrowernumber ) { my $patron = Koha::Patrons->find($resbor); - if ( $restype eq "Waiting" ) { + if ( $restype eq "Waiting" && !$ignore_reserves->{'waiting'} ) { # The item is on reserve and waiting, but has been # reserved by some other patron. @@ -1231,7 +1237,7 @@ sub CanBookBeIssued { $needsconfirmation{'resbranchcode'} = $res->{branchcode}; $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'}; $needsconfirmation{'reserve_id'} = $res->{reserve_id}; - } elsif ( $restype eq "Reserved" ) { + } elsif ( $restype eq "Reserved" && !$ignore_reserves->{'pending'} ) { # The item is on reserve for someone else. $needsconfirmation{RESERVED} = 1; @@ -1242,7 +1248,7 @@ sub CanBookBeIssued { $needsconfirmation{'resbranchcode'} = $res->{branchcode}; $needsconfirmation{'resreservedate'} = $res->{reservedate}; $needsconfirmation{'reserve_id'} = $res->{reserve_id}; - } elsif ( $restype eq "Transferred" ) { + } elsif ( $restype eq "Transferred" && !$ignore_reserves->{'transferred'} ) { # The item is determined hold being transferred for someone else. $needsconfirmation{TRANSFERRED} = 1; @@ -1253,7 +1259,7 @@ sub CanBookBeIssued { $needsconfirmation{'resbranchcode'} = $res->{branchcode}; $needsconfirmation{'resreservedate'} = $res->{reservedate}; $needsconfirmation{'reserve_id'} = $res->{reserve_id}; - } elsif ( $restype eq "Processing" ) { + } elsif ( $restype eq "Processing" && !$ignore_reserves->{'processing'} ) { # The item is determined hold being processed for someone else. $needsconfirmation{PROCESSING} = 1; diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index 8ff774edc2..2dffff8635 100644 --- a/Koha/REST/V1/Checkouts.pm +++ b/Koha/REST/V1/Checkouts.pm @@ -109,7 +109,7 @@ sub _check_availability { my ( $c, $patron, $item ) = @_; my $inprocess = 0; # What does this do? - my $ignore_reserves = 0; # Don't ignore reserves + my $ignore_reserves = undef; # Don't ignore reserves my $params = { item => $item }; my ( $impossible, $confirmation, $alerts, $messages ) = C4::Circulation::CanBookBeIssued( diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 48b4c02d2f..86d5331365 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -7348,7 +7348,7 @@ subtest 'Tests for RecordLocalUseOnReturn' => sub { }; subtest 'Test CanBookBeIssued param ignore_reserves (Bug 35322)' => sub { - plan tests => 4; + plan tests => 16; my $homebranch = $builder->build( { source => 'Branch' } ); my $holdingbranch = $builder->build( { source => 'Branch' } ); @@ -7391,26 +7391,85 @@ subtest 'Test CanBookBeIssued param ignore_reserves (Bug 35322)' => sub { reservation_date => dt_from_string, expiration_date => dt_from_string, itemnumber => $item->id, - found => 'W', } ); + my $reserve_obj = Koha::Holds->find($reserve_id); + set_userenv($holdingbranch); - my ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, 0 ); + my $ignore_reserves = { + 'pending' => 1, + 'waiting' => 1, + 'processing' => 1, + 'transferred' => 1, + }; + + # Test pending hold + my ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, undef ); + is( + keys(%$error) + keys(%$alerts), 0, + 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) + ); + is( exists $question->{RESERVED}, 1, 'RESERVED should be set' ); + + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, $ignore_reserves ); + is( + keys(%$error) + keys(%$alerts), 0, + 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) + ); + isnt( exists $question->{RESERVED}, 1, 'RESERVED should not be set' ); + + # Test processing hold + $reserve_obj->set_processing(); + + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, undef ); + is( + keys(%$error) + keys(%$alerts), 0, + 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) + ); + is( exists $question->{PROCESSING}, 1, 'PROCESSING should be set' ); + + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, $ignore_reserves ); + is( + keys(%$error) + keys(%$alerts), 0, + 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) + ); + isnt( exists $question->{PROCESSING}, 1, 'PROCESSING should not be set' ); + + # Test transferred hold + $reserve_obj->set_transfer(); + + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, undef ); + is( + keys(%$error) + keys(%$alerts), 0, + 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) + ); + is( exists $question->{TRANSFERRED}, 1, 'TRANSFERRED should be set' ); + + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, $ignore_reserves ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) ); - is( exists $question->{RESERVE_WAITING}, 1, 'RESERVE_WAITING is set' ); + isnt( exists $question->{TRANSFERRED}, 1, 'TRANSFERRED should not be set' ); + + # Test waiting hold + $reserve_obj->set_waiting(); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, 1 ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, undef ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) ); - isnt( exists $question->{RESERVE_WAITING}, 1, 'RESERVE_WAITING is not set' ); + is( exists $question->{RESERVE_WAITING}, 1, 'RESERVE_WAITING should be set' ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode, undef, undef, $ignore_reserves ); + is( + keys(%$error) + keys(%$alerts), 0, + 'There should not be any errors or alerts (impossible)' . str( $error, $question, $alerts ) + ); + isnt( exists $question->{RESERVE_WAITING}, 1, 'RESERVE_WAITING should not be set' ); }; subtest 'NoRefundOnLostFinesPaidAge' => sub { -- 2.34.1