From 4d3a249a4c4281356c1c1628f2ad33ccc97438e0 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 1 Nov 2012 18:18:21 -0400 Subject: [PATCH] Modify payday to skip payin for suspended (#350) --- gittip/billing/payday.py | 63 ++++++++++++++++++++++------------------ gittip/participant.py | 17 +++++++++++ gittip/testing.py | 35 +++++++++++++++++----- schema.sql | 2 +- tests/test_billing.py | 54 ++++++++++++++++++++++++++++++++++ tests/test_stats.py | 15 ++++++---- tests/test_suspension.py | 44 ++++++++++++++++++++++++++++ 7 files changed, 187 insertions(+), 43 deletions(-) create mode 100644 gittip/participant.py create mode 100644 tests/test_suspension.py diff --git a/gittip/billing/payday.py b/gittip/billing/payday.py index 33d5e5409d..83ca3cface 100644 --- a/gittip/billing/payday.py +++ b/gittip/billing/payday.py @@ -77,6 +77,29 @@ def __init__(self, db): self.db = db + def genparticipants(self, ts_start, for_payday): + """Generator to yield participants with tips and total. + + We re-fetch participants each time, because the second time through + we want to use the total obligations they have for next week, and + if we pass a non-False for_payday to get_tips_and_total then we + only get unfulfilled tips from prior to that timestamp, which is + none of them by definition. + + If someone changes tips after payout starts, and we crash during + payout, then their new tips_and_total will be used on the re-run. + That's okay. + + """ + for participant in self.get_participants(ts_start): + tips, total = get_tips_and_total( participant['id'] + , for_payday=for_payday + , db=self.db + ) + typecheck(total, Decimal) + yield(participant, tips, total) + + def run(self): """This is the starting point for payday. @@ -90,35 +113,9 @@ def run(self): ts_start = self.start() self.zero_out_pending(ts_start) - def genparticipants(for_payday): - """Closure generator to yield participants with tips and total. - - We re-fetch participants each time, because the second time through - we want to use the total obligations they have for next week, and - if we pass a non-False for_payday to get_tips_and_total then we - only get unfulfilled tips from prior to that timestamp, which is - none of them by definition. - - If someone changes tips after payout starts, and we crash during - payout, then their new tips_and_total will be used on the re-run. - That's okay. - - Note that we take ts_start from the outer scope when we pass it to - get_participants, but we pass in for_payday, because we might want - it to be False (per the definition of git_tips_and_total). - - """ - for participant in self.get_participants(ts_start): - tips, total = get_tips_and_total( participant['id'] - , for_payday=for_payday - , db=self.db - ) - typecheck(total, Decimal) - yield(participant, tips, total) - - self.payin(ts_start, genparticipants(ts_start)) + self.payin(ts_start, self.genparticipants(ts_start, ts_start)) self.clear_pending_to_balance() - self.payout(ts_start, genparticipants(False)) + self.payout(ts_start, self.genparticipants(ts_start, False)) self.end() @@ -187,7 +184,11 @@ def get_participants(self, ts_start): """Given a timestamp, return a list of participants dicts. """ PARTICIPANTS = """\ - SELECT id, balance, balanced_account_uri, stripe_customer_id + SELECT id + , balance + , balanced_account_uri + , stripe_customer_id + , payin_suspended FROM participants WHERE claimed_time IS NOT NULL AND claimed_time < %s @@ -229,6 +230,10 @@ def charge_and_or_transfer(self, ts_start, participant, tips, total): money between Gittip accounts. """ + if participant['payin_suspended']: + log("PAYIN SUSPENDED: %s" % participant['id']) + return + short = total - participant['balance'] if short > 0: diff --git a/gittip/participant.py b/gittip/participant.py new file mode 100644 index 0000000000..4c4ebdba16 --- /dev/null +++ b/gittip/participant.py @@ -0,0 +1,17 @@ +from gittip import db + + +class Participant(object): + + def __init__(self, participant_id): + self.participant_id = participant_id + + def suspend_payin(self): + db.execute( "UPDATE participants SET payin_suspended=true WHERE id=%s" + , (self.participant_id,) + ) + + def unsuspend_payin(self): + db.execute( "UPDATE participants SET payin_suspended=false WHERE id=%s" + , (self.participant_id,) + ) diff --git a/gittip/testing.py b/gittip/testing.py index 373286fa7e..8b48ff3eed 100644 --- a/gittip/testing.py +++ b/gittip/testing.py @@ -2,6 +2,7 @@ """ from __future__ import unicode_literals +import datetime import copy import os import re @@ -306,16 +307,31 @@ def setup_tips(*recs): good_cc can be True, False, or None """ - data = [] tips = [] _participants = {} - for tipper, tippee, amount, good_cc in recs: + for rec in recs: + defaults = good_cc, payin_suspended, claimed = (True, False, True) + + if len(rec) == 3: + tipper, tippee, amount = rec + elif len(rec) == 4: + tipper, tippee, amount, good_cc = rec + payin_suspended, claimed = (False, True) + elif len(rec) == 5: + tipper, tippee, amount, good_cc, payin_suspended = rec + claimed = True + elif len(rec) == 6: + tipper, tippee, amount, good_cc, payin_suspended, claimed = rec + assert good_cc in (True, False, None), good_cc - _participants[tipper] = good_cc + assert payin_suspended in (True, False), payin_suspended + assert claimed in (True, False), claimed + + _participants[tipper] = (good_cc, payin_suspended, claimed) if tippee not in _participants: - _participants[tippee] = None + _participants[tippee] = defaults now = utcnow() tips.append({ "ctime": now , "mtime": now @@ -324,15 +340,20 @@ def setup_tips(*recs): , "amount": Decimal(amount) }) + then = utcnow() - datetime.timedelta(seconds=3600) + participants = [] - for participant_id, good_cc in _participants.items(): + for participant_id, (good_cc, payin_suspended, claimed) in _participants.items(): rec = {"id": participant_id} if good_cc is not None: rec["last_bill_result"] = "" if good_cc else "Failure!" + rec["balanced_account_uri"] = "/v1/blah/blah/" + participant_id + rec["payin_suspended"] = payin_suspended + if claimed: + rec["claimed_time"] = then participants.append(rec) - data = ["participants"] + participants + ["tips"] + tips - return load(*data) + return ["participants"] + participants + ["tips"] + tips # Helpers for testing simplates. diff --git a/schema.sql b/schema.sql index 96b1cc13f7..5c61327899 100644 --- a/schema.sql +++ b/schema.sql @@ -200,4 +200,4 @@ ALTER TABLE participants ALTER COLUMN balance SET NOT NULL; ------------------------------------------------------------------------------- -- https://github.com/whit537/www.gittip.com/issues/350 -ALTER TABLE participants ADD COLUMN suspended bool NOT NULL DEFAULT FALSE; +ALTER TABLE participants ADD COLUMN payin_suspended bool NOT NULL DEFAULT FALSE; diff --git a/tests/test_billing.py b/tests/test_billing.py index 521a69a1d9..2e0e1371e1 100644 --- a/tests/test_billing.py +++ b/tests/test_billing.py @@ -380,6 +380,57 @@ def test_charge_success_touches_a_few_tables(hb): actual = context.diff(compact=True) assert actual == expected, actual + +@mock.patch('gittip.billing.payday.Payday.charge_on_balanced') +def test_payday_does_stuff(charge_on_balanced): + charge_on_balanced.return_value = (Decimal('10.00'), Decimal('0.68'), None) + tips = testing.setup_tips(('buz', 'bar', '6.00', True)) # under $10! + with testing.load(*tips) as context: + Payday(context.db).run() + expected = { 'exchanges': [1, 0, 0] + , 'participants': [0, 2, 0] + , 'paydays': [1, 0, 0] + , 'transfers': [1, 0, 0] + } + actual = context.diff(compact=True) + assert actual == expected, actual + +@mock.patch('gittip.billing.payday.Payday.charge_on_balanced') +def test_payday_moves_money(charge_on_balanced): + charge_on_balanced.return_value = (Decimal('10.00'), Decimal('0.68'), None) + tips = testing.setup_tips(('buz', 'bar', '6.00', True)) # under $10! + with testing.load(*tips) as context: + Payday(context.db).run() + expected = [ {"id": "buz", "balance": Decimal('3.32')} + , {"id": "bar", "balance": Decimal('6.00')} + ] + actual = context.diff()['participants']['updates'] + assert actual == expected, actual + +@mock.patch('gittip.billing.payday.Payday.charge_on_balanced') +def test_payday_doesnt_move_money_from_a_suspended_payin_account(charge_on_balanced): + charge_on_balanced.return_value = (Decimal('10.00'), Decimal('0.68'), None) + tips = testing.setup_tips(('buz', 'bar', '6.00', True, True)) # under $10! + with testing.load(*tips) as context: + Payday(context.db).run() + actual = context.diff(compact=True) + assert actual == {"paydays": [1,0,0]}, actual + +@mock.patch('gittip.billing.payday.Payday.charge_on_balanced') +def test_payday_does_move_money_TO_a_suspended_payin_account(charge_on_balanced): + charge_on_balanced.return_value = (Decimal('10.00'), Decimal('0.68'), None) + tips = testing.setup_tips( ('buz', 'bar', '6.00', True, True) + , ('foo', 'buz', '1.00') + ) # under $10! + with testing.load(*tips) as context: + Payday(context.db).run() + expected = [ {"id": "buz", "balance": Decimal('1.00')} + , {"id": "foo", "balance": Decimal('8.32')} + ] + actual = context.diff()['participants']['updates'] + assert actual == expected, actual + + # XXX I started refactoring billing tests out of test classes into module-level # functions + context managers, and this is as far as I got. @@ -680,6 +731,7 @@ def test_charge_and_or_transfer_no_tips(self, get_tips_and_total): participant = { 'balance': 1 , 'id': self.participant_id , 'balanced_account_uri': self.balanced_account_uri + , 'payin_suspended': False } initial_payday = self._get_payday() @@ -711,6 +763,7 @@ def test_charge_and_or_transfer(self, tip, get_tips_and_total): participant = { 'balance': 1 , 'id': self.participant_id , 'balanced_account_uri': self.balanced_account_uri + , 'payin_suspended': False } return_values = [1, 1, 0, -1] @@ -750,6 +803,7 @@ def test_charge_and_or_transfer_short(self, charge, get_tips_and_total): participant = { 'balance': 0 , 'id': self.participant_id , 'balanced_account_uri': self.balanced_account_uri + , 'payin_suspended': False } diff --git a/tests/test_stats.py b/tests/test_stats.py index 0f51b4529c..8a73ad164f 100644 --- a/tests/test_stats.py +++ b/tests/test_stats.py @@ -23,12 +23,15 @@ def test_commaize_commaizes_and_obeys_decimal_places(): # chart of giving +def setup(*a): + return testing.load(*testing.setup_tips(*a)) + def test_get_chart_of_giving_handles_a_tip(): tip = ("foo", "bar", "3.00", True) expected = ( [[Decimal('3.00'), 1, Decimal('3.00'), 1.0, Decimal('1')]] , 1.0, Decimal('3.00') ) - with testing.setup_tips(tip): + with setup(tip): actual = gittip.get_chart_of_giving('bar') assert actual == expected, actual @@ -37,13 +40,13 @@ def test_get_chart_of_giving_handles_a_non_standard_amount(): expected = ( [[-1, 1, Decimal('5.37'), 1.0, Decimal('1')]] , 1.0, Decimal('5.37') ) - with testing.setup_tips(tip): + with setup(tip): actual = gittip.get_chart_of_giving('bar') assert actual == expected, actual def test_get_chart_of_giving_handles_no_tips(): expected = ([], 0.0, Decimal('0.00')) - with testing.setup_tips(): + with setup(): actual = gittip.get_chart_of_giving('foo') assert actual == expected, actual @@ -56,7 +59,7 @@ def test_get_chart_of_giving_handles_multiple_tips(): ] , 2.0, Decimal('4.00') ) - with testing.setup_tips(*tips): + with setup(*tips): actual = gittip.get_chart_of_giving('bar') assert actual == expected, actual @@ -67,7 +70,7 @@ def test_get_chart_of_giving_ignores_bad_cc(): expected = ( [[Decimal('1.00'), 1L, Decimal('1.00'), 1, Decimal('1')]] , 1.0, Decimal('1.00') ) - with testing.setup_tips(*tips): + with setup(*tips): actual = gittip.get_chart_of_giving('bar') assert actual == expected, actual @@ -78,7 +81,7 @@ def test_get_chart_of_giving_ignores_missing_cc(): expected = ( [[Decimal('1.00'), 1L, Decimal('1.00'), 1, Decimal('1')]] , 1.0, Decimal('1.00') ) - with testing.setup_tips(*tips): + with setup(*tips): actual = gittip.get_chart_of_giving('bar') assert actual == expected, actual diff --git a/tests/test_suspension.py b/tests/test_suspension.py new file mode 100644 index 0000000000..ae4a3d6ce5 --- /dev/null +++ b/tests/test_suspension.py @@ -0,0 +1,44 @@ +from gittip.testing import load +from gittip.participant import Participant + + +def test_participants_start_out_unpayin_suspended(): + with load('participants', ('foo',)) as context: + actual = context.dump()['participants']['foo']['payin_suspended'] + assert actual is False, actual + +def test_suspend_suspends(): + with load('participants', ('foo',)) as context: + Participant('foo').suspend_payin() + actual = context.diff()['participants']['updates'][0]['payin_suspended'] + assert actual is True, actual + +def test_suspend_changes_one_thing_only(): + with load('participants', ('foo',)) as context: + Participant('foo').suspend_payin() + actual = context.diff(compact=True) + assert actual == {'participants': [0,1,0]}, actual + +def test_suspend_is_a_noop_when_payin_suspended(): + with load('participants', {'id': 'foo', 'payin_suspended': True}) as context: + Participant('foo').suspend_payin() + actual = context.diff(compact=True) + assert actual == {}, actual + +def test_unsuspend_is_a_noop_when_not_payin_suspended(): + with load('participants', ('foo',)) as context: + Participant('foo').unsuspend_payin() + actual = context.diff(compact=True) + assert actual == {}, actual + +def test_unsuspend_unsuspends(): + with load('participants', {'id': 'foo', 'payin_suspended': True}) as context: + Participant('foo').unsuspend_payin() + actual = context.diff()['participants']['updates'][0]['payin_suspended'] + assert actual is False, actual + +def test_unsuspend_changes_one_thing_only(): + with load('participants', {'id': 'foo', 'payin_suspended': True}) as context: + Participant('foo').unsuspend_payin() + actual = context.diff(compact=True) + assert actual == {'participants': [0,1,0]}, actual