From ee9b6fb49e01d6deddf9e5ef846440b9b9d78148 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Thu, 13 Jun 2013 09:51:51 +0000 Subject: [PATCH] Bug 8868: ILS-DI: CancelHold needs to take a reserve_id (follow-up) - Fix the text in the ilsdi.pl?service=Describe&verb=CancelHold page - Unit tests for CancelReservedFromId and GetReserve - Do not delete row in reserves table if insert in old_reserves fails Signed-off-by: Benjamin Rokseth Rebased against master --- C4/Reserves.pm | 21 +++++++++++++-------- koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt | 2 +- t/db_dependent/Reserves.t | 24 ++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 42a746c..cbbcb3a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1104,14 +1104,19 @@ sub CancelReserveFromId { WHERE reserve_id = ? }; $sth = $dbh->prepare($query); - $sth->execute($reserve_id); - - $query = qq{ - DELETE FROM reserves - WHERE reserve_id = ? - }; - $sth = $dbh->prepare($query); - $sth->execute($reserve_id); + my $rv = $sth->execute($reserve_id); + + if ($rv) { + $query = qq{ + DELETE FROM reserves + WHERE reserve_id = ? + }; + $sth = $dbh->prepare($query); + $sth->execute($reserve_id); + } else { + warn "ERROR: Failed to move reserve $reserve_id to table old_reserves."; + return; + } # now fix the priority on the others... _FixPriority( $reserve->{biblionumber}, $reserve->{borrowernumber} ); diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt b/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt index a727b29..decf7a7 100644 --- a/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt +++ b/koha-tmpl/opac-tmpl/prog/en/modules/ilsdi.tt @@ -671,7 +671,7 @@
patron_id (Required)
the unique patron identifier in the ILS; the same identifier returned by LookupPatron or AuthenticatePatron
item_id (Required)
-
system item identifier
+
system hold identifier (returned by GetRecords and GetPatronInfo into element 'reserve_id')

Example call

diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index db82296..fb07090 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -2,7 +2,11 @@ use Modern::Perl; +<<<<<<< HEAD use Test::More tests => 23; +======= +use Test::More tests => 25; +>>>>>>> Bug 8868: ILS-DI: CancelHold needs to take a reserve_id (follow-up) use MARC::Record; use DateTime::Duration; @@ -221,6 +225,26 @@ is( $messages->{ResFound}->{borrowernumber}, $requesters{'RPL'}, 'for generous library, its items fill first hold request in line (bug 10272)'); +my (undef, $reserves) = GetReservesFromBiblionumber($biblionumber); +isa_ok($reserves, 'ARRAY'); +is(scalar @$reserves, 1, "Only one reserves for this biblio"); +my $reserve_id = $reserves->[0]->{reserve_id}; + +$reserve = GetReserve($reserve_id); +isa_ok($reserve, 'HASH', "GetReserve return"); +is($reserve->{biblionumber}, $biblionumber); + +$reserve = CancelReserveFromId($reserve_id); +isa_ok($reserve, 'HASH', "CancelReserveFromId return"); +is($reserve->{biblionumber}, $biblionumber); + +$reserve = GetReserve($reserve_id); +is($reserve, undef, "GetReserve returns undef after deletion"); + +$reserve = CancelReserveFromId($reserve_id); +is($reserve, undef, "CancelReserveFromId return undef if reserve does not exist"); + + # Tests for bug 9761 (ConfirmFutureHolds): new CheckReserves lookahead parameter, and corresponding change in AddReturn # Note that CheckReserve uses its lookahead parameter and does not check ConfirmFutureHolds pref (it should be passed if needed like AddReturn does) # Test 9761a: Add a reserve without date, CheckReserve should return it -- 1.8.3.2