Attachment 92570 Details for Bug 22284 – Bug 22284: (follow-up) Fix tests

[patch] Bug 22284: (follow-up) Fix tests

Bug-22284-follow-up-Fix-tests.patch (text/plain), 58.73 KB, created by Agustín Moyano on 2019-09-03 14:46:20 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Agustín Moyano

Created: 2019-09-03 14:46:20 UTC

Size: 58.73 KB

patch

obsolete

>From ed5c2669271fc86c99415db93f842e114d9fd674 Mon Sep 17 00:00:00 2001
>From: Agustin Moyano <agustinmoyano@theke.io>
>Date: Wed, 29 May 2019 02:07:07 -0300
>Subject: [PATCH] Bug 22284: (follow-up) Fix tests
>
>Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
>---
> t/db_dependent/Koha/Biblios.t   | 407 +++++++---------------------
> t/db_dependent/Koha/Items.t     | 453 +++++++-------------------------
> t/db_dependent/Koha/Libraries.t |  57 ++--
> 3 files changed, 232 insertions(+), 685 deletions(-)
>
>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..188eebbbf6 100644
>--- a/t/db_dependent/Koha/Items.t
>+++ b/t/db_dependent/Koha/Items.t
>@@ -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 cfa7382c29..69f8b85028 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',
>@@ -117,6 +131,7 @@ subtest 'pickup_locations' => sub {
>     my $item3 = Koha::Items->find($itemnumber3);
>     my $biblio = Koha::Biblios->find($bibnum);
>     my $itemtype = $biblio->itemtype;
>+    my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $from->branchcode } } );
> 
>     subtest 'UseBranchTransferLimits = OFF' => sub {
>         plan tests => 5;
>@@ -133,7 +148,7 @@ subtest 'pickup_locations' => sub {
>         my $total_pickup = Koha::Libraries->search({
>             pickup_location => 1
>         })->count;
>-        my $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum });
>+        my $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, 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 '
>@@ -141,15 +156,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 => $bibnum });
>+        $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, 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 => $bibnum });
>+        $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, 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 => $bibnum });
>+        $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>         is(@{$pickup}, $total_pickup, '...as well as when '
>            .'BranchTransferLimitsType = ccode');
>         t::lib::Mocks::mock_preference('item-level_itypes', 1);
>@@ -179,7 +194,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 => $bibnum });
>+            my $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             my $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -188,7 +203,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) {
>@@ -208,7 +223,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 => $bibnum });
>+            $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -217,7 +232,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) {
>@@ -227,7 +242,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 => $bibnum });
>+            $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -236,7 +251,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) {
>@@ -265,7 +280,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 => $bibnum });
>+            my $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             my $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -274,7 +289,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) {
>@@ -289,7 +304,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 => $bibnum });
>+            $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -306,7 +321,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 => $bibnum });
>+            $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -315,7 +330,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) {
>@@ -342,7 +357,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 => $bibnum });
>+            my $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             my $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -351,7 +366,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) {
>@@ -371,7 +386,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 => $bibnum });
>+            $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -380,7 +395,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) {
>@@ -390,7 +405,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 => $bibnum });
>+            $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum, patron => $patron1 });
>             $found = 0;
>             foreach my $lib (@{$pickup}) {
>                 if ($lib->{'branchcode'} eq $limit->toBranch) {
>@@ -399,7 +414,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

Actions: View | Diff | Splinter Review

Attachments on bug 22284: 88138 | 88139 | 88140 | 88141 | 88142 | 88143 | 88144 | 88145 | 88369 | 88561 | 88793 | 88794 | 88795 | 88796 | 88797 | 88798 | 88799 | 88800 | 88801 | 88802 | 88939 | 88940 | 88941 | 88942 | 88943 | 88944 | 88945 | 88946 | 88947 | 88948 | 88949 | 88955 | 88956 | 88957 | 88958 | 88959 | 88960 | 88961 | 88962 | 88963 | 88964 | 88965 | 89060 | 89061 | 89062 | 89063 | 89064 | 89065 | 89066 | 89067 | 89068 | 89069 | 89070 | 89071 | 89072 | 89073 | 89074 | 89075 | 89076 | 89077 | 89078 | 89079 | 89080 | 89081 | 89082 | 89083 | 89084 | 89085 | 89122 | 89123 | 89124 | 89125 | 89126 | 89127 | 89128 | 89129 | 89130 | 89131 | 89132 | 89133 | 89134 | 89135 | 89523 | 89836 | 89837 | 89838 | 89839 | 89840 | 89841 | 89842 | 89843 | 89844 | 89845 | 89846 | 89847 | 89848 | 89849 | 90133 | 90134 | 90197 | 90198 | 90199 | 90200 | 90201 | 90469 | 90525 | 91261 | 91262 | 91263 | 91264 | 91265 | 91266 | 91267 | 91268 | 91269 | 91270 | 91271 | 91272 | 91273 | 91274 | 91275 | 91276 | 91277 | 91278 | 91279 | 91280 | 91282 | 91701 | 91702 | 91703 | 91704 | 91705 | 91706 | 91707 | 91708 | 91709 | 91710 | 91711 | 91712 | 91713 | 91714 | 91715 | 91716 | 91717 | 91718 | 91719 | 91720 | 91721 | 91750 | 91751 | 91752 | 91753 | 91754 | 91755 | 91756 | 91757 | 91758 | 92554 | 92555 | 92556 | 92557 | 92558 | 92559 | 92560 | 92561 | 92562 | 92563 | 92564 | 92565 | 92566 | 92567 | 92568 | 92569 | 92570 | 92571 | 92572 | 92573 | 92574 | 92752 | 92753 | 92754 | 92755 | 92756 | 92757 | 92758 | 92759 | 92760 | 92761 | 92762 | 92763 | 92764 | 92849 | 92850 | 92851 | 92852 | 92853 | 92854 | 92855 | 92856 | 92857 | 92858 | 92859 | 92860 | 92861 | 94483 | 94484 | 94485 | 94486 | 94487 | 94488 | 94489 | 94490 | 94491 | 94492 | 94493 | 94494 | 94495 | 96437 | 96438 | 96439 | 96440 | 96441 | 96442 | 96443 | 96444 | 96445 | 96446 | 96447 | 96448 | 96449 | 96450 | 96544 | 96545 | 96546 | 96547 | 96548 | 96549 | 96550 | 96551 | 96552 | 96553 | 96554 | 96555 | 96556 | 96557 | 96558 | 115202