From fb67b41af6e04e4e5b54fee682df6b5ea04d3716 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. --- 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 f8c9d89..f21cebb 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -2800,9 +2800,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 { @@ -2819,48 +2816,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 8572561..a2a7d7f 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -309,9 +309,6 @@ if ( $op eq 'delete_confirm' ) { $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}; @@ -410,6 +407,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 90b0dca..2216308 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -81,10 +81,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 5baf7db..70e5c65 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 7cbe5cf..7bf9ce8 100755 --- a/acqui/invoice.pl +++ b/acqui/invoice.pl @@ -140,7 +140,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}; @@ -167,11 +166,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 => GetCurrency()->{currency}, 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 c9b0804..11ccd09 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -327,13 +327,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 3e38c67..d2b75bd 100644 --- a/t/Prices.t +++ b/t/Prices.t @@ -23,7 +23,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 { @@ -55,7 +55,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_0->{rrp_tax_included}, @@ -96,22 +95,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( { @@ -121,7 +104,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_0->{unitprice_tax_included}, @@ -146,26 +128,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 { @@ -197,8 +163,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}, @@ -234,27 +198,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( { @@ -263,8 +211,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}, @@ -284,31 +231,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 { @@ -321,11 +252,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, @@ -340,14 +271,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' } @@ -379,27 +306,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( { @@ -408,8 +319,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}, @@ -429,31 +339,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 { @@ -466,7 +360,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, @@ -485,12 +379,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' } ); @@ -498,7 +391,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' } ); @@ -506,7 +399,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' } ); @@ -514,7 +407,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' } ); @@ -522,26 +415,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( { @@ -550,7 +427,7 @@ for my $currency_format ( qw( US FR ) ) { receiving => 1, } ); - # Note that this configuration is correct + compare( { got => $order_0_1->{unitprice_tax_included}, @@ -575,22 +452,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' - } - ); }; } -- 1.9.1