From 481bff7e57471ec687fe9b397483a617ddcaf66f Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Tue, 19 Jan 2016 17:10:29 +0000
Subject: [PATCH] Bug 14695 [QA Followup] - Filling one hold fills all of them

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
---
 C4/Reserves.pm                                     | 87 ++++++++++------------
 circ/returns.pl                                    | 10 +--
 .../intranet-tmpl/prog/en/modules/circ/returns.tt  |  4 +-
 3 files changed, 48 insertions(+), 53 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index a9827b6..aa2e0bc 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1312,7 +1312,7 @@ take care of the waiting status
 =cut
 
 sub ModReserveAffect {
-    my ( $itemnumber, $borrowernumber,$transferToDo ) = @_;
+    my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id ) = @_;
     my $dbh = C4::Context->dbh;
 
     # we want to attach $itemnumber to $borrowernumber, find the biblionumber
@@ -1323,44 +1323,52 @@ sub ModReserveAffect {
 
     # get request - need to find out if item is already
     # waiting in order to not send duplicate hold filled notifications
-    my $reserve_id = GetReserveId({
-        borrowernumber => $borrowernumber,
-        biblionumber   => $biblionumber,
-    });
-    return unless defined $reserve_id;
-    my $request = GetReserveInfo($reserve_id);
-    my $already_on_shelf = ($request && $request->{found} eq 'W') ? 1 : 0;
+
+    my $hold;
+    # Find hold by id if we have it
+    $hold = Koha::Holds->find( $reserve_id ) if $reserve_id;
+    # Find item level hold for this item if there is one
+    $hold ||= Koha::Holds->search( { borrowernumber => $borrowernumber, itemnumber => $itemnumber } )->next();
+    # Find record level hold if there is no item level hold
+    $hold ||= Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->next();
+
+    return unless $hold;
+
+    $reserve_id = $hold->id();
+
+    my $already_on_shelf = $hold->found && $hold->found eq 'W';
 
     # If we affect a reserve that has to be transferred, don't set to Waiting
     my $query;
     if ($transferToDo) {
-    $query = "
-        UPDATE reserves
-        SET    priority = 0,
-               itemnumber = ?,
-               found = 'T'
-        WHERE borrowernumber = ?
-          AND biblionumber = ?
-    ";
+        $hold->set(
+            {
+                priority   => 0,
+                itemnumber => $itemnumber,
+                found      => 'T',
+            }
+        );
     }
     else {
-    # affect the reserve to Waiting as well.
-        $query = "
-            UPDATE reserves
-            SET     priority = 0,
-                    found = 'W',
-                    waitingdate = NOW(),
-                    itemnumber = ?
-            WHERE borrowernumber = ?
-              AND biblionumber = ?
-        ";
+        # affect the reserve to Waiting as well.
+        $hold->set(
+            {
+                priority    => 0,
+                itemnumber  => $itemnumber,
+                found       => 'W',
+                waitingdate => dt_from_string(),
+            }
+        );
     }
-    $sth = $dbh->prepare($query);
-    $sth->execute( $itemnumber, $borrowernumber,$biblionumber);
-    _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo && !$already_on_shelf );
+    $hold->store();
+
+    _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber )
+      if ( !$transferToDo && !$already_on_shelf );
+
     _FixPriority( { biblionumber => $biblionumber } );
+
     if ( C4::Context->preference("ReturnToShelvingCart") ) {
-      CartToShelf( $itemnumber );
+        CartToShelf($itemnumber);
     }
 
     return;
@@ -2330,26 +2338,13 @@ sub GetReserveId {
 
     return unless ( ( $params->{'biblionumber'} || $params->{'itemnumber'} ) && $params->{'borrowernumber'} );
 
-    my $dbh = C4::Context->dbh();
-
-    my $sql = "SELECT reserve_id FROM reserves WHERE ";
-
-    my @params;
-    my @limits;
     foreach my $key ( keys %$params ) {
-        if ( defined( $params->{$key} ) ) {
-            push( @limits, "$key = ?" );
-            push( @params, $params->{$key} );
-        }
+        delete $params->{$key} unless defined( $params->{$key} );
     }
 
-    $sql .= join( " AND ", @limits );
-
-    my $sth = $dbh->prepare( $sql );
-    $sth->execute( @params );
-    my $row = $sth->fetchrow_hashref();
+    my $hold = Koha::Holds->search( $params )->next();
 
-    return $row->{'reserve_id'};
+    return $hold->id();
 }
 
 =head2 ReserveSlip
diff --git a/circ/returns.pl b/circ/returns.pl
index 7442206..8688c97 100755
--- a/circ/returns.pl
+++ b/circ/returns.pl
@@ -142,10 +142,10 @@ if ($query->param('WT-itemNumber')){
 	updateWrongTransfer ($query->param('WT-itemNumber'),$query->param('WT-waitingAt'),$query->param('WT-From'));
 }
 
-if ( $query->param('resbarcode') ) {
+if ( $query->param('reserve_id') ) {
     my $item           = $query->param('itemnumber');
     my $borrowernumber = $query->param('borrowernumber');
-    my $resbarcode     = $query->param('resbarcode');
+    my $reserve_id     = $query->param('reserve_id');
     my $diffBranchReturned = $query->param('diffBranch');
     my $iteminfo   = GetBiblioFromItemNumber($item);
     my $cancel_reserve = $query->param('cancel_reserve');
@@ -153,12 +153,12 @@ if ( $query->param('resbarcode') ) {
     $iteminfo->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $iteminfo->{'itype'} : $iteminfo->{'itemtype'};
 
     if ( $cancel_reserve ) {
-        CancelReserve({ borrowernumber => $borrowernumber, itemnumber => $item, charge_cancel_fee => !$forgivemanualholdsexpire });
+        CancelReserve({ reserve_id => $reserve_id, charge_cancel_fee => !$forgivemanualholdsexpire });
     } else {
         my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef;
         # diffBranchSend tells ModReserveAffect whether document is expected in this library or not,
         # i.e., whether to apply waiting status
-        ModReserveAffect( $item, $borrowernumber, $diffBranchSend);
+        ModReserveAffect( $item, $borrowernumber, $diffBranchSend, $reserve_id );
     }
 #   check if we have other reserves for this document, if we have a return send the message of transfer
     my ( $messages, $nextreservinfo ) = GetOtherReserves($item);
@@ -438,7 +438,7 @@ if ( $messages->{'ResFound'}) {
             $template->param(
                 intransit    => ($userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ),
                 transfertodo => ($userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ),
-                resbarcode   => $barcode,
+                reserve_id   => $reserve->{reserve_id},
                 reserved     => 1,
             );
         }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
index b490c05..8abb1ef 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
@@ -260,7 +260,7 @@ $(document).ready(function () {
                 <input type="hidden" name="itemnumber" value="[% itemnumber %]" />
                 <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" />
                 <input type="hidden" name="biblionumber" value="[% itembiblionumber %]" />
-                <input type="hidden" name="resbarcode" value="[% barcode %]" />
+                <input type="hidden" name="reserve_id" value="[% reserve_id %]" />
                 <input type="hidden" name="diffBranch" value="[% destbranch %]" />
                 <input type="hidden" name="exemptfine" value="[% exemptfine %]" />
                 <input type="hidden" name="dropboxmode" value="[% dropboxmode %]" />
@@ -422,7 +422,7 @@ $(document).ready(function () {
             <input type="hidden" name="itemnumber" value="[% itemnumber %]" />
             <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" />
             <input type="hidden" name="biblionumber" value="[% itembiblionumber %]" />
-            <input type="hidden" name="resbarcode" value="[% barcode %]" />
+            <input type="hidden" name="reserve_id" value="[% reserve_id %]" />
             <input type="hidden" name="diffBranch" value="[% destbranch %]" />
             <input type="hidden" name="exemptfine" value="[% exemptfine %]" />
             <input type="hidden" name="dropboxmode" value="[% dropboxmode %]" />
-- 
2.1.4