From 68a425d113e6b6b14a45eb6a5876ba0e63987d0f Mon Sep 17 00:00:00 2001 From: Silvio Gregorini Date: Thu, 9 Jan 2020 01:37:17 +0100 Subject: [PATCH] [FIX][l10n_it_account] Fix: better management of account_balance_signs upon account types and account groups --- l10n_it_account/models/account_account.py | 11 +- l10n_it_account/models/account_group.py | 119 +++++++++++++++++----- l10n_it_account/models/account_type.py | 49 ++++++++- 3 files changed, 149 insertions(+), 30 deletions(-) diff --git a/l10n_it_account/models/account_account.py b/l10n_it_account/models/account_account.py index 14fe17f9efab..fdea395891d0 100644 --- a/l10n_it_account/models/account_account.py +++ b/l10n_it_account/models/account_account.py @@ -7,5 +7,12 @@ class Account(models.Model): _inherit = 'account.account' @api.constrains('group_id') - def check_group_constrain_account_types(self): - self.mapped('group_id').check_constrain_account_types() + def check_balance_sign_coherence(self): + """ + Checks whether adding an account to (or removing it from) a group + creates incoherencies in account groups' balance signs. + """ + groups = self.mapped('group_id') + # Avoid check upon empty recordset to make it faster + if groups: + groups.check_balance_sign_coherence() diff --git a/l10n_it_account/models/account_group.py b/l10n_it_account/models/account_group.py index 2415f5ff86a8..87fbd44f47b6 100644 --- a/l10n_it_account/models/account_group.py +++ b/l10n_it_account/models/account_group.py @@ -1,6 +1,6 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -from odoo import _, api, models, fields +from odoo import _, api, fields, models from odoo.exceptions import ValidationError @@ -10,33 +10,100 @@ class AccountGroup(models.Model): account_ids = fields.One2many( comodel_name='account.account', inverse_name='group_id', - string="Accounts") + string="Accounts", + ) account_balance_sign = fields.Integer( - "Balance sign", compute="_compute_account_balance_sign") - - @api.constrains('account_ids') - def check_constrain_account_types(self): - error_msg = '\n'.join([ - _("Cannot link accounts of different types to group '{}'." - .format(group.name)) - for group in self - if len(group.account_ids.mapped('user_type_id')) > 1 - ]) - if error_msg: - raise ValidationError(error_msg) - - def get_first_account_type(self): - if self.account_ids: - return self.account_ids[0].user_type_id - children = self.search([('parent_id', '=', self.id)]) - for child in children: - return child.get_first_account_type() + compute="_compute_account_balance_sign", + string="Balance sign", + ) + + @api.constrains('parent_id') + def check_parent_recursion(self): + for group in self: + try: + group.get_group_parents() + except ValidationError as err: + raise ValidationError( + _("Can't set '{}' as parent for group '{}'." + "\n{}") + .format(group.parent_id.name_get()[0][-1], + group.name_get()[0][-1], + err.name) + ) + + @api.constrains('account_ids', 'parent_id') + def check_balance_sign_coherence(self): + """ + Checks whether every group (plus parents and subgroups) have the same + balance sign. This is done by first retrieving every group's progenitor + and then checking, for each of them, the account types' for accounts + linked to such progenitor group and its subgroups. + """ + # Force recursion check + self.check_parent_recursion() + done_group_ids, progenitor_ids = [], [] + for group in self: + if group.id in done_group_ids: + continue + progenitor = group.get_group_progenitor() + progenitor_ids.append(progenitor.id) + done_group_ids.extend(progenitor.get_group_subgroups().ids) + + for progenitor in self.browse(tuple(set(progenitor_ids))): + accounts = progenitor.get_group_accounts() + if not accounts.mapped('user_type_id').have_same_sign(): + raise ValidationError( + _("Incoherent balance signs for '{}' and its subgroups.") + .format(progenitor.name_get()[0][-1]) + ) @api.multi def _compute_account_balance_sign(self): for group in self: - acc_type = group.get_first_account_type() - if acc_type: - group.account_balance_sign = acc_type.account_balance_sign - else: - group.account_balance_sign = 1 + group.account_balance_sign = group.get_account_balance_sign() + + def get_account_balance_sign(self): + self.ensure_one() + progenitor = self.get_group_progenitor() + types = progenitor.get_group_accounts().mapped('user_type_id') + if types: + return types[0].account_balance_sign + return 1 + + def get_group_accounts(self): + """ Retrieves every account from `self` and `self`'s subgroups. """ + return (self + self.get_group_subgroups()).mapped('account_ids') + + def get_group_progenitor(self): + self.ensure_one() + if not self.parent_id: + return self + return self.get_group_parents().filtered(lambda g: not g.parent_id) + + def get_group_parents(self): + """ + Retrieves every parent for group `self`. + :return: group's parents as recordset, or empty recordset if `self` + has no parents. If a recursion is found, an error is raised. + """ + self.ensure_one() + parent_ids = [] + parent = self.parent_id + while parent: + parent_ids.append(parent.id) + parent = parent.parent_id + if parent == self: + raise ValidationError( + _("A recursion in '{}' parents has been found.") + .format(self.name_get()[0][-1]) + ) + return self.browse(parent_ids) + + def get_group_subgroups(self): + """ Retrieves every subgroup for groups `self`. """ + # Avoid recursion upon empty recordsets + if not self: + return self + subgroups = self.search([('parent_id', 'in', self.ids)]) + subgroup_ids = subgroups.ids + subgroups.get_group_subgroups().ids + return self.browse(tuple(set(subgroup_ids))) diff --git a/l10n_it_account/models/account_type.py b/l10n_it_account/models/account_type.py index 26cde5f97dd5..2189d692cbbb 100644 --- a/l10n_it_account/models/account_type.py +++ b/l10n_it_account/models/account_type.py @@ -1,4 +1,7 @@ -from odoo import models, fields, api +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). + +from odoo import _, api, fields, models +from odoo.exceptions import ValidationError ACCOUNT_TYPES_NEGATIVE_SIGN = [ 'account.data_unaffected_earnings', @@ -14,7 +17,11 @@ class AccountType(models.Model): _inherit = 'account.account.type' - account_balance_sign = fields.Integer("Balance sign", default=1) + + account_balance_sign = fields.Integer( + default=1, + string="Balance sign", + ) @api.model def set_account_types_negative_sign(self): @@ -22,3 +29,41 @@ def set_account_types_negative_sign(self): acc_type = self.env.ref(xml_id, raise_if_not_found=False) if acc_type: acc_type.account_balance_sign = -1 + + @api.constrains('account_balance_sign') + def check_balance_sign_value(self): + """ + Checks whether `account_balance_sign` gets a correct value of +1 or -1. + """ + if any(t.account_balance_sign not in (-1, 1) for t in self): + raise ValidationError( + _("Balance sign's value can only be 1 or -1.") + ) + + @api.constrains('account_balance_sign') + def check_balance_sign_coherence(self): + """ + Checks whether changes upon `account_balance_sign` create incoherencies + in account groups' balance signs. + """ + # Force check upon sign itself before checking groups signs coherence + self.check_balance_sign_value() + acc_obj = self.env['account.account'] + for account_type in self: + accounts = acc_obj.search([('user_type_id', '=', account_type.id)]) + # Avoid check upon empty recordset to make it faster + if accounts: + accounts.check_balance_sign_coherence() + + def have_same_sign(self): + """ + Checks types' signs. + :return: True if there's nothing to check or there's only one type + to check; else, returns True or False according to whether every + type has the same value for `account_balance_sign` (if it's not 0). + """ + to_check = self.filtered(lambda t: t.account_balance_sign) + if len(to_check) <= 1: + return True + benchmark = to_check[0].account_balance_sign + return all(t.account_balance_sign == benchmark for t in to_check)