Bugzilla – Attachment 14544 Details for
Bug 9367
Code optimization: CheckReserves is too often called
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9367: Code optimization: CheckReserves is too often called
Bug-9367-Code-optimization-CheckReserves-is-too-of.patch (text/plain), 9.90 KB, created by
Kyle M Hall (khall)
on 2013-01-11 19:22:08 UTC
(
hide
)
Description:
Bug 9367: Code optimization: CheckReserves is too often called
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2013-01-11 19:22:08 UTC
Size:
9.90 KB
patch
obsolete
>From f8a21aab44dac25458dae7ab33bb6717525914c3 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >Date: Tue, 8 Jan 2013 15:33:58 +0100 >Subject: [PATCH] Bug 9367: Code optimization: CheckReserves is too often called > >This patch rewrites the GetReserveStatus routine in order to take in >parameter the itemnumber and/or the biblionumber. > >In some places, the C4::Reserves::CheckReserves routine is called when >we just want to get the status of the reserve. In these cases, the >C4::Reserves::GetReserveStatus is now called. >This routine executes 1 sql query (or 2 max). > >Test plan: >Check that there is no regression on the different pages where reserves >are used. The different status will be the same than before applying >this patch. > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Biblio.pm | 11 +---------- > C4/Circulation.pm | 7 +++---- > C4/Reserves.pm | 33 ++++++++++++++++++++++++--------- > C4/Search.pm | 11 +++++------ > C4/XSLT.pm | 2 +- > circ/circulation.pl | 4 ++-- > members/moremember.pl | 1 - > opac/opac-detail.pl | 2 +- > opac/opac-user.pl | 2 +- > 9 files changed, 38 insertions(+), 35 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 7d28ccd..a63d7a3 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -785,20 +785,11 @@ sub GetBiblioData { > my ($bibnum) = @_; > my $dbh = C4::Context->dbh; > >- # my $query = C4::Context->preference('item-level_itypes') ? >- # " SELECT * , biblioitems.notes AS bnotes, biblio.notes >- # FROM biblio >- # LEFT JOIN biblioitems ON biblio.biblionumber = biblioitems.biblionumber >- # WHERE biblio.biblionumber = ? >- # AND biblioitems.biblionumber = biblio.biblionumber >- #"; >- > my $query = " SELECT * , biblioitems.notes AS bnotes, itemtypes.notforloan as bi_notforloan, biblio.notes > FROM biblio > LEFT JOIN biblioitems ON biblio.biblionumber = biblioitems.biblionumber > LEFT JOIN itemtypes ON biblioitems.itemtype = itemtypes.itemtype >- WHERE biblio.biblionumber = ? >- AND biblioitems.biblionumber = biblio.biblionumber "; >+ WHERE biblio.biblionumber = ?"; > > my $sth = $dbh->prepare($query); > $sth->execute($bibnum); >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 71fe20f..53f24f4 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2479,12 +2479,11 @@ sub CanBookBeRenewed { > $error="too_many"; > } > >- my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber); >- if ($resfound) { >+ my $resstatus = C4::Reserves::GetReserveStatus($itemnumber); >+ if ( $resstatus eq "Waiting" or $resstatus eq "Reserved" ) { > $renewokay = 0; >- $error="on_reserve" >+ $error->{message} = "on_reserve"; > } >- > } > return ($renewokay,$error); > } >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index a380bf0..ee31ea9 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -736,15 +736,29 @@ sub GetReservesForBranch { > } > > sub GetReserveStatus { >- my ($itemnumber) = @_; >- >+ my ($itemnumber, $biblionumber) = @_; >+ > my $dbh = C4::Context->dbh; >- >- my $itemstatus = $dbh->prepare("SELECT found FROM reserves WHERE itemnumber = ?"); >- >- $itemstatus->execute($itemnumber); >- my ($found) = $itemstatus->fetchrow_array; >- return $found; >+ >+ my ($sth, $found, $priority); >+ if ( $itemnumber ) { >+ $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE itemnumber = ? order by priority LIMIT 1"); >+ $sth->execute($itemnumber); >+ ($found, $priority) = $sth->fetchrow_array; >+ } >+ >+ if ( $biblionumber and not defined $found and not defined $priority ) { >+ $sth = $dbh->prepare("SELECT found, priority FROM reserves WHERE biblionumber = ? order by priority LIMIT 1"); >+ $sth->execute($biblionumber); >+ } else { >+ return; >+ } >+ ($found, $priority) = $sth->fetchrow_array; >+ >+ return 'Waiting' if $found eq 'W' and $priority == 0; >+ return 'Finished' if $found eq 'F'; >+ return 'Reserved' if $priority > 0; >+ return; > } > > =head2 CheckReserves >@@ -1441,7 +1455,8 @@ sub IsAvailableForItemLevelRequest { > if (C4::Context->preference('AllowOnShelfHolds')) { > return $available_per_item; > } else { >- return ($available_per_item and ($item->{onloan} or GetReserveStatus($itemnumber) eq "W")); >+ my $status = GetReserveStatus($itemnumber); >+ return ($available_per_item and ($item->{onloan} or $status eq "Waiting" or $status = "Reserved")); > } > } > >diff --git a/C4/Search.pm b/C4/Search.pm >index 50f7d2b..76cb615 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -28,7 +28,7 @@ use C4::Dates qw(format_date); > use C4::Members qw(GetHideLostItemsPreference); > use C4::XSLT; > use C4::Branch; >-use C4::Reserves; # CheckReserves >+use C4::Reserves; # GetReserveStatus > use C4::Debug; > use C4::Charset; > use YAML; >@@ -1854,8 +1854,7 @@ sub searchResults { > my ($transfertfrom, $transfertto); > > # is item on the reserve shelf? >- my $reservestatus = ''; >- my $reserveitem; >+ my $reservestatus = ''; > > unless ($item->{wthdrawn} > || $item->{itemlost} >@@ -1876,7 +1875,7 @@ sub searchResults { > # should map transit status to record indexed in Zebra. > # > ($transfertwhen, $transfertfrom, $transfertto) = C4::Circulation::GetTransfers($item->{itemnumber}); >- ($reservestatus, $reserveitem, undef) = C4::Reserves::CheckReserves($item->{itemnumber}); >+ $reservestatus = C4::Reserves::GetReserveStatus( $item->{itemnumber}, $oldbiblio->{biblionumber} ); > } > > # item is withdrawn, lost, damaged, not for loan, reserved or in transit >@@ -1884,14 +1883,14 @@ sub searchResults { > || $item->{itemlost} > || $item->{damaged} > || $item->{notforloan} >- || $reservestatus eq 'Waiting' >+ || $reservestatus eq 'Waiting' > || ($transfertwhen ne '')) > { > $wthdrawn_count++ if $item->{wthdrawn}; > $itemlost_count++ if $item->{itemlost}; > $itemdamaged_count++ if $item->{damaged}; > $item_in_transit_count++ if $transfertwhen ne ''; >- $item_onhold_count++ if $reservestatus eq 'Waiting'; >+ $item_onhold_count++ if $reservestatus eq 'Waiting'; > $item->{status} = $item->{wthdrawn} . "-" . $item->{itemlost} . "-" . $item->{damaged} . "-" . $item->{notforloan}; > > # can place hold on item ? >diff --git a/C4/XSLT.pm b/C4/XSLT.pm >index 19ec162..0770bda 100644 >--- a/C4/XSLT.pm >+++ b/C4/XSLT.pm >@@ -240,7 +240,7 @@ sub buildKohaItemsNamespace { > > my ( $transfertwhen, $transfertfrom, $transfertto ) = C4::Circulation::GetTransfers($item->{itemnumber}); > >- my ( $reservestatus, $reserveitem, undef ) = C4::Reserves::CheckReserves($item->{itemnumber}); >+ my $reservestatus = C4::Reserves::GetReserveStatus( $item->{itemnumber} ); > > if ( $itemtypes->{ $item->{itype} }->{notforloan} || $item->{notforloan} || $item->{onloan} || $item->{wthdrawn} || $item->{itemlost} || $item->{damaged} || > (defined $transfertwhen && $transfertwhen ne '') || $item->{itemnotforloan} || (defined $reservestatus && $reservestatus eq "Waiting") ){ >diff --git a/circ/circulation.pl b/circ/circulation.pl >index 91fd1a2..8e96c4f 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -448,10 +448,10 @@ sub build_issue_data { > $it->{'borrowernumber'},$it->{'itemnumber'} > ); > $it->{"renew_error_${can_renew_error}"} = 1 if defined $can_renew_error; >- my ( $restype, $reserves, undef ) = CheckReserves( $it->{'itemnumber'} ); >+ my $restype = C4::Reserves::GetReserveStatus( $it->{'itemnumber'} ); > $it->{'can_renew'} = $can_renew; > $it->{'can_confirm'} = !$can_renew && !$restype; >- $it->{'renew_error'} = $restype; >+ $it->{'renew_error'} = ( $restype eq "Waiting" or $restype eq "Reserved" ) ? 1 : 0; > $it->{'checkoutdate'} = C4::Dates->new($it->{'issuedate'},'iso')->output('syspref'); > $it->{'issuingbranchname'} = GetBranchName($it->{'branchcode'}); > >diff --git a/members/moremember.pl b/members/moremember.pl >index 3928286..d740566 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -47,7 +47,6 @@ use C4::Circulation; > use C4::Koha; > use C4::Letters; > use C4::Biblio; >-use C4::Reserves; > use C4::Branch; # GetBranchName > use C4::Overdues qw/CheckBorrowerDebarred/; > use C4::Form::MessagingPreferences; >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index 22c922b..3ec165e 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -559,7 +559,7 @@ for my $itm (@items) { > $itm->{'lostimageurl'} = $lostimageinfo->{ 'imageurl' }; > $itm->{'lostimagelabel'} = $lostimageinfo->{ 'label' }; > } >- my ($reserve_status) = C4::Reserves::CheckReserves($itm->{itemnumber}); >+ my $reserve_status = C4::Reserves::GetReserveStatus($itm->{itemnumber}); > if( $reserve_status eq "Waiting"){ $itm->{'waiting'} = 1; } > if( $reserve_status eq "Reserved"){ $itm->{'onhold'} = 1; } > >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index 1c942d5..bd70ccb 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -156,7 +156,7 @@ my $issues = GetPendingIssues($borrowernumber); > if ($issues){ > foreach my $issue ( sort { $b->{date_due}->datetime() cmp $a->{date_due}->datetime() } @{$issues} ) { > # check for reserves >- my ( $restype, $res, undef ) = CheckReserves( $issue->{'itemnumber'} ); >+ my $restype = GetReserveStatus( $issue->{'itemnumber'} ); > if ( $restype ) { > $issue->{'reserved'} = 1; > } >-- >1.7.2.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9367
:
14471
|
14544
|
16016
|
16018
|
16021
|
16022
|
16023