Bugzilla – Attachment 135281 Details for
Bug 30825
Get rid of GetReservesControlBranch
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30825: Pass by reference
Bug-30825-Pass-by-reference.patch (text/plain), 6.97 KB, created by
Martin Renvoize (ashimema)
on 2022-05-23 16:32:27 UTC
(
hide
)
Description:
Bug 30825: Pass by reference
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2022-05-23 16:32:27 UTC
Size:
6.97 KB
patch
obsolete
>From 94a0e29e309ace7d7e741518543d21517cdd2b83 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >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<Koha::Patron> 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<ReservesControlBranch> 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30825
:
135248
|
135249
|
135250
|
135275
|
135276
|
135277
|
135281
|
135488
|
135489
|
135490
|
152100
|
152101
|
152102
|
152103
|
152104
|
155820
|
155821
|
155822
|
155823
|
155824
|
155825
|
155826
|
155827