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

Bug 23909: Apply hold policy to OPAC REST self-checkout 

The OPAC REST self-checkout path (guarded by OpacTrustedCheckout) is
functionally SCO-over-API, so it should honour the same hold policy
as the SCO web interface. Previously _check_availability unconditionally
promoted RESERVED, RESERVE_WAITING, TRANSFERRED and PROCESSING from
the confirmation hash to the blockers hash for any public caller. This
meant the OPAC REST endpoint always refused held items regardless of
the librarian's AllowItemsOnHoldCheckoutSCO configuration.

With this patch the promotion is gated on AllowItemsOnHoldCheckoutSCO:

* 0 - Promote all four hold confirmations to blockers (previous
      behaviour for pending holds)
* 1 - Promote only waiting/transit/processing; silently allow
      checkout of items with a pending hold
* 2 - Promote none; silently allow checkout of items with any hold

Remaining hold confirmations are stripped from the response so the
caller does not have to resolve a confirmation token for a policy the
admin has already granted.

Test plan:
1. prove t/db_dependent/api/v1/checkouts.t
2. Enable OpacTrustedCheckout. POST an item with a pending hold for
   another patron to /api/v1/public/patrons/X/checkouts:
   - AllowItemsOnHoldCheckoutSCO=0: 403 Checkout not authorized
   - AllowItemsOnHoldCheckoutSCO=1: 201 Created
3. Place a waiting hold for another patron on an item. Repeat:
   - AllowItemsOnHoldCheckoutSCO=1: 403
   - AllowItemsOnHoldCheckoutSCO=2: 201

Signed-off-by: Kristi Krueger <kkrueger@cuyahogalibrary.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-04-20 17:50:43 +01:00 committed by Lucas Gass
parent cd3b0770c5
commit b80dafc210
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 71 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
Koha/REST/V1/Checkouts.pm
View File

@ -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};

55
t/db_dependent/api/v1/checkouts.t
View File

@ -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',