[Bug 23523] New: unitprice tax column values are not populated if entered upon ordering
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Bug ID: 23523 Summary: unitprice tax column values are not populated if entered upon ordering Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org To recreate: 1 - Add an item to an acquisitions basket 2 - Make sure to enter 'Actual cost' 3 - Check the db: SELECT * FROM aqorders WHERE ordernumber={your ordernumber} 4 - Note that unitprice_tax_included and unitprice_tax_excluded are not populated -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 92500 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92500&action=edit Bug 23523: unitprice tax column values are not populated if entered upon ordering To test: 1 - Add an item to an acquisitions basket 2 - Make sure to enter 'Actual cost' 3 - Check the db: SELECT * FROM aqorders WHERE ordernumber={your ordernumber} 4 - Note that unitprice_tax_included and unitprice_tax_excluded are not populated 5 - Apply patch 6 - Edit that order 7 - Check DB 8 - Values should be populated 9 - Place another order, ensude values populated on creation 10 - QA people: prove -v t/db_dependent/Acquisition/populate_order_with_prices.t -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |23522 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23522 [Bug 23522] Actual price should be shown in basket -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #2 from David Nind <david@davidnind.com> --- Works for me for new items added after the patch was applied. However, I couldn't seem to modify existing items that are part of the basket - they reverted to original price. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to David Nind from comment #2)
Works for me for new items added after the patch was applied.
However, I couldn't seem to modify existing items that are part of the basket - they reverted to original price.
Hi David, I tested again, it seems to work for both new and old orders for. Note that you won't see any change in the basket with bug 23522 - this only affects the DB. I broke them into two as I thought it made things easier to read and test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92500|0 |1 is obsolete| | --- Comment #4 from Claire Gravely <claire_gravely@hotmail.com> --- Created attachment 93950 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93950&action=edit Bug 23523: unitprice tax column values are not populated if entered upon ordering To test: 1 - Add an item to an acquisitions basket 2 - Make sure to enter 'Actual cost' 3 - Check the db: SELECT * FROM aqorders WHERE ordernumber={your ordernumber} 4 - Note that unitprice_tax_included and unitprice_tax_excluded are not populated 5 - Apply patch 6 - Edit that order 7 - Check DB 8 - Values should be populated 9 - Place another order, ensude values populated on creation 10 - QA people: prove -v t/db_dependent/Acquisition/populate_order_with_prices.t Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Summary|unitprice tax column values |Unitprice tax column values |are not populated if |are not populated if |entered upon ordering |entered upon ordering -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think usually the 'actual price' is entered on receive - can you explain the use case for entering it on order? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am not sure about these fallbacks in the code - can you please explain? + # we should use the unitprice if included + my $cost_tax_included = $order->{unitprice_tax_included} || $order->{ecost_tax_included}; + my $cost_tax_excluded = $order->{unitprice_tax_excluded} || $order->{ecost_tax_excluded}; + $order->{tax_value_on_ordering} = ( get_rounded_price($cost_tax_included) - get_rounded_price($cost_tax_excluded) ) * $order->{quantity}; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #5)
I think usually the 'actual price' is entered on receive - can you explain the use case for entering it on order?
For standing orders, or orders that are completed in one step (i.e. purchasing a database) sometimes the library knows the actual price before 'receiving' Outside of these workflows, nothing changes, this only allows libraries who do enter actual price on ordering to have values calculated correctly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #6)
I am not sure about these fallbacks in the code - can you please explain?
+ # we should use the unitprice if included + my $cost_tax_included = $order->{unitprice_tax_included} || $order->{ecost_tax_included}; + my $cost_tax_excluded = $order->{unitprice_tax_excluded} || $order->{ecost_tax_excluded}; + $order->{tax_value_on_ordering} = ( get_rounded_price($cost_tax_included) - get_rounded_price($cost_tax_excluded) ) * $order->{quantity};
This only affects the 'tax_value_on_ordering' field -if you have entered an actual cost, then those values are used to determine the tax -if you have not, we use the ecost as we did previously (fallback to original behvaiour) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Katrin Fischer from comment #5)
I think usually the 'actual price' is entered on receive - can you explain the use case for entering it on order?
The ability to enter this field on order was removed in bug 9775 and then put back again in bug 23294.. I'm still not sure I agree with it having been put back as editable, but Nicks comment regarding 'Databases where they know the price before ordering' sort of makes sense.. So... one 'can' enter it... as such we should record it and use it in calculations in my opinion. Having said that, I'm sure this will break someone's workflow to fix someone elses much like bug 9775 did in the first place.. somewhat torn. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #93950|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 95034 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95034&action=edit Bug 23523: unitprice tax column values are not populated if entered upon ordering To test: 1 - Add an item to an acquisitions basket 2 - Make sure to enter 'Actual cost' 3 - Check the db: SELECT * FROM aqorders WHERE ordernumber={your ordernumber} 4 - Note that unitprice_tax_included and unitprice_tax_excluded are not populated 5 - Apply patch 6 - Edit that order 7 - Check DB 8 - Values should be populated 9 - Place another order, ensude values populated on creation 10 - QA people: prove -v t/db_dependent/Acquisition/populate_order_with_prices.t Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 95035 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95035&action=edit Bug 23523: (QA follow-up) Minor POD corrections This just fixes a couple of typos in the POD that predate this patch but I noticed whilst QAing this ;) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, Having spoken to Nick and worked through the various use cases, workflows and scenarios I agree with this patchset. I'd really like to see more workflows documented so we lower the likelihood of going around in circles again, but that should hold up this bug. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Workflows to document. 1) Standing orders (unitprice is passed through from previous 'receipts' - restored functionality in bug 23294) 2) RRP 0, One can 'trick' the encumberment system by setting unitprice and rrp at the same time, setting rrp to 0 ensures ecost is also set to 0 and as such the cost of the item is not included in the encumberment totals (which is a function some libraries aparently want/require) In short: * There are workflows where you only want rrp (ecost) entered at ordering and unitprice entered at receipt. * There are workflows where if you know unitprice at ordering you should be able to enter it, but ecost should be set to match it. * There are workflows where you want to deliberately enter different values for the two to get the result out of the system you want. As such, I'm not sure we can easily clarify any one of those workflows at this point as all need to be available. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Manual, | |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work! Pushed to master for 19.11.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 95082 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95082&action=edit Bug 23523: (RM follow-up) Failing tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- My followup gets the tests passing but I'm pretty confident it's not right :(. The deeper down this rabbit hole I dive the less confidence in this original patchset I have I'm afraid. So, digging deeply it seems that unitprice is pretty much always passed into populate_order_with_prices (regardless of whether you have entered such a price into the UI at order time). If you have not entered a such a price then it will fallback to using 'ecost' which is calculated clientside by JS and added to a readonly form field. As such, the t/Prices.t (which before my patch always submitted ecost = unitprice), should continue to pass but it does not.. this to my highlights an issue somewhere, but I'm stuck regarding finding it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95082|0 |1 is obsolete| | --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 95102 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95102&action=edit Bug 23523: (follow-up) Adjust tests for new expectations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 95103 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95103&action=edit Bug 23523: (follow-up) Clarify meaning configurations in tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks Nick, this looks good now.. followups pushed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23523 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org