From 0d30cc2bc9b9cc2b805d6c2d0a440f185c648da0 Mon Sep 17 00:00:00 2001 From: william-andre Date: Fri, 23 Jun 2023 09:42:46 +0000 Subject: [PATCH] [IMP] models: manage sub companies with `check_company` Allow some models to be delegated to the main company of the object. For instance: * Have company Parent and Child * Child has access to all the accounts of Parent * When creating a document in Child using an account of Parent, there shouldn't be a consistency error. In order to do that, a new method `_check_company_domain` has been added. This method is used when `check_company=True` is set on the field declaration to: * filter relational fields in the UI [^1] * validate relational fields when writing on them in `create` and `write` That method should also be used in the business code for models likely to be shared amongst company branches instead of hardcoding the domain based on `company_id`. The same applies on domain restriction on the field declaration: the flag `check_company` should be prefered instead of providing the domain explicitly based on `company_id`. Since a lot of validation and filtering will now be done based on `parent_of`/`child_of` of the company, some support has been added for that special case in `filtered_domain` in order to avoid doing additional queries. task-3371677 [^1]: because of that change, support has been added on the python interpreter of the client to allow concatenatng domains. Part-of: odoo/odoo#125642 --- .../pos_online_payment/tests/test_frontend.py | 6 ++- .../static/src/core/py_js/py_interpreter.js | 3 ++ odoo/addons/base/models/res_users.py | 3 ++ odoo/fields.py | 23 ++++----- odoo/models.py | 50 ++++++++++++++----- odoo/osv/expression.py | 1 + 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/addons/pos_online_payment/tests/test_frontend.py b/addons/pos_online_payment/tests/test_frontend.py index 1bcba4722c5..095ed62c459 100644 --- a/addons/pos_online_payment/tests/test_frontend.py +++ b/addons/pos_online_payment/tests/test_frontend.py @@ -69,8 +69,10 @@ class TestUi(AccountTestInvoicingCommon, OnlinePaymentCommon): cls.payment_provider_old_company_id = cls.payment_provider.company_id.id cls.payment_provider_old_journal_id = cls.payment_provider.journal_id.id - cls.payment_provider.company_id = cls.company.id - cls.payment_provider.journal_id = cls.company_data['default_journal_bank'].id + cls.payment_provider.write({ + 'company_id': cls.company.id, + 'journal_id': cls.company_data['default_journal_bank'].id, + }) cls.online_payment_method = cls.env['pos.payment.method'].create({ 'name': 'Online payment', diff --git a/addons/web/static/src/core/py_js/py_interpreter.js b/addons/web/static/src/core/py_js/py_interpreter.js index 66d7eed620a..c08b02817cb 100644 --- a/addons/web/static/src/core/py_js/py_interpreter.js +++ b/addons/web/static/src/core/py_js/py_interpreter.js @@ -176,6 +176,9 @@ function applyBinaryOp(ast, context) { throw NotSupportedError(); } } + if (left instanceof Array && right instanceof Array) { + return [...left, ...right] + } return left + right; } diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index d3f784a9f90..57c320582f9 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -278,6 +278,9 @@ class Users(models.Model): _inherits = {'res.partner': 'partner_id'} _order = 'name, login' + def _check_company_domain(self, companies=None): + return [('company_ids', 'in', models.to_company_ids(companies))] if companies else [] + @property def SELF_READABLE_FIELDS(self): """ The list of fields a user can read on their own user record. diff --git a/odoo/fields.py b/odoo/fields.py index 09da59f98d4..53a361e93f5 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -34,6 +34,7 @@ from .tools import ( image_process, merge_sequences, SQL_ORDER_BY_TYPE, is_list_of, has_list_types, html_normalize, html_sanitize, ) +from .tools.misc import unquote from .tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT from .tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT from .tools.translate import html_translate, _ @@ -2863,23 +2864,17 @@ class _Relational(Field): _description_context = property(attrgetter('context')) def _description_domain(self, env): - if self.check_company and not self.domain: + domain = self.domain(env[self.model_name]) if callable(self.domain) else self.domain # pylint: disable=not-callable + if self.check_company: + # when using check_company=True on a field on 'res.company', the + # company_id comes from the id of the current record if self.company_dependent: - if self.comodel_name == "res.users": - # user needs access to current company (self.env.company) - return "[('company_ids', 'in', allowed_company_ids[0])]" - else: - return "[('company_id', 'in', [allowed_company_ids[0], False])]" + cid = 'allowed_company_ids[0]' else: - # when using check_company=True on a field on 'res.company', the - # company_id comes from the id of the current record cid = "id" if self.model_name == "res.company" else "company_id" - if self.comodel_name == "res.users": - # User allowed company ids = user.company_ids - return f"['|', (not {cid}, '=', True), ('company_ids', 'in', [{cid}])]" - else: - return f"[('company_id', 'in', [{cid}, False])]" - return self.domain(env[self.model_name]) if callable(self.domain) else self.domain + company_domain = env[self.comodel_name]._check_company_domain(companies=unquote(cid)) + return f"({cid} and {company_domain} or []) + ({domain})" + return domain class Many2one(_Relational): diff --git a/odoo/models.py b/odoo/models.py index 0fd0baffb60..4b7d0a2f237 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -164,6 +164,18 @@ def fix_import_export_id_paths(fieldname): return fixed_external_id.split('/') +def to_company_ids(companies): + if isinstance(companies, BaseModel): + return companies.ids + elif isinstance(companies, (list, tuple)): + return companies + return [companies] + + +def check_company_domain_parent_of(self, companies): + return ['|', ('company_id', '=', False), ('company_id', 'parent_of', to_company_ids(companies))] + + class MetaModel(api.Meta): """ The metaclass of all model classes. Its main purpose is to register the models per module. @@ -3633,6 +3645,14 @@ class BaseModel(metaclass=MetaModel): raise ValueError("Expected singleton or no record: %s" % self) return self.env['ir.config_parameter'].sudo().get_param('web.base.url') + def _check_company_domain(self, companies): + """Domain to be used for company consistency between records regarding this model. + + :param companies: the allowed companies for the related record + :type companies: BaseModel or list or tuple or int or unquote + """ + return ['|', ('company_id', '=', False), ('company_id', 'in', to_company_ids(companies))] + def _check_company(self, fnames=None): """ Check the companies of the values of the given field names. @@ -3647,7 +3667,7 @@ class BaseModel(metaclass=MetaModel): User with main company A, having access to company A and B, could be assigned or linked to records in company B. """ - if fnames is None: + if fnames is None or 'company_id' in fnames: fnames = self._fields regular_fields = [] @@ -3671,33 +3691,32 @@ class BaseModel(metaclass=MetaModel): # with the company of the origin document, i.e. `self.account_id.company_id == self.company_id` for name in regular_fields: corecord = record.sudo()[name] - # Special case with `res.users` since an user can belong to multiple companies. - if corecord._name == 'res.users' and corecord.company_ids: - if not (company <= corecord.company_ids): + if corecord: + domain = corecord._check_company_domain(company) + if domain and not corecord.filtered_domain(domain): inconsistencies.append((record, name, corecord)) - elif not (corecord.company_id <= company): - inconsistencies.append((record, name, corecord)) # The second part of the check (for property / company-dependent fields) verifies that the records # linked via those relation fields are compatible with the company that owns the property value, i.e. # the company for which the value is being assigned, i.e: # `self.property_account_payable_id.company_id == self.env.company company = self.env.company for name in property_fields: - # Special case with `res.users` since an user can belong to multiple companies. corecord = record.sudo()[name] - if corecord._name == 'res.users' and corecord.company_ids: - if not (company <= corecord.company_ids): + if corecord: + domain = corecord._check_company_domain(company) + if domain and not corecord.filtered_domain(domain): inconsistencies.append((record, name, corecord)) - elif not (corecord.company_id <= company): - inconsistencies.append((record, name, corecord)) if inconsistencies: lines = [_("Incompatible companies on records:")] company_msg = _lt("- Record is company %(company)r and %(field)r (%(fname)s: %(values)s) belongs to another company.") record_msg = _lt("- %(record)r belongs to company %(company)r and %(field)r (%(fname)s: %(values)s) belongs to another company.") + root_company_msg = _lt("- Only a root company can be set on %(record)r. Currently set to %(company)r") for record, name, corecords in inconsistencies[:5]: if record._name == 'res.company': msg, company = company_msg, record + elif record == corecords and name == 'company_id': + msg, company = root_company_msg, record.company_id else: msg, company = record_msg, record.company_id field = self.env['ir.model.fields']._get(self._name, name) @@ -5713,7 +5732,14 @@ class BaseModel(metaclass=MetaModel): else: (key, comparator, value) = leaf if comparator in ('child_of', 'parent_of'): - stack.append(set(self.with_context(active_test=False).search([('id', 'in', self.ids), leaf], order='id')._ids)) + if key == 'company_id': # avoid an explicit search + value_companies = self.env['res.company'].browse(value) + if comparator == 'child_of': + stack.append({record.id for record in self if record.company_id.parent_ids & value_companies}) + else: + stack.append({record.id for record in self if record.company_id & value_companies.parent_ids}) + else: + stack.append(set(self.with_context(active_test=False).search([('id', 'in', self.ids), leaf], order='id')._ids)) continue if key.endswith('.id'): diff --git a/odoo/osv/expression.py b/odoo/osv/expression.py index 0f7220184d1..373e1e7f041 100644 --- a/odoo/osv/expression.py +++ b/odoo/osv/expression.py @@ -895,6 +895,7 @@ class expression(object): """ Return a domain implementing the parent_of operator for [(left,parent_of,ids)], either as a range using the parent_path tree lookup field (when available), or as an expanded [(left,in,parent_ids)] """ + ids = [id for id in ids if id] # ignore (left, 'parent_of', [False]) if not ids: return [FALSE_LEAF] left_model = left_model.with_context(active_test=False)