From b7d038d48d72b96e8eb021db420a716e80e7ef43 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Fri, 26 Jan 2018 12:33:19 +0000 Subject: [PATCH] [FIX+REF] auth_signup_verify_email: Fix tests + Skip mail send in tests and use email_validator (#29) 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 | 6 +-- auth_signup_verify_email/__manifest__.py | 2 +- auth_signup_verify_email/controllers/main.py | 37 ++++++++++--- auth_signup_verify_email/tests/__init__.py | 1 - .../tests/test_verify_email.py | 53 ++++++------------- 5 files changed, 46 insertions(+), 53 deletions(-) diff --git a/auth_signup_verify_email/README.rst b/auth_signup_verify_email/README.rst index e7ebc18d56..7494948f7e 100644 --- a/auth_signup_verify_email/README.rst +++ b/auth_signup_verify_email/README.rst @@ -38,11 +38,7 @@ To use this module, you need to: :alt: Try me on Runbot :target: https://runbot.odoo-community.org/runbot/149/11.0 -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 a965fefa4d..8352ad0ebf 100644 --- a/auth_signup_verify_email/__manifest__.py +++ b/auth_signup_verify_email/__manifest__.py @@ -16,7 +16,7 @@ "external_dependencies": { "python": [ "lxml", - "validate_email", + "email_validator", ], }, "data": [ diff --git a/auth_signup_verify_email/controllers/main.py b/auth_signup_verify_email/controllers/main.py index 38ad7e4742..9b9c916802 100644 --- a/auth_signup_verify_email/controllers/main.py +++ b/auth_signup_verify_email/controllers/main.py @@ -2,21 +2,36 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). import logging -from odoo import _, http -from odoo.http import request +from odoo import _ +from odoo.http import request, route 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): - @http.route() + @route() def web_auth_signup(self, *args, **kw): if (request.params.get("login") and not request.params.get("password")): @@ -28,10 +43,16 @@ def passwordless_signup(self, values): 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 request.render("auth_signup.signup", qcontext) - elif not values.get("email"): + if not values.get("email"): values["email"] = values.get("login") # preserve user lang diff --git a/auth_signup_verify_email/tests/__init__.py b/auth_signup_verify_email/tests/__init__.py index bed770df15..f658c7d101 100644 --- a/auth_signup_verify_email/tests/__init__.py +++ b/auth_signup_verify_email/tests/__init__.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # Copyright 2016 Jairo Llopis # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). diff --git a/auth_signup_verify_email/tests/test_verify_email.py b/auth_signup_verify_email/tests/test_verify_email.py index 244535f321..fca460b134 100644 --- a/auth_signup_verify_email/tests/test_verify_email.py +++ b/auth_signup_verify_email/tests/test_verify_email.py @@ -1,44 +1,32 @@ # Copyright 2016 Jairo Llopis # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +from mock import patch from lxml.html import document_fromstring -from odoo import _ -from odoo.tests.common import HttpCase +from odoo.tests.common import at_install, post_install, HttpCase +from odoo.addons.mail.models import mail_template from odoo.tools.misc import mute_logger +@at_install(False) +@post_install(True) class UICase(HttpCase): def setUp(self): super(UICase, self).setUp() - self.icp = self.env["ir.config_parameter"] - self.old_allow_uninvited = self.icp.get_param( - "auth_signup.allow_uninvited") - self.icp.set_param("auth_signup.allow_uninvited", "True") - - # Workaround https://github.com/odoo/odoo/issues/12237 - self.cr.commit() + with self.cursor() as cr: + env = self.env(cr) + icp = env["ir.config_parameter"] + icp.set_param("auth_signup.allow_uninvited", "True") self.data = { "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 tearDown(self): - """Workaround https://github.com/odoo/odoo/issues/12237.""" - super(UICase, self).tearDown() - self.icp.set_param( - "auth_signup.allow_uninvited", self.old_allow_uninvited) - self.cr.commit() - def html_doc(self, url="/web/signup", data=None, timeout=10): + def html_doc(self, url="/web/signup", data=None, timeout=30): """Get an HTML LXML document.""" - resp = self.url_open(url, data=data, timeout=timeout) + with patch(mail_template.__name__ + ".MailTemplate.send_mail"): + resp = self.url_open(url, data=data, timeout=timeout) return document_fromstring(resp.content) def csrf_token(self): @@ -46,26 +34,15 @@ def csrf_token(self): 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"]')) @mute_logger('odoo.addons.auth_signup_verify_email.controllers.main') 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"]'))