Attachment 84302 Details for Bug 7614 – Bug 7614: Add a new method Koha::Libraries->pickup_locations

[patch] Bug 7614: Add a new method Koha::Libraries->pickup_locations

Bug-7614-Add-a-new-method-KohaLibraries-pickuploca.patch (text/plain), 19.78 KB, created by Kyle M Hall (khall) on 2019-01-22 19:07:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2019-01-22 19:07:31 UTC

Size: 19.78 KB

patch

obsolete

>From ab275adfcc427b77d7fa476fa7b660ec12ae24b2 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@jns.fi>
>Date: Thu, 9 Feb 2017 19:21:32 +0200
>Subject: [PATCH] Bug 7614: Add a new method Koha::Libraries->pickup_locations
>
>This patch adds a new method, Koha::Libraries->pickup_locations. This method
>takes either an item or a biblio as a parameter, and returns the list of
>acceptable pickup locations by considering libraries' configuration to act
>as a pickup location as well as any branch transfer limits that may apply.
>
>To test:
>1. prove t/db_dependent/Koha/Libraries.t
>
>Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> Koha/Libraries.pm               |  72 +++++++
> t/db_dependent/Koha/Libraries.t | 347 +++++++++++++++++++++++++++++++-
> 2 files changed, 418 insertions(+), 1 deletion(-)
>
>diff --git a/Koha/Libraries.pm b/Koha/Libraries.pm
>index 38a61a4325..71f6b44f05 100644
>--- a/Koha/Libraries.pm
>+++ b/Koha/Libraries.pm
>@@ -23,7 +23,10 @@ use Carp;
> 
> use C4::Context;
> 
>+use Koha::Biblios;
> use Koha::Database;
>+use Koha::Item::Transfer::Limits;
>+use Koha::Items;
> use Koha::Library;
> 
> use base qw(Koha::Objects);
>@@ -38,6 +41,75 @@ Koha::Libraries - Koha Library Object set class
> 
> =cut
> 
>+=head3 pickup_locations
>+
>+Returns available pickup locations 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');
>+
>+    my $items;
>+    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;
>+}
>+
> =head3 search_filtered
> 
> =cut
>diff --git a/t/db_dependent/Koha/Libraries.t b/t/db_dependent/Koha/Libraries.t
>index ef6a10b6be..5c356c1998 100644
>--- a/t/db_dependent/Koha/Libraries.t
>+++ b/t/db_dependent/Koha/Libraries.t
>@@ -19,8 +19,15 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 6;
>+use Test::More tests => 7;
> 
>+use C4::Biblio;
>+use C4::Context;
>+use C4::Items;
>+
>+use Koha::Biblios;
>+use Koha::Item::Transfer::Limits;
>+use Koha::Items;
> use Koha::Library;
> use Koha::Libraries;
> use Koha::Database;
>@@ -76,6 +83,344 @@ 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;
>+
>+    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 ($bibnum, $title, $bibitemnum) = create_helper_biblio('DUMMY');
>+    # Create item instance for testing.
>+    my ($item_bibnum1, $item_bibitemnum1, $itemnumber1)
>+    = AddItem({ homebranch => $from->branchcode,
>+                holdingbranch => $from->branchcode } , $bibnum);
>+    my ($item_bibnum2, $item_bibitemnum2, $itemnumber2)
>+    = AddItem({ homebranch => $from->branchcode,
>+                holdingbranch => $from->branchcode } , $bibnum);
>+    my ($item_bibnum3, $item_bibitemnum3, $itemnumber3)
>+    = AddItem({ homebranch => $from->branchcode,
>+                holdingbranch => $from->branchcode } , $bibnum);
>+    my $item1 = Koha::Items->find($itemnumber1);
>+    my $item2 = Koha::Items->find($itemnumber2);
>+    my $item3 = Koha::Items->find($itemnumber3);
>+    my $biblio = Koha::Biblios->find($bibnum);
>+    my $itemtype = $biblio->itemtype;
>+
>+    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 => $bibnum });
>+        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 => $bibnum });
>+        is(@{$pickup}, $total_pickup, '...when '
>+           .'BranchTransferLimitsType = itemtype and item-level_itypes = 1');
>+        t::lib::Mocks::mock_preference('item-level_itypes', 0);
>+        $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum });
>+        is(@{$pickup}, $total_pickup, '...as well as when '
>+           .'BranchTransferLimitsType = itemtype and item-level_itypes = 0');
>+        t::lib::Mocks::mock_preference('BranchTransferLimitsType', 'ccode');
>+        $pickup = Koha::Libraries->pickup_locations({ biblio => $bibnum });
>+        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 => $bibnum });
>+            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 });
>+            $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 => $bibnum });
>+            $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 });
>+            $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 => $bibnum });
>+            $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 });
>+            $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 => $bibnum });
>+            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 });
>+            $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 => $bibnum });
>+            $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 => $bibnum });
>+            $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 });
>+            $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 => $bibnum });
>+            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 });
>+            $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 => $bibnum });
>+            $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 });
>+            $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 => $bibnum });
>+            $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 });
>+            $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.');
>+        };
>+    };
>+};
>+
>+sub create_helper_biblio {
>+    my $itemtype = shift;
>+    my ($bibnum, $title, $bibitemnum);
>+    my $bib = MARC::Record->new();
>+    $title = 'Silence in the library';
>+    $bib->append_fields(
>+        MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'),
>+        MARC::Field->new('245', ' ', ' ', a => $title),
>+        MARC::Field->new('942', ' ', ' ', c => $itemtype),
>+    );
>+    return ($bibnum, $title, $bibitemnum) = AddBiblio($bib, '');
>+}
>+
> $schema->storage->txn_rollback;
> 
> subtest '->get_effective_marcorgcode' => sub {
>-- 
>2.17.2 (Apple Git-113)

Actions: View | Diff | Splinter Review

Attachments on bug 7614: 60141 | 60142 | 60143 | 60144 | 60145 | 60785 | 60786 | 60787 | 60938 | 60946 | 70047 | 77895 | 77896 | 77897 | 77898 | 77899 | 77900 | 78020 | 84295 | 84296 | 84297 | 84298 | 84299 | 84300 | 84301 | 84302 | 84303 | 84304 | 84305 | 84306 | 84307 | 84308 | 84309 | 84310 | 84311