From a74c5f2a24aec8908c737bb9bb4143c0c7e8dc2a Mon Sep 17 00:00:00 2001 From: Rohit Paul Kuruvilla Date: Fri, 15 May 2015 07:21:20 +0530 Subject: [PATCH 1/4] Failing tests for payment amounts --- tests/py/test_billing_payday.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index 7a908ac691..2667efca58 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -359,15 +359,16 @@ def test_card_hold_error(self, tfh, fch): payday = self.fetch_payday() assert payday['ncc_failing'] == 1 - def test_payin_doesnt_make_null_transfers(self): + def test_payin_doesnt_make_null_payments(self): + team = self.make_team('Gratiteam', is_approved=True) alice = self.make_participant('alice', claimed_time='now') - alice.set_tip_to(self.homer, 1) - alice.set_tip_to(self.homer, 0) + alice.set_subscription_to(team, 1) + alice.set_subscription_to(team, 0) a_team = self.make_participant('a_team', claimed_time='now', number='plural') a_team.add_member(alice) Payday.start().payin() - transfers0 = self.db.all("SELECT * FROM transfers WHERE amount = 0") - assert not transfers0 + payments = self.db.all("SELECT * FROM payments WHERE amount = 0") + assert not payments def test_process_subscriptions(self): alice = self.make_participant('alice', claimed_time='now', balance=1) @@ -390,6 +391,10 @@ def test_process_subscriptions(self): assert Participant.from_username('hannibal').balance == 0 assert Participant.from_username('lecter').balance == 0 + payment = self.db.one("SELECT * FROM payments") + assert payment.amount == D('0.51') + assert payment.direction == 'to-team' + @pytest.mark.xfail(reason="haven't migrated_transfer_takes yet") def test_transfer_takes(self): a_team = self.make_participant('a_team', claimed_time='now', number='plural', balance=20) @@ -443,6 +448,9 @@ def test_process_draws(self): assert Participant.from_id(alice.id).balance == D('0.49') assert Participant.from_username('hannibal').balance == D('0.51') + payment = self.db.one("SELECT * FROM payments WHERE direction='to-participant'") + assert payment.amount == D('0.51') + @pytest.mark.xfail(reason="haven't migrated_transfer_takes yet") @mock.patch.object(Payday, 'fetch_card_holds') def test_transfer_takes_doesnt_make_negative_transfers(self, fch): From 174bb6cfcbbac7b6a5d9fac859ae4a752ce0de09 Mon Sep 17 00:00:00 2001 From: Rohit Paul Kuruvilla Date: Fri, 15 May 2015 07:30:18 +0530 Subject: [PATCH 2/4] Constrain payments table to amount > 0 --- sql/payday.sql | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/sql/payday.sql b/sql/payday.sql index 9ea86906d6..8e7c538cfc 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -96,13 +96,22 @@ CREATE TEMPORARY TABLE payday_payments CREATE OR REPLACE FUNCTION pay(text, text, numeric, payment_direction) RETURNS void AS $$ + DECLARE + delta numeric; BEGIN IF ($3 = 0) THEN RETURN; END IF; + + IF ($4 = 'to-team') THEN + delta := -$3; + ELSE + delta := $3; + END IF; + UPDATE payday_participants - SET new_balance = (new_balance - $3) + SET new_balance = (new_balance + delta) WHERE username = $1; UPDATE payday_teams - SET balance = (balance + $3) + SET balance = (balance - delta) WHERE slug = $2; INSERT INTO payday_payments (participant, team, amount, direction) @@ -176,7 +185,7 @@ CREATE TRIGGER process_take AFTER INSERT ON payday_takes CREATE OR REPLACE FUNCTION process_draw() RETURNS trigger AS $$ BEGIN - EXECUTE pay(NEW.owner, NEW.slug, -NEW.balance, 'to-participant'); + EXECUTE pay(NEW.owner, NEW.slug, NEW.balance, 'to-participant'); RETURN NULL; END; $$ LANGUAGE plpgsql; From 1cd8fc71ed0e65d710600de303d03286c3db3889 Mon Sep 17 00:00:00 2001 From: Rohit Paul Kuruvilla Date: Fri, 15 May 2015 07:46:07 +0530 Subject: [PATCH 3/4] Fix _check_balances to include payments table --- gratipay/models/__init__.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/gratipay/models/__init__.py b/gratipay/models/__init__.py index 80cec4a30d..71a85fd798 100644 --- a/gratipay/models/__init__.py +++ b/gratipay/models/__init__.py @@ -67,7 +67,6 @@ def _check_balances(cursor): https://github.com/gratipay/gratipay.com/issues/1118 """ - return # XXX Bring me back! b = cursor.all(""" select p.username, expected, balance as actual from ( @@ -95,6 +94,20 @@ def _check_balances(cursor): union all + select participant as username, sum(amount) as a + from payments + where direction='to-participant' + group by participant + + union all + + select participant as username, sum(-amount) as a + from payments + where direction='to-team' + group by participant + + union all + select tippee as username, sum(amount) as a from transfers group by tippee From 46f047b0bdc3b4aa642a4ae6fc8d1c0f03665527 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 15 May 2015 11:33:08 -0400 Subject: [PATCH 4/4] Make semantics painfully clear --- sql/payday.sql | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sql/payday.sql b/sql/payday.sql index 8e7c538cfc..51cef068c1 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -97,21 +97,24 @@ CREATE TEMPORARY TABLE payday_payments CREATE OR REPLACE FUNCTION pay(text, text, numeric, payment_direction) RETURNS void AS $$ DECLARE - delta numeric; + participant_delta numeric; + team_delta numeric; BEGIN IF ($3 = 0) THEN RETURN; END IF; IF ($4 = 'to-team') THEN - delta := -$3; + participant_delta := -$3; + team_delta := $3; ELSE - delta := $3; + participant_delta := $3; + team_delta := -$3; END IF; UPDATE payday_participants - SET new_balance = (new_balance + delta) + SET new_balance = (new_balance + participant_delta) WHERE username = $1; UPDATE payday_teams - SET balance = (balance - delta) + SET balance = (balance + team_delta) WHERE slug = $2; INSERT INTO payday_payments (participant, team, amount, direction)