From d36fec48bc5c1d7bc2bfce5d5e1a8d659c8cb3eb Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 2 Aug 2017 12:46:11 -0300 Subject: [PATCH] [SIGNED-OFF] Bug 19058: Move C4::Reserves::GetReserveId to the Koha namespace GetReserveId can easily be replaced with a call to Koha::Holds->search->next->reserve_id It will ease next changes to use Koha::Hold objects Test plan: Cancel a reserve and print a slip reserve Signed-off-by: Owen Leonard --- C4/Reserves.pm | 58 ++++++++++++++------------------------------ t/db_dependent/Circulation.t | 4 +-- t/db_dependent/Holds.t | 31 ++++------------------- 3 files changed, 25 insertions(+), 68 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 575e486..3be9df9 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -846,15 +846,13 @@ sub CancelReserve { my ( $params ) = @_; my $reserve_id = $params->{'reserve_id'}; - # Filter out only the desired keys; this will insert undefined values for elements missing in - # \%params, but GetReserveId filters them out anyway. - $reserve_id = GetReserveId( { biblionumber => $params->{'biblionumber'}, borrowernumber => $params->{'borrowernumber'}, itemnumber => $params->{'itemnumber'} } ) unless ( $reserve_id ); - - return unless ( $reserve_id ); - - my $dbh = C4::Context->dbh; + my $hold; + if ( $reserve_id ) { + $hold = Koha::Holds->find( $reserve_id ); + } else { + $hold = Koha::Holds->search( $params ); # biblionumber, borrowernumber, itemnumber + } - my $hold = Koha::Holds->find( $reserve_id ); return unless $hold; logaction( 'HOLDS', 'CANCEL', $hold->reserve_id, Dumper($hold->unblessed) ) @@ -866,6 +864,7 @@ sub CancelReserve { priority = 0 WHERE reserve_id = ? "; + my $dbh = C4::Context->dbh; my $sth = $dbh->prepare($query); $sth->execute( $reserve_id ); @@ -947,13 +946,19 @@ sub ModReserve { return if $rank eq "n"; return unless ( $reserve_id || ( $borrowernumber && ( $biblionumber || $itemnumber ) ) ); - $reserve_id = GetReserveId({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber }) unless ( $reserve_id ); + + my $hold; + unless ( $reserve_id ) { + $hold = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $borrowernumber, itemnumber => $itemnumber }); + return unless $hold; # FIXME Should raise an exception + $reserve_id = $hold->reserve_id; + } if ( $rank eq "del" ) { CancelReserve({ reserve_id => $reserve_id }); } elsif ($rank =~ /^\d+/ and $rank > 0) { - my $hold = Koha::Holds->find($reserve_id); + $hold ||= Koha::Holds->find($reserve_id); logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, Dumper($hold->unblessed) ) if C4::Context->preference('HoldsLog'); @@ -1999,30 +2004,6 @@ sub RevertWaitingStatus { _FixPriority( { biblionumber => $reserve->{biblionumber} } ); } -=head2 GetReserveId - - $reserve_id = GetReserveId({ biblionumber => $biblionumber, borrowernumber => $borrowernumber [, itemnumber => $itemnumber ] }); - - Returnes the first reserve id that matches the given criteria - -=cut - -sub GetReserveId { - my ( $params ) = @_; - - return unless ( ( $params->{'biblionumber'} || $params->{'itemnumber'} ) && $params->{'borrowernumber'} ); - - foreach my $key ( keys %$params ) { - delete $params->{$key} unless defined( $params->{$key} ); - } - - my $hold = Koha::Holds->search( $params )->next(); - - return unless $hold; - - return $hold->id(); -} - =head2 ReserveSlip ReserveSlip($branchcode, $borrowernumber, $biblionumber) @@ -2047,12 +2028,9 @@ sub ReserveSlip { # return unless ( C4::Context->boolean_preference('printreserveslips') ); my $patron = Koha::Patrons->find( $borrowernumber ); - my $reserve_id = GetReserveId({ - biblionumber => $biblionumber, - borrowernumber => $borrowernumber - }) or return; - my $reserve = Koha::Holds->find($reserve_id) or return; - $reserve = $reserve->unblessed; + my $hold = Koha::Holds->search({biblionumber => $biblionumber, borrowernumber => $borrowernumber })->next; + return unless $hold; + my $reserve = $hold->unblessed; return C4::Letters::GetPreparedLetter ( module => 'circulation', diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index f6cd91c..781f496 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -377,7 +377,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved'); is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)'); - my $reserveid = C4::Reserves::GetReserveId({ biblionumber => $biblionumber, borrowernumber => $reserving_borrowernumber}); + my $reserveid = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $reserving_borrowernumber })->next->reserve_id; my $reserving_borrower = Koha::Patrons->find( $reserving_borrowernumber )->unblessed; AddIssue($reserving_borrower, $barcode3); my $reserve = $dbh->selectrow_hashref( @@ -503,7 +503,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue'); - $reserveid = C4::Reserves::GetReserveId({ biblionumber => $biblionumber, itemnumber => $itemnumber, borrowernumber => $reserving_borrowernumber}); + $reserveid = Koha::Holds->search({ biblionumber => $biblionumber, borrowernumber => $reserving_borrowernumber })->next->reserve_id; CancelReserve({ reserve_id => $reserveid }); # Bug 14101 diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 3d41670..f9083d4 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 56; +use Test::More tests => 55; use MARC::Record; use C4::Biblio; use C4::Items; @@ -197,13 +197,7 @@ AddReserve( ); $patron = Koha::Patrons->find( $borrowernumber ); $holds = $patron->holds; -my $reserveid = C4::Reserves::GetReserveId( - { - biblionumber => $biblionumber, - borrowernumber => $borrowernumber - } -); -is( $reserveid, $holds->next->reserve_id, "Test GetReserveId" ); +my $reserveid = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[0] })->next->reserve_id; ModReserveMinusPriority( $itemnumber, $reserveid ); $holds = $patron->holds; is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" ); @@ -291,12 +285,7 @@ AddReserve( 1, ); -my $reserveid1 = C4::Reserves::GetReserveId( - { - biblionumber => $bibnum, - borrowernumber => $borrowernumbers[0] - } -); +my $reserveid1 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[0] })->next->reserve_id; ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum); AddReserve( @@ -306,12 +295,7 @@ AddReserve( '', 2, ); -my $reserveid2 = C4::Reserves::GetReserveId( - { - biblionumber => $bibnum, - borrowernumber => $borrowernumbers[1] - } -); +my $reserveid2 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[1] })->next->reserve_id; CancelReserve({ reserve_id => $reserveid1 }); @@ -326,12 +310,7 @@ AddReserve( '', 2, ); -my $reserveid3 = C4::Reserves::GetReserveId( - { - biblionumber => $bibnum, - borrowernumber => $borrowernumbers[0] - } -); +my $reserveid3 = Koha::Holds->search({ biblionumber => $bibnum, borrowernumber => $borrowernumbers[0] })->next->reserve_id; my $hold3 = Koha::Holds->find( $reserveid3 ); is( $hold3->priority, 2, "New reserve for patron 0, the reserve has a priority = 2" ); -- 2.1.4