From 492c72ec5a378e260013e9b470936971c2feb7d2 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 --- Koha/Biblio.pm | 5 +++-- Koha/Biblios.pm | 4 +++- Koha/Item.pm | 11 ++++++----- t/db_dependent/Koha/Biblios.t | 4 +++- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm index a94665ddc8..3801828612 100644 --- a/Koha/Biblio.pm +++ b/Koha/Biblio.pm @@ -258,8 +258,9 @@ sub can_be_transferred { 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 0d29655938..5ed04bcca4 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -736,25 +736,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.30.2