Tryton - Issues

 

Issue9111

Title Taxes are lost when using Modify Header Wizard
Priority bug Status in-progress
Superseder Ensure all contextual field are in the cache to use the cache
View: 9187
Nosy List ced, edbo, pokoli, reviewbot
Type behavior Components account_asset, account_invoice, purchase, sale, sale_subscription
Assigned To pokoli Keywords review
Reviews 275031002
View: 275031002

Created on 2020-03-03.13:57:13 by pokoli, last changed by ced.

Messages
msg56699 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-27.15:53:06
Also sale_subscription service and account_asset product.
msg56698 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-27.15:49:05
The product on invoice should also have a company context set.
msg56697 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-27.15:48:02
Indeed it requires issue9187 to work correctly.
review275031002 updated at https://codereview.tryton.org/275031002/#ps286971002
msg56694 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-27.13:52:12
I've updated review275031002 with what is descrived on msg56647 but this does not seems to fix the issue.
msg56647 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-25.18:17:40
The context is not set because the value used is not in the depends and more over it can not be added in the depends because it is not a "real" field ('_parent_sale' see issue3901).
I think the proper way to fix that is to make such important context value use a direct field (computed). So in other word, add a 'company' field on sale line and purchase line.
And this should be done for all instances that are using the '*_used' attributes (so mainly product and party).
review275031002 updated at https://codereview.tryton.org/275031002/#ps289511002
msg56369 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-18.09:09:28
The problem is that line.product._context['company'] is None, so the issue is that the _parent_sale from [1] is empty, which causes that the product instance does not have the company set, and then taxes are empty.

Any idea on how to fix it? 

I've updated review275031002 to reproduce the issue on the text scenario.

[1] https://hg.tryton.org/modules/sale/file/fbe18aba79f1/sale.py#l1004
msg56347 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-17.22:58:12
No it should call on_change_product because taxes and unit price (and maybe other) must be recomputed.
The context must be fixed. It is strange because the product Many2One has company set in the context.
Also if it happens for sale, it must happen also for purchase as it has the same design.
review275031002 updated at https://codereview.tryton.org/275031002/#ps267011003
msg56344 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.20:17:56
Here is review275031002 which fixes the issue for me by using compute_unit_price instead of on_change_product

The only drawback is that this method was introduced on 5.4 series so it can not be backported to 5.2 nor 5.0. 

We should decide what to do with this versions.
msg56343 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.20:09:54
I've debuged the code and the problem is that product instance does not have the proper company set on the context so the customer_taxes returns an empty list and this causes that the existing taxes are updated. 

Resetting the product on the lines gives the taxes back.

I'm wondering if it won't be better to just call compute_unit_price to just update the unit price. AFAIU unit_price is the only value that should be updated.
msg56320 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-17.18:50:50
There is no way to know if the price was edited or not. So the re-computation of the price should always happen. This could probably be better documented but this is for another issue.
Here the lost of taxes is weird because the wizard is only calling on_change_product so it means that with the new header the tax setup is different. Could you check that after the header modification, resetting the product set or not the taxes.
msg56166 (view) Author: [hidden] (edbo) Date: 2020-03-09.22:40:42
> The lines are updated because you can change the party so price list and taxes should be recomputed.
> Your unit_price change is the expected behaviour?

No, the expected behaviour is that the unit_price stays the same, because I changed it myself.

So the wizard needs to check which fields are changed and act accordingly. When no party is changed, do not update the unit_price. Also some extra checks can be done when the party is changed and the previous unit_price was manually added, it should stay. Otherwise it can be changed.
msg56165 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-09.21:15:40
The lines are updated because you can change the party so price list and taxes should be recomputed.

Your unit_price change is the expected behaviour? My problem is that taxes are the right ones for the new customer but they are lost.
msg56163 (view) Author: [hidden] (edbo) Date: 2020-03-09.20:55:54
I also came across this bug. It's even worse. When you change the 'unit_price' in the line form, it will be changed back to the 'list_price' of the product.

Because the wizard is about changing the "header", why are the sale lines involved in this? Is about VAT update?

Also, what I find weird is that the wizard is not available in state 'quotation'. Because most of the time you make changes to your quotation before the sale becomes order. But that's a different issue I think.
msg56029 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-03.13:57:12
Steps to reproduce:

1. Create a sale for any party
2. Add a line with a product that has an accouting category with customer taxes set.
3. Click the "Modify Header" button and the Modify Button of the opened wizard.

After the wizard is closed the taxes are removed from the product. 

I will expect that the taxes are set as they come from the product data. 

I've found it on 5.4 series but I imagine that trunk is also afected.
History
Date User Action Args
2020-03-27 15:53:07cedsetcomponent: + account_asset, sale_subscription
messages: + msg56699
2020-03-27 15:49:05cedsetcomponent: + account_invoice
messages: + msg56698
2020-03-27 15:48:03cedsetsuperseder: + Ensure all contextual field are in the cache to use the cache
messages: + msg56697
2020-03-27 13:58:22reviewbotsetmessages: + msg56695
2020-03-27 13:52:12pokolisetmessages: + msg56694
2020-03-25 18:17:40cedsetmessages: + msg56647
2020-03-18 09:37:19reviewbotsetmessages: + msg56370
2020-03-18 09:09:28pokolisetmessages: + msg56369
2020-03-17 22:58:12cedsetcomponent: + purchase
messages: + msg56347
2020-03-17 20:31:22reviewbotsetnosy: + reviewbot
messages: + msg56346

Showing 10 items. Show all history (warning: this could be VERY long)