From b5e9f08ddca01ba800fe50ff9af68649f9e3cb4a Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Tue, 10 Sep 2019 17:45:15 +0200 Subject: [PATCH] Add normalization and optional uniqueness to partner_email_check - partner_email_check now uses email-validator instead of validate_email - Email addresses are normalized - There is a setting to enforce uniqueness of partner email addresses - There is a setting to check whether emails are deliverable (i.e. whether the domain resolves) --- partner_email_check/README.rst | 19 ++- partner_email_check/__manifest__.py | 7 +- partner_email_check/models/__init__.py | 1 + .../models/res_config_settings.py | 42 ++++++ partner_email_check/models/res_partner.py | 87 +++++++++--- .../tests/test_partner_email_check.py | 126 ++++++++++++++++++ .../views/base_config_view.xml | 39 ++++++ 7 files changed, 302 insertions(+), 19 deletions(-) create mode 100644 partner_email_check/models/res_config_settings.py create mode 100644 partner_email_check/views/base_config_view.xml diff --git a/partner_email_check/README.rst b/partner_email_check/README.rst index 98e714b1b..87043cd82 100644 --- a/partner_email_check/README.rst +++ b/partner_email_check/README.rst @@ -6,12 +6,27 @@ Partner Email Check =================== -This module validate the field ``email`` in the module ``res.partner``. +This module validates and normalizes the field ``email`` in the model +``res.partner``. + +As part of the normalization, email addresses are converted to lowercase. + +Optionally, multiple partners can not be allowed to have the same address. +This will not work with multiple comma-separated email addresses in the field, +although validation and normalization are still supported in such cases. Configuration ============= -Install python package validate_email: ``sudo pip install validate_email``. +Install python package email-validator: ``sudo pip install email-validator``. + +To not allow multiple partners to have the same email address, use the +"Filter duplicate email addresses"/``partner_email_check_filter_duplicates`` +setting. + +To validate that email addresses are deliverable (that the hostname exists), +use the "Check deliverability of email addresses"/``partner_email_check_check_deliverability`` +setting. Usage ===== diff --git a/partner_email_check/__manifest__.py b/partner_email_check/__manifest__.py index f59fa0492..3ebb7dcae 100644 --- a/partner_email_check/__manifest__.py +++ b/partner_email_check/__manifest__.py @@ -8,11 +8,14 @@ 'author': "Komit, Odoo Community Association (OCA)", 'website': 'https://github.com/OCA/partner-contact', 'category': 'Tools', - 'depends': ['base'], + 'depends': ['base_setup'], 'installable': True, 'application': False, 'license': 'AGPL-3', 'external_dependencies': { - 'python': ['validate_email'] + 'python': ['email_validator'] }, + 'data': [ + 'views/base_config_view.xml', + ] } diff --git a/partner_email_check/models/__init__.py b/partner_email_check/models/__init__.py index c6cc3b329..cf1724192 100644 --- a/partner_email_check/models/__init__.py +++ b/partner_email_check/models/__init__.py @@ -1,3 +1,4 @@ # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). +from . import res_config_settings from . import res_partner diff --git a/partner_email_check/models/res_config_settings.py b/partner_email_check/models/res_config_settings.py new file mode 100644 index 000000000..405677809 --- /dev/null +++ b/partner_email_check/models/res_config_settings.py @@ -0,0 +1,42 @@ +from odoo import api, fields, models + + +class ResConfigSettings(models.TransientModel): + _inherit = 'res.config.settings' + + partner_email_check_filter_duplicates = fields.Boolean( + string="Filter duplicate partner email addresses", + help="Don't allow multiple partners to have the same email address.", + ) + + partner_email_check_check_deliverability = fields.Boolean( + string="Check deliverability of email addresses", + help="Don't allow email addresses with providers that don't exist", + ) + + @api.model + def get_values(self): + res = super(ResConfigSettings, self).get_values() + conf = self.env['ir.config_parameter'].sudo() + res.update( + partner_email_check_filter_duplicates=conf.get_param( + 'partner_email_check_filter_duplicates', 'False' + ) == 'True', + partner_email_check_check_deliverability=conf.get_param( + 'partner_email_check_check_deliverability', 'False' + ) == 'True', + ) + return res + + @api.multi + def set_values(self): + super(ResConfigSettings, self).set_values() + conf = self.env['ir.config_parameter'].sudo() + conf.set_param( + 'partner_email_check_filter_duplicates', + self.partner_email_check_filter_duplicates + ) + conf.set_param( + 'partner_email_check_check_deliverability', + self.partner_email_check_check_deliverability + ) diff --git a/partner_email_check/models/res_partner.py b/partner_email_check/models/res_partner.py index 68f196bf4..c343ea70d 100644 --- a/partner_email_check/models/res_partner.py +++ b/partner_email_check/models/res_partner.py @@ -3,32 +3,89 @@ import logging from odoo import api, models, _ -from odoo.exceptions import UserError +from odoo.exceptions import UserError, ValidationError _logger = logging.getLogger(__name__) try: - from validate_email import validate_email + from email_validator import ( + validate_email, + EmailSyntaxError, + EmailUndeliverableError, + ) except ImportError: - _logger.debug('Cannot import "validate_email".') + _logger.debug('Cannot import "email_validator".') - def validate_email(email): - _logger.warning( - 'Can not validate email, ' - 'python dependency required "validate_email"') - return True + validate_email = None class ResPartner(models.Model): _inherit = 'res.partner' + @api.model + def email_check(self, emails): + return ','.join(self._normalize_email(email.strip()) + for email in emails.split(',')) + @api.constrains('email') - def constrains_email(self): - for rec in self.filtered("email"): - self.email_check(rec.email) + def _check_email_unique(self): + if self._should_filter_duplicates(): + for rec in self.filtered("email"): + if ',' in rec.email: + raise UserError( + _("Field contains multiple email addresses. This is " + "not supported when duplicate email addresses are " + "not allowed.") + ) + if self.search_count( + [('email', '=', rec.email), ('id', '!=', rec.id)] + ): + raise UserError( + _("Email '%s' is already in use.") % rec.email.strip() + ) + + def _normalize_email(self, email): + if validate_email is None: + _logger.warning( + 'Can not validate email, ' + 'python dependency required "email_validator"') + return email + + try: + result = validate_email( + email, + check_deliverability=self._should_check_deliverability(), + ) + except EmailSyntaxError: + raise ValidationError( + _("%s is an invalid email") % email.strip() + ) + except EmailUndeliverableError: + raise ValidationError( + _("Cannot deliver to email address %s") % email.strip() + ) + return result['local'].lower() + '@' + result['domain_i18n'] + + def _should_filter_duplicates(self): + conf = self.env['ir.config_parameter'].get_param( + 'partner_email_check_filter_duplicates', 'False' + ) + return conf == 'True' + + def _should_check_deliverability(self): + conf = self.env['ir.config_parameter'].get_param( + 'partner_email_check_check_deliverability', 'False' + ) + return conf == 'True' @api.model - def email_check(self, email): - if validate_email(email): - return True - raise UserError(_('Invalid e-mail!')) + def create(self, vals): + if vals.get('email'): + vals['email'] = self.email_check(vals['email']) + return super(ResPartner, self).create(vals) + + @api.multi + def write(self, vals): + if vals.get('email'): + vals['email'] = self.email_check(vals['email']) + return super(ResPartner, self).write(vals) diff --git a/partner_email_check/tests/test_partner_email_check.py b/partner_email_check/tests/test_partner_email_check.py index 5a9cea843..ee3dc08b6 100644 --- a/partner_email_check/tests/test_partner_email_check.py +++ b/partner_email_check/tests/test_partner_email_check.py @@ -1,8 +1,11 @@ # Copyright 2019 Komit # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). +from unittest.mock import patch + from odoo.exceptions import ValidationError from odoo.tests.common import TransactionCase +from odoo.tools.misc import mute_logger class TestPartnerEmailCheck(TransactionCase): @@ -11,6 +14,10 @@ class TestPartnerEmailCheck(TransactionCase): self.test_partner = self.env['res.partner'].create({ 'name': 'test', }) + self.wizard = self.env['res.config.settings'].create({}) + self.wizard.partner_email_check_filter_duplicates = False + self.wizard.partner_email_check_check_deliverability = False + self.wizard.set_values() def test_bad_email(self): """Test rejection of bad emails.""" @@ -21,3 +28,122 @@ class TestPartnerEmailCheck(TransactionCase): """Test acceptance of good""" self.test_partner.email = 'goodemail@domain.com' self.assertTrue(self.test_partner.email) + + def test_bad_emails(self): + """Test rejection of bad emails.""" + with self.assertRaises(ValidationError): + self.test_partner.email = 'good@domain.com,bad@email@domain..com' + + def test_good_emails(self): + """Test acceptance of good""" + self.test_partner.email = 'goodemail@domain.com,goodemail2@domain.com' + self.assertTrue(self.test_partner.email) + + def test_email_domain_normalization(self): + """Test normalization of email domain names, including punycode.""" + self.test_partner.write({'email': 'goodemail@xn--xamPle-9ua.com'}) + self.assertEqual(self.test_partner.email, u'goodemail@éxample.com') + + def test_multi_email_domain_normalization(self): + """Test normalization of email domain names of multiple addresses.""" + self.test_partner.write({ + 'email': 'goodemail@doMAIN.com,othergood@xn--xample-9ua.com' + }) + self.assertEqual( + self.test_partner.email, + u'goodemail@domain.com,othergood@éxample.com' + ) + + def test_email_local_normalization(self): + """Test normalization of the local part of email addresses.""" + self.test_partner.write({'email': 'Me@mail.org'}) + # .lower() is locale-dependent, so don't hardcode the result + self.assertEqual(self.test_partner.email, 'Me'.lower() + '@mail.org') + + def test_multi_email_local_normalization(self): + """Test normalization of the local part of multiple addresses.""" + self.test_partner.write({'email': 'You@mAiL.net,mE@mail.com'}) + self.assertEqual( + self.test_partner.email, + 'You'.lower() + '@mail.net,' + 'mE'.lower() + '@mail.com' + ) + + def disallow_duplicates(self): + self.wizard.partner_email_check_filter_duplicates = True + self.wizard.set_values() + + def test_duplicate_addresses_disallowed(self): + self.disallow_duplicates() + self.test_partner.write({'email': 'email@domain.tld'}) + with self.assertRaises(ValidationError): + self.env['res.partner'].create({ + 'name': 'alsotest', + 'email': 'email@domain.tld' + }) + + def test_duplicate_after_normalization_addresses_disallowed(self): + self.disallow_duplicates() + self.env['res.partner'].create({ + 'name': 'alsotest', + 'email': 'email@doMAIN.tld' + }) + with self.assertRaises(ValidationError): + self.test_partner.email = 'email@domain.tld' + + def test_multiple_addresses_disallowed_when_duplicates_filtered(self): + self.disallow_duplicates() + with self.assertRaises(ValidationError): + self.test_partner.email = 'foo@bar.org,email@domain.tld' + + def test_duplicate_addresses_allowed_by_default(self): + self.env['res.partner'].create({ + 'name': 'alsotest', + 'email': 'email@domain.tld', + }) + self.test_partner.email = 'email@domain.tld' + + def check_deliverability(self): + self.wizard.partner_email_check_check_deliverability = True + self.wizard.set_values() + + def test_deliverable_addresses_allowed(self): + self.check_deliverability() + # We only need a resolving domain, not a real user + self.test_partner.email = 'gooddomain-icraglusrk@gmail.com' + self.assertTrue(self.test_partner.email) + + def test_nondeliverable_addresses_not_allowed(self): + self.check_deliverability() + with self.assertRaises(ValidationError): + # This domain may resolve by mistake on certain network setups + # At least until a new version of email-validator is released + # See https://github.com/JoshData/python-email-validator/pull/30 + self.test_partner.email = 'cezrik@acoa.nrdkt' + + def test_config_getters(self): + other_wizard = self.env['res.config.settings'].create({}) + self.assertFalse(other_wizard.partner_email_check_check_deliverability) + self.assertFalse(other_wizard.partner_email_check_filter_duplicates) + self.disallow_duplicates() + self.check_deliverability() + other_wizard = self.env['res.config.settings'].create({}) + self.assertTrue(other_wizard.partner_email_check_check_deliverability) + self.assertTrue(other_wizard.partner_email_check_filter_duplicates) + + @mute_logger('odoo.addons.partner_email_check.models.res_partner') + def test_lacking_dependency_does_not_halt_execution(self): + with patch('odoo.addons.partner_email_check.models.res_partner.' + 'validate_email', None): + self.test_partner.email = 'notatallvalid@@domain' + + @mute_logger('odoo.addons.partner_email_check.models.res_partner') + def test_lacking_dependency_keeps_uniqueness_constraint_working(self): + self.disallow_duplicates() + with patch('odoo.addons.partner_email_check.models.res_partner.' + 'validate_email', None): + self.env['res.partner'].create({ + 'name': 'alsotest', + 'email': 'email@domain.tld' + }) + with self.assertRaises(ValidationError): + self.test_partner.email = 'email@domain.tld' diff --git a/partner_email_check/views/base_config_view.xml b/partner_email_check/views/base_config_view.xml new file mode 100644 index 000000000..3bc93e759 --- /dev/null +++ b/partner_email_check/views/base_config_view.xml @@ -0,0 +1,39 @@ + + + + partner_email_check + res.config.settings + + + +

Email validation

+
+
+
+ +
+
+
+
+
+
+ +
+
+
+
+
+ + + +