Skip to content

Commit

Permalink
Bug 912055: Support invalid email errors from basket.
Browse files Browse the repository at this point in the history
  • Loading branch information
pmac committed Jan 15, 2014
1 parent 8a4c103 commit c581dc0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 21 deletions.
3 changes: 2 additions & 1 deletion bedrock/mozorg/email_contribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit c581dc0

Please sign in to comment.