From 322fd538d0a75390f3ecff0a87d6fcbf337bd3a1 Mon Sep 17 00:00:00 2001 From: Joe Atzberger Date: Fri, 18 Jul 2008 13:12:26 -0500 Subject: [PATCH] Refine lateorders - error feedback, filter independence 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. Signed-off-by: Joshua Ferraro --- 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 1df612165d..a599fdd216 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 d3e63c7cc5..332d3bc477 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 7c7a766456..222b233634 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 => \@select_supplier, - -default => $supplierid, - -id => 'supplierid', - -labels => \%supplierlist, - -size => 1, - -tabindex=>'', - -multiple => 0 ); - +$template->param(SUPPLIER_LOOP => \@sloopy); $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=>\@letters) if (@letters); -my $op=$input->param("op"); -if ($op eq "send_alert"){ - my @ordernums=$input->param("claim_for"); - SendAlerts('claimacquisition',\@ordernums,$input->param("letter_code")); + +if ($op and $op eq "send_alert"){ + my @ordernums = $input->param("claim_for"); # FIXME: Fallback values? + SendAlerts('claimacquisition',\@ordernums,$input->param("letter_code")); # FIXME: Fallback value? } -$template->param(delay=>$delay) if ($delay); +$template->param(ERROR_LOOP => \@errors) if (@errors); $template->param( - CGIsupplier => $CGIsupplier, lateorders => \@lateorders, - 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 5b0deeb9f8..16788baf34 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 @@ - +
@@ -17,14 +17,17 @@

: Late orders

-
- -

-

+ + + + +

+

+ @@ -34,72 +37,74 @@ - - - - - - - - - + + - + - + - - + +

+

+ + + - - - - + + + +
Order DateBasket  
- - ( days) - - - - -
Author: - + + class="highlight"> + 		+ + ( days) + + + + +
Author: +
Published by: in - - 		- x = - + + + x = +

- 		-

" title="basket"> +

+

" title="basket"> - -

-

- 		- " /> -
+ " /> +
- Total - -   - - -  Total  
-

There are no late orders.

+ +

There are no late orders.

+
-
+

Filter Results:

-
  1. " /> days ago
    2. -
+ +

The number of days () must be a number between 0 and 999.

+ +
  1. " /> days ago
    2. +
  2. + +
-- 2.39.5