Bug 13323 - Change the tax rate on receiving
Summary: Change the tax rate on receiving
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
Depends on: 13321
Blocks: 11062 11514 16017 10253 13726
  Show dependency treegraph
 
Reported: 2014-11-21 16:08 UTC by Jonathan Druart
Modified: 2017-06-14 22:16 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13323: Tax rate can change on receiving - DB changes (4.23 KB, patch)
2014-11-24 16:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (20.61 KB, patch)
2014-11-24 16:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13323: [DO NOT PUSH] Tax rate can change on receiving - DBIC changes (2.55 KB, patch)
2014-11-24 16:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving - DB changes (3.95 KB, patch)
2016-02-01 09:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (21.00 KB, patch)
2016-02-01 09:34 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: QA fix (trailing whitespace) (1.09 KB, patch)
2016-02-01 10:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving - DB changes (4.13 KB, patch)
2016-03-09 14:52 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (21.18 KB, patch)
2016-03-09 14:52 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: QA fix (trailing whitespace) (1.09 KB, patch)
2016-03-09 14:54 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving - DB changes (4.13 KB, patch)
2016-06-03 10:11 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (21.18 KB, patch)
2016-06-03 10:11 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13323: QA fix (trailing whitespace) (610 bytes, patch)
2016-06-03 10:11 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving - DB changes (4.13 KB, patch)
2016-10-10 12:40 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (20.92 KB, patch)
2016-10-10 12:40 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: QA fix (trailing whitespace) (610 bytes, patch)
2016-10-10 12:40 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Recalculate tax_value for partially received orders (2.47 KB, patch)
2016-10-13 12:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving - DB changes (4.18 KB, patch)
2016-10-13 15:47 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (20.97 KB, patch)
2016-10-13 15:47 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 13323: QA fix (trailing whitespace) (658 bytes, patch)
2016-10-13 15:47 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 13323: Recalculate tax_value for partially received orders (2.52 KB, patch)
2016-10-13 15:47 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving - DB changes (4.23 KB, patch)
2016-10-27 00:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13323: Tax rate can change on receiving (21.03 KB, patch)
2016-10-27 00:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13323: QA fix (trailing whitespace) (715 bytes, patch)
2016-10-27 00:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 13323: Recalculate tax_value for partially received orders (2.57 KB, patch)
2016-10-27 00:59 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-11-21 16:08:46 UTC
This new feature will permit to update the tax rate on receiving.
It has also been described on the wiki page (http://wiki.koha-community.org/wiki/GST_Rewrite_RFC).
Comment 1 Jonathan Druart 2014-11-24 16:27:19 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-11-24 16:27:23 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-11-24 16:27:26 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-01-07 08:20:34 UTC
To test these patches, have a look at http://lists.koha-community.org/pipermail/koha-devel/2014-December/041024.html
Comment 5 Francois Charbonnier 2015-01-07 19:08:33 UTC
I tested this patch on BibLibre's sandbox17.

Here is what I did.

I created 4 new vendors to test 4 different set ups :

Set up 1 = List prices INCLUDE TAX + Invoice prices INCLUDE TAX
Set up 2 = List prices EXCLUDE TAX + Invoice prices EXCLUDE TAX
Set up 3 = List prices INCLUDE TAX + Invoice prices EXCLUDE TAX
Set up 4 = List prices EXCLUDE TAX + Invoice prices INCLUDE TAX

For each vendor, default tax rate is 5%.

Then, for each vendor :

* I add 1 basket with 1 item. Vendor price is 10 euros. Default tax rate is 5%.
* I check the prices and tax caculation on the basket page. Everything is well calculated (prices tax included or excluded)
* I close the basket and go to the receive page
* I receive the item and change the tax rate to 10%.
* I check the prices and tax calculation on the receive page. For the set ups 1 and 2, I have what I expected. Not for the set ups 3 and 4.

Set up 1 (List prices INCLUDE TAX + Invoice prices INCLUDE TAX):

Ordering :
Vendor price (tax incl) = 10 euros <---- what I entered
Vendor price (tax excl) = 9.52 euros 
Tax amount (5%) = 0.48 euros
Total (tax excl) = 9.52 euros
Total (tax incl) = 10 euros

Receiving :
Vendor price (tax incl) = 10 euros (= value in actual cost)
Vendor price (tax excl) = 9.09 euros 
Tax amount (10%) = 0.91 euros 
Total (tax excl) = 9.09 euros
Total (tax incl) = 10 euros

Set up 2 (List prices EXCLUDE TAX + Invoice prices EXCLUDE TAX):

Ordering :
Vendor price (tax incl) = 10.5 euros 
Vendor price (tax excl) = 10 euros <---- what I entered
Tax amount (5%) = 0.5 euros
Total (tax excl) = 10 euros
Total (tax incl) = 10.5 euros

Receiving :
Vendor price (tax incl) = 11 euros 
Vendor price (tax excl) = 10 euros (= value in actual cost)
Tax amount (10%) = 1 euros 
Total (tax excl) = 10 euros
Total (tax incl) = 11 euros

Set up 3 (List prices INCLUDE TAX + Invoice prices EXCLUDE TAX):

Ordering :
Vendor price (tax incl) = 10 euros <---- what I entered
Vendor price (tax excl) = 9.52 euros 
Tax amount (5%) = 0.48 euros
Total (tax excl) = 9.52 euros
Total (tax incl) = 10 euros

Receiving (what we have now):
Vendor price (tax incl) = 10.47 euros
Vendor price (tax excl) = 9.52 euros (= value in actual cost)
Tax amount (10%) = 0.95 euros 
Total (tax excl) = 9.52 euros
Total (tax incl) = 10.47 euros

Receiving (what I expected):
Vendor price (tax incl) = 10 euros
Vendor price (tax excl) = 9.09 euros (= value in actual cost)
Tax amount (10%) = 0.91 euros 
Total (tax excl) = 9.09 euros
Total (tax incl) = 10 euros

Since I entered the price tax included and then changed the tax rate, I expected the actual cost, which is here the price tax excluded, to be recalculated. But here, the value in actual cost is the price tax excluded calculated on the price tax included with a 5% tax (but I changed it to 10%...)

Set up 4 (List prices EXCLUDE TAX + Invoice prices INCLUDE TAX):

Ordering :
Vendor price (tax incl) = 10.5 euros 
Vendor price (tax excl) = 10 euros <---- what I entered
Tax amount (5%) = 0.5 euros
Total (tax excl) = 10 euros
Total (tax incl) = 10.5 euros

Receiving (what we have) :
Vendor price (tax incl) = 10.50 euros (= value in actual cost)
Vendor price (tax excl) = 9.55 euros 
Tax amount (10%) = 0.95 euros 
Total (tax excl) = 9.55 euros
Total (tax incl) = 10.50 euros

Receiving (what I expected) :
Vendor price (tax incl) = 11 euros (= value in actual cost)
Vendor price (tax excl) = 10 euros 
Tax amount (10%) = 1 euros 
Total (tax excl) = 10 euros
Total (tax incl) = 11 euros

Same logic than before. I expected the value in actual cost to be recalculated.

In a few words, I expect the reference value on which calculation should be based to be the one I entered when I placed the order. So, if I change the tax rate during the receiving process, I expect the actual cost to be recalculated. Here, the actual cost calculation is always based on the tax rate we have when placing the order. To me, it's not logical.

Jonathan, could you ask librarians in charge of acquisition what they think of this. I'll trust their judgment. If the logic in place is good. You can signed off these patches. If not, tell me when it's ready and I'll test it again. ;^)
Comment 6 Laurence Rault 2015-01-14 09:10:31 UTC
Set up 3 (List prices INCLUDE TAX + Invoice prices EXCLUDE TAX):

I think that Vendor price (tax excl) when receiving should be the same as calculated in Ordering. In my mind this price is kind of 'base' price for the item.
So the following seems to be logical for me : 

Ordering :
Vendor price (tax incl) = 10 euros <---- what I entered
Vendor price (tax excl) = 9.52 euros 
Tax amount (5%) = 0.48 euros
Total (tax excl) = 9.52 euros
Total (tax incl) = 10 euros

Receiving (what we have now):
Vendor price (tax incl) = 10.47 euros
Vendor price (tax excl) = 9.52 euros (= value in actual cost)
Tax amount (10%) = 0.95 euros 
Total (tax excl) = 9.52 euros
Total (tax incl) = 10.47 euros


Set up 4 (List prices EXCLUDE TAX + Invoice prices INCLUDE TAX):
is less evident : I wonder if Vendor price (tax excl) should be changed between order and reception. If it's not the case, François's proposition should be considered

But we have no library examples of case where the way of using tax include/exclude changes between ordering and receiving, so I am not really for changing the calculation just now without more real experience of how it should be.
Comment 7 Koha Team Lyon 3 2015-01-19 15:56:33 UTC
For the setup 3 (List prices INCLUDE TAX + Invoice prices EXCLUDE TAX), I would have the same analysis as Laurence : the change of tax between order and receipt should not change prices tax excl. that are the basis.
So this patch introduce the right calculation.

For the setup4 (List prices EXCLUDE TAX + Invoice prices INCLUDE TAX), It would be better that the price tax incl. was recalculated in the order receive page.

But for me, it's not the most important.  What really matters is that the calculation is right after the receipt was recorded. There's often a change of price between the order and the receipt and the calculation have to take into account the entered price.

Sonia Bouis
Comment 8 Francois Charbonnier 2015-02-24 01:41:43 UTC
In the end, about the set up 3, Laurence, Sonia, I agree with both of you.

About the set up 4, I'll keep what we have now because it shares the same logic thabn the set up 3.

I tested the patches on Biblibre's sandbox17.

Jonathan, could you sign them off on my behalf?

Thanks!
Comment 9 Jonathan Druart 2015-02-24 08:48:50 UTC
(In reply to Francois Charbonnier from comment #8)
> I tested the patches on Biblibre's sandbox17.
> 
> Jonathan, could you sign them off on my behalf?
> 
> Thanks!

Thanks François!
I will add you signoff to the commit on the remote branch.
Thanks for testing!
Comment 10 Jonathan Druart 2015-02-26 08:37:16 UTC
(In reply to Francois Charbonnier from comment #8)
> Jonathan, could you sign them off on my behalf?

Added on the remote branch :)
Comment 11 Kyle M Hall 2016-01-29 17:33:15 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Francois Charbonnier from comment #8)
> > Jonathan, could you sign them off on my behalf?
> 
> Added on the remote branch :)

Jonathan, could you rebase your patches and attach them to this bug? I'd like to try to QA them!
Comment 12 Julian Maurice 2016-02-01 09:33:51 UTC Comment hidden (obsolete)
Comment 13 Julian Maurice 2016-02-01 09:34:02 UTC Comment hidden (obsolete)
Comment 14 Julian Maurice 2016-02-01 09:50:42 UTC
All patches rebased on master.
Also, sandbox 17 has been updated with those patches: http://pro.test17.biblibre.com/
Comment 15 Julian Maurice 2016-02-01 10:47:32 UTC Comment hidden (obsolete)
Comment 16 Julian Maurice 2016-03-09 14:52:18 UTC Comment hidden (obsolete)
Comment 17 Julian Maurice 2016-03-09 14:52:29 UTC Comment hidden (obsolete)
Comment 18 Julian Maurice 2016-03-09 14:54:05 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2016-03-29 22:21:25 UTC
Dependent bug 13321 is Failed QA. Please reset status once 13321 is ok.
Comment 20 Alex Arnaud 2016-06-03 10:11:29 UTC
Created attachment 51982 [details] [review]
Bug 13323: Tax rate can change on receiving - DB changes

This patch adds 4 new DB fields to the aqorders table:
 * tax_rate_on_ordering
 * tax_rate_on_receiving
 * tax_value_on_ordering
 * tax_value_on_receiving

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 21 Alex Arnaud 2016-06-03 10:11:37 UTC
Created attachment 51983 [details] [review]
Bug 13323: Tax rate can change on receiving

This commit permits to update the tax rate on receiving.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 22 Alex Arnaud 2016-06-03 10:11:45 UTC
Created attachment 51984 [details] [review]
Bug 13323: QA fix (trailing whitespace)
Comment 23 Julian Maurice 2016-10-10 12:40:18 UTC
Created attachment 56096 [details] [review]
Bug 13323: Tax rate can change on receiving - DB changes

This patch adds 4 new DB fields to the aqorders table:
 * tax_rate_on_ordering
 * tax_rate_on_receiving
 * tax_value_on_ordering
 * tax_value_on_receiving

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 24 Julian Maurice 2016-10-10 12:40:23 UTC
Created attachment 56097 [details] [review]
Bug 13323: Tax rate can change on receiving

This commit permits to update the tax rate on receiving.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 25 Julian Maurice 2016-10-10 12:40:29 UTC
Created attachment 56098 [details] [review]
Bug 13323: QA fix (trailing whitespace)
Comment 26 Julian Maurice 2016-10-13 12:49:55 UTC
Created attachment 56387 [details] [review]
Bug 13323: Recalculate tax_value for partially received orders

Test plan:
1. Create an order with at least 2 items
2. Receive only 1 item
3. Check that the tax value of the received order line is correct on
   the parcel and invoice pages
4. Check that the tax value of the original order line is correct (on
   the basket page for example)
5. Cancel the receipt
6. Check that the tax value of the original order line is correct
Comment 27 Koha Team Lyon 3 2016-10-13 15:47:45 UTC
Created attachment 56433 [details] [review]
Bug 13323: Tax rate can change on receiving - DB changes

This patch adds 4 new DB fields to the aqorders table:
 * tax_rate_on_ordering
 * tax_rate_on_receiving
 * tax_value_on_ordering
 * tax_value_on_receiving

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>
Comment 28 Koha Team Lyon 3 2016-10-13 15:47:49 UTC
Created attachment 56434 [details] [review]
Bug 13323: Tax rate can change on receiving

This commit permits to update the tax rate on receiving.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>
Comment 29 Koha Team Lyon 3 2016-10-13 15:47:53 UTC
Created attachment 56435 [details] [review]
Bug 13323: QA fix (trailing whitespace)

Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>
Comment 30 Koha Team Lyon 3 2016-10-13 15:47:56 UTC
Created attachment 56436 [details] [review]
Bug 13323: Recalculate tax_value for partially received orders

Test plan:
1. Create an order with at least 2 items
2. Receive only 1 item
3. Check that the tax value of the received order line is correct on
   the parcel and invoice pages
4. Check that the tax value of the original order line is correct (on
   the basket page for example)
5. Cancel the receipt
6. Check that the tax value of the original order line is correct

Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>
Comment 31 Koha Team Lyon 3 2016-10-13 15:54:33 UTC
tested with all the configuration of vendor possible (GST include/exclude). 
I have tried to change the price / GST on receiving. 
I have tried partial receipt.
All works as described.
Comment 32 Nick Clemens 2016-10-27 00:59:03 UTC
Created attachment 56894 [details] [review]
Bug 13323: Tax rate can change on receiving - DB changes

This patch adds 4 new DB fields to the aqorders table:
 * tax_rate_on_ordering
 * tax_rate_on_receiving
 * tax_value_on_ordering
 * tax_value_on_receiving

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 33 Nick Clemens 2016-10-27 00:59:09 UTC
Created attachment 56895 [details] [review]
Bug 13323: Tax rate can change on receiving

This commit permits to update the tax rate on receiving.

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

Signed-off-by: Francois Charbonnier <francois.charbonnier@inlibro.com>

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 34 Nick Clemens 2016-10-27 00:59:17 UTC
Created attachment 56896 [details] [review]
Bug 13323: QA fix (trailing whitespace)

Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 35 Nick Clemens 2016-10-27 00:59:23 UTC
Created attachment 56897 [details] [review]
Bug 13323: Recalculate tax_value for partially received orders

Test plan:
1. Create an order with at least 2 items
2. Receive only 1 item
3. Check that the tax value of the received order line is correct on
   the parcel and invoice pages
4. Check that the tax value of the original order line is correct (on
   the basket page for example)
5. Cancel the receipt
6. Check that the tax value of the original order line is correct

Signed-off-by: Sonia Bouis <koha@univ-lyon3.fr>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 36 Kyle M Hall 2016-10-28 14:41:45 UTC
Pushed to master for 16.11, thanks Jonathan, Julian!