From 73d34975fe7b3554c0b1db9f516c63fd0b170164 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 8edd54e..43dcdc0 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -2815,9 +2815,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 { @@ -2834,48 +2831,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 cefdcff..c18cbf5 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -317,9 +317,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}; @@ -427,6 +424,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 0dc4db8..c49cfc9 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 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 44b2b26..0fbdc4b 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 94b906c..9ba06a9 100644 --- a/t/Prices.t +++ b/t/Prices.t @@ -39,7 +39,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 { @@ -71,7 +71,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_0->{rrp_tax_included}, @@ -112,22 +111,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( { @@ -137,7 +120,6 @@ for my $currency_format ( qw( US FR ) ) { } ); - # Note that this configuration is correct \o/ compare( { got => $order_0_0->{unitprice_tax_included}, @@ -162,26 +144,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 { @@ -213,8 +179,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}, @@ -250,27 +214,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( { @@ -279,8 +227,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}, @@ -300,31 +247,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 { @@ -337,11 +268,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, @@ -356,14 +287,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' } @@ -395,27 +322,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( { @@ -424,8 +335,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}, @@ -445,31 +355,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 { @@ -482,7 +376,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, @@ -501,12 +395,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' } ); @@ -514,7 +407,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' } ); @@ -522,7 +415,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' } ); @@ -530,7 +423,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' } ); @@ -538,26 +431,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( { @@ -566,7 +443,7 @@ for my $currency_format ( qw( US FR ) ) { receiving => 1, } ); - # Note that this configuration is correct + compare( { got => $order_0_1->{unitprice_tax_included}, @@ -591,22 +468,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.7.0