From 94a0e29e309ace7d7e741518543d21517cdd2b83 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 23 May 2022 17:30:20 +0100 Subject: [PATCH] Bug 30825: Pass by reference Not sure if this is nicer or not honestly. This changes the signature of hols_control_library to accept a hashref of patron_id and patron object reference. If the patron_object reference is found to be undefined and our syspref requires it we populate the reference with a Koha::Patron object as found from the patron_id. --- C4/Reserves.pm | 8 ++++---- Koha/Item.pm | 15 ++++++++------- opac/opac-reserve.pl | 2 +- t/db_dependent/Koha/Item.t | 12 ++++++++---- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 659f379bfd..1dcd1a3c11 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -919,10 +919,10 @@ sub CheckReserves { if ( ( $res->{'priority'} && $res->{'priority'} < $priority ) || $local_hold_match ) { $item ||= Koha::Items->find($itemnumber); next if $res->{itemtype} && $res->{itemtype} ne $item->effective_itemtype; - $patron ||= Koha::Patrons->find( $res->{borrowernumber} ); - my $branch = $item->holds_control_library( $patron ); + my $branch = $item->holds_control_library({ patron => \$patron, patron_id => $res->{borrowernumber} }); my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype); next if ($branchitemrule->{'holdallowed'} eq 'not_allowed'); + $patron ||= Koha::Patrons->find( $res->{borrowernumber} ); next if (($branchitemrule->{'holdallowed'} eq 'from_home_library') && ($item->homebranch ne $patron->branchcode)); my $library = Koha::Libraries->find({branchcode=>$item->homebranch}); next if (($branchitemrule->{'holdallowed'} eq 'from_local_hold_group') && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) )); @@ -1341,7 +1341,7 @@ sub IsAvailableForItemLevelRequest { return 0 unless $destination; return 0 unless $destination->pickup_location; return 0 unless $item->can_be_transferred( { to => $destination } ); - my $reserves_control_branch = $item->holds_control_library( $patron ); + my $reserves_control_branch = $item->holds_control_library({ patron => \$patron }); my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); my $home_library = Koha::Libraries->find( {branchcode => $item->homebranch} ); @@ -1383,7 +1383,7 @@ sub ItemsAnyAvailableAndNotRestricted { my @items = Koha::Items->search( { biblionumber => $param->{biblionumber} } )->as_list; foreach my $i (@items) { - my $reserves_control_branch = $i->holds_control_library( $param->{patron} ); + my $reserves_control_branch = $i->holds_control_library({ patron => \$param->{patron} }); my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype ); my $item_library = Koha::Libraries->find( { branchcode => $i->homebranch } ); diff --git a/Koha/Item.pm b/Koha/Item.pm index 7c5f35d033..58fbfd4754 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -434,7 +434,7 @@ sub holds { =head3 holds_control_library - my $control_library = $item->holds_control_library( $patron ); + my $control_library = $item->holds_control_library({ patron => $patron, patron_id => $patron_id }); Given a I object, this method returns a library id, for the library that is to be used for calculating circulation rules. It relies @@ -443,12 +443,13 @@ on the B system preference. =cut sub holds_control_library { - my ( $self, $patron ) = @_; + my ( $self, $params ) = @_; + + return $self->homebranch + if C4::Context->preference('ReservesControlBranch') eq 'ItemHomeLibrary'; - return ( - C4::Context->preference('ReservesControlBranch') eq 'ItemHomeLibrary' ) - ? $self->homebranch - : $patron->branchcode; + ${ $params->{patron} } //= Koha::Patrons->find( $params->{patron_id} ); + return ${ $params->{patron} }->branchcode; } =head3 request_transfer @@ -729,7 +730,7 @@ sub pickup_locations { my $patron = $params->{patron}; - my $circ_control_branch = $self->holds_control_library( $patron ); + my $circ_control_branch = $self->holds_control_library({ patron => \$patron }); my $branchitemrule = C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype ); diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index ebe6c73645..549df3594a 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -559,7 +559,7 @@ foreach my $biblioNum (@biblionumbers) { # If there is no loan, return and transfer, we show a checkbox. $itemLoopIter->{notforloan} = $itemLoopIter->{notforloan} || 0; - my $branch = $item->holds_control_library( $patron ); + my $branch = $item->holds_control_library({ patron => \$patron }); my $policy_holdallowed = !$itemLoopIter->{already_reserved}; # items_any_available defined outside of the current loop, diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index ecf05bf98e..574472e86f 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -1456,23 +1456,27 @@ subtest 'Recalls tests' => sub { subtest 'holds_control_library() tests' => sub { - plan tests => 2; + plan tests => 5; $schema->storage->txn_begin; my $library_1 = $builder->build_object({ class => 'Koha::Libraries' }); my $library_2 = $builder->build_object({ class => 'Koha::Libraries' }); - my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { branchcode => $library_1->id } }); + my $patron; + my $new_patron = $builder->build_object({ class => 'Koha::Patrons', value => { branchcode => $library_1->id } }); my $item = $builder->build_sample_item({ library => $library_2->id }); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' ); - is( $item->holds_control_library( $patron ), $library_2->id ); + is( $patron, undef, '$patron is not yet set'); + is( $item->holds_control_library({ patron => \$patron, patron_id => $new_patron->borrowernumber }), $library_2->id, 'ItemHomeLibrary correctly selected' ); + is( $patron, undef, '$patron is still not set'); t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); - is( $item->holds_control_library( $patron ), $library_1->id ); + is( $item->holds_control_library({ patron => \$patron, patron_id => $new_patron->borrowernumber }), $library_1->id, 'PatronLibrary correctly selected' ); + is(ref($patron), 'Koha::Patron', '$patron now set to Koha::Patron object'); $schema->storage->txn_rollback; }; -- 2.20.1