diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index ab99808d49..522ff799a8 100644 --- a/Koha/REST/V1/Checkouts.pm +++ b/Koha/REST/V1/Checkouts.pm @@ -120,8 +120,16 @@ sub _check_availability { if ( $c->stash('is_public') ) { # Upgrade some confirmations to blockers - my @should_block = - qw/TOO_MANY ISSUED_TO_ANOTHER RESERVED RESERVE_WAITING TRANSFERRED PROCESSING AGE_RESTRICTION/; + my @should_block = qw/TOO_MANY ISSUED_TO_ANOTHER AGE_RESTRICTION/; + + # Hold-related confirmations are gated by AllowItemsOnHoldCheckoutSCO: + # 0 = block pending and waiting holds (default) + # 1 = allow pending holds, block waiting/transit/processing holds + # 2 = allow pending and waiting/transit/processing holds + my $hold_pref = C4::Context->preference('AllowItemsOnHoldCheckoutSCO') || 0; + push @should_block, 'RESERVED' if $hold_pref < 1; + push @should_block, qw/RESERVE_WAITING TRANSFERRED PROCESSING/ if $hold_pref < 2; + for my $block (@should_block) { if ( exists( $confirmation->{$block} ) ) { $impossible->{$block} = $confirmation->{$block}; @@ -129,9 +137,15 @@ sub _check_availability { } } + # Drop the remaining hold confirmations so the caller does not have + # to resolve a confirmation token for a policy we've already permitted + for my $allowed (qw/RESERVED RESERVE_WAITING TRANSFERRED PROCESSING/) { + delete $confirmation->{$allowed}; + } + # Remove any non-public info that's returned by CanBookBeIssued my @restricted_keys = - qw/issued_borrowernumber issued_cardnumber issued_firstname issued_surname resborrowernumber resbranchcode rescardnumber reserve_id resfirstname resreservedate ressurname item_notforloan/; + qw/issued_borrowernumber issued_cardnumber issued_firstname issued_surname resborrowernumber resbranchcode rescardnumber reserve_id resfirstname resreservedate reswaitingdate ressurname item_notforloan/; for my $key (@restricted_keys) { delete $confirmation->{$key}; delete $impossible->{$key}; diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index adf7068ba1..a672f9e9f8 100755 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -380,7 +380,7 @@ subtest 'get_availability' => sub { %needsconfirmation = (); subtest 'public availability' => sub { - plan tests => 22; + plan tests => 40; # Authentication required $t->get_ok("/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id")->status_is(401); @@ -411,6 +411,59 @@ subtest 'get_availability' => sub { ->json_has('/confirmation_token'); %needsconfirmation = (); + # Hold confirmations are gated by AllowItemsOnHoldCheckoutSCO. + # Because the mocked CanBookBeIssued returns the hashes by reference + # and _check_availability mutates them, we reset both hashes before + # every request. + my $reset_hold_hashes = sub { + %issuingimpossible = (); + %needsconfirmation = ( + RESERVED => 1, + RESERVE_WAITING => 1, + TRANSFERRED => 1, + PROCESSING => 1 + ); + }; + + $reset_hold_hashes->(); + t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSCO', 0 ); + $t->get_ok( + "//$unauth_userid:$unauth_password@/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" + )->status_is(200)->json_is( + '/blockers' => { + RESERVED => 1, + RESERVE_WAITING => 1, + TRANSFERRED => 1, + PROCESSING => 1 + } + )->json_is( '/confirms' => {} )->json_is( '/warnings' => {} )->json_has('/confirmation_token'); + + $reset_hold_hashes->(); + t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSCO', 1 ); + $t->get_ok( + "//$unauth_userid:$unauth_password@/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" + )->status_is(200)->json_is( + '/blockers' => { + RESERVE_WAITING => 1, + TRANSFERRED => 1, + PROCESSING => 1 + } + )->json_is( '/confirms' => {} )->json_is( '/warnings' => {} )->json_has('/confirmation_token'); + + $reset_hold_hashes->(); + t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSCO', 2 ); + $t->get_ok( + "//$unauth_userid:$unauth_password@/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" + ) + ->status_is(200) + ->json_is( '/blockers' => {} ) + ->json_is( '/confirms' => {} ) + ->json_is( '/warnings' => {} ) + ->json_has('/confirmation_token'); + + %issuingimpossible = (); + %needsconfirmation = (); + # Remove personal information from public endpoint %issuingimpossible = ( issued_borrowernumber => 'private',