Taxes are lost when using Modify Header Wizard
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.
Activity
added sales type::behavior + 1 deleted label
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.> 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.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.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.assigned to @pokoli
review275031002 updated at https://codereview.tryton.org/275031002/#ps267011003
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#l1004review275031002 updated at https://codereview.tryton.org/275031002/#ps289511002
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 #3901).
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/#ps286971002
Indeed it requires #9187 (closed) to work correctly.
review275031002 updated at https://codereview.tryton.org/275031002/#ps299341002
> For me, product on invoice line must also have the company in the context because it calls *_used properties which depend on company multivalue.
> I ran the account_invoice tests with the product context having company set and I got no issue.
Did you have #9187 (closed) patch applied? I'm not able to make it work.
Main problem on invoice is that there are two invoice fields. One for the line and other for the invoice.
If I use the one from the line the taxes are missing because when the product is instanciated the company is empty. So not sure how to solve this case.On 2020-04-15 16:21, Sergi Almacellas Abellana wrote:
> > For me, product on invoice line must also have the company in the context because it calls *_used properties which depend on company multivalue.
> > I ran the account_invoice tests with the product context having company set and I got no issue.
>
> Did you have #9187 (closed) patch applied? I'm not able to make it work.
Yes.
> Main problem on invoice is that there are two invoice fields. One for the line and other for the invoice.
But only the company on invoice line is needed because it is required.
> If I use the one from the line the taxes are missing because when the product is instanciated the company is empty. So not sure how to solve this case.
I do not have this problem because there is a default_company on the
line which set it.review275031002 updated at https://codereview.tryton.org/275031002/#ps289771002
Ok, I updated the review to add the company of the line. But when running the test suite I get the following errors:
======================================================================
FAIL: /home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice.rst
Doctest: scenario_invoice.rst
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib64/python3.8/doctest.py", line 2197, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for scenario_invoice.rst
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice.rst", line 0
----------------------------------------------------------------------
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice.rst", line 153, in scenario_invoice.rst
Failed example:
invoice.tax_amount
Expected:
Decimal('20.00')
Got:
Decimal('0.00')
======================================================================
FAIL: /home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice_supplier.rst
Doctest: scenario_invoice_supplier.rst
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib64/python3.8/doctest.py", line 2197, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for scenario_invoice_supplier.rst
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice_supplier.rst", line 0
----------------------------------------------------------------------
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice_supplier.rst", line 118, in scenario_invoice_supplier.rst
Failed example:
invoice.tax_amount
Expected:
Decimal('10.00')
Got:
Decimal('0.00')
======================================================================
FAIL: /home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_credit_note.rst
Doctest: scenario_credit_note.rst
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib64/python3.8/doctest.py", line 2197, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for scenario_credit_note.rst
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_credit_note.rst", line 0
----------------------------------------------------------------------
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_credit_note.rst", line 99, in scenario_credit_note.rst
Failed example:
invoice.total_amount
Expected:
Decimal('-220.00')
Got:
Decimal('-200.00')
======================================================================
FAIL: /home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice_alternate_currency.rst
Doctest: scenario_invoice_alternate_currency.rst
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib64/python3.8/doctest.py", line 2197, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for scenario_invoice_alternate_currency.rst
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice_alternate_currency.rst", line 0
----------------------------------------------------------------------
File "/home/pokoli/projectes/nclone/trytond/trytond/modules/account_invoice/tests/scenario_invoice_alternate_currency.rst", line 145, in scenario_invoice_alternate_currency.rst
Failed example:
invoice.tax_amount
Expected:
Decimal('40.00')
Got:
Decimal('0.00')
All are related to not properly picking the right taxes for the invoice.review275031002 updated at https://codereview.tryton.org/275031002/#ps256971002
This is because the call to on_change_product depends implicitly of the company field to correctly instantiate the product field.
As we can not manage to keep on on_change's up to date with the proper depends, I created #9236 (closed) to add automatically the depends.I can confirm that #9236 (closed) makes the invoice module test pass.
New changeset c7fd1a45e241 by Sergi Almacellas Abellana in branch 'default':
Add company as product context field
https://hg.tryton.org/modules/account_asset/rev/c7fd1a45e241New changeset 6dc86cd483bd by Sergi Almacellas Abellana in branch 'default':
Add company as product context field
https://hg.tryton.org/modules/account_invoice/rev/6dc86cd483bdNew changeset 8a091dd118ff by Sergi Almacellas Abellana in branch 'default':
Add company as product context field
https://hg.tryton.org/modules/purchase/rev/8a091dd118ffNew changeset 1af59832ba84 by Sergi Almacellas Abellana in branch 'default':
Add company as product context field
https://hg.tryton.org/modules/sale/rev/1af59832ba84New changeset 86b81ab7f63a by Sergi Almacellas Abellana in branch 'default':
Add company as product context field
https://hg.tryton.org/modules/sale_subscription/rev/86b81ab7f63aNew changeset 6e82c234229e by Sergi Almacellas Abellana in branch 'default':
Add company as product context field
https://hg.tryton.org/tryton-env/rev/6e82c234229ementioned in issue #9187 (closed)
mentioned in issue #9263 (closed)
mentioned in issue #9380 (closed)
mentioned in issue #12295 (closed)
