From e33a69ed43f528a32502ed101ecd5627207b51b4 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 11:20:59 -0500 Subject: [PATCH 01/10] Failing test for team (project) closing --- tests/py/test_team_closing.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/py/test_team_closing.py diff --git a/tests/py/test_team_closing.py b/tests/py/test_team_closing.py new file mode 100644 index 0000000000..19ad27b32a --- /dev/null +++ b/tests/py/test_team_closing.py @@ -0,0 +1,12 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + +from gratipay.testing import Harness + + +class TestTeamClosing(Harness): + + def test_teams_can_be_closed(self): + team = self.make_team() + team.close() + assert team.is_closed From 08627ab6eef06d612adebb83c449b4ba80d353dc Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 12:17:57 -0500 Subject: [PATCH 02/10] Add Python API for closing teams --- gratipay/models/team/__init__.py | 3 ++- gratipay/models/team/mixins/__init__.py | 3 ++- gratipay/models/team/mixins/closing.py | 22 ++++++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 gratipay/models/team/mixins/closing.py diff --git a/gratipay/models/team/__init__.py b/gratipay/models/team/__init__.py index 7ba0a6120c..ae14e74131 100644 --- a/gratipay/models/team/__init__.py +++ b/gratipay/models/team/__init__.py @@ -33,7 +33,8 @@ def slugize(name): return slug -class Team(Model, mixins.Available, mixins.Membership, mixins.Takes, mixins.TipMigration): +class Team(Model, mixins.Available, mixins.Closing, mixins.Membership, mixins.Takes, + mixins.TipMigration): """Represent a Gratipay team. """ diff --git a/gratipay/models/team/mixins/__init__.py b/gratipay/models/team/mixins/__init__.py index 90e544cf4c..e7635c01d2 100644 --- a/gratipay/models/team/mixins/__init__.py +++ b/gratipay/models/team/mixins/__init__.py @@ -1,6 +1,7 @@ from .available import AvailableMixin as Available +from .closing import ClosingMixin as Closing from .membership import MembershipMixin as Membership from .takes import TakesMixin as Takes from .tip_migration import TipMigrationMixin as TipMigration -__all__ = ['Available', 'Membership', 'Takes', 'TipMigration'] +__all__ = ['Available', 'Closing', 'Membership', 'Takes', 'TipMigration'] diff --git a/gratipay/models/team/mixins/closing.py b/gratipay/models/team/mixins/closing.py new file mode 100644 index 0000000000..7ae3e517e7 --- /dev/null +++ b/gratipay/models/team/mixins/closing.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, division, print_function, unicode_literals + +from gratipay.models import add_event + + +class ClosingMixin(object): + """This mixin implements team closing. + """ + + #: Whether the team is closed or not. + + is_closed = False + + + def close(self): + """Close the team account. + """ + with self.db.get_cursor() as cursor: + cursor.run("UPDATE teams SET is_closed=true WHERE id=%s", (self.id,)) + add_event(cursor, 'team', dict(id=self.id, action='set', values=dict(is_closed=True))) + self.set_attributes(is_closed=True) From 20ec01e203d4a9c4630da620013edc6ea0391dfd Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 12:52:32 -0500 Subject: [PATCH 03/10] Harmonize "Close" styles across pages --- scss/components/danger-zone.scss | 14 ++++++++++++++ scss/components/long-form.scss | 15 --------------- scss/elements/buttons-knobs.scss | 12 ------------ www/assets/gratipay.css.spt | 1 + www/~/%username/settings/index.html.spt | 10 +++++----- 5 files changed, 20 insertions(+), 32 deletions(-) create mode 100644 scss/components/danger-zone.scss diff --git a/scss/components/danger-zone.scss b/scss/components/danger-zone.scss new file mode 100644 index 0000000000..1870fa0c68 --- /dev/null +++ b/scss/components/danger-zone.scss @@ -0,0 +1,14 @@ +.danger-zone { + margin-top: 64px; + border: 1px solid $red; + @include border-radius(5px); + padding: 20px; + h2 { + margin: 0 0 10px; + padding: 0; + color: $red; + } + button { + background: $red; + } +} diff --git a/scss/components/long-form.scss b/scss/components/long-form.scss index 0f4ae26581..e847028677 100644 --- a/scss/components/long-form.scss +++ b/scss/components/long-form.scss @@ -113,19 +113,4 @@ input.invalid:focus + .invalid-msg { display: block; } - - .danger-zone { - margin-top: 64px; - border: 1px solid $red; - @include border-radius(5px); - padding: 20px; - h2 { - margin: 0 0 10px; - padding: 0; - color: $red; - } - button { - background: $red; - } - } } diff --git a/scss/elements/buttons-knobs.scss b/scss/elements/buttons-knobs.scss index bceec29c1c..3cf1222017 100644 --- a/scss/elements/buttons-knobs.scss +++ b/scss/elements/buttons-knobs.scss @@ -48,18 +48,6 @@ button.selected:hover, button.selected.drag, background: $darker-green; } -button.close-account { - font-weight: normal; - background: none; - color: $red; - border: 1px solid $red; - &:hover { - background: $red; - color: white; - text-decoration: none; - } -} - .buttons button { margin-top: 3px; } diff --git a/www/assets/gratipay.css.spt b/www/assets/gratipay.css.spt index 30ea3a2ebe..be26cc1214 100644 --- a/www/assets/gratipay.css.spt +++ b/www/assets/gratipay.css.spt @@ -30,6 +30,7 @@ @import "scss/components/chosen"; @import "scss/components/communities"; @import "scss/components/cta"; +@import "scss/components/danger-zone"; @import "scss/components/dropdown"; @import "scss/components/emails"; @import "scss/components/js-edit"; diff --git a/www/~/%username/settings/index.html.spt b/www/~/%username/settings/index.html.spt index e15469ee62..6d24898aec 100644 --- a/www/~/%username/settings/index.html.spt +++ b/www/~/%username/settings/index.html.spt @@ -94,11 +94,11 @@ emails = participant.get_emails() {% endif %}

-
-

{{ _("Close") }}

-
- -
+
+

{{ _("Danger Zone") }}

+
+ +
{% endblock %} From d453ecd3ded7a5ed4e50528bb0113a849b447a24 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 14:33:06 -0500 Subject: [PATCH 04/10] Add UI for closing a project --- js/gratipay/edit_team.js | 9 +++++---- tests/py/test_team_closing.py | 30 ++++++++++++++++++++++++++++-- www/%team/edit/close.spt | 22 ++++++++++++++++++++++ www/%team/edit/index.html.spt | 7 ++++++- 4 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 www/%team/edit/close.spt diff --git a/js/gratipay/edit_team.js b/js/gratipay/edit_team.js index 09cc28dda7..2e251f3a43 100644 --- a/js/gratipay/edit_team.js +++ b/js/gratipay/edit_team.js @@ -1,14 +1,15 @@ Gratipay.edit_team = {} Gratipay.edit_team.initForm = function() { - $form = $("#edit-team"); - $buttons = $form.find("button"); // submit and cancel btns - $form.submit(Gratipay.edit_team.submitForm); + $('#edit-team').submit(Gratipay.edit_team.submitForm); + $('#close-team').submit(function() { return confirm('Really close project?') }); } Gratipay.edit_team.submitForm = function(e) { e.preventDefault(); + var $form = $("#edit-team"); + var $buttons = $form.find("button"); var data = new FormData($form[0]); $buttons.prop("disabled", true); @@ -27,4 +28,4 @@ Gratipay.edit_team.submitForm = function(e) { }, error: [Gratipay.error, function () { $buttons.prop("disabled", false); }] }); -} \ No newline at end of file +} diff --git a/tests/py/test_team_closing.py b/tests/py/test_team_closing.py index 19ad27b32a..29ec4a15fa 100644 --- a/tests/py/test_team_closing.py +++ b/tests/py/test_team_closing.py @@ -1,12 +1,38 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import, division, print_function, unicode_literals -from gratipay.testing import Harness +from gratipay.testing import Harness, T class TestTeamClosing(Harness): - def test_teams_can_be_closed(self): + def test_teams_can_be_closed_via_python(self): team = self.make_team() team.close() assert team.is_closed + + def test_teams_can_be_closed_via_http(self): + self.make_team() + response = self.client.PxST('/TheEnterprise/edit/close', auth_as='picard') + assert response.headers['Location'] == '/~picard/' + assert response.code == 302 + assert T('TheEnterprise').is_closed + + def test_but_not_by_anon(self): + self.make_team() + response = self.client.PxST('/TheEnterprise/edit/close') + assert response.code == 401 + + def test_nor_by_turkey(self): + self.make_participant('turkey') + self.make_team() + response = self.client.PxST('/TheEnterprise/edit/close', auth_as='turkey') + assert response.code == 403 + + def test_admin_is_cool_though(self): + self.make_participant('Q', is_admin=True) + self.make_team() + response = self.client.PxST('/TheEnterprise/edit/close', auth_as='Q') + assert response.headers['Location'] == '/~Q/' + assert response.code == 302 + assert T('TheEnterprise').is_closed diff --git a/www/%team/edit/close.spt b/www/%team/edit/close.spt new file mode 100644 index 0000000000..beb1f3c00c --- /dev/null +++ b/www/%team/edit/close.spt @@ -0,0 +1,22 @@ +from aspen import Response +from gratipay.utils import get_team + +[----------------------------------------------------------------------------] + +if user.ANON: + raise Response(401, _("You need to log in to access this page.")) + +request.allow('POST') + +team = get_team(state) + +if not user.ADMIN and user.participant.username != team.owner: + raise Response(403, _("You are not authorized to access this page.")) + +if team.is_closed: + raise Response(403, _("Already closed.")) + +team.close() + +website.redirect('/~{}/'.format(user.participant.username)) +[---] diff --git a/www/%team/edit/index.html.spt b/www/%team/edit/index.html.spt index 99de9454a5..a8f68c65be 100644 --- a/www/%team/edit/index.html.spt +++ b/www/%team/edit/index.html.spt @@ -41,7 +41,7 @@ suppress_sidebar = True } -
+ @@ -63,4 +63,9 @@ suppress_sidebar = True
+
+ +

{{ _("Danger Zone") }}

+ +
{% endblock %} From c7fa2b1654798a68f9e14dd840563c6fed425a21 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 14:33:49 -0500 Subject: [PATCH 05/10] Don't show closed teams on the homepage --- www/index.html.spt | 1 + 1 file changed, 1 insertion(+) diff --git a/www/index.html.spt b/www/index.html.spt index d06b264e08..67ab7835cd 100644 --- a/www/index.html.spt +++ b/www/index.html.spt @@ -13,6 +13,7 @@ teams = website.db.all(""" SELECT teams.*::teams FROM teams + WHERE not is_closed ORDER BY ctime DESC """) From de2aa54a0d482480908391355fe08e66c2363121 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 14:35:00 -0500 Subject: [PATCH 06/10] Trim whitespace --- www/~/%username/settings/close.spt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/www/~/%username/settings/close.spt b/www/~/%username/settings/close.spt index f4d214f2e4..7714e78566 100644 --- a/www/~/%username/settings/close.spt +++ b/www/~/%username/settings/close.spt @@ -79,12 +79,12 @@ elif nteams == 1: href="https://github.com/gratipay/gratipay.com/issues/397">data retention policy).

-

Things we clear immediately include your profile statement, +

Things we clear immediately include your profile statement, the payroll you're taking, and the payments you're giving.

-

We specifically don't delete your past giving and - taking history on the site, because that information also belongs - equally to other users (the owners of the Teams you gave to and +

We specifically don't delete your past giving and + taking history on the site, because that information also belongs + equally to other users (the owners of the Teams you gave to and took from).

After you close your account, your profile page will say, From 03b264f50230021031ef70f6d7202d52b95b6de8 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 14:43:06 -0500 Subject: [PATCH 07/10] Add projects notice to participant close page --- www/~/%username/settings/close.spt | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/www/~/%username/settings/close.spt b/www/~/%username/settings/close.spt index 7714e78566..ecbe299dc7 100644 --- a/www/~/%username/settings/close.spt +++ b/www/~/%username/settings/close.spt @@ -16,13 +16,9 @@ if request.method == 'POST': else: participant.close() website.redirect('/~%s/' % participant.username) -teams = participant.get_teams() -nteams = len(teams) -if nteams > 1: - teams = ', '.join([t.name for t in teams[:-1]]) + ' and ' + teams[-1].name + ' teams' -elif nteams == 1: - teams = teams[0].name + ' team' - +teams = website.db.all( "SELECT teams.*::teams FROM teams WHERE owner=%s AND NOT is_closed" + , (participant.username,) + ) [---] text/html {% extends "templates/base.html" %} @@ -32,9 +28,12 @@ elif nteams == 1: {% if teams %} -

You are the owner of the {{ teams }}. We don't yet support - team owners closing their accounts.

+

Please close the following projects first:

+ {% endif %} @@ -80,7 +79,7 @@ elif nteams == 1: retention policy).

Things we clear immediately include your profile statement, - the payroll you're taking, and the payments you're giving.

+ the payments you're taking, and the payments you're giving.

We specifically don't delete your past giving and taking history on the site, because that information also belongs From 73926df678ac2f197816568bf8157703a8132695 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 Jan 2017 15:01:18 -0500 Subject: [PATCH 08/10] Bring participant close tests up to date --- tests/py/test_close.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/py/test_close.py b/tests/py/test_close.py index 971c7f294d..e70dcb3b7f 100644 --- a/tests/py/test_close.py +++ b/tests/py/test_close.py @@ -60,22 +60,7 @@ def test_close_page_shows_a_message_to_team_owners(self): alice = self.make_participant('alice', claimed_time='now') self.make_team('A', alice) body = self.client.GET('/~alice/settings/close', auth_as='alice').body - assert 'You are the owner of the A team.' in body - - def test_close_page_shows_a_message_to_owners_of_two_teams(self): - alice = self.make_participant('alice', claimed_time='now') - self.make_team('A', alice) - self.make_team('B', alice) - body = self.client.GET('/~alice/settings/close', auth_as='alice').body - assert 'You are the owner of the A and B teams.' in body - - def test_close_page_shows_a_message_to_owners_of_three_teams(self): - alice = self.make_participant('alice', claimed_time='now') - self.make_team('A', alice) - self.make_team('B', alice) - self.make_team('C', alice) - body = self.client.GET('/~alice/settings/close', auth_as='alice').body - assert 'You are the owner of the A, B and C teams.' in body + assert 'Please close the following projects first:' in body # cpi - clear_payment_instructions From 6b191f091f52e2b6e5b10c75f35aeb4d8112c0c2 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 11 Jan 2017 21:40:33 -0600 Subject: [PATCH 09/10] Hide closed teams from ~user profile --- gratipay/models/participant/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gratipay/models/participant/__init__.py b/gratipay/models/participant/__init__.py index 483ffacd85..2786ba53e6 100644 --- a/gratipay/models/participant/__init__.py +++ b/gratipay/models/participant/__init__.py @@ -1081,7 +1081,9 @@ def get_teams(self, only_approved=False, cursor=None): """Return a list of teams this user is an owner or member of. """ teams = (cursor or self.db).all(""" - SELECT teams.*::teams FROM teams WHERE owner=%s + SELECT teams.*::teams FROM teams WHERE + owner=%s AND + NOT is_closed UNION From 36948c045fb2eebab59ea4ae3e5d89bcbe01e317 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 12 Jan 2017 12:10:20 -0500 Subject: [PATCH 10/10] Beef up implementation of closed filtering Better match what we've already got. --- gratipay/models/participant/__init__.py | 12 ++++---- gratipay/testing/harness.py | 6 ++-- templates/team-listing.html | 9 +++--- tests/py/test_pages.py | 39 +++++++++++++++++++++++++ tests/py/test_participant.py | 10 ++++++- 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/gratipay/models/participant/__init__.py b/gratipay/models/participant/__init__.py index 2786ba53e6..6158e000ef 100644 --- a/gratipay/models/participant/__init__.py +++ b/gratipay/models/participant/__init__.py @@ -1077,23 +1077,23 @@ def profile_url(self): return '{base_url}/{username}/'.format(**locals()) - def get_teams(self, only_approved=False, cursor=None): + def get_teams(self, only_approved=False, only_open=False, cursor=None): """Return a list of teams this user is an owner or member of. """ teams = (cursor or self.db).all(""" - SELECT teams.*::teams FROM teams WHERE - owner=%s AND - NOT is_closed + SELECT teams.*::teams FROM teams WHERE owner=%s UNION SELECT teams.*::teams FROM teams WHERE id IN ( SELECT team_id FROM current_takes WHERE participant_id=%s ) - """, (self.username, self.id) - ) + """, (self.username, self.id)) + if only_approved: teams = [t for t in teams if t.is_approved] + if only_open: + teams = [t for t in teams if not t.is_closed] return teams diff --git a/gratipay/testing/harness.py b/gratipay/testing/harness.py index 0a5b0a56ea..2916d1c7d6 100644 --- a/gratipay/testing/harness.py +++ b/gratipay/testing/harness.py @@ -155,6 +155,8 @@ def make_team(self, *a, **kw): _kw['slug_lower'] = _kw['slug'].lower() if 'is_approved' not in _kw: _kw['is_approved'] = False + if 'is_closed' not in _kw: + _kw['is_closed'] = False if 'available' not in _kw: _kw['available'] = 0 @@ -169,9 +171,9 @@ def make_team(self, *a, **kw): team = self.db.one(""" INSERT INTO teams (slug, slug_lower, name, homepage, product_or_service, - onboarding_url, owner, is_approved, available) + onboarding_url, owner, is_approved, is_closed, available) VALUES (%(slug)s, %(slug_lower)s, %(name)s, %(homepage)s, %(product_or_service)s, - %(onboarding_url)s, %(owner)s, %(is_approved)s, + %(onboarding_url)s, %(owner)s, %(is_approved)s, %(is_closed)s, %(available)s) RETURNING teams.*::teams """, _kw) diff --git a/templates/team-listing.html b/templates/team-listing.html index 51807d940e..f688395eff 100644 --- a/templates/team-listing.html +++ b/templates/team-listing.html @@ -1,16 +1,17 @@ -{% set approved_teams = participant.get_teams(only_approved=True) %} +{% set unprivileged = not(participant == user.participant or user.ADMIN) %} +{% set approved_open_teams = participant.get_teams(unprivileged, unprivileged) %} -{% if (user.ADMIN or user.participant == participant) and approved_teams %} +{% if (user.ADMIN or user.participant == participant) and approved_open_teams %}

{{ _("Projects") }}

-{% elif approved_teams %} +{% elif approved_open_teams %}

{{ _("Projects") }}

    - {% for team in approved_teams %} + {% for team in approved_open_teams %}
  • {{ team.name }}
    • {% endfor %}
diff --git a/tests/py/test_pages.py b/tests/py/test_pages.py index 30a3b6b4c0..58431dd9a1 100644 --- a/tests/py/test_pages.py +++ b/tests/py/test_pages.py @@ -266,3 +266,42 @@ def test_your_payment_template_basically_works(self): self.make_team(is_approved=True) self.make_participant('alice') assert 'your-payment' in self.client.GET('/TheEnterprise/', auth_as='alice').body + + +class TestTeamListingTemplate(Harness): + + def setUp(self): + self.make_participant('Q', claimed_time='now', is_admin=True) + self.make_participant('Rambo', claimed_time='now') + + def check(self, auth_as=None, expected=True): + body = self.client.GET('/~picard/', auth_as=auth_as).body.decode('utf8') + assert ('The Enterprise' in body) is expected + + def test_includes_approved_open_team_for_everyone(self): + self.make_team(is_approved=True, is_closed=False) + self.check('picard') + self.check('Q') + self.check('Rambo') + self.check() + + def test_includes_approved_closed_team_for_owner_and_admin(self): + self.make_team(is_approved=True, is_closed=True) + self.check('picard') + self.check('Q') + self.check('Rambo', False) + self.check(None, False) + + def test_includes_unapproved_open_team_for_owner_and_admin(self): + self.make_team(is_approved=False, is_closed=True) + self.check('picard') + self.check('Q') + self.check('Rambo', False) + self.check(None, False) + + def test_includes_unapproved_closed_team_for_owner_and_admin(self): + self.make_team(is_approved=False, is_closed=False) + self.check('picard') + self.check('Q') + self.check('Rambo', False) + self.check(None, False) diff --git a/tests/py/test_participant.py b/tests/py/test_participant.py index bf41ed86d2..07550c8c10 100644 --- a/tests/py/test_participant.py +++ b/tests/py/test_participant.py @@ -257,11 +257,19 @@ def test_get_teams_can_get_only_approved_teams(self): self.make_team('The Stargazer', owner=picard, is_approved=False) assert [t.slug for t in picard.get_teams(only_approved=True)] == ['TheEnterprise'] + def test_get_teams_can_get_only_open_teams(self): + self.make_team() + picard = P('picard') + self.make_team('The Stargazer', owner=picard, is_closed=True) + assert [t.slug for t in picard.get_teams(only_open=True)] == ['TheEnterprise'] + def test_get_teams_can_get_all_teams(self): self.make_team(is_approved=True) picard = P('picard') self.make_team('The Stargazer', owner=picard, is_approved=False) - assert [t.slug for t in picard.get_teams()] == ['TheEnterprise', 'TheStargazer'] + self.make_team('The Trident', owner=picard, is_approved=False, is_closed=True) + assert [t.slug for t in picard.get_teams()] == \ + ['TheEnterprise', 'TheStargazer', 'TheTrident'] # giving