From be67d56707a47dc68e53ad92b8d6d44704835b9d Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
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 <tomascohen@theke.io>
---
 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 b30fa4c6bee..26fb90aaefb 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 7179d061620..f01d5de427b 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.30.1