From ba5b5d50eaf16f6fc0abec3be552ab5abc9e4357 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 10 Jun 2015 23:04:45 -0400 Subject: [PATCH 1/8] Stub a test for paying out Gratipay 1.0 balances --- tests/py/test_billing_payday.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index 9ed460c769..ec8a164838 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -564,6 +564,17 @@ def test_payout_ach_error_gets_recorded(self, ach_credit): payday = self.fetch_payday() assert payday['nach_failing'] == 1 + @mock.patch('gratipay.billing.payday.ach_credit') + def test_payout_pays_out_Gratipay_1_0_balance(self, ach): + alice = self.make_participant('alice', claimed_time='now', is_suspicious=False, + balanced_customer_href='foo', + last_ach_result='') + Payday.start().payout() + + assert ach.call_count == 1 + assert ach.call_args_list[0][0][1].id == alice.id + assert ach.call_args_list[0][0][2] == 0 + class TestNotifyParticipants(EmailHarness): From c97316b19e3c786e1b2d573ca81daf8dd6f465b3 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 10 Jun 2015 23:58:53 -0400 Subject: [PATCH 2/8] Harmonize payday and masspay payout selects --- bin/masspay.py | 12 ++++++------ gratipay/billing/payday.py | 11 ++++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/bin/masspay.py b/bin/masspay.py index 0f2e2d1613..d48a060e62 100755 --- a/bin/masspay.py +++ b/bin/masspay.py @@ -132,12 +132,12 @@ def compute_input_csv(): ---- Only include team owners ---- TODO: Include members on payroll once process_payroll is implemented - AND ( SELECT count(*) - FROM teams t - WHERE t.owner = p.username - AND t.is_approved IS TRUE - AND t.is_closed IS NOT TRUE - ) > 0 + AND ( SELECT count(*) + FROM teams t + WHERE t.owner = p.username + AND t.is_approved IS TRUE + AND t.is_closed IS NOT TRUE + ) > 0 ORDER BY p.balance DESC diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index 60c47a4b76..ccafb596a3 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -408,13 +408,10 @@ def payout(self): log("Starting payout loop.") participants = self.db.all(""" SELECT p.*::participants - FROM participants p - WHERE balance > 0 - AND ( SELECT count(*) - FROM exchange_routes r - WHERE r.participant = p.id - AND network = 'balanced-ba' - ) > 0 + FROM exchange_routes r + JOIN participants p ON p.id = r.participant + WHERE r.network = 'balanced-ba' + AND p.balance > 0 ---- Only include team owners ---- TODO: Include members on payroll once process_payroll is implemented From a7ad8369535aeafaf64db05e74dc2b2067ac3e69 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 11 Jun 2015 00:07:28 -0400 Subject: [PATCH 3/8] Pay out for pending-release participants --- bin/masspay.py | 5 +++-- gratipay/billing/payday.py | 6 ++++-- sql/branch.sql | 11 +++++++++++ tests/py/test_billing_payday.py | 4 ++-- 4 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 sql/branch.sql diff --git a/bin/masspay.py b/bin/masspay.py index d48a060e62..2dcffaec10 100755 --- a/bin/masspay.py +++ b/bin/masspay.py @@ -132,12 +132,13 @@ def compute_input_csv(): ---- Only include team owners ---- TODO: Include members on payroll once process_payroll is implemented - AND ( SELECT count(*) + AND (( SELECT count(*) FROM teams t WHERE t.owner = p.username AND t.is_approved IS TRUE AND t.is_closed IS NOT TRUE - ) > 0 + ) > 0 + OR p.status_of_1_0_balance='pending-release') ORDER BY p.balance DESC diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index ccafb596a3..79a542c3b4 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -416,12 +416,14 @@ def payout(self): ---- Only include team owners ---- TODO: Include members on payroll once process_payroll is implemented - AND ( SELECT count(*) + AND (( SELECT count(*) FROM teams t WHERE t.owner = p.username AND t.is_approved IS TRUE AND t.is_closed IS NOT TRUE - ) > 0 + ) > 0 + OR p.status_of_1_0_balance='pending-release') + """) def credit(participant): if participant.is_suspicious is None: diff --git a/sql/branch.sql b/sql/branch.sql new file mode 100644 index 0000000000..5a00530afe --- /dev/null +++ b/sql/branch.sql @@ -0,0 +1,11 @@ +BEGIN; + + CREATE TYPE status_of_1_0_balance AS ENUM + ('unreleased', 'pending-release', 'released', 'refunded'); + + ALTER TABLE participants + ADD COLUMN status_of_1_0_balance status_of_1_0_balance + NOT NULL + DEFAULT 'unreleased'; + +END; diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index ec8a164838..50733b2e0f 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -567,8 +567,8 @@ def test_payout_ach_error_gets_recorded(self, ach_credit): @mock.patch('gratipay.billing.payday.ach_credit') def test_payout_pays_out_Gratipay_1_0_balance(self, ach): alice = self.make_participant('alice', claimed_time='now', is_suspicious=False, - balanced_customer_href='foo', - last_ach_result='') + balanced_customer_href='foo', last_ach_result='', + balance=20, status_of_1_0_balance='pending-release') Payday.start().payout() assert ach.call_count == 1 From e081f286623aa7cd0f04a77dd9fa567297f65304 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 11 Jun 2015 01:10:46 -0400 Subject: [PATCH 4/8] Update new status field when balance goes to 0 There are going to be two ways to resolve a Gratipay 1.0 balance, either we release it, or we refund it. We release it the first payday after someone becomes a team owner, or (now) when we set `pending-payout`. At this point I don't think we'll need a `pending-refund` value. I've changed `{un,}released` to `{un,}resolved` to cover both cases, release and refund. I implemented the status update in Postgres to cover both payday and masspay. --- bin/masspay.py | 2 +- gratipay/billing/payday.py | 2 +- sql/branch.sql | 19 +++++++++++++++++-- tests/py/test_billing_payday.py | 27 ++++++++++++++++++++++++++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/bin/masspay.py b/bin/masspay.py index 2dcffaec10..0c94dd5857 100755 --- a/bin/masspay.py +++ b/bin/masspay.py @@ -138,7 +138,7 @@ def compute_input_csv(): AND t.is_approved IS TRUE AND t.is_closed IS NOT TRUE ) > 0 - OR p.status_of_1_0_balance='pending-release') + OR p.status_of_1_0_balance='pending-payday') ORDER BY p.balance DESC diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index 79a542c3b4..27727b3dc4 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -422,7 +422,7 @@ def payout(self): AND t.is_approved IS TRUE AND t.is_closed IS NOT TRUE ) > 0 - OR p.status_of_1_0_balance='pending-release') + OR p.status_of_1_0_balance='pending-payout') """) def credit(participant): diff --git a/sql/branch.sql b/sql/branch.sql index 5a00530afe..5d6a1d888f 100644 --- a/sql/branch.sql +++ b/sql/branch.sql @@ -1,11 +1,26 @@ BEGIN; CREATE TYPE status_of_1_0_balance AS ENUM - ('unreleased', 'pending-release', 'released', 'refunded'); + ('unresolved', 'pending-payout', 'resolved'); ALTER TABLE participants ADD COLUMN status_of_1_0_balance status_of_1_0_balance NOT NULL - DEFAULT 'unreleased'; + DEFAULT 'unresolved'; + + CREATE FUNCTION set_status_of_1_0_balance_to_resolved() RETURNS trigger AS $$ + BEGIN + UPDATE participants + SET status_of_1_0_balance='resolved' + WHERE id = NEW.id; + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + + CREATE TRIGGER update_status_of_1_0_balance + AFTER UPDATE OF balance ON participants + FOR EACH ROW + WHEN (OLD.balance > 0 AND NEW.balance = 0) + EXECUTE PROCEDURE set_status_of_1_0_balance_to_resolved(); END; diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index 50733b2e0f..b4facf8a5f 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -568,13 +568,38 @@ def test_payout_ach_error_gets_recorded(self, ach_credit): def test_payout_pays_out_Gratipay_1_0_balance(self, ach): alice = self.make_participant('alice', claimed_time='now', is_suspicious=False, balanced_customer_href='foo', last_ach_result='', - balance=20, status_of_1_0_balance='pending-release') + balance=20, status_of_1_0_balance='pending-payout') Payday.start().payout() assert ach.call_count == 1 assert ach.call_args_list[0][0][1].id == alice.id assert ach.call_args_list[0][0][2] == 0 + @mock.patch('balanced.BankAccount.credit') + def test_paying_out_sets_1_0_status_to_resolved(self, credit): + alice = self.make_participant('alice', claimed_time='now', is_suspicious=False, + balanced_customer_href='foo', last_ach_result='', + balance=0, status_of_1_0_balance='pending-payout') + self.make_exchange('balanced-cc', 20, 0, alice) # sets balance, and satisfies self_check + Payday.start().payout() + alice = Participant.from_username('alice') + assert alice.status_of_1_0_balance == 'resolved' + assert alice.balance == 0 + + @mock.patch('balanced.BankAccount.credit') + def test_payout_ignores_unresolved(self, credit): + bob = self.make_participant('bob', claimed_time='now', is_suspicious=False, + balanced_customer_href='foo', last_ach_result='', + balance=13, status_of_1_0_balance='unresolved') + alice = self.make_participant('alice', claimed_time='now', is_suspicious=False, + balanced_customer_href='foo', last_ach_result='', + balance=0, status_of_1_0_balance='pending-payout') + self.make_exchange('balanced-cc', 20, 0, alice) + Payday.start().payout() + bob = Participant.from_username('bob') + assert bob.status_of_1_0_balance == 'unresolved' + assert bob.balance == 13 + class TestNotifyParticipants(EmailHarness): From fc3e438d72584a1d5dd9629a1dda23b4057346ce Mon Sep 17 00:00:00 2001 From: Rohit Paul Kuruvilla Date: Thu, 11 Jun 2015 15:43:47 +0530 Subject: [PATCH 5/8] Fix enum value in masspay --- bin/masspay.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/masspay.py b/bin/masspay.py index 0c94dd5857..ed3fe19de6 100755 --- a/bin/masspay.py +++ b/bin/masspay.py @@ -138,7 +138,7 @@ def compute_input_csv(): AND t.is_approved IS TRUE AND t.is_closed IS NOT TRUE ) > 0 - OR p.status_of_1_0_balance='pending-payday') + OR p.status_of_1_0_balance='pending-payout') ORDER BY p.balance DESC From d58af0b73b61fc23aa3730489bf34c4209d06b88 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 11 Jun 2015 12:28:11 -0400 Subject: [PATCH 6/8] Clean and DRY the SQL behind payout and masspay --- bin/masspay.py | 40 +++++++++-------------------------- gratipay/billing/exchanges.py | 30 ++++++++++++++++++++++++++ gratipay/billing/payday.py | 37 +++++++++----------------------- 3 files changed, 50 insertions(+), 57 deletions(-) diff --git a/bin/masspay.py b/bin/masspay.py index ed3fe19de6..2b477d70d6 100755 --- a/bin/masspay.py +++ b/bin/masspay.py @@ -29,6 +29,7 @@ import requests from gratipay import wireup +from gratipay.billing.exchanges import get_ready_payout_routes_by_network from httplib import IncompleteRead @@ -121,50 +122,29 @@ def assess_fee(self): def compute_input_csv(): db = wireup.db(wireup.env()) - participants = db.all(""" - - SELECT p.*, r.address AS paypal_email, r.fee_cap AS paypal_fee_cap - FROM exchange_routes r - JOIN participants p ON p.id = r.participant - WHERE r.network = 'paypal' - AND p.balance > 0 - - ---- Only include team owners - ---- TODO: Include members on payroll once process_payroll is implemented - - AND (( SELECT count(*) - FROM teams t - WHERE t.owner = p.username - AND t.is_approved IS TRUE - AND t.is_closed IS NOT TRUE - ) > 0 - OR p.status_of_1_0_balance='pending-payout') - - ORDER BY p.balance DESC - - """) + routes = get_ready_payout_routes_by_network(db, 'paypal') writer = csv.writer(open(INPUT_CSV, 'w+')) print_rule(88) headers = "username", "email", "fee cap", "balance", "tips", "amount" print("{:<24}{:<32} {:^7} {:^7} {:^7} {:^7}".format(*headers)) print_rule(88) total_gross = 0 - for participant in participants: - total = participant.giving - amount = participant.balance - total + for route in routes: + total = route.participant.giving + amount = route.participant.balance - total if amount < 0.50: # Minimum payout of 50 cents. I think that otherwise PayPal upcharges to a penny. # See https://github.com/gratipay/gratipay.com/issues/1958. continue total_gross += amount - print("{:<24}{:<32} {:>7} {:>7} {:>7} {:>7}".format( participant.username - , participant.paypal_email - , participant.paypal_fee_cap - , participant.balance + print("{:<24}{:<32} {:>7} {:>7} {:>7} {:>7}".format( route.participant.username + , route.address + , route.fee_cap + , route.participant.balance , total , amount )) - row = (participant.username, participant.paypal_email, participant.paypal_fee_cap, amount) + row = (route.username, route.address, route.fee_cap, amount) writer.writerow(row) print(" "*80, "-"*7) print("{:>88}".format(total_gross)) diff --git a/gratipay/billing/exchanges.py b/gratipay/billing/exchanges.py index 9583759f15..31853cfe18 100644 --- a/gratipay/billing/exchanges.py +++ b/gratipay/billing/exchanges.py @@ -293,6 +293,36 @@ def _prep_hit(unrounded): return cents, amount_str, upcharged, fee +def get_ready_payout_routes_by_network(db, network): + hack = db.all(""" + SELECT p.*::participants, r.*::exchange_routes + FROM participants p + JOIN current_exchange_routes r ON p.id = r.participant + WHERE p.balance > 0 + AND r.network = %s + + ---- Only include team owners + ---- TODO: Include members on payroll once process_payroll is implemented + + AND (( SELECT count(*) + FROM teams t + WHERE t.owner = p.username + AND t.is_approved IS TRUE + AND t.is_closed IS NOT TRUE + ) > 0 + OR p.status_of_1_0_balance='pending-payout') + + """, (network,)) + + # Work around lack of proper nesting in postgres.orm. + out = [] + for participant, route in hack: + route.__dict__['participant'] = participant + out.append(route) + + return out + + def record_exchange(db, route, amount, fee, participant, status, error=None): """Given a Bunch of Stuff, return an int (exchange_id). diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index 27727b3dc4..b0cc4bc5b9 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -19,7 +19,8 @@ import aspen.utils from aspen import log from gratipay.billing.exchanges import ( - ach_credit, cancel_card_hold, capture_card_hold, create_card_hold, upcharge + ach_credit, cancel_card_hold, capture_card_hold, create_card_hold, upcharge, + get_ready_payout_routes_by_network ) from gratipay.exceptions import NegativeBalance from gratipay.models import check_db @@ -406,35 +407,17 @@ def payout(self): bank accounts of participants. """ log("Starting payout loop.") - participants = self.db.all(""" - SELECT p.*::participants - FROM exchange_routes r - JOIN participants p ON p.id = r.participant - WHERE r.network = 'balanced-ba' - AND p.balance > 0 - - ---- Only include team owners - ---- TODO: Include members on payroll once process_payroll is implemented - - AND (( SELECT count(*) - FROM teams t - WHERE t.owner = p.username - AND t.is_approved IS TRUE - AND t.is_closed IS NOT TRUE - ) > 0 - OR p.status_of_1_0_balance='pending-payout') - - """) - def credit(participant): - if participant.is_suspicious is None: - log("UNREVIEWED: %s" % participant.username) + routes = get_ready_payout_routes_by_network(self.db, 'balanced-ba') + def credit(route): + if route.participant.is_suspicious is None: + log("UNREVIEWED: %s" % route.participant.username) return - withhold = participant.giving - error = ach_credit(self.db, participant, withhold) + withhold = route.participant.giving + error = ach_credit(self.db, route.participant, withhold) if error: self.mark_ach_failed() - threaded_map(credit, participants) - log("Did payout for %d participants." % len(participants)) + threaded_map(credit, routes) + log("Did payout for %d participants." % len(routes)) self.db.self_check() log("Checked the DB.") From 7506e5b2da8f80e63cc2ec7b6d213018f5e7b6f4 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 11 Jun 2015 12:31:34 -0400 Subject: [PATCH 7/8] Reformat SQL for clarity --- gratipay/billing/exchanges.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/gratipay/billing/exchanges.py b/gratipay/billing/exchanges.py index 31853cfe18..1ed29f0933 100644 --- a/gratipay/billing/exchanges.py +++ b/gratipay/billing/exchanges.py @@ -300,17 +300,25 @@ def get_ready_payout_routes_by_network(db, network): JOIN current_exchange_routes r ON p.id = r.participant WHERE p.balance > 0 AND r.network = %s + AND ( - ---- Only include team owners - ---- TODO: Include members on payroll once process_payroll is implemented + ---- Include team owners - AND (( SELECT count(*) + (SELECT count(*) FROM teams t WHERE t.owner = p.username AND t.is_approved IS TRUE AND t.is_closed IS NOT TRUE - ) > 0 - OR p.status_of_1_0_balance='pending-payout') + ) > 0 + + + OR ---- Include green-lit Gratipay 1.0 balances + + p.status_of_1_0_balance='pending-payout' + + + ---- TODO: Include members on payroll once process_payroll is implemented + ) """, (network,)) From f49da8c1330ca4536674ae0b2e444c9fe7497892 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 11 Jun 2015 12:34:17 -0400 Subject: [PATCH 8/8] I can't stop preening!!! --- gratipay/billing/exchanges.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gratipay/billing/exchanges.py b/gratipay/billing/exchanges.py index 1ed29f0933..408060a9f6 100644 --- a/gratipay/billing/exchanges.py +++ b/gratipay/billing/exchanges.py @@ -302,7 +302,7 @@ def get_ready_payout_routes_by_network(db, network): AND r.network = %s AND ( - ---- Include team owners + ----- Include team owners (SELECT count(*) FROM teams t @@ -312,14 +312,14 @@ def get_ready_payout_routes_by_network(db, network): ) > 0 - OR ---- Include green-lit Gratipay 1.0 balances + OR -- Include green-lit Gratipay 1.0 balances p.status_of_1_0_balance='pending-payout' - ---- TODO: Include members on payroll once process_payroll is implemented - ) + ----- TODO: Include members on payroll once process_payroll is implemented + ) """, (network,)) # Work around lack of proper nesting in postgres.orm.