From 507db4e179514d171ec82e8ea0cbaf2323a6c30d Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 9 Sep 2022 13:44:31 +0000 Subject: [PATCH] [REM] website: remove multi-website by country Short summary: - People can now use the snippets option to show/hide based on country - We never use / test / maintain this feature, if it still works since its introduction 4 years ago that's by luck - It is probably barely used, we never got a single question or issue about it ----------- When we introduced the multi-website feature, it came with 2 main use cases: 1. Multi website by domain -> Every website has its own domain and based on that we serve the expected website 2. Multi website by geoip -> Multiple website can have the same domain and based on GEOIP/country we serve the expected website We never really supported, highlighted or promoted the geoip case. We actually never use it ourselves when doing tests, and we don't consider it when doing specs / improvements. At most, only a very few people know about it in Odoo. That GEOIP case is probably not useful at all, as displaying a whole new website based on lang is not easy since nothing can be shared out of the box -> Every website has its own COW records. For instance, one could think of having its main website A and another website B on which he just added 3 products specific to that website B country. But such a case won't even work properly, as any adaption on website A won't be reflected on website B (design, theme etc). closes odoo/odoo#99938 Related: odoo/upgrade#3892 Signed-off-by: Quentin Smetz (qsm) --- addons/website/data/website_data.xml | 1 - addons/website/data/website_demo.xml | 1 - addons/website/models/res_config_settings.py | 3 - addons/website/models/website.py | 47 +++------- .../website/tests/test_get_current_website.py | 91 +++++-------------- .../views/res_config_settings_views.xml | 4 - addons/website/views/website_views.xml | 1 - 7 files changed, 36 insertions(+), 112 deletions(-) diff --git a/addons/website/data/website_data.xml b/addons/website/data/website_data.xml index c0e71b05e95..f5765dc8ff7 100644 --- a/addons/website/data/website_data.xml +++ b/addons/website/data/website_data.xml @@ -549,7 +549,6 @@ My Website - diff --git a/addons/website/data/website_demo.xml b/addons/website/data/website_demo.xml index 02b706c6675..181bfa75965 100644 --- a/addons/website/data/website_demo.xml +++ b/addons/website/data/website_demo.xml @@ -384,7 +384,6 @@ My Website 2 - 20 diff --git a/addons/website/models/res_config_settings.py b/addons/website/models/res_config_settings.py index 85e66424f6b..766d1fe5cfd 100644 --- a/addons/website/models/res_config_settings.py +++ b/addons/website/models/res_config_settings.py @@ -29,9 +29,6 @@ class ResConfigSettings(models.TransientModel): readonly=False) website_homepage_url = fields.Char( related='website_id.homepage_url', readonly=False) - website_country_group_ids = fields.Many2many( - related='website_id.country_group_ids', - readonly=False) website_company_id = fields.Many2one( related='website_id.company_id', string='Website Company', diff --git a/addons/website/models/website.py b/addons/website/models/website.py index e0e206a66f9..3114f59fe12 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -73,8 +73,6 @@ class Website(models.Model): name = fields.Char('Website Name', required=True) sequence = fields.Integer(default=10) domain = fields.Char('Website Domain', help='E.g. https://www.mydomain.com') - country_group_ids = fields.Many2many('res.country.group', 'website_country_group_rel', 'website_id', 'country_group_id', - string='Country Groups', help='Used when multiple websites have the same domain.') company_id = fields.Many2one('res.company', string="Company", default=lambda self: self.env.company, required=True) language_ids = fields.Many2many( 'res.lang', 'website_lang_rel', 'website_id', 'lang_id', string="Languages", @@ -152,6 +150,10 @@ class Website(models.Model): ('b2c', 'Free sign up'), ], string='Customer Account', default='b2b') + _sql_constraints = [ + ('domain_unique', 'unique(domain)', 'Website Domain should be unique.'), + ] + @api.onchange('language_ids') def _onchange_language_ids(self): language_ids = self.language_ids._origin @@ -977,41 +979,27 @@ class Website(models.Model): # The format of `httprequest.host` is `domain:port` domain_name = request and request.httprequest.host or '' - country = request and request.geoip.get('country_code') - country_id = False - if country: - country_id = self.env['res.country'].search([('code', '=', country)], limit=1).id - - website_id = self._get_current_website_id(domain_name, country_id, fallback=fallback) + website_id = self._get_current_website_id(domain_name, fallback=fallback) return self.browse(website_id) - @tools.cache('domain_name', 'country_id', 'fallback') + @tools.cache('domain_name', 'fallback') @api.model - def _get_current_website_id(self, domain_name, country_id, fallback=True): + def _get_current_website_id(self, domain_name, fallback=True): """Get the current website id. - First find all the websites for which the configured `domain` (after + First find the website for which the configured `domain` (after ignoring a potential scheme) is equal to the given - `domain_name`. If there is only one result, return it immediately. + `domain_name`. If a match is found, return it immediately. - If there are no website found for the given `domain_name`, either + If there is no website found for the given `domain_name`, either fallback to the first found website (no matter its `domain`) or return False depending on the `fallback` parameter. - If there are multiple websites for the same `domain_name`, we need to - filter them out by country. We return the first found website matching - the given `country_id`. If no found website matching `domain_name` - corresponds to the given `country_id`, the first found website for - `domain_name` will be returned (no matter its country). - :param domain_name: the domain for which we want the website. In regard to the `url_parse` method, only the `netloc` part should be given here, no `scheme`. :type domain_name: string - :param country_id: id of the country for which we want the website - :type country_id: int - :param fallback: if True and no website is found for the specificed `domain_name`, return the first website (without filtering them) :type fallback: bool @@ -1028,19 +1016,17 @@ class Website(models.Model): def _filter_domain(website, domain_name, ignore_port=False): """Ignore `scheme` from the `domain`, just match the `netloc` which is host:port in the version of `url_parse` we use.""" - # Here we add http:// to the domain if it's not set because - # `url_parse` expects it to be set to correctly return the `netloc`. website_domain = urls.url_parse(website.domain or '').netloc if ignore_port: website_domain = _remove_port(website_domain) domain_name = _remove_port(domain_name) return website_domain.lower() == (domain_name or '').lower() - # Sort on country_group_ids so that we fall back on a generic website: - # websites with empty country_group_ids will be first. - found_websites = self.search([('domain', 'ilike', _remove_port(domain_name))]).sorted('country_group_ids') + found_websites = self.search([('domain', 'ilike', _remove_port(domain_name))]) # Filter for the exact domain (to filter out potential subdomains) due # to the use of ilike. + # `domain_name` could be an empty string, in that case multiple website + # without a domain will be returned websites = found_websites.filtered(lambda w: _filter_domain(w, domain_name)) # If there is no domain matching for the given port, ignore the port. websites = websites or found_websites.filtered(lambda w: _filter_domain(w, domain_name, ignore_port=True)) @@ -1049,11 +1035,8 @@ class Website(models.Model): if not fallback: return False return self.search([], limit=1).id - elif len(websites) == 1: - return websites.id - else: # > 1 website with the same domain - country_specific_websites = websites.filtered(lambda website: country_id in website.country_group_ids.mapped('country_ids').ids) - return country_specific_websites[0].id if country_specific_websites else websites[0].id + + return websites[0].id def _force(self): self._force_website(self.id) diff --git a/addons/website/tests/test_get_current_website.py b/addons/website/tests/test_get_current_website.py index d40c21ebde8..554291a0728 100644 --- a/addons/website/tests/test_get_current_website.py +++ b/addons/website/tests/test_get_current_website.py @@ -29,104 +29,55 @@ class TestGetCurrentWebsite(TransactionCase): # clean initial state website1 = self.website website1.domain = '' - website1.country_group_ids = False - website2 = Website.create({ - 'name': 'My Website 2', - 'domain': '', - 'country_group_ids': False, - }) + website2 = Website.create({'name': 'My Website 2'}) - country1 = self.env['res.country'].create({'name': "My Country 1"}) - country2 = self.env['res.country'].create({'name': "My Country 2"}) - country3 = self.env['res.country'].create({'name': "My Country 3"}) - country4 = self.env['res.country'].create({'name': "My Country 4"}) - country5 = self.env['res.country'].create({'name': "My Country 5"}) - - country_group_1_2 = self.env['res.country.group'].create({ - 'name': "My Country Group 1-2", - 'country_ids': [(6, 0, (country1 + country2 + country5).ids)], - }) - country_group_3 = self.env['res.country.group'].create({ - 'name': "My Country Group 3", - 'country_ids': [(6, 0, (country3 + country5).ids)], - }) - - # CASE: no domain, no country: get first - self.assertEqual(Website._get_current_website_id('', False), website1.id) - self.assertEqual(Website._get_current_website_id('', country3.id), website1.id) - - # CASE: no domain, given country: get by country - website1.country_group_ids = country_group_1_2 - website2.country_group_ids = country_group_3 - - self.assertEqual(Website._get_current_website_id('', country1.id), website1.id) - self.assertEqual(Website._get_current_website_id('', country2.id), website1.id) - self.assertEqual(Website._get_current_website_id('', country3.id), website2.id) - - # CASE: no domain, wrong country: get first - self.assertEqual(Website._get_current_website_id('', country4.id), Website.search([]).sorted('country_group_ids')[0].id) - - # CASE: no domain, multiple country: get first - self.assertEqual(Website._get_current_website_id('', country5.id), website1.id) + # CASE: no domain: get first + self.assertEqual(Website._get_current_website_id(''), website1.id) # setup domain website1.domain = 'my-site-1.fr' website2.domain = 'https://my2ndsite.com:80' - website1.country_group_ids = False - website2.country_group_ids = False - # CASE: domain set: get matching domain - self.assertEqual(Website._get_current_website_id('my-site-1.fr', False), website1.id) + self.assertEqual(Website._get_current_website_id('my-site-1.fr'), website1.id) # CASE: domain set: get matching domain (scheme and port supported) - self.assertEqual(Website._get_current_website_id('my-site-1.fr:8069', False), website1.id) + self.assertEqual(Website._get_current_website_id('my-site-1.fr:8069'), website1.id) - self.assertEqual(Website._get_current_website_id('my2ndsite.com:80', False), website2.id) - self.assertEqual(Website._get_current_website_id('my2ndsite.com:8069', False), website2.id) - self.assertEqual(Website._get_current_website_id('my2ndsite.com', False), website2.id) + self.assertEqual(Website._get_current_website_id('my2ndsite.com:80'), website2.id) + self.assertEqual(Website._get_current_website_id('my2ndsite.com:8069'), website2.id) + self.assertEqual(Website._get_current_website_id('my2ndsite.com'), website2.id) # CASE: domain set, wrong domain: get first - self.assertEqual(Website._get_current_website_id('test.com', False), website1.id) + self.assertEqual(Website._get_current_website_id('test.com'), website1.id) # CASE: subdomain: not supported - self.assertEqual(Website._get_current_website_id('www.my2ndsite.com', False), website1.id) + self.assertEqual(Website._get_current_website_id('www.my2ndsite.com'), website1.id) - # CASE: domain set, given country: get by domain in priority - website1.country_group_ids = country_group_1_2 - website2.country_group_ids = country_group_3 - - self.assertEqual(Website._get_current_website_id('my2ndsite.com', country1.id), website2.id) - self.assertEqual(Website._get_current_website_id('my2ndsite.com', country2.id), website2.id) - self.assertEqual(Website._get_current_website_id('my-site-1.fr', country3.id), website1.id) - - # CASE: domain set, wrong country: get first for domain - self.assertEqual(Website._get_current_website_id('my2ndsite.com', country4.id), website2.id) - - # CASE: domain set, multiple country: get first for domain - website1.domain = website2.domain - self.assertEqual(Website._get_current_website_id('my2ndsite.com', country5.id), website1.id) + # CASE: domain set: get by domain in priority + self.assertEqual(Website._get_current_website_id('my2ndsite.com'), website2.id) + self.assertEqual(Website._get_current_website_id('my-site-1.fr'), website1.id) # CASE: overlapping domain: get exact match website1.domain = 'site-1.com' website2.domain = 'even-better-site-1.com' - self.assertEqual(Website._get_current_website_id('site-1.com', False), website1.id) - self.assertEqual(Website._get_current_website_id('even-better-site-1.com', False), website2.id) + self.assertEqual(Website._get_current_website_id('site-1.com'), website1.id) + self.assertEqual(Website._get_current_website_id('even-better-site-1.com'), website2.id) # CASE: case insensitive website1.domain = 'Site-1.com' website2.domain = 'Even-Better-site-1.com' - self.assertEqual(Website._get_current_website_id('sitE-1.com', False), website1.id) - self.assertEqual(Website._get_current_website_id('even-beTTer-site-1.com', False), website2.id) + self.assertEqual(Website._get_current_website_id('sitE-1.com'), website1.id) + self.assertEqual(Website._get_current_website_id('even-beTTer-site-1.com'), website2.id) # CASE: same domain, different port website1.domain = 'site-1.com:80' website2.domain = 'site-1.com:81' - self.assertEqual(Website._get_current_website_id('site-1.com:80', False), website1.id) - self.assertEqual(Website._get_current_website_id('site-1.com:81', False), website2.id) - self.assertEqual(Website._get_current_website_id('site-1.com:82', False), website1.id) - self.assertEqual(Website._get_current_website_id('site-1.com', False), website1.id) + self.assertEqual(Website._get_current_website_id('site-1.com:80'), website1.id) + self.assertEqual(Website._get_current_website_id('site-1.com:81'), website2.id) + self.assertEqual(Website._get_current_website_id('site-1.com:82'), website1.id) + self.assertEqual(Website._get_current_website_id('site-1.com'), website1.id) def test_02_signup_user_website_id(self): website = self.website diff --git a/addons/website/views/res_config_settings_views.xml b/addons/website/views/res_config_settings_views.xml index 0e14c11c991..c0c072a5f6a 100644 --- a/addons/website/views/res_config_settings_views.xml +++ b/addons/website/views/res_config_settings_views.xml @@ -47,10 +47,6 @@