Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Commit

Permalink
Merge pull request #121 from pmclanahan/fix-confirm-message-failures-…
Browse files Browse the repository at this point in the history 
…1090432

Bug 1090432: Retry when we see "No valid subscriber" error.
  • Loading branch information
@pmac
pmac committed Oct 28, 2014
2 parents c78d28a + dc304b8 commit 6c042b2
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 84 deletions.
15 changes: 7 additions & 8 deletions news/tasks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def update_fxa_info(email, lang, fxa_id, source_url=None):

apply_updates(settings.EXACTTARGET_DATA, record)
welcome = mogrify_message_id(FXACCOUNT_WELCOME, lang, welcome_format)
send_message(welcome, email, token, welcome_format)
send_message.delay(welcome, email, token, welcome_format)


@et_task
Expand Down Expand Up @@ -367,7 +367,7 @@ def update_get_involved(interest_id, lang, name, email, country, email_format,
'INTEREST': interest_id,
})
welcome_id = mogrify_message_id(interest.welcome_id, lang, email_format)
send_message(welcome_id, email, token, email_format)
send_message.delay(welcome_id, email, token, email_format)
interest.notify_stewards(name, email, lang, message)

if to_subscribe:
Expand Down Expand Up @@ -580,6 +580,7 @@ def apply_updates(target_et, record):
et.data_ext().add_record(target_et, record.keys(), record.values())


@et_task
def send_message(message_id, email, token, format):
"""
Ask ET to send a message.
Expand Down Expand Up @@ -615,8 +616,6 @@ def send_message(message_id, email, token, format):
# remember it's a bad message ID so we don't try again during this process.
BAD_MESSAGE_ID_CACHE.set(message_id, True)
return
elif 'There are no valid subscribers.' in e.message:
raise BasketError("ET says: there are no valid subscribers.")
# we should retry
raise

Expand Down Expand Up @@ -669,7 +668,7 @@ def send_confirm_notice(email, token, lang, format, newsletter_slugs):
welcome = CONFIRMATION_MESSAGE

welcome = mogrify_message_id(welcome, lang, format)
send_message(welcome, email, token, format)
send_message.delay(welcome, email, token, format)


def send_welcomes(user_data, newsletter_slugs, format):
Expand Down Expand Up @@ -717,8 +716,8 @@ def send_welcomes(user_data, newsletter_slugs, format):
for welcome in welcomes_to_send:
log.info("Sending welcome %s to user %s %s" %
(welcome, user_data['email'], user_data['token']))
send_message(welcome, user_data['email'], user_data['token'],
format)
send_message.delay(welcome, user_data['email'], user_data['token'],
format)


@et_task
Expand Down Expand Up @@ -828,4 +827,4 @@ def send_recovery_message_task(email):
format = user_data.get('format', 'H') or 'H'

message_id = mogrify_message_id(RECOVERY_MESSAGE_ID, lang, format)
send_message(message_id, email, user_data['token'], format)
send_message.delay(message_id, email, user_data['token'], format)
12 changes: 6 additions & 6 deletions news/tests/test_confirm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ def test_text_welcome(self, apply_updates, send_message):
}
confirm_user(token, user_data)
expected_welcome = "%s_%s_%s" % (lang, welcome, format)
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -434,7 +434,7 @@ def test_html_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# Lang code is lowercased. And we don't append anything for HTML.
expected_welcome = "%s_%s" % (lang.lower(), welcome)
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -465,7 +465,7 @@ def test_bad_lang_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# They're getting English. And we don't append anything for HTML.
expected_welcome = "en_%s" % welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -495,7 +495,7 @@ def test_long_lang_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# They're getting pt. And we don't append anything for HTML.
expected_welcome = "pt_%s" % welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -525,7 +525,7 @@ def test_other_long_lang_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# They're getting pt. And we don't append anything for HTML.
expected_welcome = "pt_%s" % welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -554,4 +554,4 @@ def test_one_lang_welcome(self, apply_updates, send_message):
}
confirm_user(token, user_data)
expected_welcome = 'en_' + welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)
72 changes: 36 additions & 36 deletions news/tests/test_send_confirm_notice.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,48 +31,48 @@ def test_default_confirm_notice_H(self, send_message):
"""Test newsletter that has no explicit confirm"""
send_confirm_notice(self.email, self.token, "en", "H", ['slug2'])
expected_message = mogrify_message_id(CONFIRMATION_MESSAGE, 'en', 'H')
send_message.assert_called_with(expected_message,
self.email,
self.token,
'H')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'H')

def test_default_confirm_notice_T(self, send_message):
"""Test newsletter that has no explicit confirm"""
send_confirm_notice(self.email, self.token, "es", "T", ['slug2'])
expected_message = mogrify_message_id(CONFIRMATION_MESSAGE, 'es', 'T')
send_message.assert_called_with(expected_message,
self.email,
self.token,
'T')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'T')

def test_default_confirm_notice_short_for_long(self, send_message):
"""Test using short form of lang that's in the newsletter list as a long lang"""
send_confirm_notice(self.email, self.token, "rr", "H", ['slug2'])
expected_message = mogrify_message_id(CONFIRMATION_MESSAGE, 'rr', 'H')
send_message.assert_called_with(expected_message,
self.email,
self.token,
'H')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'H')

### known long lang

def test_default_confirm_notice_long_lang_H(self, send_message):
"""Test newsletter that has no explicit confirm with long lang code"""
send_confirm_notice(self.email, self.token, "es-ES", "H", ['slug2'])
expected_message = mogrify_message_id(CONFIRMATION_MESSAGE, 'es', 'H')
send_message.assert_called_with(expected_message,
self.email,
self.token,
'H')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'H')

def test_default_confirm_notice_long_lang_T(self, send_message):
"""Test newsletter that has no explicit confirm with long lang code"""
send_confirm_notice(self.email, self.token, "es-ES", "T", ['slug2'])
expected_message = mogrify_message_id(CONFIRMATION_MESSAGE, 'es', 'T')
send_message.assert_called_with(expected_message,
self.email,
self.token,
'T')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'T')

### bad lang

Expand All @@ -97,39 +97,39 @@ def test_explicit_confirm_notice_H(self, send_message):
"""Test newsletter that has explicit confirm message"""
send_confirm_notice(self.email, self.token, "en", "H", ['slug1'])
expected_message = 'en_confirm1'
send_message.assert_called_with(expected_message,
self.email,
self.token,
'H')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'H')

def test_explicit_confirm_notice_T(self, send_message):
"""Test newsletter that has explicit confirm message"""
send_confirm_notice(self.email, self.token, "en", "T", ['slug1'])
expected_message = 'en_confirm1_T'
send_message.assert_called_with(expected_message,
self.email,
self.token,
'T')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'T')

### known long lang

def test_explicit_confirm_notice_long_lang_H(self, send_message):
"""Test newsletter that has explicit confirm message with long lang"""
send_confirm_notice(self.email, self.token, "en-US", "H", ['slug1'])
expected_message = 'en_confirm1'
send_message.assert_called_with(expected_message,
self.email,
self.token,
'H')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'H')

def test_explicit_confirm_notice_long_lang_T(self, send_message):
"""Test newsletter that has explicit confirm message with long lang"""
send_confirm_notice(self.email, self.token, "en-US", "T", ['slug1'])
expected_message = 'en_confirm1_T'
send_message.assert_called_with(expected_message,
self.email,
self.token,
'T')
send_message.delay.assert_called_with(expected_message,
self.email,
self.token,
'T')

### bad lang

Expand Down
12 changes: 6 additions & 6 deletions news/tests/test_send_welcomes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_text_welcome(self, apply_updates, send_message):
}
confirm_user(token, user_data)
expected_welcome = "%s_%s_%s" % (lang, welcome, format)
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -116,7 +116,7 @@ def test_html_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# Lang code is lowercased. And we don't append anything for HTML.
expected_welcome = "%s_%s" % (lang.lower(), welcome)
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -147,7 +147,7 @@ def test_bad_lang_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# They're getting English. And we don't append anything for HTML.
expected_welcome = "en_%s" % welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -177,7 +177,7 @@ def test_long_lang_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# They're getting pt. And we don't append anything for HTML.
expected_welcome = "pt_%s" % welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -207,7 +207,7 @@ def test_other_long_lang_welcome(self, apply_updates, send_message):
confirm_user(token, user_data)
# They're getting pt. And we don't append anything for HTML.
expected_welcome = "pt_%s" % welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)

@patch('news.tasks.send_message')
@patch('news.tasks.apply_updates')
Expand Down Expand Up @@ -236,4 +236,4 @@ def test_one_lang_welcome(self, apply_updates, send_message):
}
confirm_user(token, user_data)
expected_welcome = 'en_' + welcome
send_message.assert_called_with(expected_welcome, email, token, format)
send_message.delay.assert_called_with(expected_welcome, email, token, format)
8 changes: 4 additions & 4 deletions news/tests/test_tasks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ def test_email_only_in_et(self, mock_look_for_user, mock_send):
subscriber = Subscriber.objects.get(email=self.email)
self.assertEqual('USERTOKEN', subscriber.token)
message_id = mogrify_message_id(RECOVERY_MESSAGE_ID, lang, format)
mock_send.assert_called_with(message_id, self.email,
subscriber.token, format)
mock_send.delay.assert_called_with(message_id, self.email,
subscriber.token, format)

def test_email_only_in_basket(self, mock_look_for_user, mock_send):
"""Email known in basket, not in ET"""
Expand All @@ -136,8 +136,8 @@ def test_email_in_both(self, mock_look_for_user, mock_send):
}
send_recovery_message_task(self.email)
message_id = mogrify_message_id(RECOVERY_MESSAGE_ID, lang, format)
mock_send.assert_called_with(message_id, self.email,
subscriber.token, format)
mock_send.delay.assert_called_with(message_id, self.email,
subscriber.token, format)


class UpdateUserTests(TestCase):
Expand Down
24 changes: 12 additions & 12 deletions news/tests/test_update_user.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ def test_update_send_newsletter_welcome(self, get_user_data, send_message,
self.assertEqual(UU_ALREADY_CONFIRMED, rc)
apply_updates.assert_called()
# The welcome should have been sent
send_message.assert_called()
send_message.assert_called_with('en_' + welcome_id, self.sub.email,
self.sub.token, 'H')
send_message.delay.assert_called()
send_message.delay.assert_called_with('en_' + welcome_id, self.sub.email,
self.sub.token, 'H')

@patch('news.tasks.apply_updates')
@patch('news.tasks.send_message')
Expand Down Expand Up @@ -258,8 +258,8 @@ def test_update_send_newsletters_welcome(self, get_user_data,
api_call_type=SUBSCRIBE,
optin=True)
self.assertEqual(UU_EXEMPT_NEW, rc)
self.assertEqual(2, send_message.call_count)
calls_args = [x[0] for x in send_message.call_args_list]
self.assertEqual(2, send_message.delay.call_count)
calls_args = [x[0] for x in send_message.delay.call_args_list]
self.assertIn(('en_WELCOME1', self.sub.email, self.sub.token, 'H'),
calls_args)
self.assertIn(('en_WELCOME2', self.sub.email, self.sub.token, 'H'),
Expand Down Expand Up @@ -294,7 +294,7 @@ def test_update_user_works_with_no_welcome(self, get_user_data,
api_call_type=SUBSCRIBE, optin=True)
self.assertEqual(UU_ALREADY_CONFIRMED, rc)
apply_updates.assert_called()
send_message.assert_called()
send_message.delay.assert_called()

@patch('news.tasks.apply_updates')
@patch('news.tasks.send_message')
Expand Down Expand Up @@ -325,10 +325,10 @@ def test_update_user_works_with_no_lang(self, get_user_data,
api_call_type=SUBSCRIBE, optin=True)
self.assertEqual(UU_EXEMPT_NEW, rc)
apply_updates.assert_called()
send_message.assert_called_with(u'en_Welcome_T',
self.sub.email,
self.sub.token,
'T')
send_message.delay.assert_called_with(u'en_Welcome_T',
self.sub.email,
self.sub.token,
'T')

@patch('news.tasks.apply_updates')
@patch('news.tasks.send_message')
Expand Down Expand Up @@ -366,8 +366,8 @@ def test_ffos_welcome(self, get_user_data, send_message, apply_updates):
api_call_type=SUBSCRIBE,
optin=True)
self.assertEqual(UU_EXEMPT_NEW, rc)
self.assertEqual(1, send_message.call_count)
calls_args = [x[0] for x in send_message.call_args_list]
self.assertEqual(1, send_message.delay.call_count)
calls_args = [x[0] for x in send_message.delay.call_args_list]
self.assertIn(('en_FFOS_WELCOME', self.sub.email, self.sub.token, 'H'),
calls_args)
self.assertNotIn(('en_FF&Y_WELCOME', self.sub.email,
Expand Down
Loading

0 comments on commit 6c042b2

Please sign in to comment.