From 140d79c541510383d1ee7f43c4d2fd449bf2214e 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 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 --- C4/Acquisition.pm | 49 ++++++++++++++++--- C4/Budgets.pm | 32 +++++++++--- acqui/basket.pl | 8 +-- acqui/basketgroup.pl | 10 ++-- acqui/invoice.pl | 14 +++--- acqui/ordered.pl | 3 +- acqui/parcel.pl | 10 ++-- acqui/pdfformat/layout3pages.pm | 7 +-- acqui/pdfformat/layout3pagesfr.pm | 1 + 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 384b598e43..3101ed3bb9 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -93,6 +93,8 @@ BEGIN { &NotifyOrderUsers &FillWithDefaultValues + + &get_rounded_price ); } @@ -1472,8 +1474,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}); @@ -1485,8 +1487,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'; @@ -1656,8 +1658,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); @@ -2008,6 +2010,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 @@ -3008,7 +3041,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) { @@ -3042,7 +3075,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 caacd7cd9c..970700501a 100644 --- a/C4/Budgets.pm +++ b/C4/Budgets.pm @@ -210,11 +210,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'}%" | ); @@ -223,7 +224,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 @@ -235,7 +236,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 = ? | @@ -248,7 +249,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 @@ -332,7 +333,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 @@ -363,7 +364,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 @@ -1341,6 +1342,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/acqui/basket.pl b/acqui/basket.pl index 9f2ffdf7e1..46d4b3ddbf 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -346,9 +346,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}; @@ -451,8 +451,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 78514ba2ac..da94e2eb9a 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -50,7 +50,7 @@ use C4::Auth; use C4::Output; use CGI qw ( -utf8 ); -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; @@ -77,9 +77,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; @@ -170,8 +170,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 bdf9485a87..68df666485 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 e39349092e..d5fac7e39e 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; @@ -89,7 +90,7 @@ while ( my $data = $sth->fetchrow_hashref ) { $left = $data->{'quantity'}; } if ( $left && $left > 0 ) { - my $subtotal = $left * $data->{'ecost'}; + my $subtotal = $left * get_rounded_price($data->{'ecost'}); $data->{subtotal} = sprintf( "%.2f", $subtotal ); $data->{'left'} = $left; push @ordered, $data; diff --git a/acqui/parcel.pl b/acqui/parcel.pl index ec7332dfe9..54a8960e2a 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -134,7 +134,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}; @@ -152,8 +152,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}; @@ -172,7 +172,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; @@ -230,7 +230,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/pdfformat/layout3pages.pm b/acqui/pdfformat/layout3pages.pm index 8f80fe26a6..aa9fbe6fba 100644 --- a/acqui/pdfformat/layout3pages.pm +++ b/acqui/pdfformat/layout3pages.pm @@ -28,6 +28,7 @@ use List::MoreUtils qw/uniq/; use Modern::Perl; use utf8; +use C4::Acquisition; use Koha::Number::Price; use Koha::DateUtils; use Koha::Libraries; @@ -221,9 +222,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/acqui/pdfformat/layout3pagesfr.pm b/acqui/pdfformat/layout3pagesfr.pm index 5568e0157b..84e2206130 100644 --- a/acqui/pdfformat/layout3pagesfr.pm +++ b/acqui/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/spent.pl b/acqui/spent.pl index 988b8c28ee..0ff2ee2358 100755 --- a/acqui/spent.pl +++ b/acqui/spent.pl @@ -92,7 +92,7 @@ my @spent; while ( my $data = $sth->fetchrow_hashref ) { my $recv = $data->{'quantityreceived'}; if ( $recv > 0 ) { - my $rowtotal = $recv * $data->{'unitprice'}; + my $rowtotal = $recv * get_rounded_price($data->{'unitprice'}); $data->{'rowtotal'} = sprintf( "%.2f", $rowtotal ); $data->{'unitprice'} = sprintf( "%.2f", $data->{'unitprice'} ); $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.17.1