[Bug 10613] New: Gst is not calculated correctly on the invoice page
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Bug ID: 10613 Summary: Gst is not calculated correctly on the invoice page 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 The gst used is the gst defined at the supplier level. We should use the gst defined at the order level (if exists). -- 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=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=5335, | |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=5339 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=10613 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=10613 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19788 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19788&action=edit Bug 10613: The gst rate is not correctly calculated on the invoice page. Test plan: Defined a gst rate on creating an order, receive it and check that all prices are correctly calculated. /!\ Behavior change function of supplier parameters (Include/Don't include tax for list prices and invoice prices) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |liz@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 I'm just a bot <gitbot@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gitbot@bugs.koha-community. | |org When did the bot| |2013-09-29 last check this| | --- Comment #2 from I'm just a bot <gitbot@bugs.koha-community.org> --- Patch applied cleanly, go forth and signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathieu.saby@univ-rennes2.f | |r -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #3 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Hi I did not test yet, but I noticed some months ago that invoices seem to use ecost, while they should use the real unit cost (MT11242). Could you say me if this bug is also changing this behavior? M. Saby -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to mathieu saby from comment #3)
Hi I did not test yet, but I noticed some months ago that invoices seem to use ecost, while they should use the real unit cost (MT11242). Could you say me if this bug is also changing this behavior?
I don't remember. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #5 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Well, I will test and describe what I see. Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #6 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- I made 2 tests. I think some points need to be fixed, or maybe I am not well understanding the price management in Koha first test with "yes" (list price) and "yes" (invoiced price) for the 2 prefs of the vendor. 10 euros discount 10% tax rate : 5,5% on receipt page: - ecost : 9 euros - actual price : 9 euros on invoice, before clicking on "finish receiving" : - ecost : 9 euros - actual price : 9 euros - total tax inc : 9 euros => OK on invoice, after clicking on "finish receiving" : - est. : 9 euros - total tax inc : 9.48 euros => KO ================ 2d test with "yes" and "no" for the 2 prefs of the vendor. 10 euros discount 10% tax rate : 5,5% on receipt page: - ecost : 9,48 euros - actual price : 8.53 euros => KO. Ecost should be 9 euros. Probably not linked with your patch. on invoice, before clicking on "finish receiving" : - ecost : 8.53 euros - actual price : 8.53 euros - Total tax inc. : 9.00 euros => KO. Why adding the tax on the "Total tax inc."? We don't pay tax for this vendor. on invoice, after clicking on "finish receiving" : - est. : 9 euros - total tax inc : 9.48 euros => KO => Failed QA : * for "yes" and "no" (2d test) : the "total tax inc." amount should be 8.53, because we don't add tax for this vendor (if I understand well price management) * after the reception is finished, the table for "invoice details" - does not give me the actual price - still add tax even when it is not needed I note an other bug, probably in master : with "yes" / "no" preference (list price includes tax and invoiced does not include tax), the Replacement price on receipt page is bad (9.48 euros, while it should be 9 euros) Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #7 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- AHHH Sorry In fact your patch did not apply well on my VM! So I tested the current master ;-) Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #8 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- (my previous test was made on master, and I made a confusion between receipt page and invoiced page, so forget it) I tested with your patch for all test, a basket with 1 book - list price (the one I entered) : 10 eur - tax : vat = 5,5% - discount = 10% I have made a complete acq workflow and found some weird things. Most of all are are not related with your patch. Only the 4th step of each test (invoice page) is directly related with it. I made 4 full tests for all the possible vendor setting. My conclusion is : the more I test, the less I understand price management in Koha, and the documentation about it is too poor, so I cannot be sure, but I think your patch was not working well for test 1,2,3. I also think there are some other bugs in baskets and receipt pages on current master. So all that I can do is Fail QA for the moment. If I misunderstood price management, please correct me. If there are bugs to be fixed in basket and receipt page, I would like to know it (And I would also DREAM of a page on the wiki or on Koha documentation that explain precisely what Koha is supposed to do with prices and taxes..., or maybe a kind of UT for prices, but I don't know how it could be possible) Mathieu test 1 : vendor no (list price) / no (invoiced price). The list price entered by the librarian does NOT include tax, and the library does NOT pay tax basket : - RRP tax exc. : 10 - RRP tax inc. : 10.55 - ecost price tax exc. : 9 - ecost price tax inc. : 9.49 => maybe KO, but out of the scope of this bug : if the library does not pay tax, the total ecost price tax inc. should probably be the same as the total price tax exc. Or the column "ecost tax inc." should be hidden if it means nothing in this configuration. receipt page : - replacement : 10 - budgeted : 9 - actual cost : 9 => maybe KO, but out of the scope (I suppose replacement price should be tax included in remplacement cost) receipt page (after reception, table "already received") : - est cost : 9 - actual cost : 9 - total : 9 - Total tax inc. : 9.49 => KO for "Total tax inc. : 9.49", Should be "9", but out of the scope of this bug invoice page : - actual cost tax ecl : 9 - actual cost tax incl : 9.49 => KO : we don't pay tax for this vendor, so actual cost tax incl should be "9" ========= test 2: vendor yes (list price) / yes (invoiced price). The list price entered by the librarian does include tax, and the library does pay tax basket : - RRP tax exc. : 9.48 - RRP tax inc. : 10 - total price tax exc. : 8.53 - total price tax inc. : 9 => OK receipt page : - replacement : 10 - budgeted : 9 - actual cost : 9 => OK receipt page (after reception, table "already received") : - est cost : 9 - actual cost : 9 - total : 9 - Total tax inc. : 9 => OK invoice page : - actual cost tax ecl : 9 - actual cost tax incl : 9.49 => KO : we should have "8.53" and "9" ====== Test 3: vendor yes (list price) / no (invoiced price). The list price entered by the librarian does include tax, and the library does NOT pay tax basket : - RRP tax exc. : 9.48 - RRP tax inc. : 10 - total price tax exc. : 8.53 - total price tax inc. : 9 => maybe KO but out of the scope of the bug : If the library does not pay tax, ecost tax inc. should be the same as tax exc. and the 2 equal 8.53 ? Or the column should be hidden receipt page : - replacement : 9.48 - budgeted : 8.53 - actual cost : 8.53 => maybe KO : replacement price must be tax inc. (I suppose so, if the aim is to know the public price of the book to get it paid back by a reader who had lost it). receipt page (after reception, table "already received") : - est cost : 8.53 - actual cost : 8.53 - total : 8.53 - Total tax inc. : 9 => KO. Total tax inc. should be "8.53" as we pay no tax invoice page : - actual cost tax ecl : 9 - actual cost tax incl : 9.49 => KO : we should have "8.53" in both column ====== test 4: vendor no (list price) / yes (invoiced price). The list price entered by the librarian does NOT include tax, and the library does pay tax basket : - RRP tax exc. : 10 - RRP tax inc. : 10.55 - total price tax exc. : 9 - total price tax inc. : 9.49 => OK receipt page : - replacement : 10.55 - budgeted : 9.49 - actual cost : 9.49 => OK receipt page (after reception, table "already received") : - est cost : 9.49 - actual cost : 9.49 - total : 9.49 - Total tax inc. (bottom of table) : 9.49 => OK invoice page : - actual cost tax ecl : 9 - actual cost tax incl : 9.49 => OK -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 22389 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22389&action=edit Bug 10613: FIX typo supplierid vs booksellerid GetInvoiceDetails returns a hashref with a key named booksellerid, not supplierid. The bookseller was not retrieved from the DB and the listincgst value was always false. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 I'm just a bot <gitbot@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- When did the bot|2013-09-29 00:00:00 |2013-11-07 last check this| | --- Comment #10 from I'm just a bot <gitbot@bugs.koha-community.org> --- Patch applied cleanly testing 2 commit(s) (applied to 5fa6168 'Bug 11204: make pendingreserves use d') [1;32mOK[0m acqui/invoice.pl [1;32mOK[0m koha-tmpl/intranet-tmpl/prog/en/modules/acqui/invoice.tt -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #11 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- It is not working well, and most of all, improving the display of rrp and ecost price on invoices is maybe a loss of time. The prices that we MUST have on invoice page are the prices based on unitcost, aka the real cost of the item, entered at receipt stage. Other prices (rrp/ecost could be displayed but they are not needed) What we absolutely need in library is: - unitprice HT - tax on unitprice HT - unitprice TTC - quantity - unitprice TTC * quantity with clear labels on header of table to show at first glance is a price is with or without tax Here are my tests : 1st test : with no/no parameters, and an order for a book costing 100 euros in vendor's catalog (so 100 euros without tax), invoice gives me this : RRP : 100 Est : 90 Total : 90 => OK if RRP and Est are without taxes => KO if RRP and Est are WITH taxes in the bottom of the invoice table, I read that : Total tax exc. 100.00 1 90.00 Tax (5.50%) 5.50 5.50 Total tax inc. (EUR) 105.50 1 105.50 Total + shipment cost (EUR) 1 105.50 As we don't pay taxes, the taxes should not be displayed here. 2d test : with yes/yes parameters, and an order for a book costing 100 euros in vendor's catalog (so 100 euros including tax), invoice gives me this : Invoice details RRP Est. Qty. Total 100 90 1 90 We don't know if RRP and Est are with or without tax => OK if RRP and Est. are WITH tax => KO if RRP and Est. are WITHOUT tax in the bottom of the invoice table, I read that : Total tax exc. 94.79 1 94.79 Tax (5.50%) 5.21 5.21 Total tax inc. (EUR) 100.00 1 90.00 Total + shipment cost (EUR) 1 90.00 The total tax. exc. and the tax are the values calculated on public price (rrp). That's not good, Most of time, we don't need them at all. It could be displayed, but only as an indication, and with clear mention that it is based on RRP price. What we need here is the ecost price without tax. Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl --- Comment #12 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to mathieu saby from comment #11)
It is not working well, and most of all, improving the display of rrp and ecost price on invoices is maybe a loss of time.
Mathieu, are you sure your last test was with the patches correctly applied?
From what I see, patched version is calculating & displaying unitcost (aka actual cost) on invoice page, just like it should - and there is an clear indication stating if given column is with, or without tax.
We are currently testing those patches; so far with the good results for listincgst/invoiceincgst both yes/yes && no/no. Less usual combinations (no/yes, yes/no) might be a different story - I believe invoice.pl may need an additional small change like that: 185c185
if ( $bookseller->{'invoiceincgst'} ) {
--- < if ( $bookseller->{'listincgst'} ) { to behave properly for "no/yes" and "yes/no" (???). Anyway, handling prices for yes/no and no/yes combinations are apparently even more problematic in other places (like parcel.pl, basket.pl, .. - price calculations/display/column labeling etc. are not always working correctly in those other scripts - depending on the circumstances). I'm wondering if anyone is actually using such unusual combinations IRL? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11681 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11733 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #13 from mathieu saby <mathsabypro@gmail.com> --- Hi I won't have time to test this patch again. Maybe I did a mistake. So, if your tests are fine, you can change status to Signed Off. Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Back to needs signoff. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11755 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #15 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Jacek Ablewicz from comment #12)
Less usual combinations (no/yes, yes/no) might be a different story - I believe invoice.pl may need an additional small change like that:
185c185
if ( $bookseller->{'invoiceincgst'} ) {
--- < if ( $bookseller->{'listincgst'} ) {
to behave properly for "no/yes" and "yes/no" (???).
Previously we tried to test it (in 3.14 and master, with above small change included, and without it), still getting some rather weird results for "no/yes" and "yes/no" settings.. Getting bug 11755 into account (which explains at least some of this "weirdness"), question is - how should this bug be retested before sign-off: with 11755 applied first, or maybe not? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11755 --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Jacek Ablewicz from comment #15)
how should this bug be retested before sign-off: with 11755 applied first, or maybe not?
Yes, of course. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #17 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Jonathan Druart from comment #16)
(In reply to Jacek Ablewicz from comment #15)
how should this bug be retested before sign-off: with 11755 applied first, or maybe not?
Yes, of course.
I did some Q&D tests with 11755 applied (figuratively speaking - right now, 11755 apparently does not apply for current master? - for testing purposes I just altered C3/Acquisition.pm manually). So far, so good: 11755 patch seems to be f*ing golden - prices w/ & wo/ tax now being handled properly on the invoice page, for all "yes/yes", "no/no", "yes/no" and "no/yes" listpriceincgst/invoiceincgst setting combinations. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #18 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Jacek Ablewicz from comment #17)
I did some Q&D tests with 11755 applied (figuratively speaking - right now, 11755 apparently does not apply for current master? - for testing purposes I just altered C3/Acquisition.pm manually).
I rebased it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Jacek Ablewicz from comment #17)
So far, so good: 11755 patch seems to be f*ing golden - prices w/ & wo/ tax now being handled properly on the invoice page, for all "yes/yes", "no/no", "yes/no" and "no/yes" listpriceincgst/invoiceincgst setting combinations.
Great! May you sign it off then? :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <ablewicz@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19788|0 |1 is obsolete| | --- Comment #20 from Jacek Ablewicz <ablewicz@gmail.com> --- Created attachment 25889 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25889&action=edit [SIGNED-OFF] Bug 10613: The gst rate is not correctly calculated on the invoice page. Test plan: Defined a gst rate on creating an order, receive it and check that all prices are correctly calculated. /!\ Behavior change function of supplier parameters (Include/Don't include tax for list prices and invoice prices) Notes: patch tested with Bug 11755 applied first; confirmed that: - price calculations are correct for all combinations of listincgst/invoiceincgst settings in the vendor record - unitprice (aka "Actual cost") is taken into account on the invoice page instead of rrp/ecost, like it should. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <ablewicz@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22389|0 |1 is obsolete| | --- Comment #21 from Jacek Ablewicz <ablewicz@gmail.com> --- Created attachment 25890 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25890&action=edit [SIGNED-OFF] Bug 10613: FIX typo supplierid vs booksellerid GetInvoiceDetails returns a hashref with a key named booksellerid, not supplierid. The bookseller was not retrieved from the DB and the listincgst value was always false. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> 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=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #22 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- My feeling is, that we need to centralize the code doing all the calculations in acq, as the tax calculation has proven to be an endless source of pain and confusion. That said... going to QA this and hoping for an improvement :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Looking at this a bit more, I think I might have found a small mistake. With both patches applied: In acqui/invoice.pl: 103 my $bookseller = GetBookSellerFromId($details->{booksellerid}); 151 suppliername => $details->{'suppliername'}, Jonathan, could you check please? There is no "Need Feedback" status, so setting "Failed QA". -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #24 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm also: FAIL acqui/invoice.pl OK pod OK forbidden patterns FAIL valid Useless use of private variable in void context OK critic -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #25 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Katrin Fischer from comment #23)
151 suppliername => $details->{'suppliername'},
IMO this part is fine (GetInvoiceDetails() does return 'suppliername' and it seems to work like intended, at least for me), but this one: 152 supplierid => $details->{'supplierid'}, is indeed not; as a result, link via "Vendor: " name is leading to nowhere. Which I should've spotted before signing-off :(. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jacek, thx for checking :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #27 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26529 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26529&action=edit Bug 10613: FIX QA issues This patch fixes the following QA issue: FAIL acqui/invoice.pl FAIL valid Useless use of private variable in void context It fixes also the booksellerid variable (supplierid is not used anymore in the template). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #28 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #24)
FAIL acqui/invoice.pl FAIL valid Useless use of private variable in void context
(In reply to Jacek Ablewicz from comment #25)
152 supplierid => $details->{'supplierid'},
is indeed not; as a result, link via "Vendor: " name is leading to nowhere. Which I should've spotted before signing-off :(.
Good catches! Fixed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #29 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Jonathan Druart from comment #28)
FAIL valid Useless use of private variable in void context
IMO 126: my %row = %{ $order, $line }; was actually a little better ;), being equivalent of: my %row = %$line; and it tested just fine in that form ("Useles use: " warning aside). Looks like merging %$line with %order is 100% redundant (%$line is allways the superset of %$order).
152 supplierid => $details->{'supplierid'},
Fixed.
Not fully yet - vendor link still doesn't work; there is no 'supplierid' key in $details->{}, we have 'booksellerid' instead. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25889|0 |1 is obsolete| | Attachment #25890|0 |1 is obsolete| | Attachment #26529|0 |1 is obsolete| | --- Comment #30 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26534 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26534&action=edit Bug 10613: The gst rate is not correctly calculated on the invoice page. Test plan: Defined a gst rate on creating an order, receive it and check that all prices are correctly calculated. /!\ Behavior change function of supplier parameters (Include/Don't include tax for list prices and invoice prices) Notes: patch tested with Bug 11755 applied first; confirmed that: - price calculations are correct for all combinations of listincgst/invoiceincgst settings in the vendor record - unitprice (aka "Actual cost") is taken into account on the invoice page instead of rrp/ecost, like it should. Amended patch: Don't change the supplierid/booksellerid parameter. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #31 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26535 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26535&action=edit Bug 10613: FIX typo supplierid vs booksellerid GetInvoiceDetails returns a hashref with a key named booksellerid, not supplierid. The bookseller was not retrieved from the DB and the listincgst value was always false. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #32 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 26536 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26536&action=edit Bug 10613: FIX QA issues This patch fixes the following QA issue: FAIL acqui/invoice.pl FAIL valid Useless use of private variable in void context -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #33 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Jacek Ablewicz from comment #29)
and it tested just fine in that form ("Useles use: " warning aside). Looks like merging %$line with %order is 100% redundant (%$line is allways the superset of %$order).
Yes, you are right, sorry for that.
152 supplierid => $details->{'supplierid'},
Fixed.
Not fully yet - vendor link still doesn't work; there is no 'supplierid' key in $details->{}, we have 'booksellerid' instead.
I amended the first patch, which introduced this mistake. Thanks Jacek for your vigilance! The last patch needs a signoff. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <ablewicz@gmail.com> 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=10613 Jacek Ablewicz <ablewicz@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26536|0 |1 is obsolete| | --- Comment #34 from Jacek Ablewicz <ablewicz@gmail.com> --- Created attachment 26558 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26558&action=edit Bug 10613: FIX QA issues This patch fixes the following QA issue: FAIL acqui/invoice.pl FAIL valid Useless use of private variable in void context Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #35 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- Btw, the patches don't have [SIGNED-OFF] added in headers (I signed off the 3rd one without it too, for the sake of consistency). I don't know if this is really important or not - if yes, I'll try to re-sign-off all 3 patches. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jacek, it's not a problem as long as the status is correct "Signed off" it will show up in our searches :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #37 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Working on this again... it IS confusing, will need some more time for testing. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 11681 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |major -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Ok, this gives me a headache :( Can I get a consolidated test plan? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #40 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #39)
Ok, this gives me a headache :( Can I get a consolidated test plan?
A test plan could be: 0) Add a value in the gist pref (0.1 or 0.25 or something else easy). 1) a) Create a supplier "10613 0 0" with List item price includes tax: No Invoice item price includes tax: No b) Create a supplier "10613 0 1" with List item price includes tax: No Invoice item price includes tax: Yes c) Create a supplier "10613 1 0" with List item price includes tax: Yes Invoice item price includes tax: No d) Create a supplier "10613 1 1" with List item price includes tax: Yes Invoice item price includes tax: Yes 2) Create a basket for each supplier 3) Create 1+ order(s) with 1+ item(s) for each basket with a discount and a gst value. 4) Close the baskets 5) Receive the items 6) Go on acqui/invoice.pl?invoiceid=XX acqui/basket.pl?basketno=YY for each basket/invoice, click on the "Show all details" checkbox and verify that the values are the same. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #41 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jonathan, thx for the test plan, I am back on testing this now. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 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=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26534|0 |1 is obsolete| | Attachment #26535|0 |1 is obsolete| | Attachment #26558|0 |1 is obsolete| | --- Comment #42 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 26924 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26924&action=edit [PASSED QA] Bug 10613: The gst rate is not correctly calculated on the invoice page. Test plan: Defined a gst rate on creating an order, receive it and check that all prices are correctly calculated. /!\ Behavior change function of supplier parameters (Include/Don't include tax for list prices and invoice prices) Notes: patch tested with Bug 11755 applied first; confirmed that: - price calculations are correct for all combinations of listincgst/invoiceincgst settings in the vendor record - unitprice (aka "Actual cost") is taken into account on the invoice page instead of rrp/ecost, like it should. Amended patch: Don't change the supplierid/booksellerid parameter. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> 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=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off Patch complexity|Small patch |--- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12059 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 --- Comment #43 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 26925 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26925&action=edit [PASSED QA] Bug 10613: FIX typo supplierid vs booksellerid GetInvoiceDetails returns a hashref with a key named booksellerid, not supplierid. The bookseller was not retrieved from the DB and the listincgst value was always false. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> 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=10613 --- Comment #44 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 26926 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26926&action=edit [PASSED QA] Bug 10613: FIX QA issues This patch fixes the following QA issue: FAIL acqui/invoice.pl FAIL valid Useless use of private variable in void context Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Patch passes all tests and QA script. Specifically checked the t/db_depenedent/Acq* tests. A test plan could be: 0) Add a value in the gist pref (0.1 or 0.25 or something else easy). 1) a) Create a supplier "10613 0 0" with List item price includes tax: No Invoice item price includes tax: No Tax: 10% b) Create a supplier "10613 0 1" with List item price includes tax: No Invoice item price includes tax: Yes Tax: 10% c) Create a supplier "10613 1 0" with List item price includes tax: Yes Invoice item price includes tax: No Tax: 10% d) Create a supplier "10613 1 1" with List item price includes tax: Yes Invoice item price includes tax: Yes Tax: 10% 2) Create a basket for each supplier a) 00 List price: 10.00 (11.00 with 10% taxes) b) 01 List price: 10.00 (11.00 with 10% taxes) c) 10 List price: 10.00 (9.09 without taxes) d) 11 List price: 10.00 (9.09 without taxes) Note: Information on the basket page is shown correctly. If you look at the list of ordered items for the fund, the list price is used. 3) Create 1+ order(s) with 1+ item(s) for each basket with a discount and a gst value. 4) Close the baskets 5) Receive the items Left actual price as suggested: a) 00 Actual cost: 10.00 b) 01 Actual cost: 11.00 c) 10 Actual cost: 9.09 d) 11 Actual cost: 10.00 Calculations on the invoice page now all appear to be correct. Note: When you take a look at the 'ordered' list for the fund, the actual price is used as entered. 6) Go on acqui/invoice.pl?invoiceid=XX acqui/basket.pl?basketno=YY for each basket/invoice, click on the "Show all details" checkbox and verify that the values are all correct. Calculations are exactly the same for tax registered yes and no. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10461 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=11520 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_3_16_candidate -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #45 from Galen Charlton <gmcharlt@gmail.com> --- 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=10613 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12274 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10613 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |12274 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org