Added error catching for bad user input on number of days. I.E., if you
try to filter by "bAd", you now get an error message prompting for valid
digits. Also I updated highlighting to use loop_context_vars.
Fixed filtering to work on either days, vendor or both. Previously, if
you selected a number of days, you had to select a vendor or else got
empty results. DOCUMENTATION NOTE: this supplies the expected behavior,
so specifying vendor is no longer required.
Changed filters form to GET method.
---
C4/Acquisition.pm | 161 ++++++++-----------
C4/Bookseller.pm | 2 +-
acqui/lateorders.pl | 81 +++++-----
.../prog/en/modules/acqui/lateorders.tmpl | 117 ++++++++-------
4 files changed, 168 insertions(+), 193 deletions(-)
diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 1df6121..a599fdd 100644
--- a/C4/Acquisition.pm
+++ b/C4/Acquisition.pm
@@ -20,6 +20,7 @@ package C4::Acquisition;
use strict;
use C4::Context;
+use C4::Debug;
use C4::Dates qw(format_date);
use MARC::Record;
use C4::Suggestions;
@@ -977,107 +978,81 @@ sub GetLateOrders {
my $dbh = C4::Context->dbh;
#BEWARE, order of parenthesis and LEFT JOIN is important for speed
- my $strsth;
my $dbdriver = C4::Context->config("db_scheme") || "mysql";
- my @query_params = ();
-
- # warn " $dbdriver";
- if ( $dbdriver eq "mysql" ) {
- $strsth = "
- SELECT aqbasket.basketno,aqorders.ordernumber,
- DATE(aqbasket.closedate) AS orderdate,
- aqorders.quantity - IFNULL(aqorders.quantityreceived,0) AS
quantity,
- aqorders.rrp AS unitpricesupplier,
- aqorders.ecost AS unitpricelib,
- (aqorders.quantity - IFNULL(aqorders.quantityreceived,0)) *
aqorders.rrp AS subtotal,
- aqbookfund.bookfundname AS budget,
- borrowers.branchcode AS branch,
- aqbooksellers.name AS supplier,
- aqorders.title,
- biblio.author,
- biblioitems.publishercode AS publisher,
- biblioitems.publicationyear,
- DATEDIFF(CURDATE( ),closedate) AS latesince
- FROM (((
- (aqorders LEFT JOIN biblio ON biblio.biblionumber =
aqorders.biblionumber)
- LEFT JOIN biblioitems ON
biblioitems.biblionumber=biblio.biblionumber)
- LEFT JOIN aqorderbreakdown ON aqorders.ordernumber =
aqorderbreakdown.ordernumber)
- LEFT JOIN aqbookfund ON aqorderbreakdown.bookfundid =
aqbookfund.bookfundid),
- (aqbasket LEFT JOIN borrowers ON aqbasket.authorisedby =
borrowers.borrowernumber)
- LEFT JOIN aqbooksellers ON aqbasket.booksellerid = aqbooksellers.id
- WHERE aqorders.basketno = aqbasket.basketno
- AND (closedate <= DATE_SUB(CURDATE( ),INTERVAL ? DAY))
- AND ((datereceived = '' OR datereceived is null)
- OR (aqorders.quantityreceived < aqorders.quantity) )
- ";
-
- push @query_params, $delay;
-
- if ( defined $supplierid ) {
- $strsth .= ' AND aqbasket.booksellerid = ? ';
- push @query_params, $supplierid;
- }
-
- if ( defined $branch ) {
- $strsth .= ' AND borrowers.branchcode like ? ';
- push @query_params, $branch;
- }
-
- if ( C4::Context->preference("IndependantBranches")
+ my @query_params = ($delay); # delay is the first argument regardless
+ my $select = "
+ SELECT aqbasket.basketno,
+ aqorders.ordernumber,
+ DATE(aqbasket.closedate) AS orderdate,
+ aqorders.rrp AS unitpricesupplier,
+ aqorders.ecost AS unitpricelib,
+ aqbookfund.bookfundname AS budget,
+ borrowers.branchcode AS branch,
+ aqbooksellers.name AS supplier,
+ aqorders.title,
+ biblio.author,
+ biblioitems.publishercode AS publisher,
+ biblioitems.publicationyear,
+ ";
+ my $from = "
+ FROM (((
+ (aqorders LEFT JOIN biblio ON biblio.biblionumber =
aqorders.biblionumber)
+ LEFT JOIN biblioitems ON biblioitems.biblionumber =
biblio.biblionumber)
+ LEFT JOIN aqorderbreakdown ON aqorders.ordernumber =
aqorderbreakdown.ordernumber)
+ LEFT JOIN aqbookfund ON aqorderbreakdown.bookfundid =
aqbookfund.bookfundid),
+ (aqbasket LEFT JOIN borrowers ON aqbasket.authorisedby =
borrowers.borrowernumber)
+ LEFT JOIN aqbooksellers ON aqbasket.booksellerid =
aqbooksellers.id
+ WHERE aqorders.basketno = aqbasket.basketno
+ AND ( (datereceived = '' OR datereceived IS NULL)
+ OR (aqorders.quantityreceived < aqorders.quantity)
+ )
+ ";
+ my $having = "";
+ if ($dbdriver eq "mysql") {
+ $select .= "
+ aqorders.quantity - IFNULL(aqorders.quantityreceived,0)
AS quantity,
+ (aqorders.quantity - IFNULL(aqorders.quantityreceived,0)) *
aqorders.rrp AS subtotal,
+ DATEDIFF(CURDATE( ),closedate) AS latesince
+ ";
+ $from .= " AND (closedate <= DATE_SUB(CURDATE( ),INTERVAL ? DAY)) ";
+ $having = "
+ HAVING quantity <> 0
+ AND unitpricesupplier <> 0
+ AND unitpricelib <> 0
+ ";
+ } else {
+ # FIXME: account for IFNULL as above
+ $select .= "
+ aqorders.quantity AS quantity,
+ aqorders.quantity * aqorders.rrp AS subtotal,
+ (CURDATE - closedate) AS latesince
+ ";
+ $from .= " AND (closedate <= (CURDATE -(INTERVAL ? DAY)) ";
+ }
+ if (defined $supplierid) {
+ $from .= ' AND aqbasket.booksellerid = ? ';
+ push @query_params, $supplierid;
+ }
+ if (defined $branch) {
+ $from .= ' AND borrowers.branchcode LIKE ? ';
+ push @query_params, $branch;
+ }
+ if (C4::Context->preference("IndependantBranches")
&& C4::Context->userenv
&& C4::Context->userenv->{flags} != 1 ) {
- $strsth .= ' AND borrowers.branchcode like ? ';
- push @query_params, C4::Context->userenv->{branch};
- }
-
- $strsth .= " HAVING quantity <> 0
- AND unitpricesupplier <> 0
- AND unitpricelib <> 0
- ORDER BY latesince, basketno, borrowers.branchcode,
supplier ";
- } else {
- $strsth = "
- SELECT aqbasket.basketno,
- DATE(aqbasket.closedate) AS orderdate,
- aqorders.quantity, aqorders.rrp AS unitpricesupplier,
- aqorders.ecost as unitpricelib,
- aqorders.quantity * aqorders.rrp AS subtotal
- aqbookfund.bookfundname AS budget,
- borrowers.branchcode AS branch,
- aqbooksellers.name AS supplier,
- biblio.title,
- biblio.author,
- biblioitems.publishercode AS publisher,
- biblioitems.publicationyear,
- (CURDATE - closedate) AS latesince
- FROM(( (
- (aqorders LEFT JOIN biblio on biblio.biblionumber =
aqorders.biblionumber)
- LEFT JOIN biblioitems on
biblioitems.biblionumber=biblio.biblionumber)
- LEFT JOIN aqorderbreakdown on aqorders.ordernumber =
aqorderbreakdown.ordernumber)
- LEFT JOIN aqbookfund ON aqorderbreakdown.bookfundid =
aqbookfund.bookfundid),
- (aqbasket LEFT JOIN borrowers on aqbasket.authorisedby
= borrowers.borrowernumber) LEFT JOIN aqbooksellers ON aqbasket.booksellerid =
aqbooksellers.id
- WHERE aqorders.basketno = aqbasket.basketno
- AND (closedate <= (CURDATE -(INTERVAL $delay DAY))
- AND ((datereceived = '' OR datereceived is null)
- OR (aqorders.quantityreceived < aqorders.quantity) ) ";
- $strsth .= " AND aqbasket.booksellerid = $supplierid " if
($supplierid);
-
- $strsth .= " AND borrowers.branchcode like \'" . $branch . "\'" if
($branch);
- $strsth .=" AND borrowers.branchcode like \'".
C4::Context->userenv->{branch} . "\'"
- if (C4::Context->preference("IndependantBranches") &&
C4::Context->userenv->{flags} != 1 );
- $strsth .=" ORDER BY latesince,basketno,borrowers.branchcode,
supplier";
+ $from .= ' AND borrowers.branchcode LIKE ? ';
+ push @query_params, C4::Context->userenv->{branch};
}
- my $sth = $dbh->prepare($strsth);
- $sth->execute( @query_params );
+ my $query = "$select $from $having\nORDER BY latesince, basketno,
borrowers.branchcode, supplier";
+ $debug and print STDERR "GetLateOrders query: $query\nGetLateOrders
args: " . join(" ",@query_params);
+ my $sth = $dbh->prepare($query);
+ $sth->execute(@query_params);
my @results;
- my $hilighted = 1;
- while ( my $data = $sth->fetchrow_hashref ) {
- $data->{hilighted} = $hilighted if ( $hilighted > 0 );
- $data->{orderdate} = format_date( $data->{orderdate} );
+ while (my $data = $sth->fetchrow_hashref) {
+ $data->{orderdate} = format_date($data->{orderdate});
push @results, $data;
- $hilighted = -$hilighted;
}
- $sth->finish;
return @results;
}
diff --git a/C4/Bookseller.pm b/C4/Bookseller.pm
index d3e63c7..332d3bc 100644
--- a/C4/Bookseller.pm
+++ b/C4/Bookseller.pm
@@ -110,7 +110,7 @@ Searches for suppliers with late orders.
=cut
sub GetBooksellersWithLateOrders {
- my ($delay,$branch) = @_;
+ my ($delay,$branch) = @_; # FIXME: Branch argument unused.
my $dbh = C4::Context->dbh;
# FIXME NOT quite sure that this operation is valid for DBMs different from
Mysql, HOPING so
diff --git a/acqui/lateorders.pl b/acqui/lateorders.pl
index 7c7a766..222b233 100755
--- a/acqui/lateorders.pl
+++ b/acqui/lateorders.pl
@@ -43,6 +43,7 @@ To know on which branch this script have to display late
order.
=cut
use strict;
+use warnings;
use CGI;
use C4::Bookseller;
use C4::Auth;
@@ -54,67 +55,61 @@ use C4::Letters;
use C4::Branch; # GetBranches
my $input = new CGI;
-my ($template, $loggedinuser, $cookie)
-= get_template_and_user(
- {template_name => "acqui/lateorders.tmpl",
- query => $input,
- type => "intranet",
- authnotrequired => 0,
- flagsrequired => {acquisition => 1},
- debug => 1,
- });
-
-my $supplierid = $input->param('supplierid');
-my $delay = $input->param('delay');
-my $branch = $input->param('branch');
-
-#default value for delay
-$delay = 30 unless $delay;
+my ($template, $loggedinuser, $cookie) = get_template_and_user({
+ template_name => "acqui/lateorders.tmpl",
+ query => $input,
+ type => "intranet",
+ authnotrequired => 0,
+ flagsrequired => {acquisition => 1},
+ debug => 1,
+});
+
+my $supplierid = $input->param('supplierid') || undef; # we don't want "" or 0
+my $delay = $input->param('delay');
+my $branch = $input->param('branch');
+my $op = $input->param('op');
+
+my @errors = ();
+$delay = 30 unless defined $delay;
+unless ($delay =~ /^\d{1,3}$/) {
+ push @errors, {delay_digits => 1, bad_delay => $delay};
+ $delay = 30; #default value for delay
+}
my %supplierlist = GetBooksellersWithLateOrders($delay,$branch);
-my @select_supplier;
-push @select_supplier,"";
-foreach my $supplierid (keys %supplierlist){
- push @select_supplier, $supplierid;
+my (@sloopy); # supplier loop
+foreach (keys %supplierlist){
+ push @sloopy, (($supplierid and $supplierid eq $_ ) ?
+ {id=>$_, name=>$supplierlist{$_},
selected=>1} :
+ {id=>$_, name=>$supplierlist{$_}} )
;
}
-
-my $CGIsupplier=CGI::scrolling_list( -name => 'supplierid',
- -values => [EMAIL PROTECTED],
- -default => $supplierid,
- -id => 'supplierid',
- -labels => \%supplierlist,
- -size => 1,
- -tabindex=>'',
- -multiple => 0 );
-
+$template->param(SUPPLIER_LOOP => [EMAIL PROTECTED]);
$template->param(Supplier=>$supplierlist{$supplierid}) if ($supplierid);
my @lateorders = GetLateOrders($delay,$supplierid,$branch);
-my $count = scalar @lateorders;
my $total;
-foreach my $lateorder (@lateorders){
- $total+=$lateorder->{subtotal};
+foreach (@lateorders){
+ $total += $_->{subtotal};
}
my @letters;
my $letters=GetLetters("claimacquisition");
foreach (keys %$letters){
- push @letters ,{code=>$_,name=>$letters->{$_}};
+ push @letters, {code=>$_,name=>$letters->{$_}};
}
-
$template->param(letters=>[EMAIL PROTECTED]) if (@letters);
-my $op=$input->param("op");
-if ($op eq "send_alert"){
- my @ordernums=$input->param("claim_for");
- SendAlerts('claimacquisition',[EMAIL
PROTECTED],$input->param("letter_code"));
+
+if ($op and $op eq "send_alert"){
+ my @ordernums = $input->param("claim_for");
# FIXME: Fallback values?
+ SendAlerts('claimacquisition',[EMAIL
PROTECTED],$input->param("letter_code")); # FIXME: Fallback value?
}
-$template->param(delay=>$delay) if ($delay);
+$template->param(ERROR_LOOP => [EMAIL PROTECTED]) if (@errors);
$template->param(
- CGIsupplier => $CGIsupplier,
lateorders => [EMAIL PROTECTED],
- total=>$total,
+ delay => $delay,
+ total => $total,
intranetcolorstylesheet =>
C4::Context->preference("intranetcolorstylesheet"),
- );
+);
output_html_with_http_headers $input, $cookie, $template->output;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tmpl
b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tmpl
index 5b0deeb..16788ba 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tmpl
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tmpl
@@ -6,7 +6,7 @@
<!-- TMPL_INCLUDE NAME="header.inc" -->
<!-- TMPL_INCLUDE NAME="acquisitions-search.inc" -->
-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a
href="/cgi-bin/koha/acqui/acqui-home.pl">Acquisitions</a> › Late
orders</div>
+<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a
href="/cgi-bin/koha/acqui/acqui-home.pl">Acquisitions</a> › <a
href="lateorders.pl">Late orders</a></div>
<div id="doc3" class="yui-t2">
@@ -17,14 +17,17 @@
<h1><!-- TMPL_IF name="Supplier" --><!-- TMPL_VAR name="Supplier" --> :
<!--/TMPL_IF -->Late orders</h1>
<div id="acqui_lateorders">
-<!-- TMPL_IF NAME="lateorders" --><form action="lateorders.pl" name="claim"
method="post">
-<input type="hidden" name="op" value="send_alert" />
-<!-- TMPL_IF NAME="letters" --><p><label for="letter_code">Claim using notice:
</label><select name="letter_code" id="letter_code">
- <!--TMPL_LOOP Name="letters"-->
- <option value="<!--TMPL_VAR
Name="code"-->"><!--TMPL_VAR Name="name"--></option>
- <!--/TMPL_LOOP -->
- </select>
- </p><!-- /TMPL_IF -->
+<!-- TMPL_IF NAME="lateorders" -->
+<form action="lateorders.pl" name="claim" method="post">
+ <input type="hidden" name="op" value="send_alert" />
+ <!-- TMPL_IF NAME="letters" -->
+ <p><label for="letter_code">Claim using notice: </label><select
name="letter_code" id="letter_code">
+ <!--TMPL_LOOP Name="letters"-->
+ <option value="<!--TMPL_VAR Name="code"-->"><!--TMPL_VAR
Name="name"--></option>
+ <!--/TMPL_LOOP -->
+ </select>
+ </p>
+ <!-- /TMPL_IF -->
<table>
<tr>
<th>Order Date</th>
@@ -34,72 +37,74 @@
<th>Basket</th>
<th> </th>
</tr>
- <!-- TMPL_LOOP name="lateorders" -->
- <!--TMPL_IF Name="hilighted" -->
- <tr class="highlight">
- <!--TMPL_ELSE-->
- <tr>
- <!-- /TMPL_IF -->
- <td>
- <!-- TMPL_VAR name="orderdate" -->
- (<!-- TMPL_VAR name="latesince" --> days)
- </td>
- <td>
- <!-- TMPL_VAR name="supplier" -->
- </td>
- <td>
- <b><!-- TMPL_VAR name="title" --></b>
- <!-- TMPL_IF name="author" --><br/><i>Author:</i> <!--
TMPL_VAR NAME="author" --><!-- /TMPL_IF -->
- <!-- TMPL_IF name="publisher" -->
+ <!-- TMPL_LOOP name="lateorders" -->
+ <tr<!-- TMPL_UNLESS NAME="__odd__" --> class="highlight"<!--
/TMPL_UNLESS -->>
+ <td>
+ <!-- TMPL_VAR name="orderdate" -->
+ (<!-- TMPL_VAR name="latesince" --> days)
+ </td>
+ <td>
+ <!-- TMPL_VAR name="supplier" -->
+ </td>
+ <td>
+ <b><!-- TMPL_VAR name="title" --></b>
+ <!-- TMPL_IF name="author" --><br/><i>Author:</i> <!--
TMPL_VAR NAME="author" --><!-- /TMPL_IF -->
+ <!-- TMPL_IF name="publisher" -->
<br/><i>Published by:</i> <!-- TMPL_VAR
NAME="publisher" -->
<!-- TMPL_IF name="publicationyear" -->
<i> in </i><!-- TMPL_VAR name="publicationyear" -->
<!-- /TMPL_IF -->
- <!-- /TMPL_IF -->
- </td>
- <td>
- <!-- TMPL_VAR name="unitpricesupplier" -->x<!-- TMPL_VAR
name="quantity" --> =
- <!-- TMPL_VAR name="subtotal" -->
+ <!-- /TMPL_IF -->
+ </td>
+ <td>
+ <!-- TMPL_VAR name="unitpricesupplier" -->x<!-- TMPL_VAR
name="quantity" --> =
+ <!-- TMPL_VAR name="subtotal" -->
<p title="budget"><!-- TMPL_VAR name="budget" --></p>
- </td>
- <td>
- <p><a href="basket.pl?basketno=<!-- TMPL_VAR
name="basketno" -->" title="basket">
+ </td>
+ <td>
+ <p><a href="basket.pl?basketno=<!-- TMPL_VAR name="basketno"
-->" title="basket">
<!-- TMPL_VAR name="basketno" -->
- </a>
- </p>
- <p title="branch"><!-- TMPL_VAR name="branch" --></p>
- </td>
- <td>
- <input type="checkbox" name="claim_for"
value="<!--TMPL_VAR Name="ordernumber" -->" />
- </td>
- </tr>
+ </a>
+ </p>
+ <p title="branch"><!-- TMPL_VAR name="branch" --></p>
+ </td>
+ <td>
+ <input type="checkbox" name="claim_for" value="<!--TMPL_VAR
Name="ordernumber" -->" />
+ </td>
+ </tr>
<!-- /TMPL_LOOP -->
<tr>
- <th>
- Total
- </th>
- <th colspan="2">
-
- </th>
- <th>
- <!-- TMPL_VAR name="total" -->
- </th>
- <th> </th>
+ <th>Total</th>
+ <th colspan="2"> </th>
+ <th><!-- TMPL_VAR name="total" --></th>
+ <th> </th>
<td>
<input type="submit" value="Claim Order" />
</td>
</tr>
</table>
- </form><!-- TMPL_ELSE --><p>There are no late orders.</p><!--
/TMPL_IF -->
+ </form>
+<!-- TMPL_ELSE --><p>There are no late orders.</p>
+<!-- /TMPL_IF -->
</div>
</div>
</div>
<div class="yui-b">
-<form action="lateorders.pl" method="post">
+<form action="lateorders.pl" method="get">
<fieldset class="brief">
<h4>Filter Results:</h4>
-<ol> <li><label for="delay">Order date:</label><input size="3" maxlength="3"
id="delay" type="text" name="delay" value="<!--TMPL_VAR Name="delay" -->" />
days ago</li>
- <li><label for="supplierid">Vendor:</label><!-- TMPL_VAR
name="CGIsupplier" --></li></ol>
+<!-- TMPL_LOOP NAME="ERROR_LOOP" -->
+<!-- TMPL_IF NAME="delay_digits" --><p class="error">The number of days (<!--
TMPL_VAR NAME="bad_delay" -->) must be a number between 0 and 999.</p><!--
/TMPL_IF -->
+<!-- /TMPL_LOOP -->
+<ol><li><label for="delay">Order date:</label><input size="3" maxlength="3"
id="delay" type="text" name="delay" value="<!--TMPL_VAR Name="delay" -->" />
days ago</li>
+ <li><label for="supplierid">Vendor:</label>
+ <select id="supplierid" size="1" tabindex="" name="supplierid">
+ <option value=""/>
+ <!-- TMPL_LOOP NAME="SUPPLIER_LOOP" -->
+ <option value="<!-- TMPL_VAR NAME='id' -->"<!-- TMPL_IF
NAME="selected" --> selected="selected"<!-- /TMPL_IF -->><!-- TMPL_VAR
NAME="name" --></option>
+ <!-- /TMPL_LOOP -->
+ </select>
+</ol>
<fieldset class="action"><input type="submit" value="filter"
/></fieldset>
</fieldset>
</form>
--
1.5.5.GIT
_______________________________________________
Koha-patches mailing list
[email protected]
http://lists.koha.org/mailman/listinfo/koha-patches
