From 91aef2b6efa8f96265ffe51eecf9e0f11abdd30d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 22 Sep 2014 16:09:14 +0200 Subject: [PATCH] Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl Bug 12969 introduces a subroutine to centralize VAT and prices calculation. It should be use in the acqui/basketgroup.pl script. Test plan: 0/ Don't apply the patch 1/ Create 4 suppliers with the different configurations 2/ Create a basket and create several orders 3/ Close the basket and create the corresponding basket groups. 4/ Print the basket group 5/ Verify you don't see any difference before and after applying the patch on the pdf file. --- C4/Acquisition.pm | 37 ++-- acqui/basketgroup.pl | 37 +--- acqui/invoice.pl | 26 +-- .../intranet-tmpl/prog/en/modules/acqui/invoice.tt | 33 ++-- t/Prices.t | 210 ++++++++++++++++++++- 5 files changed, 256 insertions(+), 87 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index e65ebf2..277d8bf 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -2919,23 +2919,26 @@ sub populate_order_with_prices { } } - # Not used yet - #if ($receiving) { - # if ( $bookseller->{invoiceincgst} ) { - # $order->{unitpricegsti} = $order->{unitprice}; - # $order->{unitpricegste} = - # $order->{unitpricegsti} / ( 1 + $order->{gstrate} ); - # } - # else { - # $order->{unitpricegste} = $order->{unitprice}; - # $order->{unitpricegsti} = - # $order->{unitpricegste} * ( 1 + $order->{gstrate} ); - # } - # $order->{gstvalue} = - # $order->{quantityreceived} * - # $order->{unitpricegste} * - # $order->{gstrate}; - #} + if ($receiving) { + # The following is completely wrong. Will be fixed later. + # See the unit tests to know what is wrong. + if ( $bookseller->{listincgst} ) { + $order->{unitpricegsti} = Koha::Number::Price->new( $order->{unitprice} )->round; + $order->{unitpricegste} = Koha::Number::Price->new( + $order->{unitpricegsti} / ( 1 + $order->{gstrate} ) )->round; + } + else { + $order->{unitpricegste} = Koha::Number::Price->new( $order->{unitprice} )->round; + $order->{unitpricegsti} = Koha::Number::Price->new( + $order->{unitpricegste} * ( 1 + $order->{gstrate} ) )->round; + } + $order->{gstvalue} = Koha::Number::Price->new( + ( $order->{unitpricegsti} - $order->{unitpricegste} ) + * $order->{quantityreceived} )->round; + + $order->{totalgste} = $order->{unitpricegste} * $order->{quantity}; + $order->{totalgsti} = $order->{unitpricegsti} * $order->{quantity}; + } return $order; } diff --git a/acqui/basketgroup.pl b/acqui/basketgroup.pl index ef62921..eba05e9 100755 --- a/acqui/basketgroup.pl +++ b/acqui/basketgroup.pl @@ -169,28 +169,7 @@ sub printbasketgrouppdf{ croak $@; } - $ord->{rrp} = ConvertCurrency( $ord->{'currency'}, $ord->{rrp} ); - if ( $bookseller->{'listincgst'} ) { - $ord->{rrpgsti} = sprintf( "%.2f", $ord->{rrp} ); - $ord->{gstgsti} = sprintf( "%.2f", $ord->{gstrate} * 100 ); - $ord->{rrpgste} = sprintf( "%.2f", $ord->{rrp} / ( 1 + ( $ord->{gstgsti} / 100 ) ) ); - $ord->{gstgste} = sprintf( "%.2f", $ord->{gstgsti} / ( 1 + ( $ord->{gstgsti} / 100 ) ) ); - $ord->{ecostgsti} = sprintf( "%.2f", $ord->{ecost} ); - $ord->{ecostgste} = sprintf( "%.2f", $ord->{ecost} / ( 1 + ( $ord->{gstgsti} / 100 ) ) ); - $ord->{gstvalue} = sprintf( "%.2f", ( $ord->{ecostgsti} - $ord->{ecostgste} ) * $ord->{quantity}); - $ord->{totalgste} = sprintf( "%.2f", $ord->{quantity} * $ord->{ecostgste} ); - $ord->{totalgsti} = sprintf( "%.2f", $ord->{quantity} * $ord->{ecostgsti} ); - } else { - $ord->{rrpgsti} = sprintf( "%.2f", $ord->{rrp} * ( 1 + ( $ord->{gstrate} ) ) ); - $ord->{rrpgste} = sprintf( "%.2f", $ord->{rrp} ); - $ord->{gstgsti} = sprintf( "%.2f", $ord->{gstrate} * 100 ); - $ord->{gstgste} = sprintf( "%.2f", $ord->{gstrate} * 100 ); - $ord->{ecostgsti} = sprintf( "%.2f", $ord->{ecost} * ( 1 + ( $ord->{gstrate} ) ) ); - $ord->{ecostgste} = sprintf( "%.2f", $ord->{ecost} ); - $ord->{gstvalue} = sprintf( "%.2f", ( $ord->{ecostgsti} - $ord->{ecostgste} ) * $ord->{quantity}); - $ord->{totalgste} = sprintf( "%.2f", $ord->{quantity} * $ord->{ecostgste} ); - $ord->{totalgsti} = sprintf( "%.2f", $ord->{quantity} * $ord->{ecostgsti} ); - } + $ord = C4::Acquisition::populate_order_with_prices({ order => $ord, booksellerid => $bookseller->{id}, ordering => 1 }); my $bib = GetBiblioData($ord->{biblionumber}); my $itemtypes = GetItemTypes(); @@ -212,17 +191,11 @@ sub printbasketgrouppdf{ } } - my $ba_order = { - isbn => ($ord->{isbn} ? $ord->{isbn} : undef), - itemtype => ( $ord->{itemtype} and $bib->{itemtype} ? $itemtypes->{$bib->{itemtype}}->{description} : undef ), - en => ( $en ? $en : undef ), - edition => ( $edition ? $edition : undef ), - }; - for my $key ( qw/ gstrate author title itemtype publishercode copyrightdate publicationyear discount quantity rrpgsti rrpgste gstgsti gstgste ecostgsti ecostgste gstvalue totalgste totalgsti order_vendornote / ) { - $ba_order->{$key} = $ord->{$key}; - } + $ord->{itemtype} = ( $ord->{itemtype} and $bib->{itemtype} ) ? $itemtypes->{$bib->{itemtype}}->{description} : undef; + $ord->{en} = $en ? $en : undef; + $ord->{edition} = $edition ? $edition : undef; - push(@ba_orders, $ba_order); + push(@ba_orders, $ord); } $orders{$basket->{basketno}} = \@ba_orders; } diff --git a/acqui/invoice.pl b/acqui/invoice.pl index d7e40b6..72504ae 100755 --- a/acqui/invoice.pl +++ b/acqui/invoice.pl @@ -121,9 +121,16 @@ my $total_gste = 0; my $total_gsti = 0; my $total_gstvalue = 0; foreach my $order (@$orders) { + $order = C4::Acquisition::populate_order_with_prices( + { + order => $order, + booksellerid => $bookseller->{id}, + receiving => 1, + } + ); my $line = get_infos( $order, $bookseller); - $foot{$$line{gstgsti}}{gstgsti} = $$line{gstgsti}; + $foot{$$line{gstgsti}}{gstrate} = $$line{gstrate}; $foot{$$line{gstgsti}}{gstvalue} += $$line{gstvalue}; $total_gstvalue += $$line{gstvalue}; $foot{$$line{gstgsti}}{quantity} += $$line{quantity}; @@ -193,23 +200,6 @@ sub get_infos { my %line = %{ $order }; $line{order_received} = ( $qty == $order->{'quantityreceived'} ); $line{budget_name} = $budget->{budget_name}; - if ( $bookseller->{'listincgst'} ) { - $line{gstgsti} = sprintf( "%.2f", $line{gstrate} * 100 ); - $line{gstgste} = sprintf( "%.2f", $line{gstgsti} / ( 1 + ( $line{gstgsti} / 100 ) ) ); - $line{actualcostgsti} = sprintf( "%.2f", $line{unitprice} ); - $line{actualcostgste} = sprintf( "%.2f", $line{unitprice} / ( 1 + ( $line{gstgsti} / 100 ) ) ); - $line{gstvalue} = sprintf( "%.2f", ( $line{actualcostgsti} - $line{actualcostgste} ) * $line{quantity}); - $line{totalgste} = sprintf( "%.2f", $order->{quantity} * $line{actualcostgste} ); - $line{totalgsti} = sprintf( "%.2f", $order->{quantity} * $line{actualcostgsti} ); - } else { - $line{gstgsti} = sprintf( "%.2f", $line{gstrate} * 100 ); - $line{gstgste} = sprintf( "%.2f", $line{gstrate} * 100 ); - $line{actualcostgsti} = sprintf( "%.2f", $line{unitprice} * ( 1 + ( $line{gstrate} ) ) ); - $line{actualcostgste} = sprintf( "%.2f", $line{unitprice} ); - $line{gstvalue} = sprintf( "%.2f", ( $line{actualcostgsti} - $line{actualcostgste} ) * $line{quantity}); - $line{totalgste} = sprintf( "%.2f", $order->{quantity} * $line{actualcostgste} ); - $line{totalgsti} = sprintf( "%.2f", $order->{quantity} * $line{actualcostgsti} ); - } if ( $line{uncertainprice} ) { $template->param( uncertainprices => 1 ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt index 8056c1d..2abd67d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt @@ -1,5 +1,6 @@ [% USE Koha %] [% USE KohaDates %] +[% USE Price %] [% INCLUDE 'doc-head-open.inc' %] Koha › Acquisitions › Invoice @@ -161,13 +162,13 @@ [% END %]

[% order.branchcode %]

- [% order.actualcostgste %] - [% order.actualcostgsti %] + [% order.unitpricegste | $Price %] + [% order.unitpricegsti | $Price %] [% order.quantity %] - [% order.totalgste %] - [% order.totalgsti %] - [% order.gstgsti %] - [% order.gstvalue %] + [% order.totalgste | $Price %] + [% order.totalgsti | $Price %] + [% order.gstrate * 100 | $Price %] + [% order.gstvalue | $Price %] [% order.budget_name %] [% END %] @@ -175,13 +176,13 @@ [% FOR tf IN foot_loop %] - Total (GST [% tf.gstgsti %] %) + Total (GST [% tf.gstrate * 100 | $Price %] %) [% tf.quantity %] - [% tf.totalgste %] - [% tf.totalgsti %] + [% tf.totalgste | $Price %] + [% tf.totalgsti | $Price %]   - [% tf.gstvalue %] + [% tf.gstvalue | $Price %]   [% END %] @@ -189,10 +190,10 @@ Total ([% currency %]) [% total_quantity %] - [% total_gste %] - [% total_gsti %] + [% total_gste | $Price %] + [% total_gsti | $Price %]   - [% total_gstvalue %] + [% total_gstvalue | $Price %]   @@ -200,10 +201,10 @@ [% total_quantity %] - [% total_gste_shipment %] - [% total_gsti_shipment %] + [% total_gste_shipment | $Price %] + [% total_gsti_shipment | $Price %]   - [% total_gstvalue %] + [% total_gstvalue | $Price %]   diff --git a/t/Prices.t b/t/Prices.t index 1a9278e..e86f416 100644 --- a/t/Prices.t +++ b/t/Prices.t @@ -21,7 +21,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 => 7; + plan tests => 12; $bookseller_module->mock( 'fetch', sub { @@ -110,10 +110,61 @@ for my $currency_format ( qw( US FR ) ) { field => 'totalgste' } ); + + $order_0_0 = C4::Acquisition::populate_order_with_prices( + { + order => $order_0_0, + booksellerid => 'just_something', + receiving => 1, + } + ); + # Note that this configuration is *not* correct! + # unitpricegsti should be 75.28 + # totalgst should be 150.56 + compare( + { + got => $order_0_0->{unitpricegsti}, + expected => 77.49, + conf => '0 0', + field => 'unitpricegsti' + } + ); + compare( + { + got => $order_0_0->{unitpricegste}, + expected => 73.80, + conf => '0 0', + field => 'unitpricegste' + } + ); + compare( + { + got => $order_0_0->{gstvalue}, + expected => 7.38, + conf => '0 0', + field => 'gstvalue' + } + ); + compare( + { + got => $order_0_0->{totalgsti}, + expected => 154.98, + conf => '0 0', + field => 'totalgsti' + } + ); + compare( + { + got => $order_0_0->{totalgste}, + expected => 147.60, + conf => '0 0', + field => 'totalgste' + } + ); }; subtest 'Configuration 1: 1 1' => sub { - plan tests => 7; + plan tests => 12; $bookseller_module->mock( 'fetch', sub { @@ -203,10 +254,60 @@ for my $currency_format ( qw( US FR ) ) { field => 'totalgste' } ); + + $order_1_1 = C4::Acquisition::populate_order_with_prices( + { + order => $order_1_1, + booksellerid => 'just_something', + receiving => 1, + } + ); + # Note that this configuration is *not* correct! + # gstvalue should be 7.03 + compare( + { + got => $order_1_1->{unitpricegsti}, + expected => 73.80, + conf => '1 1', + field => 'unitpricegsti' + } + ); + compare( + { + got => $order_1_1->{unitpricegste}, + expected => 70.29, + conf => '1 1', + field => 'unitpricegste' + } + ); + compare( + { + got => $order_1_1->{gstvalue}, + expected => 7.02, + conf => '1 1', + field => 'gstvalue' + } + ); + compare( + { + got => $order_1_1->{totalgsti}, + expected => 147.60, + conf => '1 1', + field => 'totalgsti' + } + ); + compare( + { + got => $order_1_1->{totalgste}, + expected => 140.58, + conf => '1 1', + field => 'totalgste' + } + ); }; subtest 'Configuration 1: 1 0' => sub { - plan tests => 7; + plan tests => 12; $bookseller_module->mock( 'fetch', sub { @@ -298,10 +399,62 @@ for my $currency_format ( qw( US FR ) ) { field => 'totalgste' } ); + + $order_1_0 = C4::Acquisition::populate_order_with_prices( + { + order => $order_1_0, + booksellerid => 'just_something', + receiving => 1, + } + ); + # Note that this configuration is *not* correct! + # unitpricegsti should be 71.69 + # totalgsti should be 143.38 + # gstvalue should be 7.03 + compare( + { + got => $order_1_0->{unitpricegsti}, + expected => 73.80, + conf => '1 0', + field => 'unitpricegsti' + } + ); + compare( + { + got => $order_1_0->{unitpricegste}, + expected => 70.29, + conf => '1 0', + field => 'unitpricegste' + } + ); + compare( + { + got => $order_1_0->{gstvalue}, + expected => 7.02, + conf => '1 0', + field => 'gstvalue' + } + ); + compare( + { + got => $order_1_0->{totalgsti}, + expected => 147.60, + conf => '1 0', + field => 'totalgsti' + } + ); + compare( + { + got => $order_1_0->{totalgste}, + expected => 140.58, + conf => '1 0', + field => 'totalgste' + } + ); }; subtest 'Configuration 1: 0 1' => sub { - plan tests => 7; + plan tests => 12; $bookseller_module->mock( 'fetch', sub { @@ -390,6 +543,55 @@ for my $currency_format ( qw( US FR ) ) { field => 'totalgste' } ); + + $order_0_1 = C4::Acquisition::populate_order_with_prices( + { + order => $order_0_1, + booksellerid => 'just_something', + receiving => 1, + } + ); + # Note that this configuration is correct + compare( + { + got => $order_0_1->{unitpricegsti}, + expected => 77.49, + conf => '0 1', + field => 'unitpricegsti' + } + ); + compare( + { + got => $order_0_1->{unitpricegste}, + expected => 73.80, + conf => '0 1', + field => 'unitpricegste' + } + ); + compare( + { + got => $order_0_1->{gstvalue}, + expected => 7.38, + conf => '0 1', + field => 'gstvalue' + } + ); + compare( + { + got => $order_0_1->{totalgsti}, + expected => 154.98, + conf => '0 1', + field => 'totalgsti' + } + ); + compare( + { + got => $order_0_1->{totalgste}, + expected => 147.60, + conf => '0 1', + field => 'totalgste' + } + ); }; } -- 2.1.0