From dad87d628b3b1f5ff060759dad7eef005c7fa151 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 23 Aug 2017 12:28:10 -0400 Subject: [PATCH] Clean up email listing --- gratipay/models/participant/email.py | 11 +++ gratipay/testing/__init__.py | 4 +- gratipay/testing/browser.py | 18 +++-- gratipay/testing/email.py | 12 +++- js/gratipay/emails.js | 53 +++++++++----- scss/components/emails.scss | 33 --------- scss/pages/emails.scss | 40 +++++++++++ tests/ttw/test_email_verification.py | 104 +++++++++++++++++++++++++++ www/assets/gratipay.css.spt | 2 +- www/~/%username/emails/index.spt | 84 ++++++++++++++++------ 10 files changed, 278 insertions(+), 83 deletions(-) delete mode 100644 scss/components/emails.scss create mode 100644 scss/pages/emails.scss create mode 100644 tests/ttw/test_email_verification.py diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index c5da38de8a..029fe337ee 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -54,6 +54,17 @@ class Email(object): """ + @property + def has_pending_email_address_verifications(self): + """A boolean indicating whether there are email address verifications + outstanding for this participant. Makes a db call. + """ + for email in self.get_emails(): + if not email.verified: + return True + return False + + def start_email_verification(self, email, *packages): """Add an email address for a participant. diff --git a/gratipay/testing/__init__.py b/gratipay/testing/__init__.py index 010a00ec85..40c0874125 100644 --- a/gratipay/testing/__init__.py +++ b/gratipay/testing/__init__.py @@ -15,8 +15,10 @@ from .billing import BillingHarness from .browser import BrowserHarness from .deploy_hooks import DeployHooksHarness +from .email import SentEmailHarness, QueuedEmailHarness -__all__ = ['Harness', 'BillingHarness', 'BrowserHarness', 'DeployHooksHarness', 'D','P','T'] +__all__ = [ 'Harness', 'BillingHarness', 'BrowserHarness', 'DeployHooksHarness', 'SentEmailHarness' + , 'QueuedEmailHarness', 'D','P','T' ] class Foobar(Exception): pass diff --git a/gratipay/testing/browser.py b/gratipay/testing/browser.py index fbcab08803..9d9441590c 100644 --- a/gratipay/testing/browser.py +++ b/gratipay/testing/browser.py @@ -7,6 +7,7 @@ import time from splinter.browser import _DRIVERS +from selenium.common.exceptions import StaleElementReferenceException from gratipay.security import user @@ -55,9 +56,10 @@ def tearDown(self): self.cookies.delete() def visit(self, url): - """Extend to prefix our base URL. + """Extend to prefix our base URL if necessary. """ - return self._browser.visit(self.base_url + url) + base_url = '' if url.startswith('http') else self.base_url + return self._browser.visit(base_url + url) def sign_in(self, username): """Given a username, sign in the user. @@ -71,10 +73,10 @@ def sign_in(self, username): P(username).update_session(token, expires) self.cookies.add({user.SESSION: token}) - def css(self, selector): + def css(self, selector, element=None): """Shortcut for find_by_css. """ - return self.find_by_css(selector) + return (element or self).find_by_css(selector) def js(self, code): """Shortcut for evaluate_script. @@ -109,8 +111,12 @@ def wait_for(self, selector, timeout=2): while time.time() < end_time: if self.has_element(selector): element = self.find_by_css(selector) - if element.visible: - return element + try: + if element.visible: + return element + except StaleElementReferenceException: + # May need to wait across page reload. + pass raise NeverShowedUp(selector) def wait_for_notification(self, type='notice', message=None, timeout=2): diff --git a/gratipay/testing/email.py b/gratipay/testing/email.py index e2919df5d2..767e66bb13 100644 --- a/gratipay/testing/email.py +++ b/gratipay/testing/email.py @@ -38,13 +38,23 @@ class QueuedEmailHarness(_AbstractEmailHarness): """An email harness that pulls from the ``email_messages`` table. """ + _SELECT = 'SELECT * FROM email_messages WHERE result is null ORDER BY ctime DESC LIMIT 1' + def _get_last_email(self): - rec = self.db.one('SELECT * FROM email_messages ORDER BY ctime DESC LIMIT 1') + rec = self.db.one(self._SELECT) return self.app.email_queue._prepare_email_message_for_ses(rec) def count_email_messages(self): return self.db.one('SELECT count(*) FROM email_messages WHERE result is null') + def pop_email_message(self): + """Same as get_last_email but also marks as sent in the db. + """ + out = self.get_last_email() + rec = self.db.one(self._SELECT) + self.app.email_queue._store_result(rec.id, '', 'deadbeef') + return out + class SentEmailHarness(_AbstractEmailHarness): """An email harness that patches ``_mailer.send_email`` to ``get_last_email`` diff --git a/js/gratipay/emails.js b/js/gratipay/emails.js index 99d617ce3a..ae759e1eed 100644 --- a/js/gratipay/emails.js +++ b/js/gratipay/emails.js @@ -1,11 +1,11 @@ Gratipay.emails = {}; -Gratipay.emails.post = function(e) { - e.preventDefault(); +Gratipay.emails.post = function() { var $this = $(this); var action = this.className; var $inputs = $('.emails button, .emails input'); - var address = $this.parent().data('email') || $('input.add-email').val(); + var $row = $this.closest('tr'); + var address = $row.data('email') || $('.add input').val(); $inputs.prop('disabled', true); @@ -15,19 +15,16 @@ Gratipay.emails.post = function(e) { data: {action: action, address: address}, dataType: 'json', success: function (msg) { - if (msg) { - Gratipay.notification(msg, 'success'); - } - if (action == 'add-email') { - $('input.add-email').val(''); - setTimeout(function(){ window.location.reload(); }, 3000); - return; - } else if (action == 'set-primary') { - $('.emails li').removeClass('primary'); - $this.parent().addClass('primary'); - } else if (action == 'remove') { - $this.parent().fadeOut(); - } + switch(action) { + case 'resend': + Gratipay.notification(msg, 'success'); + break; + case 'remove': + $row.slideUp(function() { $row.remove() }); + break; + default: + window.location.reload(); + }; $inputs.prop('disabled', false); }, error: [ @@ -37,15 +34,33 @@ Gratipay.emails.post = function(e) { }); }; +Gratipay.emails.showAddForm = function() { + $('.add-form-knob').hide(); + $('.add form').show(); +} + +Gratipay.emails.hideAddForm = function() { + $('.add form').hide(); + $('.add-form-knob').show(); +} + +Gratipay.emails.handleAddForm = function(e) { + e.preventDefault(); + if (e.type === 'submit') + Gratipay.emails.post.call(this); + else + Gratipay.emails.hideAddForm(); +} -Gratipay.emails.init = function () { +Gratipay.emails.init = function() { // Wire up email addresses list. // ============================= $('.emails button, .emails input').prop('disabled', false); - $('.emails button[class]').on('click', Gratipay.emails.post); - $('form.add-email').on('submit', Gratipay.emails.post); + $('.emails tr.existing button').click(Gratipay.emails.post); + $('button.add').click(Gratipay.emails.showAddForm); + $('.add form').on('submit reset', Gratipay.emails.handleAddForm); // Wire up notification preferences diff --git a/scss/components/emails.scss b/scss/components/emails.scss deleted file mode 100644 index 33efbc1ea5..0000000000 --- a/scss/components/emails.scss +++ /dev/null @@ -1,33 +0,0 @@ -.emails { - .label-primary, .set-primary { display: none; } - .verified { - .label-unverified, .resend { display: none; } - .set-primary { display: inline-block; } - } - .primary { - .set-primary, .remove { display: none; } - .label-primary { display: inline; } - } - li { - list-style: square inside; - margin: 0 0 0.5em 0.5em; - clear: both; - } - .label-primary, .label-unverified { - font-size: 90%; - margin-left: 0.5em; - } - .label-primary { - color: #164a9a; - } - .label-unverified { - color: #aaa; - } - li button { - float: right; - margin-left: 5px; - } - .add-email { - clear: both; - } -} diff --git a/scss/pages/emails.scss b/scss/pages/emails.scss new file mode 100644 index 0000000000..4c4c6560a4 --- /dev/null +++ b/scss/pages/emails.scss @@ -0,0 +1,40 @@ +#emails #content { + .icon { + color: $black; + position: absolute; + top: 12px; + left: 16px; + font-size: 24px; + line-height: 24px; + } + button { + color: $medium-gray; + font: normal 12px/14px $Ideal; + background: $light-gray; + padding: 3px 6px; + position: relative; + z-index: 2; + &:hover { color: $white; } + &.set-primary:hover { background: $blue; } + &.remove:hover { background: $red; } + &.add:hover { background: $green; } + &.resend:hover { background: $green; } + } + .add { + form { + display: none; /* off to start, turn on via javascript */ + + input { + outline: none; + border: none; + border-bottom: 1px solid $black; + padding: 0; + } + + button { + color: $white; + background: $green; + } + } + } +} diff --git a/tests/ttw/test_email_verification.py b/tests/ttw/test_email_verification.py new file mode 100644 index 0000000000..57be0d31f5 --- /dev/null +++ b/tests/ttw/test_email_verification.py @@ -0,0 +1,104 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + +import re +from gratipay.testing import BrowserHarness, QueuedEmailHarness + + +class Tests(BrowserHarness, QueuedEmailHarness): + + + def test(self): + self.alice = self.make_participant('alice', claimed_time='now') + self.sign_in('alice') + self.visit('/~alice/emails/') + + # Helpers + + buttons = lambda: [b.text for b in self.css('button', row)] + click = lambda text: [b for b in self.css('button', row) if b.text == text][0].click() + + def add(email_address): + click('Add an email address') + self.css('input', row).fill(email_address) + click('Send verification') + + def get_verification_link(): + verification_email = self.pop_email_message() + verification_link = re.match( r'.*?(http.*?$).*' + , verification_email['body_text'] + , re.DOTALL | re.MULTILINE + ).groups()[0] + return verification_link + + + # Starts clean. + rows = self.css('#content tr') + assert len(rows) == 1 + row = rows[0] + assert buttons() == ['Add an email address', '', ''] + + # Can toggle add form on and off and on again. + click('Add an email address') + assert buttons() == ['', 'Send verification', 'Cancel'] + click('Cancel') + assert buttons() == ['Add an email address', '', ''] + + # Can submit add form. + add('alice@example.com') + row = self.wait_for('tr.existing') + assert buttons() == ['Resend verification'] + self.pop_email_message() # throw away verification message + + # Can resend verification. + click('Resend verification') + assert self.wait_for_success() == 'Check your inbox for a verification link.' + + # Can verify. + self.visit(get_verification_link()) + assert self.css('#content h1').text == 'Success!' + + # Now listed as primary -- nothing to be done with primary. + self.visit('/~alice/emails/') + rows = self.css('.emails.listing tr') + assert len(rows) == 2 + row = rows[0] + assert buttons() == [] + assert row.text.endswith('Your primary email address') + + # ... but the add form is back. Can we re-add the same? + row = rows[1] + add('alice@example.com') + assert self.wait_for_error() == 'You have already added and verified that address.' # No! + click('Cancel') + + # Let's add another! + add('alice@example.net') + self.wait_for('.emails.listing tr') + assert self.pop_email_message()['subject'] == 'New activity on your account' + self.visit(get_verification_link()) + + # Now we should have a primary and a linked address, and an add row. + self.css('#content a').click() + rows = self.wait_for('.emails.listing tr') + assert len(rows) == 3 + email_addresses = [x.text for x in self.css('.existing .listing-name')] + assert email_addresses == ['alice@example.com', 'alice@example.net'] + row = rows[2] + assert buttons() == ['Add an email address', '', ''] + + # We can promote the secondary account to primary. + row = rows[1] + click('Set as primary') + rows = self.wait_for('.emails.listing tr') + assert len(rows) == 3 + + # ... and now the order is reversed. + email_addresses = [x.text for x in self.css('.existing .listing-name')] + assert email_addresses == ['alice@example.net', 'alice@example.com'] + + # We can remove the (new) secondary account. + row = rows[1] + click('Remove') + self.wait_to_disappear('tr[data-email="alice@example.com"]') + assert len(self.css('.emails.listing tr')) == 2 diff --git a/www/assets/gratipay.css.spt b/www/assets/gratipay.css.spt index 7cc741a052..b54726e531 100644 --- a/www/assets/gratipay.css.spt +++ b/www/assets/gratipay.css.spt @@ -33,7 +33,6 @@ @import "scss/components/cta"; @import "scss/components/danger-zone"; @import "scss/components/dropdown"; -@import "scss/components/emails"; @import "scss/components/github-ribbon"; @import "scss/components/js-edit"; @import "scss/components/linear_gradient"; @@ -65,6 +64,7 @@ @import "scss/layouts/layout"; @import "scss/layouts/responsiveness"; +@import "scss/pages/emails"; @import "scss/pages/homepage"; @import "scss/pages/history"; @import "scss/pages/identities"; diff --git a/www/~/%username/emails/index.spt b/www/~/%username/emails/index.spt index d3fde7a684..89e075ddc8 100644 --- a/www/~/%username/emails/index.spt +++ b/www/~/%username/emails/index.spt @@ -1,12 +1,18 @@ -from gratipay.utils import get_participant +from gratipay.utils import get_participant, icons [-----------------------------------------------------------------------------] participant = get_participant(state, restrict=True) banner = '~' + participant.username -title = _("Emails") +title = _("Email") emails = participant.get_emails() +groups = [[], [], []] # primary, verified, unverified +for email in emails: + i = 0 if email.address == participant.email_address else 1 if email.verified else 2 + groups[i].append(email) +emails = sum(groups, []) +page_id = 'emails' [-----------------------------------------------------------------------------] text/html {% extends "templates/profile.html" %} @@ -17,26 +23,60 @@ emails = participant.get_emails() {% endblock %} {% block content %} -
- -
- - -
-
+

{{ _('Addresses') }}

+{% set has_unverified = False %} + +{% for i, email in enumerate(emails, start=1) %} + + + +{% endfor %} + {% if not participant.has_pending_email_address_verifications %} + + + + {% endif %} +
+ +
+ {{ email.address }} + {{ i }} · + {% if email.verified %} + {% if email.address == participant.email_address %} + + {{ icons.STATUS_ICONS['feature']|safe }} + {{ _('Your primary email address') }} + {% else %} + + {{ icons.STATUS_ICONS['success']|safe }} + {{ _('Linked') }} + · + · + {% endif %} + {% else %} + + {{ icons.STATUS_ICONS['warning']|safe }} + {{ _('Check your inbox') }} · + + {% endif %} +
+
+ +
+ + + +
+ + + + {{ len(emails) + 1 }} · + · + +
+
+

{{ _("Notifications") }}