From 1304bd0488c603cfccc3f1cd3f38f4eca2914c29 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Sat, 21 Dec 2019 00:14:10 -0300 Subject: [PATCH] Bug 22284: (QA follow-up) Make pickup locations be Koha::Library objects This patch makes the following methods return array references of Koha::Library objects instead or unblessed objects; - Koha::Item->pickup_locations - Koha::Biblio->pickup_locations - Koha::Libraries->pickup_locations Bonus: - The template plugin is adjusted to unbless things to keep behavior - Tests are moved to the right .t file. - Tests for the new behavior are added. Signed-off-by: Tomas Cohen Arazi --- Koha/Biblio.pm | 2 +- Koha/Item.pm | 2 +- Koha/Libraries.pm | 8 +- Koha/Template/Plugin/Branches.pm | 6 +- reserve/request.pl | 12 +- t/db_dependent/Koha/Biblio.t | 266 ++++++++++++++++++++++++++++++- t/db_dependent/Koha/Biblios.t | 243 ---------------------------- t/db_dependent/Koha/Item.t | 189 +++++++++++++++++++++- t/db_dependent/Koha/Items.t | 158 +----------------- t/db_dependent/Koha/Libraries.t | 34 ++-- 10 files changed, 489 insertions(+), 431 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index e143bfb3c6..5b63653aec 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -191,7 +191,7 @@ sub pickup_locations { my %seen; @pickup_locations = - grep { !$seen{ $_->{branchcode} }++ } @pickup_locations; + grep { !$seen{ $_->branchcode }++ } @pickup_locations; return wantarray ? @pickup_locations : \@pickup_locations; } diff --git a/Koha/Item.pm b/Koha/Item.pm index 1d18d3410c..e77f8191c4 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -342,7 +342,7 @@ sub pickup_locations { my @pickup_locations; foreach my $library (@libs) { if ($library->pickup_location && $self->can_be_transferred({ to => $library })) { - push @pickup_locations, $library->unblessed; + push @pickup_locations, $library; } } diff --git a/Koha/Libraries.pm b/Koha/Libraries.pm index ebd552b3fc..c3464ee95b 100644 --- a/Koha/Libraries.pm +++ b/Koha/Libraries.pm @@ -44,7 +44,7 @@ Koha::Libraries - Koha Library Object set class =head3 pickup_locations -Returns available pickup locations for +Returns available pickup locations (Koha::Library objects) for A. a specific item B. a biblio C. none of the above, simply all libraries with pickup_location => 1 @@ -86,17 +86,17 @@ sub pickup_locations { order_by => ['branchname'] }); - return $libraries->unblessed unless $item or $biblio; + return $libraries unless $item or $biblio; if($item) { unless (ref($item) eq 'Koha::Item') { $item = Koha::Items->find($item); - return $libraries->unblessed unless $item; + return $libraries unless $item; } return $item->pickup_locations( {patron => $patron} ); } else { unless (ref($biblio) eq 'Koha::Biblio') { $biblio = Koha::Biblios->find($biblio); - return $libraries->unblessed unless $biblio; + return $libraries unless $biblio; } return $biblio->pickup_locations( {patron => $patron} ); } diff --git a/Koha/Template/Plugin/Branches.pm b/Koha/Template/Plugin/Branches.pm index da615f5673..bdb19707de 100644 --- a/Koha/Template/Plugin/Branches.pm +++ b/Koha/Template/Plugin/Branches.pm @@ -90,9 +90,9 @@ sub pickup_locations { my $search_params = $params->{search_params} || {}; my $selected = $params->{selected}; - my $libraries = Koha::Libraries->pickup_locations($search_params); + my @libraries = map { $_->unblessed } Koha::Libraries->pickup_locations($search_params); - for my $l (@$libraries) { + for my $l (@libraries) { if ( defined $selected and $l->{branchcode} eq $selected or not defined $selected and C4::Context->userenv @@ -102,7 +102,7 @@ sub pickup_locations { } } - return $libraries; + return \@libraries; } 1; diff --git a/reserve/request.pl b/reserve/request.pl index 225f03920e..c7376c2857 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -549,7 +549,7 @@ foreach my $biblionumber (@biblionumbers) { $item->{'holdallowed'} = $branchitemrule->{'holdallowed'}; my $can_item_be_reserved = CanItemBeReserved( $patron->borrowernumber, $itemnumber )->{status}; - $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved->{status} eq 'OK' ); + $item->{not_holdable} = $can_item_be_reserved unless ( $can_item_be_reserved eq 'OK' ); $item->{item_level_holds} = Koha::IssuingRules->get_opacitemholds_policy( { item => $item_object, patron => $patron } ); @@ -566,8 +566,14 @@ foreach my $biblionumber (@biblionumbers) { $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 })); + $item->{pickup_locations} = join( ', ', + map { $_->unblessed->{branchname} } + Koha::Items->find($itemnumber) + ->pickup_locations( { patron => $patron } ) ); + $item->{pickup_locations_code} = join( ',', + map { $_->unblessed->{branchcode} } + Koha::Items->find($itemnumber) + ->pickup_locations( { patron => $patron } ) ); } push( @available_itemtypes, $item->{itype} ); diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index bdf65bd7e2..06a5b40a52 100644 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 7; +use Test::More tests => 8; use C4::Biblio; use Koha::Database; @@ -170,3 +170,267 @@ subtest 'is_serial() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'pickup_locations' => sub { + plan tests => 29; + + $schema->storage->txn_begin; + + my $dbh = C4::Context->dbh; + + # Cleanup database + Koha::Holds->search->delete; + Koha::Patrons->search->delete; + Koha::Items->search->delete; + Koha::Libraries->search->delete; + $dbh->do('DELETE FROM issues'); + $dbh->do('DELETE FROM issuingrules'); + $dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) + VALUES (?, ?, ?, ?)}, + {}, + '*', '*', '*', 25 + ); + $dbh->do('DELETE FROM circulation_rules'); + + my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); + my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); + my $root3 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); + + my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0 } } ); + 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 } } ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library1->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 1, + hold_fulfillment_policy => 'any', + returnbranch => 'any' + } + } + ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library2->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 3, + hold_fulfillment_policy => 'holdgroup', + returnbranch => 'any' + } + } + ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library3->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 3, + hold_fulfillment_policy => 'patrongroup', + returnbranch => 'any' + } + } + ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library4->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 2, + hold_fulfillment_policy => 'holdingbranch', + returnbranch => 'any' + } + } + ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library5->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 2, + hold_fulfillment_policy => 'homebranch', + returnbranch => 'any' + } + } + ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library6->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 1, + hold_fulfillment_policy => 'holdgroup', + returnbranch => 'any' + } + } + ); + + Koha::CirculationRules->set_rules( + { + branchcode => $library7->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 3, + hold_fulfillment_policy => 'holdingbranch', + returnbranch => 'any' + } + } + ); + + + Koha::CirculationRules->set_rules( + { + branchcode => $library8->branchcode, + itemtype => undef, + categorycode => undef, + rules => { + holdallowed => 2, + hold_fulfillment_policy => 'patrongroup', + returnbranch => 'any' + } + } + ); + + 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 $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 } } ); + + 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 } } ); + + 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 $item1_7 = Koha::Item->new({ + biblionumber => $biblio1->biblionumber, + biblioitemnumber => $biblioitem1->biblioitemnumber, + homebranch => $library7->branchcode, + holdingbranch => $library4->branchcode, + itype => 'test', + barcode => "item17barcode", + })->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_4 = Koha::Item->new({ + biblionumber => $biblio2->biblionumber, + biblioitemnumber => $biblioitem2->biblioitemnumber, + homebranch => $library4->branchcode, + holdingbranch => $library3->branchcode, + itype => 'test', + barcode => "item23barcode", + })->store; + + my $item2_6 = Koha::Item->new({ + biblionumber => $biblio2->biblionumber, + biblioitemnumber => $biblioitem2->biblioitemnumber, + homebranch => $library6->branchcode, + holdingbranch => $library4->branchcode, + itype => 'test', + barcode => "item26barcode", + })->store; + + 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 } } ); + + 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, + }; + + sub _doTest { + my ( $cbranch, $biblio, $patron, $results ) = @_; + t::lib::Mocks::mock_preference('ReservesControlBranch', $cbranch); + + my @pl = $biblio->pickup_locations( { patron => $patron} ); + + foreach my $pickup_location (@pl) { + is( ref($pickup_location), 'Koha::Library', 'Object type is correct' ); + } + + 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) + ); + } + + foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') { + foreach my $biblio ($biblio1, $biblio2) { + foreach my $patron ($patron1, $patron8) { + _doTest($cbranch, $biblio, $patron, $results); + } + } + } + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index 6afd72dea0..d44d566924 100644 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -217,246 +217,3 @@ subtest 'custom_cover_image_url' => sub { }; $schema->storage->txn_rollback; - - -subtest 'pickup_locations' => sub { - plan tests => 8; - - $schema->storage->txn_begin; - - # Cleanup database - Koha::Holds->search->delete; - Koha::Patrons->search->delete; - Koha::Items->search->delete; - Koha::Libraries->search->delete; - $dbh->do('DELETE FROM issues'); - $dbh->do('DELETE FROM issuingrules'); - $dbh->do( - q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) - VALUES (?, ?, ?, ?)}, - {}, - '*', '*', '*', 25 - ); - $dbh->do('DELETE FROM circulation_rules'); - - my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); - my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); - my $root3 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); - - my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); - my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); - my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0 } } ); - 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 } } ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library1->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'any', - returnbranch => 'any' - } - } - ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library2->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library3->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'patrongroup', - returnbranch => 'any' - } - } - ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library4->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library5->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'homebranch', - returnbranch => 'any' - } - } - ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library6->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 1, - hold_fulfillment_policy => 'holdgroup', - returnbranch => 'any' - } - } - ); - - Koha::CirculationRules->set_rules( - { - branchcode => $library7->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 3, - hold_fulfillment_policy => 'holdingbranch', - returnbranch => 'any' - } - } - ); - - - Koha::CirculationRules->set_rules( - { - branchcode => $library8->branchcode, - itemtype => undef, - categorycode => undef, - rules => { - holdallowed => 2, - hold_fulfillment_policy => 'patrongroup', - returnbranch => 'any' - } - } - ); - - 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 $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 } } ); - - 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 } } ); - - 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 $item1_7 = Koha::Item->new({ - biblionumber => $biblio1->biblionumber, - biblioitemnumber => $biblioitem1->biblioitemnumber, - homebranch => $library7->branchcode, - holdingbranch => $library4->branchcode, - itype => 'test', - barcode => "item17barcode", - })->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_4 = Koha::Item->new({ - biblionumber => $biblio2->biblionumber, - biblioitemnumber => $biblioitem2->biblioitemnumber, - homebranch => $library4->branchcode, - holdingbranch => $library3->branchcode, - itype => 'test', - barcode => "item23barcode", - })->store; - - my $item2_6 = Koha::Item->new({ - biblionumber => $biblio2->biblionumber, - biblioitemnumber => $biblioitem2->biblioitemnumber, - homebranch => $library6->branchcode, - holdingbranch => $library4->branchcode, - itype => 'test', - barcode => "item26barcode", - })->store; - - 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 } } ); - - 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, - }; - - sub _doTest { - my ( $cbranch, $biblio, $patron, $results ) = @_; - t::lib::Mocks::mock_preference('ReservesControlBranch', $cbranch); - - my @pl = $biblio->pickup_locations( { patron => $patron} ); - - 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)); - } - - foreach my $cbranch ('ItemHomeLibrary','PatronLibrary') { - foreach my $biblio ($biblio1, $biblio2) { - foreach my $patron ($patron1, $patron8) { - _doTest($cbranch, $biblio, $patron, $results); - } - } - } - - $schema->storage->txn_rollback; -}; \ No newline at end of file diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 58e13e99c2..459c59fc3a 100644 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 4; use C4::Biblio; @@ -143,3 +143,190 @@ subtest "as_marc_field() tests" => sub { $schema->storage->txn_rollback; }; + +subtest 'pickup_locations' => sub { + plan tests => 114; + + $schema->storage->txn_begin; + + my $dbh = C4::Context->dbh; + + # Cleanup database + Koha::Holds->search->delete; + Koha::Patrons->search->delete; + Koha::Items->search->delete; + Koha::Libraries->search->delete; + $dbh->do('DELETE FROM issues'); + $dbh->do('DELETE FROM issuingrules'); + $dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) + VALUES (?, ?, ?, ?)}, + {}, + '*', '*', '*', 25 + ); + $dbh->do('DELETE FROM circulation_rules'); + + my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); + my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); + + my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0 } } ); + my $library4 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); + + 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_1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library3->branchcode } } ); + my $group2_2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library4->branchcode } } ); + + my $biblioitem = $builder->build( { source => 'Biblioitem' } ); + + my $item1 = Koha::Item->new({ + biblionumber => $biblioitem->{biblionumber}, + biblioitemnumber => $biblioitem->{biblioitemnumber}, + homebranch => $library1->branchcode, + holdingbranch => $library2->branchcode, + barcode => '1', + itype => 'test', + })->store; + + my $item3 = Koha::Item->new({ + biblionumber => $biblioitem->{biblionumber}, + biblioitemnumber => $biblioitem->{biblioitemnumber}, + homebranch => $library3->branchcode, + holdingbranch => $library4->branchcode, + barcode => '3', + itype => 'test', + })->store; + + 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 = $item->pickup_locations( { patron => $patron} ); + my $ha_value=$ha==3?'holdgroup':($ha==2?'any':'homebranch'); + + foreach my $pickup_location (@pl) { + is( ref($pickup_location), 'Koha::Library', 'Object type is correct' ); + } + + 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) + ); + + } + + + 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); + } + } + } + } + + $schema->storage->txn_rollback; +}; diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index 50b86e12cf..94d9a45714 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 => 11; +use Test::More tests => 10; use Test::Exception; use C4::Circulation; @@ -177,159 +177,3 @@ $retrieved_item_1->delete; is( Koha::Items->search->count, $nb_of_items + 1, 'Delete should have deleted the item' ); $schema->storage->txn_rollback; - -subtest 'pickup_locations' => sub { - plan tests => 60; - - $schema->storage->txn_begin; - - # Cleanup database - Koha::Holds->search->delete; - Koha::Patrons->search->delete; - Koha::Items->search->delete; - Koha::Libraries->search->delete; - $dbh->do('DELETE FROM issues'); - $dbh->do('DELETE FROM issuingrules'); - $dbh->do( - q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) - VALUES (?, ?, ?, ?)}, - {}, - '*', '*', '*', 25 - ); - $dbh->do('DELETE FROM circulation_rules'); - - my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); - my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); - - my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); - my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); - my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0 } } ); - my $library4 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1 } } ); - - 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_1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library3->branchcode } } ); - my $group2_2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library4->branchcode } } ); - - my $biblioitem = $builder->build( { source => 'Biblioitem' } ); - - my $item1 = Koha::Item->new({ - biblionumber => $biblioitem->{biblionumber}, - biblioitemnumber => $biblioitem->{biblioitemnumber}, - homebranch => $library1->branchcode, - holdingbranch => $library2->branchcode, - barcode => '1', - itype => 'test', - })->store; - - my $item3 = Koha::Item->new({ - biblionumber => $biblioitem->{biblionumber}, - biblioitemnumber => $biblioitem->{biblioitemnumber}, - homebranch => $library3->branchcode, - holdingbranch => $library4->branchcode, - barcode => '3', - itype => 'test', - })->store; - - 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 = $item->pickup_locations( { patron => $patron} ); - my $ha_value=$ha==3?'holdgroup':($ha==2?'any':'homebranch'); - - 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)); - } - - - 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); - } - } - } - } - - $schema->storage->txn_rollback; -}; diff --git a/t/db_dependent/Koha/Libraries.t b/t/db_dependent/Koha/Libraries.t index 1f6b6dcf4c..45da1ae0ae 100644 --- a/t/db_dependent/Koha/Libraries.t +++ b/t/db_dependent/Koha/Libraries.t @@ -192,7 +192,7 @@ subtest 'pickup_locations' => sub { my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); my $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -201,7 +201,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -221,7 +221,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -230,7 +230,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item2, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -240,7 +240,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -249,7 +249,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -278,7 +278,7 @@ subtest 'pickup_locations' => sub { my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); my $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -287,7 +287,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -302,7 +302,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -319,7 +319,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -328,7 +328,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -355,7 +355,7 @@ subtest 'pickup_locations' => sub { my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); my $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -364,7 +364,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -384,7 +384,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -393,7 +393,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item3, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -403,7 +403,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } @@ -412,7 +412,7 @@ subtest 'pickup_locations' => sub { $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 }); $found = 0; foreach my $lib (@{$pickup}) { - if ($lib->{'branchcode'} eq $limit->toBranch) { + if ($lib->branchcode eq $limit->toBranch) { $found = 1; } } -- 2.24.1