Browse Source

Bug 19025: Move GetReserveInfo to Koha::Holds

This subroutine is only used once and can be replaced with a call to
Koha::Holds->find
It will avoid unnecessary joins.

Test plan:
- Define a HOLD_SLIP template notice using fields from the tables
reserves, branches, borrowers, biblio, biblioitems and items.
- Generate one and make sure the values are correctly filled

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
17.11.x
Jonathan Druart 7 years ago
parent
commit
3ecc6fd8c6
  1. 3
      C4/Reserves.pm
  2. 9
      t/db_dependent/Holds.t
  3. 1
      t/db_dependent/Reserves/GetReserveFee.t

3
C4/Reserves.pm

@ -2178,7 +2178,8 @@ sub ReserveSlip {
biblionumber => $biblionumber, biblionumber => $biblionumber,
borrowernumber => $borrowernumber borrowernumber => $borrowernumber
}) or return; }) or return;
my $reserve = GetReserveInfo($reserve_id) or return; my $reserve = Koha::Holds->find($reserve_id) or return;
$reserve = $reserve->unblessed;
return C4::Letters::GetPreparedLetter ( return C4::Letters::GetPreparedLetter (
module => 'circulation', module => 'circulation',

9
t/db_dependent/Holds.t

@ -7,7 +7,7 @@ use t::lib::TestBuilder;
use C4::Context; use C4::Context;
use Test::More tests => 58; use Test::More tests => 57;
use MARC::Record; use MARC::Record;
use C4::Biblio; use C4::Biblio;
use C4::Items; use C4::Items;
@ -211,11 +211,6 @@ ModReserveMinusPriority( $itemnumber, $reserve->{'reserve_id'} );
$holds = $patron->holds; $holds = $patron->holds;
is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" ); is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" );
my $reserve2 = GetReserveInfo( $reserve->{'reserve_id'} );
ok( $reserve->{'reserve_id'} eq $reserve2->{'reserve_id'}, "Test GetReserveInfo()" );
$holds = $biblio->holds; $holds = $biblio->holds;
$hold = $holds->next; $hold = $holds->next;
AlterPriority( 'top', $hold->reserve_id ); AlterPriority( 'top', $hold->reserve_id );
@ -323,7 +318,7 @@ my $reserveid2 = C4::Reserves::GetReserveId(
CancelReserve({ reserve_id => $reserveid1 }); CancelReserve({ reserve_id => $reserveid1 });
$reserve2 = GetReserve( $reserveid2 ); my $reserve2 = GetReserve( $reserveid2 );
is( $reserve2->{priority}, 1, "After cancelreserve, the 2nd reserve becomes 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 => $branch_1, holdingbranch => $branch_1 } , $bibnum); ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $bibnum);

1
t/db_dependent/Reserves/GetReserveFee.t

@ -193,7 +193,6 @@ subtest 'Integration with AddIssue' => sub {
$dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} ); $dbh->do( "DELETE FROM issues WHERE borrowernumber = ?", undef, $patron1->{borrowernumber} );
my $id = addreserve( $patron1->{borrowernumber} ); my $id = addreserve( $patron1->{borrowernumber} );
my $r = C4::Reserves::GetReserveInfo($id);
is( acctlines( $patron1->{borrowernumber} ), 0, 'any_time_is_collected - Patron should not be charged yet (just checking to make sure)'); is( acctlines( $patron1->{borrowernumber} ), 0, 'any_time_is_collected - Patron should not be charged yet (just checking to make sure)');
C4::Circulation::AddIssue( $patron1, $item1->{barcode}, '2015-12-31', 0, undef, 0, {} ); C4::Circulation::AddIssue( $patron1, $item1->{barcode}, '2015-12-31', 0, undef, 0, {} );
is( acctlines( $patron1->{borrowernumber} ), 1, 'any_time_is_collected - Patron should not be charged when checking out an item which was not placed hold for him' ); is( acctlines( $patron1->{borrowernumber} ), 1, 'any_time_is_collected - Patron should not be charged when checking out an item which was not placed hold for him' );

Loading…
Cancel
Save