From e315830066bb424c7bad9fd899a7c8c436f9e0a5 Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Wed, 29 May 2019 02:06:46 -0300 Subject: [PATCH] Bug 22284: (follow-up) Squash multiple follow-ups * Bug 22284: (follow-up) Use GetReserveControlBranch in Koha::Item->pickup_locations Signed-off-by: Josef Moravec * Bug 22284: (follow-up) Fix tests Signed-off-by: Josef Moravec * Bug 22284: (follow-up) Fix typo in request.tt Signed-off-by: Josef Moravec * Bug 22284: (follow-up) Filter pickup on specific item click Signed-off-by: Josef Moravec * Bug 22284: (follow-up) Fix typos transfered -> transferred Signed-off-by: Josef Moravec Signed-off-by: Liz Rea --- Koha/Biblio.pm | 2 +- Koha/Item.pm | 6 +- .../prog/en/modules/reserve/request.tt | 20 +- reserve/request.pl | 2 + t/db_dependent/Koha/Biblios.t | 407 ++++------------ t/db_dependent/Koha/Items.t | 455 ++++-------------- t/db_dependent/Koha/Libraries.t | 57 ++- 7 files changed, 250 insertions(+), 699 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index dcff529bcc..2c34490351 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -174,7 +174,7 @@ sub can_be_transferred { @pickup_locations = $biblio->pickup_locations( {patron => $patron } ) Returns possible pickup locations for this biblio items, according to patron's home library (if patron is defined and holds are allowed only from hold groups) -and if item can be transfered to each pickup location. +and if item can be transferred to each pickup location. =cut diff --git a/Koha/Item.pm b/Koha/Item.pm index 83578b9615..79d9a1f7a8 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -27,6 +27,7 @@ use Koha::DateUtils qw( dt_from_string ); use C4::Context; use C4::Circulation; +use C4::Reserves; use Koha::Checkouts; use Koha::IssuingRules; use Koha::Item::Transfer::Limits; @@ -300,7 +301,7 @@ sub can_be_transferred { @pickup_locations = $item->pickup_locations( {patron => $patron } ) Returns possible pickup locations for this item, according to patron's home library (if patron is defined and holds are allowed only from hold groups) -and if item can be transfered to each pickup location. +and if item can be transferred to each pickup location. =cut @@ -310,7 +311,7 @@ sub pickup_locations { my $patron = $params->{patron}; my $circ_control_branch = - C4::Circulation::_GetCircControlBranch( $self->unblessed(), $patron ); + C4::Reserves::GetReservesControlBranch( $self->unblessed(), $patron->unblessed ); my $branchitemrule = C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype ); @@ -345,6 +346,7 @@ sub pickup_locations { push @pickup_locations, $library->unblessed; } } + return wantarray ? @pickup_locations : \@pickup_locations; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 4b3d20c1b4..5f455413a9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -425,10 +425,10 @@ @@ -544,7 +544,7 @@ Vol no. [% END %] Information - Allowed pickup locations + Allowed pickup locations @@ -581,8 +581,8 @@ Not holdable [% ELSIF itemloo.not_holdable == 'cannotReserveFromOtherBranches' %] Patron is from different library - [% ELSIF itemloo.not_holdable == 'branchNotInHoldGroup' %] - Cannot place hold from patrons's library + [% ELSIF itemloo.not_holdable == 'branchNotInHoldGroup' %] + Cannot place hold from patron's library [% ELSIF itemloo.not_holdable == 'itemAlreadyOnHold' %] Patron already has hold for this item [% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %] @@ -680,9 +680,9 @@ Not for loan ([% AuthorisedValues.GetByCode( 'NOT_LOAN', itemloo.notforloan ) | html %]) [% END %] - - [% itemloo.pickup_locations | html %] - + + [% itemloo.pickup_locations | html %] + [% END # / UNLESS itemloo.hide %] [% END # /FOREACH itemloo %] diff --git a/reserve/request.pl b/reserve/request.pl index 7460fde426..2b42cc6298 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -564,8 +564,10 @@ foreach my $biblionumber (@biblionumbers) { $num_available++; if($branchitemrule->{'hold_fulfillment_policy'} eq 'any' ) { $item->{pickup_locations} = 'Any library'; + $item->{pickup_locations_code} = 'all'; } else { $item->{pickup_locations} = join (', ', map { $_->{branchname} } Koha::Items->find($itemnumber)->pickup_locations({ patron => $patron })); + $item->{pickup_locations_code} = join (',', map { $_->{branchcode} } Koha::Items->find($itemnumber)->pickup_locations({ patron => $patron })); } push( @available_itemtypes, $item->{itype} ); diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index c03d28434e..9adb7b5833 100644 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -192,7 +192,7 @@ $schema->storage->txn_rollback; subtest 'pickup_locations' => sub { - plan tests => 25; + plan tests => 8; $schema->storage->txn_begin; @@ -221,97 +221,12 @@ subtest 'pickup_locations' => sub { my $library4 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); my $library5 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); my $library6 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + my $library7 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + my $library8 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0 } } ); - my $group1_1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library1->branchcode } } ); - my $group1_2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library2->branchcode } } ); - - my $group2_3 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library3->branchcode } } ); - my $group2_4 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library4->branchcode } } ); - - my $group3_5 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root3->id, branchcode => $library5->branchcode } } ); - my $group3_6 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root3->id, branchcode => $library6->branchcode } } ); - - my $biblio1 = $builder->build_object( { class => 'Koha::Biblios' } ); - my $biblioitem1 = $builder->build_object( { class => 'Koha::Biblioitems', value => { biblionumber => $biblio1->biblionumber } } ); - my $biblio2 = $builder->build_object( { class => 'Koha::Biblios' } ); - my $biblioitem2 = $builder->build_object( { class => 'Koha::Biblioitems', value => { biblionumber => $biblio2->biblionumber } } ); - - my $item1_1 = Koha::Item->new({ - biblionumber => $biblio1->biblionumber, - biblioitemnumber => $biblioitem1->biblioitemnumber, - homebranch => $library1->branchcode, - holdingbranch => $library2->branchcode, - itype => 'test', - barcode => "item11barcode", - })->store; - - my $item1_3 = Koha::Item->new({ - biblionumber => $biblio1->biblionumber, - biblioitemnumber => $biblioitem1->biblioitemnumber, - homebranch => $library3->branchcode, - holdingbranch => $library4->branchcode, - itype => 'test', - barcode => "item13barcode", - })->store; - - my $item2_2 = Koha::Item->new({ - biblionumber => $biblio2->biblionumber, - biblioitemnumber => $biblioitem2->biblioitemnumber, - homebranch => $library2->branchcode, - holdingbranch => $library1->branchcode, - itype => 'test', - barcode => "item22barcode", - })->store; - - my $item2_3 = Koha::Item->new({ - biblionumber => $biblio2->biblionumber, - biblioitemnumber => $biblioitem2->biblioitemnumber, - homebranch => $library3->branchcode, - holdingbranch => $library3->branchcode, - itype => 'test', - barcode => "item23barcode", - })->store; - - my $item2_4 = Koha::Item->new({ - biblionumber => $biblio2->biblionumber, - biblioitemnumber => $biblioitem2->biblioitemnumber, - homebranch => $library4->branchcode, - holdingbranch => $library4->branchcode, - itype => 'test', - barcode => "item24barcode", - })->store; - - my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library1->branchcode } } ); - my $patron4 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library4->branchcode } } ); - - t::lib::Mocks::mock_preference('HomeOrHoldingBranch', 'homebranch'); - - #Case 1: holdallowed any, hold_fulfillment_policy any Koha::CirculationRules->set_rules( { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); - - my @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - my @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - my @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - my @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - - ok(scalar(@pl_1_1) == 5 && scalar(@pl_1_4) == 5 && scalar(@pl_2_1) == 5 && scalar(@pl_2_4) == 5, 'Returns all libraries that are pickup locations'); - - #Case 2: holdallowed homebranch, hold_fulfillment_policy any, HomeOrHoldingBranch 'homebranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, + branchcode => $library1->branchcode, itemtype => undef, categorycode => undef, rules => { @@ -322,103 +237,48 @@ subtest 'pickup_locations' => sub { } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 5 && scalar(@pl_2_4) == 5, 'Returns all libraries that are pickup locations, when item\'s hombebranch equals patron\' homebranch'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_2_1) == 0, 'Returns no pickup locations'); - - #Case 3: holdallowed holdgroup, hold_fulfillment_policy any Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library2->branchcode, itemtype => undef, categorycode => undef, rules => { holdallowed => 3, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 5 && scalar(@pl_2_4) == 5 && scalar(@pl_1_4) == 5 && scalar(@pl_2_1) == 5, 'Returns all libraries that are pickup_locations, when item\'s hombebranch is in patron\' holdgroup'); - - #Case 4: holdallowed any, hold_fulfillment_policy holdgroup - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, hold_fulfillment_policy => 'holdgroup', returnbranch => 'any' } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 3 && scalar(@pl_2_4) == 3 && scalar(@pl_1_4) == 3 && scalar(@pl_2_1) == 3, 'Returns libraries in item\'s holdgroup, and that are pickup_locations'); - - #Case 5: holdallowed homebranch, hold_fulfillment_policy holdgroup, HomeOrHoldingBranch 'homebranch' Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library3->branchcode, itemtype => undef, categorycode => undef, rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdgroup', + holdallowed => 3, + hold_fulfillment_policy => 'patrongroup', returnbranch => 'any' } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 2 && scalar(@pl_2_4) == 1, 'Returns libraries in item\'s holdgroup whose homebranch equals patron\'s homebranch, and that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_2_1) == 0, 'Returns no pickup locations'); - - #Case 6: holdallowed holdgroup, hold_fulfillment_policy holdgroup Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library4->branchcode, itemtype => undef, categorycode => undef, rules => { - holdallowed => 3, - hold_fulfillment_policy => 'holdgroup', + holdallowed => 2, + hold_fulfillment_policy => 'holdingbranch', returnbranch => 'any' } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 2 && scalar(@pl_2_1) == 2 && scalar(@pl_2_4) == 1 && scalar(@pl_1_4) == 1, 'Returns libraries in item\'s holdgroup whose homebranch is included patron\'s holdgroup, and that are pickup_locations'); - - #Case 7: holdallowed any, hold_fulfillment_policy homebranch Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library5->branchcode, itemtype => undef, categorycode => undef, rules => { @@ -429,211 +289,146 @@ subtest 'pickup_locations' => sub { } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && scalar(@pl_1_4) == 1 && scalar(@pl_2_1) == 2 && scalar(@pl_2_4) == 2, 'Returns homebranch of items in biblio, that are pickup_locations'); - - #Case 8: holdallowed homebranch, hold_fulfillment_policy homebranch, HomeOrHoldingBranch 'homebranch' Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library6->branchcode, itemtype => undef, categorycode => undef, rules => { holdallowed => 1, - hold_fulfillment_policy => 'homebranch', + hold_fulfillment_policy => 'holdgroup', returnbranch => 'any' } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && scalar(@pl_2_4) == 1 && $pl_1_1[0]->{branchcode} eq $library1->branchcode && $pl_2_4[0]->{branchcode} eq $library4->branchcode, 'Returns homebranch of items in biblio that equals patron\'s homebranch, and that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_2_1) == 0, 'No pickup locations returned'); - - #Case 9: holdallowed holdgroup, hold_fulfillment_policy homebranch Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library7->branchcode, itemtype => undef, categorycode => undef, rules => { holdallowed => 3, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && scalar(@pl_2_1) == 1 && scalar(@pl_2_4) == 1, 'Returns homebranch of items in biblio that are within patron\'s holdgroup, and that are pickup_locations'); - ok(scalar(@pl_1_4) == 0, 'No pickup locations returned'); - - #Case 10: holdallowed any, hold_fulfillment_policy holdingbranch - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, hold_fulfillment_policy => 'holdingbranch', returnbranch => 'any' } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 2 && scalar(@pl_1_4) == 2 && scalar(@pl_2_1) == 2 && scalar(@pl_2_4) == 2, 'Returns holdingbranch of items in biblio, that are pickup_locations'); - #Case 11: holdallowed homebranch, hold_fulfillment_policy holdingbranch, HomeOrHoldingBranch 'homebranch' Koha::CirculationRules->set_rules( { - branchcode => undef, + branchcode => $library8->branchcode, itemtype => undef, categorycode => undef, rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdingbranch', + holdallowed => 2, + hold_fulfillment_policy => 'patrongroup', returnbranch => 'any' } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); + my $group1_1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library1->branchcode } } ); + my $group1_2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library2->branchcode } } ); - ok(scalar(@pl_1_1) == 1 && scalar(@pl_2_4) == 1, 'Returns holdingbranch of items in biblio, whose homebranch equals patron\'s, and that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_2_1) == 0, 'No pickup locations returned'); + my $group2_3 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library3->branchcode } } ); + my $group2_4 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library4->branchcode } } ); - #Case 12: holdallowed holdgroup, hold_fulfillment_policy holdingbranch - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); + my $group3_5 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root3->id, branchcode => $library5->branchcode } } ); + my $group3_6 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root3->id, branchcode => $library6->branchcode } } ); + my $group3_7 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root3->id, branchcode => $library7->branchcode } } ); + my $group3_8 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root3->id, branchcode => $library8->branchcode } } ); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); + my $biblio1 = $builder->build_object( { class => 'Koha::Biblios', value => {title => '1'} } ); + my $biblioitem1 = $builder->build_object( { class => 'Koha::Biblioitems', value => { biblionumber => $biblio1->biblionumber } } ); + my $biblio2 = $builder->build_object( { class => 'Koha::Biblios', value => {title => '2'} } ); + my $biblioitem2 = $builder->build_object( { class => 'Koha::Biblioitems', value => { biblionumber => $biblio2->biblionumber } } ); - ok(scalar(@pl_1_1) == 1 && scalar(@pl_2_4) == 1 && scalar(@pl_1_4) == 1 && scalar(@pl_2_1) == 1, 'Returns holdingbranch of items in biblio, whose homebranch are within patron\'s holdgroup, and that are pickup_locations'); + my $item1_1 = Koha::Item->new({ + biblionumber => $biblio1->biblionumber, + biblioitemnumber => $biblioitem1->biblioitemnumber, + homebranch => $library1->branchcode, + holdingbranch => $library2->branchcode, + itype => 'test', + barcode => "item11barcode", + })->store; - t::lib::Mocks::mock_preference('HomeOrHoldingBranch', 'holdingbranch'); + my $item1_3 = Koha::Item->new({ + biblionumber => $biblio1->biblionumber, + biblioitemnumber => $biblioitem1->biblioitemnumber, + homebranch => $library3->branchcode, + holdingbranch => $library4->branchcode, + itype => 'test', + barcode => "item13barcode", + })->store; - #Case 13: holdallowed homebranch, hold_fulfillment_policy any, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); + my $item1_7 = Koha::Item->new({ + biblionumber => $biblio1->biblionumber, + biblioitemnumber => $biblioitem1->biblioitemnumber, + homebranch => $library7->branchcode, + holdingbranch => $library4->branchcode, + itype => 'test', + barcode => "item17barcode", + })->store; - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); + my $item2_2 = Koha::Item->new({ + biblionumber => $biblio2->biblionumber, + biblioitemnumber => $biblioitem2->biblioitemnumber, + homebranch => $library2->branchcode, + holdingbranch => $library1->branchcode, + itype => 'test', + barcode => "item22barcode", + })->store; - ok(scalar(@pl_1_4) == 5 && scalar(@pl_2_1) == 5 && scalar(@pl_2_4) == 5, 'Returns all libraries when item\'s holdingbranch equals patron\'s homebranch, and that are pickup_locations'); - ok(scalar(@pl_1_1) == 0, 'No pickup locations returned'); + my $item2_4 = Koha::Item->new({ + biblionumber => $biblio2->biblionumber, + biblioitemnumber => $biblioitem2->biblioitemnumber, + homebranch => $library4->branchcode, + holdingbranch => $library3->branchcode, + itype => 'test', + barcode => "item23barcode", + })->store; - #Case 14: holdallowed homebranch, hold_fulfillment_policy holdgroup, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); + my $item2_6 = Koha::Item->new({ + biblionumber => $biblio2->biblionumber, + biblioitemnumber => $biblioitem2->biblioitemnumber, + homebranch => $library6->branchcode, + holdingbranch => $library4->branchcode, + itype => 'test', + barcode => "item26barcode", + })->store; - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); + my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { firstname=>'1', branchcode => $library1->branchcode } } ); + my $patron8 = $builder->build_object( { class => 'Koha::Patrons', value => { firstname=>'8', branchcode => $library8->branchcode } } ); - ok(scalar(@pl_1_4) == 1 && scalar(@pl_2_1) == 2 && scalar(@pl_2_4) == 1, 'Returns libraries in item\'s holdgroup whose holdingbranch equals patron\'s homebranch, and that are pickup_locations'); - ok(scalar(@pl_1_1) == 0, 'No pickup locations returned'); + my $results = { + "ItemHomeLibrary-1-1" => 6, + "ItemHomeLibrary-1-8" => 1, + "ItemHomeLibrary-2-1" => 2, + "ItemHomeLibrary-2-8" => 0, + "PatronLibrary-1-1" => 6, + "PatronLibrary-1-8" => 3, + "PatronLibrary-2-1" => 0, + "PatronLibrary-2-8" => 3, + }; - #Case 15: holdallowed homebranch, hold_fulfillment_policy homebranch, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' - } - } - ); + sub _doTest { + my ( $cbranch, $biblio, $patron, $results ) = @_; + t::lib::Mocks::mock_preference('ReservesControlBranch', $cbranch); - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); + my @pl = $biblio->pickup_locations( { patron => $patron} ); - #ok(scalar(@pl_2_4) == 1 && $pl_2_4[0]->{branchcode} eq $library4->branchcode, 'Pickup location for patron 4 and item 3 renders item\'s holding branch'); - ok(scalar(@pl_2_1) == 1 && scalar(@pl_2_4) == 1, 'Returns homebranch of items in biblio whose holdingbranch equals patron\'s homebranch, and that are pickup_locations'); - ok(scalar(@pl_1_1) == 0 && scalar(@pl_1_4) == 0, 'No pickup locations returned'); + ok(scalar(@pl) == $results->{$cbranch.'-'.$biblio->title.'-'.$patron->firstname}, 'ReservesControlBranch: '.$cbranch.', biblio'.$biblio->title.', patron'.$patron->firstname.' should return '.$results->{$cbranch.'-'.$biblio->title.'-'.$patron->firstname}.' but returns '.scalar(@pl)); + } - #Case 16: holdallowed homebranch, hold_fulfillment_policy holdingbranch, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' + foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') { + foreach my $biblio ($biblio1, $biblio2) { + foreach my $patron ($patron1, $patron8) { + _doTest($cbranch, $biblio, $patron, $results); } } - ); - - @pl_1_1 = $biblio1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $biblio1->pickup_locations( { patron => $patron4 } ); - @pl_2_1 = $biblio2->pickup_locations( { patron => $patron1 } ); - @pl_2_4 = $biblio2->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_4) == 1 && scalar(@pl_2_1) == 1 && scalar(@pl_2_4) == 1, 'Returns holdingbranch of items in biblio that equals patron\'s homebranch, and that are pickup_locations'); - ok(scalar(@pl_1_1) == 0, 'No pickup locations returned'); + } $schema->storage->txn_rollback; }; \ No newline at end of file diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index d777496106..c4bae05d75 100644 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 10; +use Test::More tests => 11; use Test::Exception; use C4::Circulation; @@ -188,7 +188,7 @@ is( Koha::Items->search->count, $nb_of_items + 1, 'Delete should have deleted th $schema->storage->txn_rollback; subtest 'pickup_locations' => sub { - plan tests => 33; + plan tests => 60; $schema->storage->txn_begin; @@ -228,8 +228,8 @@ subtest 'pickup_locations' => sub { biblioitemnumber => $biblioitem->{biblioitemnumber}, homebranch => $library1->branchcode, holdingbranch => $library2->branchcode, + barcode => '1', itype => 'test', - barcode => "item1barcode", })->store; my $item3 = Koha::Item->new({ @@ -237,371 +237,108 @@ subtest 'pickup_locations' => sub { biblioitemnumber => $biblioitem->{biblioitemnumber}, homebranch => $library3->branchcode, holdingbranch => $library4->branchcode, + barcode => '3', itype => 'test', - barcode => "item3barcode", })->store; - my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library1->branchcode } } ); - my $patron4 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library4->branchcode } } ); - - t::lib::Mocks::mock_preference('HomeOrHoldingBranch', 'homebranch'); - - #Case 1: holdallowed any, hold_fulfillment_policy any - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'any', - returnbranch => 'any' + my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library1->branchcode, firstname => '1' } } ); + my $patron4 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library4->branchcode, firstname => '4' } } ); + + my $results = { + "1-1-1-any" => 3, + "1-1-1-holdgroup" => 2, + "1-1-1-patrongroup" => 2, + "1-1-1-homebranch" => 1, + "1-1-1-holdingbranch" => 1, + "1-1-2-any" => 3, + "1-1-2-holdgroup" => 2, + "1-1-2-patrongroup" => 2, + "1-1-2-homebranch" => 1, + "1-1-2-holdingbranch" => 1, + "1-1-3-any" => 3, + "1-1-3-holdgroup" => 2, + "1-1-3-patrongroup" => 2, + "1-1-3-homebranch" => 1, + "1-1-3-holdingbranch" => 1, + "1-4-1-any" => 0, + "1-4-1-holdgroup" => 0, + "1-4-1-patrongroup" => 0, + "1-4-1-homebranch" => 0, + "1-4-1-holdingbranch" => 0, + "1-4-2-any" => 3, + "1-4-2-holdgroup" => 2, + "1-4-2-patrongroup" => 1, + "1-4-2-homebranch" => 1, + "1-4-2-holdingbranch" => 1, + "1-4-3-any" => 0, + "1-4-3-holdgroup" => 0, + "1-4-3-patrongroup" => 0, + "1-4-3-homebranch" => 0, + "1-4-3-holdingbranch" => 0, + "3-1-1-any" => 0, + "3-1-1-holdgroup" => 0, + "3-1-1-patrongroup" => 0, + "3-1-1-homebranch" => 0, + "3-1-1-holdingbranch" => 0, + "3-1-2-any" => 3, + "3-1-2-holdgroup" => 1, + "3-1-2-patrongroup" => 2, + "3-1-2-homebranch" => 0, + "3-1-2-holdingbranch" => 1, + "3-1-3-any" => 0, + "3-1-3-holdgroup" => 0, + "3-1-3-patrongroup" => 0, + "3-1-3-homebranch" => 0, + "3-1-3-holdingbranch" => 0, + "3-4-1-any" => 0, + "3-4-1-holdgroup" => 0, + "3-4-1-patrongroup" => 0, + "3-4-1-homebranch" => 0, + "3-4-1-holdingbranch" => 0, + "3-4-2-any" => 3, + "3-4-2-holdgroup" => 1, + "3-4-2-patrongroup" => 1, + "3-4-2-homebranch" => 0, + "3-4-2-holdingbranch" => 1, + "3-4-3-any" => 3, + "3-4-3-holdgroup" => 1, + "3-4-3-patrongroup" => 1, + "3-4-3-homebranch" => 0, + "3-4-3-holdingbranch" => 1 + }; + + sub _doTest { + my ( $item, $patron, $ha, $hfp, $results ) = @_; + + Koha::CirculationRules->set_rules( + { + branchcode => undef, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => $ha, + hold_fulfillment_policy => $hfp, + returnbranch => 'any' + } } - } - ); - - my @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - my @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - my @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - my @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == scalar(@pl_1_4) && scalar(@pl_1_1) == scalar(@pl_3_1) && scalar(@pl_1_1) == scalar(@pl_3_4), 'All combinations of patron/item renders the same number of locations'); - - #Case 2: holdallowed homebranch, hold_fulfillment_policy any, HomeOrHoldingBranch 'homebranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 3, 'Pickup location for patron 1 and item 1 renders all libraries that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_3_4) == 0, 'Any other combination renders no locations'); - - #Case 3: holdallowed holdgroup, hold_fulfillment_policy any - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 3, 'Pickup location for patron 1 and item 1 renders all libraries that are pickup_locations'); - ok(scalar(@pl_3_4) == 3, 'Pickup location for patron 4 and item 3 renders all libraries that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0, 'Any other combination renders no locations'); - - #Case 4: holdallowed any, hold_fulfillment_policy holdgroup - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); + ); + my @pl = $item->pickup_locations( { patron => $patron} ); + my $ha_value=$ha==3?'holdgroup':($ha==2?'any':'homebranch'); - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 2 && scalar(@pl_1_4) == 2, 'Pickup locations for item 1 renders all libraries in items\'s holdgroup that are pickup_locations'); - ok(scalar(@pl_3_1) == 1 && scalar(@pl_3_4) == 1, 'Pickup locations for item 3 renders all libraries in items\'s holdgroup that are pickup_locations'); - - #Case 5: holdallowed homebranch, hold_fulfillment_policy holdgroup, HomeOrHoldingBranch 'homebranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 2, 'Pickup location for patron 1 and item 1 renders all libraries in holdgroup that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_3_4) == 0, 'Any other combination renders no locations'); - - #Case 6: holdallowed holdgroup, hold_fulfillment_policy holdgroup - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 2, 'Pickup location for patron 1 and item 1 renders all libraries that are pickup_locations'); - ok(scalar(@pl_3_4) == 1, 'Pickup location for patron 4 and item 3 renders all libraries that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0, 'Any other combination renders no locations'); - - #Case 7: holdallowed any, hold_fulfillment_policy homebranch - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && scalar(@pl_1_4) == 1 && $pl_1_1[0]->{branchcode} eq $library1->branchcode && $pl_1_4[0]->{branchcode} eq $library1->id, 'Pickup locations for item 1 renders item\'s homelibrary'); - ok(scalar(@pl_3_1) == 0 && scalar(@pl_3_4) == 0, 'Any other combination renders no locations, because library3 is not pickup_location'); - - #Case 8: holdallowed homebranch, hold_fulfillment_policy homebranch, HomeOrHoldingBranch 'homebranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && $pl_1_1[0]->{branchcode} eq $library1->branchcode, 'Pickup location for patron 1 and item 1 renders item\'s homebranch'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_3_4) == 0, 'Any other combination renders no locations'); - - #Case 9: holdallowed holdgroup, hold_fulfillment_policy homebranch - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1, 'Pickup location for patron 1 and item 1 renders item\'s homebranch'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_3_4) == 0, 'Any other combination renders no locations'); - - #Case 10: holdallowed any, hold_fulfillment_policy holdingbranch - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && scalar(@pl_1_4) == 1 && $pl_1_1[0]->{branchcode} eq $library2->branchcode && $pl_1_4[0]->{branchcode} eq $library2->branchcode, 'Pickup locations for item 1 renders item\'s holding branch'); - ok(scalar(@pl_3_1) == 1 && scalar(@pl_3_4) == 1 && $pl_3_1[0]->{branchcode} eq $library4->branchcode && $pl_3_4[0]->{branchcode} eq $library4->branchcode, 'Pickup locations for item 3 renders item\'s holding branch'); - - - #Case 11: holdallowed homebranch, hold_fulfillment_policy holdingbranch, HomeOrHoldingBranch 'homebranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && $pl_1_1[0]->{branchcode} eq $library2->branchcode, 'Pickup location for patron 1 and item 1 renders item\'s holding branch'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_3_4) == 0, 'Any other combination renders no locations'); - - #Case 12: holdallowed holdgroup, hold_fulfillment_policy holdingbranch - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_1_1) == 1 && $pl_1_1[0]->{branchcode} eq $library2->branchcode, 'Pickup location for patron 1 and item 1 renders item\'s holding branch'); - ok(scalar(@pl_3_4) == 1 && $pl_3_4[0]->{branchcode} eq $library4->branchcode, 'Pickup location for patron 4 and item 3 renders item\'s holding branch'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0, 'Any other combination renders no locations'); - - t::lib::Mocks::mock_preference('HomeOrHoldingBranch', 'holdingbranch'); - - #Case 13: holdallowed homebranch, hold_fulfillment_policy any, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); + ok(scalar(@pl) == $results->{$item->barcode.'-'.$patron->firstname.'-'.$ha.'-'.$hfp}, 'item'.$item->barcode.', patron'.$patron->firstname.', holdallowed: '.$ha_value.', hold_fulfillment_policy: '.$hfp.' should return '.$results->{$item->barcode.'-'.$patron->firstname.'-'.$ha.'-'.$hfp}.' but returns '.scalar(@pl)); + } - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_3_4) == 3, 'Pickup location for patron 4 and item 3 renders all libraries that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_1_1) == 0, 'Any other combination renders no locations'); - - #Case 14: holdallowed homebranch, hold_fulfillment_policy holdgroup, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_3_4) == 1, 'Pickup location for patron 4 and item 3 renders all libraries in holdgroup that are pickup_locations'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_1_1) == 0, 'Any other combination renders no locations'); - - #Case 15: holdallowed homebranch, hold_fulfillment_policy homebranch, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' + foreach my $item ($item1, $item3) { + foreach my $patron ($patron1, $patron4) { + #holdallowed 1: homebranch, 2: any, 3: holdgroup + foreach my $ha (1, 2, 3) { + foreach my $hfp ('any', 'holdgroup', 'patrongroup', 'homebranch', 'holdingbranch') { + _doTest($item, $patron, $ha, $hfp, $results); + } } } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - #ok(scalar(@pl_3_4) == 1 && $pl_3_4[0]->{branchcode} eq $library4->branchcode, 'Pickup location for patron 4 and item 3 renders item\'s holding branch'); - ok(scalar(@pl_3_4) == 0 && scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_1_1) == 0, 'Any combination of patron/item renders no locations'); - - #Case 16: holdallowed homebranch, hold_fulfillment_policy holdingbranch, HomeOrHoldingBranch 'holdingbranch' - Koha::CirculationRules->set_rules( - { - branchcode => undef, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); - - @pl_1_1 = $item1->pickup_locations( { patron => $patron1 } ); - @pl_1_4 = $item1->pickup_locations( { patron => $patron4 } ); - @pl_3_1 = $item3->pickup_locations( { patron => $patron1 } ); - @pl_3_4 = $item3->pickup_locations( { patron => $patron4 } ); - - ok(scalar(@pl_3_4) == 1 && $pl_3_4[0]->{branchcode} eq $library4->branchcode, 'Pickup location for patron 1 and item 1 renders item\'s holding branch'); - ok(scalar(@pl_1_4) == 0 && scalar(@pl_3_1) == 0 && scalar(@pl_1_1) == 0, 'Any other combination renders no locations'); + } $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Libraries.t b/t/db_dependent/Koha/Libraries.t index a59b4f756f..1f6b6dcf4c 100644 --- a/t/db_dependent/Koha/Libraries.t +++ b/t/db_dependent/Koha/Libraries.t @@ -31,6 +31,7 @@ use Koha::Items; use Koha::Library; use Koha::Libraries; use Koha::Database; +use Koha::CirculationRules; use t::lib::Mocks; use t::lib::TestBuilder; @@ -90,6 +91,19 @@ isa_ok( $srstages->next, 'Koha::StockRotationStage', "Relationship correctly cre subtest 'pickup_locations' => sub { plan tests => 2; + Koha::CirculationRules->set_rules( + { + branchcode => undef, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 2, + hold_fulfillment_policy => 'any', + returnbranch => 'any' + } + } + ); + my $from = Koha::Library->new({ branchcode => 'zzzfrom', branchname => 'zzzfrom', @@ -112,6 +126,7 @@ subtest 'pickup_locations' => sub { my $item1 = $builder->build_sample_item({%$item_info}); my $item2 = $builder->build_sample_item({%$item_info}); my $item3 = $builder->build_sample_item({%$item_info}); + my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $from->branchcode } } ); subtest 'UseBranchTransferLimits = OFF' => sub { plan tests => 5; @@ -128,7 +143,7 @@ subtest 'pickup_locations' => sub { my $total_pickup = Koha::Libraries->search({ pickup_location => 1 })->count; - my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); is(C4::Context->preference('UseBranchTransferLimits'), 0, 'Given system ' .'preference UseBranchTransferLimits is switched OFF,'); is(@{$pickup}, $total_pickup, 'Then the total number of pickup locations ' @@ -136,15 +151,15 @@ subtest 'pickup_locations' => sub { t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype'); t::lib::Mocks::mock_preference('item-level_itypes', 1); - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); is(@{$pickup}, $total_pickup, '...when ' .'BranchTransferLimitsType = itemtype and item-level_itypes = 1'); t::lib::Mocks::mock_preference('item-level_itypes', 0); - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); is(@{$pickup}, $total_pickup, '...as well as when ' .'BranchTransferLimitsType = itemtype and item-level_itypes = 0'); t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'ccode'); - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); is(@{$pickup}, $total_pickup, '...as well as when ' .'BranchTransferLimitsType = ccode'); t::lib::Mocks::mock_preference('item-level_itypes', 1); @@ -174,7 +189,7 @@ subtest 'pickup_locations' => sub { 'Given all items of a biblio have same the itemtype,'); is($limit->itemtype, $item1->effective_itemtype, 'and given there ' .'is an existing transfer limit for that itemtype,'); - my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber}); + my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); my $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -183,7 +198,7 @@ subtest 'pickup_locations' => sub { } is($found, 0, 'Then the to-library of which the limit applies for, ' .'is not included in the list of pickup libraries.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item1 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -203,7 +218,7 @@ subtest 'pickup_locations' => sub { is(Koha::Item::Transfer::Limits->search({ itemtype => $item2->effective_itemtype })->count, 0, 'and it is' .' not restricted by transfer limits,'); - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -212,7 +227,7 @@ subtest 'pickup_locations' => sub { } is($found, 1, 'Then the to-library of which the limit applies for, ' .'is included in the list of pickup libraries.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item2 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item2, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -222,7 +237,7 @@ subtest 'pickup_locations' => sub { is($found, 1, 'The same applies when asking pickup locations of ' .'a that particular item.'); Koha::Item::Transfer::Limits->delete; - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -231,7 +246,7 @@ subtest 'pickup_locations' => sub { } is($found, 1, 'Given we deleted transfer limit, the previously ' .'transfer-limited library is included in the list.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item1 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -260,7 +275,7 @@ subtest 'pickup_locations' => sub { 'Given items use biblio-level itemtype,'); is($limit->itemtype, $item1->effective_itemtype, 'and given there ' .'is an existing transfer limit for that itemtype,'); - my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); my $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -269,7 +284,7 @@ subtest 'pickup_locations' => sub { } is($found, 0, 'Then the to-library of which the limit applies for, ' .'is not included in the list of pickup libraries.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item1 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -284,7 +299,7 @@ subtest 'pickup_locations' => sub { is(@{$pickup}, $others, 'However, the number of other pickup ' .'libraries is correct.'); Koha::Item::Transfer::Limits->delete; - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -301,7 +316,7 @@ subtest 'pickup_locations' => sub { ok($item1->itype ne $item1->effective_itemtype && $limit->itemtype eq $item1->itype, 'Given we have added a limit' .' matching ITEM-level itemtype,'); - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -310,7 +325,7 @@ subtest 'pickup_locations' => sub { } is($found, 1, 'Then the limited branch is still included as a pickup' .' library.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item1 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -337,7 +352,7 @@ subtest 'pickup_locations' => sub { is($limit->ccode, $item1->ccode, 'Given there ' .'is an existing transfer limit for that ccode,'); - my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); my $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -346,7 +361,7 @@ subtest 'pickup_locations' => sub { } is($found, 0, 'Then the to-library of which the limit applies for, ' .'is not included in the list of pickup libraries.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item1 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -366,7 +381,7 @@ subtest 'pickup_locations' => sub { is(Koha::Item::Transfer::Limits->search({ ccode => $item3->ccode })->count, 0, 'and it is' .' not restricted by transfer limits,'); - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -375,7 +390,7 @@ subtest 'pickup_locations' => sub { } is($found, 1, 'Then the to-library of which the limit applies for, ' .'is included in the list of pickup libraries.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item3 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item3, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -385,7 +400,7 @@ subtest 'pickup_locations' => sub { is($found, 1, 'The same applies when asking pickup locations of ' .'a that particular item.'); Koha::Item::Transfer::Limits->delete; - $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber }); + $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { @@ -394,7 +409,7 @@ subtest 'pickup_locations' => sub { } is($found, 1, 'Given we deleted transfer limit, the previously ' .'transfer-limited library is included in the list.'); - $pickup = Koha::Libraries->pickup_locations({ item => $item1 }); + $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { if ($lib->{'branchcode'} eq $limit->toBranch) { -- 2.17.1