From 93e312ef08ea5be364227d53f5197b788c6b3984 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 28 Dec 2017 15:15:47 +0000 Subject: [PATCH] Bug 18736: Use rounding syspref to determine correct prices in calculations Content-Type: text/plain; charset=utf-8 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 Signed-off-by: Marcel de Rooy --- C4/Acquisition.pm | 49 ++++++++++++++++++---- C4/Budgets.pm | 32 +++++++++++--- Koha/pdfformat/layout3pages.pm | 7 ++-- Koha/pdfformat/layout3pagesfr.pm | 1 + acqui/basket.pl | 8 ++-- acqui/basketgroup.pl | 10 ++--- acqui/invoice.pl | 14 +++---- acqui/ordered.pl | 3 +- acqui/parcel.pl | 10 ++--- acqui/spent.pl | 2 +- .../bug18736_add_rounding_syspref.perl | 2 +- reports/orders_by_fund.pl | 4 +- 12 files changed, 99 insertions(+), 43 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 1ad7a2361c..9f892e99b2 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -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; diff --git a/C4/Budgets.pm b/C4/Budgets.pm index 6c7e9460e7..4b6cce9a4c 100644 --- a/C4/Budgets.pm +++ b/C4/Budgets.pm @@ -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 @@ -1346,6 +1347,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; diff --git a/Koha/pdfformat/layout3pages.pm b/Koha/pdfformat/layout3pages.pm index d02160ebcd..400a883720 100644 --- a/Koha/pdfformat/layout3pages.pm +++ b/Koha/pdfformat/layout3pages.pm @@ -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; diff --git a/Koha/pdfformat/layout3pagesfr.pm b/Koha/pdfformat/layout3pagesfr.pm index 68e45e85fe..2fdce7a4df 100644 --- a/Koha/pdfformat/layout3pagesfr.pm +++ b/Koha/pdfformat/layout3pagesfr.pm @@ -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; diff --git a/acqui/basket.pl b/acqui/basket.pl index 618c4e383d..e9bf7cd440 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -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}; diff --git a/acqui/basketgroup.pl b/acqui/basketgroup.pl index 4cb1be1478..cc3f9fcc48 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -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; diff --git a/acqui/invoice.pl b/acqui/invoice.pl index ca401d3163..0e3d8214c5 100755 --- a/acqui/invoice.pl +++ b/acqui/invoice.pl @@ -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; diff --git a/acqui/ordered.pl b/acqui/ordered.pl index 7366cca04d..b574d2eb39 100755 --- a/acqui/ordered.pl +++ b/acqui/ordered.pl @@ -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; diff --git a/acqui/parcel.pl b/acqui/parcel.pl index da060030a8..9c16c9ccfe 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -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; diff --git a/acqui/spent.pl b/acqui/spent.pl index 6a47161a90..3801b44a03 100755 --- a/acqui/spent.pl +++ b/acqui/spent.pl @@ -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; diff --git a/installer/data/mysql/atomicupdate/bug18736_add_rounding_syspref.perl b/installer/data/mysql/atomicupdate/bug18736_add_rounding_syspref.perl index 0f3c77f026..1f3e249389 100644 --- a/installer/data/mysql/atomicupdate/bug18736_add_rounding_syspref.perl +++ b/installer/data/mysql/atomicupdate/bug18736_add_rounding_syspref.perl @@ -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"; diff --git a/reports/orders_by_fund.pl b/reports/orders_by_fund.pl index 25b0adeacd..4e797c2d25 100755 --- a/reports/orders_by_fund.pl +++ b/reports/orders_by_fund.pl @@ -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'})); -- 2.11.0