From 3777f484a276066ef90f98fe72036c5c41ad8377 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 17 Nov 2014 12:06:09 +0100 Subject: [PATCH] Bug 13321: Fix the prices calculation method Well, we have finally arrived here \o/ The method where the prices are calculated uses the equations listed on the wiki page (http://wiki.koha-community.org/wiki/GST_Rewrite_RFC). The ecost is calculated from the rrp (using the discount and the tax rate). That's why we removed the ability to edit this value. That's why we remove the ability to edit the ecost on ordering in a previous commit (bug 12840). The total is now calculated in the scripts. That's why this patch removes lines in the test file. In C4::Acquisition::populate_order_with_prices, the calculation on receiving must depend on the 'invoiceincgst' supplier parameter, and not listincgst (which is used on ordering). It also removes the rounding errors, now we store "exact" values in DB (10^-6). The values will be displayed using the Price TT plugin it will round the values for us. Signed-off-by: Laurence Rault Signed-off-by: Francois Charbonnier Signed-off-by: Sonia Bouis --- C4/Acquisition.pm | 70 ++++---- acqui/basket.pl | 6 +- acqui/basketgroup.pl | 4 - acqui/finishreceive.pl | 20 +-- acqui/invoice.pl | 11 +- acqui/parcel.pl | 4 +- .../prog/en/modules/acqui/orderreceive.tt | 9 +- t/Prices.t | 179 +++------------------ 8 files changed, 75 insertions(+), 228 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index a6640d5..d0edc7d 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -2823,9 +2823,6 @@ sub GetBiblioCountByBasketno { return $sth->fetchrow; } -# This is *not* the good way to calcul prices -# But it's how it works at the moment into Koha -# This will be fixed later. # Note this subroutine should be moved to Koha::Acquisition::Order # Will do when a DBIC decision will be taken. sub populate_order_with_prices { @@ -2842,48 +2839,59 @@ sub populate_order_with_prices { my $discount = $order->{discount}; $discount /= 100 if $discount > 1; - $order->{rrp} = Koha::Number::Price->new( $order->{rrp} )->round; - $order->{ecost} = Koha::Number::Price->new( $order->{ecost} )->round; if ($ordering) { if ( $bookseller->{listincgst} ) { + # The user entered the rrp tax included $order->{rrp_tax_included} = $order->{rrp}; - $order->{rrp_tax_excluded} = Koha::Number::Price->new( - $order->{rrp_tax_included} / ( 1 + $order->{tax_rate} ) )->round; - $order->{ecost_tax_included} = $order->{ecost}; - $order->{ecost_tax_excluded} = Koha::Number::Price->new( - $order->{ecost} / ( 1 + $order->{tax_rate} ) )->round; - $order->{tax_value} = Koha::Number::Price->new( - ( $order->{ecost_tax_included} - $order->{ecost_tax_excluded} ) * - $order->{quantity} )->round; + + # rrp tax excluded = rrp tax included / ( 1 + tax rate ) + $order->{rrp_tax_excluded} = $order->{rrp_tax_included} / ( 1 + $order->{tax_rate} ); + + # 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 ); } else { + # The user entered the rrp tax excluded $order->{rrp_tax_excluded} = $order->{rrp}; - $order->{rrp_tax_included} = Koha::Number::Price->new( - $order->{rrp} * ( 1 + $order->{tax_rate} ) )->round; - $order->{ecost_tax_excluded} = $order->{ecost}; - $order->{ecost_tax_included} = Koha::Number::Price->new( - $order->{ecost} * ( 1 + $order->{tax_rate} ) )->round; - $order->{tax_value} = Koha::Number::Price->new( - ( $order->{ecost_tax_included} - $order->{ecost_tax_excluded} ) * - $order->{quantity} )->round; + + # rrp tax included = rrp tax excluded * ( 1 - tax rate ) + $order->{rrp_tax_included} = $order->{rrp_tax_excluded} * ( 1 + $order->{tax_rate} ); + + # 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} ) * + ( 1 - $discount ); } + + # tax value = quantity * ecost tax excluded * tax rate + $order->{tax_value} = $order->{quantity} * $order->{ecost_tax_excluded} * $order->{tax_rate}; } if ($receiving) { - if ( $bookseller->{listincgst} ) { - $order->{unitprice_tax_included} = Koha::Number::Price->new( $order->{unitprice} )->round; - $order->{unitprice_tax_excluded} = Koha::Number::Price->new( - $order->{unitprice_tax_included} / ( 1 + $order->{tax_rate} ) )->round; + if ( $bookseller->{invoiceincgst} ) { + # The user entered the unit price tax included + $order->{unitprice_tax_included} = $order->{unitprice}; + + # unit price tax excluded = unit price tax included / ( 1 + tax rate ) + $order->{unitprice_tax_excluded} = $order->{unitprice_tax_included} / ( 1 + $order->{tax_rate} ); } else { - $order->{unitprice_tax_excluded} = Koha::Number::Price->new( $order->{unitprice} )->round; - $order->{unitprice_tax_included} = Koha::Number::Price->new( - $order->{unitprice_tax_excluded} * ( 1 + $order->{tax_rate} ) )->round; + # The user entered the unit price tax excluded + $order->{unitprice_tax_excluded} = $order->{unitprice}; + + # unit price tax included = unit price tax included * ( 1 + tax rate ) + $order->{unitprice_tax_included} = $order->{unitprice_tax_excluded} * ( 1 + $order->{tax_rate} ); } - $order->{tax_value} = Koha::Number::Price->new( - ( $order->{unitprice_tax_included} - $order->{unitprice_tax_excluded} ) - * $order->{quantityreceived} )->round; + # tax value = quantity * unit price tax excluded * tax rate + $order->{tax_value} = $order->{quantity} * $order->{unitprice_tax_excluded} * $order->{tax_rate}; } return $order; diff --git a/acqui/basket.pl b/acqui/basket.pl index 5d3690d..306d3d7 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -315,9 +315,6 @@ elsif ( $op eq 'ediorder' ) { $template->param( uncertainprices => 1 ); } - $line->{total_tax_excluded} = Koha::Number::Price->new( $line->{ecost_tax_excluded} * $line->{quantity} )->format; - $line->{total_tax_included} = Koha::Number::Price->new( $line->{ecost_tax_included} * $line->{quantity} )->format; - push @books_loop, $line; $foot{$$line{tax_rate}}{tax_rate} = $$line{tax_rate}; @@ -423,6 +420,9 @@ 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}; + if ( $line{uncertainprice} ) { $line{rrp_tax_excluded} .= ' (Uncertain)'; } diff --git a/acqui/basketgroup.pl b/acqui/basketgroup.pl index 232547c..a4994d3 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -80,10 +80,6 @@ sub BasketTotal { } else { $total = $total + ( $order->{ecost_tax_excluded} * $order->{quantity} ); } - if ($bookseller->{invoiceincgst} && ! $bookseller->{listincgst} && ( $bookseller->{tax_rate} // C4::Context->preference("gist") )) { - my $gst = $bookseller->{tax_rate} // C4::Context->preference("gist"); - $total = $total * ( $gst / 100 +1); - } } $total .= " " . ($bookseller->{invoiceprice} // 0); return $total; diff --git a/acqui/finishreceive.pl b/acqui/finishreceive.pl index 53a7412..35fa750 100755 --- a/acqui/finishreceive.pl +++ b/acqui/finishreceive.pl @@ -54,8 +54,6 @@ my $invoice = GetInvoice($invoiceid); my $invoiceno = $invoice->{invoicenumber}; my $booksellerid = $input->param('booksellerid'); my $cnt = 0; -my $ecost = $input->param('ecost'); -my $rrp = $input->param('rrp'); my $bookfund = $input->param("bookfund"); my $order = GetOrder($ordernumber); my $new_ordernumber = $ordernumber; @@ -83,25 +81,9 @@ if ($quantityrec > $origquantityrec ) { } $order->{order_internalnote} = $input->param("order_internalnote"); - $order->{rrp} = $rrp; - $order->{ecost} = $ecost; $order->{unitprice} = $unitprice; - # FIXME is it still useful? my $bookseller = Koha::Acquisition::Bookseller->fetch({ id => $booksellerid }); - if ( $bookseller->{listincgst} ) { - if ( not $bookseller->{invoiceincgst} ) { - $order->{rrp} = $order->{rrp} * ( 1 + $order->{tax_rate} ); - $order->{ecost} = $order->{ecost} * ( 1 + $order->{tax_rate} ); - $order->{unitprice} = $order->{unitprice} * ( 1 + $order->{tax_rate} ); - } - } else { - if ( $bookseller->{invoiceincgst} ) { - $order->{rrp} = $order->{rrp} / ( 1 + $order->{tax_rate} ); - $order->{ecost} = $order->{ecost} / ( 1 + $order->{tax_rate} ); - $order->{unitprice} = $order->{unitprice} / ( 1 + $order->{tax_rate} ); - } - } $order = C4::Acquisition::populate_order_with_prices( { @@ -171,7 +153,7 @@ ModItem( booksellerid => $booksellerid, dateaccessioned => $datereceived, price => $unitprice, - replacementprice => $rrp, + replacementprice => $order->{rrp}, replacementpricedate => $datereceived, }, $biblionumber, diff --git a/acqui/invoice.pl b/acqui/invoice.pl index dd6ed3b..a878e39 100755 --- a/acqui/invoice.pl +++ b/acqui/invoice.pl @@ -141,7 +141,6 @@ foreach my $order (@$orders) { push @foot_loop, map {$_} values %foot; -my $format = "%.2f"; my $budgets = GetBudgets(); my @budgets_loop; my $shipmentcost_budgetid = $details->{shipmentcost_budgetid}; @@ -168,11 +167,11 @@ $template->param( orders_loop => \@orders_loop, foot_loop => \@foot_loop, total_quantity => $total_quantity, - total_tax_excluded => sprintf( $format, $total_tax_excluded ), - total_tax_included => sprintf( $format, $total_tax_included ), - total_tax_value => sprintf( $format, $total_tax_value ), - total_tax_excluded_shipment => sprintf( $format, $total_tax_excluded + $details->{shipmentcost}), - total_tax_included_shipment => sprintf( $format, $total_tax_included + $details->{shipmentcost}), + total_tax_excluded => $total_tax_excluded, + total_tax_included => $total_tax_included, + total_tax_value => $total_tax_value, + total_tax_excluded_shipment => $total_tax_excluded + $details->{shipmentcost}, + total_tax_included_shipment => $total_tax_included + $details->{shipmentcost}, invoiceincgst => $bookseller->{invoiceincgst}, currency => Koha::Acquisition::Currencies->get_active, budgets_loop => \@budgets_loop, diff --git a/acqui/parcel.pl b/acqui/parcel.pl index ffe41d8..79c5604 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -149,8 +149,8 @@ for my $order ( @orders ) { $line{budget} = GetBudgetByOrderNumber( $line{ordernumber} ); $foot{$line{tax_rate}}{tax_rate} = $line{tax_rate}; $foot{$line{tax_rate}}{tax_value} += $line{tax_value}; - $total_tax_excluded += Koha::Number::Price->new( $line{ecost_tax_excluded} * $line{quantity} )->format; - $total_tax_included += Koha::Number::Price->new( $line{ecost_tax_included} * $line{quantity} )->format; + $total_tax_excluded += $line{unitprice_tax_excluded} * $line{quantity}; + $total_tax_included += $line{unitprice_tax_included} * $line{quantity}; my $suggestion = GetSuggestionInfoFromBiblionumber($line{biblionumber}); $line{suggestionid} = $suggestion->{suggestionid}; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt index 477094f..afbcfb9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -328,13 +328,14 @@ [% END %][%# IF (AcqCreateItemReceiving) %] -
    • -
    • + +
  • [% rrp | $Price %]
    • +
  • [% ecost | $Price %]
  • [% IF ( unitprice ) %] - + [% ELSE %] - + [% END %]
    • [% IF order_vendornote %] diff --git a/t/Prices.t b/t/Prices.t index 61513d4..c5c744f 100644 --- a/t/Prices.t +++ b/t/Prices.t @@ -47,7 +47,7 @@ my $today; for my $currency_format ( qw( US FR ) ) { t::lib::Mocks::mock_preference( 'CurrencyFormat', $currency_format ); subtest 'Configuration 1: 0 0' => sub { - plan tests => 12; + plan tests => 8; $bookseller_module->mock( 'fetch', sub { @@ -79,7 +79,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_0->{rrp_tax_included}, @@ -120,22 +119,6 @@ for my $currency_format ( qw( US FR ) ) { field => 'tax_value' } ); - compare( - { - got => $order_0_0->{total_tax_included}, - expected => 154.98, - conf => '0 0', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_0_0->{total_tax_excluded}, - expected => 147.60, - conf => '0 0', - field => 'total_tax_excluded' - } - ); $order_0_0 = C4::Acquisition::populate_order_with_prices( { @@ -145,7 +128,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_0->{unitprice_tax_included}, @@ -170,26 +152,10 @@ for my $currency_format ( qw( US FR ) ) { field => 'tax_value' } ); - compare( - { - got => $order_0_0->{total_tax_included}, - expected => 154.98, - conf => '0 0', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_0_0->{total_tax_excluded}, - expected => 147.60, - conf => '0 0', - field => 'total_tax_excluded' - } - ); }; subtest 'Configuration 1: 1 1' => sub { - plan tests => 12; + plan tests => 8; $bookseller_module->mock( 'fetch', sub { @@ -221,8 +187,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is *not* correct - # tax_value should be 7.03 instead of 7.02 compare( { got => $order_1_1->{rrp_tax_included}, @@ -258,27 +222,11 @@ for my $currency_format ( qw( US FR ) ) { compare( { got => $order_1_1->{tax_value}, - expected => 7.02, + expected => 7.03, conf => '1 1', field => 'tax_value' } ); - compare( - { - got => $order_1_1->{total_tax_included}, - expected => 147.60, - conf => '1 1', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_1_1->{total_tax_excluded}, - expected => 140.58, - conf => '1 1', - field => 'total_tax_excluded' - } - ); $order_1_1 = C4::Acquisition::populate_order_with_prices( { @@ -287,8 +235,7 @@ for my $currency_format ( qw( US FR ) ) { receiving => 1, } ); - # Note that this configuration is *not* correct! - # tax_value should be 7.03 + compare( { got => $order_1_1->{unitprice_tax_included}, @@ -308,31 +255,15 @@ for my $currency_format ( qw( US FR ) ) { compare( { got => $order_1_1->{tax_value}, - expected => 7.02, + expected => 7.03, conf => '1 1', field => 'tax_value' } ); - compare( - { - got => $order_1_1->{total_tax_included}, - expected => 147.60, - conf => '1 1', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_1_1->{total_tax_excluded}, - expected => 140.58, - conf => '1 1', - field => 'total_tax_excluded' - } - ); }; subtest 'Configuration 1: 1 0' => sub { - plan tests => 12; + plan tests => 8; $bookseller_module->mock( 'fetch', sub { @@ -345,11 +276,11 @@ for my $currency_format ( qw( US FR ) ) { biblionumber => $biblionumber_1_0, quantity => 2, listprice => 82.000000, - unitprice => 73.804500, + unitprice => 70.290000, quantityreceived => 2, basketno => $basketno_1_1, invoiceid => $invoiceid_1_1, - rrp => 82.01, + rrp => 82.00, ecost => 73.80, tax_rate => 0.0500, discount => 10.0000, @@ -364,14 +295,10 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is *not* correct! - # rrp_tax_included should be 82 (what we inserted!) - # tax_value should be 7.03 instead of 7.02 - compare( { got => $order_1_0->{rrp_tax_included}, - expected => 82.01, + expected => 82, conf => '1 0', field => 'rrp_tax_included' } @@ -403,27 +330,11 @@ for my $currency_format ( qw( US FR ) ) { compare( { got => $order_1_0->{tax_value}, - expected => 7.02, + expected => 7.03, conf => '1 0', field => 'tax_value' } ); - compare( - { - got => $order_1_0->{total_tax_included}, - expected => 147.60, - conf => '1 0', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_1_0->{total_tax_excluded}, - expected => 140.58, - conf => '1 0', - field => 'total_tax_excluded' - } - ); $order_1_0 = C4::Acquisition::populate_order_with_prices( { @@ -432,8 +343,7 @@ for my $currency_format ( qw( US FR ) ) { receiving => 1, } ); - # Note that this configuration is *not* correct! - # gstvalue should be 7.03 + compare( { got => $order_1_0->{unitprice_tax_included}, @@ -453,31 +363,15 @@ for my $currency_format ( qw( US FR ) ) { compare( { got => $order_1_0->{tax_value}, - expected => 7.02, + expected => 7.03, conf => '1 0', field => 'tax_value' } ); - compare( - { - got => $order_1_0->{total_tax_included}, - expected => 147.60, - conf => '1 0', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_1_0->{total_tax_excluded}, - expected => 140.58, - conf => '1 0', - field => 'total_tax_excluded' - } - ); }; subtest 'Configuration 1: 0 1' => sub { - plan tests => 12; + plan tests => 8; $bookseller_module->mock( 'fetch', sub { @@ -490,7 +384,7 @@ for my $currency_format ( qw( US FR ) ) { biblionumber => $biblionumber_0_1, quantity => 2, listprice => 82.000000, - unitprice => 73.800000, + unitprice => 77.490000, quantityreceived => 2, basketno => $basketno_1_1, invoiceid => $invoiceid_1_1, @@ -509,12 +403,11 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_1->{rrp_tax_included}, expected => 86.10, - conf => '1 0', + conf => '0 1', field => 'rrp_tax_included' } ); @@ -522,7 +415,7 @@ for my $currency_format ( qw( US FR ) ) { { got => $order_0_1->{rrp_tax_excluded}, expected => 82.00, - conf => '1 0', + conf => '0 1', field => 'rrp_tax_excluded' } ); @@ -530,7 +423,7 @@ for my $currency_format ( qw( US FR ) ) { { got => $order_0_1->{ecost_tax_included}, expected => 77.49, - conf => '1 0', + conf => '0 1', field => 'ecost_tax_included' } ); @@ -538,7 +431,7 @@ for my $currency_format ( qw( US FR ) ) { { got => $order_0_1->{ecost_tax_excluded}, expected => 73.80, - conf => '1 0', + conf => '0 1', field => 'ecost_tax_excluded' } ); @@ -546,26 +439,10 @@ for my $currency_format ( qw( US FR ) ) { { got => $order_0_1->{tax_value}, expected => 7.38, - conf => '1 0', + conf => '0 1', field => 'tax_value' } ); - compare( - { - got => $order_0_1->{total_tax_included}, - expected => 154.98, - conf => '1 0', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_0_1->{total_tax_excluded}, - expected => 147.60, - conf => '1 0', - field => 'total_tax_excluded' - } - ); $order_0_1 = C4::Acquisition::populate_order_with_prices( { @@ -574,7 +451,7 @@ for my $currency_format ( qw( US FR ) ) { receiving => 1, } ); - # Note that this configuration is correct + compare( { got => $order_0_1->{unitprice_tax_included}, @@ -599,22 +476,6 @@ for my $currency_format ( qw( US FR ) ) { field => 'tax_value' } ); - compare( - { - got => $order_0_1->{total_tax_included}, - expected => 154.98, - conf => '0 1', - field => 'total_tax_included' - } - ); - compare( - { - got => $order_0_1->{total_tax_excluded}, - expected => 147.60, - conf => '0 1', - field => 'total_tax_excluded' - } - ); }; } -- 2.1.4