Bugzilla – Attachment 86658 Details for
Bug 18736
Problems in order calculations (rounding errors)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18736: Calculate tax depending on rounding
Bug-18736-Calculate-tax-depending-on-rounding.patch (text/plain), 14.62 KB, created by
Marcel de Rooy
on 2019-03-15 10:06:31 UTC
(
hide
)
Description:
Bug 18736: Calculate tax depending on rounding
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2019-03-15 10:06:31 UTC
Size:
14.62 KB
patch
obsolete
>From 240a39b078ba77a5552fd142e3fe8d0ca0d83fd3 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 15 Jan 2019 15:06:06 +0000 >Subject: [PATCH] Bug 18736: Calculate tax depending on rounding >Content-Type: text/plain; charset=utf-8 > >Marcel's comments pointed out that while I tried to avoid storing >rounded values it is required for tax generation. > >This patch makes that change and adds test coverage and POD for >populate_order_with_prices > >To test: >Follow plan on other patches, ensure that orders and totals match on the >basket, invoice, and budget pages > >prove -v t/db_dependent/Acquisition/populate_order_with_prices.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >--- > C4/Acquisition.pm | 56 +++++-- > acqui/ordered.pl | 2 +- > .../Acquisition/populate_order_with_prices.t | 165 +++++++++++++++++++++ > 3 files changed, 209 insertions(+), 14 deletions(-) > create mode 100644 t/db_dependent/Acquisition/populate_order_with_prices.t > >diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm >index dfea465a5f..4bf8c17ae7 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -3000,8 +3000,38 @@ sub GetBiblioCountByBasketno { > return $sth->fetchrow; > } > >-# Note this subroutine should be moved to Koha::Acquisition::Order >-# Will do when a DBIC decision will be taken. >+=head3 populate_order_with_prices >+ >+$order = populate_order_with_prices({ >+ order => $order #a hashref with the order values >+ booksellerid => $booksellerid #FIXME - should obtain from order basket >+ receiving => 1 # boolean representing order stage, should pass only this or ordering >+ ordering => 1 # boolean representing order stage >+}); >+ >+ >+Sets calculated values for an order - all values are stored with pull precision regardless of rounding preference except fot >+tax value which is calculated on rounded values if requested >+ >+For ordering the values set are: >+ rrp_tax_included >+ rrp_tax_excluded >+ ecost_tax_included >+ ecost_tax_excluded >+ tax_value_on_ordering >+For receiving the value set are: >+ unitprice_tax_included >+ unitprice_tax_excluded >+ tax_value_on_receiving >+ >+Note: When receiving if the rounded value of the unitprice matches the rounded value of the ecost then then ecost (full precision) is used. >+ >+Returns a hashref of the order >+ >+FIXME: Move this to Koha::Acquisition::Order.pm >+ >+=cut >+ > sub populate_order_with_prices { > my ($params) = @_; > >@@ -3025,11 +3055,15 @@ sub populate_order_with_prices { > # rrp tax excluded = rrp tax included / ( 1 + tax rate ) > $order->{rrp_tax_excluded} = $order->{rrp_tax_included} / ( 1 + $order->{tax_rate_on_ordering} ); > >+ # ecost tax included = rrp tax included ( 1 - discount ) >+ $order->{ecost_tax_included} = $order->{rrp_tax_included} * ( 1 - $discount ); >+ > # ecost tax excluded = rrp tax excluded * ( 1 - discount ) > $order->{ecost_tax_excluded} = $order->{rrp_tax_excluded} * ( 1 - $discount ); > >- # ecost tax included = rrp tax included ( 1 - discount ) >- $order->{ecost_tax_included} = $order->{rrp_tax_included} * ( 1 - $discount ); >+ # tax value = quantity * ecost tax excluded * tax rate >+ $order->{tax_value_on_ordering} = ( get_rounded_price($order->{ecost_tax_included}) - get_rounded_price($order->{ecost_tax_excluded}) ) * $order->{quantity}; >+ > } > else { > # The user entered the rrp tax excluded >@@ -3041,16 +3075,12 @@ sub populate_order_with_prices { > # ecost tax excluded = rrp tax excluded * ( 1 - discount ) > $order->{ecost_tax_excluded} = $order->{rrp_tax_excluded} * ( 1 - $discount ); > >- # ecost tax included = rrp tax excluded * ( 1 + tax rate ) * ( 1 - discount ) >- $order->{ecost_tax_included} = >- $order->{rrp_tax_excluded} * >- ( 1 + $order->{tax_rate_on_ordering} ) * >- ( 1 - $discount ); >- } >+ # ecost tax included = rrp tax excluded * ( 1 + tax rate ) * ( 1 - discount ) = ecost tax excluded * ( 1 + tax rate ) >+ $order->{ecost_tax_included} = $order->{ecost_tax_excluded} * ( 1 + $order->{tax_rate_on_ordering} ); > >- # tax value = quantity * ecost tax excluded * tax rate >- $order->{tax_value_on_ordering} = >- $order->{quantity} * get_rounded_price($order->{ecost_tax_excluded}) * $order->{tax_rate_on_ordering}; >+ # tax value = quantity * ecost tax included * tax rate >+ $order->{tax_value_on_ordering} = $order->{quantity} * get_rounded_price($order->{ecost_tax_excluded}) * $order->{tax_rate_on_ordering}; >+ } > } > > if ($receiving) { >diff --git a/acqui/ordered.pl b/acqui/ordered.pl >index b574d2eb39..3eb0519c65 100755 >--- a/acqui/ordered.pl >+++ b/acqui/ordered.pl >@@ -99,7 +99,7 @@ while ( my $data = $sth->fetchrow_hashref ) { > $left = $data->{'quantity'}; > } > if ( $left && $left > 0 ) { >- my $subtotal = $left * get_rounded_price($data->{'ecost_tax_included'}); >+ my $subtotal = get_rounded_price( $left * $data->{'ecost_tax_included'} ); > $data->{subtotal} = sprintf( "%.2f", $subtotal ); > $data->{'left'} = $left; > push @ordered, $data; >diff --git a/t/db_dependent/Acquisition/populate_order_with_prices.t b/t/db_dependent/Acquisition/populate_order_with_prices.t >new file mode 100644 >index 0000000000..16772e9eb8 >--- /dev/null >+++ b/t/db_dependent/Acquisition/populate_order_with_prices.t >@@ -0,0 +1,165 @@ >+#!/usr/bin/env perl >+ >+use Modern::Perl; >+ >+use Test::More tests => 34; >+use C4::Acquisition; >+use C4::Context; >+use Koha::Database; >+use t::lib::TestBuilder; >+use t::lib::Mocks; >+ >+# Start transaction >+my $schema = Koha::Database->new()->schema(); >+$schema->storage->txn_begin(); >+ >+my $dbh = C4::Context->dbh; >+$dbh->{RaiseError} = 1; >+ >+my $builder = t::lib::TestBuilder->new; >+ >+my $bookseller_inc_tax = Koha::Acquisition::Bookseller->new( >+ { >+ name => "Tax included", >+ address1 => "bookseller's address", >+ phone => "0123456", >+ active => 1, >+ listincgst => 1, >+ invoiceincgst => 1, >+ } >+)->store; >+ >+my $bookseller_exc_tax = Koha::Acquisition::Bookseller->new( >+ { >+ name => "Tax excluded", >+ address1 => "bookseller's address", >+ phone => "0123456", >+ active => 1, >+ listincgst => 0, >+ invoiceincgst => 0, >+ } >+)->store; >+ >+my $order_exc_tax = { >+ tax_rate => .1965, >+ discount => .42, >+ rrp => 16.99, >+ unitprice => 9.85, >+ quantity => 8, >+}; >+ >+#Vendor prices exclude tax, no rounding, ordering >+t::lib::Mocks::mock_preference('OrderPriceRounding', ''); >+my $order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ ordering => 1, >+ booksellerid => $bookseller_exc_tax->id, >+ order => $order_exc_tax, >+}); >+ >+is( $order_with_prices->{rrp_tax_excluded}+0 ,16.99 ,"Ordering tax excluded, no round: rrp tax excluded is rrp"); >+is( $order_with_prices->{rrp_tax_included}+0 ,20.328535 ,"Ordering tax excluded, no round: rrp tax included is rr tax excluded * (1 + tax rate on ordering)"); >+is( $order_with_prices->{ecost_tax_excluded}+0 ,9.8542 ,"Ordering tax excluded, no round: ecost tax excluded is rrp * ( 1 - discount )"); >+is( $order_with_prices->{ecost_tax_included}+0 ,11.7905503 ,"Ordering tax excluded, no round: ecost tax included is ecost tax excluded * (1 + tax rate on ordering)"); >+is( $order_with_prices->{tax_value_on_ordering}+0 ,15.4908024 ,"Ordering tax excluded, no round: tax value on ordering is quantity * ecost_tax_excluded * tax rate on ordering"); >+ >+#Vendor prices exclude tax, no rounding, receiving >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ receiving => 1, >+ booksellerid => $bookseller_exc_tax->id, >+ order => $order_exc_tax, >+}); >+ >+is( $order_with_prices->{unitprice}+0 ,9.8542 ,"Receiving tax excluded, no round, rounded ecost tax excluded = rounded unitprice : unitprice is ecost tax excluded"); >+is( $order_with_prices->{unitprice_tax_excluded}+0 ,9.8542 ,"Receiving tax excluded, no round, rounded ecost tax excluded = rounded unitprice : unitprice tax excluded is ecost tax excluded"); >+is( $order_with_prices->{unitprice_tax_included}+0 ,11.7905503 ,"Receiving tax excluded, no round: unitprice tax included is unitprice tax excluded * (1 + tax rate on ordering)"); >+is( $order_with_prices->{tax_value_on_ordering}+0 ,15.4908024 ,"Receiving tax excluded, no round: tax value on receiving is quantity * unitprice_tax_excluded * tax rate on receiving"); >+ >+#Vendor prices exclude tax, rounding to nearest cent, ordering >+t::lib::Mocks::mock_preference('OrderPriceRounding', 'nearest_cent'); >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ ordering => 1, >+ booksellerid => $bookseller_exc_tax->id, >+ order => $order_exc_tax, >+}); >+ >+is( $order_with_prices->{rrp_tax_excluded}+0 ,16.99 ,"Ordering tax excluded, round: rrp tax excluded is rrp"); >+is( $order_with_prices->{rrp_tax_included}+0 ,20.328535 ,"Ordering tax excluded, round: rrp tax included is rr tax excluded * (1 + tax rate on ordering)"); >+is( $order_with_prices->{ecost_tax_excluded}+0 ,9.8542 ,"Ordering tax excluded, round: ecost tax excluded is rrp * ( 1 - discount )"); >+is( $order_with_prices->{ecost_tax_included}+0 ,11.7905503 ,"Ordering tax excluded, round: ecost tax included is ecost tax excluded * (1 + tax rate on ordering)"); >+is( $order_with_prices->{tax_value_on_ordering}+0 ,15.4842 ,"Ordering tax excluded, round: tax value on ordering is quantity * ecost_tax_excluded * tax rate on ordering"); >+ >+#Vendor prices exclude tax, no rounding, receiving >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ receiving => 1, >+ booksellerid => $bookseller_exc_tax->id, >+ order => $order_exc_tax, >+}); >+ >+is( $order_with_prices->{unitprice_tax_excluded}+0 ,9.8542 ,"Receiving tax excluded, round, rounded ecost tax excluded = rounded unitprice : unitprice tax excluded is ecost tax excluded"); >+is( $order_with_prices->{unitprice_tax_included}+0 ,11.7905503 ,"Receiving tax excluded, round: unitprice tax included is unitprice tax excluded * (1 + tax rate on ordering)"); >+is( $order_with_prices->{tax_value_on_receiving}+0 ,15.4842 ,"Receiving tax excluded, round: tax value on receiving is quantity * unitprice_tax_excluded * tax rate on receiving"); >+ >+ >+ >+my $order_inc_tax = { >+ tax_rate => .1965, >+ discount => .42, >+ rrp => 20.33, >+ unitprice => 11.79, >+ quantity => 8, >+}; >+ >+#Vendor prices include tax, no rounding, ordering >+t::lib::Mocks::mock_preference('OrderPriceRounding', ''); >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ ordering => 1, >+ booksellerid => $bookseller_inc_tax->id, >+ order => $order_inc_tax, >+}); >+ >+is( $order_with_prices->{rrp_tax_included}+0 ,20.33 ,"Ordering tax included, no round: rrp tax included is rrp"); >+is( $order_with_prices->{rrp_tax_excluded}+0 ,16.9912244045132 ,"Ordering tax included, no round: rrp tax excluded is rrp tax included / (1 + tax rate on ordering)"); >+is( $order_with_prices->{ecost_tax_included}+0 ,11.7914 ,"Ordering tax included, no round: ecost tax included is rrp tax included * (1 - discount)"); >+is( $order_with_prices->{ecost_tax_excluded}+0 ,9.85491015461764 ,"Ordering tax included, no round: ecost tax excluded is rrp tax excluded * ( 1 - discount )"); >+is( $order_with_prices->{tax_value_on_ordering}+0 ,15.4919187630589 ,"Ordering tax included, no round: tax value on ordering is ( ecost tax included - ecost tax excluded ) * quantity"); >+ >+ >+#Vendor prices include tax, no rounding, receiving >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ receiving => 1, >+ booksellerid => $bookseller_inc_tax->id, >+ order => $order_inc_tax, >+}); >+ >+is( $order_with_prices->{unitprice}+0 ,11.7914 ,"Receiving tax included, no round, rounded ecost tax excluded = rounded unitprice : unitprice is ecost tax excluded"); >+is( $order_with_prices->{unitprice_tax_included}+0 ,11.7914 ,"Receiving tax included, no round: unitprice tax included is unitprice"); >+is( $order_with_prices->{unitprice_tax_excluded}+0 ,9.85491015461764 ,"Receiving tax included, no round: unitprice tax excluded is unitprice tax included / (1 + tax rate on receiving)"); >+is( $order_with_prices->{tax_value_on_ordering}+0 ,15.4919187630589 ,"Receiving tax included, no round: tax value on receiving is quantity * unitprice_tax_excluded * tax rate on receiving"); >+ >+#Vendor prices include tax, rounding to nearest cent, ordering >+t::lib::Mocks::mock_preference('OrderPriceRounding', 'nearest_cent'); >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ ordering => 1, >+ booksellerid => $bookseller_inc_tax->id, >+ order => $order_inc_tax, >+}); >+ >+is( $order_with_prices->{rrp_tax_included}+0 ,20.33 ,"Ordering tax included, round: rrp tax included is rrp"); >+is( $order_with_prices->{rrp_tax_excluded}+0 ,16.9912244045132 ,"Ordering tax included, round: rrp tax excluded is rounded rrp tax included * (1 + tax rate on ordering)"); >+is( $order_with_prices->{ecost_tax_included}+0 ,11.7914 ,"Ordering tax included, round: ecost tax included is rounded rrp * ( 1 - discount )"); >+is( $order_with_prices->{ecost_tax_excluded}+0 ,9.85491015461764 ,"Ordering tax included, round: ecost tax excluded is rounded ecost tax excluded * (1 - discount)"); >+is( $order_with_prices->{tax_value_on_ordering}+0 ,15.52 ,"Ordering tax included, round: tax value on ordering is (ecost_tax_included - ecost_tax_excluded) * quantity"); >+ >+#Vendor prices include tax, no rounding, receiving >+$order_with_prices = C4::Acquisition::populate_order_with_prices({ >+ receiving => 1, >+ booksellerid => $bookseller_inc_tax->id, >+ order => $order_inc_tax, >+}); >+ >+is( $order_with_prices->{unitprice_tax_included}+0 ,11.7914 ,"Receiving tax included, round: rounded ecost tax included = rounded unitprice : unitprice tax excluded is ecost tax included"); >+is( $order_with_prices->{unitprice_tax_excluded}+0 ,9.85491015461764 ,"Receiving tax included, round: unitprice tax excluded is unitprice tax included / (1 + tax rate on ordering)"); >+is( $order_with_prices->{tax_value_on_receiving}+0 ,15.4842 ,"Receiving tax included, round: tax value on receiving is quantity * (rounded unitprice_tax_excluded) * tax rate on receiving"); >+ >+ >+$schema->storage->txn_rollback(); >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18736
:
64061
|
64065
|
70187
|
70188
|
70189
|
70768
|
71062
|
71063
|
71627
|
71628
|
71629
|
71630
|
71631
|
71632
|
74335
|
74336
|
74337
|
74338
|
74339
|
74340
|
74341
|
74342
|
74370
|
74371
|
74374
|
74397
|
74398
|
74422
|
74427
|
74438
|
76979
|
76980
|
76981
|
76982
|
76983
|
76984
|
76985
|
76986
|
76987
|
76988
|
76989
|
76990
|
76991
|
77614
|
77615
|
77616
|
77617
|
77618
|
78101
|
78120
|
78981
|
78982
|
78983
|
78984
|
78985
|
78986
|
78987
|
78988
|
79087
|
79088
|
79089
|
79090
|
79091
|
79092
|
79093
|
79094
|
79098
|
79099
|
79100
|
79101
|
79102
|
79103
|
79104
|
79105
|
79187
|
79223
|
79562
|
79563
|
82967
|
82968
|
82969
|
82970
|
82971
|
82972
|
82973
|
82974
|
82975
|
82976
|
82977
|
83212
|
83213
|
83214
|
83973
|
83974
|
83975
|
83976
|
83977
|
83978
|
83979
|
83980
|
83981
|
83982
|
83983
|
83984
|
83985
|
86646
|
86647
|
86648
|
86649
|
86650
|
86651
|
86652
|
86653
|
86654
|
86655
|
86656
|
86657
| 86658