Summary: | Refactor VAT and price calculation - basketgroup page | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | paola.rossi, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 12969 | ||
Bug Blocks: | |||
Attachments: |
Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl
Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl [PASSED QA] Bug 12975: Use the centralize VAT and prices calculation - basketgroup.pl |
Description
Jonathan Druart
2014-09-22 14:08:57 UTC
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... 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! |