Replace throttling with verification limitation

gratipay/billing/payday.py

@@ -476,8 +476,7 @@ def notify_participants(self):
                 'charge_'+e.status,
                 exchange=dict(id=e.id, amount=e.amount, fee=e.fee, note=e.note),
                 nteams=nteams,
-                top_team=top_team,
-                _user_initiated=False
+                top_team=top_team
             )
 
 

gratipay/cli/queue_branch_email.py

@@ -90,4 +90,4 @@ def prompt(msg):
         _print( "{:>4} queuing for {} ({}={})".format(i, p.email_address, p.username, p.id)
               , file=sys.stderr
                )
-        app.email_queue.put(p, 'branch', include_unsubscribe=False, _user_initiated=False)
+        app.email_queue.put(p, 'branch', include_unsubscribe=False)

gratipay/email.py

@@ -13,7 +13,6 @@
 from jinja2 import Environment
 from markupsafe import escape as htmlescape
 
-from gratipay.exceptions import Throttled
 from gratipay.models.participant import Participant
 from gratipay.utils import find_files, i18n
 
@@ -55,35 +54,24 @@ 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, **context):
         """Put an email message on the queue.
 
         :param Participant to: the participant to send the email 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.
-
         :returns: ``None``
 
         """
         with self.db.get_cursor() as cursor:
             cursor.run("""
                 INSERT INTO email_queue
-                            (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_queue '
-                               'WHERE participant=%s AND user_initiated', (to.id,))
-                if n > self.allow_up_to:
-                    raise Throttled()
+                            (participant, spt_name, context)
+                     VALUES (%s, %s, %s)
+            """, (to.id, template, pickle.dumps(context)))
 
 
     def flush(self):

gratipay/exceptions.py

@@ -44,20 +44,19 @@ def lazy_body(self, _):
 
 class EmailNotOnFile(ProblemChangingEmail):
     def lazy_body(self, _):
-         return _("That email address is not on file for this package.")
+        return _("That email address is not on file for this package.")
 
 class EmailNotVerified(ProblemChangingEmail):
     def lazy_body(self, _):
-         return _("That email address is not verified.")
+        return _("That email address is not verified.")
 
 class TooManyEmailAddresses(ProblemChangingEmail):
     def lazy_body(self, _):
-         return _("You've reached the maximum number of email addresses we allow.")
+        return _("You've reached the maximum number of email addresses we allow.")
 
-
-class Throttled(LocalizedErrorResponse):
+class TooManyVerifications(ProblemChangingEmail):
     def lazy_body(self, _):
-         return _("You've initiated too many emails too quickly. Please try again in a minute or two.")
+        return _( "You have an outstanding verification request that must be completed before you can verify a different email address.")
 
 
 class ProblemChangingNumber(Exception):

gratipay/models/participant/email.py

@@ -9,7 +9,8 @@
 
 import gratipay
 from gratipay.exceptions import EmailAlreadyVerified, EmailTaken, CannotRemovePrimaryEmail
-from gratipay.exceptions import EmailNotVerified, TooManyEmailAddresses, EmailNotOnFile
+from gratipay.exceptions import EmailNotVerified, EmailNotOnFile
+from gratipay.exceptions import TooManyEmailAddresses, TooManyVerifications
 from gratipay.security.crypto import constant_time_compare
 from gratipay.utils import encode_for_querystring
 
@@ -72,7 +73,8 @@ def start_email_verification(self, email, *packages):
         :raises EmailNotOnFile: if the email address is not on file for any of
             the packages
         :raises TooManyEmailAddresses: if the participant already has 10 emails
-        :raises Throttled: if the participant adds too many emails too quickly
+        :raises TooManyVerifications: if the participant already has a
+            verification attempt outstanding for a different email address
 
         """
         with self.db.get_cursor() as c:
@@ -91,11 +93,6 @@ def start_email_verification(self, email, *packages):
         if self.email_address and self.email_address != email:
             self.app.email_queue.put( self
                                     , 'verification-notice'
-
-                                    # Don't count this one against their sending quota.
-                                    # It's going to their own verified address, anyway.
-                                    , _user_initiated=False
-
                                     , **kwargs
                                      )
 
@@ -106,6 +103,16 @@ def validate_email_verification_request(self, c, email, *packages):
         if not all(email in p.emails for p in packages):
             raise EmailNotOnFile()
 
+        unverified_email = c.one("""
+            SELECT address
+              FROM emails
+             WHERE participant_id=%(participant_id)s
+               AND not (verified IS true)
+        """, dict(participant_id=self.id))
+
+        if unverified_email and unverified_email != email:
+            raise TooManyVerifications()
+
         owner_id = c.one("""
             SELECT participant_id
               FROM emails

gratipay/models/team/review_status.py

@@ -56,5 +56,4 @@ def _set_is_approved(self, cursor, is_approved, recorder):
                                     , team_name=self.name
                                     , review_url=self.review_url
                                     , include_unsubscribe=False
-                                    , _user_initiated=False
                                      )

gratipay/project_review_process.py

@@ -63,7 +63,6 @@ def start(self, *teams):
                             , 'project-review'
                             , review_url=team.review_url
                             , include_unsubscribe=False
-                            , _user_initiated=False
                              )
 
         return review_url

sql/branch.sql

@@ -0,0 +1 @@
+ALTER TABLE email_queue DROP COLUMN user_initiated;

tests/py/test_email.py

@@ -5,7 +5,6 @@
 import mock
 from pytest import raises
 
-from gratipay.exceptions import Throttled
 from gratipay.testing import Harness
 from gratipay.testing.email import SentEmailHarness
 
@@ -14,36 +13,24 @@ class TestPut(SentEmailHarness):
 
     def setUp(self):
         SentEmailHarness.setUp(self)
-        self.alice = self.make_participant('alice', claimed_time='now', email_address='[email protected]')
+        self.alice = self.make_participant( 'alice'
+                                          , claimed_time='now'
+                                          , email_address='[email protected]'
+                                           )
 
-    def test_queueing_email_is_throttled(self):
+    def test_put_puts(self):
+        assert self.app.email_queue.flush() == 0
         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")
+        assert self.app.email_queue.flush() == 3
 
     def test_queueing_email_writes_timestamp(self):
         self.app.email_queue.put(self.alice, "base")
 
         ctime = self.db.one("SELECT EXTRACT(epoch FROM ctime) FROM email_queue")
         assert abs(ctime - time.time()) < 300
 
-    def test_only_user_initiated_messages_count_towards_throttling(self):
-        self.app.email_queue.put(self.alice, "base")
-        self.app.email_queue.put(self.alice, "base", _user_initiated=False)
-        self.app.email_queue.put(self.alice, "base")
-        self.app.email_queue.put(self.alice, "base", _user_initiated=False)
-        self.app.email_queue.put(self.alice, "base")
-        self.app.email_queue.put(self.alice, "base", _user_initiated=False)
-        raises(Throttled, self.app.email_queue.put, self.alice, "branch")
-
-    def test_flushing_queue_resets_throttling(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")
-        assert self.app.email_queue.flush() == 3
-        self.app.email_queue.put(self.alice, "base")
-
 
 class TestFlush(SentEmailHarness):
 

tests/py/test_packages.py

@@ -169,14 +169,14 @@ def test_ordering(self):
         nonce = alice.get_email('primary').nonce
         alice.finish_email_verification('primary', nonce)
 
-        # linked to self but unverified
-        alice.start_email_verification('lumail')
-
         # verified for self
         alice.start_email_verification('amail')
         nonce = alice.get_email('amail').nonce
         alice.finish_email_verification('amail', nonce)
 
+        # linked to self but unverified
+        alice.start_email_verification('lumail')
+
         # unlinked
         pass  # cmail, email -- tests alphabetization within grouping
 

tests/py/test_participant_emails.py

@@ -10,7 +10,7 @@
 from pytest import raises
 
 from gratipay.exceptions import CannotRemovePrimaryEmail, EmailTaken, EmailNotVerified
-from gratipay.exceptions import TooManyEmailAddresses, Throttled, EmailAlreadyVerified
+from gratipay.exceptions import TooManyEmailAddresses, TooManyVerifications, EmailAlreadyVerified
 from gratipay.exceptions import EmailNotOnFile, ProblemChangingEmail
 from gratipay.testing import P, Harness
 from gratipay.testing.email import QueuedEmailHarness
@@ -57,7 +57,7 @@ def hit_email_spt(self, action, address, user='alice', package_ids=[], should_fa
                     , auth_as=user
                     , HTTP_ACCEPT_LANGUAGE=b'en'
                      )
-        if issubclass(response.__class__, (ProblemChangingEmail, Throttled)):
+        if issubclass(response.__class__, (ProblemChangingEmail, TooManyVerifications)):
             response.render_body({'_': lambda a: a})
         return response
 
@@ -109,6 +109,24 @@ def test_verifying_second_email_sends_verification_notice(self):
         expected = "We are connecting [email protected] to the alice account on Gratipay"
         assert expected in last_email['body_text']
 
+    def test_can_only_have_one_verification_outstanding_at_a_time(self):
+        response = self.hit_email_spt('add-email', '[email protected]')
+        assert response.code == 200
+        response = self.hit_email_spt('add-email', '[email protected]', should_fail=True)
+        assert response.code == 400
+        assert response.__class__ is TooManyVerifications
+        assert response.body == 'You have an outstanding verification request that must be ' \
+                                'completed before you can verify a different email address.'
+
+    def test_can_resend_verification_message_for_same_email(self):
+        response = self.hit_email_spt('add-email', '[email protected]')
+        assert response.code == 200
+        response = self.hit_email_spt('add-email', '[email protected]', should_fail=True)
+        assert response.code == 400
+        assert response.__class__ is TooManyVerifications
+        assert response.body == 'You have an outstanding verification request that must be ' \
+                                'completed before you can verify a different email address.'
+
     def test_post_anon_returns_401(self):
         response = self.hit_email_spt('add-email', '[email protected]', user=None, should_fail=True)
         assert response.code == 401
@@ -129,14 +147,6 @@ def test_post_with_looooong_address_is_400(self):
         response = self.hit_email_spt('add-email', ('a'*243) + '@example.com', should_fail=True)
         assert response.code == 400
 
-    def test_post_too_quickly_is_400(self):
-        self.hit_email_spt('add-email', '[email protected]')
-        self.hit_email_spt('add-email', '[email protected]')
-        self.hit_email_spt('add-email', '[email protected]')
-        response = self.hit_email_spt('add-email', '[email protected]', should_fail=True)
-        assert response.code == 400
-        assert 'too quickly' in response.body
-
     def test_verify_email_without_adding_email(self):
         response = self.hit_verify_spt('', 'sample-nonce')
         assert 'Bad Info' in response.body

tests/py/test_take_over.py

@@ -184,41 +184,37 @@ def test_idempotent(self):
         self.db.self_check()
 
     def test_email_addresses_merging(self):
-        # person starts a bunch of email verifications as alice
+        # person verifies an email as alice
         alice = self.make_participant('alice')
         alice.start_email_verification('[email protected]')
-        alice.start_email_verification('alice@example.net')
-        alice.start_email_verification('alice@example.org')
+        nonce = alice.get_email('alice@example.com').nonce
+        alice.finish_email_verification('alice@example.com', nonce)
 
-        # person finishes one of them, as alice
-        nonce = alice.get_email('[email protected]').nonce
-        alice.finish_email_verification('[email protected]', nonce)
+        # person starts another verification as alice
+        alice.start_email_verification('[email protected]')
 
         # person signs up as bob
         bob_github = self.make_elsewhere('github', 2, 'bob')
         bob = bob_github.opt_in('bob')[0].participant
 
-        # person verifies one of the same emails as alice, using bob
-        bob.start_email_verification('[email protected]')
-        nonce = bob.get_email('[email protected]').nonce
-        bob.finish_email_verification('[email protected]', nonce)
-
-        # ... starts verification for another of the same
-        bob.start_email_verification('[email protected]')
-
-        # ... and for a new email
+        # verifies an email
         bob.start_email_verification('[email protected]')
+        nonce = bob.get_email('[email protected]').nonce
+        bob.finish_email_verification('[email protected]', nonce)
+
+        # and starts verification for alice's unverified email
+        bob.start_email_verification('[email protected]')
 
         # Now: alice takes over bob!
         alice.take_over(bob_github, have_confirmation=True)
 
         # alice gets all the addresses, verified or not
         alice_emails = {e.address: e for e in alice.get_emails()}
-        assert len(alice_emails) == 4
+        assert len(alice_emails) == 3
         assert alice_emails['[email protected]'].verified
-        assert alice_emails['alice@example.org'].verified
-        assert not alice_emails['alice@example.net'].verified
-        assert not alice_emails['bob@example.net'].verified
+        assert alice_emails['bob@example.net'].verified
+        assert not alice_emails['alice@example.org'].verified
+        assert alice.email_address == 'alice@example.com'
 
         # bob has no email addresses
         assert not Participant.from_id(bob.id).get_emails()

www/~/%username/identities/%country.spt

@@ -44,7 +44,6 @@ if identity is not None and participant != user.participant:
                                , country_name=country_name
                                , country_code=country.code
                                , include_unsubscribe=False
-                               , _user_initiated=False
                                 )
 
 # handle POST requests

www/~/%username/payout-status.spt

@@ -50,7 +50,6 @@ if template:
     website.app.email_queue.put( participant
                                , template
                                , include_unsubscribe=False
-                               , _user_initiated=False
                                 )
 
 out = {"status": new_status}

www/~/%username/toggle-is-suspicious.json.spt

@@ -46,7 +46,6 @@ with website.db.get_cursor() as c:
         website.app.email_queue.put( participant
                                    , 'suspicious-'+status
                                    , include_unsubscribe=False
-                                   , _user_initiated=False
                                     )
 
 [---] application/json via json_dump