[REF] l10n_mx: Completely refactored Mexican Chart Template #14491

nhomar · 2016-11-28T10:36:52Z

Description of the issue/feature this PR addresses:

Added refactor of the CoA for Mexico.

Current behavior before PR:

The CoA is pretty useless there is now too much manual configurations to be done after install, too complicated to be used with electronic accounting in Mexico.

Desired behavior after PR is merged:

l10n_mx: Completely refactored Mexican CoA Template

Added accounts provided following the structure provided by the SAT
this accounts are only the minimal necessary ones, with the objective
to set the first easiest engagement to new users and tags will represent the actual information
from the document linked.
The name of the tag is the concatenation of tag.code + tag.name, because the account.tag
model have not the code field.

Note: we will need more accounts but some of them for special operations
which are not basic at all, then it is better just propose the operational accounts
once we understand a proper closing process.
Added the proper user-types in order to be able to use it with the official reports this CoA
for operational purpose.
Added nature field and data in tags to allow set this value in electronic account report, and
set this value with data pre loaded, we did not use user.type for this due to the fact that the same user type can have different nature (if it grows for credit or debit), I do not know if this field brakes the stable rule we can discuse about that.
Assigned the account tags with in the Mexican CoA template automatically, using a server
action (in order to set the feature configurable activating and deactivating the server action
when needed).

Rationale: The code of the accounts following the pattern 111.00.00 or 111-00-00 then it will
set the tag 111.00 and 111 IF the tag exists, if not then this will be ignored.
```
**Sample:**

102.01.01 Bank1------102.01 Bancos nacionales
102.01.02 Bank2------102.01 Bancos nacionales
102.01.02 Bank3------102.01 Bancos nacionales
```
The green color is assigned to this tags which represent the third level on the structure, the second level will be red, with such field we re-use the current data model and is easiest to filter.
Generate an account tag by each account in SAT catalog, and assign to the corresponding account.
Considered @qdp-odoo comments from this PR [IMP] l10n_mx: Changed Mexican Chart Template #13939 (review)

Closes vauxoo/mexico#330

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

nhomar · 2016-11-28T14:18:49Z

Pending:

Check why tax is showing 0 once it is paid. (It is an issue in odoo core: [10.0] account_tax_cash_basis: Amount set to 0 #14501)
Tax report is not shown Tax Pending for Payment (how to do that?)

pedrobaeza · 2016-11-28T14:25:08Z

addons/l10n_mx/data/account_configuration.yml

+-
+  !record {model: account.config.settings, id: mx_default_account_properties}:
+    {
+      'use_anglo_saxon': 1,


You shouldn't change this configuration in case of a multi-company with other localizations installed and used.

The anglosaxon feature and Odoo itself is not multicompany aware in terms of localization IMHO it is ok in this way, in case that at least 1 company have it configured the one that need multi-localization will need to make an external feature.

I don't think so. And why you need to do this then if anglosaxon is installed?

Because is illegal not process your costs as anglo_saxon does.

AFAICR @qdp-odoo mentioned that anglo saxon will be migrated inside account and not optional at all (because it is the standard in United sate also).

The point here is that you can always make an external module (a multilocalization environment is far from being managed in saas-x and at least until we finish 10 or 20 localization we will have several of this pointless discussions.

If you are installing l10n_mx it is suposed you want to comply (without human intervention extra) with the legality of México, think otherwise is pointless on this stage of the solution and bring more early problems in the incorrect time.

In the near future this will absolutely change I am sure.

No, your module shouldn't magically auto-configure what you don't want to configure in all of your instances. If you do that, other localization module can do the reverse. Then, if you install both, your system will be again "unconfigured".

Well, I am not agreed with the last statement, again, As I mentioned before, the localization is not "multi-localization" the point is autoconfigure everything.

I can say the same with any module that overwrite any method.

If you are localizing a database what I expect is that all hidden features works as expected, anglo-saxon is one of them, (you can realize this was incorrectly missed Month after you started the operations and ugly things happen).

Nhomar, you know our real case of a DB with localization for Mexico, Panama and Spain, so this auto-configuration is a real problem. Let's see what @qdp-odoo thinks about this.

For Pa and MX it works and for Spain also (if you set no real time valuation which is the way to work in spain) I do not see any problem, I have it in my production environment on this way.

I do not know how to comply with the legal part of México without this configuration the module itself should be multi/country/company aware not the configuration itself, I will try to make a Mockup for that, AFAICK this modules will be rewritten in master in the near future.

pedrobaeza · 2016-11-28T14:26:21Z

addons/l10n_mx/models/account.py

+
+    @api.multi
+    def assign_account_tag(self):
+        """Based on account code will be assigned by default the corresponding


This sounds like a hack to auto-assign tags that doesn't work well with all the localizations. You should avoid this.

This is the way proposed from Odoo itself (did you read the comment inline)

This is mandatory for the configuration of electronic accounting.
If the accounts does not work you can see the logic with the regex is completelly ignored.

For Example if you have an account XXXXXXXX then this will be early retuned and nothing happen.

And it is called from a server action, you can mark the server action as inactive and all works as original one.

OK, I'll dig deeper in the caller of this method then.

pedrobaeza · 2016-11-28T14:27:40Z

addons/l10n_mx/models/chart_template.py

+            company, code_digits=code_digits,
+            transfer_account_id=transfer_account_id, account_ref=account_ref,
+            taxes_ref=taxes_ref)
+        account_tax_obj = self.env['account.tax']


You shouldn't do this unless the localization you're installing is Mexican one. Please check the account chart you are installing before.

This code is in the l10n_mx module re-check that is redundant AFAICU.

You are overwriting a method that will be called each time you install a tax chart if l10n_mx is installed. You can have an installation with l10n_mx and l10n_es installed and both shouldn't collide. I repeat: you should check that the account chart you're installing is really the Mexican one.

I will check deeper the impact, as you know this is a WiP yet.

pedrobaeza · 2016-11-28T14:27:59Z

addons/l10n_mx/models/chart_template.py

+            self, tax_template_ref, acc_template_ref, code_digits, company):
+        res = super(AccountChartTemplate, self).generate_account(
+            tax_template_ref, acc_template_ref, code_digits, company)
+        self.env['account.account'].browse(res.values()).assign_account_tag()


You shouldn't do this unless the localization you're installing is Mexican one. Please check the account chart you are installing before.

This code is in the l10n_mx module re-check that is redundant AFAICU.

pedrobaeza · 2016-11-28T14:28:03Z

addons/l10n_mx/models/chart_template.py

+    def generate_journals(self, acc_template_ref, company, journals_dict=None):
+        res = super(AccountChartTemplate, self).generate_journals(
+            acc_template_ref, company, journals_dict=journals_dict)
+        journal_basis = self.env['account.journal'].search([


You shouldn't do this unless the localization you're installing is Mexican one. Please check the account chart you are installing before.

This code is in the l10n_mx module re-check that is redundant AFAICU.

pedrobaeza · 2016-11-28T14:28:09Z

addons/l10n_mx/models/chart_template.py

+            self, acc_template_ref, company, journals_dict=None):
+        res = super(AccountChartTemplate, self)._prepare_all_journals(
+            acc_template_ref, company, journals_dict=journals_dict)
+        res.append({


You shouldn't do this unless the localization you're installing is Mexican one. Please check the account chart you are installing before.

This code is in the l10n_mx module re-check that is redundant AFAICU.

pedrobaeza · 2016-11-28T14:28:34Z

addons/l10n_mx/models/chart_template.py

+        # Overwrite this account code because this is created by python code
+        # https://goo.gl/tiqfVm, and to get the chart template like the
+        # Mexican charts needs.
+        account = account_obj.search([


You shouldn't do this unless the localization you're installing is Mexican one. Please check the account chart you are installing before. Anyway, I think there should be a more elegant way to do this.

This code is in the l10n_mx module re-check that is redundant AFAICU.

Done (against what I think) but if it helps to show a little need in the ORM for a multilocalization environments then GO ahead.

As you can see the need is the same check for every method, that can be done with a little decorator ever writable in the model. (let's see)

Use instead if not self == self.env.ref('l10n_mx.vauxoo_mx_chart_template') (and you should rename the XML-ID 😉)

Ok, Done you win (I am making conclusions with the information of the huge amount of bugs related to this since v5.0 but I tested and YES it is possible to load with admin correctly another COAand be consistent), thanks for point that out.

Hehe, man, I'm not doing this for winning, just to get the best of you! 😄

pedrobaeza · 2016-11-29T08:47:28Z

addons/l10n_mx/models/chart_template.py

@@ -17,6 +17,8 @@ def _load_template(
            company, code_digits=code_digits,
            transfer_account_id=transfer_account_id, account_ref=account_ref,
            taxes_ref=taxes_ref)
+        if not self.env.user.company_id.country_id == self.env.ref('base.mx'):


It's better to just see if self is the Mexican chart template. You can install a localization with admin being in another company.

It is a bad practice to do that, a lot of things depends of the company of the user, you can not install the localization like that.

I do not like check the module I prefer the country itself. I will think on that BTW.

No, that's not true for the chart template installation, that can be done from Settings menu under Accounting. Anyway, I have already told you how to check this properly in other comment.

qdp-odoo · 2016-12-01T12:27:28Z

@nhomar sorry, I'll check that beginning of next week.

related to the anglo-saxon discussion you had: you should just set the boolean on the chart template. No need to touch the settings for that

nhomar · 2016-12-03T05:03:56Z

Thanks @qdp-odoo I did not know such feature which is company dependent, that's nice!

It is already done as asked "everybody happy"

qdp-odoo · 2016-12-06T08:40:46Z

addons/l10n_mx/models/chart_template.py

+        return accounts, taxes
+
+    @api.multi
+    def generate_account(


we don't need this as we can assign tags on the account.template and they will be copied on the accounts

The point here was the fact of add 2 tags and we agreed in add only one, it is pointless add both we can save the parent as a report.line which is by far better to maintain than add 2 tags and code.

I am removing the search of the 2nd level tag (the one with 3 chars) and letting only the assignation of the last level tag.

I will delete all tags from data that represent this tags also.

thanks.

qdp-odoo · 2016-12-06T08:41:25Z

addons/l10n_mx/__manifest__.py

+        "account",
+        "base_vat",
+        "account_tax_cash_basis",
+        "base_action_rule",


remove dependancy to base_action_rule which is unneeded and unwanted

qdp-odoo · 2016-12-06T08:49:58Z

addons/l10n_mx/models/account.py

+            return account_tag_obj
+        account = self.get_account_tuple()
+        tag_parent = account_tag_obj.search([
+            ('name', 'ilike', str(account[0])),


I suppose it should be [('name', '=', str(account[0])), ('color', '=', 1)]

because you want the tag code to be equal to the 1st element of the account code. No?

the tag code is:

COD.E NAME_OF_TAG because the tag do not have code information.

i.e: a tag is "102.02 Banks" then we are looking by the first lart of the tag that's why we are using ilike.

Is that ok for you? or do you think in another approach¿?

qdp-odoo · 2016-12-06T08:50:41Z

addons/l10n_mx/models/account.py

+            ('name', 'ilike', str(account[0])),
+            ('color', '=', 1)])
+        tag = account_tag_obj.search([
+            ('name', 'ilike', '%s.%s' % (account[0], account[1])),


same remark => ('name', '=', '%s.%s' % (account[0], account[1])) ??

the tag code is:

COD.E NAME_OF_TAG because the tag do not have code information.

i.e: a tag is "102.02 Banks" then we are looking by the first lart of the tag that's why we are using ilike.

Is that ok for you? or do you think in another approach¿?

ok for the name like this, don't you want to use '=like' then?

qdp-odoo · 2016-12-06T08:53:46Z

addons/l10n_mx/models/account.py

+        account_tag_obj = self.env['account.account.tag']
+        if not self.code:
+            return account_tag_obj
+        if not self.is_valid_account_number():


if not self.search_regex(): return account_tag_obj

function is_valid_account_number() is useless as it return the boolean value of somthing that can already be casted as a boolean value

qdp-odoo · 2016-12-06T09:01:02Z

addons/l10n_mx/models/chart_template.py

+
+    @api.model
+    def generate_journals(self, acc_template_ref, company, journals_dict=None):
+        res = super(AccountChartTemplate, self).generate_journals(


add docstring:
"""set the tax_cash_basis_journal_id on the company"""

qdp-odoo · 2016-12-06T09:01:41Z

addons/l10n_mx/models/chart_template.py

+        return res
+
+    @api.multi
+    def _prepare_all_journals(


add docstring:
"""create the tax_cash_basis_journal_id"""

qdp-odoo · 2016-12-06T09:37:12Z

addons/l10n_mx/models/account.py

+    _inherit = 'account.account.tag'
+
+    nature = fields.Selection([
+        ('D', _('Debitable Account')), ('A', _('Creditable Account'))],


why not using 'D' and 'C' as values? it would have been more meaningful

We decided use directly the value that we actually need in the xml:

But I can left this english valid and convert afterwards (Are you agreed left this on this way? or do you prefer another approach?)

okay it's fine for me that way ;-)

qdp-odoo · 2016-12-06T10:29:05Z

addons/l10n_mx/models/chart_template.py

+            ('code', '=', '999999'), ('user_type_id', '=', self.env.ref(
+                "account.data_unaffected_earnings").id)])
+        account.write({'code': '811.01.01'})
+        account.assign_account_tag()


the above part isn't needed anymore since: a20d1e9

indeed, you can now create the 'undistributed profits/losses' account template directly in the CoA and assign the right code and tags directly.

qdp-odoo · 2016-12-06T10:32:16Z

addons/l10n_mx/models/chart_template.py

+        account_obj.search([
+            ('code', 'in', ('102.01.02', '101.01.01')),
+            ('user_type_id', '=', type_liquidity)]).assign_account_tag()
+        return res


for this last part, you should inherit _prepare_liquidity_account() and assign there the tags for the liquidity accounts. That way, the tags will be assigned also if you create manually bank/cash journals afterward.

So, with the above comment, it means you don't need to overwrite execute() function anymore.

- Changed the mexican chart template by the accounts provided by the SAT this accounts are only the minimal necessary ones, with the objective to set the first engagement easiest. The mexican electronic accounting is according this file: http://goo.gl/7Olr4J we will need more accounts but some of them for special operations which are not basic at all, then it is better just propose the operational ones once we understand a proper closing process with version 10.0. Added the proper user-types in order to be able to use it with the official reports this CoA. - Assigned the account tags with in the Mexican CoA template automatically, using the code of the accounts following the pattern 111.00.00 or 111-00-00 then it will set the tag 111.00 and 111 IF the tag exists, if not then this will be ignored, With this feature. Added method to be used in a server action that set account tag in each account when this is created or updated, this server action search account tag with the same code and assign the parent and corresponding tag. Added nature field and data in tags to allow set this value in electronic account report, and set this value with data data Sample: 102.01.01 Bank1------102.01 Bancos nacionales 102.01.02 Bank2------102.01 Bancos nacionales 102.01.02 Bank3------102.01 Bancos nacionales - Created the account tag data based in the SAT document (http://goo.gl/xTGz7s), where each account parent is a new tag in this data. The name is the concatenation of tag.code + tag.name, because the account.tag model have not the code field. The green color is assigned to this tags. - Generate an account tag by each account in SAT catalog, and assign to the corresponding account. This tags are red color. And are created, because could be created more accounts out of SAT catalog, and that have relation to one same tag. - l10n_mx: load chart templates multi country/company aware. - Use anglo saxon in the CoA l10n_mx.

qdp-odoo · 2016-12-09T14:55:07Z

merged at revision 9a0e3a1

:-)

thanks for the contribution guys

nhomar force-pushed the 10.0-l10n_mx branch from b15db7f to c3fe147 Compare November 28, 2016 13:44

pedrobaeza requested changes Nov 28, 2016

View reviewed changes

pedrobaeza reviewed Nov 29, 2016

View reviewed changes

nhomar force-pushed the 10.0-l10n_mx branch 3 times, most recently from f38117f to c9241d0 Compare November 30, 2016 08:11

mart-e assigned qdp-odoo Dec 2, 2016

nhomar force-pushed the 10.0-l10n_mx branch from c9241d0 to 91f3f39 Compare December 3, 2016 04:40

nhomar force-pushed the 10.0-l10n_mx branch 3 times, most recently from 7debc3b to 33de336 Compare December 6, 2016 06:31

qdp-odoo requested changes Dec 6, 2016

View reviewed changes

nhomar force-pushed the 10.0-l10n_mx branch 4 times, most recently from 5fd8036 to 93bd767 Compare December 7, 2016 14:29

nhomar force-pushed the 10.0-l10n_mx branch from 93bd767 to 2b14695 Compare December 7, 2016 15:55

qdp-odoo closed this Dec 9, 2016

luisg123v deleted the 10.0-l10n_mx branch July 14, 2022 01:05

[REF] l10n_mx: Completely refactored Mexican Chart Template #14491

[REF] l10n_mx: Completely refactored Mexican Chart Template #14491

Conversation

nhomar commented Nov 28, 2016 • edited

Current behavior before PR:

Desired behavior after PR is merged:

l10n_mx: Completely refactored Mexican CoA Template

nhomar commented Nov 28, 2016 • edited

Pending:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhomar Nov 29, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qdp-odoo commented Dec 1, 2016

nhomar commented Dec 3, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qdp-odoo commented Dec 9, 2016

nhomar commented Nov 28, 2016 •

edited

nhomar commented Nov 28, 2016 •

edited

nhomar Nov 29, 2016 •

edited

nhomar commented Dec 3, 2016 •

edited