From d133df581964b0dab95eb92d8fd79227488b79de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= Date: Mon, 7 Oct 2019 11:57:56 +0000 Subject: [PATCH] [FIX] website: Display duplicate login error message When you try to create two users with the same login, the shown error message is the Postgresql integrity error instead of a nice (translated) validation error. This happens since bf88f3e which changed the way error messages are translated. The reason is the manually created unique index `res_users_login_key_unique_website_index`. The error handling and translation mechanisms don't know it exists. The index was created for performance reasons in commit b5a12b4 where the original python constraint was replaced with this custom SQL index. In this commit, the python constraint is re-introduced to remove the index hack, but the implementation now uses a single SQL query, which should be quite fast. closes odoo/odoo#38737 X-original-commit: 817a811ec98b0c313b12e6464b5e98ca432cb3fc Signed-off-by: Christophe Simonis --- addons/website/i18n/website.pot | 2 + addons/website/models/res_users.py | 32 ++++++++------ addons/website/tests/__init__.py | 1 + addons/website/tests/test_res_users.py | 60 ++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 addons/website/tests/test_res_users.py diff --git a/addons/website/i18n/website.pot b/addons/website/i18n/website.pot index 9bfcdb778c0..369cd44b0cf 100644 --- a/addons/website/i18n/website.pot +++ b/addons/website/i18n/website.pot @@ -5632,7 +5632,9 @@ msgid "" msgstr "" #. module: website +#: code:addons/website/models/res_users.py:0 #: model:ir.model.constraint,message:website.constraint_res_users_login_key +#, python-format msgid "You can not have two users with the same login!" msgstr "" diff --git a/addons/website/models/res_users.py b/addons/website/models/res_users.py index 617cc6e9ba4..39eaf5babd6 100644 --- a/addons/website/models/res_users.py +++ b/addons/website/models/res_users.py @@ -2,7 +2,8 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import logging -from odoo import api, fields, models, tools +from odoo import api, fields, models, tools, _ +from odoo.exceptions import ValidationError from odoo.http import request _logger = logging.getLogger(__name__) @@ -14,9 +15,7 @@ class ResUsers(models.Model): website_id = fields.Many2one('website', related='partner_id.website_id', store=True, related_sudo=False, readonly=False) _sql_constraints = [ - # Partial constraint, complemented by unique index (see below). Still - # useful to keep because it provides a proper error message when a - # violation occurs, as it shares the same prefix as the unique index. + # Partial constraint, complemented by a python constraint (see below). ('login_key', 'unique (login, website_id)', 'You can not have two users with the same login!'), ] @@ -26,6 +25,23 @@ class ResUsers(models.Model): return True return super(ResUsers, self)._has_unsplash_key_rights() + @api.constrains('login', 'website_id') + def _check_login(self): + """ Do not allow two users with the same login without website """ + self.flush(['login', 'website_id']) + self.env.cr.execute( + """SELECT login + FROM res_users + WHERE login IN (SELECT login FROM res_users WHERE id IN %s AND website_id IS NULL) + AND website_id IS NULL + GROUP BY login + HAVING COUNT(*) > 1 + """, + (tuple(self.ids),) + ) + if self.env.cr.rowcount: + raise ValidationError(_('You can not have two users with the same login!')) + @api.model def _get_login_domain(self, login): website = self.env['website'].get_current_website() @@ -46,14 +62,6 @@ class ResUsers(models.Model): current_website = self.env['website'].get_current_website() return current_website.auth_signup_uninvited or super(ResUsers, self)._get_signup_invitation_scope() - def _auto_init(self): - result = super(ResUsers, self)._auto_init() - # Use unique index to implement unique constraint per website, even if website_id is null - # (not possible using a constraint) - tools.create_unique_index(self._cr, 'res_users_login_key_unique_website_index', - self._table, ['login', 'COALESCE(website_id,-1)']) - return result - @classmethod def authenticate(cls, db, login, password, user_agent_env): """ Override to link the logged in user's res.partner to website.visitor """ diff --git a/addons/website/tests/__init__.py b/addons/website/tests/__init__.py index 9bb3efd60de..bab3d80edb0 100644 --- a/addons/website/tests/__init__.py +++ b/addons/website/tests/__init__.py @@ -9,6 +9,7 @@ from . import test_lang_url from . import test_menu from . import test_page from . import test_qweb +from . import test_res_users from . import test_ui from . import test_views from . import test_website_favicon diff --git a/addons/website/tests/test_res_users.py b/addons/website/tests/test_res_users.py new file mode 100644 index 00000000000..ef8d97ed060 --- /dev/null +++ b/addons/website/tests/test_res_users.py @@ -0,0 +1,60 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from psycopg2 import IntegrityError + +from odoo.tests.common import TransactionCase, new_test_user +from odoo.exceptions import ValidationError +from odoo.service.model import check +from odoo.tools import mute_logger + + +class TestWebsiteResUsers(TransactionCase): + + def setUp(self): + super().setUp() + websites = self.env['website'].create([ + {'name': 'Test Website'}, + {'name': 'Test Website 2'}, + ]) + self.website_1, self.website_2 = websites + + def test_no_website(self): + new_test_user(self.env, login='Pou', website_id=False) + with self.assertRaises(ValidationError): + new_test_user(self.env, login='Pou', website_id=False) + + def test_websites_set_null(self): + user_1 = new_test_user(self.env, login='Pou', website_id=self.website_1.id) + user_2 = new_test_user(self.env, login='Pou', website_id=self.website_2.id) + with self.assertRaises(ValidationError): + (user_1 | user_2).write({'website_id': False}) + + def test_null_and_website(self): + new_test_user(self.env, login='Pou', website_id=self.website_1.id) + new_test_user(self.env, login='Pou', website_id=False) + + def test_change_login(self): + new_test_user(self.env, login='Pou', website_id=self.website_1.id) + user_belle = new_test_user(self.env, login='Belle', website_id=self.website_1.id) + with self.assertRaises(IntegrityError), mute_logger('odoo.sql_db'): + user_belle.login = 'Pou' + + def test_change_login_no_website(self): + new_test_user(self.env, login='Pou', website_id=False) + user_belle = new_test_user(self.env, login='Belle', website_id=False) + with self.assertRaises(ValidationError): + user_belle.login = 'Pou' + + def test_same_website_message(self): + + @check # Check decorator, otherwise translation is not applied + def check_new_test_user(dbname): + new_test_user(self.env(context={'land': 'en_US'}), login='Pou', website_id=self.website_1.id) + + new_test_user(self.env, login='Pou', website_id=self.website_1.id) + + # Should be a ValidationError (with a nice translated error message), + # not an IntegrityError + with self.assertRaises(ValidationError), mute_logger('odoo.sql_db'): + check_new_test_user(self.env.registry._db.dbname)