Bug 18736: Use rounding syspref to determine correct prices in calculations
authorNick Clemens <nick@bywatersolutions.com>
Thu, 28 Dec 2017 15:15:47 +0000 (15:15 +0000)
committerNick Clemens <nick@bywatersolutions.com>
Thu, 21 Mar 2019 16:27:09 +0000 (16:27 +0000)
To test:
Place an order (no tax just for simplicity)
 listprice/rrp = 16.99
 discount = 42%
 quantity = 8
 estimated calculated at 9.85
 but order total is 78.83, but 8 times 9.85 = 78.80
Apply patches, set OrderPriceRounding syspref to 'Nearest cent'
Not order total is now as expected
View ordered.pl and confirm values are correct
Complete order, view invoice and confirm values
View spent.pl and confirm values
Go through acquisitions module and confirm prices throughout are
correct.

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

12 files changed:
C4/Acquisition.pm
C4/Budgets.pm
Koha/pdfformat/layout3pages.pm
Koha/pdfformat/layout3pagesfr.pm
acqui/basket.pl
acqui/basketgroup.pl
acqui/invoice.pl
acqui/ordered.pl
acqui/parcel.pl
acqui/spent.pl
installer/data/mysql/atomicupdate/bug18736_add_rounding_syspref.perl
reports/orders_by_fund.pl

index 1ad7a23..9f892e9 100644 (file)
@@ -93,6 +93,8 @@ BEGIN {
         &NotifyOrderUsers
 
         &FillWithDefaultValues
+
+        &get_rounded_price
     );
 }
 
@@ -1461,8 +1463,8 @@ sub ModReceiveOrder {
         $dbh->do(q|
             UPDATE aqorders
             SET
-                tax_value_on_ordering = quantity * ecost_tax_excluded * tax_rate_on_ordering,
-                tax_value_on_receiving = quantity * unitprice_tax_excluded * tax_rate_on_receiving
+                tax_value_on_ordering = quantity * | . _get_rounding_sql(q|ecost_tax_excluded|) . q| * tax_rate_on_ordering,
+                tax_value_on_receiving = quantity * | . _get_rounding_sql(q|unitprice_tax_excluded|) . q| * tax_rate_on_receiving
             WHERE ordernumber = ?
         |, undef, $order->{ordernumber});
 
@@ -1474,8 +1476,8 @@ sub ModReceiveOrder {
         $order->{tax_rate_on_ordering} //= 0;
         $order->{unitprice_tax_excluded} //= 0;
         $order->{tax_rate_on_receiving} //= 0;
-        $order->{tax_value_on_ordering} = $order->{quantity} * $order->{ecost_tax_excluded} * $order->{tax_rate_on_ordering};
-        $order->{tax_value_on_receiving} = $order->{quantity} * $order->{unitprice_tax_excluded} * $order->{tax_rate_on_receiving};
+        $order->{tax_value_on_ordering} = $order->{quantity} * get_rounded_price($order->{ecost_tax_excluded}) * $order->{tax_rate_on_ordering};
+        $order->{tax_value_on_receiving} = $order->{quantity} * get_rounded_price($order->{unitprice_tax_excluded}) * $order->{tax_rate_on_receiving};
         $order->{datereceived} = $datereceived;
         $order->{invoiceid} = $invoice->{invoiceid};
         $order->{orderstatus} = 'complete';
@@ -1645,8 +1647,8 @@ sub CancelReceipt {
         $dbh->do(q|
             UPDATE aqorders
             SET
-                tax_value_on_ordering = quantity * ecost_tax_excluded * tax_rate_on_ordering,
-                tax_value_on_receiving = quantity * unitprice_tax_excluded * tax_rate_on_receiving
+                tax_value_on_ordering = quantity * | . _get_rounding_sql(q|ecost_tax_excluded|) . q| * tax_rate_on_ordering,
+                tax_value_on_receiving = quantity * | . _get_rounding_sql(q|unitprice_tax_excluded|) . q| * tax_rate_on_receiving
             WHERE ordernumber = ?
         |, undef, $parent_ordernumber);
 
@@ -1998,6 +2000,37 @@ sub TransferOrder {
     return $newordernumber;
 }
 
+=head3 _get_rounding_sql
+
+    $rounding_sql = _get_rounding_sql("mysql_variable_to_round_string");
+
+returns the correct SQL routine based on OrderPriceRounding system preference.
+
+=cut
+
+sub _get_rounding_sql {
+    my ( $round_string ) = @_;
+    my $rounding_pref = C4::Context->preference('OrderPriceRounding');
+    if ( $rounding_pref eq "nearest_cent"  ) { return ("CAST($round_string*100 AS INTEGER)/100"); }
+    else                                     { return ("$round_string"); }
+}
+
+=head3 get_rounded_price
+
+    $rounded_price = get_rounded_price( $price );
+
+returns a price rounded as specified in OrderPriceRounding system preference.
+
+=cut
+
+sub get_rounded_price {
+    my ( $price ) =  @_;
+    my $rounding_pref = C4::Context->preference('OrderPriceRounding');
+    if( $rounding_pref eq 'nearest_cent' ) { return Koha::Number::Price->new( $price )->format(); }
+    else                                   { return $price; }
+}
+
+
 =head2 FUNCTIONS ABOUT PARCELS
 
 =head3 GetParcels
@@ -3012,7 +3045,7 @@ sub populate_order_with_prices {
 
         # tax value = quantity * ecost tax excluded * tax rate
         $order->{tax_value_on_ordering} =
-            $order->{quantity} * $order->{ecost_tax_excluded} * $order->{tax_rate_on_ordering};
+            $order->{quantity} * get_rounded_price($order->{ecost_tax_excluded}) * $order->{tax_rate_on_ordering};
     }
 
     if ($receiving) {
@@ -3046,7 +3079,7 @@ sub populate_order_with_prices {
         }
 
         # tax value = quantity * unit price tax excluded * tax rate
-        $order->{tax_value_on_receiving} = $order->{quantity} * $order->{unitprice_tax_excluded} * $order->{tax_rate_on_receiving};
+        $order->{tax_value_on_receiving} = $order->{quantity} * get_rounded_price($order->{unitprice_tax_excluded}) * $order->{tax_rate_on_receiving};
     }
 
     return $order;
index 620daf6..39bec9e 100644 (file)
@@ -211,11 +211,12 @@ sub GetBudgetsPlanCell {
     my ( $cell, $period, $budget ) = @_;
     my ($actual, $sth);
     my $dbh = C4::Context->dbh;
+    my $roundsql = _get_rounding_sql(qq|ecost_tax_included|);
     if ( $cell->{'authcat'} eq 'MONTHS' ) {
         # get the actual amount
         $sth = $dbh->prepare( qq|
 
-            SELECT SUM(ecost_tax_included) AS actual FROM aqorders
+            SELECT SUM(| .  $roundsql . qq|) AS actual FROM aqorders
                 WHERE    budget_id = ? AND
                 entrydate like "$cell->{'authvalue'}%"  |
         );
@@ -224,7 +225,7 @@ sub GetBudgetsPlanCell {
         # get the actual amount
         $sth = $dbh->prepare( qq|
 
-            SELECT SUM(ecost_tax_included) FROM aqorders
+            SELECT SUM(| . $roundsql . qq|) FROM aqorders
                 LEFT JOIN aqorders_items
                 ON (aqorders.ordernumber = aqorders_items.ordernumber)
                 LEFT JOIN items
@@ -236,7 +237,7 @@ sub GetBudgetsPlanCell {
         # get the actual amount
         $sth = $dbh->prepare(  qq|
 
-            SELECT SUM( ecost_tax_included *  quantity) AS actual
+            SELECT SUM( | . $roundsql . qq| *  quantity) AS actual
                 FROM aqorders JOIN biblioitems
                 ON (biblioitems.biblionumber = aqorders.biblionumber )
                 WHERE aqorders.budget_id = ? and itemtype  = ? |
@@ -249,7 +250,7 @@ sub GetBudgetsPlanCell {
         # get the actual amount
         $sth = $dbh->prepare( qq|
 
-        SELECT  SUM(ecost_tax_included * quantity) AS actual
+        SELECT  SUM(| . $roundsql . qq| * quantity) AS actual
             FROM aqorders
             JOIN aqbudgets ON (aqbudgets.budget_id = aqorders.budget_id )
             WHERE  aqorders.budget_id = ? AND
@@ -333,7 +334,7 @@ sub GetBudgetSpent {
     # unitprice_tax_included should always been set here
     # we should not need to retrieve ecost_tax_included
     my $sth = $dbh->prepare(qq|
-        SELECT SUM( COALESCE(unitprice_tax_included, ecost_tax_included) * quantity ) AS sum FROM aqorders
+        SELECT SUM( | . _get_rounding_sql("COALESCE(unitprice_tax_included, ecost_tax_included)") . qq| * quantity ) AS sum FROM aqorders
             WHERE budget_id = ? AND
             quantityreceived > 0 AND
             datecancellationprinted IS NULL
@@ -364,7 +365,7 @@ sub GetBudgetOrdered {
        my ($budget_id) = @_;
        my $dbh = C4::Context->dbh;
        my $sth = $dbh->prepare(qq|
-        SELECT SUM(ecost_tax_included *  quantity) AS sum FROM aqorders
+        SELECT SUM(| . _get_rounding_sql(qq|ecost_tax_included|) . qq| *  quantity) AS sum FROM aqorders
             WHERE budget_id = ? AND
             quantityreceived = 0 AND
             datecancellationprinted IS NULL
@@ -1364,6 +1365,25 @@ sub MoveOrders {
     return \@report;
 }
 
+=head1 INTERNAL FUNCTIONS
+
+=cut
+
+=head3 _get_rounding_sql
+
+    $rounding_sql = _get_rounding_sql("mysql_variable_to_round_string");
+
+returns the correct SQL routine based on OrderPriceRounding system preference.
+
+=cut
+
+sub _get_rounding_sql {
+    my $to_round = shift;
+    my $rounding_pref = C4::Context->preference('OrderPriceRounding');
+    if   ($rounding_pref eq 'nearest_cent') { return "CAST($to_round*100 AS INTEGER)/100"; }
+    else { return "$to_round"; }
+}
+
 END { }    # module clean-up code here (global destructor)
 
 1;
index d02160e..400a883 100644 (file)
@@ -27,6 +27,7 @@ use List::MoreUtils qw/uniq/;
 use Modern::Perl;
 use utf8;
 
+use C4::Acquisition;
 use Koha::Number::Price;
 use Koha::DateUtils;
 use Koha::Libraries;
@@ -220,9 +221,9 @@ sub printbaskets {
             $total_tax_excluded += $ord->{total_tax_excluded};
             $total_tax_included += $ord->{total_tax_included};
             $totaltax_value += $ord->{tax_value};
-            $totaldiscount += ($ord->{rrp_tax_excluded} - $ord->{ecost_tax_excluded} ) * $ord->{quantity};
-            $total_rrp_tax_excluded += $ord->{rrp_tax_excluded} * $ord->{quantity};
-            $total_rrp_tax_included += $ord->{rrp_tax_included} * $ord->{quantity};
+            $totaldiscount += (get_rounded_price($ord->{rrp_tax_excluded}) - get_rounded_price($ord->{ecost_tax_excluded}) ) * $ord->{quantity};
+            $total_rrp_tax_excluded += get_rounded_price($ord->{rrp_tax_excluded}) * $ord->{quantity};
+            $total_rrp_tax_included += get_rounded_price($ord->{rrp_tax_included}) * $ord->{quantity};
             push @gst, $ord->{tax_rate};
         }
         @gst = uniq map { $_ * 100 } @gst;
index 68e45e8..2fdce7a 100644 (file)
@@ -27,6 +27,7 @@ use List::MoreUtils qw/uniq/;
 use Modern::Perl;
 use utf8;
 
+use C4::Acquisition;
 use Koha::Number::Price;
 use Koha::DateUtils;
 use Koha::Libraries;
index 618c4e3..e9bf7cd 100755 (executable)
@@ -350,9 +350,9 @@ if ( $op eq 'list' ) {
         push @books_loop, $line;
 
         $foot{$$line{tax_rate}}{tax_rate} = $$line{tax_rate};
-        $foot{$$line{tax_rate}}{tax_value} += $$line{tax_value};
+        $foot{$$line{tax_rate}}{tax_value} += get_rounded_price($$line{tax_value});
         $total_tax_value += $$line{tax_value};
-        $foot{$$line{tax_rate}}{quantity}  += $$line{quantity};
+        $foot{$$line{tax_rate}}{quantity}  += get_rounded_price($$line{quantity});
         $total_quantity += $$line{quantity};
         $foot{$$line{tax_rate}}{total_tax_excluded} += $$line{total_tax_excluded};
         $total_tax_excluded += $$line{total_tax_excluded};
@@ -459,8 +459,8 @@ sub get_order_infos {
     $line{basketno}       = $basketno;
     $line{budget_name}    = $budget->{budget_name};
 
-    $line{total_tax_included} = $line{ecost_tax_included} * $line{quantity};
-    $line{total_tax_excluded} = $line{ecost_tax_excluded} * $line{quantity};
+    $line{total_tax_included} = get_rounded_price($line{ecost_tax_included}) * $line{quantity};
+    $line{total_tax_excluded} = get_rounded_price($line{ecost_tax_excluded}) * $line{quantity};
     $line{tax_value} = $line{tax_value_on_ordering};
     $line{tax_rate} = $line{tax_rate_on_ordering};
 
index 4cb1be1..cc3f9fc 100755 (executable)
@@ -51,7 +51,7 @@ use C4::Output;
 use CGI qw ( -utf8 );
 use File::Spec;
 
-use C4::Acquisition qw/CloseBasketgroup ReOpenBasketgroup GetOrders GetBasketsByBasketgroup GetBasketsByBookseller ModBasketgroup NewBasketgroup DelBasketgroup GetBasketgroups ModBasket GetBasketgroup GetBasket GetBasketGroupAsCSV/;
+use C4::Acquisition qw/CloseBasketgroup ReOpenBasketgroup GetOrders GetBasketsByBasketgroup GetBasketsByBookseller ModBasketgroup NewBasketgroup DelBasketgroup GetBasketgroups ModBasket GetBasketgroup GetBasket GetBasketGroupAsCSV get_rounded_price/;
 use Koha::EDI qw/create_edi_order get_edifact_ean/;
 
 use Koha::Biblioitems;
@@ -78,9 +78,9 @@ sub BasketTotal {
     for my $order (@orders){
         # FIXME The following is wrong
         if ( $bookseller->listincgst ) {
-            $total = $total + ( $order->{ecost_tax_included} * $order->{quantity} );
+            $total = $total + ( get_rounded_price($order->{ecost_tax_included}) * $order->{quantity} );
         } else {
-            $total = $total + ( $order->{ecost_tax_excluded} * $order->{quantity} );
+            $total = $total + ( get_rounded_price($order->{ecost_tax_excluded}) * $order->{quantity} );
         }
     }
     return $total;
@@ -169,8 +169,8 @@ sub printbasketgrouppdf{
 
             $ord->{tax_value} = $ord->{tax_value_on_ordering};
             $ord->{tax_rate} = $ord->{tax_rate_on_ordering};
-            $ord->{total_tax_included} = $ord->{ecost_tax_included} * $ord->{quantity};
-            $ord->{total_tax_excluded} = $ord->{ecost_tax_excluded} * $ord->{quantity};
+            $ord->{total_tax_included} = get_rounded_price($ord->{ecost_tax_included}) * $ord->{quantity};
+            $ord->{total_tax_excluded} = get_rounded_price($ord->{ecost_tax_excluded}) * $ord->{quantity};
 
             my $biblioitem = Koha::Biblioitems->search({ biblionumber => $ord->{biblionumber} })->next;
 
index ca401d3..0e3d821 100755 (executable)
@@ -165,21 +165,21 @@ my $total_tax_value = 0;
 foreach my $order (@$orders) {
     my $line = get_infos( $order, $bookseller);
 
-    $line->{total_tax_excluded} = $line->{unitprice_tax_excluded} * $line->{quantity};
-    $line->{total_tax_included} = $line->{unitprice_tax_included} * $line->{quantity};
+    $line->{total_tax_excluded} = get_rounded_price($line->{unitprice_tax_excluded}) * $line->{quantity};
+    $line->{total_tax_included} = get_rounded_price($line->{unitprice_tax_included}) * $line->{quantity};
 
     $line->{tax_value} = $line->{tax_value_on_receiving};
     $line->{tax_rate} = $line->{tax_rate_on_receiving};
 
     $foot{$$line{tax_rate}}{tax_rate} = $$line{tax_rate};
-    $foot{$$line{tax_rate}}{tax_value} += $$line{tax_value};
+    $foot{$$line{tax_rate}}{tax_value} += get_rounded_price($$line{tax_value});
     $total_tax_value += $$line{tax_value};
     $foot{$$line{tax_rate}}{quantity}  += $$line{quantity};
     $total_quantity += $$line{quantity};
-    $foot{$$line{tax_rate}}{total_tax_excluded} += $$line{total_tax_excluded};
-    $total_tax_excluded += $$line{total_tax_excluded};
-    $foot{$$line{tax_rate}}{total_tax_included} += $$line{total_tax_included};
-    $total_tax_included += $$line{total_tax_included};
+    $foot{$$line{tax_rate}}{total_tax_excluded} += get_rounded_price($$line{total_tax_excluded});
+    $total_tax_excluded += get_rounded_price($$line{total_tax_excluded});
+    $foot{$$line{tax_rate}}{total_tax_included} += get_rounded_price($$line{total_tax_included});
+    $total_tax_included += get_rounded_price($$line{total_tax_included});
 
     $line->{orderline} = $line->{parent_ordernumber};
     push @orders_loop, $line;
index 7366cca..b574d2e 100755 (executable)
@@ -33,6 +33,7 @@ use CGI qw ( -utf8 );
 use C4::Auth;
 use C4::Output;
 use Koha::Acquisition::Invoice::Adjustments;
+use C4::Acquisition;
 
 my $dbh     = C4::Context->dbh;
 my $input   = new CGI;
@@ -98,7 +99,7 @@ while ( my $data = $sth->fetchrow_hashref ) {
         $left = $data->{'quantity'};
     }
     if ( $left && $left > 0 ) {
-        my $subtotal = $left * $data->{'ecost_tax_included'};
+        my $subtotal = $left * get_rounded_price($data->{'ecost_tax_included'});
         $data->{subtotal} = sprintf( "%.2f", $subtotal );
         $data->{'left'} = $left;
         push @ordered, $data;
index da06003..9c16c9c 100755 (executable)
@@ -136,7 +136,7 @@ for my $order ( @orders ) {
         $order->{unitprice} = $order->{unitprice_tax_excluded};
     }
 
-    $order->{total} = $order->{unitprice} * $order->{quantity};
+    $order->{total} = get_rounded_price($order->{unitprice}) * $order->{quantity};
 
     my %line = %{ $order };
     $line{invoice} = $invoice->{invoicenumber};
@@ -154,8 +154,8 @@ for my $order ( @orders ) {
     $line{tax_rate} = $line{tax_rate_on_receiving};
     $foot{$line{tax_rate}}{tax_rate} = $line{tax_rate};
     $foot{$line{tax_rate}}{tax_value} += $line{tax_value};
-    $total_tax_excluded += $line{unitprice_tax_excluded} * $line{quantity};
-    $total_tax_included += $line{unitprice_tax_included} * $line{quantity};
+    $total_tax_excluded += get_rounded_price($line{unitprice_tax_excluded}) * $line{quantity};
+    $total_tax_included += get_rounded_price($line{unitprice_tax_included}) * $line{quantity};
 
     my $suggestion   = GetSuggestionInfoFromBiblionumber($line{biblionumber});
     $line{suggestionid}         = $suggestion->{suggestionid};
@@ -174,7 +174,7 @@ for my $order ( @orders ) {
     my $budget_name = GetBudgetName( $line{budget_id} );
     $line{budget_name} = $budget_name;
 
-    $subtotal_for_funds->{ $line{budget_name} }{ecost} += $order->{ecost} * $order->{quantity};
+    $subtotal_for_funds->{ $line{budget_name} }{ecost} += get_rounded_price($order->{ecost}) * $order->{quantity};
     $subtotal_for_funds->{ $line{budget_name} }{unitprice} += $order->{total};
 
     push @loop_received, \%line;
@@ -232,7 +232,7 @@ unless( defined $invoice->{closedate} ) {
         } else {
             $order->{ecost} = $order->{ecost_tax_excluded};
         }
-        $order->{total} = $order->{ecost} * $order->{quantity};
+        $order->{total} = get_rounded_price($order->{ecost}) * $order->{quantity};
 
         my %line = %$order;
 
index 6a47161..3801b44 100755 (executable)
@@ -107,7 +107,7 @@ my @spent;
 while ( my $data = $sth->fetchrow_hashref ) {
     my $recv = $data->{'quantityreceived'};
     if ( $recv > 0 ) {
-        my $rowtotal = $recv * $data->{'unitprice_tax_included'};
+        my $rowtotal = $recv * get_rounded_price($data->{'unitprice_tax_included'});
         $data->{'rowtotal'}  = sprintf( "%.2f", $rowtotal );
         $data->{'unitprice_tax_included'} = sprintf( "%.2f", $data->{'unitprice_tax_included'} );
         $subtotal += $rowtotal;
index 0f3c77f..1f3e249 100644 (file)
@@ -1,6 +1,6 @@
 $DBversion = 'XXX';  # will be replaced by the RM
 if( CheckVersion( $DBversion ) ) {
-    # $dbh->do( "INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES ('OrderPriceRounding',NULL,'Local preference for rounding orders before calculations to ensure correct calculations','|nearest_cent','Choice')" );
+    $dbh->do( "INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('OrderPriceRounding',NULL,'Local preference for rounding orders before calculations to ensure correct calculations','|nearest_cent','Choice')" );
 
     SetVersion( $DBversion );
     print "Upgrade to $DBversion done (Bug 18736 - Add syspref to control order rounding)\n";
index 25b0ade..4e797c2 100755 (executable)
@@ -107,8 +107,8 @@ if ( $get_orders ) {
         $order->{title} = $biblio ? $biblio->title : '';
         $order->{title} ||= $order->{biblionumber};
 
-        $order->{'total_rrp'} = $order->{'quantity'} * $order->{'rrp'};
-        $order->{'total_ecost'} = $order->{'quantity'} * $order->{'ecost'};
+        $order->{'total_rrp'} = get_rounded_price($order->{'quantity'}) * $order->{'rrp'};
+        $order->{'total_ecost'} = get_rounded_price($order->{'quantity'}) * $order->{'ecost'};
 
         # Format the dates and currencies correctly
         $order->{'datereceived'} = output_pref(dt_from_string($order->{'datereceived'}));