From 7ad0d483d7d9a4d9c2310e2a0669738ac2a3c4a9 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 28 Aug 2017 08:05:13 -0400 Subject: [PATCH 1/2] Improve escaping while we're at it Cf. #4585. --- www/on/npm/%package.spt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/www/on/npm/%package.spt b/www/on/npm/%package.spt index 9756f27809..6eb89e603a 100644 --- a/www/on/npm/%package.spt +++ b/www/on/npm/%package.spt @@ -124,7 +124,7 @@ if user.participant: {% endif %}

{{ _( 'Addresses are from {a}{code}maintainers{_code}{_a}.' - , a=('')|safe + , a=''|safe % package.remote_api_url , _a=''|safe , code=''|safe , _code=''|safe @@ -132,7 +132,7 @@ if user.participant:

{{ _( 'Out of date? Update {a}at npm{_a} and refresh.' - , a=('')|safe + , a=''|safe % package.remote_human_url , _a=''|safe ) }}

From 687736630b8ded8d429c091628a2be1175f2748a Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 28 Aug 2017 08:56:28 -0400 Subject: [PATCH 2/2] Only start claims when address verified If unverified, allow to use the package page to start verification. --- js/gratipay/packages.js | 33 ++++++++---- js/gratipay/select.js | 9 ++-- scss/pages/package.scss | 6 +++ tests/ttw/test_package_claiming.py | 70 +++++++++++++++++++------- www/on/npm/%package.spt | 28 +++++++---- www/~/%username/emails/modify.json.spt | 2 +- 6 files changed, 106 insertions(+), 42 deletions(-) diff --git a/js/gratipay/packages.js b/js/gratipay/packages.js index cb47b88453..55c59b5873 100644 --- a/js/gratipay/packages.js +++ b/js/gratipay/packages.js @@ -1,14 +1,20 @@ Gratipay.packages = {}; Gratipay.packages.initBulk = function() { - $('button.apply').on('click', Gratipay.packages.postBulk); + $('.important-button button.apply').on('click', Gratipay.packages.postBulk); }; Gratipay.packages.initSingle = function() { - Gratipay.Select('.gratipay-select'); - $('button.apply').on('click', Gratipay.packages.postOne); + Gratipay.Select('.gratipay-select', Gratipay.packages.selectOne); + $('.important-button button').on('click', Gratipay.packages.postOne); + Gratipay.packages.selectOne($('.gratipay-select li.selected')); }; +Gratipay.packages.selectOne = function($li) { + var action = $li.data('action'); + $('.important-button span').hide(); + $('.important-button span.' + action).show(); +}; Gratipay.packages.postBulk = function(e) { e.preventDefault(); @@ -20,20 +26,25 @@ Gratipay.packages.postBulk = function(e) { package_ids_by_email[pkg.email].push(pkg.packageId); }); for (email in package_ids_by_email) - Gratipay.packages.post(email, package_ids_by_email[email], true); + Gratipay.packages.post(email, package_ids_by_email[email], 'yes'); }; Gratipay.packages.postOne = function(e) { e.preventDefault(); - var email = $('input[name=email]:checked').val(); - var package_id = $('input[name=package_id]').val(); - Gratipay.packages.post(email, [package_id]); + var $input = $('input[name=email]:checked'); + var email = $input.val(); + var package_ids; + var show_address_in_message = 'no'; + if ($input.closest('li').data('action') === 'apply') { + package_ids = [$('input[name=package_id]').val()]; + show_address_in_message = 'yes'; + } + Gratipay.packages.post(email, package_ids, show_address_in_message); } - -Gratipay.packages.post = function(email, package_ids, show_email) { +Gratipay.packages.post = function(email, package_ids, show_address_in_message) { var action = 'start-verification'; - var $button = $('button.apply') + var $button = $('.important-button button') $button.prop('disabled', true); function reenable() { $button.prop('disabled', false); } @@ -43,7 +54,7 @@ Gratipay.packages.post = function(email, package_ids, show_email) { data: { action: action , address: email , package_id: package_ids - , show_address_in_message: true + , show_address_in_message: show_address_in_message }, traditional: true, dataType: 'json', diff --git a/js/gratipay/select.js b/js/gratipay/select.js index 25691a5c09..4a82daf1f7 100644 --- a/js/gratipay/select.js +++ b/js/gratipay/select.js @@ -1,6 +1,7 @@ -Gratipay.Select = function(selector) { +Gratipay.Select = function(selector, onselect) { var $ul = $('ul', selector); var $labels = $('label', $ul); + var onselect = onselect || function() {}; // state for vertical position var topFactor = 0; // float between 0 and $labels.length-1 @@ -51,9 +52,11 @@ Gratipay.Select = function(selector) { function close($label) { if ($label) { - if ($label.closest('li').hasClass('disabled')) return; + var $li = $label.closest('li'); + if ($li.hasClass('disabled')) return; $('.selected', $ul).removeClass('selected') - $label.closest('li').addClass('selected').removeClass('hover'); + $li.addClass('selected').removeClass('hover'); + onselect($li); } $ul.css({'top': 0}).removeClass('open'); $ul.unbind('mousewheel'); diff --git a/scss/pages/package.scss b/scss/pages/package.scss index a9f1ccdfbe..bfa49920b3 100644 --- a/scss/pages/package.scss +++ b/scss/pages/package.scss @@ -14,6 +14,12 @@ color: $medium-gray; } + .important-button { + span { + display: none; + } + } + .status-icon { font-size: 12px; line-height: 12px; diff --git a/tests/ttw/test_package_claiming.py b/tests/ttw/test_package_claiming.py index 472005955b..5d1b32f1ed 100644 --- a/tests/ttw/test_package_claiming.py +++ b/tests/ttw/test_package_claiming.py @@ -9,13 +9,25 @@ class Test(BrowserHarness): - def check(self, choice=0): - self.make_participant('alice', claimed_time='now') + def setUp(self): + BrowserHarness.setUp(self) + self.alice = self.make_participant( 'alice' + , claimed_time='now' + , email_address='alice@example.com' + ) + self.add_and_verify_email(self.alice, 'bob@example.com') self.sign_in('alice') + + def choose(self, choice=0): + self.css('#content li.selected label').click() # activate select + want = self.css('#content label')[choice] + want.click() + return want.text + + def check(self, choice=0): self.visit('/on/npm/foo') - self.css('#content label')[0].click() # activate select - self.css('#content label')[choice].click() - self.css('#content button')[0].click() + self.choose(choice) + self.css('#content .important-button button').click() address = ('alice' if choice == 0 else 'bob') + '@example.com' assert self.wait_for_success() == 'Check {} for a verification link.'.format(address) return self.db.one('select address from claims c ' @@ -39,18 +51,42 @@ def test_can_send_to_second_email(self): self.make_package(emails=['alice@example.com', 'bob@example.com']) assert self.check(choice=1) == 'bob@example.com' - def test_disabled_items_are_disabled(self): - self.make_package(emails=['alice@example.com', 'bob@example.com']) - alice = self.make_participant('alice', claimed_time='now') - self.add_and_verify_email(alice, 'alice@example.com', 'bob@example.com') - self.sign_in('alice') + def test_button_varies_with_email_state(self): + self.make_package(emails=['alice@example.com', 'bob@example.com', 'cat@example.com', + 'doug@example.com', 'edna@example.com']) + self.make_participant('doug', claimed_time='now', email_address='doug@example.com') + self.alice.start_email_verification('cat@example.com') + self.visit('/on/npm/foo') + + self.choose(0) == 'alice@example.com' + self.css('li.selected').text.endswith('Your primary email address') + self.css('button').text == 'Apply to accept payments' + + self.choose(1) == 'bob@example.com' + self.css('li.selected').text.endswith('Linked to your account') + self.css('button').text == 'Apply to accept payments' + + self.choose(2) == 'cat@example.com' + self.css('li.selected').text.endswith('Verification pending') + self.css('button').text == 'Resend verification' + + self.choose(3) == 'edna@example.com' + self.css('li.selected').text.endswith('Unverified') + self.css('button').text == 'Verify email address' + + # doug is last, can't even be selected + self.choose(4) == 'doug@example.com' + self.css('li.selected label') == 'edna@example.com' + self.css('#content li')[4].text.endswith('Linked to a different account') + + def test_sending_to_unverified_doesnt_start_a_claim(self): + self.make_package(emails=['alice@example.com', 'cat@example.com']) self.visit('/on/npm/foo') - self.css('#content label')[0].click() # activate select - self.css('#content label')[1].click() # click second item - self.css('#content li')[0].has_class('selected') # first item is still selected - self.css('#content ul')[0].has_class('open') # still open - self.css('#content button').has_class('disabled') - assert self.db.all('select * from claims') == [] + self.choose(1) + self.css('#content .important-button button').click() + assert self.wait_for_success() == 'Check your inbox for a verification link.' + assert self.db.one('select address from claims c ' + 'join email_addresses e on c.nonce = e.nonce') is None def test_claimed_packages_can_be_given_to(self): @@ -79,7 +115,7 @@ def test_visiting_verify_link_shows_helpful_information(self): self.make_package() self.check() - link = pickle.loads(self.db.one('select context from email_messages'))['link'] + link = pickle.loads(self.db.all('select context from email_messages')[-1])['link'] link = link[len(self.base_url):] # strip because visit will add it back self.visit(link) diff --git a/www/on/npm/%package.spt b/www/on/npm/%package.spt index 6eb89e603a..808077bd1c 100644 --- a/www/on/npm/%package.spt +++ b/www/on/npm/%package.spt @@ -22,7 +22,6 @@ banner = package.name if user.participant: emails = package.classify_emails_for_participant(user.participant) - has_email_options = bool([x for x in emails if x[1] != OTHER]) [---] text/html {% extends "templates/base.html" %} @@ -77,7 +76,11 @@ if user.participant: