[Bug 12969] New: VAT and price calculation should be done in a subroutine
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Bug ID: 12969 Summary: VAT and price calculation should be done in a subroutine Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@biblibre.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Component|Acquisitions |Architecture, internals, | |and plumbing Assignee|koha-bugs@lists.koha-commun |jonathan.druart@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 31746 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31746&action=edit Bug 12969: Add a subroutine to calculate VAT and prices This patch adds a new subroutine populate_order_with_prices in the C4::Acquisition module. Its goal is to refactore the VAT and prices calculation into Koha. All scripts will use this subroutine. Test plan: Verify that the prices in t/Prices.t are consistent with the values listed in the file "Prices and VAT calculation - before" submit on bug 12964. Verify that prove t/Prices.t returns green -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12970 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12896 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12975 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12976 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |paola.rossi@cineca.it --- Comment #2 from Paola Rossi <paola.rossi@cineca.it> --- I've applied the patch against master 3.17.00.025. [Firstly I've applied: 5342, 12830, 12896] perl t/Prices.t 1..4 Can't locate Koha/Number/Price.pm in @INC (@INC contains: /var/root-koha/bug-XXXX /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl .) at /var/root-koha/bug-XXXX/C4/Acquisition.pm line 35. BEGIN failed--compilation aborted at /var/root-koha/bug-XXXX/C4/Acquisition.pm line 35. Compilation failed in require at t/Prices.t line 5. BEGIN failed--compilation aborted at t/Prices.t line 5. # Looks like your test exited with 2 before it could output anything. So I pass the patch to "Failed QA" status -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff Depends on| |12844 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Paola, yes sorry, I forgot 1 dependency. Patches on bug 12844 have to be applied too, otherwise the prices are not rounded and the calculation is wrong. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|12844 | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Paola, to test this patch, you must apply, the patches from: bug 5342, bug 12844, bug 12830, bug 12896 and finally bug 12969. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- And don't forget bug 12852 after 5342. Otherwise the serial/claims.pl will raise an error. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12852 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- 12852 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31746|0 |1 is obsolete| | --- Comment #7 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 31841 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31841&action=edit Bug 12969: Add a subroutine to calculate VAT and prices I've applied the patch against master 3.17.00.025. Firstly I applied: 5342, 12852 (3), 12844 (4), 12830 (2), 12986. prove is OK. So I pass the patch to "Signed Off" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31841|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 31877 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31877&action=edit Bug 12969: Add a subroutine to calculate VAT and prices This patch adds a new subroutine populate_order_with_prices in the C4::Acquisition module. Its goal is to refactore the VAT and prices calculation into Koha. All scripts will use this subroutine. Test plan: Verify that the prices in t/Prices.t are consistent with the values listed in the file "Prices and VAT calculation - before" submit on bug 12964. Verify that prove t/Prices.t returns green -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- I reset the status to NSO, the patch didn't take into account the CurrencyFormat. The unit tests didn't pass if the value 'FR' was chosen. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31877|0 |1 is obsolete| | --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 31878 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31878&action=edit Bug 12969: Add a subroutine to calculate VAT and prices This patch adds a new subroutine populate_order_with_prices in the C4::Acquisition module. Its goal is to refactore the VAT and prices calculation into Koha. All scripts will use this subroutine. Test plan: Verify that the prices in t/Prices.t are consistent with the values listed in the file "Prices and VAT calculation - before" submit on bug 12964. Verify that prove t/Prices.t returns green -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #11 from Paola Rossi <paola.rossi@cineca.it> --- 1) Reading the file Prices.t of the patch, the "$order_x->rrpgste" always has "rrpgsti" as the corresponding "field", instead of "rrpgste". For example this is the compare of the first configuration 0-0: + compare( + { + got => $order_0_0->{rrpgste}, + expected => 82.00, + conf => '0 0', + field => 'rrpgsti' + } + ); In all the 4 Conf this compare has the same "field" rrpgsti. Is it an error? 2) Comparing the two files, in Conf III 1 0 I have some doubts: A) unitprice is 73.804500, whilst the 12964's file-doc says 73.80; B) rrp is 82.01, instead of 82.00 (in the 12964's file-doc) C) so, in the following: + compare( + { + got => $order_1_0->{rrpgsti}, + expected => 82.01, the "expected" should be 82.00 instead. Anyway soon after B) you wrote: + # Note that this configuration is *not* correct! + # rrp gsti should be 82 (what we inserted!) + # gstvalue should be 7.03 instead of 7.02 so 2) could be right. The 2) note is the only difference I've seen between the patch and the 12964's file-doc. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 31897 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31897&action=edit Bug 12969: Fix typo rrpgsti should be rrpgste -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #13 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Paola Rossi from comment #11)
1) Reading the file Prices.t of the patch, the "$order_x->rrpgste" always has "rrpgsti" as the corresponding "field", instead of "rrpgste".
For example this is the compare of the first configuration 0-0: + compare( + { + got => $order_0_0->{rrpgste}, + expected => 82.00, + conf => '0 0', + field => 'rrpgsti' + } + );
In all the 4 Conf this compare has the same "field" rrpgsti. Is it an error?
It's a typo. It is not important, this is only used to display the test description. Fixed in the followup.
2) Comparing the two files, in Conf III 1 0 I have some doubts:
A) unitprice is 73.804500, whilst the 12964's file-doc says 73.80; B) rrp is 82.01, instead of 82.00 (in the 12964's file-doc) C) so, in the following: + compare( + { + got => $order_1_0->{rrpgsti}, + expected => 82.01,
the "expected" should be 82.00 instead.
Anyway soon after B) you wrote: + # Note that this configuration is *not* correct! + # rrp gsti should be 82 (what we inserted!) + # gstvalue should be 7.03 instead of 7.02
so 2) could be right.
The 2) note is the only difference I've seen between the patch and the 12964's file-doc.
Actually, I thought it was always 82.01 (what you found this morning on bug 12964. In master, if you didn't receive the order yet, the displayed value is 82.00 BUT it the order is received, the value becomes 82.01 (...) The tests consider that the orders have already been received (the datereceived and quantityreceived are set). The 82.01 is correct here. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13001 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31878|0 |1 is obsolete| | --- Comment #14 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 31920 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31920&action=edit Bug 12969: Add a subroutine to calculate VAT and prices I've applied the patches against 3.17.00.025. Firstly I applied: 5342 (2), 12852 (3), 12844 (4), 12830 (2), 12986. prove is OK. So I pass the patch to "Signed Off" status. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31897|0 |1 is obsolete| | --- Comment #15 from Paola Rossi <paola.rossi@cineca.it> --- Created attachment 31921 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31921&action=edit Bug 12969: Fix typo -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #16 from Paola Rossi <paola.rossi@cineca.it> --- (In reply to Jonathan Druart from comment #9)
[...] the patch didn't take into account the CurrencyFormat. The unit tests didn't pass if the value 'FR' was chosen.
Today this test is OK too. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED CC| |katrin.fischer@bsz-bw.de --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Blocked by bug 12896 currently not applying. :( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Maybe some more comments later could improve readability, but no reason to fail :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #31920|0 |1 is obsolete| | Attachment #31921|0 |1 is obsolete| | --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 34731 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34731&action=edit [PASSED QA] Bug 12969: Add a subroutine to calculate VAT and prices This patch adds a new subroutine populate_order_with_prices in the C4::Acquisition module. Its goal is to refactore the VAT and prices calculation into Koha. All scripts will use this subroutine. Test plan: Verify that the prices in t/Prices.t are consistent with the values listed in the file "Prices and VAT calculation - before" submit on bug 12964. Verify that prove t/Prices.t returns green Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> http://bugs.koha-community.org/show_bug.cgi?id=12896 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 34732 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34732&action=edit [PASSED QA] Bug 12969: Fix typo rrpgsti should be rrpgste Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Passed QA |Pushed to Master --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Robin Sheat <robin@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |robin@catalyst.net.nz --- Comment #22 from Robin Sheat <robin@catalyst.net.nz> --- (In reply to Tomás Cohen Arazi from comment #21)
Patch pushed to master.
Thanks Jonathan!
This causes tests to fail. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 Robin Sheat <robin@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13554 See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=13554 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12969 --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Robin Sheat from comment #22)
This causes tests to fail.
I think the problem got fixed now, with bug 13554. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org