diff --git a/addons/mail/models/__init__.py b/addons/mail/models/__init__.py index 7f45f11aece..0421976df53 100644 --- a/addons/mail/models/__init__.py +++ b/addons/mail/models/__init__.py @@ -26,6 +26,7 @@ from . import ir_action_act_window from . import ir_actions from . import ir_attachment from . import ir_autovacuum +from . import ir_config_parameter from . import ir_http from . import ir_model from . import ir_model_fields diff --git a/addons/mail/models/ir_config_parameter.py b/addons/mail/models/ir_config_parameter.py new file mode 100644 index 00000000000..865d58e7a26 --- /dev/null +++ b/addons/mail/models/ir_config_parameter.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import api, models + + +class IrConfigParameter(models.Model): + _inherit = 'ir.config_parameter' + + @api.model_create_multi + def create(self, vals_list): + for vals in vals_list: + if vals.get('key') in ['mail.bounce.alias', 'mail.catchall.alias']: + vals['value'] = self.env['mail.alias']._clean_and_check_unique(vals.get('value')) + return super().create(vals_list) + + def write(self, vals): + if 'value' in vals and self.key in ['mail.bounce.alias', 'mail.catchall.alias']: + vals['value'] = self.env['mail.alias']._clean_and_check_unique(vals.get('value')) + return super().write(vals) diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index 43c2350fdb3..e1c4874f7d5 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -6,7 +6,7 @@ import logging import re from odoo import _, api, fields, models -from odoo.exceptions import ValidationError +from odoo.exceptions import ValidationError, UserError from odoo.tools import remove_accents, is_html_empty _logger = logging.getLogger(__name__) @@ -30,7 +30,7 @@ class Alias(models.Model): _rec_name = 'alias_name' _order = 'alias_model_id, alias_name' - alias_name = fields.Char('Alias Name', help="The name of the email alias, e.g. 'jobs' if you want to catch emails for ") + alias_name = fields.Char('Alias Name', copy=False, help="The name of the email alias, e.g. 'jobs' if you want to catch emails for ") alias_model_id = fields.Many2one('ir.model', 'Aliased Model', required=True, ondelete="cascade", help="The model (Odoo Document Kind) to which this alias " "corresponds. Any incoming email that does not reply to an " @@ -92,15 +92,15 @@ class Alias(models.Model): @api.model def create(self, vals): """ Creates an email.alias record according to the values provided in ``vals``, - with 2 alterations: the ``alias_name`` value may be suffixed in order to - make it unique (and certain unsafe characters replaced), and - he ``alias_model_id`` value will set to the model ID of the ``model_name`` - context value, if provided. + with 2 alterations: the ``alias_name`` value may be cleaned by replacing + certain unsafe characters, and the ``alias_model_id`` value will set to the + model ID of the ``model_name`` context value, if provided. Also, it raises + UserError if given alias name is already assigned. """ model_name = self._context.get('alias_model_name') parent_model_name = self._context.get('alias_parent_model_name') if vals.get('alias_name'): - vals['alias_name'] = self._clean_and_make_unique(vals.get('alias_name')) + vals['alias_name'] = self._clean_and_check_unique(vals.get('alias_name')) if model_name: model = self.env['ir.model']._get(model_name) vals['alias_model_id'] = model.id @@ -110,9 +110,9 @@ class Alias(models.Model): return super(Alias, self).create(vals) def write(self, vals): - """"give a unique alias name if given alias name is already assigned""" + """"Raises UserError if given alias name is already assigned""" if vals.get('alias_name') and self.ids: - vals['alias_name'] = self._clean_and_make_unique(vals.get('alias_name'), alias_ids=self.ids) + vals['alias_name'] = self._clean_and_check_unique(vals.get('alias_name')) return super(Alias, self).write(vals) def name_get(self): @@ -130,29 +130,21 @@ class Alias(models.Model): res.append((record['id'], _("Inactive Alias"))) return res - @api.model - def _find_unique(self, name, alias_ids=False): - """Find a unique alias name similar to ``name``. If ``name`` is - already taken, make a variant by adding an integer suffix until - an unused alias is found. - """ - sequence = None - while True: - new_name = "%s%s" % (name, sequence) if sequence is not None else name - domain = [('alias_name', '=', new_name)] - if alias_ids: - domain += [('id', 'not in', alias_ids)] - if not self.search(domain): - break - sequence = (sequence + 1) if sequence else 2 - return new_name + def _clean_and_check_unique(self, name): + """When an alias name appears to already be an email, we keep the local + part only. A sanitizing / cleaning is also performed on the name. If + name already exists an UserError is raised. """ + sanitized_name = remove_accents(name).lower().split('@')[0] + sanitized_name = re.sub(r'[^\w+.]+', '-', sanitized_name) - @api.model - def _clean_and_make_unique(self, name, alias_ids=False): - # when an alias name appears to already be an email, we keep the local part only - name = remove_accents(name).lower().split('@')[0] - name = re.sub(r'[^\w+.]+', '-', name) - return self._find_unique(name, alias_ids=alias_ids) + catchall_alias = self.env['ir.config_parameter'].sudo().get_param('mail.catchall.alias') + bounce_alias = self.env['ir.config_parameter'].sudo().get_param('mail.bounce.alias') + domain = [('alias_name', '=', sanitized_name)] + if self: + domain += [('id', 'not in', self.ids)] + if sanitized_name in [catchall_alias, bounce_alias] or self.search_count(domain): + raise UserError(_('The e-mail alias is already used. Please enter another one.')) + return sanitized_name def open_document(self): if not self.alias_model_id or not self.alias_force_thread_id: diff --git a/addons/maintenance/data/maintenance_demo.xml b/addons/maintenance/data/maintenance_demo.xml index 69e91400fa6..5707b3e6782 100644 --- a/addons/maintenance/data/maintenance_demo.xml +++ b/addons/maintenance/data/maintenance_demo.xml @@ -14,26 +14,21 @@ Computers - Software - Printers - Monitors 3 - Phones - diff --git a/addons/maintenance/tests/test_maintenance.py b/addons/maintenance/tests/test_maintenance.py index e4acceb1767..6cd04bd4192 100644 --- a/addons/maintenance/tests/test_maintenance.py +++ b/addons/maintenance/tests/test_maintenance.py @@ -37,8 +37,7 @@ class TestEquipment(TransactionCase): )) self.equipment_monitor = self.env['maintenance.equipment.category'].create({ - 'name': 'Monitors', - 'alias_id': self.env.ref('maintenance.mail_alias_equipment').id, + 'name': 'Monitors - Test', }) def test_10_equipment_request_category(self): diff --git a/addons/maintenance/tests/test_maintenance_multicompany.py b/addons/maintenance/tests/test_maintenance_multicompany.py index 7c615921f4b..5ec2a6bce71 100644 --- a/addons/maintenance/tests/test_maintenance_multicompany.py +++ b/addons/maintenance/tests/test_maintenance_multicompany.py @@ -78,21 +78,21 @@ class TestEquipmentMulticompany(TransactionCase): # create equipment category for equipment manager category_1 = Category.with_user(equipment_manager).with_context(allowed_company_ids=cids).create({ - 'name': 'Monitors', + 'name': 'Monitors - Test', 'company_id': company_b.id, 'technician_user_id': equipment_manager.id, }) # create equipment category for equipment manager Category.with_user(equipment_manager).with_context(allowed_company_ids=cids).create({ - 'name': 'Computers', + 'name': 'Computers - Test', 'company_id': company_b.id, 'technician_user_id': equipment_manager.id, }) # create equipment category for equipment user Category.with_user(equipment_manager).create({ - 'name': 'Phones', + 'name': 'Phones - Test', 'company_id': company_a.id, 'technician_user_id': equipment_manager.id, }) diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index 5a63940756b..d536d0236ae 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -77,6 +77,44 @@ class TestMailAlias(TestMailCommon): alias = self.env['mail.alias'].with_context(alias_model_name='mail.test').create({'alias_name': 'b4r+_#_R3wl$$'}) self.assertEqual(alias.alias_name, 'b4r+_-_r3wl-', 'Disallowed chars should be replaced by hyphens') + def test_alias_name_unique(self): + alias_model_id = self.env['ir.model']._get('mail.test.gateway').id + catchall_alias = self.env['ir.config_parameter'].sudo().get_param('mail.catchall.alias') + bounce_alias = self.env['ir.config_parameter'].sudo().get_param('mail.bounce.alias') + + # test you cannot create aliases matching bounce / catchall + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + self.env['mail.alias'].create({'alias_model_id': alias_model_id, 'alias_name': catchall_alias}) + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + self.env['mail.alias'].create({'alias_model_id': alias_model_id, 'alias_name': bounce_alias}) + + new_mail_alias = self.env['mail.alias'].create({ + 'alias_model_id': alias_model_id, + 'alias_name': 'unused.test.alias' + }) + + # test that re-using catchall and bounce alias raises UserError + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + new_mail_alias.write({ + 'alias_name': catchall_alias + }) + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + new_mail_alias.write({ + 'alias_name': bounce_alias + }) + + new_mail_alias.write({'alias_name': 'another.unused.test.alias'}) + + # test that duplicating an alias should have blank name + copy_new_mail_alias = new_mail_alias.copy() + self.assertFalse(copy_new_mail_alias.alias_name) + + # cannot set catchall / bounce to used alias + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + self.env['ir.config_parameter'].sudo().set_param('mail.catchall.alias', new_mail_alias.alias_name) + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + self.env['ir.config_parameter'].sudo().set_param('mail.bounce.alias', new_mail_alias.alias_name) + @tagged('mail_gateway') class TestMailgateway(TestMailCommon):