Bug 6801: checkoverdues returns unnecessary fields, causing slowness
Explicitly specifies which fields to return in C4::Overdues::checkoverdues SQL: all of biblio, items, and issues, and everything in biblioitems EXCEPT marc, marcxml and timestamp. Bug 6801: member details page taking long time to load when many checkouts present This patch removes the call to GetMemberDetails in build_issue_data; this heavy-weight subroutine was being run for every single item a patron (or their relatives) have checked out. Instead, the borrowers first name, surname and cardnumber are added to the GetPendingIssues query. I believe this is reasonable since GetPendingIssues can now return issues for multiple borrowers. Also corrects the $borrowernumber used for GetIssuesCharges and CanItemBeRenewed; was using the borrower whose page we were on, NOT the borrower of that specific item (which would be different in the Relatives Checkouts tab). Template calls to [% scope.borrowername %] are now broken up into [% scope.firstname %] [% scope.surname %]. Signed-off-by: Liz Rea <lrea@nekls.org> On my test data, a patron with 180 checkouts (without this patch) would take more than a minute to bring back the circulation.pl and moremember.pl pages. With this patch, the time is reduced to 5 or so seconds. Big ups to Ian for tenaciously hunting this one down. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Chris Nighswonger <chris.nighswonger@gmail.com>
This commit is contained in:
Ian Walls
Chris Nighswonger
parent
9898e6d50e
commit
27de4e13e4
|
|
@ -1041,7 +1041,7 @@ sub GetPendingIssues {
|
|||
# Borrowers part of the query
|
||||
my $bquery = '';
|
||||
for (my $i = 0; $i < @borrowernumbers; $i++) {
|
||||
$bquery .= ' borrowernumber = ?';
|
||||
$bquery .= ' issues.borrowernumber = ?';
|
||||
if ($i < $#borrowernumbers ) {
|
||||
$bquery .= ' OR';
|
||||
}
|
||||
|
|
@ -1068,6 +1068,9 @@ sub GetPendingIssues {
|
|||
biblioitems.volumedesc,
|
||||
biblioitems.lccn,
|
||||
biblioitems.url,
|
||||
borrowers.firstname,
|
||||
borrowers.surname,
|
||||
borrowers.cardnumber,
|
||||
issues.timestamp AS timestamp,
|
||||
issues.renewals AS renewals,
|
||||
issues.borrowernumber AS borrowernumber,
|
||||
|
|
@ -1076,6 +1079,7 @@ sub GetPendingIssues {
|
|||
LEFT JOIN items ON items.itemnumber = issues.itemnumber
|
||||
LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
|
||||
LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
|
||||
LEFT JOIN borrowers ON issues.borrowernumber = borrowers.borrowernumber
|
||||
WHERE
|
||||
$bquery
|
||||
ORDER BY issues.issuedate"
|
||||
|
|
|
|||
|
|
@ -165,8 +165,37 @@ Returns a count and a list of overdueitems for a given borrowernumber
|
|||
|
||||
sub checkoverdues {
|
||||
my $borrowernumber = shift or return;
|
||||
# don't select biblioitems.marc or biblioitems.marcxml... too slow on large systems
|
||||
my $sth = C4::Context->dbh->prepare(
|
||||
"SELECT * FROM issues
|
||||
"SELECT biblio.*, items.*, issues.*,
|
||||
biblioitems.volume,
|
||||
bibliotiems.number,
|
||||
biblioitems.itemtype,
|
||||
biblioitems.isbn,
|
||||
biblioitems.issn,
|
||||
biblioitems.publicationyear,
|
||||
biblioitems.publishercode,
|
||||
biblioitems.volumedate,
|
||||
biblioitems.volumedesc,
|
||||
biblioitems.collectiontitle,
|
||||
biblioitems.collectionissn,
|
||||
biblioitems.collectionvolume,
|
||||
biblioitems.editionstatement,
|
||||
biblioitems.editionresponsibility,
|
||||
biblioitems.illus,
|
||||
biblioitems.pages,
|
||||
biblioitems.notes,
|
||||
biblioitems.size,
|
||||
biblioitems.place,
|
||||
biblioitems.lccn,
|
||||
biblioitems.url,
|
||||
biblioitems.cn_source,
|
||||
biblioitems.cn_class,
|
||||
biblioitems.cn_item,
|
||||
biblioitems.cn_suffix,
|
||||
biblioitems.cn_sort,
|
||||
biblioitems.totalissues
|
||||
FROM issues
|
||||
LEFT JOIN items ON issues.itemnumber = items.itemnumber
|
||||
LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
|
||||
LEFT JOIN biblioitems ON items.biblioitemnumber = biblioitems.biblioitemnumber
|
||||
|
|
|
|||
|
|
@ -418,19 +418,15 @@ sub build_issue_data {
|
|||
foreach my $it ( @$issueslist ) {
|
||||
my $itemtypeinfo = getitemtypeinfo( (C4::Context->preference('item-level_itypes')) ? $it->{'itype'} : $it->{'itemtype'} );
|
||||
|
||||
# Getting borrower details
|
||||
my $memberdetails = GetMemberDetails($it->{'borrowernumber'});
|
||||
$it->{'borrowername'} = $memberdetails->{'firstname'} . " " . $memberdetails->{'surname'};
|
||||
$it->{'cardnumber'} = $memberdetails->{'cardnumber'};
|
||||
# set itemtype per item-level_itype syspref - FIXME this is an ugly hack
|
||||
$it->{'itemtype'} = ( C4::Context->preference( 'item-level_itypes' ) ) ? $it->{'itype'} : $it->{'itemtype'};
|
||||
|
||||
($it->{'charge'}, $it->{'itemtype_charge'}) = GetIssuingCharges(
|
||||
$it->{'itemnumber'}, $borrower->{'borrowernumber'}
|
||||
$it->{'itemnumber'}, $it->{'borrowernumber'}
|
||||
);
|
||||
$it->{'charge'} = sprintf("%.2f", $it->{'charge'});
|
||||
my ($can_renew, $can_renew_error) = CanBookBeRenewed(
|
||||
$borrower->{'borrowernumber'},$it->{'itemnumber'}
|
||||
$it->{'borrowernumber'},$it->{'itemnumber'}
|
||||
);
|
||||
$it->{"renew_error_${can_renew_error}"} = 1 if defined $can_renew_error;
|
||||
my ( $restype, $reserves ) = CheckReserves( $it->{'itemnumber'} );
|
||||
|
|
|
|||
|
|
@ -701,7 +701,8 @@ No patron matched <span class="ex">[% message %]</span>
|
|||
<td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% todayissue.biblionumber %]&type=intra"><strong>[% todayissue.title |html %]</strong></a>[% IF ( todayissue.author ) %], by [% todayissue.author %][% END %][% IF ( todayissue.itemnotes ) %]- <span class="circ-hlt">[% todayissue.itemnotes %]</span>[% END %] <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% todayissue.biblionumber %]&itemnumber=[% todayissue.itemnumber %]#item[% todayissue.itemnumber %]">[% todayissue.barcode %]</a></td>
|
||||
<td>[% UNLESS ( noItemTypeImages ) %] [% IF ( todayissue.itemtype_image ) %]<img src="[% todayissue.itemtype_image %]" alt="" />[% END %][% END %][% todayissue.itemtype %]</td>
|
||||
<td>[% todayissue.checkoutdate %]</td>
|
||||
[% IF ( todayissue.multiple_borrowers ) %]<td>[% todayissue.borrowername %]</td>[% END %]
|
||||
[% IF ( todayissue.multiple_borrowers ) %]<td>[% todayissue.firstname %] [% todayissue.surname %]</td>[% END %]
|
||||
<td>[% todayissue.issuingbranchname %]</td>
|
||||
<td>[% todayissue.itemcallnumber %]</td>
|
||||
<td>[% todayissue.charge %]</td>
|
||||
<td>[% todayissue.replacementprice %]</td>
|
||||
|
|
@ -775,7 +776,12 @@ No patron matched <span class="ex">[% message %]</span>
|
|||
[% previssue.itemtype %]
|
||||
</td>
|
||||
<td>[% previssue.displaydate %]</td>
|
||||
<<<<<<< HEAD
|
||||
[% IF ( previssue.multiple_borrowers ) %]<td>[% previssue.borrowername %]</td>[% END %]
|
||||
=======
|
||||
[% IF ( previssue.multiple_borrowers ) %]<td>[% previssue.firstname %] [% previssue.surname %]</td>[% END %]
|
||||
<td>[% previssue.issuingbranchname %]</td>
|
||||
>>>>>>> 26acaf3... Bug 6801: checkoverdues returns unnecessary fields, causing slowness
|
||||
<td>[% previssue.itemcallnumber %]</td>
|
||||
<td>[% previssue.charge %]</td>
|
||||
<td>[% previssue.replacementprice %]</td>
|
||||
|
|
@ -881,7 +887,7 @@ No patron matched <span class="ex">[% message %]</span>
|
|||
<td>[% relissue.displaydate %]</td>
|
||||
<td>[% relissue.itemcallnumber %]</td>
|
||||
<td>[% relissue.charge %]</td>
|
||||
<td>[% relissue.replacementprice %]</td><td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissue.borrowernumber %]">[% relissue.borrowername %] ([% relissue.cardnumber %])</a></td>
|
||||
<td>[% relissue.replacementprice %]</td><td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissue.borrowernumber %]">[% relissue.firstname %] [% relissue.surname %] ([% relissue.cardnumber %])</a></td>
|
||||
</tr>
|
||||
[% END %] <!-- /loop relissues -->
|
||||
<!-- /if relissues -->[% END %]
|
||||
|
|
@ -900,10 +906,10 @@ No patron matched <span class="ex">[% message %]</span>
|
|||
<td>[% UNLESS noItemTypeImages %][% IF relprevissue.itemtype_image %]<img src="[% relprevissue.itemtype_image %]" alt="" />[% END %][% END %][% relprevissue.itemtype %]</td>
|
||||
<td>[% relprevissue.displaydate %]</td>
|
||||
<td>[% relprevissue.itemcallnumber %]</td>
|
||||
[% IF ( relprevissue.multiple_borrowers ) %]<td>[% relprevissue.borrowername %]</td>[% END %]
|
||||
[% IF ( relprevissue.multiple_borrowers ) %]<td>[% relprevissue.firstname %] [% relprevissue.surname %]</td>[% END %]
|
||||
<td>[% relprevissue.charge %]</td>
|
||||
<td>[% relprevissue.replacementprice %]</td>
|
||||
<td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relprevissue.borrowernumber %]">[% relprevissue.borrowername %] ([% relprevissue.cardnumber %])</a></td>
|
||||
<td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relprevissue.borrowernumber %]">[% relprevissue.firstname %] [% relprevissue.surname %] ([% relprevissue.cardnumber %])</a></td>
|
||||
|
||||
</tr>
|
||||
<!-- /loop relprevissue -->[% END %]
|
||||
|
|
|
|||
|
|
@ -562,7 +562,7 @@ function validate1(date) {
|
|||
<td>[% relissueloo.itemcallnumber %]</td>
|
||||
<td>[% relissueloo.charge %]</td>
|
||||
<td>[% relissueloo.replacementprice %]</td>
|
||||
<td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissueloo.borrowernumber %]">[% relissueloo.borrowername %] ([% relissueloo.cardnumber %])</a></td>
|
||||
<td><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% relissueloo.borrowernumber %]">[% relissueloo.firstname %] [% relissueloo.surname %] ([% relissueloo.cardnumber %])</a></td>
|
||||
</tr>
|
||||
[% END %]
|
||||
</tbody>
|
||||
|
|
|
|||
|
|
@ -259,10 +259,6 @@ sub build_issue_data {
|
|||
my $localissue;
|
||||
|
||||
for ( my $i = 0 ; $i < $issuecount ; $i++ ) {
|
||||
# Getting borrower details
|
||||
my $memberdetails = GetMemberDetails($issue->[$i]{'borrowernumber'});
|
||||
$issue->[$i]{'borrowername'} = $memberdetails->{'firstname'} . " " . $memberdetails->{'surname'};
|
||||
$issue->[$i]{'cardnumber'} = $memberdetails->{'cardnumber'};
|
||||
my $datedue = $issue->[$i]{'date_due'};
|
||||
my $issuedate = $issue->[$i]{'issuedate'};
|
||||
$issue->[$i]{'date_due'} = C4::Dates->new($issue->[$i]{'date_due'}, 'iso')->output('syspref');
|
||||
|
|
@ -305,7 +301,7 @@ sub build_issue_data {
|
|||
|
||||
#find the charge for an item
|
||||
my ( $charge, $itemtype ) =
|
||||
GetIssuingCharges( $issue->[$i]{'itemnumber'}, $borrowernumber );
|
||||
GetIssuingCharges( $issue->[$i]{'itemnumber'}, $issue->[$i]{'borrowernumber'} );
|
||||
|
||||
my $itemtypeinfo = getitemtypeinfo($itemtype);
|
||||
$row{'itemtype_description'} = $itemtypeinfo->{description};
|
||||
|
|
@ -313,7 +309,7 @@ sub build_issue_data {
|
|||
|
||||
$row{'charge'} = sprintf( "%.2f", $charge );
|
||||
|
||||
my ( $renewokay,$renewerror ) = CanBookBeRenewed( $borrowernumber, $issue->[$i]{'itemnumber'}, $override_limit );
|
||||
my ( $renewokay,$renewerror ) = CanBookBeRenewed( $issue->[$i]{'borrowernumber'}, $issue->[$i]{'itemnumber'}, $override_limit );
|
||||
$row{'norenew'} = !$renewokay;
|
||||
$row{'can_confirm'} = ( !$renewokay && $renewerror ne 'on_reserve' );
|
||||
$row{"norenew_reason_$renewerror"} = 1 if $renewerror;
|
||||
|
|
|
|||
Loading…
Reference in New Issue