Extend verification template for package claiming

emails/verification-notice.spt

@@ -1,13 +1,72 @@
-{{ _("Connecting {0} to {1} on Gratipay.", new_email, username) }}
+{{ _("New activity on your account") }}
 
 [---] text/html
-{{ _("We are connecting {0} to the {1} account on Gratipay. This is a notification "
-     "sent to {2} because that is the primary email address we have on file.",
-     ('<b>%s</b>'|safe) % new_email,
-     ('<b><a href="https://gratipay.com/{0}">{0}</a></b>'|safe).format(username),
-     ('<b>%s</b>'|safe) % email) }}
-
+{% if new_email_verified %}
+{{ ngettext( "We are connecting the {package_name} npm package to the {username} account on "
+             "Gratipay. This is a notification sent to {email_address} because that is the "
+             "primary email address we have on file."
+           , "We are connecting {n} npm packages to the {username} account on Gratipay. This is "
+             "a notification sent to {email_address} because that is the primary email address "
+             "we have on file."
+           , n=npackages
+           , package_name=('<b>{}</b>'|safe).format(package_name)
+           , username=('<b><a href="https://gratipay.com/{0}/">{0}</a></b>'|safe).format(username)
+           , email_address=('<b>{}</b>'|safe).format(email)
+            ) }}
+{% elif npackages > 0 %}
+{{ ngettext( "We are connecting {email_address} and the {package_name} npm package to the "
+             "{username} account on Gratipay. This is a notification sent to {email_address_2} "
+             "because that is the primary email address we have on file."
+           , "We are connecting {email_address} and {n} npm packages to the {username} account on "
+             "Gratipay. This is a notification sent to {email_address_2} because that is the "
+             "primary email address we have on file."
+           , n=npackages
+           , package_name=('<b>{}</b>'|safe).format(package_name)
+           , username=('<b><a href="https://gratipay.com/{0}/">{0}</a></b>'|safe).format(username)
+           , email_address=('<b>{}</b>'|safe).format(new_email)
+           , email_address_2=('<b>{}</b>'|safe).format(email)
+            ) }}
+{% else %}
+{{ _( "We are connecting {email_address} to the {username} account on Gratipay. This is a "
+      "notification sent to {email_address_2} because that is the primary email address we have "
+      "on file."
+    , username=('<b><a href="https://gratipay.com/{0}/">{0}</a></b>'|safe).format(username)
+    , email_address=('<b>{}</b>'|safe).format(new_email)
+    , email_address_2=('<b>{}</b>'|safe).format(email)
+     ) }}
+ {% endif %}
 [---] text/plain
-{{ _("We are connecting {0} to the {1} account on Gratipay. This is a notification "
-     "sent to {2} because that is the primary email address we have on file.",
-     new_email, username, email) }}
+{% if new_email_verified %}
+{{ ngettext( "We are connecting the {package_name} npm package to the {username} account on "
+             "Gratipay. This is a notification sent to {email_address} because that is the "
+             "primary email address we have on file."
+           , "We are connecting {n} npm packages to the {username} account on Gratipay. This is "
+             "a notification sent to {email_address} because that is the primary email address "
+             "we have on file."
+           , n=npackages
+           , package_name=package_name
+           , username=username
+           , email_address=email
+            ) }}
+{% elif npackages > 0 %}
+{{ ngettext( "We are connecting {email_address} and the {package_name} npm package to the "
+             "{username} account on Gratipay. This is a notification sent to {email_address_2} "
+             "because that is the primary email address we have on file."
+           , "We are connecting {email_address} and {n} npm packages to the {username} account on "
+             "Gratipay. This is a notification sent to {email_address_2} because that is the "
+             "primary email address we have on file."
+           , n=npackages
+           , package_name=package_name
+           , username=username
+           , email_address=new_email
+           , email_address_2=email
+            ) }}
+{% else %}
+{{ _( "We are connecting {email_address} to the {username} account on Gratipay. This is a "
+      "notification sent to {email_address_2} because that is the primary email address we have "
+      "on file."
+    , username=username
+    , email_address=new_email
+    , email_address_2=email
+     ) }}
+ {% endif %}

emails/verification.spt

@@ -1,16 +1,63 @@
 {{ _("Connect to {0} on Gratipay?", username) }}
 
 [---] text/html
-{{ _("We've received a request to connect {0} to the {1} account on Gratipay. Sound familiar?",
-     ('<b>%s</b>'|safe) % email,
-     ('<b><a href="https://gratipay.com/~{0}/">{0}</a></b>'|safe).format(username)) }}
+{% if new_email_verified %}
+{{ ngettext( "We've received a request to connect the {package_name} npm package to the "
+             "{username} account on Gratipay. Sound familiar?"
+           , "We've received a request to connect {n} npm packages to the {username} account "
+             "on Gratipay. Sound familiar?"
+           , n=npackages
+           , package_name=('<b>{}</b>'|safe).format(package_name)
+           , username=('<b><a href="https://gratipay.com/~{0}/">{0}</a></b>'|safe).format(username)
+            ) }}
+{% elif npackages > 0 %}
+{{ ngettext( "We've received a request to connect {email_address} and the {package_name} npm "
+             "package to the {username} account on Gratipay. Sound familiar?"
+           , "We've received a request to connect {email_address} and {n} npm packages to the "
+             "{username} account on Gratipay. Sound familiar?"
+           , n=npackages
+           , package_name=('<b>{}</b>'|safe).format(package_name)
+           , email_address=('<b>{}</b>'|safe).format(new_email)
+           , username=('<b><a href="https://gratipay.com/~{0}/">{0}</a></b>'|safe).format(username)
+            ) }}
+{% else %}
+{{ _( "We've received a request to connect {email_address} to the {username} account on Gratipay. "
+      "Sound familiar?"
+    , email_address=('<b>{}</b>'|safe).format(new_email)
+    , username=('<b><a href="https://gratipay.com/~{0}/">{0}</a></b>'|safe).format(username)
+     ) }}
+{% endif %}
 <br>
 <br>
 <a href="{{ link }}" style="{{ button_style }}">{{ _("Yes, proceed!") }}</a>
 
 [---] text/plain
-{{ _("We've received a request to connect {0} to the {1} account on Gratipay. Sound familiar?",
-     email, username) }}
+{% if new_email_verified %}
+{{ ngettext( "We've received a request to connect the {package_name} npm package to the "
+             "{username} account on Gratipay. Sound familiar?"
+           , "We've received a request to connect {n} npm packages to the {username} account "
+             "on Gratipay. Sound familiar?"
+           , n=npackages
+           , package_name=package_name
+           , username=username
+            ) }}
+{% elif npackages > 0 %}
+{{ ngettext( "We've received a request to connect {email_address} and the {package_name} npm "
+             "package to the {username} account on Gratipay. Sound familiar?"
+           , "We've received a request to connect {email_address} and {n} npm packages to the "
+             "{username} account on Gratipay. Sound familiar?"
+           , n=npackages
+           , package_name=package_name
+           , email_address=new_email
+           , username=username
+            ) }}
+{% else %}
+{{ _( "We've received a request to connect {email_address} to the {username} account on Gratipay. "
+      "Sound familiar?"
+    , email_address=new_email
+    , username=username
+     ) }}
+{% endif %}
 
 {{ _("Follow this link to finish connecting your email:") }}
 

gratipay/models/participant/email.py

@@ -65,21 +65,24 @@ def start_email_verification(self, email, *packages):
             self.validate_email_verification_request(c, email, *packages)
             link = self.get_email_verification_link(c, email, *packages)
 
-        self.app.email_queue.put( self
-                                , 'verification'
-                                , email=email
-                                , link=link
-                                , include_unsubscribe=False
-                                 )
-        if self.email_address:
+        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
+                      )
+        self.app.email_queue.put(self, 'verification', email=email, **kwargs)
+        if self.email_address and self.email_address != email:
             self.app.email_queue.put( self
                                     , 'verification-notice'
-                                    , new_email=email
-                                    , include_unsubscribe=False
 
                                     # Don't count this one against their sending quota.
                                     # It's going to their own verified address, anyway.
                                     , _user_initiated=False
+
+                                    , **kwargs
                                      )
 
 

tests/py/test_email.py

@@ -307,51 +307,55 @@ def test_html_escaping(self):
         assert 'foo'bar' in last_email['body_html']
         assert ''' not in last_email['body_text']
 
+    def test_npm_package_name_is_handled_safely(self):
+        foo = self.make_package(name='<script>')
+        self.alice.start_email_verification("[email protected]", foo)
+        last_email = self.get_last_email()
+        assert '<b>&lt;script&gt;</b>' in last_email['body_html']
+        assert '<script>' in last_email['body_text']
+
     def test_queueing_email_is_throttled(self):
-        self.app.email_queue.put(self.alice, "verification")
-        self.app.email_queue.put(self.alice, "branch")
-        self.app.email_queue.put(self.alice, "verification-notice")
-        raises(Throttled, self.app.email_queue.put, self.alice, "branch")
+        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_only_user_initiated_messages_count_towards_throttling(self):
-        self.app.email_queue.put(self.alice, "verification")
-        self.app.email_queue.put(self.alice, "verification", _user_initiated=False)
-        self.app.email_queue.put(self.alice, "branch")
-        self.app.email_queue.put(self.alice, "branch", _user_initiated=False)
-        self.app.email_queue.put(self.alice, "verification-notice")
-        self.app.email_queue.put(self.alice, "verification-notice", _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)
+        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.add(self.alice, '[email protected]')
         assert self.app.email_queue.flush() == 1
-        self.app.email_queue.put(self.alice, "verification")
-        self.app.email_queue.put(self.alice, "branch")
-        self.app.email_queue.put(self.alice, "verification-notice")
+        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, "verification-notice")
+        self.app.email_queue.put(self.alice, "base")
 
 
 class FlushEmailQueue(SentEmailHarness):
 
     def test_can_flush_an_email_from_the_queue(self):
         larry = self.make_participant('larry', email_address='[email protected]')
-        self.app.email_queue.put(larry, "verification")
-
-        assert self.db.one("SELECT spt_name FROM email_queue") == "verification"
+        self.app.email_queue.put(larry, "base")
+        assert self.db.one("SELECT spt_name FROM email_queue") == "base"
         self.app.email_queue.flush()
         assert self.count_email_messages() == 1
         last_email = self.get_last_email()
         assert last_email['to'] == 'larry <[email protected]>'
-        expected = "connect larry"
+        expected = "Something not right?"
         assert expected in last_email['body_text']
         assert self.db.one("SELECT spt_name FROM email_queue") is None
 
     def test_flushing_an_email_without_address_just_skips_it(self):
-        larry = self.make_participant('larry')
-        self.app.email_queue.put(larry, "verification")
-
-        assert self.db.one("SELECT spt_name FROM email_queue") == "verification"
+        self.app.email_queue.put(self.make_participant('larry'), "base")
+        assert self.db.one("SELECT spt_name FROM email_queue") == "base"
         self.app.email_queue.flush()
         assert self.count_email_messages() == 0
         assert self.db.one("SELECT spt_name FROM email_queue") is None
@@ -483,8 +487,7 @@ def test_can_claim_package_even_when_address_already_verified(self):
         self.add(self.alice, '[email protected]')
         foo = self.make_package()
         self.alice.start_email_verification('[email protected]', foo)
-        assert self.get_last_email()['subject'] == \
-                                               'Connecting [email protected] to alice on Gratipay.'
+        assert self.get_last_email()['subject'] == 'Connect to alice on Gratipay?'
         assert self.db.one('select package_id from claims') == foo.id
 
     def test_claiming_package_with_verified_address_doesnt_count_against_max(self):
@@ -576,3 +579,101 @@ def test_adds_events(self):
             self.make_participant('alice').get_email_verification_link(c, '[email protected]', foo)
         events = [e.payload['action'] for e in self.db.all('select * from events order by id')]
         assert events == ['add', 'start-claim']
+
+
+class VerificationBase(Alice):
+
+    def check(self, *package_names, **kw):
+        packages = [self.make_package(name=n) for n in package_names]
+        self.alice.start_email_verification('[email protected]', *packages)
+        message = self.get_last_email()
+        return message['subject'], message['body_html'], message['body_text']
+
+    def preverify(self, address='[email protected]'):
+        self.alice.start_email_verification(address)
+        nonce = self.alice.get_email(address).nonce
+        self.alice.verify_email(address, nonce)
+
+
+class VerificationMessage(VerificationBase):
+
+    def check(self, *a, **kw):
+        subject, html, text = VerificationBase.check(self, *a, **kw)
+        assert subject == 'Connect to alice on Gratipay?'
+        return html, text
+
+    def test_chokes_on_just_verified_address(self):
+        self.preverify()
+        raises(EmailAlreadyVerified, self.check)
+
+    def test_handles_just_address(self):
+        html, text = self.check()
+        assert ' connect <b>[email protected]</b> to ' in html
+        assert ' connect [email protected] to ' in text
+
+    # NB: The next two also exercise skipping the verification notice when
+    # sending package verification to an already-verified address, since the
+    # last email sent would be the verification notice if we didn't skip it.
+
+    def test_handles_verified_address_and_one_package(self):
+        self.preverify()
+        html, text = self.check('foo')
+        assert ' connect the <b>foo</b> npm package ' in html
+        assert ' connect the foo npm package ' in text
+
+    def test_handles_verified_address_and_multiple_packages(self):
+        self.preverify()
+        html, text = self.check('foo', 'bar')
+        assert ' connect 2 npm packages ' in html
+        assert ' connect 2 npm packages ' in text
+
+    def test_handles_unverified_address_and_one_package(self):
+        html, text = self.check('foo')
+        assert ' <b>[email protected]</b> and the <b>foo</b> npm package ' in html
+        assert ' [email protected] and the foo npm package ' in text
+
+    def test_handles_unverified_address_and_multiple_packages(self):
+        html, text = self.check('foo', 'bar')
+        assert ' <b>[email protected]</b> and 2 npm packages ' in html
+        assert ' [email protected] and 2 npm packages ' in text
+
+
+class VerificationNotice(VerificationBase):
+
+    def setUp(self):
+        VerificationBase.setUp(self)
+        self.preverify('[email protected]')
+
+    def check(self, *a, **kw):
+        subject, html, text = VerificationBase.check(self, *a, **kw)
+        assert subject == 'New activity on your account'
+        assert ' notification sent to <b>[email protected]</b> because' in html
+        assert ' notification sent to [email protected] because' in text
+        return html, text
+
+    def test_sends_notice_for_new_address(self):
+        html, text = self.check()
+        assert ' connecting <b>[email protected]</b> to ' in html
+        assert ' connecting [email protected] to ' in text
+
+    def test_sends_notice_for_verified_address_and_one_package(self):
+        self.preverify()
+        html, text = self.check('foo')
+        assert ' connecting the <b>foo</b> npm package ' in html
+        assert ' connecting the foo npm package to ' in text
+
+    def test_sends_notice_for_verified_address_and_multiple_packages(self):
+        self.preverify()
+        html, text = self.check('foo', 'bar')
+        assert ' connecting 2 npm packages ' in html
+        assert ' connecting 2 npm packages ' in text
+
+    def test_sends_notice_for_unverified_address_and_one_package(self):
+        html, text = self.check('foo')
+        assert ' connecting <b>[email protected]</b> and the <b>foo</b> npm package ' in html
+        assert ' connecting [email protected] and the foo npm package ' in text
+
+    def test_sends_notice_for_unverified_address_and_multiple_packages(self):
+        html, text = self.check('foo', 'bar')
+        assert ' connecting <b>[email protected]</b> and 2 npm packages ' in html
+        assert ' connecting [email protected] and 2 npm packages ' in text