Attachment 89842 Details for Bug 22284 – Bug 22284: Opac pickup_locations

[patch] Bug 22284: Opac pickup_locations

Bug-22284-Opac-pickuplocations.patch (text/plain), 45.05 KB, created by Agustín Moyano on 2019-05-16 14:18:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Agustín Moyano

Created: 2019-05-16 14:18:58 UTC

Size: 45.05 KB

patch

obsolete

>From 1f6318daf90a7eaee999bc158f3b549aa00f59e6 Mon Sep 17 00:00:00 2001
>From: Agustin Moyano <agustinmoyano@theke.io>
>Date: Fri, 12 Apr 2019 10:32:48 +0000
>Subject: [PATCH] Bug 22284: Opac pickup_locations
>
>This patch modifies Koha::Libraries->pickup_location and moves most of the logic to
>Koha::Item and Koha::Biblio in preparation for api endpoints in the future.
>
>There where 2 methods added
>
>1) Koha::Item->pickup_locations that given a patron, returns all pickup locations of
>this item, considering hold fulfillment rules, and hold allowed rules.
>
>2) Koha::Biblio->pickup_locations that given a patron, returns a distinct list of
>libraries returned by each of this biblio items pickup location.
>
>Koha::Libraries->pickup_location analyzes input param and calls Koha::Item->pickup_locations
>or Koha::Biblio->pickup_locations as needed.
>
>Also in opac-reserve.tt the way options where obtained to fill the pickup location select
>was modified to pass the patron as a parameter.
>
>To test:
>1) opac: try to place hold on a item and check that all libraries are shown in the
>pickup location select.
>2) intranet: in Library groups, add 2 root groups marked as local hold group and
>add different libraries to each.
>3) opac: login as a user of a library belonging to one hold group, and search try to
>place a hold on an item belongin to the other hold group.
>4) intranet: in Circulation and fines rules, play with 'Hold policy' and 'Hold pickup
>library match' rules.
>5) opac: On each modification of the rules reload the page.
>SUCCESS => Every time you reload the page, the number of pickup locations showed in
>select varies.
>6) prove t/db_dependent/Koha/Biblios.t t/db_dependent/Koha/Items.t
>SUCCESS => Result: PASS
>7) Sign off
>
>Sponsored-by: VOKAL
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Biblio.pm                                     |  27 ++
> Koha/Item.pm                                       |  56 ++++
> Koha/Libraries.pm                                  |  24 +-
> .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt |   4 +-
> t/db_dependent/Koha/Biblios.t                      | 344 ++++++++++++++++++++-
> t/db_dependent/Koha/Items.t                        | 315 ++++++++++++++++++-
> 6 files changed, 750 insertions(+), 20 deletions(-)
>
>diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm
>index fae91cef33..ff5aac8fa1 100644
>--- a/Koha/Biblio.pm
>+++ b/Koha/Biblio.pm
>@@ -188,6 +188,33 @@ sub can_be_transferred {
>     return 0;
> }
> 
>+
>+=head3 pickup_locations
>+
>+@pickup_locations = $biblio->pickup_locations( {patron => $patron } )
>+
>+Returns possible pickup locations for this biblio items, according to patron's home library (if patron is defined and holds are allowed only from hold groups)
>+and if item can be transfered to each pickup location.
>+
>+=cut
>+
>+sub pickup_locations {
>+    my ($self, $params) = @_;
>+
>+    my $patron = $params->{patron};
>+
>+    my @pickup_locations;
>+    foreach my $item_of_bib ($self->items) {
>+        push @pickup_locations, $item_of_bib->pickup_locations( {patron => $patron} );
>+    }
>+
>+    my %seen;
>+    @pickup_locations =
>+      grep { !$seen{ $_->{branchcode} }++ } @pickup_locations;
>+
>+    return wantarray ? @pickup_locations : \@pickup_locations;
>+}
>+
> =head3 hidden_in_opac
> 
> my $bool = $biblio->hidden_in_opac({ [ rules => $rules ] })
>diff --git a/Koha/Item.pm b/Koha/Item.pm
>index 4bd3ba2d1c..e108c7650e 100644
>--- a/Koha/Item.pm
>+++ b/Koha/Item.pm
>@@ -26,6 +26,7 @@ use Koha::Database;
> use Koha::DateUtils qw( dt_from_string );
> 
> use C4::Context;
>+use C4::Circulation;
> use Koha::Checkouts;
> use Koha::IssuingRules;
> use Koha::Item::Transfer::Limits;
>@@ -37,6 +38,8 @@ use Koha::StockRotationRotas;
> 
> use base qw(Koha::Object);
> 
>+use Data::Dumper;
>+
> =head1 NAME
> 
> Koha::Item - Koha Item object class
>@@ -277,6 +280,59 @@ sub can_be_transferred {
>     })->count ? 0 : 1;
> }
> 
>+=head3 pickup_locations
>+
>+@pickup_locations = $item->pickup_locations( {patron => $patron } )
>+
>+Returns possible pickup locations for this item, according to patron's home library (if patron is defined and holds are allowed only from hold groups)
>+and if item can be transfered to each pickup location.
>+
>+=cut
>+
>+sub pickup_locations {
>+    my ($self, $params) = @_;
>+
>+    my $patron = $params->{patron};
>+
>+    my $circ_control_branch =
>+      C4::Circulation::_GetCircControlBranch( $self->unblessed(), $patron );
>+    my $branchitemrule =
>+      C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype );
>+
>+    my $branch_control = C4::Context->preference('HomeOrHoldingBranch');
>+    my $library = $branch_control eq 'holdingbranch' ? $self->holding_branch : $self->home_branch;
>+
>+    #warn $branch_control.' '.$branchitemrule->{holdallowed}.' '.$library->branchcode.' '.$patron->branchcode;
>+
>+    my @libs;
>+    if(defined $patron) {
>+        return @libs if $branchitemrule->{holdallowed} == 3 && !$library->validate_hold_sibling( {branchcode => $patron->branchcode} );
>+        return @libs if $branchitemrule->{holdallowed} == 1 && $library->branchcode ne $patron->branchcode;
>+    }
>+
>+    if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup') {
>+        @libs  = $library->get_hold_libraries;
>+    } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'homebranch') {
>+        push @libs, $self->home_branch;
>+    } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'holdingbranch') {
>+        push @libs, $self->holding_branch;
>+    } else {
>+        @libs = Koha::Libraries->search({
>+            pickup_location => 1
>+        }, {
>+            order_by => ['branchname']
>+        })->as_list;
>+    }
>+
>+    my @pickup_locations;
>+    foreach my $library (@libs) {
>+        if ($library->pickup_location && $self->can_be_transferred({ to => $library })) {
>+            push @pickup_locations, $library->unblessed;
>+        }
>+    }
>+    return wantarray ? @pickup_locations : \@pickup_locations;
>+}
>+
> =head3 article_request_type
> 
> my $type = $item->article_request_type( $borrower )
>diff --git a/Koha/Libraries.pm b/Koha/Libraries.pm
>index 56eaf4feab..bc92f0bb10 100644
>--- a/Koha/Libraries.pm
>+++ b/Koha/Libraries.pm
>@@ -66,12 +66,17 @@ sub pickup_locations {
> 
>     my $item = $params->{'item'};
>     my $biblio = $params->{'biblio'};
>+    my $patron = $params->{'patron'};
>+
>     if ($biblio && $item) {
>         Koha::Exceptions::BadParameter->throw(
>             error => "Koha::Libraries->pickup_locations takes either 'biblio' or "
>             ." 'item' as parameter, but not both."
>         );
>     }
>+    unless (! defined $patron || ref($patron) eq 'Koha::Patron') {
>+        $patron = Koha::Items->find($patron);
>+    }
> 
>     # Select libraries that are configured as pickup locations
>     my $libraries = $self->search({
>@@ -81,32 +86,19 @@ sub pickup_locations {
>     });
> 
>     return $libraries->unblessed unless $item or $biblio;
>-    return $libraries->unblessed
>-        unless C4::Context->preference('UseBranchTransferLimits');
>-    my $limittype = C4::Context->preference('BranchTransferLimitsType');
>-
>-    if ($item) {
>+    if($item) {
>         unless (ref($item) eq 'Koha::Item') {
>             $item = Koha::Items->find($item);
>             return $libraries->unblessed 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 $biblio->pickup_locations( {patron => $patron} );
>     }
>-
>-    my @pickup_locations;
>-    foreach my $library ($libraries->as_list) {
>-        if ($item && $item->can_be_transferred({ to => $library })) {
>-            push @pickup_locations, $library->unblessed;
>-        } elsif ($biblio && $biblio->can_be_transferred({ to => $library })) {
>-            push @pickup_locations, $library->unblessed;
>-        }
>-    }
>-
>-    return wantarray ? @pickup_locations : \@pickup_locations;
> }
> 
> =head3 search_filtered
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>index b26fbaf99d..00f125eb9b 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>@@ -231,12 +231,12 @@
>                                                                 <label for="branch_[% bibitemloo.biblionumber | html %]">Pick up location:</label>
>                                                                 [% UNLESS ( bibitemloo.holdable ) %]
>                                                                     <select name="branch" id="branch_[% bibitemloo.biblionumber | html %]" disabled="disabled">
>-                                                                        [% PROCESS options_for_libraries libraries => Branches.pickup_locations({ search_params => { biblio => bibitemloo.biblionumber }, selected => branch }) %]
>+                                                                        [% PROCESS options_for_libraries libraries => Branches.pickup_locations({ search_params => { biblio => bibitemloo.biblionumber, patron => logged_in_user }, selected => branch }) %]
>                                                                     </select>
>                                                                 [% ELSE %]
>                                                                     [% SET at_least_one_library_not_available_for_pickup = 0 %]
>                                                                     <select name="branch" id="branch_[% bibitemloo.biblionumber | html %]">
>-                                                                        [% FOREACH library IN Branches.all({ search_params => { pickup_location => 1 }, selected => branch }) %]
>+                                                                        [% FOREACH library IN Branches.pickup_locations({ search_params => { biblio => bibitemloo.biblionumber, patron => logged_in_user }, selected => branch }) %]
>                                                                             [% SET pickup_available_at = bibitemloo.not_available_at.grep(library.branchcode).size ? 0 : 1 %]
>                                                                             [% IF library.selected AND pickup_available_at %]
>                                                                                 <option value="[% library.branchcode | html %]" selected="selected" >[% library.branchname | html %]</option>
>diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t
>index a3892c5468..9d3bca16bd 100644
>--- a/t/db_dependent/Koha/Biblios.t
>+++ b/t/db_dependent/Koha/Biblios.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 5;
>+use Test::More tests => 6;
> use Test::Exception;
> 
> use C4::Items;
>@@ -36,6 +36,8 @@ use t::lib::Mocks;
> my $schema = Koha::Database->new->schema;
> $schema->storage->txn_begin;
> 
>+my $dbh     = C4::Context->dbh;
>+
> my $builder = t::lib::TestBuilder->new;
> my $patron = $builder->build( { source => 'Borrower' } );
> $patron = Koha::Patrons->find( $patron->{borrowernumber} );
>@@ -187,3 +189,343 @@ subtest 'can_be_transferred' => sub {
> };
> 
> $schema->storage->txn_rollback;
>+
>+
>+subtest 'pickup_locations' => sub {
>+    plan tests => 25;
>+
>+    $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 branch_item_rules');
>+    $dbh->do('DELETE FROM default_branch_circ_rules');
>+    $dbh->do('DELETE FROM default_branch_item_rules');
>+    $dbh->do('DELETE FROM default_circ_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 $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
>+    $dbh->do(
>+        q{INSERT INTO default_circ_rules (holdallowed, hold_fulfillment_policy, returnbranch)
>+        VALUES (?,?,?)},
>+        {},
>+        2, 'any', '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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, '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, '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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, '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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        2, 'holdgroup'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdgroup'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, 'holdgroup'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        2, 'homebranch'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'homebranch'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, 'homebranch'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        2, 'holdingbranch'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdingbranch'
>+    );
>+
>+    @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, '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');
>+
>+    #Case 12: holdallowed holdgroup, hold_fulfillment_policy holdingbranch
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, 'holdingbranch'
>+    );
>+
>+    @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 && 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');
>+
>+    t::lib::Mocks::mock_preference('HomeOrHoldingBranch', 'holdingbranch');
>+
>+    #Case 13: holdallowed homebranch, hold_fulfillment_policy any, HomeOrHoldingBranch 'holdingbranch'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, '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_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');
>+
>+    #Case 14: holdallowed homebranch, hold_fulfillment_policy holdgroup, HomeOrHoldingBranch 'holdingbranch'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdgroup'
>+    );
>+
>+    @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) == 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');
>+
>+    #Case 15: holdallowed homebranch, hold_fulfillment_policy homebranch, HomeOrHoldingBranch 'holdingbranch'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'homebranch'
>+    );
>+
>+    @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_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');
>+
>+    #Case 16: holdallowed homebranch, hold_fulfillment_policy holdingbranch, HomeOrHoldingBranch 'holdingbranch'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdingbranch'
>+    );
>+
>+    @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 98e51b3fec..e0e5faeedc 100644
>--- a/t/db_dependent/Koha/Items.t
>+++ b/t/db_dependent/Koha/Items.t
>@@ -19,10 +19,11 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 9;
>+use Test::More tests => 10;
> use Test::Exception;
> 
> use C4::Circulation;
>+use C4::Context;
> use Koha::Item;
> use Koha::Item::Transfer::Limits;
> use Koha::Items;
>@@ -34,6 +35,8 @@ use t::lib::Mocks;
> my $schema = Koha::Database->new->schema;
> $schema->storage->txn_begin;
> 
>+my $dbh     = C4::Context->dbh;
>+
> my $builder     = t::lib::TestBuilder->new;
> my $biblioitem  = $builder->build( { source => 'Biblioitem' } );
> my $library     = $builder->build( { source => 'Branch' } );
>@@ -165,3 +168,313 @@ 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;
>+
>+    $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 branch_item_rules');
>+    $dbh->do('DELETE FROM default_branch_circ_rules');
>+    $dbh->do('DELETE FROM default_branch_item_rules');
>+    $dbh->do('DELETE FROM default_circ_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,
>+        itype            => 'test',
>+        barcode          => "item1barcode",
>+    })->store;
>+
>+    my $item3  = Koha::Item->new({
>+        biblionumber     => $biblioitem->{biblionumber},
>+        biblioitemnumber => $biblioitem->{biblioitemnumber},
>+        homebranch       => $library3->branchcode,
>+        holdingbranch    => $library4->branchcode,
>+        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
>+    $dbh->do(
>+        q{INSERT INTO default_circ_rules (holdallowed, hold_fulfillment_policy, returnbranch)
>+        VALUES (?,?,?)},
>+        {},
>+        2, 'any', '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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, '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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, '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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        2, 'holdgroup'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdgroup'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, 'holdgroup'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        2, '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) == 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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, '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) == 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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, '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) == 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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        2, 'holdingbranch'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdingbranch'
>+    );
>+
>+    @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
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        3, 'holdingbranch'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, '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) == 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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdgroup'
>+    );
>+
>+    @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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, '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_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'
>+    $dbh->do(
>+        q{UPDATE default_circ_rules set holdallowed = ?, hold_fulfillment_policy = ?},
>+        {},
>+        1, 'holdingbranch'
>+    );
>+
>+    @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;
>+};
>-- 
>2.11.0

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