From f10e1c7ae914784a9f69a1b7d54c0e84aebf008d Mon Sep 17 00:00:00 2001 From: Kwesi Aguillera Date: Thu, 2 Mar 2017 13:15:04 -0500 Subject: [PATCH 1/6] Added ref parameter to the record_exchange def Tests should fail --- gratipay/billing/exchanges.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/gratipay/billing/exchanges.py b/gratipay/billing/exchanges.py index cba53fd7a5..b5fab07bd0 100644 --- a/gratipay/billing/exchanges.py +++ b/gratipay/billing/exchanges.py @@ -98,7 +98,8 @@ def create_card_hold(db, participant, amount): log(msg + "succeeded.") else: log(msg + "failed: %s" % error) - record_exchange(db, route, amount, fee, participant, 'failed', error) + ref = result.transaction.id + record_exchange(db, route, amount, fee, participant, 'failed', error, ref) return hold, error @@ -118,14 +119,15 @@ def capture_card_hold(db, participant, amount, hold): cents, amount_str, charge_amount, fee = _prep_hit(amount) amount = charge_amount - fee # account for possible rounding - e_id = record_exchange(db, route, amount, fee, participant, 'pre') + ref = hold.id + e_id = record_exchange(db, route, amount, fee, participant, 'pre', ref) # TODO: Find a way to link transactions and corresponding exchanges # meta = dict(participant_id=participant.id, exchange_id=e_id) error = '' try: - result = braintree.Transaction.submit_for_settlement(hold.id, str(cents/100.00)) + result = braintree.Transaction.submit_for_settlement(ref, str(cents/100.00)) assert result.is_success if result.transaction.status != 'submitted_for_settlement': error = result.transaction.status @@ -228,7 +230,7 @@ def get_ready_payout_routes_by_network(db, network): return out -def record_exchange(db, route, amount, fee, participant, status, error=None): +def record_exchange(db, route, amount, fee, participant, status, error=None, ref): """Given a Bunch of Stuff, return an int (exchange_id). Records in the exchanges table have these characteristics: @@ -240,6 +242,8 @@ def record_exchange(db, route, amount, fee, participant, status, error=None): fee The payment processor's fee. It's always positive. + ref transaction id in the external system. + """ assert route.participant.id == participant.id @@ -248,10 +252,10 @@ def record_exchange(db, route, amount, fee, participant, status, error=None): exchange_id = cursor.one(""" INSERT INTO exchanges - (amount, fee, participant, status, route, note) - VALUES (%s, %s, %s, %s, %s, %s) + (amount, fee, participant, status, route, note, ref) + VALUES (%s, %s, %s, %s, %s, %s, %s) RETURNING id - """, (amount, fee, participant.username, status, route.id, error)) + """, (amount, fee, participant.username, status, route.id, error, ref)) if status == 'failed': propagate_exchange(cursor, participant, route, error, 0) From cdaf7a275daabd24745d43ce3652365716f248f9 Mon Sep 17 00:00:00 2001 From: Kwesi Aguillera Date: Thu, 2 Mar 2017 14:42:55 -0500 Subject: [PATCH 2/6] Fixed the test that were failing due to changes. --- gratipay/billing/exchanges.py | 7 ++++--- tests/py/test_billing_exchanges.py | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/gratipay/billing/exchanges.py b/gratipay/billing/exchanges.py index b5fab07bd0..8c074cf897 100644 --- a/gratipay/billing/exchanges.py +++ b/gratipay/billing/exchanges.py @@ -74,6 +74,7 @@ def create_card_hold(db, participant, amount): hold = None error = "" + ref = "" try: result = braintree.Transaction.sale({ 'amount': str(cents/100.0), @@ -82,6 +83,7 @@ def create_card_hold(db, participant, amount): 'options': { 'submit_for_settlement': False }, 'custom_fields': {'participant_id': participant.id} }) + ref = result.transaction.id if result.is_success and result.transaction.status == 'authorized': error = "" @@ -98,8 +100,7 @@ def create_card_hold(db, participant, amount): log(msg + "succeeded.") else: log(msg + "failed: %s" % error) - ref = result.transaction.id - record_exchange(db, route, amount, fee, participant, 'failed', error, ref) + record_exchange(db, route, amount, fee, participant, 'failed', ref, error) return hold, error @@ -230,7 +231,7 @@ def get_ready_payout_routes_by_network(db, network): return out -def record_exchange(db, route, amount, fee, participant, status, error=None, ref): +def record_exchange(db, route, amount, fee, participant, status, ref, error=None): """Given a Bunch of Stuff, return an int (exchange_id). Records in the exchanges table have these characteristics: diff --git a/tests/py/test_billing_exchanges.py b/tests/py/test_billing_exchanges.py index 00e919e0db..daf46ce396 100644 --- a/tests/py/test_billing_exchanges.py +++ b/tests/py/test_billing_exchanges.py @@ -279,9 +279,10 @@ def test_re_records_exchange(self): , fee=D("0.41") , participant=alice , status='pre' + , ref='alice-braintree-cc-trans-id' ) actual = self.db.one(""" - SELECT amount, fee, participant, status, route + SELECT amount, fee, participant, status, route, ref FROM exchanges """, back_as=dict) expected = { "amount": D('0.59') @@ -289,6 +290,7 @@ def test_re_records_exchange(self): , "participant": "alice" , "status": 'pre' , "route": ExchangeRoute.from_network(alice, 'braintree-cc').id + , "ref" : 'alice-braintree-cc-trans-id' } assert actual == expected @@ -302,6 +304,7 @@ def test_re_requires_valid_route(self): , fee=D("0.41") , participant=alice , status='pre' + , ref='bob-braintree-cc-trans-id' ) def test_re_stores_error_in_note(self): @@ -312,6 +315,7 @@ def test_re_stores_error_in_note(self): , fee=D("0.41") , participant=alice , status='pre' + , ref='alice-braintree-cc-trans-id' , error='Card payment failed' ) exchange = self.db.one("SELECT * FROM exchanges") @@ -325,6 +329,7 @@ def test_re_doesnt_update_balance_for_positive_amounts(self): , fee=D("0.41") , participant=alice , status='pre' + , ref='alice-braintree-cc-trans-id' ) assert P('alice').balance == D('0.00') @@ -336,6 +341,7 @@ def test_re_updates_balance_for_negative_amounts(self): , fee=D('0.75') , participant=alice , status='pre' + , ref='alice-paypal-trans-id' ) assert P('alice').balance == D('13.41') @@ -343,12 +349,12 @@ def test_re_fails_if_negative_balance(self): alice = self.make_participant('alice', last_paypal_result='') ba = ExchangeRoute.from_network(alice, 'paypal') with pytest.raises(NegativeBalance): - record_exchange(self.db, ba, D("-10.00"), D("0.41"), alice, 'pre') + record_exchange(self.db, ba, D("-10.00"), D("0.41"), alice, 'pre', 'paypal-transid') def test_re_result_restores_balance_on_error(self): alice = self.make_participant('alice', balance=30, last_paypal_result='') ba = ExchangeRoute.from_network(alice, 'paypal') - e_id = record_exchange(self.db, ba, D('-27.06'), D('0.81'), alice, 'pre') + e_id = record_exchange(self.db, ba, D('-27.06'), D('0.81'), alice, 'pre', 'paypal-transid') assert alice.balance == D('02.13') record_exchange_result(self.db, e_id, 'failed', 'SOME ERROR', alice) assert P('alice').balance == D('30.00') @@ -356,7 +362,7 @@ def test_re_result_restores_balance_on_error(self): def test_re_result_restores_balance_on_error_with_invalidated_route(self): alice = self.make_participant('alice', balance=37, last_paypal_result='') pp = ExchangeRoute.from_network(alice, 'paypal') - e_id = record_exchange(self.db, pp, D('-32.45'), D('0.86'), alice, 'pre') + e_id = record_exchange(self.db, pp, D('-32.45'), D('0.86'), alice, 'pre', 'paypal-transid') assert alice.balance == D('3.69') pp.update_error('invalidated') record_exchange_result(self.db, e_id, 'failed', 'oops', alice) @@ -367,7 +373,7 @@ def test_re_result_restores_balance_on_error_with_invalidated_route(self): def test_re_result_doesnt_restore_balance_on_success(self): alice = self.make_participant('alice', balance=50, last_paypal_result='') ba = ExchangeRoute.from_network(alice, 'paypal') - e_id = record_exchange(self.db, ba, D('-43.98'), D('1.60'), alice, 'pre') + e_id = record_exchange(self.db, ba, D('-43.98'), D('1.60'), alice, 'pre', 'paypal-transid') assert alice.balance == D('4.42') record_exchange_result(self.db, e_id, 'succeeded', None, alice) assert P('alice').balance == D('4.42') @@ -375,7 +381,7 @@ def test_re_result_doesnt_restore_balance_on_success(self): def test_re_result_updates_balance_for_positive_amounts(self): alice = self.make_participant('alice', balance=4, last_bill_result='') cc = ExchangeRoute.from_network(alice, 'braintree-cc') - e_id = record_exchange(self.db, cc, D('31.59'), D('0.01'), alice, 'pre') + e_id = record_exchange(self.db, cc, D('31.59'), D('0.01'), alice, 'pre', 'paypal-transid') assert alice.balance == D('4.00') record_exchange_result(self.db, e_id, 'succeeded', None, alice) assert P('alice').balance == D('35.59') From 4bafcf911f8accb2ff8afb27c398ac626eef1855 Mon Sep 17 00:00:00 2001 From: Kwesi Aguillera Date: Thu, 2 Mar 2017 16:11:30 -0500 Subject: [PATCH 3/6] Fixed the make_exchange in harness that uses record_exchange --- gratipay/testing/harness.py | 4 ++-- gratipay/utils/fake_data.py | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/gratipay/testing/harness.py b/gratipay/testing/harness.py index 2916d1c7d6..1192eb5756 100644 --- a/gratipay/testing/harness.py +++ b/gratipay/testing/harness.py @@ -237,7 +237,7 @@ def fetch_payday(self): def make_exchange(self, route, amount, fee, participant, status='succeeded', error='', - address='dummy-address'): + ref='dummy-trans-id', address='dummy-address'): """Factory for exchanges. """ if not isinstance(route, ExchangeRoute): @@ -246,7 +246,7 @@ def make_exchange(self, route, amount, fee, participant, status='succeeded', err if not route: route = ExchangeRoute.insert(participant, network, address) assert route - e_id = record_exchange(self.db, route, amount, fee, participant, 'pre') + e_id = record_exchange(self.db, route, amount, fee, participant, 'pre', ref) record_exchange_result(self.db, e_id, status, error, participant) return e_id diff --git a/gratipay/utils/fake_data.py b/gratipay/utils/fake_data.py index d5db0c0a15..075c7c52ec 100644 --- a/gratipay/utils/fake_data.py +++ b/gratipay/utils/fake_data.py @@ -220,6 +220,8 @@ def fake_payment(db, participant, team, timestamp, amount, payday, direction): def fake_exchange(db, participant, amount, fee, timestamp): + route=get_exchange_route(db, participant.id) + ref = '%s-%s-%s' % (participant.username, route, timestamp.microsecond) return insert_fake_data( db , "exchanges" , timestamp=timestamp @@ -227,7 +229,8 @@ def fake_exchange(db, participant, amount, fee, timestamp): , amount=amount , fee=fee , status='succeeded' - , route=get_exchange_route(db, participant.id) + , route=route + , ref=ref ) From 54f70d157cf969148e7fcdf157863bc0998ce7d2 Mon Sep 17 00:00:00 2001 From: Kwesi Aguillera Date: Fri, 3 Mar 2017 16:17:34 -0500 Subject: [PATCH 4/6] Change ref to Null if try...except fails in create_card_hold --- gratipay/billing/exchanges.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gratipay/billing/exchanges.py b/gratipay/billing/exchanges.py index 8c074cf897..d65bfc5e70 100644 --- a/gratipay/billing/exchanges.py +++ b/gratipay/billing/exchanges.py @@ -74,7 +74,7 @@ def create_card_hold(db, participant, amount): hold = None error = "" - ref = "" + ref = None try: result = braintree.Transaction.sale({ 'amount': str(cents/100.0), From 9445ddaae7e95eb5ccba144f19ac30b4e1e66adf Mon Sep 17 00:00:00 2001 From: Kwesi Aguillera Date: Fri, 17 Mar 2017 14:31:41 -0400 Subject: [PATCH 5/6] Attempt to resolve github file conflict --- gratipay/utils/fake_data.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gratipay/utils/fake_data.py b/gratipay/utils/fake_data.py index 075c7c52ec..045520a17e 100644 --- a/gratipay/utils/fake_data.py +++ b/gratipay/utils/fake_data.py @@ -220,6 +220,8 @@ def fake_payment(db, participant, team, timestamp, amount, payday, direction): def fake_exchange(db, participant, amount, fee, timestamp): + """Create a fake exchange for a participant + """ route=get_exchange_route(db, participant.id) ref = '%s-%s-%s' % (participant.username, route, timestamp.microsecond) return insert_fake_data( db From b14e1e3525bc45cd94f60a42ebad3d019cdeb8d9 Mon Sep 17 00:00:00 2001 From: Kwesi Aguillera Date: Fri, 17 Mar 2017 14:49:54 -0400 Subject: [PATCH 6/6] Moved fake_data.py location as per master current state. --- gratipay/{utils => }/fake_data.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename gratipay/{utils => }/fake_data.py (100%) diff --git a/gratipay/utils/fake_data.py b/gratipay/fake_data.py similarity index 100% rename from gratipay/utils/fake_data.py rename to gratipay/fake_data.py