gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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 <tomascohen@theke.io>
(cherry picked from commit ef8adf34fd)
Signed-off-by: Jacob O'Mara <jacobomara901@gmail.com>
This commit is contained in:
Nick Clemens 2023-04-20 15:44:53 +00:00 committed by Jacob O'Mara
parent 5eec29d2da
commit 4e51e04ce1
4 changed files with 16 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Koha/Biblio.pm
View File

@ -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

4
Koha/Biblios.pm
View File

@ -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

11
Koha/Item.pm
View File

@ -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') {

4
t/db_dependent/Koha/Biblios.t
View File

@ -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(