Bug 8037: (follow-up) 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".

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 88783e7..2271653 100644 (file)
--- 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,13 +2459,11 @@ sub GetInvoiceDetails {
 
     my $invoice = $sth->fetchrow_hashref;
 
-    $query = qq{
-        SELECT aqorders.*, biblio.*, aqorders_items.itemnumber,
-        aqbasket.basketname
+    $query = q{
+        SELECT aqorders.*, biblio.*, 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 7ad184d..ad0e51d 100755 (executable)
--- a/acqui/parcel.pl
+++ b/acqui/parcel.pl
@@ -149,8 +149,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;
@@ -161,21 +161,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};
@@ -191,7 +195,7 @@ for my $item ( @parcelitems ) {
 
     if ( $line{parent_ordernumber} != $line{ordernumber} ) {
         if ( grep { $_->{ordernumber} == $line{parent_ordernumber} }
-            @parcelitems
+            @orders
             )
         {
             $line{cannot_cancel} = 1;
@@ -202,7 +206,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 ba4fc86..3bd842e 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt
@@ -361,7 +361,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 %]