From 8f2421acc9d95f9833d7e891b6355be17058c9db Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Fri, 25 Oct 2013 14:27:10 +0200
Subject: [PATCH] Bug 8037: FIX bad sql query and variable renaming

The first patch does a left join on aqorders_items which returns too
much order lines.

This patch follows the Galen's suggestion: it removes the join and calls
the GetItemnumbersFromOrder routine for retrieving itemnumbers.

Bonus: the "parcelitems" variable is badly named and obfuscates the code.
I changed it for "orders".
---
 C4/Acquisition.pm                                  |    8 ++----
 acqui/parcel.pl                                    |   30 +++++++++++---------
 .../intranet-tmpl/prog/en/modules/acqui/parcel.tt  |    8 +++++-
 3 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 29cc872..dffeef2 100644
--- a/C4/Acquisition.pm
+++ b/C4/Acquisition.pm
@@ -2448,7 +2448,7 @@ sub GetInvoiceDetails {
     }
 
     my $dbh = C4::Context->dbh;
-    my $query = qq{
+    my $query = q{
         SELECT aqinvoices.*, aqbooksellers.name AS suppliername
         FROM aqinvoices
           LEFT JOIN aqbooksellers ON aqinvoices.booksellerid = aqbooksellers.id
@@ -2459,14 +2459,12 @@ sub GetInvoiceDetails {
 
     my $invoice = $sth->fetchrow_hashref;
 
-    $query = qq{
+    $query = q{
         SELECT aqorders.*, biblio.*,
-        aqbasket.basketname,
-        aqorders_items.itemnumber
+        aqbasket.basketname
         FROM aqorders
           LEFT JOIN aqbasket ON aqorders.basketno = aqbasket.basketno
           LEFT JOIN biblio ON aqorders.biblionumber = biblio.biblionumber
-          LEFT JOIN aqorders_items ON aqorders.ordernumber = aqorders_items.ordernumber
         WHERE invoiceid = ?
     };
     $sth = $dbh->prepare($query);
diff --git a/acqui/parcel.pl b/acqui/parcel.pl
index d1acce5..d26516d 100755
--- a/acqui/parcel.pl
+++ b/acqui/parcel.pl
@@ -147,8 +147,8 @@ my $gst = $bookseller->{gstrate} // C4::Context->preference("gist") // 0;
 my $datereceived = C4::Dates->new();
 
 my $cfstr         = "%.2f";                                                           # currency format string -- could get this from currency table.
-my @parcelitems   = @{ $invoice->{orders} };
-my $countlines    = scalar @parcelitems;
+my @orders        = @{ $invoice->{orders} };
+my $countlines    = scalar @orders;
 my $totalprice    = 0;
 my $totalquantity = 0;
 my $total;
@@ -159,21 +159,25 @@ my $total_quantity = 0;
 my $total_gste = 0;
 my $total_gsti = 0;
 
-for my $item ( @parcelitems ) {
-    $item->{unitprice} = get_value_with_gst_params( $item->{unitprice}, $item->{gstrate}, $bookseller );
-    $total = ( $item->{'unitprice'} ) * $item->{'quantityreceived'};
-    $item->{'unitprice'} += 0;
-    my %line = %{ $item };
+for my $order ( @orders ) {
+    $order->{unitprice} = get_value_with_gst_params( $order->{unitprice}, $order->{gstrate}, $bookseller );
+    $total = ( $order->{unitprice} ) * $order->{quantityreceived};
+    $order->{'unitprice'} += 0;
+    my %line = %{ $order };
     my $ecost = get_value_with_gst_params( $line{ecost}, $line{gstrate}, $bookseller );
     $line{ecost} = sprintf( "%.2f", $ecost );
     $line{invoice} = $invoice->{invoicenumber};
     $line{total} = sprintf($cfstr, $total);
     $line{booksellerid} = $invoice->{booksellerid};
-    my ($count) = &GetReservesFromBiblionumber($line{biblionumber},undef,$item->{itemnumber});
-    $line{holds} = $count;
+    $line{holds} = 0;
+    my @itemnumbers = GetItemnumbersFromOrder( $order->{ordernumber} );
+    for my $itemnumber ( @itemnumbers ) {
+        my ( $count ) = &GetReservesFromBiblionumber($line{biblionumber}, undef, $itemnumber);
+        $line{holds} += $count;
+    }
     $line{budget} = GetBudgetByOrderNumber( $line{ordernumber} );
-    $totalprice += $item->{'unitprice'};
-    $line{unitprice} = sprintf( $cfstr, $item->{'unitprice'} );
+    $totalprice += $order->{unitprice};
+    $line{unitprice} = sprintf( $cfstr, $order->{unitprice} );
     my $gste = get_gste( $line{total}, $line{gstrate}, $bookseller );
     my $gst = get_gst( $line{total}, $line{gstrate}, $bookseller );
     $foot{$line{gstrate}}{gstrate} = $line{gstrate};
@@ -189,7 +193,7 @@ for my $item ( @parcelitems ) {
 
     if ( $line{parent_ordernumber} != $line{ordernumber} ) {
         if ( grep { $_->{ordernumber} == $line{parent_ordernumber} }
-            @parcelitems
+            @orders
             )
         {
             $line{cannot_cancel} = 1;
@@ -200,7 +204,7 @@ for my $item ( @parcelitems ) {
     $line{budget_name} = $budget->{'budget_name'};
 
     push @loop_received, \%line;
-    $totalquantity += $item->{'quantityreceived'};
+    $totalquantity += $order->{quantityreceived};
 
 }
 push @book_foot_loop, map { $_ } values %foot;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
index e540fd7..248ed18 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
@@ -330,7 +330,13 @@
             <tr>
                 <td>[% loop_receive.basketname %] (<a href="/cgi-bin/koha/acqui/basket.pl?basketno=[% loop_receive.basketno %]">[% loop_receive.basketno %]</a>)</td>
                 <td><a href="neworderempty.pl?ordernumber=[% loop_receive.ordernumber %]&amp;booksellerid=[% booksellerid %]">[% loop_receive.ordernumber %]</a></td>
-                <td>[% IF loop_receive.holds %]<span class="error"><a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% loop_receive.biblionumber %]">[% loop_receive.holds %]</a></span>[% END %]</td>
+                <td>
+                  [% IF loop_receive.holds > 0 %]
+                    <span class="error"><a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% loop_receive.biblionumber %]">[% loop_receive.holds %]</a></span>
+                  [% ELSE %]
+                    0
+                  [% END %]
+                </td>
                 <td><a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loop_receive.biblionumber %]">[% loop_receive.title |html %]</a>
                 [% IF ( loop_receive.author ) %] / [% loop_receive.author %][% END %]
                 [% IF ( loop_receive.isbn ) %] - [% loop_receive.isbn %][% END %]
-- 
1.7.10.4