From dd2185981de2bf6e8d14bcdd4b6ae0da6cf2cbed Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Wed, 29 May 2013 14:38:40 +0200
Subject: [PATCH] Bug 9394: QA Followup

* C4::Reserves::_FixPriority
  - The previous code checked the cancellationdate. If think you never pass
in it with bad parameters, but in order to be sure I added the check on
this value.
  - The reservedates array was never used.

* circ/circulation.tt
There was a bug: it was not possible to remove an hold from the
circulation page. Passing reserve_id fixes the issue.

* C4::Reserves::GetReserveId
This subroutine did not have a unit test.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
---
 C4/Reserves.pm                                    | 11 +++++++----
 .../prog/en/modules/circ/circulation.tt           |  2 +-
 t/db_dependent/Holds.t                            | 15 +++++++++++----
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 813b1ccb4d..13787fc109 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1466,9 +1466,14 @@ sub AlterPriority {
 
     my $reserve = GetReserve( $reserve_id );
 
+    if ( $reserve->{cancellationdate} ) {
+        warn "I cannot alter the priority for reserve_id $reserve_id, the reserve has been cancelled (".$reserve->{cancellationdate}.')';
+        return;
+    }
+
     if ( $where eq 'up' || $where eq 'down' ) {
-    
-      my $priority = $reserve->{'priority'};        
+
+      my $priority = $reserve->{'priority'};
       $priority = $where eq 'up' ? $priority - 1 : $priority + 1;
       _FixPriority( $reserve_id, $priority )
 
@@ -1628,7 +1633,6 @@ sub _FixPriority {
         $sth->execute( $reserve_id );
     }
     my @priority;
-    my @reservedates;
 
     # get whats left
     my $query = "
@@ -1641,7 +1645,6 @@ sub _FixPriority {
     my $sth = $dbh->prepare($query);
     $sth->execute( $res->{'biblionumber'} );
     while ( my $line = $sth->fetchrow_hashref ) {
-        push( @reservedates, $line );
         push( @priority,     $line );
     }
 
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
index c243487d20..290c4c3de7 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ -1136,7 +1136,7 @@ No patron matched <span class="ex">[% message %]</span>
                 </select>
                 <input type="hidden" name="biblionumber" value="[% reservloo.biblionumber %]" />
                 <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" />
-                <input type="hidden" name="reservenumber" value="[% reservloo.reservenumber %]" />
+                <input type="hidden" name="reserve_id" value="[% reservloo.reserve_id %]" />
             </td>
             <td>[% IF ( reservloo.suspend ) %]Suspended [% IF ( reservloo.suspend_until ) %] until [% reservloo.suspend_until | $KohaDates %][% END %][% END %]</td>
             </tr>
diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t
index 801a55c31f..c7a95fd1e0 100755
--- a/t/db_dependent/Holds.t
+++ b/t/db_dependent/Holds.t
@@ -4,15 +4,15 @@ use strict;
 use warnings;
 use C4::Branch;
 
-use Test::More tests => 18;
+use Test::More tests => 19;
 use MARC::Record;
 use C4::Biblio;
 use C4::Items;
 
 BEGIN {
-      use FindBin;
-   use lib $FindBin::Bin;
- use_ok('C4::Reserves');
+    use FindBin;
+    use lib $FindBin::Bin;
+    use_ok('C4::Reserves');
 }
 
 my $borrowers_count = 5;
@@ -121,6 +121,13 @@ AddReserve(
     my $found,
 );
 ( $reserve ) = GetReservesFromBorrowernumber($borrowernumber);
+my $reserveid = C4::Reserves::GetReserveId(
+    {
+        biblionumber => $biblionumber,
+        borrowernumber => $borrowernumber
+    }
+);
+is( $reserveid, $reserve->{reserve_id}, "Test GetReserveId" );
 ModReserveMinusPriority( $itemnumber, $reserve->{'reserve_id'} );
 ( $reserve ) = GetReservesFromBorrowernumber($borrowernumber);
 ok( $reserve->{'itemnumber'} eq $itemnumber, "Test ModReserveMinusPriority()" );
-- 
2.39.5