Attachment 22390 Details for Bug 8037 – Bug 8037: FIX bad sql query and variable renaming

[patch] Bug 8037: FIX bad sql query and variable renaming

Bug-8037-FIX-bad-sql-query-and-variable-renaming.patch (text/plain), 6.24 KB, created by Jonathan Druart on 2013-10-25 12:32:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2013-10-25 12:32:18 UTC

Size: 6.24 KB

patch

obsolete

>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

Actions: View | Diff | Splinter Review

Attachments on bug 8037: 9383 | 9384 | 11089 | 13152 | 13153 | 14892 | 16336 | 16635 | 16651 | 16660 | 16749 | 18021 | 18022 | 18023 | 18693 | 19672 | 19789 | 19790 | 19791 | 22244 | 22390 | 22636 | 22637 | 22638 | 22639 | 22647