From 1b1da812f703bb7a1d3c35c2fe71c475f4dbc7be Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 26 May 2017 11:09:04 -0400 Subject: [PATCH 1/3] Implement bulk claiming on /on/npm/ --- js/gratipay/packages.js | 49 +++++++--- scss/components/listing.scss | 18 ++++ scss/components/text-treatments.scss | 12 ++- scss/pages/package.scss | 6 -- tests/py/test_www_npm_package.py | 10 ++ tests/ttw/test_package_claiming.py | 92 +++++++++++++++++- www/on/npm/%package/index.html.spt | 32 ++++--- www/on/npm/index.html.spt | 126 ++++++++++++++++++++++++- www/~/%username/emails/modify.json.spt | 6 +- 9 files changed, 311 insertions(+), 40 deletions(-) diff --git a/js/gratipay/packages.js b/js/gratipay/packages.js index 67ad49d1c3..cb47b88453 100644 --- a/js/gratipay/packages.js +++ b/js/gratipay/packages.js @@ -1,33 +1,60 @@ Gratipay.packages = {}; -Gratipay.packages.init = function() { +Gratipay.packages.initBulk = function() { + $('button.apply').on('click', Gratipay.packages.postBulk); +}; + +Gratipay.packages.initSingle = function() { Gratipay.Select('.gratipay-select'); - $('button.apply').on('click', Gratipay.packages.post); + $('button.apply').on('click', Gratipay.packages.postOne); }; -Gratipay.packages.post = function(e) { + +Gratipay.packages.postBulk = function(e) { + e.preventDefault(); + var pkg, email, package_id, package_ids_by_email={}; + $('table.listing td.item ').not('.disabled').each(function() { + pkg = $(this).data(); + if (package_ids_by_email[pkg.email] === undefined) + package_ids_by_email[pkg.email] = []; + 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.postOne = function(e) { e.preventDefault(); - var $this = $(this); - var action = 'start-verification'; - var package_id = $('input[name=package_id]').val(); var email = $('input[name=email]:checked').val(); + var package_id = $('input[name=package_id]').val(); + Gratipay.packages.post(email, [package_id]); +} + - var $inputs = $('input, button'); - $inputs.prop('disabled', true); +Gratipay.packages.post = function(email, package_ids, show_email) { + var action = 'start-verification'; + var $button = $('button.apply') + $button.prop('disabled', true); + function reenable() { $button.prop('disabled', false); } $.ajax({ url: '/~' + Gratipay.username + '/emails/modify.json', type: 'POST', - data: {action: action, address: email, package_id: package_id}, + data: { action: action + , address: email + , package_id: package_ids + , show_address_in_message: true + }, + traditional: true, dataType: 'json', success: function (msg) { if (msg) { Gratipay.notification(msg, 'success'); + reenable(); } - $inputs.prop('disabled', false); }, error: [ - function () { $inputs.prop('disabled', false); }, + reenable, Gratipay.error ], }); diff --git a/scss/components/listing.scss b/scss/components/listing.scss index 54118b4934..1ac66a6109 100644 --- a/scss/components/listing.scss +++ b/scss/components/listing.scss @@ -94,6 +94,24 @@ table.listing { border-bottom: 1px solid $light-brown; } } + + &.disabled { + img.avatar { + filter: grayscale(100%) brightness(110%); + } + .package-manager img { + filter: grayscale(100%) brightness(120%); + } + .listing-name { + color: $light-gray ! important; + } + .status a { + color: $medium-gray! important; + } + .owner a { + color: $medium-gray! important; + } + } } } .with-sidebar table.listing td.item .listing-details { diff --git a/scss/components/text-treatments.scss b/scss/components/text-treatments.scss index 80fce1ea13..0cba42793a 100644 --- a/scss/components/text-treatments.scss +++ b/scss/components/text-treatments.scss @@ -15,12 +15,13 @@ } } - .sorry { + .instructions { text-align: center; - font: normal 12px/15px $Ideal; - color: $medium-gray; + margin: 0 0 30px; + font-family: $Ideal; } - .note { + + .note, .sorry, .fine-print { font: normal 12px/15px $Ideal; color: $medium-gray; a { @@ -29,6 +30,9 @@ color: $medium-gray; } } + .sorry, .fine-print { + text-align: center; + } .listing-name { color: $black; font: bold 20px/24px $Ideal; diff --git a/scss/pages/package.scss b/scss/pages/package.scss index 1bd615d4e6..a9f1ccdfbe 100644 --- a/scss/pages/package.scss +++ b/scss/pages/package.scss @@ -1,12 +1,6 @@ #package #content { text-align: center; - .instructions { - text-align: center; - margin: 0 0 30px; - font-family: $Ideal; - } - .selected .icon { display: block; } .icon { display: none; diff --git a/tests/py/test_www_npm_package.py b/tests/py/test_www_npm_package.py index c99d6078b8..e1ea7801f9 100644 --- a/tests/py/test_www_npm_package.py +++ b/tests/py/test_www_npm_package.py @@ -53,3 +53,13 @@ def test_package_redirects_to_project_if_claimed(self): def test_package_served_as_project_if_claimed(self): self.claim_package() assert 'owned by' in self.client.GET('/foo/').body + + +class Bulk(Harness): + + def setUp(self): + self.make_package() + + def test_anon_gets_signin_page(self): + body = self.client.GET('/on/npm/').body + assert '0 out of all 1 npm package' in body diff --git a/tests/ttw/test_package_claiming.py b/tests/ttw/test_package_claiming.py index 86051152f0..14501bd7a1 100644 --- a/tests/ttw/test_package_claiming.py +++ b/tests/ttw/test_package_claiming.py @@ -16,7 +16,8 @@ def check(self, choice=0): self.css('label')[0].click() # activate select self.css('label')[choice].click() self.css('button')[0].click() - assert self.wait_for_success() == 'Check your inbox for a verification link.' + 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 join emails e on c.nonce = e.nonce') def finish_claiming(self): @@ -143,3 +144,92 @@ def test_jdorfman_can_merge_accounts(self): payload = eval(self.css('table#events td.payload').text) assert payload['action'] == 'take-over' assert payload['values']['exchange_routes'] == [r.id for r in jdorfman.get_payout_routes()] + + +class BulkClaiming(BrowserHarness): + + def setUp(self): + self.make_package() + self.make_package( name='bar' + , description='Bar barringly' + , emails=['alice@example.com', 'bob@example.com'] + ) + self.make_package( name='baz' + , description='Baz bazzingly' + , emails=['alice@example.com', 'bob@example.com', 'cat@example.com'] + ) + + def visit_as(self, username): + self.visit('/') + self.sign_in(username) + self.visit('/on/npm/') + + def test_anon_gets_sign_in_prompt(self): + self.visit('/on/npm/') + assert self.css('.important-button button').text == 'Sign in / Sign up' + + def test_auth_without_email_gets_highlighted_link_to_email(self): + self.make_participant('alice', claimed_time='now') + self.visit_as('alice') + assert self.css('.highlight').text == 'Link an email' + + def test_auth_without_claimable_packages_gets_disabled_apply_button(self): + self.make_participant('doug', claimed_time='now', email_address='doug@example.com') + self.visit_as('doug') + button = self.css('.important-button button') + assert button.text == 'Apply to accept payments' + assert button['disabled'] == 'true' + + def test_auth_with_claimable_packages_gets_apply_button(self): + self.make_participant('alice', claimed_time='now', email_address='alice@example.com') + self.add_and_verify_email('alice', 'bob@example.com') + self.visit_as('alice') + button = self.css('.important-button button') + assert button.text == 'Apply to accept payments' + assert button['disabled'] is None + + def test_differentiates_claimed_packages(self): + self.make_participant('bob', claimed_time='now', email_address='bob@example.com') + self.make_participant('alice', claimed_time='now', email_address='alice@example.com') + self.claim_package('alice', 'foo') + self.claim_package('bob', 'bar') + self.visit_as('alice') + assert self.css('.i1').has_class('disabled') + assert self.css('.i1 .owner a').text == '~bob' + assert not self.css('.i2').has_class('disabled') + assert self.css('.i3').has_class('disabled') + assert self.css('.i3 .owner a').text == 'you' + + def test_sends_mail(self): + self.make_participant('cat', claimed_time='now', email_address='cat@example.com') + self.visit_as('cat') + self.css('.important-button button').click() + assert self.wait_for_success() == 'Check cat@example.com for a verification link.' + + def test_sends_one_mail_per_address(self): + cat = self.make_participant('cat', claimed_time='now', email_address='cat@example.com') + self.add_and_verify_email(cat, 'bob@example.com') + self.visit_as('cat') + self.css('.important-button button').click() + assert self.wait_for_success('Check bob@example.com for a verification link.') + assert self.wait_for_success('Check cat@example.com for a verification link.') + + def test_sends_one_mail_for_multiple_packages(self): + self.make_participant('alice', claimed_time='now', email_address='alice@example.com') + self.visit_as('alice') + self.css('.important-button button').click() + assert len(self.css('table.listing td.item')) == 3 + assert self.wait_for_success() == 'Check alice@example.com for a verification link.' + assert self.db.one('select count(*) from claims') == 3 + assert self.db.one('select count(*) from email_queue') == 1 + + def test_doesnt_send_for_unclaimable_packages(self): + self.make_participant('alice', claimed_time='now', email_address='alice@example.com') + self.make_participant('cat', claimed_time='now', email_address='cat@example.com') + self.claim_package('cat', 'baz') + self.visit_as('alice') + self.css('.important-button button').click() + assert len(self.css('table.listing td.item')) == 3 + assert self.wait_for_success() == 'Check alice@example.com for a verification link.' + assert self.db.one('select count(*) from claims') == 2 + assert self.db.one('select count(*) from email_queue') == 1 diff --git a/www/on/npm/%package/index.html.spt b/www/on/npm/%package/index.html.spt index 3c195ece60..9efb9543bb 100644 --- a/www/on/npm/%package/index.html.spt +++ b/www/on/npm/%package/index.html.spt @@ -37,7 +37,7 @@ if user.participant: {% block scripts %} {{ super() }} @@ -48,14 +48,14 @@ if user.participant: {% if package.description %}

{{ package.description }}

{% else %} -

{{ _("No description available.") }}

+

{{ _("No description available.") }}

{% endif %}

{{ _( 'Apply to accept payments for the {package_link} npm package:' , package_link=('' + package_name + '')|safe ) }} -

+

{% if user.ANON %}
@@ -63,7 +63,7 @@ if user.participant:
{% else %} {% if len(emails) == 0 %} -

{{ _("No email addresses on file.") }}

+

{{ _("No email addresses on file.") }}

{% else %}
@@ -115,15 +115,19 @@ if user.participant:
{% endif %} -

{{ _( 'Addresses are from {a}{code}maintainers{_code}{_a}.' - , a=('')|safe - , _a=''|safe - , code=''|safe - , _code=''|safe - ) }}

-

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

+

+ {{ _( 'Addresses are from {a}{code}maintainers{_code}{_a}.' + , a=('')|safe + , _a=''|safe + , code=''|safe + , _code=''|safe + ) }} +

+

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

{% endif %} {% endblock %} diff --git a/www/on/npm/index.html.spt b/www/on/npm/index.html.spt index e751da7303..80fd43eb1c 100644 --- a/www/on/npm/index.html.spt +++ b/www/on/npm/index.html.spt @@ -1,9 +1,129 @@ +from gratipay.utils import icons [---] -title = "Search npm" +banner = manager = "npm" suppress_sidebar = True -website.redirect('/') # nothing here yet +npackages, Npackages = website.db.one(''' + select (select count(*) from teams_to_packages) as n, (select count(*) from packages) as t +''') +if user.participant: + packages_for_claiming = user.participant.get_packages_for_claiming(manager) + any_claimable = any([rec.claimed_by is None for rec in packages_for_claiming]) [---] {% extends "templates/base.html" %} + +{% block banner %} + +
+ + +
+ {{ super() }} + +{% endblock %} + +{% block scripts %} + +{{ super() }} +{% endblock %} + {% block content %} - + +

+ {{ _('Free as in money.') }} +

+ +

+ {{ _('Apply to accept payments for your npm packages:') }} +

+ +{% if user.ANON %} +
+ {{ sign_in_using(button_class='large') }} +
+{% else %} + + {% if not packages_for_claiming %} +

{{ _("No packages found.") }}

+ {% else %} + + {% for i, rec in enumerate(packages_for_claiming, start=1) %} + + + + {% endfor %} +
+ +
+ + {{ rec.package.name }} + + +
+ {{ i }} + {% if rec.claimed_by %} + · + + {{ icons.STATUS_ICONS['failure']|safe }} + {% if rec.claimed_by == user.participant %} + {{ _( "already claimed by {a}you{_a}" + , a=(''|safe + ).format(rec.claimed_by.url_path) + , _a=''|safe + ) }} + {% else %} + {{ _( "already claimed by {a}{owner}{_a}" + , owner='~'+rec.claimed_by.username + , a=(''|safe + ).format(rec.claimed_by.url_path) + , _a=''|safe + ) }} + {% endif %} + + {% else %} + + · + {% if rec.email_address_is_primary %} + + {{ icons.STATUS_ICONS['feature']|safe }} + {% else %} + + {{ icons.STATUS_ICONS['success']|safe }} + {% endif %} + {{ rec.email_address }} + + {% endif %} + · + {{ rec.package.description }} + +
+
+ {% endif %} + +
+ +
+ +

+ {{ _( "{a}Link an email{_a} from npm to see related packages." + , a=(''|safe + if packages_for_claiming else + ''|safe) + , _a=''|safe + ) }} +

+{% endif %} +

+ {{ _( "{n} out of all {N} npm packages are on Gratipay." + , n=format_number(npackages) + , N=format_number(Npackages) + ) }} +

{% endblock %} diff --git a/www/~/%username/emails/modify.json.spt b/www/~/%username/emails/modify.json.spt index b99cc33895..dff3c86915 100644 --- a/www/~/%username/emails/modify.json.spt +++ b/www/~/%username/emails/modify.json.spt @@ -17,6 +17,7 @@ participant = get_participant(state, restrict=True) action = request.body['action'] address = request.body['address'] +show_address_in_message = bool(request.body.get('show_address_in_message', '')) # Basic checks. The real validation will happen when we send the email. if (len(address) > 254) or not email_re.match(address): @@ -46,7 +47,10 @@ if action in ('add-email', 'resend', 'start-verification'): raise Response(400) participant.start_email_verification(address, *packages) - msg = _("Check your inbox for a verification link.") + if show_address_in_message: + msg = _("Check {email_address} for a verification link.", email_address=address) + else: + msg = _("Check your inbox for a verification link.") elif action == 'set-primary': participant.set_primary_email(address) elif action == 'remove': From c6d6a6388317f79b5ee342db36fa1105ceaef4f1 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 15 Jun 2017 10:28:40 -0400 Subject: [PATCH 2/3] Explain show_address_in_message --- www/~/%username/emails/modify.json.spt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/www/~/%username/emails/modify.json.spt b/www/~/%username/emails/modify.json.spt index dff3c86915..da38b70824 100644 --- a/www/~/%username/emails/modify.json.spt +++ b/www/~/%username/emails/modify.json.spt @@ -48,8 +48,15 @@ if action in ('add-email', 'resend', 'start-verification'): participant.start_email_verification(address, *packages) if show_address_in_message: + + # When reverifying an already-verified email (package claiming is a + # special case of this), then don't worry about content spoofing, + msg = _("Check {email_address} for a verification link.", email_address=address) else: + + # ... but otherwise, do: https://hackerone.com/reports/117187. + msg = _("Check your inbox for a verification link.") elif action == 'set-primary': participant.set_primary_email(address) From affc93b5d8397b438d23b4101d78966ebc3c2387 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 15 Jun 2017 10:54:57 -0400 Subject: [PATCH 3/3] Light refactor on npm_stats for the page --- www/on/npm/index.html.spt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/www/on/npm/index.html.spt b/www/on/npm/index.html.spt index 80fd43eb1c..a2bcf17065 100644 --- a/www/on/npm/index.html.spt +++ b/www/on/npm/index.html.spt @@ -2,8 +2,9 @@ from gratipay.utils import icons [---] banner = manager = "npm" suppress_sidebar = True -npackages, Npackages = website.db.one(''' - select (select count(*) from teams_to_packages) as n, (select count(*) from packages) as t +npm_stats = website.db.one(''' + select (select count(*) from teams_to_packages) as claimed_packages + , (select count(*) from packages) as total_packages ''') if user.participant: packages_for_claiming = user.participant.get_packages_for_claiming(manager) @@ -122,8 +123,8 @@ if user.participant: {% endif %}

{{ _( "{n} out of all {N} npm packages are on Gratipay." - , n=format_number(npackages) - , N=format_number(Npackages) + , n=format_number(npm_stats.claimed_packages) + , N=format_number(npm_stats.total_packages) ) }}

{% endblock %}