[Koha-patches] [PATCH] Bug 10641 - GetBooksellerWithLateOrders in C4::Bookseller.pm has some incoherences
Srdjan
srdjan at catalyst.net.nz
Tue Jul 30 06:18:59 CEST 2013
From: root <root at kenza-VirtualBox>
This patch fixes some incoherences of the routine GetBooksellerWithOrders.
Now it considers the field $estimateddeliverydateto and it replaces it by now() only if it is undef.
More it doesn't test if $aqbookseller.deliverytime is not Null anymore but if $deliverytime = null or undef, it replaces it by 0.
It also verifies if $delay is >= 0 and return undef if it is a negative value.
To Test:
prove t/db_dependent/Bookseller.t
t/db_dependent/Bookseller.t ..
[Some warnings about uninitialized values]
WARNING: GetBooksellerWithLateOrders is called with a negative value at C4/Bookseller.pm line 135.
t/db_dependent/Bookseller.t .. ok
All tests successful.
Signed-off-by: Srdjan <srdjan at catalyst.net.nz>
---
C4/Bookseller.pm | 32 ++++++++++++++++-----------
t/db_dependent/Bookseller.t | 53 +++++++++++++++++++--------------------------
2 files changed, 41 insertions(+), 44 deletions(-)
diff --git a/C4/Bookseller.pm b/C4/Bookseller.pm
index 3e1454f..3bd0b28 100644
--- a/C4/Bookseller.pm
+++ b/C4/Bookseller.pm
@@ -111,10 +111,10 @@ sub GetBooksellersWithLateOrders {
# FIXME NOT quite sure that this operation is valid for DBMs different from Mysql, HOPING so
# should be tested with other DBMs
- my $strsth;
+ my $query;
my @query_params = ();
my $dbdriver = C4::Context->config("db_scheme") || "mysql";
- $strsth = "
+ $query = "
SELECT DISTINCT aqbasket.booksellerid, aqbooksellers.name
FROM aqorders LEFT JOIN aqbasket ON aqorders.basketno=aqbasket.basketno
LEFT JOIN aqbooksellers ON aqbasket.booksellerid = aqbooksellers.id
@@ -128,24 +128,30 @@ sub GetBooksellersWithLateOrders {
AND aqorders.quantity - COALESCE(aqorders.quantityreceived,0) <> 0
AND aqbasket.closedate IS NOT NULL
";
- if ( defined $delay ) {
- $strsth .= " AND (closedate <= DATE_SUB(CAST(now() AS date),INTERVAL ? DAY)) ";
+ if ( defined $delay && $delay >= 0 ) {
+ $query .= " AND (closedate <= DATE_SUB(CAST(now() AS date),INTERVAL ? + COALESCE(aqbooksellers.deliverytime,0) DAY)) ";
push @query_params, $delay;
+ } elsif ( $delay < 0 ){
+ warn 'WARNING: GetBooksellerWithLateOrders is called with a negative value';
+ return;
}
if ( defined $estimateddeliverydatefrom ) {
- $strsth .= '
- AND aqbooksellers.deliverytime IS NOT NULL
- AND ADDDATE(aqbasket.closedate, INTERVAL aqbooksellers.deliverytime DAY) >= ?';
- push @query_params, $estimateddeliverydatefrom;
+ $query .= '
+ AND ADDDATE(aqbasket.closedate, INTERVAL COALESCE(aqbooksellers.deliverytime,0) DAY) >= ?';
+ push @query_params, $estimateddeliverydatefrom;
+ if ( defined $estimateddeliverydateto ) {
+ $query .= ' AND ADDDATE(aqbasket.closedate, INTERVAL COALESCE(aqbooksellers.deliverytime, 0) DAY) <= ?';
+ push @query_params, $estimateddeliverydateto;
+ } else {
+ $query .= ' AND ADDDATE(aqbasket.closedate, INTERVAL COALESCE(aqbooksellers.deliverytime, 0) DAY) <= CAST(now() AS date)';
+ }
}
- if ( defined $estimateddeliverydatefrom and defined $estimateddeliverydateto ) {
- $strsth .= ' AND ADDDATE(aqbasket.closedate, INTERVAL aqbooksellers.deliverytime DAY) <= ?';
+ if ( defined $estimateddeliverydateto ) {
+ $query .= ' AND ADDDATE(aqbasket.closedate, INTERVAL COALESCE(aqbooksellers.deliverytime,0) DAY) <= ?';
push @query_params, $estimateddeliverydateto;
- } elsif ( defined $estimateddeliverydatefrom ) {
- $strsth .= ' AND ADDDATE(aqbasket.closedate, INTERVAL aqbooksellers.deliverytime DAY) <= CAST(now() AS date)';
}
- my $sth = $dbh->prepare($strsth);
+ my $sth = $dbh->prepare($query);
$sth->execute( @query_params );
my %supplierlist;
while ( my ( $id, $name ) = $sth->fetchrow ) {
diff --git a/t/db_dependent/Bookseller.t b/t/db_dependent/Bookseller.t
index 71307f1..0ad91ce 100644
--- a/t/db_dependent/Bookseller.t
+++ b/t/db_dependent/Bookseller.t
@@ -2,7 +2,7 @@
use Modern::Perl;
-use Test::More tests => 53;
+use Test::More tests => 61;
use C4::Context;
use Koha::DateUtils;
use DateTime::Duration;
@@ -476,17 +476,15 @@ isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" )
%suppliers = C4::Bookseller::GetBooksellersWithLateOrders( 4, undef, undef );
isnt( exists( $suppliers{$id_supplier1} ),
1, "Supplier1 has late orders but today > now() - 4 days" );
-#FIXME: If only the field delay is given, it doen't consider the deliverytime
-#isnt( exists( $suppliers{$id_supplier2} ),
-# 1, "Supplier2 has late orders and $daysago5 <= now() - (4 days+2)" );
+isnt( exists( $suppliers{$id_supplier2} ),
+ 1, "Supplier2 has late orders and $daysago5 <= now() - (4 days+2)" );
ok( exists( $suppliers{$id_supplier3} ),
"Supplier3 has late orders and $daysago10 <= now() - (4 days+3)" );
isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" );
#Case 3: With $delay = -1
-#FIXME: GetBooksellersWithLateOrders doesn't test if the delay is a positive value
-#is( C4::Bookseller::GetBooksellersWithLateOrders( -1, undef, undef ),
-# undef, "-1 is a wrong value for a delay" );
+is( C4::Bookseller::GetBooksellersWithLateOrders( -1, undef, undef ),
+ undef, "-1 is a wrong value for a delay" );
#Case 4: With $delay = 0
# today == now-0 -LATE- (if no deliverytime or deliverytime == 0)
@@ -516,9 +514,8 @@ my $daysago4 = output_pref( $dt_today3, 'iso', '24hr', 1 );
%suppliers =
C4::Bookseller::GetBooksellersWithLateOrders( undef, $daysago4, undef );
-#FIXME: if the deliverytime is undef, it doesn't consider the supplier
-#ok( exists( $suppliers{$id_supplier1} ),
-# "Supplier1 has late orders and $today >= $daysago4 -deliverytime undef" );
+ok( exists( $suppliers{$id_supplier1} ),
+ "Supplier1 has late orders and $today >= $daysago4 -deliverytime undef" );
ok( exists( $suppliers{$id_supplier2} ),
"Supplier2 has late orders and $daysago5 + 2 days >= $daysago4 " );
isnt( exists( $suppliers{$id_supplier3} ),
@@ -552,13 +549,11 @@ isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" );
# quantityreceived = quantity -NOT LATE-
%suppliers =
C4::Bookseller::GetBooksellersWithLateOrders( undef, undef, $daysago5 );
-#FIXME: if only the estimateddeliverydatefrom is given, it doesn't consider the parameters,
-#but it replaces it today's date
-#isnt( exists( $suppliers{$id_supplier1} ),
-# 1,
-# "Supplier1 has late orders but $today >= $daysago5 - deliverytime undef" );
-#isnt( exists( $suppliers{$id_supplier2} ),
-# 1, "Supplier2 has late orders but $daysago5 + 2 days > $daysago5 " );
+isnt( exists( $suppliers{$id_supplier1} ),
+ 1,
+ "Supplier1 has late orders but $today >= $daysago5 - deliverytime undef" );
+isnt( exists( $suppliers{$id_supplier2} ),
+ 1, "Supplier2 has late orders but $daysago5 + 2 days > $daysago5 " );
ok( exists( $suppliers{$id_supplier3} ),
"Supplier3 has late orders and $daysago10 + 3 <= $daysago5" );
isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" );
@@ -616,13 +611,11 @@ isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" );
C4::Bookseller::GetBooksellersWithLateOrders( 5, undef, $daysago5 );
isnt( exists( $suppliers{$id_supplier1} ),
1, "Supplier2 has late orders but today > $daysago5 today > now() -5" );
-#FIXME: GetBookSellersWithLateOrders replace estimateddeliverydateto by
-#today's date when no estimateddeliverydatefrom is give
-#isnt(
-# exists( $suppliers{$id_supplier2} ),
-# 1,
-#"Supplier2 has late orders but $daysago5 + 2 days > $daysago5 and $daysago5 > now() - 2+5 days"
-#);
+isnt(
+ exists( $suppliers{$id_supplier2} ),
+ 1,
+"Supplier2 has late orders but $daysago5 + 2 days > $daysago5 and $daysago5 > now() - 2+5 days"
+);
ok(
exists( $suppliers{$id_supplier3} ),
"Supplier2 has late orders and $daysago10 + 3 days <= $daysago5 and $daysago10 <= now() - 3+5 days "
@@ -643,10 +636,9 @@ $basket1info = {
ModBasket($basket1info);
%suppliers = C4::Bookseller::GetBooksellersWithLateOrders( undef, $daysago10,
$daysago10 );
-#FIXME :GetBookSellers doesn't take care if the closedate is ==$estimateddeliverydateto
-# ok( exists( $suppliers{$id_supplier1} ),
-# "Supplier1 has late orders and $daysago10==$daysago10==$daysago10 " )
-# ;
+ok( exists( $suppliers{$id_supplier1} ),
+ "Supplier1 has late orders and $daysago10==$daysago10==$daysago10 " )
+ ;
isnt( exists( $suppliers{$id_supplier2} ),
1,
"Supplier2 has late orders but $daysago10==$daysago10<$daysago5+2" );
@@ -658,9 +650,8 @@ isnt( exists( $suppliers{$id_supplier4} ), 1, "Supplier4 hasnt late orders" );
#Case 12: closedate == $estimateddeliverydatefrom =today-10
%suppliers =
C4::Bookseller::GetBooksellersWithLateOrders( undef, $daysago10, undef );
-#FIXME :GetBookSellers doesn't take care if the closedate is ==$estimateddeliverydateto
-#ok( exists( $suppliers{$id_supplier1} ),
-# "Supplier1 has late orders and $daysago10==$daysago10 " );
+ok( exists( $suppliers{$id_supplier1} ),
+ "Supplier1 has late orders and $daysago10==$daysago10 " );
#Case 13: closedate == $estimateddeliverydateto =today-10
%suppliers =
--
1.8.1.2
More information about the Koha-patches
mailing list