From ac840a846011f8f6f24686ef68f73e7b59f9704e Mon Sep 17 00:00:00 2001
From: Alex Arnaud <alex.arnaud@biblibre.com>
Date: Wed, 10 Jan 2018 15:31:54 +0000
Subject: [PATCH] Bug 19945: ILSDI - Return the reason a reserve is impossible

Currently, the ILDSI services HoldTitle and HoldItem always return a
"NotHoldable" code is the reserve is impossible. We need to know why

Test plan:

 - Apply this patch
 - Place a hold on a non reservable title using ILS-DI web service
   (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=)
 - you should get the reason instead of NotHoldable,
 - Place a hold on a non reservable item using ILS-DI web service
   (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1)
 - you should get the reason instead of NotHoldable,

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
---
 C4/ILSDI/Services.pm            | 10 ++--
 t/db_dependent/ILSDI_Services.t | 91 ++++++++++++++++++++++++++-------
 2 files changed, 79 insertions(+), 22 deletions(-)

diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm
index 689973ea3e..4b03330428 100644
--- a/C4/ILSDI/Services.pm
+++ b/C4/ILSDI/Services.pm
@@ -663,7 +663,8 @@ sub HoldTitle {
     my $title = $biblio ? $biblio->title : '';
 
     # Check if the biblio can be reserved
-    return { code => 'NotHoldable' } unless CanBookBeReserved( $borrowernumber, $biblionumber )->{status} eq 'OK';
+    my $code = CanBookBeReserved( $borrowernumber, $biblionumber )->{status};
+    return { code => $code } unless ( $code eq 'OK' );
 
     my $branch;
 
@@ -739,9 +740,10 @@ sub HoldItem {
     return { code => 'RecordNotFound' } if $$item{biblionumber} ne $biblio->biblionumber;
 
     # Check for item disponibility
-    my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber );
-    my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber );
-    return { code => 'NotHoldable' } unless $canbookbereserved->{status} eq 'OK' and $canitembereserved->{status} eq 'OK';
+    my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber )->{status};
+    my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber )->{status};
+    return { code => $canitembereserved } unless $canitembereserved eq 'OK';
+    return { code => $canbookbereserved } unless $canbookbereserved eq 'OK';
 
     # Pickup branch management
     my $branch;
diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t
index 232a54a699..6891660d31 100644
--- a/t/db_dependent/ILSDI_Services.t
+++ b/t/db_dependent/ILSDI_Services.t
@@ -297,41 +297,96 @@ subtest 'LookupPatron test' => sub {
     $schema->storage->txn_rollback;
 };
 
-# This is a stub, as it merely is for triggering the GetMarcBiblio call.
-subtest 'GetRecords' => sub {
+subtest 'Holds test' => sub {
 
-    plan tests => 1;
+    plan tests => 3;
 
     $schema->storage->txn_begin;
 
+    t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 );
+
+    my $patron = $builder->build({
+        source => 'Borrower',
+    });
+
     my $biblio = $builder->build({
         source => 'Biblio',
-        value  => {
-            title => 'Title 1',    },
+    });
+
+    my $biblioitems = $builder->build({
+        source => 'Biblioitem',
+        value => {
+            biblionumber => $biblio->{biblionumber},
+        }
     });
 
     my $item = $builder->build({
         source => 'Item',
-        value  => {
+        value => {
             biblionumber => $biblio->{biblionumber},
-        },
+            damaged => 1
+        }
     });
 
-    my $biblioitem = $builder->build({
-        source => 'Biblioitem',
-        value  => {
+    my $query = new CGI;
+    $query->param( 'patron_id', $patron->{borrowernumber});
+    $query->param( 'bib_id', $biblio->{biblionumber});
+
+    my $reply = C4::ILSDI::Services::HoldTitle( $query );
+    is( $reply->{code}, 'damaged', "Item damaged" );
+
+    my $item_o = Koha::Items->find($item->{itemnumber});
+    $item_o->damaged(0)->store;
+
+    my $hold = $builder->build({
+        source => 'Reserve',
+        value => {
+            borrowernumber => $patron->{borrowernumber},
             biblionumber => $biblio->{biblionumber},
-            itemnumber   => $item->{itemnumber},
-        },
+            itemnumber => $item->{itemnumber}
+        }
+    });
+
+    $reply = C4::ILSDI::Services::HoldTitle( $query );
+    is( $reply->{code}, 'itemAlreadyOnHold', "Item already on hold" );
+
+    my $biblio2 = $builder->build({
+        source => 'Biblio',
+    });
+
+    my $biblioitems2 = $builder->build({
+        source => 'Biblioitem',
+        value => {
+            biblionumber => $biblio2->{biblionumber},
+        }
     });
 
-    my $query = CGI->new({
-        'schema' => 'MARCXML',
-        'id'     => [ $biblio->{biblionumber} ]
+    my $item2 = $builder->build({
+        source => 'Item',
+        value => {
+            biblionumber => $biblio2->{biblionumber},
+            damaged => 0
+        }
+    });
+
+    t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' );
+    my $issuingrule = $builder->build({
+        source => 'Issuingrule',
+        value => {
+            categorycode => $patron->{categorycode},
+            itemtype => $item2->{itype},
+            branchcode => $patron->{branchcode},
+            reservesallowed => 0,
+        }
     });
 
-    my $result = C4::ILSDI::Services::GetRecords($query);
-    ok($result,'There is a result');
+    $query = new CGI;
+    $query->param( 'patron_id', $patron->{borrowernumber});
+    $query->param( 'bib_id', $biblio2->{biblionumber});
+    $query->param( 'item_id', $item2->{itemnumber});
+
+    $reply = C4::ILSDI::Services::HoldItem( $query );
+    is( $reply->{code}, 'tooManyReserves', "Too many reserves" );
 
     $schema->storage->txn_rollback;
-}
+};
-- 
2.39.5