From 5efa30c30204336eb6d43119b3f6a55dcb10c9ae Mon Sep 17 00:00:00 2001 From: Nans Lefebvre Date: Tue, 15 Oct 2019 14:49:25 +0000 Subject: [PATCH] [FIX] fields, test_access_rights: get currency field in sudo This commits fixes user duplication. Suppose there is a monetary field, with a currency_field that is a related. When going through model's _create, we do: `if field.type in ('one2many', 'many2many'): self.env.cache.set(record, field, ())` knowing that thse values are false, with the intent to clean them later: `for record, field in cachetoclear` However, when setting the scalar values, we go through: `accessing cache_value = field.convert_to_cache(value, record)` In the case of the monetary field, this depends on another field value (the currency_field). If it is a related, we can access its value. However, at this point, if we check any access rights, we might use the value of a relational in cache for which the value is incorrectly set to False. In the case of the user, this is what happens: it inherits its currency_id from partner, as well as its debit_limit which depends on it. When the access if checked, company_ids is set to False in cache. So when going through the rule 'user rule', which checks that the company_ids intersects with the env.companies.ids, the result is always False. In some way this is essentially hiding the problem, but the true fix is probably not feasible in stable. opw 2086661 closes odoo/odoo#39017 X-original-commit: aa05a9359b42317261c8e7ffcadbeb9fe74dcd58 Signed-off-by: Nans Lefebvre (len) --- odoo/addons/test_access_rights/models.py | 15 ++++++++ .../test_access_rights/tests/__init__.py | 1 + .../tests/test_access_monetary_related.py | 36 +++++++++++++++++++ odoo/fields.py | 2 +- 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 odoo/addons/test_access_rights/tests/test_access_monetary_related.py diff --git a/odoo/addons/test_access_rights/models.py b/odoo/addons/test_access_rights/models.py index 1ce608325f7..52eee53979a 100644 --- a/odoo/addons/test_access_rights/models.py +++ b/odoo/addons/test_access_rights/models.py @@ -51,3 +51,18 @@ class FakeTicket(models.Model): name = fields.Char() message_partner_ids = fields.Many2many(comodel_name='res.partner') + + +class ResPartner(models.Model): + """User inherits partner, so we are implicitly adding these fields to User + This essentially reproduces the (sad) situation introduced by account. + """ + _name = 'res.partner' + _inherit = 'res.partner' + + currency_id = fields.Many2one('res.currency', compute='_get_company_currency', readonly=True) + monetary = fields.Monetary() # implicitly depends on currency_id as currency_field + + def _get_company_currency(self): + for partner in self: + partner.currency_id = partner.sudo().company_id.currency_id \ No newline at end of file diff --git a/odoo/addons/test_access_rights/tests/__init__.py b/odoo/addons/test_access_rights/tests/__init__.py index 1e05ba54c86..3ae99ca28b4 100644 --- a/odoo/addons/test_access_rights/tests/__init__.py +++ b/odoo/addons/test_access_rights/tests/__init__.py @@ -1,3 +1,4 @@ from . import test_ir_rules from . import test_feedback from . import test_check_access +from . import test_access_monetary_related diff --git a/odoo/addons/test_access_rights/tests/test_access_monetary_related.py b/odoo/addons/test_access_rights/tests/test_access_monetary_related.py new file mode 100644 index 00000000000..176bf9a73bf --- /dev/null +++ b/odoo/addons/test_access_rights/tests/test_access_monetary_related.py @@ -0,0 +1,36 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +import odoo.tests + + +class TestMonetaryAccess(odoo.tests.HttpCase): + + def test_monetary_access_create(self): + """Monetary fields that depend on compute/related currency + have never really been supported by the ORM. + However most currency fields are related. + This limitation can cause monetary fields to not be rounded, + as well as trigger spurious ACL errors. + """ + user_admin = self.env.ref("base.user_admin") + user_demo = self.env.ref("base.user_demo").with_user(user_admin) + + # this would raise without the fix introduced in this commit + new_user = user_demo.copy({'monetary': 1/3}) + + # The following is here to document how the ORM behaves, not really part of the test; + # in particular these specific points highlight the discrepancy between what is sent + # to the database and what we get on the ORM side. + # (to be fair, these are pre-existing ORM limitations that should have been avoided + # by using more careful field definitions and testing) + self.assertEqual(new_user.currency_id.id, False, + "The cache contains the wrong value for currency.") + self.assertEqual(new_user.monetary, 1/3, + "Because of previous point, no rounding was done.") + + new_user.invalidate_cache() + + self.assertEqual(new_user.currency_id.rounding, 0.01, + "We now get the correct currency.") + self.assertEqual(new_user.monetary, 0.33, + "The value was rounded when added to the cache.") diff --git a/odoo/fields.py b/odoo/fields.py index f244eb56163..720c3bcb7c0 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1340,7 +1340,7 @@ class Monetary(Field): def convert_to_cache(self, value, record, validate=True): # cache format: float value = float(value or 0.0) - if validate and record[self.currency_field]: + if validate and record.sudo()[self.currency_field]: # FIXME @rco-odoo: currency may not be already initialized if it is # a function or related field! value = record[self.currency_field].round(value)