Bug 22284: (QA follow-up) Make pickup locations be Koha::Library objects
authorTomas Cohen Arazi <tomascohen@theke.io>
Sat, 21 Dec 2019 03:14:10 +0000 (00:14 -0300)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Fri, 3 Jan 2020 12:58:06 +0000 (12:58 +0000)
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 <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Koha/Biblio.pm
Koha/Item.pm
Koha/Libraries.pm
Koha/Template/Plugin/Branches.pm
reserve/request.pl
t/db_dependent/Koha/Biblio.t
t/db_dependent/Koha/Biblios.t
t/db_dependent/Koha/Item.t
t/db_dependent/Koha/Items.t
t/db_dependent/Koha/Libraries.t

index e143bfb..5b63653 100644 (file)
@@ -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;
 }
index 1d18d34..e77f819 100644 (file)
@@ -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;
         }
     }
 
index ebd552b..c3464ee 100644 (file)
@@ -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} );
     }
index da615f5..bdb1970 100644 (file)
@@ -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;
index 225f039..c7376c2 100755 (executable)
@@ -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} );
index bdf65bd..06a5b40 100644 (file)
@@ -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;
+};
index 6afd72d..d44d566 100644 (file)
@@ -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
index 58e13e9..459c59f 100644 (file)
@@ -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;
+};
index 50b86e1..94d9a45 100644 (file)
@@ -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;
-};
index 1f6b6dc..45da1ae 100644 (file)
@@ -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;
                 }
             }