Bugzilla – Attachment 113526 Details for
Bug 26963
Improve Koha::Item::pickup_locations performance
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26963: (QA follow-up) Convert to ResultSets
Bug-26963-QA-follow-up-Convert-to-ResultSets.patch (text/plain), 6.80 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-11-11 18:26:17 UTC
(
hide
)
Description:
Bug 26963: (QA follow-up) Convert to ResultSets
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-11-11 18:26:17 UTC
Size:
6.80 KB
patch
obsolete
>From 29b9c8227287f4e5cad9d9686104862ef8ab7922 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Wed, 11 Nov 2020 08:57:21 +0000 >Subject: [PATCH] Bug 26963: (QA follow-up) Convert to ResultSets > >This patch removes the previously introduced private method by >converting the arrayref returns to ResultSets appropriately and inlining >the filter search queries. > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/Item.pm | 114 ++++++++++++------------------------- > Koha/Library.pm | 2 +- > t/db_dependent/Koha/Item.t | 2 +- > 3 files changed, 37 insertions(+), 81 deletions(-) > >diff --git a/Koha/Item.pm b/Koha/Item.pm >index 71f6b7827b8..db87a0aaf7e 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -553,67 +553,6 @@ sub can_be_transferred { > > } > >-=head3 _can_pickup_locations >- >-$item->_can_pickup_locations({ to => $to_libraries, from => $from_library }) >-Checks if an item can be transferred to given libraries. >- >-This feature is controlled by two system preferences: >-UseBranchTransferLimits to enable / disable the feature >-BranchTransferLimitsType to use either an itemnumber or ccode as an identifier >- for setting the limitations >- >-Takes HASHref that can have the following parameters: >- MANDATORY PARAMETERS: >- $to : Array of Koha::Libraries >- OPTIONAL PARAMETERS: >- $from : Koha::Library # if not given, item holdingbranch >- # will be used instead >- >-Returns arry of Koha::Libraries that item can be transferred to $to_library and >-are pickup_locations >- >-If checking only one library please use $item->can_be_transferred. >- >-=cut >- >-sub _can_pickup_locations { >- my ($self, $params ) = @_; >- >- my $to = $params->{to}; >- my $from = $params->{from}; >- $from = defined $from ? $from->branchcode : $self->holdingbranch; >- >- my @pickup_locations; >- my @destination_codes; >- foreach my $lib (@$to){ >- next unless $lib->pickup_location; >- push @destination_codes, $lib->branchcode; >- push @pickup_locations, $lib; >- } >- >- return \@pickup_locations unless C4::Context->preference('UseBranchTransferLimits'); >- >- my $limittype = C4::Context->preference('BranchTransferLimitsType'); >- my $limiter = $limittype eq 'itemtype' ? $self->effective_itemtype : $self->ccode; >- >- my $limits = Koha::Item::Transfer::Limits->search({ >- fromBranch => $from, >- $limittype => $limiter >- }); >- my @limits = $limits->get_column('toBranch'); >- return \@pickup_locations unless @limits; >- >- my @can_transfer = Koha::Libraries->search({ >- pickup_location => 1, >- branchcode => { >- -in => \@destination_codes, >- -not_in => \@limits, >- } >- }); >- return \@can_transfer; >-} >- > =head3 pickup_locations > > $pickup_locations = $item->pickup_locations( {patron => $patron } ) >@@ -633,36 +572,53 @@ sub pickup_locations { > my $branchitemrule = > C4::Circulation::GetBranchItemRule( $circ_control_branch, $self->itype ); > >- my @libs; > if(defined $patron) { >- return \@libs if $branchitemrule->{holdallowed} == 3 && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} ); >- return \@libs if $branchitemrule->{holdallowed} == 1 && $self->home_branch->branchcode ne $patron->branchcode; >+ return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} == 3 && !$self->home_branch->validate_hold_sibling( {branchcode => $patron->branchcode} ); >+ return Koha::Libraries->new()->empty if $branchitemrule->{holdallowed} == 1 && $self->home_branch->branchcode ne $patron->branchcode; > } > >+ my $pickup_libraries = Koha::Libraries->search(); > if ($branchitemrule->{hold_fulfillment_policy} eq 'holdgroup') { >- @libs = $self->home_branch->get_hold_libraries; >- push @libs, $self->home_branch unless scalar(@libs) > 0; >+ $pickup_libraries = $self->home_branch->get_hold_libraries; > } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'patrongroup') { > my $plib = Koha::Libraries->find({ branchcode => $patron->branchcode}); >- @libs = $plib->get_hold_libraries; >- push @libs, $self->home_branch unless scalar(@libs) > 0; >+ $pickup_libraries = $plib->get_hold_libraries; > } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'homebranch') { >- push @libs, $self->home_branch; >+ $pickup_libraries = Koha::Libraries->search({ branchcode => $self->homebranch }); > } elsif ($branchitemrule->{hold_fulfillment_policy} eq 'holdingbranch') { >- push @libs, $self->holding_branch; >- } else { >- @libs = Koha::Libraries->search({ >+ $pickup_libraries = Koha::Libraries->search({ branchcode => $self->holdingbranch }); >+ }; >+ >+ return $pickup_libraries->search( >+ { > pickup_location => 1 >- }, { >+ }, >+ { > order_by => ['branchname'] >- })->as_list; >- } >+ } >+ ) unless C4::Context->preference('UseBranchTransferLimits'); > >- my $pickup_locations = $self->_can_pickup_locations({ >- to => \@libs >- }); >+ my $limittype = C4::Context->preference('BranchTransferLimitsType'); >+ my $limiter = $limittype eq 'itemtype' ? $self->effective_itemtype : $self->ccode; >+ my $limits = Koha::Item::Transfer::Limits->search( >+ { >+ fromBranch => $self->holdingbranch, >+ $limittype => $limiter >+ }, >+ { columns => ['toBranch'] } >+ ); > >- return $pickup_locations; >+ return $pickup_libraries->search( >+ { >+ pickup_location => 1, >+ branchcode => { >+ '-not_in' => $limits->_resultset->as_query >+ } >+ }, >+ { >+ order_by => ['branchname'] >+ } >+ ); > } > > =head3 article_request_type >diff --git a/Koha/Library.pm b/Koha/Library.pm >index 86291b0c7f5..b69fdaaeabf 100644 >--- a/Koha/Library.pm >+++ b/Koha/Library.pm >@@ -229,7 +229,7 @@ sub get_hold_libraries { > @hold_libraries = > grep { !$seen{ $_->id }++ } @hold_libraries; > >- return @hold_libraries; >+ return Koha::Libraries->search({ branchcode => { '-in' => [ keys %seen ] } }); > } > > =head3 validate_hold_sibling >diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t >index e8fd36a9a8d..8cb1b399bd9 100755 >--- a/t/db_dependent/Koha/Item.t >+++ b/t/db_dependent/Koha/Item.t >@@ -290,7 +290,7 @@ subtest 'pickup_locations' => sub { > } > } > ); >- my @pl = @{ $item->pickup_locations( { patron => $patron} ) }; >+ my @pl = $item->pickup_locations( { patron => $patron} )->as_list; > my $ha_value=$ha==3?'holdgroup':($ha==2?'any':'homebranch'); > > foreach my $pickup_location (@pl) { >-- >2.29.2
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 26963
:
113282
|
113283
|
113329
|
113330
|
113333
|
113339
|
113340
|
113419
|
113420
|
113422
|
113423
|
113424
|
113425
|
113451
|
113452
|
113453
|
113454
|
113455
|
113480
|
113481
|
113483
|
113498
|
113503
|
113504
|
113505
|
113506
|
113507
|
113508
|
113509
|
113510
|
113511
|
113512
|
113513
|
113514
|
113515
|
113516
|
113517
|
113518
|
113519
|
113520
|
113521
|
113523
|
113524
|
113525
| 113526 |
113527
|
113528
|
113529
|
113530
|
113531
|
113586
|
113729