[16.0][FIX] account_chart_update: avoid fiscal position duplication when it's archived #1973
Conversation
| fiscal_position = self.env["account.fiscal.position"].search( | ||
| [("name", "=", self.fp_template.name), ("company_id", "=", self.company.id)] | ||
| ) |
There was a problem hiding this comment.
issue: the test is incomplete AFAICS.
The use case we're testing involves a fiscal position template with tax mappings, and a fiscal position with the same tax mappings.
The issue was when those tax mappings were creating duplicate keys, so without tax mappings, the test is incomplete.
There was a problem hiding this comment.
Ok, I've made one mapping but I'm not sure if its the right way to declare them. I'll leave this review open to check this out.
There was a problem hiding this comment.
If this assertion passes, then the test is OK:
| fiscal_position = self.env["account.fiscal.position"].search( | |
| [("name", "=", self.fp_template.name), ("company_id", "=", self.company.id)] | |
| ) | |
| fiscal_position = self.env["account.fiscal.position"].search( | |
| [("name", "=", self.fp_template.name), ("company_id", "=", self.company.id)] | |
| ) | |
| self.assertEqual(fiscal_position.tax_ids.tax_src_id.name, self.tax_template.name) | |
| self.assertEqual(fiscal_position.tax_ids.tax_dest_id.name, self.tax_template.name) |
edlopen
force-pushed
the
16.0-fix-account_chart_update-fiscal_position_archived
branch
from
c506083
to
1998550
Compare
| fiscal_position = self.env["account.fiscal.position"].search( | ||
| [("name", "=", self.fp_template.name), ("company_id", "=", self.company.id)] | ||
| ) |
There was a problem hiding this comment.
If this assertion passes, then the test is OK:
| fiscal_position = self.env["account.fiscal.position"].search( | |
| [("name", "=", self.fp_template.name), ("company_id", "=", self.company.id)] | |
| ) | |
| fiscal_position = self.env["account.fiscal.position"].search( | |
| [("name", "=", self.fp_template.name), ("company_id", "=", self.company.id)] | |
| ) | |
| self.assertEqual(fiscal_position.tax_ids.tax_src_id.name, self.tax_template.name) | |
| self.assertEqual(fiscal_position.tax_ids.tax_dest_id.name, self.tax_template.name) |
| wizard.action_update_records() | ||
| wizard.unlink() | ||
| self.assertTrue(fiscal_position.exists()) | ||
| self.assertFalse(fiscal_position.active) |
There was a problem hiding this comment.
Please add again the assertion that checks tax mappings are OK.
There was a problem hiding this comment.
Waiting for all checks to be passed but I guess it's ready to be merged now.
There was a problem hiding this comment.
The test is failing:
2024-11-21 08:11:35,717 926 ERROR odoo odoo.addons.account_chart_update.tests.test_account_chart_update: FAIL: TestAccountChartUpdate.test_01_archived_fiscal_position
Traceback (most recent call last):
File "/__w/account-financial-tools/account-financial-tools/account_chart_update/tests/test_account_chart_update.py", line 483, in test_01_archived_fiscal_position
self.assertFalse(fiscal_position.active)
AssertionError: True is not false
There was a problem hiding this comment.
Oh! I see. I just found out that as the account.fiscal.position.template doesn't have the 'active' field, so when the wizard tries to update it makes it active again as is the default value for such field.
So now the error has been fixed but I would raise this question: Is it desirable to make those fiscal fiscal positions active again each time you run the wizard?
edlopen
force-pushed
the
16.0-fix-account_chart_update-fiscal_position_archived
branch
4 times, most recently
from
6fdf2ce
to
52aed36
Compare
…hen it's archived
edlopen
force-pushed
the
16.0-fix-account_chart_update-fiscal_position_archived
branch
from
52aed36
to
dd6bf4f
Compare
Hi y'all!
I have encountered an error when running the chart of accounts update and one of its fiscal positions is archived, an attempt is made to recreate it and an error occurs as the unique key is duplicated. This simple correction makes the wizard take into account the fiscal positions that are archived.
@moduon MT-8097
@yajo @rafaelbn @fcvalgar please review, if you can.