Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Show as closed #4292

Merged
merged 5 commits into from
Jan 14, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions gratipay/models/participant/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,13 @@ def set_as_claimed(self):
# Closing
# =======

def close(self):
def close(self, require_zero_balance=True):
"""Close the participant's account.
"""
with self.db.get_cursor() as cursor:
self.clear_payment_instructions(cursor)
self.clear_personal_information(cursor)
self.final_check(cursor)
self.final_check(cursor, require_zero_balance)
self.update_is_closed(True, cursor)

def update_is_closed(self, is_closed, cursor=None):
Expand Down Expand Up @@ -1195,12 +1195,12 @@ def get_age_in_seconds(self):
class StillOnATeam(Exception): pass
class BalanceIsNotZero(Exception): pass

def final_check(self, cursor):
def final_check(self, cursor, require_zero_balance=True):
"""Sanity-check that teams and balance have been dealt with.
"""
if self.get_teams(cursor=cursor, only_open=True):
raise self.StillOnATeam
if self.balance != 0:
if require_zero_balance and self.balance != 0:
raise self.BalanceIsNotZero

def archive(self, cursor):
Expand Down
9 changes: 8 additions & 1 deletion scss/components/danger-zone.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
.danger-zone {
margin-top: 64px;
margin: 1em 0 1em;
&.more-top-margin {
margin-top: 4em;
}
&.less-bottom-margin {
margin-bottom: 0;
}
border: 1px solid $red;
@include border-radius(5px);
padding: 20px;
color: $red;
h2 {
margin: 0 0 10px;
padding: 0;
Expand Down
127 changes: 100 additions & 27 deletions tests/py/test_close.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
from gratipay.testing import Harness, D,P


class TestClosing(Harness):

# close
class TestClose(Harness):

def test_close_closes(self):
alice = self.make_participant('alice', claimed_time='now')
Expand All @@ -25,6 +23,16 @@ def test_close_fails_if_still_a_balance(self):
with pytest.raises(alice.BalanceIsNotZero):
alice.close()

def test_close_can_be_overriden_for_balance_though(self):
alice = self.make_participant('alice', claimed_time='now', balance=D('10.00'))
alice.close(require_zero_balance=False)
assert P('alice').is_closed

def test_close_can_be_overriden_for_negative_balance_too(self):
alice = self.make_participant('alice', claimed_time='now', balance=D('10.00'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-10.00?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Fixed in 0d2deb6.

alice.close(require_zero_balance=False)
assert P('alice').is_closed

def test_close_fails_if_still_owns_a_team(self):
alice = self.make_participant('alice', claimed_time='now')
self.make_team(owner=alice)
Expand All @@ -37,17 +45,19 @@ def test_close_succeeds_if_team_is_closed(self):
alice.close()
assert P('alice').is_closed


class TestClosePage(Harness):

def test_close_page_is_usually_available(self):
self.make_participant('alice', claimed_time='now')
body = self.client.GET('/~alice/settings/close', auth_as='alice').body
assert 'Personal Information' in body

def test_close_page_is_not_available_during_payday(self):
Payday.start()
self.make_participant('alice', claimed_time='now')
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 'Personal Information' not in body
assert 'Try Again Later' in body
assert 'Please close the following projects first:' in body

def test_can_post_to_close_page(self):
self.make_participant('alice', claimed_time='now')
Expand All @@ -56,22 +66,85 @@ def test_can_post_to_close_page(self):
assert response.headers['Location'] == '/~alice/'
assert P('alice').is_closed


# balance checks

def test_close_page_is_not_available_to_owner_with_positive_balance(self):
self.make_participant('alice', claimed_time='now', last_paypal_result='', balance=10)
body = self.client.GET('/~alice/settings/close', auth_as='alice').body
assert 'Personal Information' not in body
assert 'You have a balance' in body

def test_close_page_is_not_available_to_owner_with_negative_balance(self):
self.make_participant('alice', claimed_time='now', last_paypal_result='', balance=-10)
body = self.client.GET('/~alice/settings/close', auth_as='alice').body
assert 'Personal Information' not in body
assert 'You have a balance' in body

def test_sends_owner_with_balance_and_no_paypal_on_quest_for_paypal(self):
self.make_participant('alice', claimed_time='now', balance=10)
body = self.client.GET('/~alice/settings/close', auth_as='alice').body
assert 'Personal Information' not in body
assert 'You have a balance' not in body
assert 'link a PayPal account' in body

def test_but_close_page_is_available_to_admin_even_with_positive_balance(self):
self.make_participant('admin', claimed_time='now', is_admin=True)
self.make_participant('alice', claimed_time='now', balance=10)
body = self.client.GET('/~alice/settings/close', auth_as='admin').body
assert 'Personal Information' in body
assert 'Careful!' in body

def test_and_close_page_is_available_to_admin_even_with_negative_balance(self):
self.make_participant('admin', claimed_time='now', is_admin=True)
self.make_participant('alice', claimed_time='now', balance=-10)
body = self.client.GET('/~alice/settings/close', auth_as='admin').body
assert 'Personal Information' in body
assert 'Careful!' in body

def test_posting_with_balance_fails_for_owner(self):
self.make_participant('alice', claimed_time='now', balance=10)
response = self.client.PxST('/~alice/settings/close', auth_as='alice')
assert response.code == 400
assert not P('alice').is_closed

def test_posting_with_balance_succeeds_for_admin(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append "_with_negative_balance" in method name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh. We have tests above for the full matrix of {positive,negative} * {owner,admin}. The point here is to test with any balance. The fact that this one is negative while the previous test uses a positive balance is just extra.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because here we're running through the simplate, where the positive/negative isn't so much an issue as the permissions for various roles.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a principle in testing that making test case mimimal/simplest to achieve the test goal. Any additional conditions would add noise and unnecessery maintanance burden (consider other contributors having the same confusion as me).

Of course that's in theory :)

self.make_participant('admin', claimed_time='now', is_admin=True)
self.make_participant('alice', claimed_time='now', balance=-10)
response = self.client.PxST('/~alice/settings/close', auth_as='admin')
assert response.code == 302
assert response.headers['Location'] == '/~alice/'
assert P('alice').is_closed


# payday check

def check_under_payday(self, username):
body = self.client.GET('/~alice/settings/close', auth_as=username).body
assert 'Personal Information' not in body
assert 'Try Again Later' in body

def test_close_page_is_not_available_during_payday(self):
self.make_participant('alice', claimed_time='now')
Payday.start()
self.check_under_payday('alice')

def test_even_for_admin(self):
self.make_participant('admin', claimed_time='now', is_admin=True)
self.make_participant('alice', claimed_time='now')
Payday.start()
self.check_under_payday('admin')

def test_cant_post_to_close_page_during_payday(self):
Payday.start()
self.make_participant('alice', claimed_time='now')
body = self.client.POST('/~alice/settings/close', auth_as='alice').body
assert 'Try Again Later' in body

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 'Please close the following projects first:' in body


# cpi - clear_payment_instructions
class TestClearPaymentInstructions(Harness):

def test_cpi_clears_payment_instructions(self):
def test_clears_payment_instructions(self):
alice = self.make_participant('alice', claimed_time='now', last_bill_result='')
alice.set_payment_instruction(self.make_team(), D('1.00'))
npayment_instructions = lambda: self.db.one( "SELECT count(*) "
Expand All @@ -84,7 +157,7 @@ def test_cpi_clears_payment_instructions(self):
alice.clear_payment_instructions(cursor)
assert npayment_instructions() == 0

def test_cpi_doesnt_duplicate_zero_payment_instructions(self):
def test_doesnt_duplicate_zero_payment_instructions(self):
alice = self.make_participant('alice', claimed_time='now')
A = self.make_team()
alice.set_payment_instruction(A, D('1.00'))
Expand All @@ -96,7 +169,7 @@ def test_cpi_doesnt_duplicate_zero_payment_instructions(self):
alice.clear_payment_instructions(cursor)
assert npayment_instructions() == 2

def test_cpi_doesnt_zero_when_theres_no_payment_instruction(self):
def test_doesnt_zero_when_theres_no_payment_instruction(self):
alice = self.make_participant('alice')
npayment_instructions = lambda: self.db.one("SELECT count(*) FROM payment_instructions "
"WHERE participant_id=%s", (alice.id,))
Expand All @@ -105,7 +178,7 @@ def test_cpi_doesnt_zero_when_theres_no_payment_instruction(self):
alice.clear_payment_instructions(cursor)
assert npayment_instructions() == 0

def test_cpi_clears_multiple_payment_instructions(self):
def test_clears_multiple_payment_instructions(self):
alice = self.make_participant('alice', claimed_time='now')
alice.set_payment_instruction(self.make_team('A'), D('1.00'))
alice.set_payment_instruction(self.make_team('B'), D('1.00'))
Expand All @@ -123,10 +196,10 @@ def test_cpi_clears_multiple_payment_instructions(self):
assert npayment_instructions() == 0


# cpi - clear_personal_information
class TestClearPersonalInformation(Harness):

@mock.patch.object(Participant, '_mailer')
def test_cpi_clears_personal_information(self, mailer):
def test_clears_personal_information(self, mailer):
alice = self.make_participant( 'alice'
, anonymous_giving=True
, avatar_url='img-url'
Expand Down Expand Up @@ -155,7 +228,7 @@ def test_cpi_clears_personal_information(self, mailer):
assert alice.session_expires.year == new_alice.session_expires.year == date.today().year
assert not alice.get_emails()

def test_cpi_clears_communities(self):
def test_clears_communities(self):
alice = self.make_participant('alice')
alice.insert_into_communities(True, 'test', 'test')
bob = self.make_participant('bob')
Expand All @@ -168,7 +241,7 @@ def test_cpi_clears_communities(self):

assert Community.from_slug('test').nmembers == 1

def test_cpi_clears_personal_identities(self):
def test_clears_personal_identities(self):
alice = self.make_participant('alice', email_address='[email protected]')
US = self.db.one("SELECT id FROM countries WHERE code='US'")
alice.store_identity_info(US, 'nothing-enforced', {'name': 'Alice'})
Expand All @@ -182,24 +255,24 @@ def test_cpi_clears_personal_identities(self):
assert self.db.one('SELECT count(*) FROM participant_identities;') == 0


# uic = update_is_closed
class TestUpdateIsClosed(Harness):

def test_uic_updates_is_closed(self):
def test_updates_is_closed(self):
alice = self.make_participant('alice')
alice.update_is_closed(True)

assert alice.is_closed
assert P('alice').is_closed

def test_uic_updates_is_closed_False(self):
def test_updates_is_closed_False(self):
alice = self.make_participant('alice')
alice.update_is_closed(True)
alice.update_is_closed(False)

assert not alice.is_closed
assert not P('alice').is_closed

def test_uic_uses_supplied_cursor(self):
def test_uses_supplied_cursor(self):
alice = self.make_participant('alice')

with self.db.get_cursor() as cursor:
Expand Down
3 changes: 2 additions & 1 deletion www/%team/edit/index.html.spt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ suppress_sidebar = True
<button onclick="window.location='../';return false;">{{ _("Cancel") }}</button>

</form>
<form action="close" method="POST" class="danger-zone" id="close-team">
<form action="close" method="POST" class="danger-zone more-top-margin less-bottom-margin"
id="close-team">
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
<h2>{{ _("Danger Zone") }}</h2>
<button type="submit">{{ _("Close Project") }}</button>
Expand Down
10 changes: 10 additions & 0 deletions www/%team/index.html.spt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ is_team_owner = not user.ANON and team.owner == user.participant.username

</p>

{% if team.is_closed %}
<div class="danger-zone">
<h2>{{ _("Project Closed") }}</h2>
{{ _( 'Please {a}email support{_a} to reopen.'
, a='<a href="mailto:[email protected]">'|safe
, _a='</a>'|safe
) }}
</div>
{% endif %}

<div class="statement profile-statement">
{{ markdown.render(team.product_or_service) }}
</div>
Expand Down
2 changes: 1 addition & 1 deletion www/~/%username/identities/%country.spt
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ if request.method == 'POST':
onclick="javascript: window.location='./'" formnovalidate>Cancel</button>

{% if identity != None %}
<div class="danger-zone">
<div class="danger-zone more-top-margin less-bottom-margin">
<h2>{{ _("Danger Zone") }}</h2>
<button type="submit"
name="action" value="remove">{{ _("Remove This Identity") }}</button>
Expand Down
29 changes: 21 additions & 8 deletions www/~/%username/settings/close.spt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from aspen import Response
from gratipay.utils import get_participant, to_javascript
[---]
participant = get_participant(state, restrict=True)
Expand All @@ -14,17 +15,21 @@ if request.method == 'POST':
if payday_is_running:
pass # User will get the "Try Again Later" message.
else:
participant.close()
try:
participant.close(require_zero_balance=not user.ADMIN)
except (participant.StillOnATeam, participant.BalanceIsNotZero):
raise Response(400)
website.redirect('/~%s/' % participant.username)
teams = website.db.all( "SELECT teams.*::teams FROM teams WHERE owner=%s AND NOT is_closed"
, (participant.username,)
)
balance_okay = (participant.balance == 0) or user.ADMIN
[---] text/html
{% extends "templates/base.html" %}

{% block content %}

{% if teams or participant.balance > 0 %}
{% if teams or not balance_okay %}

{% if teams %}

Expand All @@ -37,7 +42,7 @@ teams = website.db.all( "SELECT teams.*::teams FROM teams WHERE owner=%s AND NOT

{% endif %}

{% if participant.balance > 0 %}
{% if not balance_okay %}
{% if participant.has_payout_route %}

<p>You have a balance of <b>${{ participant.balance }}</b>, and you've
Expand Down Expand Up @@ -68,6 +73,12 @@ teams = website.db.all( "SELECT teams.*::teams FROM teams WHERE owner=%s AND NOT
an hour.</p>

{% else %}
{% if participant.balance != 0 %}
<div class="danger-zone">
<h2>Careful!</h2>
~{{ participant.username }} still has a balance of {{ participant.balance }}.
</div>
{% endif %}
<form method="POST">

<h2>Personal Information</h2>
Expand All @@ -83,7 +94,7 @@ teams = website.db.all( "SELECT teams.*::teams FROM teams WHERE owner=%s AND NOT

<p>We specifically <i>don't</i> 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
equally to other users (the owners of the projects you gave to and
took from).</p>

<p>After you close your account, your profile page will say,
Expand All @@ -109,11 +120,13 @@ teams = website.db.all( "SELECT teams.*::teams FROM teams WHERE owner=%s AND NOT
<p>We have no control over content indexed by search engines
like Google.</p>

<h2>Ready?</h2>
<div class="danger-zone less-bottom-margin">
<h2>Ready?</h2>

<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
<button class="selected larger close-account" id="save"
type="submit">Yes, Close My Gratipay Account</button>
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
<button class="selected larger close-account" id="save"
type="submit">Yes, Close My Gratipay Account</button>
</div>
</form>
{% endif %}
{% endblock %}
Expand Down
Loading