Bug 11680 - tax rate changes on edit
Summary: tax rate changes on edit
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jacek Ablewicz
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-04 14:20 UTC by Nicole C. Engard
Modified: 2015-06-04 23:30 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (1.34 KB, patch)
2014-02-05 11:19 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11680 - tax rate changes on edit (1.43 KB, patch)
2014-02-05 16:21 UTC, Nicole C. Engard
Details | Diff | Splinter Review
[PASSED QA] Bug 11680 - tax rate changes on edit (1.59 KB, patch)
2014-02-09 16:24 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11680: (follow-up) tax rate changes on edit (1.43 KB, patch)
2014-02-10 09:28 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 11680 - tax rate changes on edit (1.65 KB, patch)
2014-03-19 14:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11680: (follow-up) tax rate changes on edit (1.49 KB, patch)
2014-03-19 14:13 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2014-02-04 14:20:34 UTC
See this: http://screencast.com/t/d06EsMl925WO 

When you have a record with a tax rate saved on it and then click 'modify' to edit it the tax rate defaults to the first tax rate value even if that's not what was chosen before.

Tested on Master.
Comment 1 Jacek Ablewicz 2014-02-05 11:19:08 UTC Comment hidden (obsolete)
Comment 2 Jacek Ablewicz 2014-02-05 11:30:20 UTC
(In reply to Nicole C. Engard from comment #0)
> See this: http://screencast.com/t/d06EsMl925WO 
> 

Nicole,
I'm not sure if proposed patch will actually resolve this particular problem as seen in the video (unless you have trailing zeroes in your 6.0% tax rate definition in gist system preference?); it may as well be different issue (with the same symptoms) which I encountered while trying to reproduce this bug.
Comment 3 Nicole C. Engard 2014-02-05 14:23:36 UTC
I did not have a trailing 0 on my tax rate it was 0|0.6 in the preference.
Comment 4 Jacek Ablewicz 2014-02-05 15:55:12 UTC
(In reply to Nicole C. Engard from comment #3)
> I did not have a trailing 0 on my tax rate it was 0|0.6 in the preference.

You mean '0|0.06' (0.6 would be 60.0% tax)?
Also, one or more spaces in gist setting (possibly at the end, where they are not clearly visible) - something like '0|0.06 ' - will result pretty much with the same symptoms. Please try to apply this patch even if this is not the case - it may not work for you, but such test still will help us exclude some other possible causes of this problem.
Comment 5 Nicole C. Engard 2014-02-05 16:21:33 UTC Comment hidden (obsolete)
Comment 6 Nicole C. Engard 2014-02-05 16:21:52 UTC
It does appear to solve my problem - I don't know why  -but it does :)
Comment 7 Katrin Fischer 2014-02-09 16:21:54 UTC
This problem is also present on editing/saving a vendor's default tax rate.
Comment 8 Katrin Fischer 2014-02-09 16:24:31 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2014-02-09 16:26:00 UTC
The patch fixes one case and passes QA, but it's only a partial fix.
Comment 10 Jacek Ablewicz 2014-02-10 09:28:57 UTC Comment hidden (obsolete)
Comment 11 Galen Charlton 2014-02-10 15:45:44 UTC
Setting back to needs signoff so that the follow-up can be tested.

A couple notes:

[1] The reason for the bug is that Template Toolkit's equality operator (==) is implemented using the Perl 'eq' operator.  If the comparison were numeric, then

  0.2 == 0.20 evaluates to true

But since it's using 'eq',

  '0.2' eq '0.20' is NOT true

The trick of adding 0.0 forces a normalization of the tax rate.

[2] It would be worth writing a subroutine for getting the list of tax rates.
Comment 12 Jacek Ablewicz 2014-02-18 10:28:01 UTC
On the related subject: how do we want it to behave on vendor / order edit if particular, previously used tax rate from gist setting got stored in some orders / vendor records, but later removed from the gist preference?

Currently, if tax rate stored in order or vendor record is no longer present in the gist setting, Koha will default to the first value (usually 0%) on edit - for both vendor and order records. While such effect may be perhaps desired (?) while editing vendors, it may be a different story for orders.. Shouldn't it rather be possible to keep tax rate originally chosen for an order, even if it's no longer on the list / not in use for any new orders?
Comment 13 Katrin Fischer 2014-02-18 10:36:30 UTC
Hi Jacek, I think you are on to something here... my feeling is that we should not ever sneakily change data like that - not without a warning and an option to undo not save the changes. Also I think you are right - for a vendor it would make sense to be forced to select a new 'valid' tax rate, but for old orders I wouldn't change it automatically. It's too dangerous that someone changes it by accident, because another detail of the order is edited.
Comment 14 Chris Cormack 2014-03-10 20:44:39 UTC
Im confused by the last 2 comments, is this ready for signoff? Or should it be in discussion?
Comment 15 Koha Team University Lyon 3 2014-03-12 10:29:00 UTC
I test the 2 cases :
- add a tax rate in the vendor page and save. Go to this vendor and modify : the tax rate is still here.
- add a new order with a tax rate and save. Go to this order and modify it and save again : the tax rate is still here.

About the comments 12 and 13, I think it's preferable to keep the gst values in order even they're no more in syspref. But I agree with Katrin, in the vendor page it's better to use a gst defined in sypref. But I think, we can open a new bug for this if it's necessary.
Comment 16 Jonathan Druart 2014-03-19 14:13:49 UTC
Created attachment 26452 [details] [review]
Bug 11680 - tax rate changes on edit

If the tax rates in Acquisitions -> gist system preference are entered
with trailing zeroes, given order tax rate value may not be correctly
handled on order edit.

To test:

1) insert some tax rates with trailing zeroes in gist
system preference (e.g: '0|0.080|0.12|0.20|0.23')
2) place some new orders, choosing 8.0% 12.0% 20.0% 23.0%
as gst rate
3) try to modify them
4) note that orders with 12.0% and 23.0% tax rates are
preserving previously choosen rates on edit, while
the ones with 8.0% and 20.0% do not (they are defaulting
to the first defined tax rate)
5) apply the patch
6) repeat 2) and 3)
7) all tax rates configured in system prefrence shall now
behave properly while editing orders

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
All tests pass

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Confirmed the problem and that this patch fixes it.
Problem also exists for editing the default tax rate of a vendor.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 17 Jonathan Druart 2014-03-19 14:13:57 UTC
Created attachment 26453 [details] [review]
Bug 11680: (follow-up) tax rate changes on edit

Follow-up to fix similar issue on vendor edit.

If the tax rates in Acquisitions -> gist system preference
are entered with trailing zeroes, given vendor tax rate value
may not be correctly handled on vendor edit.

Test plan for this follow-up:

1) insert some tax rates with trailing zeroes in gist
system preference (e.g: '0|0.080|0.12|0.20|0.23')
2) add some vendors, choosing 8.0% 12.0% 20.0% 23.0%
as gst rate
3) try to modify them
4) note that vendors with 12.0% and 23.0% tax rates are
preserving previously choosen rates on edit, while
the ones with 8.0% and 20.0% do not (they are defaulting
to the first defined tax rate)
5) apply the patch
6) repeat 2) and 3)
7) all tax rates configured in system prefrence shall now
behave properly while editing vendors

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 18 Galen Charlton 2014-04-18 22:15:14 UTC
Pushed to master.  Thanks, Jacek!
Comment 19 Fridolin Somers 2014-08-06 12:05:31 UTC
Pushed to 3.14.x, will be in 3.14.10
Comment 20 Kyle M Hall 2014-08-13 15:33:16 UTC
Picked for 3.12.15!