From 71495e2f6f633fe2c7bd3a74a70cfd13b0ec67c9 Mon Sep 17 00:00:00 2001 From: Lari Taskula 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 Signed-off-by: Kyle M Hall Signed-off-by: Nick Clemens (cherry picked from commit 2101d0af0034bf8278893b4d4ca7d80c29b3bc75) Signed-off-by: Martin Renvoize --- 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.39.5