Attachment 27428 Details for Bug 12086 – [SIGNED-OFF] Bug 12086 - Hold priorities incorrect, when waiting status was reversed

[patch] [SIGNED-OFF] Bug 12086 - Hold priorities incorrect, when waiting status was reversed

SIGNED-OFF-Bug-12086---Hold-priorities-incorrect-w.patch (text/plain), 4.77 KB, created by Owen Leonard on 2014-04-22 16:54:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2014-04-22 16:54:10 UTC

Size: 4.77 KB

patch

obsolete

>From ecc612a55e07ed9b9c09058f811c0dcf1639560b 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] [SIGNED-OFF] Bug 12086 - Hold priorities incorrect, when
> waiting status was reversed
>Content-Type: text/plain; charset="utf-8"
>
>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.
>---
> C4/Reserves.pm               |    5 ++-
> t/db_dependent/Holds_12086.t |   99 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 102 insertions(+), 2 deletions(-)
> create mode 100755 t/db_dependent/Holds_12086.t
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 49f8c11..94b82a4 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -2113,7 +2113,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.
> 
>@@ -2168,7 +2168,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_12086.t b/t/db_dependent/Holds_12086.t
>new file mode 100755
>index 0000000..a3d7648
>--- /dev/null
>+++ b/t/db_dependent/Holds_12086.t
>@@ -0,0 +1,99 @@
>+#!/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;
>+
>+BEGIN {
>+    use FindBin;
>+    use lib $FindBin::Bin;
>+}
>+
>+my $dbh = C4::Context->dbh;
>+
>+# Start transaction
>+$dbh->{AutoCommit} = 0;
>+$dbh->{RaiseError} = 1;
>+
>+$dbh->do("TRUNCATE TABLE 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 = '99999999999999999999';
>+my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => 'CPL', holdingbranch => 'CPL', barcode => $item_barcode } , $bibnum);
>+
>+# Create some borrowers
>+my @borrowernumbers;
>+foreach (1..$borrowers_count) {
>+    my $borrowernumber = AddMember(
>+        firstname =>  'my firstname',
>+        surname => 'my surname ' . $_,
>+        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, '');
>+}
>-- 
>1.7.9.5

Actions: View | Diff | Splinter Review

Attachments on bug 12086: 27262 | 27263 | 27406 | 27407 | 27428 | 28471 | 28482 | 28873 | 29162 | 29163