From d607ea97a0ba00a328cbef1a11f81e40d3beb268 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Mon, 9 Nov 2020 13:53:51 +0000
Subject: [PATCH] Bug 26963: Don't call 'can_be_transferred' for each possible
library for each item
Currently When calling Koha::Template::Plugin::Branches::pickup_locations
we call pickup_location for each item of the bib, and for each item we get a list of possible
branches, we then check those branches against the transfer limits, this is inefficent
Given a system with 100 branches, and each branch having an item attached to one bib (100 items)
we will call can_be_transferred ~10000 times - and that will happen for each hold placed on the bib
For me this patch reduced load time from 77 seconds to 18 seconds
To test:
1 - Find a bib
2 - Place 4 title level holds
3 - Add some branches and items for this bib to your system:
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"D"),CONCAT(branchname,"A"),pickup_location FROM branches;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"D"),CONCAT(branchname,"B"),pickup_location FROM branches;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"D"),CONCAT(branchname,"C"),pickup_location FROM branches;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"D"),CONCAT(branchname,"D"),pickup_location FROM branches;
INSERT INTO items (biblionumber,biblioitemnumber,barcode,itype,homebranch,holdingbranch) SELECT 1,1,CONCAT("test-",branchcode),'BKS',branchcode,branchcode FROM branches;
4 - Set systempreferences:
UseBranchTransferLimits = 'enforce'
BranchTransferLimitsType = 'item type'
5 - Find the bib and click the holds tab
6 - Wait for a long time, it shoudl eventually load
7 - Apply patch and restart al the things
8 - Load the page again, it should be much faster
---
Koha/Item.pm | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 67 insertions(+), 7 deletions(-)
diff --git a/Koha/Item.pm b/Koha/Item.pm
index bedf4d1856..8b3936c727 100644
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -550,6 +550,69 @@ sub can_be_transferred {
$limittype => $limittype eq 'itemtype'
? $self->effective_itemtype : $self->ccode
})->count ? 0 : 1;
+
+}
+
+=head3 _can_pickup_at
+
+$item->_can_pickup_at({ 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_at {
+ 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 @cant_transfer;
+ 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
@@ -596,14 +659,11 @@ sub pickup_locations {
})->as_list;
}
- my @pickup_locations;
- foreach my $library (@libs) {
- if ($library->pickup_location && $self->can_be_transferred({ to => $library })) {
- push @pickup_locations, $library;
- }
- }
+ my $pickup_locations = $self->_can_pickup_at({
+ to => \@libs
+ });
- return \@pickup_locations;
+ return $pickup_locations;
}
=head3 article_request_type
--
2.11.0