From fb6ef59e545fb06024961fc632d03c0bbf20d6b0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 13 Dec 2013 13:03:20 -0500 Subject: [PATCH] [PASSED QA] Bug 11336: (follow-up) Priority is not updated on deleting holds - moremember Patch does not fix issue when canceling hold from moremember.pl. This followup fixes that issue. Signed-off-by: Kyle M Hall Signed-off-by: Katrin Fischer Preparations: - Create holds for different patrons on a record: * 1st - title level hold * 2nd - item level hold * 3rd - title level hold * 4th - title level hold - AllowOnShelfHolds = On/Allow (items were not checked out) Tests: Deleted holds from various pages, confirming bugs first, then testing with applied patches. Reloading database after each test. 1) Cancel holds from OPAC patron account /cgi-bin/koha/opac-user.pl#opac-user-holds - Cancel 4th - ok, before and after applying the patch - Cancel 2nd - ok, after applying the patch 2) Cancel hold from holds tab on staff detail page /cgi-bin/koha/reserve/request.pl?biblionumber=7 a) Setting priority to 'del', submitting with 'Update holds' - Cancel first (1st) - ok, before and after - Cancel hold in the middle (was 3rd) - ok, before and after - Cancel last (was 4th) -ok, before and after b) Using red X - Repeating tests from a) - before the patch is applied holds get totally 'out of order' - after applying the patch, it works correctly Additional tests done on this page: - Change priority using up, down, to top, to bottom icons - Change priority with 'toggle to lowest' 3) Cancel hold from the patron's account a) Check out tab - Delete? Yes, 'Cancel marked holds' /cgi-bin/koha/circ/circulation.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch b) Details tab - Delete? yes, 'Cancel marked holds' /cgi-bin/koha/members/moremember.pl?borrowernumber=X - Cancel first (1st) - ok, after applying the patch - Cancel hold in the middle (was 3rd) - ok, after applying the patch - Cancel last (was 4th) - ok, after applying the patch Without the patch, holds priorities get out of order. Additional tests done: - Check in one item to trigger first hold - Check in one item to trigger second hold - Check out first item Priorities are kept while the item is waiting, when it's checked out, priorities of remaining holds get reset correctly. Conclusion: Big improvement, no regressions found. Passes all tests in t, xt and QA script. Also: t/db_dependent/Holds.t t/db_dependent/HoldsQueue.t t/db_dependent/Reserves.t --- C4/Reserves.pm | 23 +---------------------- t/db_dependent/Holds.t | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 7fd39dc..5a91d27 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1101,28 +1101,7 @@ sub ModReserve { my $dbh = C4::Context->dbh; if ( $rank eq "del" ) { - my $query = " - UPDATE reserves - SET cancellationdate=now() - WHERE reserve_id = ? - "; - my $sth = $dbh->prepare($query); - $sth->execute( $reserve_id ); - $query = " - INSERT INTO old_reserves - SELECT * - FROM reserves - WHERE reserve_id = ? - "; - $sth = $dbh->prepare($query); - $sth->execute( $reserve_id ); - $query = " - DELETE FROM reserves - WHERE reserve_id = ? - "; - $sth = $dbh->prepare($query); - $sth->execute( $reserve_id ); - + CancelReserve({ reserve_id => $reserve_id }); } elsif ($rank =~ /^\d+/ and $rank > 0) { my $query = " diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index b22f5a1..2c28c07 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -6,7 +6,7 @@ use t::lib::Mocks; use C4::Context; use C4::Branch; -use Test::More tests => 23; +use Test::More tests => 25; use MARC::Record; use C4::Biblio; use C4::Items; @@ -253,8 +253,30 @@ my $reserveid2 = C4::Reserves::GetReserveId( CancelReserve({ reserve_id => $reserveid1 }); $reserve2 = GetReserve( $reserveid2 ); -is( $reserve2->{priority}, 1, "After cancelreserve, the 2nd reserve become the first on the waiting list" ); +is( $reserve2->{priority}, 1, "After cancelreserve, the 2nd reserve becomes the first on the waiting list" ); +($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => 'CPL', holdingbranch => 'CPL' } , $bibnum); +AddReserve( + $branch, + $borrowernumbers[0], + $bibnum, + 'a', + '', + 2, +); +my $reserveid3 = C4::Reserves::GetReserveId( + { + biblionumber => $bibnum, + borrowernumber => $borrowernumbers[0] + } +); + +my $reserve3 = GetReserve( $reserveid3 ); +is( $reserve3->{priority}, 2, "New reserve for patron 0, the reserve has a priority = 2" ); + +ModReserve({ reserve_id => $reserveid2, rank => 'del' }); +$reserve3 = GetReserve( $reserveid3 ); +is( $reserve3->{priority}, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" ); # Helper method to set up a Biblio. -- 1.8.3.2