Finish test suite for package linking via email

gratipay/models/participant/email.py

@@ -223,23 +223,23 @@ def _set_primary_email(self, email, cursor):
 
 
     def finish_email_verification(self, email, nonce):
-        if '' in (email, nonce):
+        if '' in (email.strip(), nonce.strip()):
             return VERIFICATION_MISSING
-        with self.db.get_cursor() as c:
-            r = self.get_email(email, c)
-            if r is None:
+        with self.db.get_cursor() as cursor:
+            record = self.get_email(email, cursor)
+            if record is None:
                 return VERIFICATION_FAILED
-            packages = self.get_packages_claiming(c, nonce)
-            if r.verified and not packages:
-                assert r.nonce is None  # and therefore, order of conditions matters
+            packages = self.get_packages_claiming(cursor, nonce)
+            if record.verified and not packages:
+                assert record.nonce is None  # and therefore, order of conditions matters
                 return VERIFICATION_REDUNDANT
-            if not constant_time_compare(r.nonce, nonce):
+            if not constant_time_compare(record.nonce, nonce):
                 return VERIFICATION_FAILED
-            if (utcnow() - r.verification_start) > EMAIL_HASH_TIMEOUT:
+            if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT:
                 return VERIFICATION_EXPIRED
             try:
-                self.save_email_address(c, email)
-                self.finish_package_claims(c, *packages)
+                self.finish_package_claims(cursor, nonce, *packages)
+                self.save_email_address(cursor, email)
             except IntegrityError:
                 return VERIFICATION_STYMIED
             return VERIFICATION_SUCCEEDED
@@ -272,9 +272,10 @@ def save_email_address(self, cursor, address):
             self.set_primary_email(address, cursor)
 
 
-    def finish_package_claims(self, cursor, *packages):
+    def finish_package_claims(self, cursor, nonce, *packages):
         """Create teams if needed and associate them with the packages.
         """
+        cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,))
         package_ids = []
         for package in packages:
             package.get_or_create_linked_team(cursor, self)

tests/py/test_email.py

@@ -2,15 +2,16 @@
 
 import json
 import sys
-
 import urllib
+
 from pytest import raises
 
 from gratipay.exceptions import CannotRemovePrimaryEmail, EmailTaken, EmailNotVerified
 from gratipay.exceptions import TooManyEmailAddresses, Throttled, EmailAlreadyVerified
 from gratipay.exceptions import EmailNotOnFile, ProblemChangingEmail
 from gratipay.testing import P, Harness
 from gratipay.testing.email import QueuedEmailHarness, SentEmailHarness
+from gratipay.models.package import Package
 from gratipay.models.participant import email as _email
 from gratipay.utils import encode_for_querystring
 from gratipay.cli import queue_branch_email as _queue_branch_email
@@ -167,10 +168,18 @@ def test_verify_email_expired_nonce(self):
     def test_finish_email_verification(self):
         self.hit_email_spt('add-email', '[email protected]')
         nonce = self.alice.get_email('[email protected]').nonce
-        self.finish_email_verification('[email protected]', nonce)
-        expected = '[email protected]'
-        actual = P('alice').email_address
-        assert expected == actual
+        assert self.finish_email_verification('[email protected]', nonce).code == 200
+        assert P('alice').email_address == '[email protected]'
+
+    def test_empty_email_results_in_missing(self):
+        for empty in ('', '    '):
+            result = self.alice.finish_email_verification(empty, 'foobar')
+            assert result == _email.VERIFICATION_MISSING
+
+    def test_empty_nonce_results_in_missing(self):
+        for empty in ('', '    '):
+            result = self.alice.finish_email_verification('foobar', empty)
+            assert result == _email.VERIFICATION_MISSING
 
     def test_email_verification_is_backwards_compatible(self):
         """Test email verification still works with unencoded email in verification link.
@@ -679,36 +688,59 @@ def test_sends_notice_for_unverified_address_and_multiple_packages(self):
         assert ' connecting [email protected] and 2 npm packages ' in text
 
 
-class FinishEmailVerification(VerificationBase):
+class PackageLinking(VerificationBase):
+
+    address = '[email protected]'
 
     def start(self, address, *package_names):
         packages = [self.make_package(name=name, emails=[address]) for name in package_names]
         self.alice.start_email_verification(address, *packages)
         return self.alice.get_email(address).nonce
 
-    def test_handles_new_address(self):
-        address = '[email protected]'
+    def check(self, *package_names):
+        nonce = self.start(self.address, *package_names)
+        retval = self.alice.finish_email_verification(self.address, nonce)
+        assert retval == _email.VERIFICATION_SUCCEEDED
+        assert self.alice.email_address == P('alice').email_address == self.address
+        for name in package_names:
+            package = Package.from_names('npm', name)
+            assert package.team.package == package
+
+
+    def test_preverify_preverifies(self):
         assert self.alice.email_address is None
-        self.alice.finish_email_verification(address, self.start(address))
-        assert self.alice.email_address == P('alice').email_address == address
+        self.preverify()
+        assert self.alice.email_address == self.address
 
-    def test_handles_verified_address_and_no_packages(self):
-        raise NotImplementedError  # should error
 
-    def test_handles_verified_address_and_one_package(self):
+    def test_unverified_address_and_no_packages_succeeds(self):
+        self.check()
+
+    def test_unverified_address_and_one_package_succeeds(self):
+        self.check('foo')
+
+    def test_unverified_address_and_multiple_packages_succeeds(self):
+        self.check('foo', 'bar')
+
+    def test_verified_address_and_no_packages_is_a_no_go(self):
         self.preverify()
-        address = '[email protected]'
-        assert self.alice.email_address == address
-        self.alice.finish_email_verification(address, self.start(address, 'foo'))
-        assert self.alice.email_address == P('alice').email_address == address
-        raise NotImplementedError  # assert we connect the package
+        raises(EmailAlreadyVerified, self.check)
 
-    def test_handles_verified_address_and_multiple_packages(self):
+    def test_verified_address_and_one_package_succeeds(self):
         self.preverify()
-        raise NotImplementedError
+        self.check('foo')
 
-    def test_handles_unverified_address_and_one_package(self):
-        raise NotImplementedError
+    def test_verified_address_and_multiple_packages_succeeds(self):
+        self.preverify()
+        self.check('foo', 'bar')
 
-    def test_handles_unverified_address_and_multiple_packages(self):
-        raise NotImplementedError
+
+    def test_while_we_are_at_it_that_packages_have_unique_teams_that_survive_comparison(self):
+        self.test_verified_address_and_multiple_packages_succeeds()
+
+        foo = Package.from_names('npm', 'foo')
+        bar = Package.from_names('npm', 'bar')
+
+        assert foo.team == foo.team
+        assert bar.team == bar.team
+        assert foo.team != bar.team