Bugzilla – Attachment 20628 Details for
Bug 10641
GetBooksellerWithLateOrders in C4::Bookseller.pm has some incoherences
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
Bug 10641 - GetBooksellerWithLateOrders in C4::Bookseller.pm has some incoherences
Bug-10641---GetBooksellerWithLateOrders-in-C4Books.patch (text/plain), 10.02 KB, created by
kenza
on 2013-08-26 14:54:56 UTC
(
hide
)
Description:
Bug 10641 - GetBooksellerWithLateOrders in C4::Bookseller.pm has some incoherences
Filename:
MIME Type:
Creator:
kenza
Created:
2013-08-26 14:54:56 UTC
Size:
10.02 KB
patch
obsolete
>From 92363084236f905c621343078eb2dac4cea34807 Mon Sep 17 00:00:00 2001 >From: root <root@kenza-VirtualBox> >Date: Thu, 25 Jul 2013 10:49:21 +0200 >Subject: [PATCH] Bug 10641 - GetBooksellerWithLateOrders in C4::Bookseller.pm > has some incoherences > >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: >Before, this routine sorts out the BookSellerWithLateOrders. If a Bookseller did not specify a deliverytime, he would never appears in the list of LateOrders.More if the field "Estimated delivery date to" was specified, it didn't take care of the value and it returns the late order up to today's date. >Now, the returned list considers all the fields give and if the deliverytime of the Bookseller is not specified, it calculates the late orders as if the deliverytime is 0.By default , all booksellers which have orders in late until today are listed unless "estimated delivery date to" is specified. > >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@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.7.9.5
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=20628">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 10641
:
19942
|
19975
|
19984
|
20628
|
20629
|
20662