Bug 12969 introduces a subroutine to centralize the VAT and price calculation. The basketgroup page should use it.
Created attachment 31811 [details] [review] 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.
Created attachment 31830 [details] [review] 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.
Created attachment 31879 [details] [review] 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.
Created attachment 31981 [details] [review] Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl I've applied the patch against master 3.17.00.025 Firstly I've applied: 5342 (2), 12852 (3), 12844 (4), 12830 (2), 12896 (1), 12969 (2). Everything is OK. NB. in Conf III 1-0 RRP tax inc. : 162.02 instead of 164.0 (file-doc in 12964) 82.01 instead of 82.0 (file-doc in 12964) I pass the patch to "Signed Off" status.
(In reply to Paola Rossi from comment #4) > Created attachment 31981 [details] [review] [review] Paola, you forgot to attach your signoff patch :)
(In reply to Jonathan Druart from comment #5) > (In reply to Paola Rossi from comment #4) > > Created attachment 31981 [details] [review] [review] [review] > > Paola, you forgot to attach your signoff patch :) Sorry, end of the day...
Blocked by bug 12896 currently not applying. :(
Comment on attachment 31981 [details] [review] Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl Review of attachment 31981 [details] [review]: ----------------------------------------------------------------- ::: acqui/basketgroup.pl @@ -212,5 @@ > } > } > > - my $ba_order = { > - isbn => ($ord->{isbn} ? $ord->{isbn} : undef), Hi Jonathan, bit irritated by this line - was removing it intentional? I see that that PDF should be printing the ISBN, but have a bit of trouble making it work in my installation currently.
Changing to 'failed qa' for now, waiting for Jonathan to take a look at the last comment.
(In reply to Katrin Fischer from comment #8) > Comment on attachment 31981 [details] [review] [review] > Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl > > Review of attachment 31981 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: acqui/basketgroup.pl > @@ -212,5 @@ > > } > > } > > > > - my $ba_order = { > > - isbn => ($ord->{isbn} ? $ord->{isbn} : undef), > > Hi Jonathan, bit irritated by this line - was removing it intentional? I see > that that PDF should be printing the ISBN, but have a bit of trouble making > it work in my installation currently. Hi Katrin, Yes it is intentional. Actually before the patch, the $ba_order was pushed to @ba_orders. Now, the order $ord is pushed. We could add: $ord->{isbn} = $ord->{isbn} ? $order->{isbn} : undef; But it does not make sense, except if we need to manage the empty string or 0, but it would not be a valid ISBN.
Hi Jonathan, I think you lost me there, but I believe you :) It looks like the printed PDF should include the ISBN, which also seems crucial for an order to a vendor. Does it print for you? It didn't for me, so maybe that's a separate bug to file.
(In reply to Katrin Fischer from comment #11) > Hi Jonathan, I think you lost me there, but I believe you :) > > It looks like the printed PDF should include the ISBN, which also seems > crucial for an order to a vendor. Does it print for you? It didn't for me, > so maybe that's a separate bug to file. Yes, I see it with English 3-pages with and without the patch.
Created attachment 35594 [details] [review] [PASSED QA] 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. Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Works as described, passes tests and QA script.
Patch pushed to master. Thanks Jonathan!