[Koha-patches] [PATCH] Bug 3130 and general cleanup
Joe Atzberger
joe.atzberger at liblime.com
Wed Apr 29 01:23:36 CEST 2009
Javascript error in FF and IE when loading page and no data matches criteria.
This was because it was passing more holds than necessary to the template and
letting a template conditional inside the loop control whether to display or not.
That doesn't make sense, and it forces the table to be displayed even when all
rows FAIL the conditional. jquery.tablesorter was confused trying to be added
on top of an empty table with nothing to sort.
There were several other errors addressed by this patch:
~ 13 unused variables deleted.
~ regexps run on user-supplied $var before checking $var defined
~ decimal ratio prohibited and silently replaced with "3"
I also added the hold ratio to a column display, with a jquery tweak to put it in
the ratio input box on click. Hidden .sql div now contains the actual query run,
like the other wizard reports.
---
circ/reserveratios.pl | 66 ++++++-----------
.../prog/en/modules/circ/reserveratios.tmpl | 77 ++++++++++----------
2 files changed, 63 insertions(+), 80 deletions(-)
diff --git a/circ/reserveratios.pl b/circ/reserveratios.pl
index 5d6a2a2..b28dc32 100755
--- a/circ/reserveratios.pl
+++ b/circ/reserveratios.pl
@@ -28,12 +28,10 @@ use C4::Debug;
use Date::Calc qw/Today Add_Delta_YM/;
my $input = new CGI;
-my $order = $input->param('order');
-my $startdate=$input->param('from');
-my $enddate=$input->param('to');
-my $ratio=$input->param('ratio');
-
-my $theme = $input->param('theme'); # only used if allowthemeoverride is set
+my $order = $input->param('order');
+my $startdate = $input->param('from');
+my $enddate = $input->param('to');
+my $ratio = $input->param('ratio');
my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
{
@@ -46,19 +44,6 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
}
);
-my $duedate;
-my $borrowernumber;
-my $itemnum;
-my $data1;
-my $data2;
-my $data3;
-my $name;
-my $phone;
-my $email;
-my $biblionumber;
-my $title;
-my $author;
-
my ( $year, $month, $day ) = Today();
my $todaysdate = sprintf("%-04.4d-%-02.2d-%02.2d", $year, $month, $day);
# Find yesterday for the default shelf pull start and end dates
@@ -66,18 +51,14 @@ my $todaysdate = sprintf("%-04.4d-%-02.2d-%02.2d", $year, $month, $day);
my $datelastyear = sprintf("%-04.4d-%-02.2d-%02.2d", Add_Delta_YM($year, $month, $day, -1, 0));
# Predefine the start and end dates if they are not already defined
-$startdate =~ s/^\s+//;
-$startdate =~ s/\s+$//;
-$enddate =~ s/^\s+//;
-$enddate =~ s/\s+$//;
# Check if null, should string match, if so set start and end date to yesterday
-if (!defined($startdate) or $startdate eq "") {
+if (!defined($startdate) or $startdate !~ s/^\s*(\S+)\s*$/$1/) { # strip spaces, remove Taint
$startdate = format_date($datelastyear);
}
-if (!defined($enddate) or $enddate eq "") {
- $enddate = format_date($todaysdate);
+if (!defined($enddate) or $enddate !~ s/^\s*(\S+)\s*$/$1/) { # strip spaces, remove Taint
+ $enddate = format_date($todaysdate);
}
-if (!defined($ratio) or $ratio eq "" or $ratio !~ /^\s*\d+\s*$/ ) {
+if (!defined($ratio) or $ratio !~ s/^\s*(0?\.?\d+)(\.0*)?\s*$/$1/) { # strip spaces, remove Taint
$ratio = 3;
}
if ($ratio == 0) {
@@ -144,20 +125,23 @@ notforloan = 0 AND damaged = 0 AND itemlost = 0 AND wthdrawn = 0
$sqldatewhere
";
-
if (C4::Context->preference('IndependantBranches')){
$strsth .= " AND items.holdingbranch=? ";
push @query_params, C4::Context->userenv->{'branch'};
}
$strsth .= " GROUP BY reserves.biblionumber " . $sqlorderby;
+
+$template->param(sql => $strsth);
my $sth = $dbh->prepare($strsth);
$sth->execute(@query_params);
+my $ratio_atleast1 = ($ratio >= 1) ? 1 : 0;
my @reservedata;
while ( my $data = $sth->fetchrow_hashref ) {
- my @itemlist;
- my $ratiocalc = int(10 * $data->{reservecount} / $data->{itemcount} / $ratio )/10;
+ my $thisratio = $data->{reservecount} / $data->{itemcount};
+ my $ratiocalc = ($thisratio / $ratio);
+ ($thisratio / $ratio) >= 1 or next; # TODO: tighter targeting -- get ratio limit into SQL using HAVING clause
push(
@reservedata,
{
@@ -166,34 +150,32 @@ while ( my $data = $sth->fetchrow_hashref ) {
name => $data->{borrower},
title => $data->{title},
author => $data->{author},
- notes => $data->{notes},
+ notes => $data->{notes},
itemnum => $data->{itemnumber},
biblionumber => $data->{biblionumber},
holdingbranch => $data->{holdingbranch},
- listbranch => $data->{listbranch},
+ listbranch => $data->{listbranch},
branch => $data->{branch},
itemcallnumber => $data->{itemcallnumber},
- location => $data->{l_location},
- itype => $data->{l_itype},
+ location => $data->{l_location},
+ itype => $data->{l_itype},
reservecount => $data->{reservecount},
- itemcount => $data->{itemcount},
- ratiocalc => $ratiocalc,
- ratio_ge_one => $ratiocalc ge 1.0 ? 1 : "",
- listcall => $data->{listcall}
+ itemcount => $data->{itemcount},
+ ratiocalc => sprintf("%.0d", $ratio_atleast1 ? ($thisratio / $ratio) : $thisratio),
+ thisratio => sprintf("%.2f", $thisratio),
+ thisratio_atleast1 => ($thisratio >= 1) ? 1 : 0,
+ listcall => $data->{listcall}
}
);
}
-
-$sth->finish;
-
$template->param(
+ ratio_atleast1 => $ratio_atleast1,
todaysdate => format_date($todaysdate),
from => $startdate,
to => $enddate,
ratio => $ratio,
reserveloop => \@reservedata,
- "BiblioDefaultView".C4::Context->preference("BiblioDefaultView") => 1,
DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(),
);
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/reserveratios.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/reserveratios.tmpl
index fb18f9c..0eeb7cf 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/reserveratios.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/reserveratios.tmpl
@@ -9,7 +9,7 @@
<script type="text/javascript" src="<!-- TMPL_VAR name="themelang" -->/lib/calendar/calendar-setup.js"></script>
<!-- End of additions -->
<script type="text/javascript" src="<!-- TMPL_VAR name="themelang" -->/lib/jquery/plugins/jquery.tablesorter.min.js"></script>
-<script type="text/JavaScript" language="JavaScript">
+<script type="text/javascript" language="JavaScript">
//<![CDATA[
$.tablesorter.addParser({
id: 'articles',
@@ -19,20 +19,31 @@ $.tablesorter.addParser({
});
$(document).ready(function() {
$("th a").hide();
+ $(".ratiolimit").click(function () {
+ $("#ratio").val($(this).html());
+ });
+ $(".ratiolimit").hover(
+ function () { $(this).toggleClass("ulined") },
+ function () { $(this).toggleClass("ulined") }
+ );
$.tablesorter.defaults.widgets = ['zebra'];
- $("#holdst").tablesorter({
+ $("#holdst:has(tbody tr)").tablesorter({ // only add sort if the table has a body and rows
sortList: [[0,0]],
headers: { 1: { sorter: 'articles' }}
});
});
//]]>
</script>
+<style type="text/css">
+ .sql { display: none; }
+ .ulined { text-decoration: underline; }
+ .ratiolimit { color: blue; cursor: pointer; }
+</style>
</head>
<body>
<!-- TMPL_INCLUDE NAME="header.inc" -->
<!-- TMPL_INCLUDE NAME="circ-search.inc" -->
-
<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/circ/circulation-home.pl">Circulation</a> › Hold Ratios</div>
<div id="doc3" class="yui-t2">
@@ -43,7 +54,9 @@ $.tablesorter.addParser({
<h1>Hold Ratios to Calculate Items Needed</h1>
<h3>Calculated on <!-- TMPL_VAR NAME="todaysdate" -->. From <!-- TMPL_VAR NAME="from" -->
to <!-- TMPL_VAR NAME="to" --></h3>
-<p>These items have a large number of holds.</p>
+<p>These items have a hold ratio ≥ <!-- TMPL_VAR NAME="ratio" -->.</p>
+<div class="sql"><!-- TMPL_VAR NAME="sql" DEFAULT="" --></div>
+
<!-- TMPL_IF NAME="reserveloop" -->
<table id="holdst">
<thead> <tr>
@@ -53,6 +66,7 @@ $.tablesorter.addParser({
<th>Items
<a href="/cgi-bin/koha/circ/reserveratios.pl?ratio=<!-- TMPL_VAR NAME="ratio" -->&order=itemcount&from=<!-- TMPL_VAR NAME="from" -->&to=<!-- TMPL_VAR NAME="to" -->">Sort</a>
</th>
+ <th>Hold Ratio</th>
<th>Title
<a href="/cgi-bin/koha/circ/reserveratios.pl?ratio=<!-- TMPL_VAR NAME="ratio" -->&order=biblio&from=<!-- TMPL_VAR NAME="from" -->&to=<!-- TMPL_VAR NAME="to" -->">Sort</a>
</th>
@@ -73,42 +87,29 @@ $.tablesorter.addParser({
</tr></thead>
<tbody><!-- TMPL_LOOP NAME="reserveloop" -->
- <!-- TMPL_IF name="ratio_ge_one" -->
<tr>
- <td>
- <p><!-- TMPL_VAR NAME="reservecount" --></p>
- </td>
- <td>
- <p><!-- TMPL_VAR NAME="itemcount" --></p>
- </td>
- <td>
- <p>
- <!-- TMPL_IF name="BiblioDefaultViewmarc" -->
- <a href="/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" ESCAPE="URL" -->">
- <!-- TMPL_VAR NAME="title" escape="html" --> <!-- TMPL_VAR NAME="subtitle" -->
- </a>
- <!-- TMPL_ELSE -->
- <!-- TMPL_IF name="BiblioDefaultViewisbd" -->
- <a href="/cgi-bin/koha/catalogue/ISBDdetail.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" ESCAPE="URL" -->">
- <!-- TMPL_VAR NAME="title" escape="html" --> <!-- TMPL_VAR NAME="subtitle" -->
- </a>
- <!-- TMPL_ELSE -->
- <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" ESCAPE="URL" -->">
- <!-- TMPL_VAR NAME="title" escape="html" --> <!-- TMPL_VAR NAME="subtitle" -->
- </a>
- <!-- /TMPL_IF -->
- <!-- /TMPL_IF -->
- </p>
- <p><!-- TMPL_VAR NAME="notes" --></p>
- </td>
-
- <td><p><!-- TMPL_VAR NAME="listbranch" --></p></td>
- <td><p><!-- TMPL_VAR NAME="location" --></p></td>
- <td><p><!-- TMPL_VAR NAME="itype" --></p></td>
- <td><p><!-- TMPL_VAR NAME="listcall" --></p></td>
- <td><p><b><!-- TMPL_VAR NAME="ratiocalc" --> to order</b></p></td>
+ <td><p><!-- TMPL_VAR NAME="reservecount" --></p></td>
+ <td><p><!-- TMPL_VAR NAME="itemcount" --></p></td>
+ <td><p class="ratiolimit"><!-- TMPL_VAR NAME="thisratio" --></p></td>
+ <td><p>
+ <!-- TMPL_IF name="BiblioDefaultViewmarc" -->
+ <a href="/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" ESCAPE="URL" -->">
+ <!-- TMPL_ELSIF name="BiblioDefaultViewisbd" -->
+ <a href="/cgi-bin/koha/catalogue/ISBDdetail.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" ESCAPE="URL" -->">
+ <!-- TMPL_ELSE -->
+ <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=<!-- TMPL_VAR NAME="biblionumber" ESCAPE="URL" -->">
+ <!-- /TMPL_IF -->
+ <!-- TMPL_VAR NAME="title" escape="html" --> <!-- TMPL_VAR NAME="subtitle" -->
+ </a>
+ </p>
+ <p><!-- TMPL_VAR NAME="notes" --></p>
+ </td>
+ <td><p><!-- TMPL_VAR NAME="listbranch" --></p></td>
+ <td><p><!-- TMPL_VAR NAME="location" --></p></td>
+ <td><p><!-- TMPL_VAR NAME="itype" --></p></td>
+ <td><p><!-- TMPL_VAR NAME="listcall" --></p></td>
+ <td><!-- TMPL_IF NAME="thisratio_atleast1" --><p><b><!-- TMPL_VAR NAME="ratiocalc" --> to order</b></p><!-- /TMPL_IF --></td>
</tr>
- <!-- /TMPL_IF -->
<!-- /TMPL_LOOP --></tbody>
</table>
<!-- TMPL_ELSE -->
--
1.5.6.5
More information about the Koha-patches
mailing list