Browse Source

Bug 14711: Change prototype for AddReserve - pass a hashref

The number of parameters of AddReserve makes it hard to read and
maintain.
This patch replace it with a hashref, which will make the calls more
readable.

Moreover the bibitems has been removed as it was not used by the
subroutine.

Test plan:
- Make sure the tests pass
- Read the diff and search for typos
- Place a hold on few items

Note for QA: reservation_date and expiration_date do not match the DB column's names,
should we?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
20.05.x
Jonathan Druart 4 years ago
committed by Martin Renvoize
parent
commit
11b44869d9
Signed by: martin.renvoize GPG Key ID: 422B469130441A0F
  1. 26
      C4/ILSDI/Services.pm
  2. 33
      C4/Reserves.pm
  3. 8
      C4/SIP/ILS/Transaction/Hold.pm
  4. 24
      Koha/Club/Hold.pm
  5. 25
      Koha/REST/V1/Holds.pm
  6. 17
      opac/opac-reserve.pl
  7. 54
      reserve/placerequest.pl
  8. 11
      serials/routing-preview.pl
  9. 82
      t/db_dependent/Circulation.t
  10. 12
      t/db_dependent/Circulation/issue.t
  11. 149
      t/db_dependent/Holds.t
  12. 90
      t/db_dependent/Holds/HoldFulfillmentPolicy.t
  13. 30
      t/db_dependent/Holds/HoldItemtypeLimit.t
  14. 17
      t/db_dependent/Holds/LocalHoldsPriority.t
  15. 16
      t/db_dependent/Holds/RevertWaitingStatus.t
  16. 231
      t/db_dependent/HoldsQueue.t
  17. 37
      t/db_dependent/Items/GetItemsForInventory.t
  18. 36
      t/db_dependent/Koha/Acquisition/Order.t
  19. 17
      t/db_dependent/Koha/Biblios.t
  20. 85
      t/db_dependent/Koha/Holds.t
  21. 19
      t/db_dependent/Koha/Patrons.t
  22. 21
      t/db_dependent/Letters/TemplateToolkit.t
  23. 339
      t/db_dependent/Reserves.t
  24. 18
      t/db_dependent/Reserves/GetReserveFee.t
  25. 18
      t/db_dependent/Reserves/MultiplePerRecord.t
  26. 29
      t/db_dependent/SIP/Transaction.t
  27. 10
      t/db_dependent/UsageStats.t
  28. 49
      t/db_dependent/api/v1/holds.t

26
C4/ILSDI/Services.pm

@ -733,7 +733,15 @@ sub HoldTitle {
# $constraint, $bibitems, $priority, $resdate, $expdate, $notes,
# $title, $checkitem, $found
my $priority= C4::Reserves::CalculatePriority( $biblionumber );
AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, undef, undef );
AddReserve(
{
branch => $branch,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => $priority,
title => $title,
}
);
# Hashref building
my $out;
@ -808,11 +816,17 @@ sub HoldItem {
return { code => $canitembereserved } unless $canitembereserved eq 'OK';
# Add the reserve
# $branch, $borrowernumber, $biblionumber,
# $constraint, $bibitems, $priority, $resdate, $expdate, $notes,
# $title, $checkitem, $found
my $priority= C4::Reserves::CalculatePriority( $biblionumber );
AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, $itemnumber, undef );
my $priority = C4::Reserves::CalculatePriority($biblionumber);
AddReserve(
{
branch => $branch,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => $priority,
title => $title,
itemnumber => $itemnumber,
}
);
# Hashref building
my $out;

33
C4/Reserves.pm

@ -141,7 +141,21 @@ BEGIN {
=head2 AddReserve
AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
AddReserve(
{
branch => $branchcode,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => $priority,
reservation_date => $reservation_date,
expiration_date => $expiration_date,
notes => $notes,
title => $title,
itemnumber => $itemnumber,
found => $found,
itemtype => $itemtype,
}
);
Adds reserve and generates HOLDPLACED message.
@ -157,11 +171,18 @@ The following tables are available witin the HOLDPLACED message:
=cut
sub AddReserve {
my (
$branch, $borrowernumber, $biblionumber, $bibitems,
$priority, $resdate, $expdate, $notes,
$title, $checkitem, $found, $itemtype
) = @_;
my ($params) = @_;
my $branch = $params->{branchcode};
my $borrowernumber = $params->{borrowernumber};
my $biblionumber = $params->{biblionumber};
my $priority = $params->{priority};
my $resdate = $params->{reservation_date};
my $expdate = $params->{expiration_date};
my $notes = $params->{notes};
my $title = $params->{title};
my $checkitem = $params->{itemnumber};
my $found = $params->{found};
my $itemtype = $params->{itemtype};
$resdate = output_pref( { str => dt_from_string( $resdate ), dateonly => 1, dateformat => 'iso' })
or output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' });

8
C4/SIP/ILS/Transaction/Hold.pm

@ -66,7 +66,13 @@ sub do_hold {
return $self;
}
AddReserve( $branch, $patron->borrowernumber, $item->biblionumber );
AddReserve(
{
branch => $branch,
borrowernumber => $patron->borrowernumber,
biblionumber => $item->biblionumber
}
);
# unfortunately no meaningful return value
$self->ok(1);

24
Koha/Club/Hold.pm

@ -91,18 +91,18 @@ sub add {
my $priority = C4::Reserves::CalculatePriority($params->{biblio_id});
my $hold_id = C4::Reserves::AddReserve(
$params->{pickup_library_id},
$patron_id,
$params->{biblio_id},
undef, # $bibitems param is unused
$priority,
undef, # hold date, we don't allow it currently
$params->{expiration_date},
$params->{notes},
$biblio->title,
$params->{item_id},
undef, # TODO: Why not?
$params->{item_type}
{
branchcode => $params->{pickup_library_id},
borrowernumber => $patron_id,
biblionumber => $params->{biblio_id},
priority => $priority,
expiration_date => $params->{expiration_date},
notes => $params->{notes},
title => $biblio->title,
itemnumber => $params->{item_id},
found => undef, # TODO: Why not?
itemtype => $params->{item_type},
}
);
if ($hold_id) {
Koha::Club::Hold::PatronHold->new({

25
Koha/REST/V1/Holds.pm

@ -161,18 +161,19 @@ sub add {
}
my $hold_id = C4::Reserves::AddReserve(
$pickup_library_id,
$patron_id,
$biblio_id,
undef, # $bibitems param is unused
$priority,
$hold_date,
$expiration_date,
$notes,
$biblio->title,
$item_id,
undef, # TODO: Why not?
$item_type
{
branchcode => $pickup_library_id,
borrowernumber => $patron_id,
biblionumber => $biblio_id,
priority => $priority,
reservation_date => $hold_date,
expiration_date => $expiration_date,
notes => $notes,
title => $biblio->title,
itemnumber => $item_id,
found => undef, # TODO: Why not?
itemtype => $item_type,
}
);
unless ($hold_id) {

17
opac/opac-reserve.pl

@ -309,10 +309,19 @@ if ( $query->param('place_reserve') ) {
# Here we actually do the reserveration. Stage 3.
if ($canreserve) {
my $reserve_id = AddReserve(
$branch, $borrowernumber, $biblioNum,
[$biblioNum], $rank, $startdate,
$expiration_date, $notes, $biblioData->{title},
$itemNum, $found, $itemtype,
{
branchcode => $branch,
borrowernumber => $borrowernumber,
biblionumber => $biblioNum,
priority => $rank,
reservation_date => $startdate,
expiration_date => $expiration_date,
notes => $notes,
title => $biblioData->{title},
itemnumber => $itemNum,
found => $found,
itemtype => $itemtype,
}
);
$failed_holds++ unless $reserve_id;
++$reserve_cnt;

54
reserve/placerequest.pl

@ -100,24 +100,62 @@ if ( $type eq 'str8' && $borrower ) {
my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $branch)->{status};
if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) {
AddReserve( $branch, $borrower->{'borrowernumber'},
$biblionumber, \@realbi, $rank[0], $startdate, $expirationdate, $notes, $title,
$checkitem, $found, $itemtype );
AddReserve(
{
branchcode => $branch,
borrowernumber => $borrower->{'borrowernumber'},
biblionumber => $biblionumber,
priority => $rank[0],
reservation_date => $startdate,
expiration_date => $expirationdate,
notes => $notes,
title => $title,
itemnumber => $checkitem,
found => $found,
itemtype => $itemtype,
}
);
}
} elsif ($multi_hold) {
my $bibinfo = $bibinfos{$biblionumber};
if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) {
AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,[$biblionumber],
$bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found);
AddReserve(
{
branchcode => $branch,
borrowernumber => $borrower->{'borrowernumber'},
biblionumber => $biblionumber,
priority => $bibinfo->{rank},
reservation_date => $startdate,
expiration_date => $expirationdate,
notes => $notes,
title => $bibinfo->{title},
itemnumber => $checkitem,
found => $found,
itemtype => $itemtype,
}
);
}
} else {
# place a request on 1st available
for ( my $i = 0 ; $i < $holds_to_place_count ; $i++ ) {
if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) {
AddReserve( $branch, $borrower->{'borrowernumber'},
$biblionumber, \@realbi, $rank[0], $startdate, $expirationdate, $notes, $title,
$checkitem, $found, $itemtype );
AddReserve(
{
branchcode => $branch,
borrowernumber => $borrower->{'borrowernumber'},
biblionumber => $biblionumber,
priority => $rank[0],
reservation_date => $startdate,
expiration_date => $expirationdate,
notes => $notes,
title => $title,
itemnumber => $checkitem,
found => $found,
itemtype => $itemtype,
}
);
}
}
}

11
serials/routing-preview.pl

@ -94,7 +94,16 @@ if($ok){
branchcode => $branch
});
} else {
AddReserve($branch,$routing->{borrowernumber},$biblionumber,undef,$routing->{ranking}, undef, undef, $notes,$title);
AddReserve(
{
branchcode => $branch,
borrowernumber => $routing->{borrowernumber},
biblionumber => $biblionumber,
priority => $routing->{ranking},
notes => $notes,
title => $title,
}
);
}
}
}

82
t/db_dependent/Circulation.t

@ -385,9 +385,17 @@ subtest "CanBookBeRenewed tests" => sub {
# Biblio-level hold, renewal test
AddReserve(
$branch, $reserving_borrowernumber, $biblio->biblionumber,
$bibitems, $priority, $resdate, $expdate, $notes,
'a title', $checkitem, $found
{
branchcode => $branch,
borrowernumber => $reserving_borrowernumber,
biblionumber => $biblio->biblionumber,
priority => $priority,
reservation_date => $resdate,
expiration_date => $expdate,
notes => $notes,
itemnumber => $checkitem,
found => $found,
}
);
# Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds
@ -458,9 +466,17 @@ subtest "CanBookBeRenewed tests" => sub {
# Item-level hold, renewal test
AddReserve(
$branch, $reserving_borrowernumber, $biblio->biblionumber,
$bibitems, $priority, $resdate, $expdate, $notes,
'a title', $item_1->itemnumber, $found
{
branchcode => $branch,
borrowernumber => $reserving_borrowernumber,
biblionumber => $biblio->biblionumber,
priority => $priority,
reservation_date => $resdate,
expiration_date => $expdate,
notes => $notes,
itemnumber => $item_1->itemnumber,
found => $found,
}
);
( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1);
@ -1378,9 +1394,12 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with no hold on the record' );
AddReserve(
$library2->{branchcode}, $borrowernumber2, $biblio->biblionumber,
'', 1, undef, undef, '',
undef, undef, undef
{
branchcode => $library2->{branchcode},
borrowernumber => $borrowernumber2,
biblionumber => $biblio->biblionumber,
priority => 1,
}
);
Koha::CirculationRules->set_rules(
@ -1799,9 +1818,17 @@ subtest 'MultipleReserves' => sub {
);
my $reserving_borrowernumber1 = Koha::Patron->new(\%reserving_borrower_data1)->store->borrowernumber;
AddReserve(
$branch, $reserving_borrowernumber1, $biblio->biblionumber,
$bibitems, $priority, $resdate, $expdate, $notes,
'a title', $checkitem, $found
{
branchcode => $branch,
borrowernumber => $reserving_borrowernumber1,
biblionumber => $biblio->biblionumber,
priority => $priority,
reservation_date => $resdate,
expiration_date => $expdate,
notes => $notes,
itemnumber => $checkitem,
found => $found,
}
);
my %reserving_borrower_data2 = (
@ -1812,9 +1839,17 @@ subtest 'MultipleReserves' => sub {
);
my $reserving_borrowernumber2 = Koha::Patron->new(\%reserving_borrower_data2)->store->borrowernumber;
AddReserve(
$branch, $reserving_borrowernumber2, $biblio->biblionumber,
$bibitems, $priority, $resdate, $expdate, $notes,
'a title', $checkitem, $found
{
branchcode => $branch,
borrowernumber => $reserving_borrowernumber2,
biblionumber => $biblio->biblionumber,
priority => $priority,
reservation_date => $resdate,
expiration_date => $expdate,
notes => $notes,
itemnumber => $checkitem,
found => $found,
}
);
{
@ -2861,8 +2896,13 @@ subtest 'Set waiting flag' => sub {
set_userenv( $library_2 );
my $reserve_id = AddReserve(
$library_2->{branchcode}, $patron_2->{borrowernumber}, $item->{biblionumber},
'', 1, undef, undef, '', undef, $item->{itemnumber},
{
branchcode => $library_2->{branchcode},
borrowernumber => $patron_2->{borrowernumber},
biblionumber => $item->{biblionumber},
priority => 1,
itemnumber => $item->{itemnumber},
}
);
set_userenv( $library_1 );
@ -2899,7 +2939,13 @@ subtest 'Cancel transfers on lost items' => sub {
set_userenv( $library_2 );
my $reserve_id = AddReserve(
$library_2->{branchcode}, $patron_2->{borrowernumber}, $item->biblionumber, '', 1, undef, undef, '', undef, $item->itemnumber,
{
branchcode => $library_2->{branchcode},
borrowernumber => $patron_2->{borrowernumber},
biblionumber => $item->biblionumber,
priority => 1,
itemnumber => $item->itemnumber,
}
);
#Return book and add transfer

12
t/db_dependent/Circulation/issue.t

@ -433,8 +433,16 @@ ok( $item2->permanent_location eq '' , q{UpdateItemLocationOnCheckin does not up
# Bug 14640 - Cancel the hold on checking out if asked
my $reserve_id = AddReserve($branchcode_1, $borrower_id1, $biblionumber,
undef, 1, undef, undef, "a note", "a title", undef, '');
my $reserve_id = AddReserve(
{
branchcode => $branchcode_1,
borrowernumber => $borrower_id1,
biblionumber => $biblionumber,
priority => 1,
notes => "a note",
title => "a title",
}
);
ok( $reserve_id, 'The reserve should have been inserted' );
AddIssue( $borrower_2, $barcode_1, dt_from_string, 'cancel' );
my $hold = Koha::Holds->find( $reserve_id );

149
t/db_dependent/Holds.t

@ -76,17 +76,13 @@ foreach (1..$borrowers_count) {
# Create five item level holds
foreach my $borrowernumber ( @borrowernumbers ) {
AddReserve(
$branch_1,
$borrowernumber,
$biblio->biblionumber,
my $bibitems = q{},
my $priority = C4::Reserves::CalculatePriority( $biblio->biblionumber ),
my $resdate,
my $expdate,
my $notes = q{},
'a title',
my $checkitem = $itemnumber,
my $found,
{
branchcode => $branch_1,
borrowernumber => $borrowernumber,
biblionumber => $biblio->biblionumber,
priority => C4::Reserves::CalculatePriority( $biblio->biblionumber ),
itemnumber => $itemnumber,
}
);
}
@ -191,19 +187,14 @@ is( $hold->suspend, 0, "Test resuming with SuspendAll()" );
is( $hold->suspend_until, undef, "Test resuming with SuspendAll(), should have no suspend until date" );
# Add a new hold for the borrower whose hold we canceled earlier, this time at the bib level
AddReserve(
$branch_1,
$borrowernumbers[0],
$biblio->biblionumber,
my $bibitems = q{},
my $priority,
my $resdate,
my $expdate,
my $notes = q{},
'a title',
my $checkitem,
my $found,
);
AddReserve(
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[0],
biblionumber => $biblio->biblionumber,
}
);
$patron = Koha::Patrons->find( $borrowernumber );
$holds = $patron->holds;
my $reserveid = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $borrowernumbers[0] })->next->reserve_id;
@ -297,11 +288,35 @@ ok(
# Regression test for bug 11336 # Test if ModReserve correctly recalculate the priorities
$biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
my $reserveid1 = AddReserve($branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1);
my $reserveid1 = AddReserve(
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[0],
biblionumber => $biblio->biblionumber,
priority => 1
}
);
($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
my $reserveid2 = AddReserve($branch_1, $borrowernumbers[1], $biblio->biblionumber, '', 2);
my $reserveid2 = AddReserve(
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[1],
biblionumber => $biblio->biblionumber,
priority => 2
}
);
($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
my $reserveid3 = AddReserve($branch_1, $borrowernumbers[2], $biblio->biblionumber, '', 3);
my $reserveid3 = AddReserve(
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[2],
biblionumber => $biblio->biblionumber,
priority => 3
}
);
my $hhh = Koha::Holds->search({ biblionumber => $biblio->biblionumber });
my $hold3 = Koha::Holds->find( $reserveid3 );
is( $hold3->priority, 3, "The 3rd hold should have a priority set to 3" );
@ -335,11 +350,12 @@ ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for d
$biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblio->biblionumber);
AddReserve(
$branch_1,
$borrowernumbers[0],
$biblio->biblionumber,
'',
1,
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[0],
biblionumber => $biblio->biblionumber,
priority => 1,
}
);
is(
CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
@ -442,7 +458,14 @@ Koha::CirculationRules->set_rules(
is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status},
'OK', 'Patron can reserve item with hold limit of 1, no holds placed' );
my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
my $res_id = AddReserve(
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[0],
biblionumber => $biblio->biblionumber,
priority => 1,
}
);
is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status},
'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' );
@ -474,9 +497,17 @@ subtest 'Test max_holds per library/patron category' => sub {
}
}
);
AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
for ( 1 .. 3 ) {
AddReserve(
{
branchcode => $branch_1,
borrowernumber => $borrowernumbers[0],
biblionumber => $biblio->biblionumber,
priority => 1,
}
);
}
my $count =
Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count();
@ -635,7 +666,14 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
'Patron can reserve item with hold limit of 1, no holds placed'
);
AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_1->biblionumber, '', 1, );
AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron->borrowernumber,
biblionumber => $biblio_1->biblionumber,
priority => 1,
}
);
is_deeply(
CanItemBeReserved( $patron->borrowernumber, $itemnumber_1 ),
@ -662,7 +700,14 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
);
# Add a second reserve
my $res_id = AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_2->biblionumber, '', 1, );
my $res_id = AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron->borrowernumber,
biblionumber => $biblio_2->biblionumber,
priority => 1,
}
);
is_deeply(
CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ),
{ status => 'tooManyReservesToday', limit => 2 },
@ -718,14 +763,36 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
{ status => 'OK' },
'Patron can reserve if holds_per_day is undef (i.e. undef is unlimited daily cap)'
);
AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_1->biblionumber, '', 1, );
AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_2->biblionumber, '', 1, );
AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron->borrowernumber,
biblionumber => $biblio_1->biblionumber,
priority => 1,
}
);
AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron->borrowernumber,
biblionumber => $biblio_2->biblionumber,
priority => 1,
}
);
is_deeply(
CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ),
{ status => 'OK' },
'Patron can reserve if holds_per_day is undef (i.e. undef is unlimited daily cap)'
);
AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_3->biblionumber, '', 1, );
AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron->borrowernumber,
biblionumber => $biblio_3->biblionumber,
priority => 1,
}
);
is_deeply(
CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ),
{ status => 'tooManyReserves', limit => 3 },

90
t/db_dependent/Holds/HoldFulfillmentPolicy.t

@ -85,19 +85,40 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
my $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
my $reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
my ( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Hold where pickup branch matches home branch targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is($status, q{}, "Hold where pickup ne home, pickup eq home not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
@ -116,19 +137,40 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, q{}, "Hold where pickup eq home, pickup ne holding not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
@ -147,19 +189,40 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Hold where pickup eq home, pickup ne holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
@ -176,7 +239,14 @@ my $limit = Koha::Item::Transfer::Limit->new(
itemtype => $item->effective_itemtype,
}
)->store();
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1
}
);
($status) = CheckReserves($itemnumber);
is( $status, '', "No hold where branch transfer is not allowed" );
Koha::Holds->find($reserve_id)->cancel;

30
t/db_dependent/Holds/HoldItemtypeLimit.t

@ -99,19 +99,43 @@ Koha::CirculationRules->set_rules(
);
# Itemtypes match
my $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
my $reserve_id = AddReserve(
{
branchcode => $branchcode,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
itemtype => $right_itemtype,
}
);
my ( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Hold where itemtype matches item's itemtype targed" );
Koha::Holds->find( $reserve_id )->cancel;
# Itemtypes don't match
$reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
$reserve_id = AddReserve(
{
branchcode => $branchcode,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
itemtype => $wrong_itemtype,
}
);
( $status ) = CheckReserves($itemnumber);
is($status, q{}, "Hold where itemtype does not match item's itemtype not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# No itemtype set
$reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
$reserve_id = AddReserve(
{
branchcode => $branchcode,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
( $status ) = CheckReserves($itemnumber);
is( $status, 'Reserved', "Item targeted with no hold itemtype set" );
Koha::Holds->find( $reserve_id )->cancel;

17
t/db_dependent/Holds/LocalHoldsPriority.t

@ -69,17 +69,12 @@ foreach ( 1 .. $borrowers_count ) {
my $i = 1;
foreach my $borrowernumber (@borrowernumbers) {
AddReserve(
$branchcodes[$i],
$borrowernumber,
$biblio->biblionumber,
my $bibitems = q{},
my $priority = $i,
my $resdate,
my $expdate,
my $notes = q{},
'a title',
my $checkitem,
my $found,
{
branchcode => $branchcodes[$i],
borrowernumber => $borrowernumber,
biblionumber => $biblio->biblionumber,
priority => $i,
}
);
$i++;

16
t/db_dependent/Holds/RevertWaitingStatus.t

@ -73,17 +73,11 @@ foreach my $i ( 1 .. $borrowers_count ) {
# Create five item level holds
foreach my $borrowernumber (@borrowernumbers) {
AddReserve(
$branchcode,
$borrowernumber,
$biblio->biblionumber,
my $bibitems = q{},
my $priority,
my $resdate,
my $expdate,
my $notes = q{},
'a title',
my $checkitem,
my $found,
{
branchcode => $branchcode,
borrowernumber => $borrowernumber,
biblionumber => $biblio->biblionumber,
}
);
}

231
t/db_dependent/HoldsQueue.t

@ -114,7 +114,14 @@ $dbh->do("DELETE FROM reserves");
my $bibitems = undef;
my $priority = 1;
# Make a reserve
AddReserve ( $borrower_branchcode, $borrowernumber, $biblionumber, $bibitems, $priority );
AddReserve(
{
branchcode => $borrower_branchcode,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => $priority,
}
);
# $resdate, $expdate, $notes, $title, $checkitem, $found
$dbh->do("UPDATE reserves SET reservedate = DATE_SUB( reservedate, INTERVAL 1 DAY )");
@ -533,7 +540,14 @@ $dbh->do( "UPDATE systempreferences SET value = ? WHERE variable = 'StaticHoldsQ
undef, join( ',', $library_B, $library_A, $library_C ) );
$dbh->do( "UPDATE systempreferences SET value = 0 WHERE variable = 'RandomizeHoldsQueueWeight'" );
my $reserve_id = AddReserve ( $library_C, $borrowernumber, $biblionumber, '', 1 );
my $reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
is( @$holds_queue, 1, "Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from le");
@ -577,7 +591,15 @@ $dbh->do("
VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype')
");
$reserve_id = AddReserve ( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
is( @$holds_queue, 0, "Bug 15062 - Holds queue with Transport Cost Matrix will transfer item even if transfers disabled");
@ -628,21 +650,46 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Hold where pickup branch matches home branch targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 0, "Hold where pickup ne home, pickup eq home not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" );
@ -662,21 +709,45 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 0, "Hold where pickup eq home, pickup ne holding not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" );
@ -696,21 +767,45 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Hold where pickup eq home, pickup ne holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_B,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
$reserve_id = AddReserve(
{
branchcode => $library_C,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Hold where pickup ne home, pickup ne holding targeted" );
@ -763,21 +858,47 @@ Koha::CirculationRules->set_rules(
);
# Home branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
itemtype => $wrong_itemtype,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 0, "Item with incorrect itemtype not targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Holding branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
itemtype => $right_itemtype,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Item with matching itemtype is targeted" );
Koha::Holds->find( $reserve_id )->cancel;
# Neither branch matches pickup branch
$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
$reserve_id = AddReserve(
{
branchcode => $library_A,
borrowernumber => $borrowernumber,
biblionumber => $biblionumber,
priority => 1,
}
);
C4::HoldsQueue::CreateQueue();
$holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
is( @$holds_queue, 1, "Item targeted when hold itemtype is not set" );
@ -819,12 +940,22 @@ subtest "Test Local Holds Priority - Bib level" => sub {
}
);
my $reserve_id =
AddReserve( $branch2->branchcode, $other_patron->borrowernumber,
$biblio->biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
my $reserve_id2 =
AddReserve( $item->homebranch, $local_patron->borrowernumber,
$biblio->biblionumber, '', 2, undef, undef, undef, undef, undef, undef, undef );
my $reserve_id = AddReserve(
{
branchcode => $branch2->branchcode,
borrowernumber => $other_patron->borrowernumber,
biblionumber => $biblio->biblionumber,
priority => 1,
}
);
my $reserve_id2 = AddReserve(
{
branchcode => $item->homebranch,
borrowernumber => $local_patron->borrowernumber,
biblionumber => $biblio->biblionumber,
priority => 2,
}
);
C4::HoldsQueue::CreateQueue();
@ -871,12 +1002,24 @@ subtest "Test Local Holds Priority - Item level" => sub {
}
);
my $reserve_id =
AddReserve( $branch2->branchcode, $other_patron->borrowernumber,
$biblio->biblionumber, '', 1, undef, undef, undef, undef, $item->id, undef, undef );
my $reserve_id2 =
AddReserve( $item->homebranch, $local_patron->borrowernumber,
$biblio->biblionumber, '', 2, undef, undef, undef, undef, $item->id, undef, undef );
my $reserve_id = AddReserve(
{
branchcode => $branch2->branchcode,
borrowernumber => $other_patron->borrowernumber,
biblionumber => $biblio->biblionumber,
priority => 1,
itemnumber => $item->id,
}
);
my $reserve_id2 = AddReserve(
{
branchcode => $item->homebranch,
borrowernumber => $local_patron->borrowernumber,
biblionumber => $biblio->biblionumber,
priority => 2,
itemnumber => $item->id,
}
);
C4::HoldsQueue::CreateQueue();
@ -924,12 +1067,23 @@ subtest "Test Local Holds Priority - Item level hold over Record level hold (Bug
}
);
my $reserve_id =
AddReserve( $branch2->branchcode, $other_patron->borrowernumber,
$biblio->biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
my $reserve_id2 =
AddReserve( $item->homebranch, $local_patron->borrowernumber,
$biblio->biblionumber, '', 2, undef, undef, undef, undef, $item->id, undef, undef );
my $reserve_id = AddReserve(
{
branchcode => $branch2->branchcode,
borrowernumber => $other_patron->borrowernumber,
biblionumber => $biblio->biblionumber,
priority => 1,
}
);
my $reserve_id2 = AddReserve(
{
branchcode => $item->homebranch,
borrowernumber => $local_patron->borrowernumber,
biblionumber => $biblio->biblionumber,
priority => 2,
itemnumber => $item->id,
}
);
C4::HoldsQueue::CreateQueue();
@ -987,9 +1141,14 @@ subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue
}
);
$reserve_id =
AddReserve( $item1->homebranch, $patron->borrowernumber, $biblio->id, '',
1, undef, undef, undef, undef, undef, undef, undef );
$reserve_id = AddReserve(
{
branchcode => $item1->homebranch,
borrowernumber => $patron->borrowernumber,
biblionumber => $biblio->id,
priority => 1
}
);
C4::HoldsQueue::CreateQueue();

37
t/db_dependent/Items/GetItemsForInventory.t

@ -100,15 +100,34 @@ subtest 'Skip items with waiting holds' => sub {
is( $first_items_count + 2, $second_items_count, 'Two items added, count makes sense' );
# Add 2 waiting holds
C4::Reserves::AddReserve( $library->branchcode, $patron_1->borrowernumber,
$item_1->biblionumber, '', 1, undef, undef, '', "title for fee",
$item_1->itemnumber, 'W' );
C4::Reserves::AddReserve( $library->branchcode, $patron_1->borrowernumber,
$item_2->biblionumber, '', 1, undef, undef, '', "title for fee",
$item_2->itemnumber, undef );
C4::Reserves::AddReserve( $library->branchcode, $patron_2->borrowernumber,
$item_2->biblionumber, '', 2, undef, undef, '', "title for fee",
$item_2->itemnumber, undef );
C4::Reserves::AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron_1->borrowernumber,
biblionumber => $item_1->biblionumber,
priority => 1,
itemnumber => $item_1->itemnumber,
found => 'W'
}
);
C4::Reserves::AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron_1->borrowernumber,
biblionumber => $item_2->biblionumber,
priority => 1,
itemnumber => $item_2->itemnumber,
}
);
C4::Reserves::AddReserve(
{
branchcode => $library->branchcode,
borrowernumber => $patron_2->borrowernumber,
biblionumber => $item_2->biblionumber,
priority => 2,
itemnumber => $item_2->itemnumber,
}
);
my ( $new_items, $new_items_count ) = GetItemsForInventory( { ignore_waiting_holds => 1 } );
is( $new_items_count, $first_items_count + 1, 'Item on hold skipped, count makes sense' );

36
t/db_dependent/Koha/Acquisition/Order.t

@ -296,26 +296,32 @@ subtest 'current_item_level_holds() tests' => sub {
my $item_3 = $builder->build_sample_item( { biblionumber => $biblio->biblionu