From 780ced27f5ac90d227c88e6fcab2fa43c0b3ffb5 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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 Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Bob Bennhoff Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart --- Koha/Item.pm | 73 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 707bef5a41..9766d8be29 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -550,6 +550,68 @@ 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 $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 +658,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.39.5