This repository has been archived by the owner on Feb 8, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 308
Show as closed #4292
Merged
Merged
Show as closed #4292
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
22c2d8e
Improve logic in account closing
chadwhitacre 02bc546
Rationalize danger-zone a bit
chadwhitacre 2fcdd1b
Add "closed" notice to project pages
chadwhitacre f985bd1
Update test suite
chadwhitacre 0d2deb6
Test the thing we said we were testing
chadwhitacre File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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') | ||
|
@@ -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')) | ||
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) | ||
|
@@ -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') | ||
|
@@ -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): | ||
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(*) " | ||
|
@@ -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')) | ||
|
@@ -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,)) | ||
|
@@ -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')) | ||
|
@@ -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' | ||
|
@@ -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') | ||
|
@@ -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'}) | ||
|
@@ -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: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)