[18.0][MIG] account_ecotax_tax - #562
Conversation
a228729 to
d9ad959
Compare
|
/ocabot migration account_ecotax_tax |
cb2b74f to
9d42c54
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
Bot
added
the
stale
…_tax The goal is to be able to choose between the implementation with and without using the odoo tax mechanism. The advantages of the implementation based on Odoo tax mechanims are : - Possibility to choose if product price include or exclude the ecotax amounts - Isolate the ecotax amounts into a specifc accounting account The disadvantage is that it adds a small layer of complexity and you have to manage the tax configuration and see all those ecotax taxes on your invoices And a major difference which can be good or not depending on your use cases, the ecotax amounts are not in the turnover when using Odoo tax mechanism
…n from tests The dependency does not really ease the present test but forces us to put the test as post-installed which cant work as account_ecotax_tax changes the account_ecotax behavior Instead of isolating the tests of account_ecotax, it seems better to get rid of the AccountTestInvoicingCommon dependency. It also speed up the tests.
Currently translated at 100.0% (9 of 9 strings) Translation: account-fiscal-rule-16.0/account-fiscal-rule-16.0-account_ecotax_tax Translate-URL: https://translation.odoo-community.org/projects/account-fiscal-rule-16-0/account-fiscal-rule-16-0-account_ecotax_tax/it/
… excluded tax This is the main real use case and the price include case does not work at the moment
9d42c54 to
0b78f90
Compare
github-actions
Bot
removed
the
stale
| # result = quantity and product.weight_based_ecotax * quantity or 0.0 | ||
| result = quantity and product.fixed_ecotax * quantity or 0.0 | ||
| # for weight based ecotax: | ||
| # quantity and product.weight_based_ecotax * quantity or 0.0 |
There was a problem hiding this comment.
comments are not allowed anymore, I run into the erreor :
Malformed formula '# price_unit
# product: product.product object or None
# partner: res.partner object or None
# for weight based ecotax:
# quantity and product['weight_based_ecotax'] * quantity or 0.0
quantity and product['fixed_ecotax'] * quantity or 0.0' at position 0
I guess we have to remove the whole comment.
I am not sure if there is a reason to create 2 ecotax taxes, one for fixed amount an the other one for weight amount, like suggested by the comment.
Is there any reason not to used the product.ecotax_amount instead, to have the whole amount ?
Also, if we only have one excotax, tax, I guess it would make sense to create it by default ? Well this could be a latter improvement of course, not really related to the migration.
Otherwise, it seems to work as expected.
There was a problem hiding this comment.
comments are not allowed anymore, I run into the erreor :
Malformed formula '# price_unit # product: product.product object or None # partner: res.partner object or None # for weight based ecotax: # quantity and product['weight_based_ecotax'] * quantity or 0.0 quantity and product['fixed_ecotax'] * quantity or 0.0' at position 0
You are right, we also removed it for our other customer who tested it all with their accountant:
I guess we have to remove the whole comment. I am not sure if there is a reason to create 2 ecotax taxes, one for fixed amount an the other one for weight amount, like suggested by the comment. Is there any reason not to used the
product.ecotax_amountinstead, to have the whole amount ?
Look I didn't design the module and I'm not sure either about that... (may be it came from C2C?) All I can tell you is that we got an accountant validated everything for France when configured as above.
Also, if we only have one excotax, tax, I guess it would make sense to create it by default ? Well this could be a latter improvement of course, not really related to the migration.
Done. Notice I had to hard-code it for country_id France to get tests pass. But it is also noupdate=True so I think this is de good default.
Otherwise, it seems to work as expected.
That's what we found as well...
0b78f90 to
6571cc1
Compare
b913f39 to
09a6d72
Compare
09a6d72 to
187a68e
Compare
florian-dacosta
left a comment
florian-dacosta
left a comment
There was a problem hiding this comment.
Minor remark I think it would be good to change but not blocking.
Done. Notice I had to hard-code it for country_id France to get tests pass. But it is also noupdate=True so I think this is de good default.
No sure about this one, not sure it is a good thing to make a change so the test pass, can't we adapt the test if needed.
Anyway, we may go this way and see how we adapt, I think that the ecotax management for foreign countries is not well managed in the module yet, we'll probably have to improve stuff around this.
| # result = quantity and product.weight_based_ecotax * quantity or 0.0 | ||
| result = quantity and product.fixed_ecotax * quantity or 0.0 | ||
| self.formula = """ | ||
| quantity and product.fixed_ecotax * quantity or 0.0 |
There was a problem hiding this comment.
Well, since there is no obvious reason to have one dedicated tax to fixed ecotax and one for weight ecotax, I'd prefer to switch on one taxe for both case by default, seems simpler.
If it becomes an issue, we'll just change back and add a comment somewhere to explain why it would not be advised.
In the meantime, one can still duplicated the tax manually to have both if he likes.
The only reason I see to add multiple ecotax tax is to manage multiple account.account. But, we could also want multiple fixed amount tax, it depends on the accountant I guess.
| <field name="is_ecotax" eval="True" /> | ||
| <field | ||
| name="formula" | ||
| >quantity and product.fixed_ecotax * quantity or 0.0</field> |
There was a problem hiding this comment.
Well, since there is no obvious reason to have one dedicated tax to fixed ecotax and one for weight ecotax, I'd prefer to switch on one taxe for both case by default, seems simpler.
If it becomes an issue, we'll just change back and add a comment somewhere to explain why it would not be advised.
In the meantime, one can still duplicated the tax manually to have both if he likes.
The only reason I see to add multiple ecotax tax is to manage multiple account.account. But, we could also want multiple fixed amount tax, it depends on the accountant I guess.
4 participants
standard migration from the 17.0 migration #561