From 1630cade3517cec315dcb0d106898ace96ad0191 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Thu, 18 Sep 2014 07:44:36 -0400
Subject: [PATCH] Bug 11634 [QA Followup 3] - Found holds should be considered unavailable

---
 C4/Circulation.pm            |    3 ++-
 C4/Reserves.pm               |   31 +++++++++++++++++++++++++++++--
 t/db_dependent/Circulation.t |   29 +++++++++++++++++++++++++++--
 3 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 7d957fb..26da5e5 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -2674,7 +2674,8 @@ sub CanBookBeRenewed {
         foreach my $b (@borrowernumbers) {
             foreach my $i (@itemnumbers) {
                 if (   IsAvailableForItemLevelRequest($i)
-                    && CanItemBeReserved( $b, $i ) )
+                    && CanItemBeReserved( $b, $i )
+                    && !IsItemOnHoldAndFound($i) )
                 {
                     push( @reservable, $i );
                 }
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index a6704ba..8701cbe 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -119,7 +119,7 @@ BEGIN {
         
         &CheckReserves
         &CanBookBeReserved
-	&CanItemBeReserved
+        &CanItemBeReserved
         &CancelReserve
         &CancelExpiredReserves
 
@@ -135,6 +135,8 @@ BEGIN {
         &SuspendAll
 
         &GetReservesControlBranch
+
+        IsItemOnHoldAndFound
     );
     @EXPORT_OK = qw( MergeHolds );
 }    
@@ -204,6 +206,7 @@ sub AddReserve {
         $const,          $priority,     $notes,   $checkitem,
         $found,          $waitingdate,	$expdate
     );
+    my $reserve_id = $sth->{mysql_insertid};
 
     # Send e-mail to librarian if syspref is active
     if(C4::Context->preference("emailLibrarianWhenHoldIsPlaced")){
@@ -247,7 +250,7 @@ sub AddReserve {
         $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
     }
         
-    return;     # FIXME: why not have a useful return value?
+    return $reserve_id;
 }
 
 =head2 GetReserve
@@ -2336,6 +2339,30 @@ sub CalculatePriority {
     return @row ? $row[0]+1 : 1;
 }
 
+=head2 IsItemOnHoldAndFound
+
+    my $bool = IsItemFoundHold( $itemnumber );
+
+    Returns true if the item is currently on hold
+    and that hold has a non-null found status ( W, T, etc. )
+
+=cut
+
+sub IsItemOnHoldAndFound {
+    my ($itemnumber) = @_;
+
+    my $rs = Koha::Database->new()->schema()->resultset('Reserve');
+
+    my $found = $rs->count(
+        {
+            itemnumber => $itemnumber,
+            found      => { '!=' => undef }
+        }
+    );
+
+    return $found;
+}
+
 =head1 AUTHOR
 
 Koha Development Team <http://koha-community.org/>
diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index a284c4f..8aa2dd3 100755
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -26,7 +26,7 @@ use C4::Reserves;
 use Koha::DateUtils;
 use Koha::Database;
 
-use Test::More tests => 57;
+use Test::More tests => 59;
 
 BEGIN {
     use_ok('C4::Circulation');
@@ -233,7 +233,7 @@ C4::Context->dbh->do("DELETE FROM accountlines");
         $biblionumber
     );
 
-    # Create 2 borrowers
+    # Create borrowers
     my %renewing_borrower_data = (
         firstname =>  'John',
         surname => 'Renewal',
@@ -248,8 +248,16 @@ C4::Context->dbh->do("DELETE FROM accountlines");
         branchcode => $branch,
     );
 
+    my %hold_waiting_borrower_data = (
+        firstname =>  'Kyle',
+        surname => 'Reservation',
+        categorycode => 'S',
+        branchcode => $branch,
+    );
+
     my $renewing_borrowernumber = AddMember(%renewing_borrower_data);
     my $reserving_borrowernumber = AddMember(%reserving_borrower_data);
+    my $hold_waiting_borrowernumber = AddMember(%hold_waiting_borrower_data);
 
     my $renewing_borrower = GetMember( borrowernumber => $renewing_borrowernumber );
 
@@ -282,11 +290,28 @@ C4::Context->dbh->do("DELETE FROM accountlines");
         $title, $checkitem, $found
     );
 
+    # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds
     C4::Context->set_preference('AllowRenewalIfOtherItemsAvailable', 1 );
     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber);
     is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds');
     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber2);
     is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds');
+    # Now let's add a waiting hold on the 3rd item, it's no longer available tp check out by just anyone, so we should no longer
+    # be able to renew these items 
+    my $hold = Koha::Database->new()->schema()->resultset('Reserve')->create(
+        {
+            borrowernumber => $hold_waiting_borrowernumber,
+            biblionumber   => $biblionumber,
+            itemnumber     => $itemnumber3,
+            branchcode     => $branch,
+            priority       => 0,
+            found          => 'W'
+        }
+    );
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber);
+    is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds');
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber2);
+    is( $renewokay, 0, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds');
     C4::Context->set_preference('AllowRenewalIfOtherItemsAvailable', 0 );
 
     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber);
-- 
1.7.2.5