From e5d5fbdd3e76bf3df62a5ec4462a482ecbe0545d Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 8 Mar 2021 12:03:01 -0300 Subject: [PATCH] Bug 27797: (QA follow-up) Pickup locations can be overridden too This patch adds regression tests for overridding pickup locations, which was inadvertedly not covered by the original dev. What this does, is moving the $can_override variable definition above, and avoid returning 400 if conditions are not met for the passed pickup_library_id, when $can_override is true. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart --- Koha/REST/V1/Holds.pm | 8 ++++---- t/db_dependent/api/v1/holds.t | 18 +++++++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index d4530e7dd0..579962005b 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -80,6 +80,9 @@ sub add { my $hold_date = $body->{hold_date}; my $non_priority = $body->{non_priority}; + my $overrides = $c->stash('koha.overrides'); + my $can_override = $overrides->{any} && C4::Context->preference('AllowHoldPolicyOverride'); + if(!C4::Context->preference( 'AllowHoldDateInFuture' ) && $hold_date) { return $c->render( status => 400, @@ -160,7 +163,7 @@ sub add { openapi => { error => 'The supplied pickup location is not valid' } - ) unless $valid_pickup_location; + ) unless $valid_pickup_location || $can_override; my $can_place_hold = $item_id @@ -171,9 +174,6 @@ sub add { $can_place_hold->{status} = 'tooManyReserves'; } - my $overrides = $c->stash('koha.overrides'); - my $can_override = $overrides->{any} && C4::Context->preference('AllowHoldPolicyOverride'); - unless ( $can_override || $can_place_hold->{status} eq 'OK' ) { return $c->render( status => 403, diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 1a80452d88..661ff03b34 100755 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -358,7 +358,7 @@ $schema->storage->txn_rollback; subtest 'x-koha-override and AllowHoldPolicyOverride tests' => sub { - plan tests => 12; + plan tests => 16; $schema->storage->txn_begin; @@ -373,15 +373,17 @@ subtest 'x-koha-override and AllowHoldPolicyOverride tests' => sub { $patron->discard_changes; my $userid = $patron->userid; + my $renegade_library = $builder->build_object({ class => 'Koha::Libraries' }); + t::lib::Mocks::mock_preference( 'AllowHoldPolicyOverride', 0 ); # Make sure pickup location checks doesn't get in the middle my $mock_biblio = Test::MockModule->new('Koha::Biblio'); $mock_biblio->mock( 'pickup_locations', - sub { return Koha::Libraries->search; } ); + sub { return Koha::Libraries->search({ branchcode => { '!=' => $renegade_library->branchcode } }); } ); my $mock_item = Test::MockModule->new('Koha::Item'); $mock_item->mock( 'pickup_locations', - sub { return Koha::Libraries->search } ); + sub { return Koha::Libraries->search({ branchcode => { '!=' => $renegade_library->branchcode } }) } ); my $can_item_be_reserved_result; my $mock_reserves = Test::MockModule->new('C4::Reserves'); @@ -433,6 +435,16 @@ subtest 'x-koha-override and AllowHoldPolicyOverride tests' => sub { { 'x-koha-override' => 'any' } => json => $post_data ) ->status_is(201); + # Test pickup locations can be overridden + $post_data->{pickup_library_id} = $renegade_library->branchcode; + + $t->post_ok( "//$userid:$password@/api/v1/holds" => json => $post_data ) + ->status_is(400); + + $t->post_ok( "//$userid:$password@/api/v1/holds" => + { 'x-koha-override' => 'any' } => json => $post_data ) + ->status_is(201); + $schema->storage->txn_rollback; }; -- 2.39.5