From 0f6ff541047c0afa83077e0e8d0d0860e6a4ffd7 Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Thu, 17 Apr 2014 12:10:21 -0400
Subject: [PATCH] Bug 12086 - Hold priorities incorrect, when waiting status
 was reversed

1) Test record has 1 single item, checked out to patron X
2) Place 3 holds for patrons A, B and C, all title level hold this time
   A, B, C, item branches and staff branch are the same.
3) Return item, confirm hold
4) Confirm item is now waiting for patron A
   Priorities are: A = Waiting, B = 1, C = 2
5) Open patron account of user B, checkout book
   Koha asks: Item X has been waiting for patron A... Revert
   waiting status
   Confirm.
6) Check priorities:
   Hold list shows: A = 1, C = 1
   Database says: A = 1, C = 3
7) Apply this patch
8) Repeat steps 1-6
9) Note the priorities are correct

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Test plan correctly predicts the error and the correction made by the
patch.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
---
 C4/Reserves.pm                             |   5 +-
 t/db_dependent/Holds/RevertWaitingStatus.t | 100 +++++++++++++++++++++
 2 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100755 t/db_dependent/Holds/RevertWaitingStatus.t

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index d399ee04e2..a98eed4704 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -2104,7 +2104,7 @@ sub MergeHolds {
 
 =head2 RevertWaitingStatus
 
-  $success = RevertWaitingStatus({ itemnumber => $itemnumber });
+  RevertWaitingStatus({ itemnumber => $itemnumber });
 
   Reverts a 'waiting' hold back to a regular hold with a priority of 1.
 
@@ -2159,7 +2159,8 @@ sub RevertWaitingStatus {
       reserve_id = ?
     ";
     $sth = $dbh->prepare( $query );
-    return $sth->execute( $reserve->{'reserve_id'} );
+    $sth->execute( $reserve->{'reserve_id'} );
+    _FixPriority( { biblionumber => $reserve->{biblionumber} } );
 }
 
 =head2 GetReserveId
diff --git a/t/db_dependent/Holds/RevertWaitingStatus.t b/t/db_dependent/Holds/RevertWaitingStatus.t
new file mode 100755
index 0000000000..4adde496e0
--- /dev/null
+++ b/t/db_dependent/Holds/RevertWaitingStatus.t
@@ -0,0 +1,100 @@
+#!/usr/bin/perl
+
+use Modern::Perl;
+
+use t::lib::Mocks;
+use C4::Context;
+use C4::Branch;
+
+use Test::More tests => 3;
+use MARC::Record;
+use C4::Biblio;
+use C4::Items;
+use C4::Members;
+use C4::Reserves;
+
+my $dbh = C4::Context->dbh;
+
+# Start transaction
+$dbh->{AutoCommit} = 0;
+$dbh->{RaiseError} = 1;
+
+$dbh->do("DELETE FROM reserves");
+$dbh->do("DELETE FROM old_reserves");
+
+*C4::Context::userenv = \&Mock_userenv;
+
+sub Mock_userenv {
+    my $userenv = { flags => 1, id => '1', branch => 'CPL' };
+    return $userenv;
+}
+
+my $borrowers_count = 3;
+
+# Setup Test------------------------
+# Helper biblio.
+diag("Creating biblio instance for testing.");
+my ( $bibnum, $title, $bibitemnum ) = create_helper_biblio();
+
+# Helper item for that biblio.
+diag("Creating item instance for testing.");
+my $item_barcode = 'my_barcode';
+my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem(
+    { homebranch => 'CPL', holdingbranch => 'CPL', barcode => $item_barcode },
+    $bibnum );
+
+# Create some borrowers
+my @borrowernumbers;
+foreach my $i ( 1 .. $borrowers_count ) {
+    my $borrowernumber = AddMember(
+        firstname    => 'my firstname',
+        surname      => 'my surname ' . $i,
+        categorycode => 'S',
+        branchcode   => 'CPL',
+    );
+    push @borrowernumbers, $borrowernumber;
+}
+
+my $biblionumber = $bibnum;
+
+my @branches = GetBranchesLoop();
+my $branch   = $branches[0][0]{value};
+
+# Create five item level holds
+foreach my $borrowernumber (@borrowernumbers) {
+    AddReserve(
+        $branch,
+        $borrowernumber,
+        $biblionumber,
+        my $constraint = 'a',
+        my $bibitems   = q{},
+        my $priority,
+        my $resdate,
+        my $expdate,
+        my $notes = q{},
+        $title,
+        my $checkitem,
+        my $found,
+    );
+}
+
+ModReserveAffect( $itemnumber, $borrowernumbers[0] );
+C4::Circulation::AddIssue( GetMember( borrowernumber => $borrowernumbers[1] ),
+    $item_barcode, my $datedue, my $cancelreserve = 'revert' );
+
+my $priorities = $dbh->selectall_arrayref(
+    "SELECT priority FROM reserves ORDER BY priority ASC");
+ok( scalar @$priorities == 2,   'Only 2 holds remain in the reserves table' );
+ok( $priorities->[0]->[0] == 1, 'First hold has a priority of 1' );
+ok( $priorities->[1]->[0] == 2, 'Second hold has a priority of 2' );
+
+# Helper method to set up a Biblio.
+sub create_helper_biblio {
+    my $bib   = MARC::Record->new();
+    my $title = 'Silence in the library';
+    $bib->append_fields(
+        MARC::Field->new( '100', ' ', ' ', a => 'Moffat, Steven' ),
+        MARC::Field->new( '245', ' ', ' ', a => $title ),
+    );
+    return ( $bibnum, $title, $bibitemnum ) = AddBiblio( $bib, '' );
+}
-- 
2.39.2