From f414eda1846a0ac8608a688c81f566d86a1ab3f2 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 20 Nov 2020 10:31:33 +0100 Subject: [PATCH] Bug 27062: Fix pickup_location tests when existing libraries exist If there are existing libraries in the DB, the tests will fail. There were several changes related to those tests recently, this bug report is a follow-up for them. bug 26984, bug 27002 and bug 26963. --- t/db_dependent/Koha/Biblio.t | 17 +++------ t/db_dependent/Koha/Item.t | 73 ++++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 39 deletions(-) diff --git a/t/db_dependent/Koha/Biblio.t b/t/db_dependent/Koha/Biblio.t index 485b31a14e..718b69ebf5 100755 --- a/t/db_dependent/Koha/Biblio.t +++ b/t/db_dependent/Koha/Biblio.t @@ -189,7 +189,6 @@ subtest 'pickup_locations' => sub { $schema->storage->txn_begin; - my $nb_libraries = Koha::Libraries->count; Koha::CirculationRules->search->delete; Koha::CirculationRules->set_rules( { @@ -368,11 +367,11 @@ subtest 'pickup_locations' => sub { my $patron8 = $builder->build_object( { class => 'Koha::Patrons', value => { firstname=>'8', branchcode => $library8->branchcode } } ); my $results = { - "ItemHomeLibrary-1-1" => 6 + $nb_libraries, + "ItemHomeLibrary-1-1" => 6, "ItemHomeLibrary-1-8" => 1, "ItemHomeLibrary-2-1" => 2, "ItemHomeLibrary-2-8" => 0, - "PatronLibrary-1-1" => 6 + $nb_libraries, + "PatronLibrary-1-1" => 6, "PatronLibrary-1-8" => 3, "PatronLibrary-2-1" => 0, "PatronLibrary-2-8" => 3, @@ -382,14 +381,10 @@ subtest 'pickup_locations' => sub { my ( $cbranch, $biblio, $patron, $results ) = @_; t::lib::Mocks::mock_preference('ReservesControlBranch', $cbranch); - my @pl = $biblio->pickup_locations( { patron => $patron } ); - - foreach my $pickup_location (@pl) { - next - unless grep { $pickup_location eq $_ } @branchcodes; - - is( ref($pickup_location), 'Koha::Library', 'Object type is correct' ); - } + my @pl = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $biblio->pickup_locations( { patron => $patron } )->as_list; ok( scalar(@pl) == $results->{ $cbranch . '-' diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 10c5f1dff3..f2713b4e26 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -212,20 +212,18 @@ subtest 'pickup_locations' => sub { 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 $all_count = Koha::Libraries->search({ pickup_location => 1})->count(); - my $results = { - "1-1-1-any" => $all_count, + "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" => $all_count, + "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" => $all_count, + "1-1-3-any" => 3, "1-1-3-holdgroup" => 2, "1-1-3-patrongroup" => 2, "1-1-3-homebranch" => 1, @@ -235,7 +233,7 @@ subtest 'pickup_locations' => sub { "1-4-1-patrongroup" => 0, "1-4-1-homebranch" => 0, "1-4-1-holdingbranch" => 0, - "1-4-2-any" => $all_count, + "1-4-2-any" => 3, "1-4-2-holdgroup" => 2, "1-4-2-patrongroup" => 1, "1-4-2-homebranch" => 1, @@ -250,7 +248,7 @@ subtest 'pickup_locations' => sub { "3-1-1-patrongroup" => 0, "3-1-1-homebranch" => 0, "3-1-1-holdingbranch" => 0, - "3-1-2-any" => $all_count, + "3-1-2-any" => 3, "3-1-2-holdgroup" => 1, "3-1-2-patrongroup" => 2, "3-1-2-homebranch" => 0, @@ -265,12 +263,12 @@ subtest 'pickup_locations' => sub { "3-4-1-patrongroup" => 0, "3-4-1-homebranch" => 0, "3-4-1-holdingbranch" => 0, - "3-4-2-any" => $all_count, + "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" => $all_count, + "3-4-3-any" => 3, "3-4-3-holdgroup" => 1, "3-4-3-patrongroup" => 1, "3-4-3-homebranch" => 0, @@ -291,14 +289,13 @@ subtest 'pickup_locations' => sub { } } ); - my @pl = $item->pickup_locations( { patron => $patron} )->as_list; my $ha_value=$ha==3?'holdgroup':($ha==2?'any':'homebranch'); - foreach my $pickup_location (@pl) { - next - unless grep { $pickup_location eq $_ } @branchcodes; - is( ref($pickup_location), 'Koha::Library', 'Object type is correct' ); - } + my @pl = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item->pickup_locations( { patron => $patron } )->as_list; + ok( scalar(@pl) == $results->{ $item->copynumber . '-' @@ -374,8 +371,12 @@ subtest 'pickup_locations' => sub { } ); - my $pickup_locations = $item1->pickup_locations( { patron => $patron1 } )->as_list; - is( scalar @$pickup_locations, $all_count - 1, "With a transfer limits we get back the libraries that are pickup locations minus 1 limited library"); + my @pickup_locations = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item1->pickup_locations( { patron => $patron1 } )->as_list; + + is( scalar @pickup_locations, 3 - 1, "With a transfer limits we get back the libraries that are pickup locations minus 1 limited library"); $builder->build_object( { @@ -389,15 +390,25 @@ subtest 'pickup_locations' => sub { } ); - $pickup_locations = $item1->pickup_locations( { patron => $patron1 } )->as_list; - is( scalar @$pickup_locations, $all_count - 2, "With 2 transfer limits we get back the libraries that are pickup locations minus 2 limited libraries"); + @pickup_locations = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item1->pickup_locations( { patron => $patron1 } )->as_list; - t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'ccode'); - $pickup_locations = $item1->pickup_locations( { patron => $patron1 } )->as_list; - is( scalar @$pickup_locations, $all_count, "With no transfer limits of type ccode we get back the libraries that are pickup locations"); + is( scalar @pickup_locations, 3 - 2, "With 2 transfer limits we get back the libraries that are pickup locations minus 2 limited libraries"); - $pickup_locations = $item_no_ccode->pickup_locations( { patron => $patron1 } )->as_list; - is( scalar @$pickup_locations, $all_count, "With no transfer limits of type ccode and an item with no ccode we get back the libraries that are pickup locations"); + t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'ccode'); + @pickup_locations = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item1->pickup_locations( { patron => $patron1 } )->as_list; + is( scalar @pickup_locations, 3, "With no transfer limits of type ccode we get back the libraries that are pickup locations"); + + @pickup_locations = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item_no_ccode->pickup_locations( { patron => $patron1 } )->as_list; + is( scalar @pickup_locations, 3, "With no transfer limits of type ccode and an item with no ccode we get back the libraries that are pickup locations"); $builder->build_object( { @@ -411,8 +422,11 @@ subtest 'pickup_locations' => sub { } ); - $pickup_locations = $item1->pickup_locations( { patron => $patron1 } )->as_list; - is( scalar @$pickup_locations, $all_count - 1, "With a transfer limits we get back the libraries that are pickup locations minus 1 limited library"); + @pickup_locations = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item1->pickup_locations( { patron => $patron1 } )->as_list; + is( scalar @pickup_locations, 3 - 1, "With a transfer limits we get back the libraries that are pickup locations minus 1 limited library"); $builder->build_object( { @@ -426,8 +440,11 @@ subtest 'pickup_locations' => sub { } ); - $pickup_locations = $item1->pickup_locations( { patron => $patron1 } )->as_list; - is( scalar @$pickup_locations, $all_count - 2, "With 2 transfer limits we get back the libraries that are pickup locations minus 2 limited libraries"); + @pickup_locations = map { + my $pickup_location = $_; + grep { $pickup_location->branchcode eq $_ } @branchcodes + } $item1->pickup_locations( { patron => $patron1 } )->as_list; + is( scalar @pickup_locations, 3 - 2, "With 2 transfer limits we get back the libraries that are pickup locations minus 2 limited libraries"); t::lib::Mocks::mock_preference('UseBranchTransferLimits', 0); -- 2.20.1