From 3f4407eb2367e126a0cda58b9f1dda86f2a5763c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20BEAU?= Date: Fri, 3 Apr 2020 22:33:21 +0200 Subject: [PATCH 1/2] [FIX] fix issue when creating binding from the main record Indeed the implementation of write on one2many (see odoo/fields.py:2235) call the method create/write in mode norecompute This mean that the field is_urls_sync_required is not computed after the call to super of write/create of the binding but later during the create/write of the main record. To solve the issue we sync the url after calling the method "_recompute_done". This also simplify the code \o/ --- base_url/models/abstract_url.py | 18 +++++----------- base_url/tests/models.py | 18 +++++++++++++++- base_url/tests/models_mixin.py | 1 + base_url/tests/test_abstract_url.py | 32 ++++++++++++++++++++++++++++- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/base_url/models/abstract_url.py b/base_url/models/abstract_url.py index 86eefb09bf..afa5346abe 100644 --- a/base_url/models/abstract_url.py +++ b/base_url/models/abstract_url.py @@ -237,19 +237,11 @@ def _sync_urls(self): record.set_url(record.url_key) return records - @api.model - def create(self, value): - res = super(AbstractUrl, self).create(value) - synced = res._sync_urls() - super(AbstractUrl, synced).write({"is_urls_sync_required": False}) - return res - - @api.multi - def write(self, value): - res = super(AbstractUrl, self).write(value) - synced = self._sync_urls() - super(AbstractUrl, synced).write({"is_urls_sync_required": False}) - return res + def _recompute_done(self, field): + super(AbstractUrl, self)._recompute_done(field) + if field.name == "is_urls_sync_required": + synced = self.exists()._sync_urls() + synced.write({"is_urls_sync_required": False}) @api.multi def unlink(self): diff --git a/base_url/tests/models.py b/base_url/tests/models.py index 850bca81ce..e511cc3d85 100644 --- a/base_url/tests/models.py +++ b/base_url/tests/models.py @@ -18,6 +18,15 @@ class UrlBackendFake(models.Model, TestMixin): name = fields.Char(required=True) +class ResPartner(models.Model, TestMixin): + _name = "res.partner" + _inherit = "res.partner" + _test_teardown_no_delete = True + _test_purge_fields = ["binding_ids"] + + binding_ids = fields.One2many("res.partner.addressable.fake", "record_id") + + class ResPartnerAddressableFake(models.Model, TestMixin): _name = "res.partner.addressable.fake" _inherit = "abstract.url" @@ -25,8 +34,15 @@ class ResPartnerAddressableFake(models.Model, TestMixin): _description = "Fake partner addressable" backend_id = fields.Many2one(comodel_name="url.backend.fake") + special_code = fields.Char() @api.multi - @api.depends("lang_id", "record_id.name") + @api.depends("lang_id", "special_code", "record_id.name") def _compute_automatic_url_key(self): self._generic_compute_automatic_url_key() + + def _get_url_keywords(self): + res = super(ResPartnerAddressableFake, self)._get_url_keywords() + if self.special_code: + res += [self.special_code] + return res diff --git a/base_url/tests/models_mixin.py b/base_url/tests/models_mixin.py index 214a6dbbaf..2f0b264d26 100644 --- a/base_url/tests/models_mixin.py +++ b/base_url/tests/models_mixin.py @@ -59,6 +59,7 @@ def _test_teardown_model(cls, env): model = env[cls._name] if fname in model: model._pop_field(fname) + model._proper_fields.remove(fname) if not getattr(cls, "_test_teardown_no_delete", False): del env.registry.models[cls._name] diff --git a/base_url/tests/test_abstract_url.py b/base_url/tests/test_abstract_url.py index 9c4b8c1283..4a7e169bb8 100644 --- a/base_url/tests/test_abstract_url.py +++ b/base_url/tests/test_abstract_url.py @@ -5,7 +5,7 @@ from odoo.exceptions import ValidationError from odoo.tests import SavepointCase -from .models import ResPartnerAddressableFake, UrlBackendFake +from .models import ResPartner, ResPartnerAddressableFake, UrlBackendFake class TestAbstractUrl(SavepointCase): @@ -14,6 +14,7 @@ def setUpClass(cls): super(TestAbstractUrl, cls).setUpClass() UrlBackendFake._test_setup_model(cls.env) ResPartnerAddressableFake._test_setup_model(cls.env) + ResPartner._test_setup_model(cls.env) cls.lang = cls.env.ref("base.lang_en") cls.UrlUrl = cls.env["url.url"] cls.ResPartnerAddressable = cls.env["res.partner.addressable.fake"] @@ -27,6 +28,7 @@ def setUpClass(cls): def tearDownClass(cls): ResPartnerAddressableFake._test_teardown_model(cls.env) UrlBackendFake._test_teardown_model(cls.env) + ResPartner._test_teardown_model(cls.env) super(TestAbstractUrl, cls).tearDownClass() def _get_default_partner_value(self): @@ -125,3 +127,31 @@ def test_onchange_do_nothing(self): {}, ["automatic_url_key"], {} ) self.assertEqual(result, {"value": {}}) + + def test_create_from_main(self): + partner = self.env["res.partner"].create({"name": self.name}) + partner.write( + { + "binding_ids": [ + ( + 0, + 0, + { + "lang_id": self.lang.id, + "url_builder": "auto", + "backend_id": self.url_backend.id, + }, + ) + ] + } + ) + self._check_url_key(partner.binding_ids, "partner-name") + + def test_write_from_main(self): + my_partner = self._create_auto() + my_partner.record_id.write( + {"binding_ids": [(1, my_partner.id, {"special_code": "foo"})]} + ) + self.assertEqual(2, len(my_partner.url_url_ids)) + url_keys = set(my_partner.mapped("url_url_ids.url_key")) + self.assertSetEqual(url_keys, {"partner-name-foo", "partner-name"}) From 9e0b9399e9ebe277f36fcd49cce40f3403940663 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20BEAU?= Date: Sun, 5 Apr 2020 00:36:22 +0200 Subject: [PATCH 2/2] [FIX] fix test and use use odoo-test-helper lib \o/ --- .isort.cfg | 2 +- base_url/tests/models.py | 12 +-- base_url/tests/models_mixin.py | 117 ---------------------------- base_url/tests/test_abstract_url.py | 24 +++--- test-requirements.txt | 1 + 5 files changed, 20 insertions(+), 136 deletions(-) delete mode 100644 base_url/tests/models_mixin.py diff --git a/.isort.cfg b/.isort.cfg index 8aba7bf289..c18657fed3 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -6,4 +6,4 @@ force_grid_wrap=0 combine_as_imports=True use_parentheses=True line_length=79 -known_third_party = StringIO,cerberus,mock,odoo,openupgradelib,psycopg2,requests,setuptools,urllib2,urlparse,vcr,vcr_unittest,werkzeug +known_third_party = StringIO,cerberus,mock,odoo,odoo_test_helper,openupgradelib,psycopg2,requests,setuptools,urllib2,urlparse,vcr,vcr_unittest,werkzeug diff --git a/base_url/tests/models.py b/base_url/tests/models.py index e511cc3d85..5e837bfbcc 100644 --- a/base_url/tests/models.py +++ b/base_url/tests/models.py @@ -5,29 +5,23 @@ from odoo import api, fields, models -from .models_mixin import TestMixin - _logger = logging.getLogger(__name__) -class UrlBackendFake(models.Model, TestMixin): - +class UrlBackendFake(models.Model): _name = "url.backend.fake" _description = "Url Backend" name = fields.Char(required=True) -class ResPartner(models.Model, TestMixin): - _name = "res.partner" +class ResPartner(models.Model): _inherit = "res.partner" - _test_teardown_no_delete = True - _test_purge_fields = ["binding_ids"] binding_ids = fields.One2many("res.partner.addressable.fake", "record_id") -class ResPartnerAddressableFake(models.Model, TestMixin): +class ResPartnerAddressableFake(models.Model): _name = "res.partner.addressable.fake" _inherit = "abstract.url" _inherits = {"res.partner": "record_id"} diff --git a/base_url/tests/models_mixin.py b/base_url/tests/models_mixin.py deleted file mode 100644 index 2f0b264d26..0000000000 --- a/base_url/tests/models_mixin.py +++ /dev/null @@ -1,117 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2018 Simone Orsi - Camptocamp SA -# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -from operator import attrgetter - -import mock - - -class TestMixin(object): - """Mixin to setup fake models for tests. - - Usage - the model: - - class FakeModel(models.Model, TestMixin): - _name = 'fake.model' - - name = fields.Char() - - Usage - the test klass: - - @classmethod - def setUpClass(cls): - super().setUpClass() - FakeModel._test_setup_model(cls.env) - - @classmethod - def tearDownClass(cls): - FakeModel._test_teardown_model(cls.env) - super().tearDownClass() - """ - - # Generate xmlids - # This is needed if you want to load data tied to a test model via xid. - _test_setup_gen_xid = False - # If you extend a real model (ie: res.partner) you must enable this - # to not delete the model on tear down. - _test_teardown_no_delete = False - # You can add custom fields to real models (eg: res.partner). - # In this case you must delete them to leave registry and model clean. - # This is mandatory for relational fields that link a fake model. - _test_purge_fields = [] - - @classmethod - def _test_setup_model(cls, env): - """Initialize it.""" - with mock.patch.object(env.cr, "commit"): - cls._build_model(env.registry, env.cr) - env.registry.setup_models(env.cr) - ctx = dict(env.context, update_custom_fields=True) - if cls._test_setup_gen_xid: - ctx["module"] = cls._module - env.registry.init_models(env.cr, [cls._name], ctx) - - @classmethod - def _test_teardown_model(cls, env): - """Cleanup registry and real models.""" - - for fname in cls._test_purge_fields: - model = env[cls._name] - if fname in model: - model._pop_field(fname) - model._proper_fields.remove(fname) - - if not getattr(cls, "_test_teardown_no_delete", False): - del env.registry.models[cls._name] - # here we must remove the model from list of children of inherited - # models - parents = cls._inherit - parents = ( - [parents] - if isinstance(parents, basestring) - else (parents or []) - ) - # kepp a copy to be sure to not modify the original _inherit - parents = list(parents) - parents.extend(cls._inherits.keys()) - parents.append("base") - funcs = [ - attrgetter(kind + "_children") - for kind in ["_inherits", "_inherit"] - ] - for parent in parents: - for func in funcs: - children = func(env.registry[parent]) - if cls._name in children: - # at this stage our cls is referenced as children of - # parent -> must un reference it - children.remove(cls._name) - - def _test_get_model_id(self): - self.env.cr.execute( - "SELECT id FROM ir_model WHERE model = %s", (self._name,) - ) - res = self.env.cr.fetchone() - return res[0] if res else None - - def _test_create_ACL(self, **kw): - model_id = self._test_get_model_id() - if not model_id: - self._reflect() - model_id = self._test_get_model_id() - if model_id: - vals = self._test_ACL_values(model_id) - vals.update(kw) - self.env["ir.model.access"].create(vals) - - def _test_ACL_values(self, model_id): - values = { - "name": "Fake ACL for %s" % self._name, - "model_id": model_id, - "perm_read": 1, - "perm_create": 1, - "perm_write": 1, - "perm_unlink": 1, - "active": True, - } - return values diff --git a/base_url/tests/test_abstract_url.py b/base_url/tests/test_abstract_url.py index 4a7e169bb8..932b1da60c 100644 --- a/base_url/tests/test_abstract_url.py +++ b/base_url/tests/test_abstract_url.py @@ -4,17 +4,25 @@ import mock from odoo.exceptions import ValidationError from odoo.tests import SavepointCase +from odoo_test_helper import FakeModelLoader -from .models import ResPartner, ResPartnerAddressableFake, UrlBackendFake - -class TestAbstractUrl(SavepointCase): +class TestAbstractUrl(SavepointCase, FakeModelLoader): @classmethod def setUpClass(cls): super(TestAbstractUrl, cls).setUpClass() - UrlBackendFake._test_setup_model(cls.env) - ResPartnerAddressableFake._test_setup_model(cls.env) - ResPartner._test_setup_model(cls.env) + cls.loader = FakeModelLoader(cls.env, cls.__module__) + cls.loader.backup_registry() + from .models import ( + UrlBackendFake, + ResPartner, + ResPartnerAddressableFake, + ) + + cls.loader.update_registry( + (UrlBackendFake, ResPartner, ResPartnerAddressableFake) + ) + cls.lang = cls.env.ref("base.lang_en") cls.UrlUrl = cls.env["url.url"] cls.ResPartnerAddressable = cls.env["res.partner.addressable.fake"] @@ -26,9 +34,7 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): - ResPartnerAddressableFake._test_teardown_model(cls.env) - UrlBackendFake._test_teardown_model(cls.env) - ResPartner._test_teardown_model(cls.env) + cls.loader.restore_registry() super(TestAbstractUrl, cls).tearDownClass() def _get_default_partner_value(self): diff --git a/test-requirements.txt b/test-requirements.txt index 532164aee0..5c690d9baa 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -7,3 +7,4 @@ requests_mock vcrpy vcrpy-unittest unittest2 # For shopinvader test_controller, which inherits component +odoo-test-helper