diff --git a/deploy/after.py b/deploy/after.py new file mode 100644 index 0000000000..a1709e9968 --- /dev/null +++ b/deploy/after.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + +import pickle + +from gratipay import wireup + + +db = wireup.db(wireup.env()) + + +with db.get_cursor() as cursor: + + # Add a required `recipient_address` field to `email_messages` that holds + # the recipient email address. Populate it from context or participant, + # and then constrain it to non-NULL. + + cursor.run('ALTER TABLE email_messages ADD COLUMN recipient_address text') + existing = cursor.all('SELECT id, p.email_address, context FROM email_messages em ' + 'JOIN participants ON p.id = em.participant') + for rec in existing: + context = pickle.loads(rec.context) + recipient_address = context.get('email', rec.email_address) + cursor.run( 'UPDATE email_messages SET recipient_address=%s WHERE id=%s' + , (recipient_address, rec.id) + ) + cursor.run('ALTER TABLE email_messages ALTER COLUMN recipient_address SET NOT NULL') + + + # Now convert `participant` into a NULL-able `sender_id` field, and fold + # the `email_messages.user_initiated` into that. + + cursor.run('ALTER TABLE email_messages RENAME COLUMN participant TO sender_id') + cursor.run('ALTER TABLE email_messages ALTER COLUMN sender_id DROP NOT NULL') + cursor.run('UPDATE TABLE email_messages SET sender_id=null WHERE not user_initiated') diff --git a/deploy/test.py b/deploy/test.py new file mode 100644 index 0000000000..154b8aff56 --- /dev/null +++ b/deploy/test.py @@ -0,0 +1,36 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + +import pickle + +from gratipay.exceptions import Throttled +from gratipay.testing import DeployHooksHarness + + +class Tests(DeployHooksHarness): + + def old_put(self, to, template, _user_initiated=True, **context): + with self.app.db.get_cursor() as cursor: + cursor.run(""" + INSERT INTO email_messages + (participant, spt_name, context, user_initiated) + VALUES (%s, %s, %s, %s) + """, (to.id, template, pickle.dumps(context), _user_initiated)) + if _user_initiated: + n = cursor.one(""" + SELECT count(*) + FROM email_messages + WHERE participant=%s + AND result is null + AND user_initiated + """, (to.id,)) + if n > self.app.allow_up_to: + raise Throttled() + + + def test_it_works(self): + alice = self.make_participant('alice', email_address='alice@example.com') + self.old_put(alice, 'verification', True) + self.run_deploy_hooks() + actual = self.db.all('SELECT recipient_address FROM email_messages') + assert actual == ['alice@example.com'] diff --git a/gratipay/email.py b/gratipay/email.py index 356ed18955..753c2ee250 100644 --- a/gratipay/email.py +++ b/gratipay/email.py @@ -56,41 +56,47 @@ def _have_ses(self, env): and env.aws_ses_default_region - def put(self, to, template, _user_initiated=True, **context): - """Put an email message on the queue. - - :param Participant to: the participant to send the email message to + def put(self, sender, recipient_address, template, accept_lang, **context): + """Put an email message on the outbound queue. + + :param Participant sender: the participant on whose behalf we are + sending the message. In the case of system-generated messages, this + can be ``None``. + :param unicode recipient_address: The email address to send the message + to. :param unicode template: the name of the template to use when rendering the email, corresponding to a filename in ``emails/`` without the file extension - :param bool _user_initiated: user-initiated emails are throttled; - system-initiated messages don't count against throttling :param dict context: the values to use when rendering the template - :raise Throttled: if the participant already has a few messages in the - queue (that they put there); the specific number is tunable with - the ``EMAIL_QUEUE_ALLOW_UP_TO`` envvar. + :raise Throttled: if the sender already has a few messages in the + queue; the specific number is tunable with the + ``EMAIL_QUEUE_ALLOW_UP_TO`` envvar. :returns: ``None`` """ + with self.db.get_cursor() as cursor: - cursor.run(""" - INSERT INTO email_messages - (participant, spt_name, context, user_initiated) - VALUES (%s, %s, %s, %s) - """, (to.id, template, pickle.dumps(context), _user_initiated)) - if _user_initiated: - n = cursor.one(""" + if sender is None: + sender_id = None + else: + sender_id = sender.id + nqueued = cursor.one(""" SELECT count(*) FROM email_messages - WHERE participant=%s + WHERE sender_id=%s AND result is null - AND user_initiated - """, (to.id,)) - if n > self.allow_up_to: + """, (sender_id,)) + if nqueued > self.allow_up_to: raise Throttled() + cursor.run(""" + INSERT INTO email_queue + (sender_id, recipient_address, spt_name, context) + VALUES (%s, %s, %s, %s) + """, (sender_id, recipient_address, template, pickle.dumps(context))) + def flush(self): """Load messages queued for sending, and send them. @@ -130,32 +136,24 @@ def _prepare_email_message_for_ses(self, rec): """Prepare an email message for delivery via Amazon SES. :param Record rec: a database record from the ``email_messages`` table - - :returns: ``dict`` if we can find an email address to send to - :raises: ``NoEmailAddress`` if we can't find an email address to send to - - We look for an email address to send to in two places: - - #. the context stored in ``rec.context``, and then - #. ``participant.email_address``. + :returns: ``dict`` """ - to = Participant.from_id(rec.participant) spt = self._email_templates[rec.spt_name] context = pickle.loads(rec.context) - context['participant'] = to - context['username'] = to.username + context['recipient_address'] = rec.recipient_address + if participant: + context['participant'] = participant + context['username'] = participant.username context['button_style'] = ( "color: #fff; text-decoration:none; display:inline-block; " "padding: 0 15px; background: #396; white-space: nowrap; " "font: normal 14px/40px Arial, sans-serif; border-radius: 3px" ) context.setdefault('include_unsubscribe', True) - email = context.setdefault('email', to.email_address) - if not email: - raise NoEmailAddress() - langs = i18n.parse_accept_lang(to.email_lang or 'en') + + langs = i18n.parse_accept_lang(rec.accept_lang) locale = i18n.match_lang(langs) i18n.add_helpers_to_context(self.tell_sentry, context, locale) context['escape'] = lambda s: s @@ -163,6 +161,7 @@ def _prepare_email_message_for_ses(self, rec): i18n.add_helpers_to_context(self.tell_sentry, context_html, locale) context_html['escape'] = htmlescape base_spt = self._email_templates['base'] + def render(t, context): b = base_spt[t].render(context).strip() return b.replace('$body', spt[t].render(context).strip()) @@ -170,7 +169,7 @@ def render(t, context): message = {} message['Source'] = 'Gratipay Support ' message['Destination'] = {} - message['Destination']['ToAddresses'] = ["%s <%s>" % (to.username, email)] # "Name " + message['Destination']['ToAddresses'] = [rec.recipient_address] message['Message'] = {} message['Message']['Subject'] = {} message['Message']['Subject']['Data'] = spt['subject'].render(context).strip() diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index 029fe337ee..00e4d9b687 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -93,7 +93,6 @@ def start_email_verification(self, email, *packages): verified_emails = self.get_verified_email_addresses() kwargs = dict( npackages=len(packages) , package_name=packages[0].name if packages else '' - , new_email=email , new_email_verified=email in verified_emails , link=link , include_unsubscribe=False diff --git a/tests/py/test_email.py b/tests/py/test_email.py index c93b538e36..0a76f91814 100644 --- a/tests/py/test_email.py +++ b/tests/py/test_email.py @@ -6,22 +6,57 @@ from gratipay.exceptions import NoEmailAddress, Throttled from gratipay.testing import Harness -from gratipay.testing.email import SentEmailHarness +from gratipay.testing.email import SentEmailHarness, QueuedEmailHarness -class TestPut(SentEmailHarness): +class TestPut(QueuedEmailHarness): def setUp(self): - SentEmailHarness.setUp(self) + QueuedEmailHarness.setUp(self) self.alice = self.make_participant('alice', claimed_time='now', email_address='alice@example.com') - def test_queueing_email_is_throttled(self): + def test_put_with_only_email(self): + self.app.email_queue.put(None, 'base', email='dummy@example.com') + last_email = self.get_last_email() + assert last_email['to'] == 'dummy@example.com' + + def test_put_with_only_participant(self): + self.app.email_queue.put(self.alice, 'base') + + # Should pickup primary email + assert self.get_last_email()['to'] == 'alice ' + + def test_put_with_participant_and_email(self): + self.app.email_queue.put(self.alice, 'base', email='alice2@example.com') + + # Should prefer given email over primary + assert self.get_last_email()['to'] == 'alice ' + + def test_put_complains_if_participant_and_email_not_provided(self): + with raises(AssertionError): + self.app.email_queue.put(None, "base") + + def test_put_throttles_on_participant(self): self.app.email_queue.put(self.alice, "base") self.app.email_queue.put(self.alice, "base") self.app.email_queue.put(self.alice, "base") raises(Throttled, self.app.email_queue.put, self.alice, "base") - def test_queueing_email_writes_timestamp(self): + def test_put_throttles_on_email(self): + self.app.email_queue.put(None, "base", email="alice@gratipay.com") + self.app.email_queue.put(None, "base", email="alice@gratipay.com") + self.app.email_queue.put(None, "base", email="alice@gratipay.com") + with raises(Throttled): + self.app.email_queue.put(None, "base", email="alice@gratipay.com") + + def test_put_throttles_on_participant_across_different_emails(self): + self.app.email_queue.put(self.alice, "base", email="a@b.com") + self.app.email_queue.put(self.alice, "base", email="b@c.com") + self.app.email_queue.put(self.alice, "base", email="c@d.com") + with raises(Throttled): + self.app.email_queue.put(self.alice, "base", email="d@e.com") + + def test_timestamp_is_written(self): self.app.email_queue.put(self.alice, "base") ctime = self.db.one("SELECT EXTRACT(epoch FROM ctime) FROM email_messages")