Skip to content

Commit

Permalink
[FIX+REF] auth_signup_verify_email: Fix tests + Skip mail send in tes…
Browse files Browse the repository at this point in the history
…ts and use email_validator (OCA#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 syrusakbary/validate_email#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.
  • Loading branch information
yajo authored and em230418 committed May 15, 2020
1 parent 946ca3b commit f12d7ad
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 53 deletions.
6 changes: 1 addition & 5 deletions auth_signup_verify_email/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
===========
Expand Down
2 changes: 1 addition & 1 deletion auth_signup_verify_email/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"external_dependencies": {
"python": [
"lxml",
"validate_email",
"email_validator",
],
},
"data": [
Expand Down
37 changes: 29 additions & 8 deletions auth_signup_verify_email/controllers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")):
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion auth_signup_verify_email/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-
# Copyright 2016 Jairo Llopis <jairo.llopis@tecnativa.com>
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

Expand Down
53 changes: 15 additions & 38 deletions auth_signup_verify_email/tests/test_verify_email.py
Original file line number Diff line number Diff line change
@@ -1,71 +1,48 @@
# Copyright 2016 Jairo Llopis <jairo.llopis@tecnativa.com>
# 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):
"""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"]'))

@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"]'))

0 comments on commit f12d7ad

Please sign in to comment.