Shelve

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')

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='[email protected]')
+        self.old_put(alice, 'verification', True)
+        self.run_deploy_hooks()
+        actual = self.db.all('SELECT recipient_address FROM email_messages')
+        assert actual == ['[email protected]']

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,47 +136,40 @@ 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
         context_html = dict(context)
         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())
 
         message = {}
         message['Source'] = 'Gratipay Support <[email protected]>'
         message['Destination'] = {}
-        message['Destination']['ToAddresses'] = ["%s <%s>" % (to.username, email)] # "Name <[email protected]>"
+        message['Destination']['ToAddresses'] = [rec.recipient_address]
         message['Message'] = {}
         message['Message']['Subject'] = {}
         message['Message']['Subject']['Data'] = spt['subject'].render(context).strip()

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

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='[email protected]')
 
-    def test_queueing_email_is_throttled(self):
+    def test_put_with_only_email(self):
+        self.app.email_queue.put(None, 'base', email='[email protected]')
+        last_email = self.get_last_email()
+        assert last_email['to'] == '[email protected]'
+
+    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 <[email protected]>'
+
+    def test_put_with_participant_and_email(self):
+        self.app.email_queue.put(self.alice, 'base', email='[email protected]')
+
+        # Should prefer given email over primary
+        assert self.get_last_email()['to'] == 'alice <[email protected]>'
+
+    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="[email protected]")
+        self.app.email_queue.put(None, "base", email="[email protected]")
+        self.app.email_queue.put(None, "base", email="[email protected]")
+        with raises(Throttled):
+            self.app.email_queue.put(None, "base", email="[email protected]")
+
+    def test_put_throttles_on_participant_across_different_emails(self):
+        self.app.email_queue.put(self.alice, "base", email="[email protected]")
+        self.app.email_queue.put(self.alice, "base", email="[email protected]")
+        self.app.email_queue.put(self.alice, "base", email="[email protected]")
+        with raises(Throttled):
+            self.app.email_queue.put(self.alice, "base", email="[email protected]")
+
+    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")