Bug 11634 [QA Followup 3] - Found holds should be considered unavailable

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

C4/Circulation.pm

@@ -2683,7 +2683,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 );
                 }

C4/Reserves.pm

@@ -136,6 +136,8 @@ BEGIN {
         &SuspendAll
 
         &GetReservesControlBranch
+
+        IsItemOnHoldAndFound
     );
     @EXPORT_OK = qw( MergeHolds );
 }    
@@ -205,6 +207,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")){
@@ -248,7 +251,7 @@ sub AddReserve {
         $sth->execute($borrowernumber, $biblionumber, $resdate, $_);
     }
         
-    return;     # FIXME: why not have a useful return value?
+    return $reserve_id;
 }
 
 =head2 GetReserve
@@ -2411,6 +2414,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/>

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);