Skip to content

Commit

Permalink
Merge pull request #1590 from pmclanahan/validate-email-addresses-912055
Browse files Browse the repository at this point in the history
Bug 912055: Support invalid email errors from basket.
  • Loading branch information
jgmize committed Feb 24, 2014
2 parents ec4bd19 + c581dc0 commit 08eb3fd
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 22 deletions.
3 changes: 2 additions & 1 deletion bedrock/mozorg/email_contribute.py
Expand Up @@ -139,7 +139,8 @@ def handle_form(request, form):
pass
else:
try:
basket.subscribe(data['email'], 'about-mozilla')
basket.subscribe(data['email'], 'about-mozilla',
source_url=request.build_absolute_uri())
except basket.BasketException:
pass

Expand Down
2 changes: 1 addition & 1 deletion bedrock/mozorg/tests/test_views.py
Expand Up @@ -232,7 +232,7 @@ def test_webmaker_mentor_signup_functional_area_fail(self, mock_post, mock_subsc
self.data.update(interest='coding', newsletter=True)
self.client.post(self.url_en, self.data)

mock_subscribe.assert_called_with(self.contact, 'about-mozilla')
mock_subscribe.assert_called_with(self.contact, 'about-mozilla', source_url=ANY)
assert_false(mock_post.called)

@patch.object(ReCaptchaField, 'clean', Mock())
Expand Down
16 changes: 9 additions & 7 deletions bedrock/newsletter/helpers.py
Expand Up @@ -5,12 +5,13 @@
import logging

import basket
import basket.errors
import jingo
import jinja2

from lib.l10n_utils import get_locale
from lib.l10n_utils.dotlang import _lazy
from bedrock.newsletter.forms import NewsletterFooterForm
from bedrock.newsletter.views import invalid_email_address, general_error


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -52,12 +53,13 @@ def email_newsletter_form(ctx, newsletters='mozilla-and-you', title=None,
try:
basket.subscribe(data['email'], form.newsletters,
**kwargs)
except basket.BasketException:
log.exception("Error subscribing %s to newsletter %s" %
(data['email'], newsletters))
msg = _lazy("We are sorry, but there was a problem "
"with our system. Please try again later!")
form.errors['__all__'] = form.error_class([msg])
except basket.BasketException as e:
if e.code == basket.errors.BASKET_INVALID_EMAIL:
form.errors['email'] = form.error_class([invalid_email_address])
else:
log.exception("Error subscribing %s to newsletter %s" %
(data['email'], form.newsletters))
form.errors['__all__'] = form.error_class([general_error])
else:
success = True

Expand Down
7 changes: 5 additions & 2 deletions bedrock/newsletter/tests/test_misc.py
Expand Up @@ -2,7 +2,7 @@

from django.test.utils import override_settings

from basket import BasketException
from basket import BasketException, errors
from bedrock.mozorg.tests import TestCase
from bedrock.newsletter.utils import get_newsletters, get_languages_for_newsletters
from bedrock.newsletter.tests import newsletters
Expand Down Expand Up @@ -31,7 +31,10 @@ def test_simple_get(self):
def test_get_newsletters_fallback(self, mock_basket_get_newsletters):
# if get_newsletters() cannot reach basket, it returns the
# newsletters from settings
mock_basket_get_newsletters.side_effect = BasketException
mock_basket_get_newsletters.side_effect = BasketException(
'network error',
code=errors.BASKET_NETWORK_FAILURE,
)
default_value = mock.Mock()
with override_settings(DEFAULT_NEWSLETTERS=default_value):
return_value = get_newsletters()
Expand Down
11 changes: 7 additions & 4 deletions bedrock/newsletter/tests/test_views.py
Expand Up @@ -6,7 +6,7 @@
from django.http import HttpResponse
from django.test.client import RequestFactory

from basket import BasketException
from basket import BasketException, errors
from funfactory.urlresolvers import reverse
from mock import DEFAULT, Mock, patch
from nose.tools import eq_, ok_
Expand Down Expand Up @@ -452,7 +452,8 @@ def test_basket_down(self):
def test_bad_token(self):
"""If the token is bad, we report the appropriate error"""
with patch('basket.confirm') as confirm:
confirm.side_effect = BasketException(status_code=403)
confirm.side_effect = BasketException(status_code=403,
code=errors.BASKET_UNKNOWN_TOKEN)
with patch('lib.l10n_utils.render') as mock_render:
mock_render.return_value = HttpResponse('')
rsp = self.client.get(self.url, follow=True)
Expand All @@ -466,7 +467,8 @@ def test_bad_token(self):

class TestRecoveryView(TestCase):
def setUp(self):
self.url = reverse('newsletter.recovery')
with self.activate('en-US'):
self.url = reverse('newsletter.recovery')

def test_bad_email(self):
"""Email syntax errors are caught"""
Expand All @@ -479,7 +481,8 @@ def test_bad_email(self):
def test_unknown_email(self, mock_basket):
"""Unknown email addresses give helpful error message"""
data = {'email': 'unknown@example.com'}
mock_basket.side_effect = BasketException(status_code=404)
mock_basket.side_effect = BasketException(status_code=404,
code=errors.BASKET_UNKNOWN_EMAIL)
rsp = self.client.post(self.url, data)
self.assertTrue(mock_basket.called)
self.assertEqual(200, rsp.status_code)
Expand Down
16 changes: 10 additions & 6 deletions bedrock/newsletter/views.py
Expand Up @@ -14,6 +14,7 @@
from django.views.decorators.cache import never_cache

import basket
import basket.errors
import commonware.log
import lib.l10n_utils as l10n_utils
import requests
Expand Down Expand Up @@ -45,6 +46,8 @@
u'This email address is not in our system. Please double check your '
u'address or <a href="%s">subscribe to our newsletters.</a>')

invalid_email_address = _lazy(u'This is not a valid email address. '
u'Please check the spelling.')

UNSUB_UNSUBSCRIBED_ALL = 1
UNSUB_REASONS_SUBMITTED = 2
Expand Down Expand Up @@ -72,8 +75,7 @@ def confirm(request, token):
result = basket.confirm(token)
except basket.BasketException as e:
log.exception("Exception confirming token %s" % token)
if e.status_code == 403:
# Basket returns 403 on bad token
if e.code == basket.errors.BASKET_UNKNOWN_TOKEN:
token_error = True
else:
# Any other exception
Expand Down Expand Up @@ -142,8 +144,8 @@ def existing(request, token=None):
log.exception("Basket timeout")
messages.add_message(request, messages.ERROR, general_error)
return l10n_utils.render(request, 'newsletter/existing.html')
except basket.BasketException:
log.exception("FAILED to get user from token")
except basket.BasketException as e:
log.exception("FAILED to get user from token (%s)", e.desc)
else:
user_exists = True

Expand Down Expand Up @@ -384,8 +386,10 @@ def recovery(request):
# Try it - basket will return an error if the email is unknown
basket.send_recovery_message(email)
except basket.BasketException as e:
# Was it that their email was not known?
if e.status_code == 404:
# Was it that their email was not known? Or it could be invalid,
# but that doesn't really make a difference.
if e.code in (basket.errors.BASKET_UNKNOWN_EMAIL,
basket.errors.BASKET_INVALID_EMAIL):
# Tell them, give them a link to go subscribe if they want
url = reverse('newsletter.mozilla-and-you')
form.errors['email'] = \
Expand Down
2 changes: 1 addition & 1 deletion vendor-local/src/basket-client

0 comments on commit 08eb3fd

Please sign in to comment.