diff --git a/bedrock/mozorg/email_contribute.py b/bedrock/mozorg/email_contribute.py index 42dfe937c25..a2d71ac3858 100644 --- a/bedrock/mozorg/email_contribute.py +++ b/bedrock/mozorg/email_contribute.py @@ -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 diff --git a/bedrock/mozorg/tests/test_views.py b/bedrock/mozorg/tests/test_views.py index 71401395381..c9a7fe1c17b 100644 --- a/bedrock/mozorg/tests/test_views.py +++ b/bedrock/mozorg/tests/test_views.py @@ -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()) diff --git a/bedrock/newsletter/helpers.py b/bedrock/newsletter/helpers.py index deea369a628..6576c3f9ff6 100644 --- a/bedrock/newsletter/helpers.py +++ b/bedrock/newsletter/helpers.py @@ -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__) @@ -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 diff --git a/bedrock/newsletter/tests/test_misc.py b/bedrock/newsletter/tests/test_misc.py index 94be1cd785c..5a0896cd062 100644 --- a/bedrock/newsletter/tests/test_misc.py +++ b/bedrock/newsletter/tests/test_misc.py @@ -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 @@ -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() diff --git a/bedrock/newsletter/tests/test_views.py b/bedrock/newsletter/tests/test_views.py index dd649f9ca1c..5b2c9d2f0e8 100644 --- a/bedrock/newsletter/tests/test_views.py +++ b/bedrock/newsletter/tests/test_views.py @@ -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_ @@ -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) @@ -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""" @@ -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) diff --git a/bedrock/newsletter/views.py b/bedrock/newsletter/views.py index d92d996b821..a7663f71766 100644 --- a/bedrock/newsletter/views.py +++ b/bedrock/newsletter/views.py @@ -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 @@ -45,6 +46,8 @@ u'This email address is not in our system. Please double check your ' u'address or subscribe to our newsletters.') +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 @@ -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 @@ -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 @@ -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'] = \ diff --git a/vendor-local/src/basket-client b/vendor-local/src/basket-client index fb6b817e401..1ca127a17e1 160000 --- a/vendor-local/src/basket-client +++ b/vendor-local/src/basket-client @@ -1 +1 @@ -Subproject commit fb6b817e40165b8137c006f3f6433ca8e7ade80c +Subproject commit 1ca127a17e17f5e67c2d9f9a500d85e97460f13d