diff --git a/deploy/before.sql b/deploy/before.sql new file mode 100644 index 0000000000..2844fde1b2 --- /dev/null +++ b/deploy/before.sql @@ -0,0 +1,10 @@ +BEGIN; + -- In some cases, we don't have a participant linked to emails + ALTER TABLE email_messages ALTER COLUMN participant DROP NOT NULL; + + -- Email address to send emails to. If not provided, participant's primary email will be used. + ALTER TABLE email_messages ADD COLUMN email_address text; + + ALTER TABLE email_messages ADD CONSTRAINT email_or_participant_required + CHECK ((participant IS NOT NULL) OR (email_address IS NOT NULL)); +END; diff --git a/emails/initial.spt b/emails/initial.spt deleted file mode 100644 index f5cb5ca152..0000000000 --- a/emails/initial.spt +++ /dev/null @@ -1,21 +0,0 @@ -{{ _("Connect to {0} on Gratipay?", username) }} - -[---] text/html -A while ago we received a request to connect {{ escape(email) }} to the -{{ '{0}'.format(username) }} -account on Gratipay (formerly -Gittip). Now we're finally sending a verification email! Ring a bell? -
-
-Yes, proceed! - -[---] text/plain - -A while ago we received a request to connect `{{ email }}` -to the `{{ username }}` account on Gratipay (formerly Gittip). -Now we're finally sending a verification email! - -Ring a bell? Follow this link to finish connecting your email: - -{{ link }} diff --git a/gratipay/email.py b/gratipay/email.py index 356ed18955..0b99aa14e3 100644 --- a/gratipay/email.py +++ b/gratipay/email.py @@ -56,13 +56,17 @@ def _have_ses(self, env): and env.aws_ses_default_region - def put(self, to, template, _user_initiated=True, **context): + def put(self, to, template, _user_initiated=True, email=None, **context): """Put an email message on the queue. - :param Participant to: the participant to send the email message to + :param Participant to: the participant to send the email message to. + In cases where an email is not linked to a participant, this can be + ``None``. :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 unicode email: The email address to send this message to. If not + provided, the ``to`` participant's primary email is used. :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 @@ -74,24 +78,59 @@ def put(self, to, template, _user_initiated=True, **context): :returns: ``None`` """ + + assert to or email # Either participant or email address required. + with self.db.get_cursor() as cursor: + participant_id = to.id if to else None 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)) + (participant, + email_address, + spt_name, + context, + user_initiated) + VALUES (%s, %s, %s, %s, %s) + """, (participant_id, email, 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.allow_up_to: + nqueued = self._get_nqueued(cursor, to, email) + if nqueued > self.allow_up_to: raise Throttled() + def _get_nqueued(self, cursor, participant, email_address): + """Returns the number of messages already queued for the given + participant or email address. Prefers participant if provided, falls + back to email_address otherwise. + + :param Participant participant: The participant to check queued messages + for. + + :param unicode email_address: The email address to check queued messages + for. + + :returns number of queued messages + """ + + if participant: + return cursor.one(""" + SELECT COUNT(*) + FROM email_messages + WHERE user_initiated + AND participant=%s + AND result is null + """, (participant.id, )) + else: + return cursor.one(""" + SELECT COUNT(*) + FROM email_messages + WHERE user_initiated + AND email_address=%s + AND result is null + """, (email_address, )) + + def flush(self): """Load messages queued for sending, and send them. """ @@ -140,22 +179,34 @@ def _prepare_email_message_for_ses(self, rec): #. ``participant.email_address``. """ - to = Participant.from_id(rec.participant) + participant = Participant.from_id(rec.participant) spt = self._email_templates[rec.spt_name] context = pickle.loads(rec.context) - context['participant'] = to - context['username'] = to.username + email = rec.email_address or participant.email_address + + # Previously, email_address was stored in the 'email' key on `context` + # and not in the `email_address` field. Let's handle that case so that + # old emails don't suffer + # + # TODO: Remove this once we're sure old emails have gone out. + email = context.get('email', email) + + if not email: + raise NoEmailAddress() + + context['email'] = email + 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') + accept_lang = (participant and participant.email_lang) or 'en' + langs = i18n.parse_accept_lang(accept_lang) locale = i18n.match_lang(langs) i18n.add_helpers_to_context(self.tell_sentry, context, locale) context['escape'] = lambda s: s @@ -170,7 +221,12 @@ def render(t, context): message = {} message['Source'] = 'Gratipay Support ' message['Destination'] = {} - message['Destination']['ToAddresses'] = ["%s <%s>" % (to.username, email)] # "Name " + if participant: + # "username " + destination = "%s <%s>" % (participant.username, email) + else: + destination = email + message['Destination']['ToAddresses'] = [destination] message['Message'] = {} message['Message']['Subject'] = {} message['Message']['Subject']['Data'] = spt['subject'].render(context).strip() 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")