From 5ae8869d57dba0b5b9ca9090c198c5c3a4be7b22 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 Fixed test t/db_dependant/Reserves.t and updated return reference in CancelReserveFromId. Tested via ILS-DI : placing hold (HoldItem) checking patron info (GetPatronInfo) cancelling hold (CancelHold) Verified that CancelHold parameter item_id takes reserve_id May need additional testing and sign-off Signed-off-by: Benjamin Rokseth --- C4/ILSDI/Services.pm | 30 +++++++++--------------------- C4/Reserves.pm | 6 +++--- t/db_dependent/Reserves.t | 4 ++-- 3 files changed, 14 insertions(+), 26 deletions(-) mode change 100755 => 100644 t/db_dependent/Reserves.t diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index bd201d3..0df6680 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -26,7 +26,7 @@ use C4::Circulation; use C4::Branch; use C4::Accounts; use C4::Biblio; -use C4::Reserves qw(AddReserve CancelReserve GetReservesFromBiblionumber GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved); +use C4::Reserves qw(AddReserve GetReservesFromBiblionumber GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved); use C4::Context; use C4::AuthoritiesMarc; use XML::Simple; @@ -724,7 +724,7 @@ Parameters: - patron_id (Required) a borrowernumber - item_id (Required) - an itemnumber + a reserve_id =cut @@ -736,26 +736,14 @@ sub CancelHold { my $borrower = GetMemberDetails( $borrowernumber ); return { code => 'PatronNotFound' } unless $$borrower{borrowernumber}; - # Get the item or return an error code - my $itemnumber = $cgi->param('item_id'); - my $item = GetItem( $itemnumber ); - return { code => 'RecordNotFound' } unless $$item{itemnumber}; - - # Get borrower's reserves - my @reserves = GetReservesFromBorrowernumber( $borrowernumber, undef ); - my @reserveditems; - - # ...and loop over it to build an array of reserved itemnumbers - foreach my $reserve (@reserves) { - push @reserveditems, $reserve->{'itemnumber'}; - } - - # if the item was not reserved by the borrower, returns an error code - return { code => 'NotCanceled' } unless any { $itemnumber eq $_ } @reserveditems; - - # Cancel the reserve - CancelReserve({ itemnumber => $itemnumber, borrowernumber => $borrowernumber }); + # Get the reserve or return an error code + my $reserve_id = $cgi->param('item_id'); + my $reserve = C4::Reserves::GetReserve($reserve_id); + return { code => 'RecordNotFound' } unless $reserve; + return { code => 'RecordNotFound' } unless ($reserve->{borrowernumber} == $borrowernumber); + C4::Reserves::CancelReserveFromId($reserve_id); + return { code => 'Canceled' }; } diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 6d50ca4..5a21c9a 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -119,6 +119,7 @@ BEGIN { &CanBookBeReserved &CanItemBeReserved &CancelReserve + &CancelReserveFromId &CancelExpiredReserves &AutoUnsuspendReserves @@ -1059,7 +1060,7 @@ sub CancelReserve { $sth->execute( $reserve_id ); # now fix the priority on the others.... - _FixPriority({ biblionumber => $reserve->{biblionumber} }); + _FixPriority({ biblionumber => $reserve->{biblionumber} }); } =head2 CancelReserveFromId @@ -1110,11 +1111,10 @@ sub CancelReserveFromId { } # now fix the priority on the others... - _FixPriority( $reserve->{biblionumber}, $reserve->{borrowernumber} ); + _FixPriority({ biblionumber => $reserve->{biblionumber} }); } return $reserve; ->>>>>>> Bug 8868: ILS-DI: CancelHold needs to take a reserve_id (follow-up) } =head2 ModReserve diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t old mode 100755 new mode 100644 index c517112..c055540 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -2,7 +2,7 @@ use Modern::Perl; -use Test::More tests => 31; +use Test::More tests => 39; use MARC::Record; use DateTime::Duration; @@ -229,7 +229,7 @@ is( $messages->{ResFound}->{borrowernumber}, $requesters{'RPL'}, 'for generous library, its items fill first hold request in line (bug 10272)'); -my (undef, $reserves) = GetReservesFromBiblionumber($biblionumber); +my $reserves = GetReservesFromBiblionumber({ biblionumber => $biblionumber}); isa_ok($reserves, 'ARRAY'); is(scalar @$reserves, 1, "Only one reserves for this biblio"); my $reserve_id = $reserves->[0]->{reserve_id}; -- 1.8.3.2