Bugzilla – Attachment 31879 Details for
Bug 12975
Refactor VAT and price calculation - basketgroup page
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl
Bug-12975-Use-the-centralize-VAT-and-prices-calcul.patch (text/plain), 19.57 KB, created by
Jonathan Druart
on 2014-09-25 14:20:50 UTC
(
hide
)
Description:
Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2014-09-25 14:20:50 UTC
Size:
19.57 KB
patch
obsolete
>From 91aef2b6efa8f96265ffe51eecf9e0f11abdd30d Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@biblibre.com> >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' %] > <title>Koha › Acquisitions › Invoice</title> >@@ -161,13 +162,13 @@ > [% END %] > </td> > <td><p>[% order.branchcode %]</p></td> >- <td class="number gste">[% order.actualcostgste %]</td> >- <td class="number gsti">[% order.actualcostgsti %]</td> >+ <td class="number gste">[% order.unitpricegste | $Price %]</td> >+ <td class="number gsti">[% order.unitpricegsti | $Price %]</td> > <td class="number">[% order.quantity %]</td> >- <td class="number gste">[% order.totalgste %]</td> >- <td class="number gsti">[% order.totalgsti %]</td> >- <td class="number">[% order.gstgsti %]</td> >- <td class="number">[% order.gstvalue %]</td> >+ <td class="number gste">[% order.totalgste | $Price %]</td> >+ <td class="number gsti">[% order.totalgsti | $Price %]</td> >+ <td class="number">[% order.gstrate * 100 | $Price %]</td> >+ <td class="number">[% order.gstvalue | $Price %]</td> > <td>[% order.budget_name %]</td> > </tr> > [% END %] >@@ -175,13 +176,13 @@ > <tfoot> > [% FOR tf IN foot_loop %] > <tr> >- <th colspan='3'>Total (GST [% tf.gstgsti %] %)</th> >+ <th colspan='3'>Total (GST [% tf.gstrate * 100 | $Price %] %)</th> > <th class="gste"/><th class="gsti"/> > <th>[% tf.quantity %]</th> >- <th class="gste">[% tf.totalgste %]</th> >- <th class="gsti">[% tf.totalgsti %]</th> >+ <th class="gste">[% tf.totalgste | $Price %]</th> >+ <th class="gsti">[% tf.totalgsti | $Price %]</th> > <th> </th> >- <th>[% tf.gstvalue %]</th> >+ <th>[% tf.gstvalue | $Price %]</th> > <th> </th> > </tr> > [% END %] >@@ -189,10 +190,10 @@ > <th colspan='3'>Total ([% currency %])</th> > <th class="gste"/><th class="gsti"/> > <th>[% total_quantity %]</th> >- <th class="gste">[% total_gste %]</th> >- <th class="gsti">[% total_gsti %]</th> >+ <th class="gste">[% total_gste | $Price %]</th> >+ <th class="gsti">[% total_gsti | $Price %]</th> > <th> </th> >- <th>[% total_gstvalue %]</th> >+ <th>[% total_gstvalue | $Price %]</th> > <th> </th> > </tr> > <tr> >@@ -200,10 +201,10 @@ > <th class="gste"></th> > <th class="gsti"></th> > <th>[% total_quantity %]</th> >- <th class="gste">[% total_gste_shipment %]</th> >- <th class="gsti">[% total_gsti_shipment %]</th> >+ <th class="gste">[% total_gste_shipment | $Price %]</th> >+ <th class="gsti">[% total_gsti_shipment | $Price %]</th> > <th> </th> >- <th>[% total_gstvalue %]</th> >+ <th>[% total_gstvalue | $Price %]</th> > <th> </th> > </tr> > </tfoot> >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
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 12975
:
31811
|
31830
|
31879
|
31981
|
35594