From a3ec118dd9b8b6ff34ab721e81198dafb92c703e Mon Sep 17 00:00:00 2001
From: Agustin Moyano <agustinmoyano@theke.io>
Date: Thu, 3 Oct 2019 14:14:47 -0300
Subject: [PATCH] Bug 23710: (follow-up) Add tests for new features in
 Koha::REST::V!::Holds::add and return error when hold date in future is not
 allowed and it is passed as parameter

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/REST/V1/Holds.pm         |  9 ++++-
 t/db_dependent/api/v1/holds.t | 66 +++++++++++++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm
index 04d1014b6a..00faf2ffed 100644
--- a/Koha/REST/V1/Holds.pm
+++ b/Koha/REST/V1/Holds.pm
@@ -84,7 +84,14 @@ sub add {
         my $item_type         = $body->{item_type};
         my $expiration_date   = $body->{expiration_date};
         my $notes             = $body->{notes};
-        my $hold_date         = C4::Context->preference( 'AllowHoldDateInFuture' )?$body->{hold_date}:undef;
+        my $hold_date         = $body->{hold_date};
+
+        if(!C4::Context->preference( 'AllowHoldDateInFuture' ) && $hold_date) {
+            return $c->render(
+                status  => 400,
+                openapi => { error => "Hold date in future not allowed" }
+            );
+        }
 
         if ( $item_id and $biblio_id ) {
 
diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t
index 099757934c..a1234dae5c 100644
--- a/t/db_dependent/api/v1/holds.t
+++ b/t/db_dependent/api/v1/holds.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 6;
+use Test::More tests => 8;
 use Test::Mojo;
 use t::lib::TestBuilder;
 use t::lib::Mocks;
@@ -34,6 +34,7 @@ use Koha::DateUtils;
 use Koha::Biblios;
 use Koha::Biblioitems;
 use Koha::Items;
+use Koha::CirculationRules;
 
 my $schema  = Koha::Database->new->schema;
 my $builder = t::lib::TestBuilder->new();
@@ -267,6 +268,67 @@ subtest 'Reserves with itemtype' => sub {
       ->json_is('/0/item_type', $itemtype);
 };
 
+
+subtest 'test AllowHoldDateInFuture' => sub {
+
+    plan tests => 6;
+
+    $dbh->do('DELETE FROM reserves');
+
+    my $future_hold_date = DateTime->now->add(days => 10)->truncate( to => 'day' );
+
+    my $post_data = {
+        patron_id => int($patron_1->borrowernumber),
+        biblio_id => int($biblio_1->biblionumber),
+        item_id => int($item_1->itemnumber),
+        pickup_library_id => $branchcode,
+        expiration_date => output_pref({ dt => $expiration_date, dateformat => 'rfc3339', dateonly => 1 }),
+        hold_date => output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 }),
+        priority => 2,
+    };
+
+    t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 0 );
+
+    $t->post_ok( "//$userid_3:$password@/api/v1/holds" => json => $post_data )
+      ->status_is(400)
+      ->json_has('/error');
+
+    t::lib::Mocks::mock_preference( 'AllowHoldDateInFuture', 1 );
+
+    $t->post_ok( "//$userid_3:$password@/api/v1/holds" => json => $post_data )
+      ->status_is(201)
+      ->json_is('/hold_date', output_pref({ dt => $future_hold_date, dateformat => 'rfc3339', dateonly => 1 }));
+};
+
+subtest 'test AllowHoldPolicyOverride' => sub {
+
+    plan tests => 5;
+
+    $dbh->do('DELETE FROM reserves');
+
+    Koha::CirculationRules->set_rules(
+        {
+            categorycode => undef,
+            itemtype     => undef,
+            branchcode   => undef,
+            rules        => {
+                holdallowed              => 1
+            }
+        }
+    );
+
+    t::lib::Mocks::mock_preference( 'AllowHoldPolicyOverride', 0 );
+
+    $t->post_ok( "//$userid_3:$password@/api/v1/holds" => json => $post_data )
+      ->status_is(403)
+      ->json_has('/error');
+
+    t::lib::Mocks::mock_preference( 'AllowHoldPolicyOverride', 1 );
+
+    $t->post_ok( "//$userid_3:$password@/api/v1/holds" => json => $post_data )
+      ->status_is(201);
+};
+
 $schema->storage->txn_rollback;
 
 subtest 'suspend and resume tests' => sub {
@@ -442,4 +504,4 @@ subtest 'PUT /holds/{hold_id}/priority tests' => sub {
     is( $hold_3->discard_changes->priority, 3, 'Priority adjusted correctly' );
 
     $schema->storage->txn_rollback;
-};
+};
\ No newline at end of file
-- 
2.39.5