From 386fb31275d15d1cd040baaddd18737d94f66681 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Wed, 13 Jun 2018 10:23:19 +0200 Subject: [PATCH] [REF] auth_signup_verify_email: Skip mail send in tests and use email_validator (#1247) This addon introduced an integration conflict when tested in a database that had `mail_tracking_mass_mailing` installed, producing this failure: Traceback (most recent call last): File "/opt/odoo/auto/addons/auth_signup_verify_email/controllers/main.py", line 44, in passwordless_signup sudo_users.reset_password(values.get("login")) File "/usr/lib/python2.7/contextlib.py", line 24, in __exit__ self.gen.next() File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 419, in savepoint self.execute('RELEASE SAVEPOINT "%s"' % name) File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 154, in wrapper return f(self, *args, **kwargs) File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 231, in execute res = self._obj.execute(query, params) InternalError: no such savepoint Which in turn produced the following in the next test: InternalError: current transaction is aborted, commands ignored until end of transaction block The problem comes from the fact that one cannot rollback to a nested savepoint if the parent savepoint was released. It became a problem because that's the strategy that both this addon and upstream's `TestCursor` follow. To avoid that, tests now mock the `send_mail` method. This results also in having a predictable outcome from the test `test_good_email`, so it is more meaningful now. Besides, previously we were using the `validate_email` package, which is currently a dead project that can silently fail under certain environments, as seen in https://github.com/syrusakbary/validate_email/pull/80. There's the `email_validator` package, freely available, supported, and it provides a human-readable error message whenever some format from the email fails. As such, here I'm switching the dependency, while still adding a backwards compatibility layer for preexisting installations. --- auth_signup_verify_email/README.rst | 2 ++ auth_signup_verify_email/__manifest__.py | 10 +++--- auth_signup_verify_email/controllers/main.py | 31 +++++++++++++++--- .../tests/test_verify_email.py | 32 ++++++------------- requirements.txt | 2 +- 5 files changed, 42 insertions(+), 35 deletions(-) diff --git a/auth_signup_verify_email/README.rst b/auth_signup_verify_email/README.rst index 041de578e..c4aaaa8e6 100644 --- a/auth_signup_verify_email/README.rst +++ b/auth_signup_verify_email/README.rst @@ -45,6 +45,8 @@ Known issues / Roadmap * Remove calls to ``cr.commit()`` in tests when https://github.com/odoo/odoo/issues/12237 gets fixed. +* Drop support for ``validate_email`` in favor of ``email_validator``. + Bug Tracker =========== diff --git a/auth_signup_verify_email/__manifest__.py b/auth_signup_verify_email/__manifest__.py index 3908671e1..98608a10e 100644 --- a/auth_signup_verify_email/__manifest__.py +++ b/auth_signup_verify_email/__manifest__.py @@ -1,14 +1,12 @@ # -*- coding: utf-8 -*- -# © 2015 Antiun Ingeniería, S.L. # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). { "name": "Verify email at signup", "summary": "Force uninvited users to use a good email for signup", - "version": "10.0.1.0.0", + "version": "10.0.2.0.0", "category": "Authentication", - "website": "http://www.tecnativa.com", - "author": "Antiun Ingeniería S.L., " - "Tecnativa, " + "website": "http://github.com/OCA/server-tools", + "author": "Tecnativa, " "Odoo Community Association (OCA)", "license": "AGPL-3", "application": False, @@ -16,7 +14,7 @@ "external_dependencies": { "python": [ "lxml", - "validate_email", + "email_validator", ], }, "depends": [ diff --git a/auth_signup_verify_email/controllers/main.py b/auth_signup_verify_email/controllers/main.py index 0fb5e0663..c2fd9800c 100644 --- a/auth_signup_verify_email/controllers/main.py +++ b/auth_signup_verify_email/controllers/main.py @@ -9,9 +9,24 @@ from odoo.addons.auth_signup.controllers.main import AuthSignupHome _logger = logging.getLogger(__name__) try: - from validate_email import validate_email + from email_validator import validate_email, EmailSyntaxError except ImportError: - _logger.debug("Cannot import `validate_email`.") + # TODO Remove in v12, dropping backwards compatibility with validate_email + # pragma: no-cover + try: + from validate_email import validate_email as _validate + + class EmailSyntaxError(Exception): + message = False + + def validate_email(*args, **kwargs): + if not _validate(*args, **kwargs): + raise EmailSyntaxError + + except ImportError: + _logger.debug("Cannot import `email_validator`.") + else: + _logger.warning("Install `email_validator` to get full support.") class SignupVerifyEmail(AuthSignupHome): @@ -27,10 +42,16 @@ class SignupVerifyEmail(AuthSignupHome): qcontext = self.get_auth_signup_qcontext() # Check good format of e-mail - if not validate_email(values.get("login", "")): - qcontext["error"] = _("That does not seem to be an email address.") + try: + validate_email(values.get("login", "")) + except EmailSyntaxError as error: + qcontext["error"] = getattr( + error, + "message", + _("That does not seem to be an email address."), + ) return http.request.render("auth_signup.signup", qcontext) - elif not values.get("email"): + if not values.get("email"): values["email"] = values.get("login") # Remove password diff --git a/auth_signup_verify_email/tests/test_verify_email.py b/auth_signup_verify_email/tests/test_verify_email.py index daca352a6..276b3d575 100644 --- a/auth_signup_verify_email/tests/test_verify_email.py +++ b/auth_signup_verify_email/tests/test_verify_email.py @@ -3,9 +3,10 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). from urllib import urlencode +from mock import patch from lxml.html import document_fromstring -from openerp import _ -from openerp.tests.common import at_install, post_install, HttpCase +from odoo.tests.common import at_install, post_install, HttpCase +from odoo.addons.mail.models import mail_template @at_install(False) @@ -21,43 +22,28 @@ class UICase(HttpCase): "csrf_token": self.csrf_token(), "name": "Somebody", } - self.msg = { - "badmail": _("That does not seem to be an email address."), - "failure": _( - "Something went wrong, please try again later or contact us."), - "success": _("Check your email to activate your account!"), - } def html_doc(self, url="/web/signup", data=None, timeout=30): """Get an HTML LXML document.""" if data: data = bytes(urlencode(data)) - return document_fromstring(self.url_open(url, data, timeout).read()) + with patch(mail_template.__name__ + ".MailTemplate.send_mail"): + result = self.url_open(url, data, timeout) + return document_fromstring(result.read()) def csrf_token(self): """Get a valid CSRF token.""" doc = self.html_doc() return doc.xpath("//input[@name='csrf_token']")[0].get("value") - def search_text(self, doc, text): - """Search for any element containing the text.""" - return doc.xpath("//*[contains(text(), '%s')]" % text) - def test_bad_email(self): """Test rejection of bad emails.""" self.data["login"] = "bad email" doc = self.html_doc(data=self.data) - self.assertTrue(self.search_text(doc, self.msg["badmail"])) + self.assertTrue(doc.xpath('//p[@class="alert alert-danger"]')) def test_good_email(self): - """Test acceptance of good emails. - - This test could lead to success if your SMTP settings are correct, or - to failure otherwise. Any case is expected, since tests usually run - under unconfigured demo instances. - """ + """Test acceptance of good emails.""" self.data["login"] = "good@example.com" doc = self.html_doc(data=self.data) - self.assertTrue( - self.search_text(doc, self.msg["failure"]) or - self.search_text(doc, self.msg["success"])) + self.assertTrue(doc.xpath('//p[@class="alert alert-success"]')) diff --git a/requirements.txt b/requirements.txt index 619afec50..2d61fd57f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ python-ldap unidecode acme_tiny IPy -validate_email +email_validator pyotp pysftp fdb