From 4e51e04ce150896a6429ccf9af07d6460efb89e1 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 20 Apr 2023 15:44:53 +0000 Subject: [PATCH] Bug 33447: (follow-up) Fix tests and make assumption explicit The patches made an assumption that patron would always be passed. It is within Koha, but not in the Biblios tests. There is no scenario where we can determine pickup locations that are not in reference to a patron (who is picking it up?) so we should always have this parameter Signed-off-by: Tomas Cohen Arazi (cherry picked from commit ef8adf34fde2352680934ba6a36de89e3f905a36) Signed-off-by: Jacob O'Mara --- Koha/Biblio.pm | 7 ++++--- Koha/Biblios.pm | 4 +++- Koha/Item.pm | 11 ++++++----- t/db_dependent/Koha/Biblios.t | 4 +++- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index e5cf7ae2c5..5c82b271a3 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -240,11 +240,12 @@ sub can_be_transferred { =head3 pickup_locations - my $pickup_locations = $biblio->pickup_locations( {patron => $patron } ); + my $pickup_locations = $biblio->pickup_locations( { patron => $patron } ); Returns a Koha::Libraries set of possible pickup locations for this biblio's items, -according to patron's home library (if patron is defined and holds are allowed -only from hold groups) and if item can be transferred to each pickup location. +according to patron's home library and if item can be transferred to each pickup location. + +Patron is a required parameter. =cut diff --git a/Koha/Biblios.pm b/Koha/Biblios.pm index cf1cbdb83a..af8de37cb1 100644 --- a/Koha/Biblios.pm +++ b/Koha/Biblios.pm @@ -40,7 +40,9 @@ Koha::Biblios - Koha Biblio object set class my $biblios = Koha::Biblios->search(...); my $pickup_locations = $biblios->pickup_locations({ patron => $patron }); -For a given resultset, it returns all the pickup locations +For a given resultset, it returns all the pickup locations. + +Patron is a required parameter. =cut diff --git a/Koha/Item.pm b/Koha/Item.pm index dbcede82d7..55c02e31e4 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -735,25 +735,26 @@ sub can_be_transferred { $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) +Returns possible pickup locations for this item, according to patron's home library and if item can be transferred to each pickup location. +Patron parameter is required. + =cut sub pickup_locations { my ($self, $params) = @_; my $patron = $params->{patron}; + # FIXME We should throw an exception if not passed my $circ_control_branch = C4::Reserves::GetReservesControlBranch( $self->unblessed(), $patron->unblessed ); my $branchitemrule = C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype ); - if(defined $patron) { - return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_local_hold_group' && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} ); - return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_home_library' && $self->home_branch->branchcode ne $patron->branchcode; - } + return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_local_hold_group' && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} ); + return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} eq 'from_home_library' && $self->home_branch->branchcode ne $patron->branchcode; my $pickup_libraries = Koha::Libraries->search(); if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup') { diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t index b93efebb21..bf9d36d216 100755 --- a/t/db_dependent/Koha/Biblios.t +++ b/t/db_dependent/Koha/Biblios.t @@ -278,6 +278,8 @@ subtest 'pickup_locations() tests' => sub { my $item_2_1 = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber }); my $item_2_2 = $builder->build_sample_item({ biblionumber => $biblio_2->biblionumber }); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $biblios = Koha::Biblios->search( { biblionumber => [ $biblio_1->biblionumber, $biblio_2->biblionumber ] @@ -298,7 +300,7 @@ subtest 'pickup_locations() tests' => sub { ]; my $pickup_locations_ids = [ - $biblios->pickup_locations->_resultset->get_column('branchcode')->all + $biblios->pickup_locations({ patron => $patron })->_resultset->get_column('branchcode')->all ]; is_deeply( -- 2.39.5