Bug 8017 reduce manipulation of GetAllIssues return

GetAllIssues can produce large lists
For performance purposes:
Dont loop over the list without cause
Dont do expensive processing in the loop
Dont needlessly copy the array
Do display formatting in the template
Dont extract the barcode list unless we are producing it
Reduce db calls by using the data to hand

Make the table in the template a bit more readable
where everything was stuffed into one line

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This commit is contained in:
Colin Campbell 2012-04-27 18:20:08 +01:00 committed by Paul Poulain
parent 2bcc305f86
commit ef7ebc4ed4
5 changed files with 177 additions and 158 deletions

View File

@ -1072,10 +1072,9 @@ C<items> tables of the Koha database.
sub GetAllIssues {
my ( $borrowernumber, $order, $limit ) = @_;
#FIXME: sanity-check order and limit
my $dbh = C4::Context->dbh;
my $dbh = C4::Context->dbh;
my $query =
"SELECT *, issues.timestamp as issuestimestamp, issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
'SELECT *, issues.timestamp as issuestimestamp, issues.renewals AS renewals,items.renewals AS totalrenewals,items.timestamp AS itemstimestamp
FROM issues
LEFT JOIN items on items.itemnumber=issues.itemnumber
LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
@ -1088,20 +1087,14 @@ sub GetAllIssues {
LEFT JOIN biblio ON items.biblionumber=biblio.biblionumber
LEFT JOIN biblioitems ON items.biblioitemnumber=biblioitems.biblioitemnumber
WHERE borrowernumber=? AND old_issues.itemnumber IS NOT NULL
order by $order";
if ( $limit != 0 ) {
order by ' . $order;
if ($limit) {
$query .= " limit $limit";
}
my $sth = $dbh->prepare($query);
$sth->execute($borrowernumber, $borrowernumber);
my @result;
my $i = 0;
while ( my $data = $sth->fetchrow_hashref ) {
push @result, $data;
}
return \@result;
$sth->execute( $borrowernumber, $borrowernumber );
return $sth->fetchall_arrayref( {} );
}

View File

@ -1,3 +1,4 @@
[% USE KohaDates %]
[% INCLUDE 'doc-head-open.inc' %]
<title>Circulation History for [% INCLUDE 'patron-title.inc' %]</title>
[% INCLUDE 'doc-head-close.inc' %]
@ -26,7 +27,7 @@
<div class="yui-b">
[% INCLUDE 'circ-toolbar.inc' %]
<h1>Circulation history</h1>
[% IF ( loop_reading ) %]
[% IF loop_reading %]
<form action="/cgi-bin/koha/members/readingrec.pl" method="get"><input type="hidden" name="borrowernumber" id="borrowernumber" value="[% borrowernumber %]" /></form>
@ -43,29 +44,37 @@
<th>Date due</th>
<th>Return date</th>
</thead>
[% FOREACH loop_readin IN loop_reading %]
[% IF ( loop_readin.returndate ) %]<tr>[% ELSE %]<tr class="onissue">[% END %]
[% FOREACH issue IN loop_reading %]
[% IF issue.returndate %]<tr>[% ELSE %]<tr class="onissue">[% END %]
<td>
[% loop_readin.issuestimestamp %]
[% issue.issuestimestamp | $KohaDates %]
</td>
<td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loop_readin.biblionumber %]">[% loop_readin.title |html %]</a></td>
<td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% issue.biblionumber %]">[% issue.title |html %]</a></td>
<td>[% loop_readin.author %]</td>
<td>[% issue.author %]</td>
<td>[% loop_readin.classification %]</td>
<td>
[% IF issue.classification %]
[% issue.classification %]
[% ELSE %]
[% issue.itemcallnumber %]
[% END %]
</td>
<td>[% loop_readin.barcode %]</td>
<td>[% issue.barcode %]</td>
<td>
[% loop_readin.renewals %]</td>
[% issue.renewals %]</td>
<td>
[% loop_readin.issuedate %]</td>
[% issue.issuedate | $KohaDates %]</td>
<td>
[% loop_readin.issuingbranch %]</td>
<td>[% IF ( loop_readin.date_due ) %][% loop_readin.date_due %][% ELSE %]&nbsp;[% END %]</td>
[% issue.issuingbranch %]</td>
<td>[% IF issue.date_due %]
[% issue.date_due | $KohaDates %]
[% ELSE %]&nbsp;[% END %]</td>
<td>
[% IF ( loop_readin.returndate ) %]
[% loop_readin.returndate %]
[% IF issue.returndate %]
[% issue.returndate | $KohaDates %]
[% ELSE %]
Checked Out
[% END %]

View File

@ -26,7 +26,7 @@ $(document).ready(function(){
<!--CONTENT-->
<h3><a href="/cgi-bin/koha/opac-user.pl">[% firstname %] [% surname %]'s account</a> &#8674; Checkout history</h3>
[% UNLESS ( count ) %]
[% IF READING_RECORD.size() == 0 %]
You have never borrowed anything from this library.
[% ELSE %]
<div id="opac-user-readingrec" class="statictabs">
@ -64,32 +64,67 @@ You have never borrowed anything from this library.
[% END %]
</tr>
[% FOREACH READING_RECOR IN READING_RECORD %]
[% FOREACH issue IN READING_RECORD %]
[% UNLESS ( loop.odd ) %]<tr class="highlight">[% ELSE %]<tr>[% END %]
[% IF loop.even %]<tr class="highlight">[% ELSE %]<tr>[% END %]
<td>
[% IF ( OPACAmazonCoverImages ) %][% IF ( READING_RECOR.normalized_isbn ) %]<a href="http://www.amazon.com/gp/reader/[% READING_RECOR.normalized_isbn %]/ref=sib_dp_pt/002-7879865-0184864#reader-link"><img border="0" src="http://images.amazon.com/images/P/[% READING_RECOR.normalized_isbn %].01.THUMBZZZ.jpg" alt="Cover Image" /></a>[% ELSE %]<span class="no-image">No cover image available</span>[% END %][% END %]
[% IF OPACAmazonCoverImages %]
[% IF issue.normalized_isbn %]
<a href="http://www.amazon.com/gp/reader/[% issue.normalized_isbn %]/ref=sib_dp_pt/002-7879865-0184864#reader-link"><img border="0" src="http://images.amazon.com/images/P/[% issue.normalized_isbn %].01.THUMBZZZ.jpg" alt="Cover Image" /></a>
[% ELSE %]
<span class="no-image">No cover image available</span>
[% END %]
[% END %]
[% IF ( GoogleJackets ) %][% IF ( READING_RECOR.normalized_isbn ) %]<div style="block" title="[% READING_RECOR.biblionumber |url %]" class="[% READING_RECOR.normalized_isbn %]" id="gbs-thumbnail[% loop.count %]"></div>[% ELSE %]<span class="no-image">No cover image available</span>[% END %][% END %]
[% IF GoogleJackets %]
[% IF issue.normalized_isbn %]
<div style="block" title="[% issue.biblionumber |url %]" class="[% issue.normalized_isbn %]" id="gbs-thumbnail[% loop.count %]"></div>
[% ELSE %]
<span class="no-image">No cover image available</span>
[% END %]
[% END %]
[% IF ( BakerTaylorEnabled ) %][% IF ( READING_RECOR.normalized_isbn ) %]<a href="https://[% BakerTaylorBookstoreURL |html %][% READING_RECOR.normalized_isbn %]"><img alt="See Baker &amp; Taylor" src="[% BakerTaylorImageURL |html %][% READING_RECOR.normalized_isbn %]" /></a>[% ELSE %]<span class="no-image">No cover image available</span>[% END %][% END %]
[% IF BakerTaylorEnabled %]
[% IF issue.normalized_isbn %]
<a href="https://[% BakerTaylorBookstoreURL |html %][% issue.normalized_isbn %]"><img alt="See Baker &amp; Taylor" src="[% BakerTaylorImageURL |html %][% issue.normalized_isbn %]" /></a>
[% ELSE %]
<span class="no-image">No cover image available</span>
[% END %]
[% END %]
[% IF ( SyndeticsEnabled ) %][% IF ( SyndeticsCoverImages ) %][% IF ( using_https ) %]
<img src="https://secure.syndetics.com/index.aspx?isbn=[% READING_RECOR.normalized_isbn %]/SC.GIF&amp;client=[% SyndeticsClientCode %]&amp;type=xw10&amp;upc=[% READING_RECOR.normalized_upc %]&amp;oclc=[% READING_RECOR.normalized_oclc %]" alt="" class="thumbnail" />
[% ELSE %]
<img src="http://www.syndetics.com/index.aspx?isbn=[% READING_RECOR.normalized_isbn %]/SC.GIF&amp;client=[% SyndeticsClientCode %]&amp;type=xw10&amp;upc=[% READING_RECOR.normalized_upc %]&amp;oclc=[% READING_RECOR.normalized_oclc %]" alt="" class="thumbnail" />[% END %][% END %][% END %]
[% IF SyndeticsEnabled %]
[% IF SyndeticsCoverImages %]
[% IF using_https %]
<img src="https://secure.syndetics.com/index.aspx?isbn=[% issue.normalized_isbn %]/SC.GIF&amp;client=[% SyndeticsClientCode %]&amp;type=xw10&amp;upc=[% issue.normalized_upc %]&amp;oclc=[% issue.normalized_oclc %]" alt="" class="thumbnail" />
[% ELSE %]
<img src="http://www.syndetics.com/index.aspx?isbn=[% issue.normalized_isbn %]/SC.GIF&amp;client=[% SyndeticsClientCode %]&amp;type=xw10&amp;upc=[% issue.normalized_upc %]&amp;oclc=[% issue.normalized_oclc %]" alt="" class="thumbnail" />
[% END %]
[% END %]
[% END %]
</td>
<td>[% IF ( READING_RECOR.BiblioDefaultViewmarc ) %]<a class="title" href="/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=[% READING_RECOR.biblionumber |url %]">[% READING_RECOR.title |html %] [% IF ( READING_RECOR.subtitle ) %][% FOREACH subtitl IN READING_RECOR.subtitle %][% subtitl.subfield %][% END %][% END %]</a>[% ELSE %]
[% IF ( READING_RECOR.BiblioDefaultViewisbd ) %]<a class="title" href="/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=[% READING_RECOR.biblionumber |url %]">[% READING_RECOR.title |html %] [% IF ( READING_RECOR.subtitle ) %][% FOREACH subtitl IN READING_RECOR.subtitle %][% subtitl.subfield %][% END %][% END %]</a>[% ELSE %]
<a class="title" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% READING_RECOR.biblionumber |url %]">[% READING_RECOR.title |html %] [% IF ( READING_RECOR.subtitle ) %][% FOREACH subtitl IN READING_RECOR.subtitle %][% subtitl.subfield %][% END %][% END %]</a>[% END %][% END %]
<td>
[% IF issue.BiblioDefaultViewmarc %]
<a class="title" href="/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=[% issue.biblionumber |url %]">[% issue.title |html %] [% IF issue.subtitle %][% FOREACH subtitl IN issue.subtitle %][% subtitl.subfield %][% END %][% END %]</a>
[% ELSIF issue.BiblioDefaultViewisbd %]
<a class="title" href="/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=[% issue.biblionumber |url %]">[% issue.title |html %] [% IF issue.subtitle %][% FOREACH subtitl IN issue.subtitle %][% subtitl.subfield %][% END %][% END %]</a>
[% ELSE %]
<a class="title" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% issue.biblionumber |url %]">[% issue.title |html %] [% IF issue.subtitle %][% FOREACH subtitl IN issue.subtitle %][% subtitl.subfield %][% END %][% END %]</a>
[% END %]
<span class="item-details">
[% READING_RECOR.author %]
[% issue.author %]
</span></td>
<td>[% UNLESS ( noItemTypeImages ) %][% IF ( READING_RECOR.imageurl ) %]<img src="[% READING_RECOR.imageurl %]" alt="" />[% END %][% END %] [% READING_RECOR.description %]</td>
<td>[% READING_RECOR.itemcallnumber %]</td>
<td>[% IF ( READING_RECOR.returndate ) %][% READING_RECOR.returndate | $KohaDates %][% ELSE %]<em>(Checked out)</em>[% END %]</td>
[% IF ( OPACMySummaryHTML ) %]
<td>[% READING_RECOR.MySummaryHTML %]</td>
<td>
[% UNLESS ( noItemTypeImages ) %][% IF ( issue.imageurl ) %]<img src="[% issue.imageurl %]" alt="" />[% END %][% END %] [% issue.description %]</td>
<td>[% issue.itemcallnumber %]</td>
<td>
[% IF issue.returndate %]
[% issue.returndate | $KohaDates %]
[% ELSE %]
<em>(Checked out)</em>
[% END %]
</td>
[% IF OPACMySummaryHTML %]
<td>[% issue.MySummaryHTML %]</td>
[% END %]
</tr>

View File

@ -28,7 +28,7 @@ use CGI;
use C4::Auth;
use C4::Output;
use C4::Members;
use C4::Branch;
use C4::Branch qw(GetBranches);
use List::MoreUtils qw/any uniq/;
use Koha::DateUtils;
@ -62,44 +62,29 @@ if ($input->param('borrowernumber')) {
my $order = 'date_due desc';
my $limit = 0;
my ( $issues ) = GetAllIssues($borrowernumber,$order,$limit);
my $issues = GetAllIssues($borrowernumber,$order,$limit);
my @loop_reading;
my @barcodes;
my $today = C4::Dates->new();
$today = $today->output("iso");
foreach my $issue (@{$issues}){
my %line;
$line{issuestimestamp} = format_date($issue->{'issuestimestamp'});
$line{biblionumber} = $issue->{'biblionumber'};
$line{title} = $issue->{'title'};
$line{author} = $issue->{'author'};
$line{classification} = $issue->{'classification'} || $issue->{'itemcallnumber'};
$line{date_due} = format_sqldatetime($issue->{date_due});
$line{returndate} = format_sqldatetime($issue->{returndate});
$line{issuedate} = format_sqldatetime($issue->{issuedate});
$line{issuingbranch} = GetBranchName($issue->{'branchcode'});
$line{renewals} = $issue->{'renewals'};
$line{barcode} = $issue->{'barcode'};
$line{volumeddesc} = $issue->{'volumeddesc'};
push(@loop_reading,\%line);
my $return_dt = Koha::DateUtils::dt_from_string($issue->{'returndate'}, 'iso');
if ( ( $input->param('op') eq 'export_barcodes' ) and ( $today eq $return_dt->ymd() ) ) {
push( @barcodes, $issue->{'barcode'} );
}
my $branches = GetBranches();
foreach my $issue ( @{$issues} ) {
$issue->{issuingbranch} = $branches->{ $issue->{branchcode} }->{branchname};
}
if ($input->param('op') eq 'export_barcodes') {
my $borrowercardnumber = GetMember( borrowernumber => $borrowernumber )->{'cardnumber'} ;
# barcode export
if ( $input->param('op') eq 'export_barcodes' ) {
my $today = C4::Dates->new();
$today = $today->output('iso');
my @barcodes =
map { $_->{barcode} } grep { $_->{returndate} =~ m/^$today/o } @{$issues};
my $borrowercardnumber =
GetMember( borrowernumber => $borrowernumber )->{'cardnumber'};
my $delimiter = "\n";
binmode( STDOUT, ":encoding(UTF-8)");
binmode( STDOUT, ":encoding(UTF-8)" );
print $input->header(
-type => 'application/octet-stream',
-charset => 'utf-8',
-attachment => "$today-$borrowercardnumber-checkinexport.txt"
);
my $content = join($delimiter, uniq(@barcodes));
my $content = join $delimiter, uniq(@barcodes);
print $content;
exit;
}
@ -116,6 +101,7 @@ if (! $limit){
$limit = 'full';
}
my ($picture, $dberror) = GetPatronImage($data->{'cardnumber'});
$template->param( picture => 1 ) if $picture;
@ -128,36 +114,31 @@ if (C4::Context->preference('ExtendedPatronAttributes')) {
}
$template->param(
readingrecordview => 1,
biblionumber => $data->{'biblionumber'},
title => $data->{'title'},
initials => $data->{'initials'},
surname => $data->{'surname'},
othernames => $data->{'othernames'},
borrowernumber => $borrowernumber,
limit => $limit,
firstname => $data->{'firstname'},
cardnumber => $data->{'cardnumber'},
categorycode => $data->{'categorycode'},
category_type => $data->{'category_type'},
# category_description => $data->{'description'},
categoryname => $data->{'description'},
address => $data->{'address'},
address2 => $data->{'address2'},
city => $data->{'city'},
state => $data->{'state'},
zipcode => $data->{'zipcode'},
country => $data->{'country'},
phone => $data->{'phone'},
email => $data->{'email'},
branchcode => $data->{'branchcode'},
is_child => ($data->{'category_type'} eq 'C'),
branchname => GetBranchName($data->{'branchcode'}),
showfulllink => (scalar @loop_reading > 50),
loop_reading => \@loop_reading,
activeBorrowerRelationship => (C4::Context->preference('borrowerRelationship') ne ''),
readingrecordview => 1,
title => $data->{title},
initials => $data->{initials},
surname => $data->{surname},
othernames => $data->{othernames},
borrowernumber => $borrowernumber,
firstname => $data->{firstname},
cardnumber => $data->{cardnumber},
categorycode => $data->{categorycode},
category_type => $data->{category_type},
categoryname => $data->{description},
address => $data->{address},
address2 => $data->{address2},
city => $data->{city},
state => $data->{state},
zipcode => $data->{zipcode},
country => $data->{country},
phone => $data->{phone},
email => $data->{email},
branchcode => $data->{branchcode},
is_child => ( $data->{category_type} eq 'C' ),
branchname => $branches->{ $data->{branchcode} }->{branchname},
loop_reading => $issues,
activeBorrowerRelationship =>
( C4::Context->preference('borrowerRelationship') ne '' ),
);
output_html_with_http_headers $input, $cookie, $template->output;

View File

@ -27,8 +27,10 @@ use C4::Biblio;
use C4::Circulation;
use C4::Members;
use Koha::DateUtils;
use MARC::Record;
use C4::Output;
use C4::Charset qw(StripNonXmlChars);
my $query = new CGI;
my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
@ -50,65 +52,65 @@ $template->param(%{$borr});
my $itemtypes = GetItemTypes();
# get the record
my $order = $query->param('order') || '';
if ( $order eq '' ) {
my $order = $query->param('order') || '';
if ( $order eq 'title' ) {
$template->param( orderbytitle => 1 );
}
elsif ( $order eq 'author' ) {
$template->param( orderbyauthor => 1 );
}
else {
$order = "date_due desc";
$template->param( orderbydate => 1 );
}
if ( $order eq 'title' ) {
$template->param( orderbytitle => 1 );
}
if ( $order eq 'author' ) {
$template->param( orderbyauthor => 1 );
}
my $limit = $query->param('limit');
$limit = ( $limit eq 'full' ) ? 0 : 50;
my $limit = $query->param('limit') || 50;
if ( $limit eq 'full' ) {
$limit = 0;
}
else {
$limit = 50;
}
my $issues = GetAllIssues( $borrowernumber, $order, $limit );
my ( $issues ) = GetAllIssues( $borrowernumber, $order, $limit );
my $itype_attribute =
( C4::Context->preference('item-level_itypes') ) ? 'itype' : 'itemtype';
my @loop_reading;
foreach my $issue (@{$issues} ) {
my %line;
my $record = GetMarcBiblio($issue->{'biblionumber'});
# XISBN Stuff
my $isbn = GetNormalizedISBN($issue->{'isbn'});
$line{normalized_isbn} = $isbn;
$line{biblionumber} = $issue->{'biblionumber'};
$line{title} = $issue->{'title'};
$line{author} = $issue->{'author'};
$line{itemcallnumber} = $issue->{'itemcallnumber'};
$line{date_due} = $issue->{'date_due'};
$line{returndate} = $issue->{'returndate'};
$line{volumeddesc} = $issue->{'volumeddesc'};
$issue->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $issue->{'itype'} : $issue->{'itemtype'};
if($issue->{'itemtype'}) {
$line{'description'} = $itemtypes->{ $issue->{'itemtype'} }->{'description'};
$line{imageurl} = getitemtypeimagelocation( 'opac', $itemtypes->{ $issue->{'itemtype'} }->{'imageurl'} );
my $opac_summary_html = C4::Context->preference('OPACMySummaryHTML');
foreach my $issue ( @{$issues} ) {
$issue->{normalized_isbn} = GetNormalizedISBN( $issue->{isbn} );
if ( $issue->{$itype_attribute} ) {
$issue->{description} =
$itemtypes->{ $issue->{$itype_attribute} }->{description};
$issue->{imageurl} =
getitemtypeimagelocation( 'opac',
$itemtypes->{ $issue->{$itype_attribute} }->{imageurl} );
}
if ( $issue->{marcxml} ) {
my $marcxml = StripNonXmlChars( $issue->{marcxml} );
my $marc_rec =
MARC::Record::new_from_xml( $marcxml, 'utf8',
C4::Context->preference('marcflavour') );
$issue->{subtitle} =
GetRecordValue( 'subtitle', $marc_rec, $issue->{frameworkcode} );
}
# My Summary HTML
if (my $my_summary_html = C4::Context->preference('OPACMySummaryHTML')){
$line{author} ? $my_summary_html =~ s/{AUTHOR}/$line{author}/g : $my_summary_html =~ s/{AUTHOR}//g;
$line{title} =~ s/\/+$//; # remove trailing slash
$line{title} =~ s/\s+$//; # remove trailing space
$line{title} ? $my_summary_html =~ s/{TITLE}/$line{title}/g : $my_summary_html =~ s/{TITLE}//g;
$line{normalized_isbn} ? $my_summary_html =~ s/{ISBN}/$line{normalized_isbn}/g : $my_summary_html =~ s/{ISBN}//g;
$line{biblionumber} ? $my_summary_html =~ s/{BIBLIONUMBER}/$line{biblionumber}/g : $my_summary_html =~ s/{BIBLIONUMBER}//g;
$line{MySummaryHTML} = $my_summary_html;
if ($opac_summary_html) {
my $my_summary_html = $opac_summary_html;
$issue->{author}
? $my_summary_html =~ s/{AUTHOR}/$issue->{author}/g
: $my_summary_html =~ s/{AUTHOR}//g;
my $title = $issue->{title};
$title =~ s/\/+$//; # remove trailing slash
$title =~ s/\s+$//; # remove trailing space
$title
? $my_summary_html =~ s/{TITLE}/$title/g
: $my_summary_html =~ s/{TITLE}//g;
$issue->{normalized_isbn}
? $my_summary_html =~ s/{ISBN}/$issue->{normalized_isbn}/g
: $my_summary_html =~ s/{ISBN}//g;
$issue->{biblionumber}
? $my_summary_html =~ s/{BIBLIONUMBER}/$issue->{biblionumber}/g
: $my_summary_html =~ s/{BIBLIONUMBER}//g;
$issue->{MySummaryHTML} = $my_summary_html;
}
push( @loop_reading, \%line );
$line{subtitle} = GetRecordValue('subtitle', $record, GetFrameworkCode($issue->{'biblionumber'}));
}
if (C4::Context->preference('BakerTaylorEnabled')) {
@ -135,12 +137,11 @@ for(qw(AmazonCoverImages GoogleJackets)) { # BakerTaylorEnabled handled above
}
$template->param(
READING_RECORD => \@loop_reading,
READING_RECORD => $issues,
limit => $limit,
showfulllink => 1,
readingrecview => 1,
count => scalar @loop_reading,
OPACMySummaryHTML => (C4::Context->preference("OPACMySummaryHTML")) ? 1 : 0,
readingrecview => 1,
OPACMySummaryHTML => $opac_summary_html ? 1 : 0,
);
output_html_with_http_headers $query, $cookie, $template->output;