diff --git a/setup/.setuptools-odoo-make-default-ignore b/setup/.setuptools-odoo-make-default-ignore new file mode 100644 index 0000000000..207e615334 --- /dev/null +++ b/setup/.setuptools-odoo-make-default-ignore @@ -0,0 +1,2 @@ +# addons listed in this file are ignored by +# setuptools-odoo-make-default (one addon per line) diff --git a/setup/README b/setup/README new file mode 100644 index 0000000000..a63d633e86 --- /dev/null +++ b/setup/README @@ -0,0 +1,2 @@ +To learn more about this directory, please visit +https://pypi.python.org/pypi/setuptools-odoo diff --git a/setup/shopinvader_base_url/odoo/addons/shopinvader_base_url b/setup/shopinvader_base_url/odoo/addons/shopinvader_base_url new file mode 120000 index 0000000000..3141ea0e7e --- /dev/null +++ b/setup/shopinvader_base_url/odoo/addons/shopinvader_base_url @@ -0,0 +1 @@ +../../../../shopinvader_base_url \ No newline at end of file diff --git a/setup/shopinvader_base_url/setup.py b/setup/shopinvader_base_url/setup.py new file mode 100644 index 0000000000..28c57bb640 --- /dev/null +++ b/setup/shopinvader_base_url/setup.py @@ -0,0 +1,6 @@ +import setuptools + +setuptools.setup( + setup_requires=['setuptools-odoo'], + odoo_addon=True, +) diff --git a/shopinvader_base_url/models/abstract_url.py b/shopinvader_base_url/models/abstract_url.py index 3ec3857f6a..1736456a59 100644 --- a/shopinvader_base_url/models/abstract_url.py +++ b/shopinvader_base_url/models/abstract_url.py @@ -3,10 +3,11 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). import logging -from collections import defaultdict from odoo import _, api, fields, models -from odoo.exceptions import UserError, ValidationError +from odoo.exceptions import UserError + +DEFAULT_LANG = "en_US" _logger = logging.getLogger(__name__) @@ -16,50 +17,12 @@ _logger.debug("Cannot `import slugify`.") -def get_model_ref(record): - return "{},{}".format(record._name, record.id) - - class AbstractUrl(models.AbstractModel): _name = "abstract.url" _description = "Abstract Url" + _have_url = True - url_builder = fields.Selection( - selection=[("auto", "Automatic"), ("manual", "Manual")], default="auto" - ) - automatic_url_key = fields.Char(compute="_compute_automatic_url_key", store=True) - manual_url_key = fields.Char() - url_key = fields.Char(string="Url key", compute="_compute_url_key", store=True) - url_url_ids = fields.One2many( - compute="_compute_url_url_ids", comodel_name="url.url" - ) - redirect_url_url_ids = fields.One2many( - compute="_compute_redirect_url_url_ids", comodel_name="url.url" - ) - lang_id = fields.Many2one("res.lang", string="Lang", required=True) - active = fields.Boolean(default=True) - - @api.constrains("url_builder", "manual_url_key") - def _check_manual_url_key(self): - for rec in self: - if rec.url_builder == "manual" and not rec.manual_url_key: - raise ValidationError( - _("Manual url key is required if builder is set to manual") - ) - - @api.onchange("manual_url_key") - def on_url_key_change(self): - self.ensure_one() - if self.manual_url_key: - url = slugify(self.manual_url_key) - if url != self.manual_url_key: - self.manual_url_key = url - return { - "warning": { - "title": "Adapt text rules", - "message": "it will be adapted to %s" % url, - } - } + url_ids = fields.One2many("url.url", "res_id") def _get_url_keywords(self): """This method return a list of keyword that will be concatenated @@ -71,160 +34,111 @@ def _get_url_keywords(self): self.ensure_one() # TODO: IMO we should add the ID here by default # to make sure the URL is always unique + # seb.beau: not sure, most of site do not have id in url + # the url have an seo impact so it's better to only put meaning information + # moreover for unicity you can put the default code of the product or ean13 return [self.name] - def _post_process_url_key(self, key): - """This method allow you to customized the url key. - you can use it to build full path be adding the url of parent record - Ex: key is 42 you can prefix it with "foo" and so return "foo/42" - - Note: the self do not include in the context the lang of the record - """ + def _generate_url_key(self): self.ensure_one() - return key - - def _generic_compute_automatic_url_key(self): - records_by_lang = defaultdict(self.browse) - for record in self: - records_by_lang[record.lang_id] |= record + return slugify("-".join(self._get_url_keywords())) - key_by_id = {} - for lang_id, records in records_by_lang.items(): - for record in records.with_context(lang=lang_id.code): - if not isinstance(record.id, models.NewId): - key_by_id[record.id] = slugify("-".join(record._get_url_keywords())) - - for record in self: - if not isinstance(record.id, models.NewId): - record.automatic_url_key = record._post_process_url_key( - key_by_id[record.id] - ) - else: - record.automatic_url_key = False - - def _compute_automatic_url_key_depends(self): - return ["lang_id", "record_id.name"] - - @api.depends(lambda self: self._compute_automatic_url_key_depends()) - def _compute_automatic_url_key(self): - raise NotImplementedError( - "Automatic url key must be computed in concrete model" - ) - - @api.depends("manual_url_key", "automatic_url_key", "url_builder", "active") - def _compute_url_key(self): - for record in self: - if not record.active: - record.url_key = "" - record._redirect_existing_url() - else: - if record.url_builder == "manual": - new_url = record.manual_url_key - else: - new_url = record.automatic_url_key - if record.url_key != new_url: - record.url_key = new_url - record.set_url(record.url_key) - - @api.depends("url_key") - def _compute_redirect_url_url_ids(self): - self.flush() - for record in self: - record.redirect_url_url_ids = record.env["url.url"].search( - [ - ("model_id", "=", get_model_ref(record)), - ("redirect", "=", True), - ] + def _get_redirect_urls(self, referential, lang): + self.ensure_one() + return self.url_ids.filtered( + lambda s: ( + s.lang_id.code == lang and s.referential == referential and s.redirect ) + ) - @api.depends("url_key") - def _compute_url_url_ids(self): - self.flush() - for record in self: - record.url_url_ids = record.env["url.url"].search( - [("model_id", "=", get_model_ref(record))] + def _get_main_url(self, referential, lang): + self.ensure_one() + return self.url_ids.filtered( + lambda s: ( + s.lang_id.code == lang + and s.referential == referential + and not s.redirect ) + ) @api.model - def _prepare_url(self, url_key): + def _prepare_url(self, url_key, referential): return { "url_key": url_key, "redirect": False, - "model_id": get_model_ref(self), + "res_model": self._name, + "res_id": self.id, + "referential": referential, } def _reuse_url(self, existing_url): # TODO add user notification in the futur SEO dashboard - existing_url.write({"model_id": get_model_ref(self), "redirect": False}) - - def set_url(self, url_key): - """Se a new url - backup old url - - 1 find url redirect true and same model_id - if other model id refuse - 2 if exists set to False + existing_url.write( + { + "res_model": self._name, + "res_id": self.id, + "redirect": False, + } + ) - 3 write the new one - """ + def _update_url_key(self, referential="global", lang=DEFAULT_LANG): + for record in self.with_context(lang=lang): + # TODO maybe we should have a computed field that flag the + # current url if the key used for building the url have changed + # so we can skip this check if nothing have changed + url_key = record._get_url_key() + current_url = record._get_main_url() + if current_url.key != url_key: + current_url.redirect = True + record._add_url(referential, url_key, lang) + + def _add_url(self, referential, url_key, lang): self.ensure_one() existing_url = self.env["url.url"].search( [ - ("url_key", "=", url_key), - ("backend_id", "=", get_model_ref(self.backend_id)), - ("lang_id", "=", self.lang_id.id), + ("referential", "=", referential)("lang_id.code", "=", lang), + ("key", "=", url_key), ] ) if existing_url: - if self != existing_url.model_id: - if existing_url.redirect: - self._reuse_url(existing_url) - else: - raise UserError( - _( - "Url_key already exist in other model" - "\n- name: %(model_name)s\n - id: %(model_id)s\n" - "- url_key: %(url_key)s\n - url_key_id %(url_id)s" - ) - % dict( - model_name=existing_url.model_id.name, - model_id=existing_url.model_id.id, - url_key=existing_url.url_key, - url_id=existing_url.id, - ) - ) + if existing_url.redirect: + self._reuse_url(existing_url) else: - existing_url.write({"redirect": False}) + raise UserError( + _( + "Url_key already exist in other model" + "\n- name: %(model_name)s\n - id: %(model_id)s\n" + "- url_key: %(url_key)s\n - url_key_id %(url_id)s" + ) + % dict( + model_name=existing_url.model_id.name, + model_id=existing_url.model_id.id, + url_key=existing_url.url_key, + url_id=existing_url.id, + ) + ) + else: - # no existing key creating one if not empty - self.env["url.url"].create(self._prepare_url(url_key)) - # other url of object set redirect to True - redirect_urls = self.env["url.url"].search( - [ - ("model_id", "=", get_model_ref(self)), - ("url_key", "!=", url_key), - ("redirect", "=", False), - ] - ) - redirect_urls.write({"redirect": True}) - # we must explicitly invalidate the cache since there is no depends - # defined on this computed fields and this field could have already - # been loaded into the cache - self.invalidate_cache(fnames=["url_url_ids"], ids=self.ids) + vals = self._prepare_url(url_key, referential) + self.env["url.url"].create(vals) - def _redirect_existing_url(self): + def _redirect_existing_url(self, action): """ This method is called when the record is deactivated to give a chance to the concrete model to implement a redirect strategy + action can be "archived" or "unlink" """ return True def unlink(self): for record in self: - # TODO we should propose to redirect the old url - urls = record.env["url.url"].search( - [("model_id", "=", get_model_ref(record))] - ) - urls.unlink() - self.flush() - return super(AbstractUrl, self).unlink() + record._redirect_existing_url("unlink") + # Remove dead url that have been not redirected + record.url_ids.unlink() + return super().unlink() + + def write(self, vals): + res = super().write(vals) + if "active" in vals and not vals["active"]: + self._redirect_existing_url("archived") + return res diff --git a/shopinvader_base_url/models/url_url.py b/shopinvader_base_url/models/url_url.py index 407843cf85..1670b97766 100644 --- a/shopinvader_base_url/models/url_url.py +++ b/shopinvader_base_url/models/url_url.py @@ -4,64 +4,60 @@ import logging -from odoo import api, fields, models - -from .abstract_url import get_model_ref +from odoo import api, fields, models, tools _logger = logging.getLogger(__name__) class UrlUrl(models.Model): - _name = "url.url" _description = "Url" - - url_key = fields.Char(required=True) - model_id = fields.Reference( - selection="_selection_target_model", - help="The id of content linked to the url.", + _order = "res_model,res_id,redirect desc" + + manual = fields.Boolean() + key = fields.Char(required=True, index=True) + res_id = fields.Many2oneReference( + string="Record ID", + help="ID of the target record in the database", + model_field="res_model", readonly=True, - string="Model", - required=True, index=True, ) - redirect = fields.Boolean(help="If tick this url is a redirection to the new url") - backend_id = fields.Reference( - selection="_selection_target_model", - compute="_compute_related_fields", - store=True, - help="Backend linked to this URL", - string="Backend", + res_model = fields.Selection( + selection=lambda s: s._get_model_with_url_selection(), readonly=True, index=True ) - lang_id = fields.Many2one( - "res.lang", "Lang", compute="_compute_related_fields", store=True + redirect = fields.Boolean(help="If tick this url is a redirection to the new url") + referential = fields.Selection( + selection=lambda s: s._get_all_referential(), + index=True, + default="global", ) + lang_id = fields.Many2one("res.lang", "Lang", index=True, required=True) _sql_constraints = [ ( - "unique_key_per_backend_per_lang", - "unique(url_key, backend_id, lang_id)", + "unique_key_per_referential_per_lang", + "unique(url_key, referential, lang_id)", "Already exists in database", ) ] + @tools.ormcache() @api.model - def _selection_target_model(self): - models = self.env["ir.model"].search([]) - return [(model.model, model.name) for model in models] - - @api.depends("model_id") - def _compute_related_fields(self): - for record in self: - record.backend_id = get_model_ref(record.model_id.backend_id) - record.lang_id = record.model_id.lang_id - - @api.model - def _reference_models(self): - return [] - - def _get_object(self, url): - """ - :return: return object attach to the url + def _get_model_with_url_selection(self): + return [ + (model, self.env[model]._description) + for model in self.env + if ( + hasattr(self.env[model], "_have_url") + and not self.env[model]._abstract + and not self.env[model]._transient + ) + ] + + def _get_all_referential(self): + """Return the list of referential for your url, by default it's global + but you can do your own implementation to have url per search engine + index for example """ - return self.search([("url_key", "=", url)]).model_id + return [("global", "Global")] diff --git a/shopinvader_base_url/tests/models.py b/shopinvader_base_url/tests/models.py index dc9c25176e..da8dd4fbae 100644 --- a/shopinvader_base_url/tests/models.py +++ b/shopinvader_base_url/tests/models.py @@ -1,34 +1,20 @@ # Copyright 2019 ACSONE SA/NV # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -import logging from odoo import api, fields, models -_logger = logging.getLogger(__name__) +class FakeProduct(models.Model): + _inherit = ["abstract.url"] + _name = "fake.product" -class UrlBackendFake(models.Model): - _name = "url.backend.fake" - _description = "Url Backend" + code = fields.Char() + name = fields.Char(translate=True) + active = fields.Boolean(default=True) - name = fields.Char(required=True) - - -class ResPartner(models.Model): - _inherit = "res.partner" - - binding_ids = fields.One2many("res.partner.addressable.fake", "record_id") - - -class ResPartnerAddressableFake(models.Model): - _name = "res.partner.addressable.fake" - _inherit = "abstract.url" - _inherits = {"res.partner": "record_id"} - _description = "Fake partner addressable" - - backend_id = fields.Many2one(comodel_name="url.backend.fake") - special_code = fields.Char() - - @api.depends("lang_id", "special_code", "record_id.name") + @api.depends("name") def _compute_automatic_url_key(self): self._generic_compute_automatic_url_key() + + def _get_url_keywords(self): + return super()._get_url_keywords() + ["code"] diff --git a/shopinvader_base_url/tests/test_abstract_url.py b/shopinvader_base_url/tests/test_abstract_url.py index 397b6cb2c9..559ebf6971 100644 --- a/shopinvader_base_url/tests/test_abstract_url.py +++ b/shopinvader_base_url/tests/test_abstract_url.py @@ -3,121 +3,109 @@ import mock from odoo_test_helper import FakeModelLoader -from odoo.exceptions import ValidationError -from odoo.tests import SavepointCase +from odoo.tests import TransactionCase -class TestAbstractUrl(SavepointCase, FakeModelLoader): +class TestAbstractUrl(TransactionCase, FakeModelLoader): @classmethod def setUpClass(cls): - super(TestAbstractUrl, cls).setUpClass() + super().setUpClass() cls.loader = FakeModelLoader(cls.env, cls.__module__) cls.loader.backup_registry() - from .models import ResPartner, ResPartnerAddressableFake, UrlBackendFake + from .models import FakeProduct - cls.loader.update_registry( - (UrlBackendFake, ResPartner, ResPartnerAddressableFake) + cls.loader.update_registry([FakeProduct]) + + cls.lang_en = cls.env.ref("base.lang_en") + cls.lang_fr = cls.env.ref("base.lang_fr") + cls.lang_fr.active = True + cls.product = ( + cls.env["fake.product"] + .with_context(lang="en_US") + .create({"name": "My Product"}) ) + cls.product.with_context(lang="fr_FR").name = "Mon Produit" - cls.lang = cls.env.ref("base.lang_en") - cls.UrlUrl = cls.env["url.url"] - cls.ResPartnerAddressable = cls.env["res.partner.addressable.fake"] - cls.url_backend = cls.env["url.backend.fake"].create({"name": "fake backend"}) - cls.name = "partner name" - cls.auto_key = "partner-name" + def _expect_url_for_lang(self, url_key, lang): + self.assertEqual(self.product._get_main_url_key("global", lang), url_key) @classmethod def tearDownClass(cls): cls.loader.restore_registry() - super(TestAbstractUrl, cls).tearDownClass() - - def _get_default_partner_value(self): - return { - "name": self.name, - "lang_id": self.lang.id, - "url_builder": "auto", - "backend_id": self.url_backend.id, - } - - def _create_auto(self): - return self.ResPartnerAddressable.create(self._get_default_partner_value()) - - def _check_url_key(self, partner_addressable, key_name): - self.assertEqual(partner_addressable.url_key, key_name) - self.assertEqual(len(partner_addressable.url_url_ids), 1) - url_url = partner_addressable.url_url_ids - self.assertEqual(url_url.url_key, key_name) - self.assertEqual(url_url.lang_id, self.lang) - self.assertEqual(url_url.model_id, partner_addressable) - - def test_create_auto_url(self): - my_partner = self.ResPartnerAddressable.create( - self._get_default_partner_value() - ) - self._check_url_key(my_partner, self.auto_key) - - def test_create_manual_url_contrains(self): - value = self._get_default_partner_value() - value["url_builder"] = "manual" - with self.assertRaises(ValidationError): - self.ResPartnerAddressable.create(value) - value["manual_url_key"] = "my url key" - res = self.ResPartnerAddressable.create(value) - self.assertTrue(res) + super().tearDownClass() + + def test_update_url_key(self): + self.product._update_url_key("global", "en_US") + self._expect_url_for_lang("en_US", "my-product") + url = self.product.url_ids.filtered(lambda s: s.lang_id == self.lang_en) + self.assertEqual(len(url), 1) + self.assertTrue(url.auto) + self.assertFalse(url.redirect) + + def test_update_url_2_lang(self): + self.product._update_url_key("global", "en_US") + self.product._update_url_key("global", "fr_FR") + self._expect_url_for_lang("en_US", "my-product") + self._expect_url_for_lang("fr_FR", "mon-produit") + self.assertEqual(len(self.product.url_ids), 2) + + url = self.product.url_ids.filtered(lambda s: s.lang_id == self.lang_fr) + self.assertEqual(len(url), 1) + self.assertTrue(url.auto) + self.assertFalse(url.redirect) + + def test_update_no_translatable_field(self): + self.product._update_url_key("global", "en_US") + self.product._update_url_key("global", "fr_FR") + self.product.write({"code": "1234"}) + self.product._update_url_key("global", "en_US") + self.product._update_url_key("global", "fr_FR") + self.assertEqual(len(self.product.url_ids), 4) + redirects = self.product._get_redirect_urls("global", "en_US") + self._expect_url_for_lang("en_US", "my-product-1234") + self.assertEqual(len(redirects), 1) + redirects = self.product._get_redirect_urls("global", "fr_FR") + self.assertEqual(len(redirects), 1) + self._expect_url_for_lang("fr_FR", "mon-produit-1234") + + def test_update_translatable_field(self): + self.product._update_url_key("global", "en_US") + self.product._update_url_key("global", "fr_FR") + self.product.name = "My Product With Redirect" + self.product._update_url_key("global", "en_US") + self.product._update_url_key("global", "fr_FR") + self.assertEqual(len(self.product.url_ids), 3) + redirects = self.product._get_redirect_urls("global", "en_US") + self._expect_url_for_lang("en_US", "my-product-with-redirect") + self.assertEqual(len(redirects), 1) + + def test_update_product_never_generated(self): + self.product.name = "My product never had url generated" + self.product._update_url_key("global", "en_US") + self._expect_url_for_lang("en_US", "my-product-never-had-url-generated") + self.assertEqual(len(self.product.url_ids), 1) def test_create_manual_url(self): - value = self._get_default_partner_value() - manual_url_key = "manual-url key" - value.update({"url_builder": "manual", "manual_url_key": manual_url_key}) - my_partner = self.ResPartnerAddressable.create(value) - self.assertTrue(my_partner) - self._check_url_key(my_partner, manual_url_key) - - def test_write_url_builder_constrains(self): - my_partner = self._create_auto() - with self.assertRaises(ValidationError): - my_partner.url_builder = "manual" - my_partner.write({"url_builder": "manual", "manual_url_key": "manual url key"}) - - def test_write_url_builder(self): - # tests that a new url is created we update the url builder - my_partner = self._create_auto() - self._check_url_key(my_partner, "partner-name") - manual_url_key = "manual-url key" - my_partner.write({"url_builder": "manual", "manual_url_key": manual_url_key}) - url_keys = set(my_partner.mapped("url_url_ids.url_key")) - self.assertSetEqual(url_keys, {manual_url_key, self.auto_key}) - # if we reset the auto key, no new url.url should be created - my_partner.write({"url_builder": "auto"}) - self.assertEqual(2, len(my_partner.url_url_ids)) - url_keys = set(my_partner.mapped("url_url_ids.url_key")) - self.assertSetEqual(url_keys, {manual_url_key, self.auto_key}) - - def test_write_launching_automatic_url_key(self): - my_partner = self._create_auto() - # call flush to force to apply the recompute - my_partner.flush() - my_partner.name = "my new name" - self.assertEqual(2, len(my_partner.url_url_ids)) - url_keys = set(my_partner.mapped("url_url_ids.url_key")) - self.assertSetEqual(url_keys, {"my-new-name", self.auto_key}) - - def test_write_on_related_record_launching_automatic_url_key(self): - my_partner = self._create_auto() - # call flush to force to apply the recompute - my_partner.flush() - my_partner.record_id.name = "my new name" - self.assertEqual(2, len(my_partner.url_url_ids)) - url_keys = set(my_partner.mapped("url_url_ids.url_key")) - self.assertSetEqual(url_keys, {"my-new-name", self.auto_key}) + self.env["url.url"].create( + { + "manual": True, + "key": "my-custom-key", + "res_id": self.product.id, + "res_model": "fake.product", + "referential": "global", + } + ) + self._expect_url_for_lang("en_US", "my-custom-key") + self.assertEqual(len(self.product.url_ids), 1) + + self.product.name = "My name have change but my url is the same" + self.product._update_url_key("global", "en_US") + self._expect_url_for_lang("en_US", "my-custom-key") def test_write_inactive(self): - my_partner = self._create_auto() # when we deactivate a record, the redirect method should be called with mock.patch.object( - self.ResPartnerAddressable.__class__, "_redirect_existing_url" + self.product.__class__, "_redirect_existing_url" ) as mocked_redirect: - my_partner.active = False - # call flush to force to apply the recompute - my_partner.flush() + self.product.active = False mocked_redirect.assert_called_once()