From 0f01ff22bb8153ff79eb1bd9c2e4e0a4fb5129ad Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Thu, 13 Jun 2013 09:51:51 +0000
Subject: [PATCH] Bug 8868: ILS-DI: CancelHold needs to take a reserve_id (follow-up)

- Fix the text in the ilsdi.pl?service=Describe&verb=CancelHold page
- Unit tests for CancelReservedFromId and GetReserve
- Do not delete row in reserves table if insert in old_reserves fails
---
 C4/Reserves.pm                               |   21 +++++++++++++--------
 koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt |    2 +-
 t/db_dependent/Reserves.t                    |   20 +++++++++++++++++++-
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 8e90b17..b3e7ead 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1111,14 +1111,19 @@ sub CancelReserveFromId {
             WHERE reserve_id = ?
         };
         $sth = $dbh->prepare($query);
-        $sth->execute($reserve_id);
-
-        $query = qq{
-            DELETE FROM reserves
-            WHERE reserve_id = ?
-        };
-        $sth = $dbh->prepare($query);
-        $sth->execute($reserve_id);
+        my $rv = $sth->execute($reserve_id);
+
+        if ($rv) {
+            $query = qq{
+                DELETE FROM reserves
+                WHERE reserve_id = ?
+            };
+            $sth = $dbh->prepare($query);
+            $sth->execute($reserve_id);
+        } else {
+            warn "ERROR: Failed to move reserve $reserve_id to table old_reserves.";
+            return;
+        }
 
         # now fix the priority on the others...
         _FixPriority( $reserve->{biblionumber}, $reserve->{borrowernumber} );
diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt b/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt
index 9311599..1e57466 100644
--- a/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt
+++ b/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt
@@ -671,7 +671,7 @@
                     <dt><strong>patron_id</strong> (Required)</dt>
                     <dd>the unique patron identifier in the ILS; the same identifier returned by LookupPatron or AuthenticatePatron</dd>
                     <dt><strong>item_id</strong> (Required)</dt>
-                    <dd>system item identifier</dd>
+                    <dd>system hold identifier (returned by GetRecords and GetPatronInfo into element 'reserve_id')</dd>
                 </dl>
                 <h4>Example call</h4>
                 <a href="ilsdi.pl?service=CancelHold&amp;patron_id=1&amp;item_id=1">
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 1e20318..a39d106 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 use C4::Branch;
 
-use Test::More tests => 4;
+use Test::More tests => 12;
 use MARC::Record;
 use C4::Biblio;
 use C4::Items;
@@ -66,6 +66,24 @@ ok($status eq "Reserved", "CheckReserves Test 2");
 ($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode);
 ok($status eq "Reserved", "CheckReserves Test 3");
 
+my (undef, $reserves) = GetReservesFromBiblionumber($biblionumber);
+isa_ok($reserves, 'ARRAY');
+is(scalar @$reserves, 1, "Only one reserves for this biblio");
+my $reserve_id = $reserves->[0]->{reserve_id};
+
+$reserve = GetReserve($reserve_id);
+isa_ok($reserve, 'HASH', "GetReserve return");
+is($reserve->{biblionumber}, $biblionumber);
+
+$reserve = CancelReserveFromId($reserve_id);
+isa_ok($reserve, 'HASH', "CancelReserveFromId return");
+is($reserve->{biblionumber}, $biblionumber);
+
+$reserve = GetReserve($reserve_id);
+is($reserve, undef, "GetReserve returns undef after deletion");
+
+$reserve = CancelReserveFromId($reserve_id);
+is($reserve, undef, "CancelReserveFromId return undef if reserve does not exist");
 
 # Teardown Test---------------------
 # Delete item.
-- 
1.7.2.5