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 <andrew@bywatersolutions.com>
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Nick Clemens 2020-11-09 13:53:51 +00:00 committed by Jonathan Druart
parent db1865ba34
commit 780ced27f5
1 changed files with 66 additions and 7 deletions

View File

@ -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