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

Commit

Permalink
Put off whitelisting as long as possible (#354)
Browse files Browse the repository at this point in the history 
This makes logging nicer. Now we know if we see an UNREVIEWED then we
otherwise would have actually charged/credited them.
  • Loading branch information
@chadwhitacre
chadwhitacre committed Nov 8, 2012
1 parent f8432c8 commit 624caeb
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 105 deletions.
58 changes: 40 additions & 18 deletions gittip/billing/payday.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from aspen.utils import typecheck
from gittip import get_tips_and_total
from psycopg2 import IntegrityError
from psycopg2.extras import RealDictRow


# Set fees and minimums.
Expand Down Expand Up @@ -61,7 +62,7 @@ def skim_credit(amount):
assert upcharge(MINIMUM_CHARGE) == (Decimal('10.00'), Decimal('0.68'))


def whitelist(participant):
def is_whitelisted(participant):
"""Given a dict, return bool, possibly logging.
We only perform credit card charges and bank deposits for whitelisted
Expand Down Expand Up @@ -235,8 +236,7 @@ def payout(self, ts_start, participants):
for i, (participant, tips, total) in enumerate(participants, start=1):
if i % 100 == 0:
log("Payout done for %d participants." % i)
if whitelist(participant):
self.ach_credit(ts_start, participant, tips, total)
self.ach_credit(ts_start, participant, tips, total)
log("Did payout for %d participants." % i)


Expand All @@ -248,7 +248,7 @@ def charge_and_or_transfer(self, ts_start, participant, tips, total):
"""
short = total - participant['balance']
if short > 0 and whitelist(participant):
if short > 0:

# The participant's Gittip account is short the amount needed to
# fund all their tips. Let's try pulling in money from their credit
Expand All @@ -257,11 +257,7 @@ def charge_and_or_transfer(self, ts_start, participant, tips, total):
# at least *some* tips. The charge method will have set
# last_bill_result to a non-empty string if the card did fail.

self.charge( participant['id']
, participant['balanced_account_uri']
, participant['stripe_customer_id']
, short
)
self.charge(participant, short)

nsuccessful_tips = 0
for tip in tips:
Expand Down Expand Up @@ -428,22 +424,39 @@ def credit_participant(self, cursor, participant, amount):
# Move money between Gittip and the outside world.
# ================================================

def charge(self, participant_id, balanced_account_uri, stripe_customer_id, amount):
"""Given three unicodes and a Decimal, return a boolean.
def charge(self, participant, amount):
"""Given dict and Decimal, return None.
This is the only place where we actually charge credit cards. Amount
should be the nominal amount. We'll compute Gittip's fee below this
function and add it to amount to end up with charge_amount.
"""
typecheck(participant, RealDictRow, amount, Decimal)

participant_id = participant['id']
balanced_account_uri = participant['balanced_account_uri']
stripe_customer_id = participant['stripe_customer_id']

typecheck( participant_id, unicode
, balanced_account_uri, (unicode, None)
, amount, Decimal
, stripe_customer_id, (unicode, None)
)


# Perform some last-minute checks.
# ================================

if balanced_account_uri is None and stripe_customer_id is None:
self.mark_missing_funding()
return False
return # Participant has no funding source.

if not is_whitelisted(participant):
return # Participant not trusted.


# Go to Balanced or Stripe.
# =========================

if balanced_account_uri is not None:
things = self.charge_on_balanced( participant_id
Expand All @@ -469,8 +482,6 @@ def charge(self, participant_id, balanced_account_uri, stripe_customer_id, amoun
, participant_id
)

return not bool(error) # True indicates success


def ach_credit(self, ts_start, participant, tips, total):

Expand All @@ -481,10 +492,14 @@ def ach_credit(self, ts_start, participant, tips, total):

balance = participant['balance']
assert balance is not None, balance # sanity check

amount = balance - total


# Do some last-minute checks.
# ===========================

if amount <= 0:
return # Participant not owed anything.
return # Participant not owed anything.

if amount < MINIMUM_CREDIT:
also_log = ""
Expand All @@ -493,7 +508,14 @@ def ach_credit(self, ts_start, participant, tips, total):
also_log %= (balance, total)
log("Minimum payout is $%s. %s is only due $%s%s."
% (MINIMUM_CREDIT, participant['id'], amount, also_log))
return # Participant owed too little.
return # Participant owed too little.

if not is_whitelisted(participant):
return # Participant not trusted.


# Do final calculations.
# ======================

credit_amount, fee = skim_credit(amount)
cents = credit_amount * 100
Expand Down
11 changes: 8 additions & 3 deletions gittip/testing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,16 @@ def start_payday(*data):
def setup_tips(*recs):
"""Setup some participants and tips. recs is a list of:
("tipper", "tippee", '2.00', False)
^
("tipper", "tippee", '2.00', True, False, True)
^ ^ ^
| | |
| | -- claimed?
| -- is_suspicious?
|-- good cc?
good_cc can be True, False, or None
is_suspicious can be True, False, or None
claimed can be True or False
"""
tips = []
Expand All @@ -327,7 +332,7 @@ def setup_tips(*recs):
tipper, tippee, amount, good_cc, is_suspicious, claimed = rec

assert good_cc in (True, False, None), good_cc
assert is_suspicious in (True, False), is_suspicious
assert is_suspicious in (True, False, None), is_suspicious
assert claimed in (True, False), claimed

_participants[tipper] = (good_cc, is_suspicious, claimed)
Expand Down
128 changes: 44 additions & 84 deletions tests/test_billing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,50 +312,58 @@ def get_numbers(context):
return [v for k,v in sorted(payday.items()) if k.startswith('n')]


def test_charge_without_cc_details_returns_False():
def test_charge_without_cc_details_returns_None():
with testing.start_payday("participants", ("foo",)) as context:
assert not context.payday.charge('foo', None, None, Decimal('1.00'))
participant = context.db.fetchone("SELECT * FROM participants")
actual = context.payday.charge(participant, Decimal('1.00'))
assert actual is None, actual

def test_charge_without_cc_marked_as_failure():
with testing.start_payday("participants", ("foo",)) as context:
context.payday.charge('foo', None, None, Decimal('1.00'))
participant = context.db.fetchone("SELECT * FROM participants")
context.payday.charge(participant, Decimal('1.00'))
actual = get_numbers(context)
assert actual == [0, 0, 1, 0, 0, 0, 0, 0], actual

@mock.patch('gittip.billing.payday.Payday.charge_on_balanced')
def test_charge_failure_returns_False(cob):
data = ("participants", {"id": "foo", "last_bill_result": "failure"})
def test_charge_failure_returns_None(cob):
data = ("participants", { "id": "foo"
, "last_bill_result": "failure"
, "balanced_account_uri": balanced_account_uri
, "stripe_customer_id": STRIPE_CUSTOMER_ID
, "is_suspicious": False
})
with testing.start_payday(*data) as context:
participant = context.db.fetchone("SELECT * FROM participants")
cob.return_value = (Decimal('10.00'), Decimal('0.68'), 'FAILED')
actual = context.payday.charge('foo'
, balanced_account_uri
, STRIPE_CUSTOMER_ID
, Decimal('1.00')
)
assert actual is False, actual
actual = context.payday.charge(participant, Decimal('1.00'))
assert actual is None, actual

@mock.patch('gittip.billing.payday.Payday.charge_on_balanced')
def test_charge_success_returns_True(hb):
data = ("participants", {"id": "foo", "last_bill_result": "failure"})
def test_charge_success_returns_None(hb):
data = ("participants", { "id": "foo"
, "last_bill_result": "failure"
, "balanced_account_uri": balanced_account_uri
, "stripe_customer_id": STRIPE_CUSTOMER_ID
, "is_suspicious": False
})
with testing.start_payday(*data) as context:
participant = context.db.fetchone("SELECT * FROM participants")
hb.return_value = (Decimal('10.00'), Decimal('0.68'), None)
actual = context.payday.charge( 'foo'
, balanced_account_uri
, STRIPE_CUSTOMER_ID
, Decimal('1.00')
)
assert actual is True, actual
actual = context.payday.charge(participant, Decimal('1.00'))
assert actual is None, actual

@mock.patch('gittip.billing.payday.Payday.charge_on_balanced')
def test_charge_success_updates_participant(hb):
data = ("participants", {"id": "foo", "last_bill_result": "failure"})
def test_charge_success_updates_participant(charge_on_balanced):
charge_on_balanced.return_value = (Decimal('10.00'), Decimal('0.68'), None)
data = ("participants", { "id": "foo"
, "balanced_account_uri": balanced_account_uri
, "last_bill_result": "failure"
, "is_suspicious": False
})
with testing.start_payday(*data) as context:
hb.return_value = (Decimal('10.00'), Decimal('0.68'), None)
context.payday.charge( 'foo'
, balanced_account_uri
, STRIPE_CUSTOMER_ID
, Decimal('1.00')
)
participant = context.db.fetchone("SELECT * FROM participants")
context.payday.charge(participant, Decimal('1.00'))
expected = [{ 'id': 'foo'
, 'balance': Decimal('9.32')
, 'last_bill_result': ''
Expand All @@ -364,15 +372,16 @@ def test_charge_success_updates_participant(hb):
assert actual == expected, actual

@mock.patch('gittip.billing.payday.Payday.charge_on_balanced')
def test_charge_success_touches_a_few_tables(hb):
data = ("participants", {"id": "foo", "last_bill_result": "failure"})
def test_charge_success_touches_a_few_tables(charge_on_balanced):
charge_on_balanced.return_value = (Decimal('10.00'), Decimal('0.68'), None)
data = ("participants", { "id": "foo"
, "balanced_account_uri": balanced_account_uri
, "last_bill_result": "failure"
, "is_suspicious": False
})
with testing.start_payday(*data) as context:
hb.return_value = (Decimal('10.00'), Decimal('0.68'), None)
context.payday.charge( 'foo'
, balanced_account_uri
, STRIPE_CUSTOMER_ID
, Decimal('1.00')
)
participant = context.db.fetchone("SELECT * FROM participants")
context.payday.charge(participant, Decimal('1.00'))
expected = { "exchanges": [1,0,0]
, "participants": [0,1,0]
, "paydays": [1,0,0]
Expand Down Expand Up @@ -453,19 +462,6 @@ def setUp(self):
'''
self.db.execute(insert)

@mock.patch('gittip.billing.payday.Payday.charge_on_balanced')
@mock.patch('gittip.billing.payday.Payday.mark_charge_success')
def test_charge_success(self, ms, hb):
hb.return_value = (Decimal('10.00'), Decimal('0.68'), None)
result = self.payday.charge( self.participant_id
, self.balanced_account_uri
, self.stripe_customer_id
, Decimal('1.00')
)
self.assertEqual(hb.call_count, 1)
self.assertEqual(ms.call_count, 1)
self.assertTrue(result)

def test_mark_missing_funding(self):
query = '''
select ncc_missing
Expand Down Expand Up @@ -700,42 +696,6 @@ def _get_payday(self):
'''
return self.db.fetchone(SELECT_PAYDAY)

@staticmethod
def genparticipants():
for i in range(100):
yield { 'is_suspicious': False
, 'id': 'something'
, 'balance': Decimal('0.00')
, 'balanced_account_uri': ''
, 'stripe_customer_id': ''
}, [], Decimal('2.00')
yield { 'is_suspicious': None
, 'id': 'something'
, 'balance': Decimal('0.00')
, 'balanced_account_uri': ''
, 'stripe_customer_id': ''
}, [], Decimal('2.00')

@mock.patch('gittip.billing.payday.log')
@mock.patch('gittip.billing.payday.Payday.charge')
@mock.patch('gittip.billing.payday.Payday.tip')
def test_payin(self, tip, charge, log):
start = mock.Mock()
self.payday.payin(start, self.genparticipants())
self.assertEqual(log.call_count, 4)
self.assertEqual(charge.call_count, 100)
self.assertTrue(charge.called_with(start))

@mock.patch('gittip.billing.payday.log')
@mock.patch('gittip.billing.payday.Payday.ach_credit')
def test_payout(self, ach_credit, log):
start = mock.Mock()
self.payday.payout(start, self.genparticipants())

self.assertEqual(log.call_count, 4)
self.assertEqual(ach_credit.call_count, 100)
self.assertTrue(ach_credit.called_with(start))

def test_assert_one_payday(self):
with self.assertRaises(AssertionError):
self.payday.assert_one_payday(None)
Expand Down

0 comments on commit 624caeb

Please sign in to comment.