[Bug 13321] New: Tax and prices calculation rewrite
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Bug ID: 13321 Summary: Tax and prices calculation rewrite 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 When all patches blocking bug 12826 will be pushed, all the plumbing part will be done. We will be ready to properly handle the prices calculation. The idea is to create additional DB fields to store value tax included and tax excluded. For instance, for the ecost we will have ecost_tax_included and ecost_tax_excluded. On this way the calculation is done only one, on inserting/updating the order, and not every time we need to display these values. Note that the existing field will be kept, for 2 reasons: 1/ in some place in the code, we just need to retrieve what the user has inserted and 2/ it will be possible to go back if needed. In the long term, they will be removed. To be consistent, all occurrences of 'gste' will be replaced with 'tax_excluded", same for gsti. On the way, gstrate will be replaced with tax_rate, gstvalue with tax_value. Important note: It seems that the prices are generally correctly calculated (on master), the only problem I found is inconsistency in rounding. So we should not have a "migration problem" as I expected. -- 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=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12825 Depends on| |12826 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=13321 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33789 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33789&action=edit Bug 13321: Rename variables This patch renames the variable according to the new DB column names * gste => tax_excluded * gsti => tax_included * gstrate => tax_rate * gstvalue => tax_value This patch also modify the ModReceiveOrder subroutine: * Edit vendor note on receiving is not possible, so the code should not permit that. * Update ModReceiveOrder to pass a hashref And that's all! git grep on gste, gsti, gstrate and gstvalue should not return any code that can be executed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33790 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33790&action=edit Bug 13321: use the populate_with_prices where needed Now we need to calculate the prices (ecost, rrp, unitprice) and tax (tax_rate, tax_value) when the price is set for an order. This only appends in the 3 files impacted by this patch. addorder*.pl on ordering finishreceive.pl on receiving -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33791 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33791&action=edit Bug 13321: Fix the prices calculation method Well, we have finally arrived here \o/ The method where the prices are calculated uses the equations listed on the wiki page (http://wiki.koha-community.org/wiki/GST_Rewrite_RFC). The ecost is calculated from the rrp (using the discount and the tax rate). That's why we removed the ability to edit this value. That's why we remove the ability to edit the ecost on ordering in a previous commit (bug 12840). The total is now calculated in the scripts. That's why this patch removes lines in the test file. In C4::Acquisition::populate_order_with_prices, the calculation on receiving must depend on the 'invoiceincgst' supplier parameter, and not listincgst (which is used on ordering). It also removes the rounding errors, now we store "exact" values in DB (10^-6). The values will be displayed using the Price TT plugin it will round the values for us. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33792 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33792&action=edit Bug 13321: Tax and prices calculation - DB Changes This patch adds 7 columns to the aqorders table: * unitprice_tax_excluded * unitprice_tax_included * rrp_tax_excluded * rrp_tax_included * ecost_tax_excluded * ecost_tax_included * tax_value It also renames: * aqorders.gstrate with aqorders.tax_rate * aqbooksellers.gstrate with aqbooksellers.tax_rate The new columns are filled with the previous calculation method. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 33793 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33793&action=edit Bug 13321: [DO NOT PUSH] Tax and prices calculation - DBIC files This patch is for testing purpose only. Correct files will be generated by the RM on push. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- These patches will be tested with BibLibre coworker before. But you can help us and test them if you like :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13323 Summary|Tax and prices calculation |Tax and prices calculation |rewrite |need to be fixed -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |13324 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Koha Team Lyon 3 <koha@univ-lyon3.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |koha@univ-lyon3.fr --- Comment #7 from Koha Team Lyon 3 <koha@univ-lyon3.fr> --- I try to test theses patchs on biblibre sandbox n° 17 (wich has been frozen with these patchs, among others). I've done several tests with 4 vendors (with discount): 1. configured with List prices don't include tax / Invoice prices don't include tax (0 - 0) 2. configured with List prices don't include tax / Invoice prices include tax (0 - 1) 3. configured with List prices include tax / Invoice prices don't include tax (1 - 0) 4. configured with List prices include tax / Invoice prices include tax (1 - 1) For each vendor, I try receipt without any change, partial receipt, receipt with change of price, receipt with change of GST, receipt after a transfer from another vendor, with price needing some round calculation. In all the case (calculation in the basket, print of the basket group, export CSV of the basket group, receipt and invoice), the fair prices are calculated, consistently with the setting of the tax in the vendor page. Except in one case : the CSV export of a basket group after a transfer of an order from a vendor where list prices don't include tax forward a vendor where List prices include tax. Sonia BOUIS -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 34432 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34432&action=edit Bug 13321: Csv export - Display rrp and ecost with and without taxes If a basketgroup is exported as a CSV file, both prices should be displayed to avoid confusion. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Koha Team Lyon 3 from comment #7) Thanks for testing Sonia!
For each vendor, I try receipt without any change, partial receipt, receipt with change of price, receipt with change of GST, receipt after a transfer from another vendor, with price needing some round calculation.
Good to know (almost) everything is fine :)
In all the case (calculation in the basket, print of the basket group, export CSV of the basket group, receipt and invoice), the fair prices are calculated, consistently with the setting of the tax in the vendor page. Except in one case : the CSV export of a basket group after a transfer of an order from a vendor where list prices don't include tax forward a vendor where List prices include tax.
Good catch, I submitted a new patch (applied on the sandbox). With this patch, both prices (including and excluding tax) is displayed in the csv file. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #10 from Jonathan Druart <jonathan.druart@biblibre.com> --- To test these patches, have a look at http://lists.koha-community.org/pipermail/koha-devel/2014-December/041024.ht... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 35152 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35152&action=edit rounded issue for conf 1 0 This screenshot highlights the problem raised by François and Marc on bug 13324 comment 3 to 6. On receiving, the rrp and ecost is rounded to '9.52' (for the display), but the exact values are 9.523810 (which is stored in DB). So the unitprice is displayed 9.52 too and will be stored in DB with this value (the rounded value). Should we store the exact unitprice value even if "9.52" is displayed? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Francois Charbonnier <francois.charbonnier@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |francois.charbonnier@inlibr | |o.com --- Comment #12 from Francois Charbonnier <francois.charbonnier@inlibro.com> --- +1 for your option : exact value stored in database and rounded value displayed. This way, we are more likely to get accurate values down to the last cents. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #13 from Marc Véron <veron@veron.ch> --- +1 for exact value stored in database -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Yes, it should be quite easy to do. But I am wondering how to be clear on the order receive screen. The input will contain 9.52, but the inserted value will be 9.52XXX. Side effect: the user won't be able to insert 9.52 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 35165 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35165&action=edit Bug 13321: Fix unitprice rounding We need to display the rounded value, but insert the exact value. The unit price can be change on receiving, so the input value can be modified. But if it's not, we must insert the exact (not rounded) unit price, which is the ecost value. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- New commit on kohac/ft/bug_13323 and applied on the sandbox. It should fix the rounding problem for unit price on receiving. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Colin Campbell <colin.campbell@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |colin.campbell@ptfs-europe. | |com --- Comment #17 from Colin Campbell <colin.campbell@ptfs-europe.com> --- The real question is what we are doing with the values less than a cent? Remember also floats are not accurate. Financial apps that value accuracy convert the database value to an integer in cents and do all calculations on that then store it in a monetary form when saving it. That way any rounding is done predictably and it avoids errors arising from the approximation in how floats are stored and the inaccuracies in floating point arithmetic. If calculating a percentage for tax or discount purposes it needs to be calculated on the summed values, adding the individual values where they contain subdivisions of a cent are prone to error -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #18 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Colin Campbell from comment #17)
The real question is what we are doing with the values less than a cent?
Hum, what are the different choices? :)
Remember also floats are not accurate. Financial apps that value accuracy convert the database value to an integer in cents and do all calculations on that then store it in a monetary form when saving it. That way any rounding is done predictably and it avoids errors arising from the approximation in how floats are stored and the inaccuracies in floating point arithmetic.
Yes, you are totally right, we should not use float. But I did not want to change to many things. It could be done later.
If calculating a percentage for tax or discount purposes it needs to be calculated on the summed values, adding the individual values where they contain subdivisions of a cent are prone to error
The estimated cost (discounted) is calculated for 1 item, and the total is * by the quantity. On the contrary, the tax value should be correctly calculated. I did not see any problem with the estimated cost so far. Did you understand the purpose of the last patch (Fix unitprice rounding)? Do you think it makes sense to fix the problem on this way? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Zeno Tajoli <z.tajoli@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion CC| |z.tajoli@cineca.it -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Zeno Tajoli <z.tajoli@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |paola.rossi@cineca.it --- Comment #19 from Paola Rossi <paola.rossi@cineca.it> --- On bug 12826 all the 11 "Depends-on" bugs [except the "Overview" Bug 12964 ...] have already been pushed to master/stable. So I've tried to applied the 7 patches of this bug 13321 against master 3.19.00.011 head 13645. The first patch: Applying: Bug 13321: Rename variables fatal: sha1 information is lacking or useless (C4/Acquisition.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 13321: Rename variables 1) So I pass the patch to "Patch doesn't apply" status. 2) Otherwise, should the 7 patches of this bug 13321 still be tested on the sandbox 17? If this is the case, I beg your pardon and ask you, please, to pass this bug back to "Needs Signoff" status. 3) Are the problems in comment 18 been resolved by the 7 current patches? Or would this Bug 13321 be passed to an "In discussion" status? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Paola Rossi from comment #19) Hi Paola, See comment 10, the patches attached to this bug are only there for reference. I don't update/maintain them.
3) Are the problems in comment 18 been resolved by the 7 current patches? Or would this Bug 13321 be passed to an "In discussion" status?
I did not find any issues when testing, so yes, it is the way it's done. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #21 from Francois Charbonnier <francois.charbonnier@inlibro.com> --- I tested on BibLibre's sandbox 17 all the configurations from the RFC (http://wiki.koha-community.org/wiki/GST_Rewrite_RFC) and compared every displayed numbers. All calculations are good except for one. It's a very small glitch on the invoice page. The tax valule is not correct. It doesn't take the quantity into consideration. To reproduce the problem : 1. Create a vendor (ordre price includes tax, invoice price includes tax) 2. Add an order : - 2 items - vendor price : 10 euros - tax rate : 5% tax 3. Close the basket and receive the 2 ordered items 4. Check the "Already received" table : the tax is 0,95 euros 5. Click finish receiving (invoice.pl) 6. Check the "Invoice details" table : the tax is 0.48 euros (the value for 1 item). It should be multiply by the quantity. Jonathan, do you want to fix this here OR do you want to open another issue specific to the invoice page? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- Very nice catch François :) That comes from a commit on bug 13323: commit b5e9c8a8821f950933c19b8524fe2af3a98d05d7 Author: Jonathan Druart <jonathan.druart@biblibre.com> Date: Wed Nov 19 17:01:17 2014 +0100 Bug 13323: Tax rate can change on receiving This commit permits to update the tax rate on receiving. diff --git a/acqui/invoice.pl b/acqui/invoice.pl index 3a163c5..9fcef0e 100755 --- a/acqui/invoice.pl +++ b/acqui/invoice.pl @@ -126,7 +126,8 @@ foreach my $order (@$orders) { - $line->{tax_value} *= $line->{quantity}; + $line->{tax_value} = $line->{tax_value_on_receiving}; + $line->{tax_rate} = $line->{tax_rate_on_receiving}; It should have been: - $line->{tax_value} *= $line->{quantity}; + $line->{tax_value} = $line->{tax_value_on_receiving} * $line->{quantity}; + $line->{tax_rate} = $line->{tax_rate_on_receiving}; The quantity has been forgotten. I updated the commit, the branch and the sandbox with this change. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Francois Charbonnier <francois.charbonnier@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #23 from Francois Charbonnier <francois.charbonnier@inlibro.com> --- I tested jonathan's last patch today. It works nicely. I placed orders with different vendors (different settings) and coulnd't find any side effects. Considering all the tests I did yesterday, I can safely sign off these patches. I tested on BibLibre's sandbox 17. Jonathan, could you sign this off for me? Thanks. PS : about comment 15, I tested the patch for BZ13324 (see comment 8). It works perfectly. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #24 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Francois Charbonnier from comment #23)
Jonathan, could you sign this off for me? Thanks.
Added on the remote branch :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Working on this... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jonathan, can you please point me the right way for the branch? ft/bug_13321 looks not to be the right one. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- ... was my fault. I forgot to git fetch the biblibre repository. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Starting for real now - testing this bug together with 13323 and 13324. Some first feedback, I will number every point and keep a list outside of bugzilla too: DATABASE UPDATE 1) Some warns: Use of uninitialized value in string eq at ./installer/data/mysql/updatedatabase.pl line 10228. Use of uninitialized value in string eq at ./installer/data/mysql/updatedatabase.pl line 10228. Use of uninitialized value in string eq at ./installer/data/mysql/updatedatabase.pl line 10228. Use of uninitialized value in string eq at ./installer/data/mysql/updatedatabase.pl line 10228. Upgrade to 3.19.00.XXX done (Bug 13321 - Tax and prices calculation: add new columns to aqorders) Upgrade to 3.19.00.XXX done (Bug 13321 - Tax and prices calculation: tax_value contains the value for 1 item) Upgrade to 3.19.00.XXX done (Bug 13323 - Tax rate can change on receiving) 2) Shouldn't the second database update entry refer to the quantity somehow? Why the hardcoded 2? And shouldn't we fix the first database entry here? Bit worried about the round: + for my $order ( @$orders ) { + $sth->execute( $order->{tax_value} * 2, $order->{ordernumber} ); + } QA SCRIPT AND AUTOMATED TESTS 3) Failing tests: - t/db_dependent/Budgets.t not ok 61 - total spent for budget1 is 160 # Failed test 'total spent for budget1 is 160' # at t/db_dependent/Budgets.t line 371. # got: '0' # expected: '160' not ok 62 - total spent for budget11 is 100 # Failed test 'total spent for budget11 is 100' # at t/db_dependent/Budgets.t line 372. # got: '0' # expected: '100' not ok 63 - total spent for budget111 is 20 # Failed test 'total spent for budget111 is 20' # at t/db_dependent/Budgets.t line 373. # got: '0' # expected: '20' -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I will continue with code review tomorrow! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #30 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #28)
Starting for real now - testing this bug together with 13323 and 13324.
Thanks! :)
DATABASE UPDATE
1) Some warns: Use of uninitialized value in string eq at ./installer/data/mysql/updatedatabase.pl line 10228.
Yes, wrong check, the column is "orderstatus", not "status". (Commit "Bug 13321: Tax and prices calculation - DB Changes" amended)
2) Shouldn't the second database update entry refer to the quantity somehow? Why the hardcoded 2? And shouldn't we fix the first database entry here? Bit worried about the round:
+ for my $order ( @$orders ) { + $sth->execute( $order->{tax_value} * 2, $order->{ordernumber} ); + }
Hum... That's weird :) Should be tax_value = tax_value / quantity. Commit "Bug 13321: tax_value should contain the value for 1 item" amended.
QA SCRIPT AND AUTOMATED TESTS
3) Failing tests:
- t/db_dependent/Budgets.t
not ok 61 - total spent for budget1 is 160 # Failed test 'total spent for budget1 is 160' # at t/db_dependent/Budgets.t line 371. # got: '0' # expected: '160' not ok 62 - total spent for budget11 is 100 # Failed test 'total spent for budget11 is 100' # at t/db_dependent/Budgets.t line 372. # got: '0' # expected: '100' not ok 63 - total spent for budget111 is 20 # Failed test 'total spent for budget111 is 20' # at t/db_dependent/Budgets.t line 373. # got: '0' # expected: '20'
I forgot to change the call to ModReceiveOrder. Same for t/db_dependent/Acquisition/OrderUsers.t (new file). Commit "Bug 13321: Rename variables" amended. Branch updated. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #31 from Kyle M Hall <kyle@bywatersolutions.com> --- I've been testing https://git.biblibre.com/biblibre/kohac/commits/ft/bug_13323 Branch definitely needs rebased, but I tested it as is. My own attempts to rebase it ended with the unit tests failing, which I'm hoping is just my fault Not sure which of the three bugs to put this on, so it's going here ; ) There seems to be an issue where if you receive multiple, the receiving screen lists the total, but then the total is multiplied by the number of items received! http://screencast.com/t/0gpbhP0P2tdC http://screencast.com/t/NDwekDjesa http://screencast.com/t/Fk6PBDv15o2K Other than that, everything looks good! So, to keep going: 1) Fix the bug above 2) Rebase your branch to master 3) Update the patches here on bugzilla -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Kyle M Hall from comment #31)
I've been testing https://git.biblibre.com/biblibre/kohac/commits/ft/bug_13323
Branch definitely needs rebased, but I tested it as is. My own attempts to rebase it ended with the unit tests failing, which I'm hoping is just my fault
I didn't get any conflict (except updatedatabase.pl) on rebasing the branch on master.
Not sure which of the three bugs to put this on, so it's going here ; )
There seems to be an issue where if you receive multiple, the receiving screen lists the total, but then the total is multiplied by the number of items received!
http://screencast.com/t/0gpbhP0P2tdC http://screencast.com/t/NDwekDjesa http://screencast.com/t/Fk6PBDv15o2K
1 screen is missing, the neworderempty when you have created the order. I have tested on my local branch (before the jajm rebase) and with the remote branch, it seems to work as excepted. - Create an order without filling "actual cost" - Receive 1 or 2 item(s) (the actual cost is automatically filled with the budgeted cost, for 1 item. For you, it's 18, it should be 9 on your second screenshot. Did you update it manually or it was automatically filled with 18?). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Looking at this - could it be auser error? I think Kyle wanted to receive 2 items (9 was the calculated cost), so he entered the price for 2 = 18 into actual cost on receive - but it expects the price for a single item, so he should have entered 9 and let Koha mulitply it. (I checked, that's how it's currently working too in 3.18.8). It could be a nice enh for the future to add a little text hint there. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Back to the needs QA queue. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Summary|Tax and prices calculation |[DEPENDS 12826] Tax and |need to be fixed |prices calculation need to | |be fixed -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Bug 13321 depends on bug 12826, which changed state. Bug 12826 Summary: GST / VAT rewrite - plumbing http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12826 What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13321 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|[DEPENDS 12826] Tax and |Tax and prices calculation |prices calculation need to |need to be fixed |be fixed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org