Bug 24368: Remove Koha::Libraries->pickup_locations

This patch removes the unused method, and cleans the tests.
To test:
1. Verify Koha::Libraries->pickup_locations is not used in the code:
   $ git grep 'Koha::Libraries\->pickup_locations'
=> SUCCESS: no uses
2. Apply this patch
3. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Libraries.t
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/Koha/Libraries.pm b/Koha/Libraries.pm
index 27e4a23..8a45cc2 100644 (file)
--- a/Koha/Libraries.pm
+++ b/Koha/Libraries.pm
@@ -38,77 +38,7 @@ Koha::Libraries - Koha Library Object set class
 
 =head1 API
 
-=head2 Class Methods
-
-=cut
-
-=head3 pickup_locations
-
-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
-
-This method determines the pickup location by two factors:
-    1. is the library configured as pickup location
-    2. can a specific item / at least one of the items of a biblio be transferred
-       into the library
-
-OPTIONAL PARAMETERS:
-    item   # Koha::Item object / itemnumber, find pickup locations for item
-    biblio # Koha::Biblio object / biblionumber, find pickup locations for biblio
-
-If no parameters are given, all libraries with pickup_location => 1 are returned.
-
-=cut
-
-sub pickup_locations {
-    my ($self, $params) = @_;
-
-    my $item = $params->{'item'};
-    my $biblio = $params->{'biblio'};
-    if ($biblio && $item) {
-        Koha::Exceptions::BadParameter->throw(
-            error => "Koha::Libraries->pickup_locations takes either 'biblio' or "
-            ." 'item' as parameter, but not both."
-        );
-    }
-
-    # Select libraries that are configured as pickup locations
-    my $libraries = $self->search({
-        pickup_location => 1
-    }, {
-        order_by => ['branchname']
-    });
-
-    return $libraries->unblessed unless $item or $biblio;
-    return $libraries->unblessed
-        unless C4::Context->preference('UseBranchTransferLimits');
-    my $limittype = C4::Context->preference('BranchTransferLimitsType');
-
-    if ($item) {
-        unless (ref($item) eq 'Koha::Item') {
-            $item = Koha::Items->find($item);
-            return $libraries->unblessed unless $item;
-        }
-    } else {
-        unless (ref($biblio) eq 'Koha::Biblio') {
-            $biblio = Koha::Biblios->find($biblio);
-            return $libraries->unblessed unless $biblio;
-        }
-    }
-
-    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;
-}
+=head2 Class methods
 
 =head3 search_filtered
 

diff --git a/t/db_dependent/Koha/Libraries.t b/t/db_dependent/Koha/Libraries.t
index 5b44ea5..ff9f6a9 100644 (file)
--- a/t/db_dependent/Koha/Libraries.t
+++ b/t/db_dependent/Koha/Libraries.t
@@ -19,7 +19,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 10;
+use Test::More tests => 9;
 
 use C4::Biblio;
 use C4::Context;
@@ -88,339 +88,6 @@ is( $srstages->count, 3, 'Correctly fetched stockrotationstages associated with
 
 isa_ok( $srstages->next, 'Koha::StockRotationStage', "Relationship correctly creates Koha::Objects." );
 
-subtest 'pickup_locations' => sub {
-    plan tests => 2;
-
-    Koha::CirculationRules->set_rules(
-        {
-            branchcode => undef,
-            itemtype   => undef,
-            rules => {
-                holdallowed => 2,
-                hold_fulfillment_policy => 'any',
-                returnbranch => 'any'
-            }
-        }
-    );
-
-    my $from = Koha::Library->new({
-        branchcode => 'zzzfrom',
-        branchname => 'zzzfrom',
-        branchnotes => 'zzzfrom',
-    })->store;
-    my $to = Koha::Library->new({
-        branchcode => 'zzzto',
-        branchname => 'zzzto',
-        branchnotes => 'zzzto',
-    })->store;
-
-
-    my $biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
-    my $itemtype = $biblio->itemtype;
-    my $item_info = {
-        biblionumber => $biblio->biblionumber,
-        library      => $from->branchcode,
-        itype        => $itemtype
-    };
-    my $item1 = $builder->build_sample_item({%$item_info});
-    my $item2 = $builder->build_sample_item({%$item_info});
-    my $item3 = $builder->build_sample_item({%$item_info});
-    my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $from->branchcode } } );
-
-    subtest 'UseBranchTransferLimits = OFF' => sub {
-        plan tests => 5;
-
-        t::lib::Mocks::mock_preference('UseBranchTransferLimits', 0);
-        t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
-        t::lib::Mocks::mock_preference('item-level_itypes', 1);
-        Koha::Item::Transfer::Limits->delete;
-        Koha::Item::Transfer::Limit->new({
-            fromBranch => $from->branchcode,
-            toBranch => $to->branchcode,
-            itemtype => $biblio->itemtype,
-        })->store;
-        my $total_pickup = Koha::Libraries->search({
-            pickup_location => 1
-        })->count;
-        my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-        is(C4::Context->preference('UseBranchTransferLimits'), 0, 'Given system '
-           .'preference UseBranchTransferLimits is switched OFF,');
-        is(@{$pickup}, $total_pickup, 'Then the total number of pickup locations '
-           .'equal number of libraries with pickup_location => 1');
-
-        t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'itemtype');
-        t::lib::Mocks::mock_preference('item-level_itypes', 1);
-        $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1  });
-        is(@{$pickup}, $total_pickup, '...when '
-           .'BranchTransferLimitsType = itemtype and item-level_itypes = 1');
-        t::lib::Mocks::mock_preference('item-level_itypes', 0);
-        $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1  });
-        is(@{$pickup}, $total_pickup, '...as well as when '
-           .'BranchTransferLimitsType = itemtype and item-level_itypes = 0');
-        t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'ccode');
-        $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1  });
-        is(@{$pickup}, $total_pickup, '...as well as when '
-           .'BranchTransferLimitsType = ccode');
-        t::lib::Mocks::mock_preference('item-level_itypes', 1);
-    };
-
-    subtest 'UseBranchTransferLimits = ON' => sub {
-        plan tests => 4;
-        t::lib::Mocks::mock_preference('UseBranchTransferLimits', 1);
-
-        is(C4::Context->preference('UseBranchTransferLimits'), 1, 'Given system '
-           .'preference UseBranchTransferLimits is switched ON,');
-
-        subtest 'Given BranchTransferLimitsType = itemtype and '
-               .'item-level_itypes = ON' => sub {
-            plan tests => 11;
-
-            t::lib::Mocks::mock_preference('BranchTransferLimitsType','itemtype');
-            t::lib::Mocks::mock_preference('item-level_itypes', 1);
-            Koha::Item::Transfer::Limits->delete;
-            my $limit = Koha::Item::Transfer::Limit->new({
-                fromBranch => $from->branchcode,
-                toBranch => $to->branchcode,
-                itemtype => $item1->effective_itemtype,
-            })->store;
-            ok($item1->effective_itemtype eq $item2->effective_itemtype
-               && $item1->effective_itemtype eq $item3->effective_itemtype,
-               'Given all items of a biblio have same the itemtype,');
-            is($limit->itemtype, $item1->effective_itemtype, 'and given there '
-               .'is an existing transfer limit for that itemtype,');
-            my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            my $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 0, 'Then the to-library of which the limit applies for, '
-               .'is not included in the list of pickup libraries.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 0, 'The same applies when asking pickup locations of '
-               .'a single item.');
-            my $others = Koha::Libraries->search({
-                pickup_location => 1,
-                branchcode => { 'not in' => [$limit->toBranch] }})->count;
-            is(@{$pickup}, $others, 'However, the number of other pickup '
-               .'libraries is correct.');
-            $item2->itype('BK')->store;
-            ok($item1->effective_itemtype ne $item2->effective_itemtype,
-               'Given one of the item in this biblio has a different itemtype,');
-            is(Koha::Item::Transfer::Limits->search({
-                itemtype => $item2->effective_itemtype })->count, 0, 'and it is'
-               .' not restricted by transfer limits,');
-            $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1  });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'Then the to-library of which the limit applies for, '
-               .'is included in the list of pickup libraries.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item2, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'The same applies when asking pickup locations of '
-               .'a that particular item.');
-            Koha::Item::Transfer::Limits->delete;
-            $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'Given we deleted transfer limit, the previously '
-               .'transfer-limited library is included in the list.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'The same applies when asking pickup locations of '
-               .'a single item.');
-        };
-
-        subtest 'Given BranchTransferLimitsType = itemtype and '
-               .'item-level_itypes = OFF' => sub {
-            plan tests => 9;
-
-            t::lib::Mocks::mock_preference('BranchTransferLimitsType','itemtype');
-            t::lib::Mocks::mock_preference('item-level_itypes', 0);
-            $biblio->biblioitem->itemtype('BK')->store;
-            Koha::Item::Transfer::Limits->delete;
-            my $limit = Koha::Item::Transfer::Limit->new({
-                fromBranch => $from->branchcode,
-                toBranch => $to->branchcode,
-                itemtype => $item1->effective_itemtype,
-            })->store;
-
-            ok($item1->effective_itemtype eq 'BK',
-               'Given items use biblio-level itemtype,');
-            is($limit->itemtype, $item1->effective_itemtype, 'and given there '
-               .'is an existing transfer limit for that itemtype,');
-            my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            my $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 0, 'Then the to-library of which the limit applies for, '
-               .'is not included in the list of pickup libraries.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 0, 'The same applies when asking pickup locations of '
-               .'a single item.');
-            my $others = Koha::Libraries->search({
-                pickup_location => 1,
-                branchcode => { 'not in' => [$limit->toBranch] }})->count;
-            is(@{$pickup}, $others, 'However, the number of other pickup '
-               .'libraries is correct.');
-            Koha::Item::Transfer::Limits->delete;
-            $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'Given we deleted transfer limit, the previously '
-               .'transfer-limited library is included in the list.');
-            $limit = Koha::Item::Transfer::Limit->new({
-                fromBranch => $from->branchcode,
-                toBranch => $to->branchcode,
-                itemtype => $item1->itype,
-            })->store;
-            ok($item1->itype ne $item1->effective_itemtype
-               && $limit->itemtype eq $item1->itype, 'Given we have added a limit'
-               .' matching ITEM-level itemtype,');
-            $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'Then the limited branch is still included as a pickup'
-               .' library.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'The same applies when asking pickup locations of '
-               .'a single item.');
-        };
-
-        subtest 'Given BranchTransferLimitsType = ccode' => sub {
-            plan tests => 10;
-
-            t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'ccode');
-            $item1->ccode('hi')->store;
-            $item2->ccode('hi')->store;
-            $item3->ccode('hi')->store;
-            Koha::Item::Transfer::Limits->delete;
-            my $limit = Koha::Item::Transfer::Limit->new({
-                fromBranch => $from->branchcode,
-                toBranch => $to->branchcode,
-                ccode => $item1->ccode,
-            })->store;
-
-            is($limit->ccode, $item1->ccode, 'Given there '
-               .'is an existing transfer limit for that ccode,');
-            my $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            my $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 0, 'Then the to-library of which the limit applies for, '
-               .'is not included in the list of pickup libraries.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 0, 'The same applies when asking pickup locations of '
-               .'a single item.');
-            my $others = Koha::Libraries->search({
-                pickup_location => 1,
-                branchcode => { 'not in' => [$limit->toBranch] }})->count;
-            is(@{$pickup}, $others, 'However, the number of other pickup '
-               .'libraries is correct.');
-            $item3->ccode('yo')->store;
-            ok($item1->ccode ne $item3->ccode,
-               'Given one of the item in this biblio has a different ccode,');
-            is(Koha::Item::Transfer::Limits->search({
-                ccode => $item3->ccode })->count, 0, 'and it is'
-               .' not restricted by transfer limits,');
-            $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'Then the to-library of which the limit applies for, '
-               .'is included in the list of pickup libraries.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item3, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'The same applies when asking pickup locations of '
-               .'a that particular item.');
-            Koha::Item::Transfer::Limits->delete;
-            $pickup = Koha::Libraries->pickup_locations({ biblio => $biblio->biblionumber, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'Given we deleted transfer limit, the previously '
-               .'transfer-limited library is included in the list.');
-            $pickup = Koha::Libraries->pickup_locations({ item => $item1, patron => $patron1 });
-            $found = 0;
-            foreach my $lib (@{$pickup}) {
-                if ($lib->{branchcode} eq $limit->toBranch) {
-                    $found = 1;
-                }
-            }
-            is($found, 1, 'The same applies when asking pickup locations of '
-               .'a single item.');
-        };
-    };
-};
-
 $schema->storage->txn_rollback;
 
 subtest '->get_effective_marcorgcode' => sub {