From 36059eeeb86b0de055c572197ba0258ba3264080 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 May 2016 22:53:22 -0400 Subject: [PATCH 1/7] Rename process_draws to process_remainder Fits the new nomenclature better --- gratipay/billing/payday.py | 10 +++++----- tests/py/test_billing_payday.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index f7f737bc9c..bd777af20b 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -73,7 +73,7 @@ class Payday(object): create_card_holds process_payment_instructions transfer_takes - process_draws + process_remainder settle_card_holds update_balances take_over_balances @@ -154,7 +154,7 @@ def payin(self): holds = self.create_card_holds(cursor) self.process_payment_instructions(cursor) self.transfer_takes(cursor, self.ts_start) - self.process_draws(cursor) + self.process_remainder(cursor) _payments_for_debugging = cursor.all(""" SELECT * FROM payments WHERE "timestamp" > %s """, (self.ts_start,)) @@ -288,10 +288,10 @@ def transfer_takes(cursor, ts_start): @staticmethod - def process_draws(cursor): - """Send whatever remains after payouts to the team owner. + def process_remainder(cursor): + """Send whatever remains after processing takes to the team owner. """ - log("Processing draws.") + log("Processing remainder.") cursor.run("UPDATE payday_teams SET is_drained=true;") diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index cb4c43b216..57fb142303 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -491,7 +491,7 @@ def test_transfer_takes(self): else: assert p.balance == 0 - def test_process_draws(self): + def test_process_remainder(self): alice = self.make_participant('alice', claimed_time='now', balance=1) picard = self.make_participant('picard', claimed_time='now', last_paypal_result='') Enterprise = self.make_team('The Enterprise', picard, is_approved=True) @@ -502,7 +502,7 @@ def test_process_draws(self): payday.prepare(cursor) payday.process_payment_instructions(cursor) payday.transfer_takes(cursor, payday.ts_start) - payday.process_draws(cursor) + payday.process_remainder(cursor) assert cursor.one("select new_balance from payday_participants " "where username='picard'") == D('0.51') assert cursor.one("select balance from payday_teams where slug='TheEnterprise'") == 0 From d69300c024c3031ca3351f1e1ec555fb4a56de59 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 May 2016 10:37:43 -0400 Subject: [PATCH 2/7] Bring back takes in payday --- gratipay/billing/payday.py | 44 ++++++------ sql/payday.sql | 46 +++++++----- tests/py/test_billing_payday.py | 122 +++++++++++++++++++++++++++++++- 3 files changed, 171 insertions(+), 41 deletions(-) diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index bd777af20b..90930b7fd6 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -72,7 +72,7 @@ class Payday(object): prepare create_card_holds process_payment_instructions - transfer_takes + process_takes process_remainder settle_card_holds update_balances @@ -153,7 +153,7 @@ def payin(self): self.prepare(cursor) holds = self.create_card_holds(cursor) self.process_payment_instructions(cursor) - self.transfer_takes(cursor, self.ts_start) + self.process_takes(cursor, self.ts_start) self.process_remainder(cursor) _payments_for_debugging = cursor.all(""" SELECT * FROM payments WHERE "timestamp" > %s @@ -261,28 +261,30 @@ def process_payment_instructions(cursor): @staticmethod - def transfer_takes(cursor, ts_start): - return # XXX Bring me back! + def process_takes(cursor, ts_start): + log("Processing takes.") cursor.run(""" INSERT INTO payday_takes - SELECT team, member, amount - FROM ( SELECT DISTINCT ON (team, member) - team, member, amount, ctime - FROM takes - WHERE mtime < %(ts_start)s - ORDER BY team, member, mtime DESC - ) t - WHERE t.amount > 0 - AND t.team IN (SELECT username FROM payday_participants) - AND t.member IN (SELECT username FROM payday_participants) - AND ( SELECT id - FROM payday_transfers_done t2 - WHERE t.team = t2.tipper - AND t.member = t2.tippee - AND context = 'take' - ) IS NULL - ORDER BY t.team, t.ctime DESC; + SELECT team_id, participant_id, amount + FROM ( SELECT DISTINCT ON (team_id, participant_id) + team_id, participant_id, amount, ctime + FROM takes + WHERE mtime < %(ts_start)s + ORDER BY team_id, participant_id, mtime DESC + ) t + WHERE t.amount > 0 + AND t.team_id IN (SELECT id FROM payday_teams) + AND t.participant_id IN (SELECT id FROM payday_participants) + AND ( SELECT ppd.id + FROM payday_payments_done ppd + JOIN participants ON participants.id = t.participant_id + JOIN teams ON teams.id = t.team_id + WHERE participants.username = ppd.participant + AND teams.slug = ppd.team + AND direction = 'to-participant' + ) IS NULL + ORDER BY t.team_id, t.amount ASC; """, dict(ts_start=ts_start)) diff --git a/sql/payday.sql b/sql/payday.sql index 38fee52285..14dc274172 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -29,6 +29,7 @@ CREATE TABLE payday_teams AS SELECT t.id , slug , owner + , available , 0::numeric(35, 2) AS balance , false AS is_drained FROM teams t @@ -86,9 +87,9 @@ UPDATE payday_participants pp DROP TABLE IF EXISTS payday_takes; CREATE TABLE payday_takes -( team text -, member text -, amount numeric(35,2) +( team_id bigint +, participant_id bigint +, amount numeric(35,2) ); DROP TABLE IF EXISTS payday_payments; @@ -204,30 +205,37 @@ CREATE TRIGGER process_payment_instruction BEFORE UPDATE OF is_funded ON payday_ WHEN (NEW.is_funded IS true AND OLD.is_funded IS NOT true) EXECUTE PROCEDURE process_payment_instruction(); --- Create a trigger to process takes -CREATE OR REPLACE FUNCTION process_take() RETURNS trigger AS $$ +-- Create a trigger to process distributions based on takes + +CREATE OR REPLACE FUNCTION process_distribution() RETURNS trigger AS $$ DECLARE - actual_amount numeric(35,2); - team_balance numeric(35,2); + amount numeric(35,2); + balance_ numeric(35,2); + available_ numeric(35,2); BEGIN - team_balance := ( - SELECT new_balance - FROM payday_participants - WHERE username = NEW.team - ); - IF (team_balance <= 0) THEN RETURN NULL; END IF; - actual_amount := NEW.amount; - IF (team_balance < NEW.amount) THEN - actual_amount := team_balance; + amount := NEW.amount; + + balance_ := (SELECT balance FROM payday_teams WHERE id = NEW.team_id); + IF balance_ < amount THEN + amount := balance_; + END IF; + + available_ := (SELECT available FROM payday_teams WHERE id = NEW.team_id); + IF available_ < amount THEN + amount := available_; + END IF; + + IF amount > 0 THEN + UPDATE payday_teams SET available = (available - amount) WHERE id = NEW.team_id; + EXECUTE pay(NEW.participant_id, NEW.team_id, amount, 'to-participant'); END IF; - EXECUTE transfer(NEW.team, NEW.member, actual_amount, 'take'); RETURN NULL; END; $$ LANGUAGE plpgsql; -CREATE TRIGGER process_take AFTER INSERT ON payday_takes - FOR EACH ROW EXECUTE PROCEDURE process_take(); +CREATE TRIGGER process_takes AFTER INSERT ON payday_takes + FOR EACH ROW EXECUTE PROCEDURE process_distribution(); -- Create a trigger to process draws diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index 57fb142303..55dd75abf2 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -11,6 +11,7 @@ from gratipay.billing.payday import NoPayday, Payday from gratipay.exceptions import NegativeBalance from gratipay.models.participant import Participant +from gratipay.models.team.mixins.takes import NotAllowed from gratipay.testing import Foobar, D,P from gratipay.testing.billing import BillingHarness from gratipay.testing.emails import EmailHarness @@ -501,8 +502,8 @@ def test_process_remainder(self): with self.db.get_cursor() as cursor: payday.prepare(cursor) payday.process_payment_instructions(cursor) - payday.transfer_takes(cursor, payday.ts_start) payday.process_remainder(cursor) + payday.process_takes(cursor, payday.ts_start) assert cursor.one("select new_balance from payday_participants " "where username='picard'") == D('0.51') assert cursor.one("select balance from payday_teams where slug='TheEnterprise'") == 0 @@ -568,6 +569,125 @@ def test_payin_dumps_transfers_for_debugging(self, cch, fch): assert filename.endswith('_payments.csv') os.unlink(filename) + +class TestTakes(BillingHarness): + + tearDownClass = None + + def setUp(self): + self.enterprise = self.make_team('The Enterprise', is_approved=True) + self.set_available(500) + self.picard = P('picard') + + def make_member(self, username, take): + member = self.make_participant( username + , email_address=username+'@x.y' + , claimed_time='now' + , verified_in='TT' + ) + self.enterprise.add_member(member, self.picard) + self.enterprise.set_take_for(member, take, member) + return member + + def set_available(self, available): + # hack since we don't have Python API for this yet + self.db.run('UPDATE teams SET available=%s', (available,)) + self.enterprise.set_attributes(available=available) + + + payday = None + def start_payday(self): + self.payday = Payday.start() + + def run_through_takes(self): + if not self.payday: + self.start_payday() + with self.db.get_cursor() as cursor: + self.payday.prepare(cursor) + cursor.run("UPDATE payday_teams SET balance=537") + self.payday.process_takes(cursor, self.payday.ts_start) + self.payday.update_balances(cursor) + + + # pt - process_takes + + def test_pt_processes_takes(self): + self.make_member('crusher', 150) + self.make_member('bruiser', 250) + self.run_through_takes() + assert P('crusher').balance == D('150.00') + assert P('bruiser').balance == D('250.00') + assert P('picard').balance == D(' 0.00') + + def test_pt_processes_takes_again(self): + # This catches a bug where payday_takes.amount was an int! + self.make_member('crusher', D('0.01')) + self.make_member('bruiser', D('5.37')) + self.run_through_takes() + assert P('crusher').balance == D('0.01') + assert P('bruiser').balance == D('5.37') + assert P('picard').balance == D('0.00') + + def test_pt_ignores_takes_set_after_the_start_of_payday(self): + self.make_member('crusher', 150) + self.start_payday() + self.make_member('bruiser', 250) + self.run_through_takes() + assert P('crusher').balance == D('150.00') + assert P('bruiser').balance == D(' 0.00') + assert P('picard').balance == D(' 0.00') + + def test_pt_ignores_takes_that_have_already_been_processed(self): + self.make_member('crusher', 150) + self.start_payday() + self.make_member('bruiser', 250) + self.run_through_takes() + self.run_through_takes() + self.run_through_takes() + self.run_through_takes() + self.run_through_takes() + assert P('crusher').balance == D('150.00') + assert P('bruiser').balance == D(' 0.00') + assert P('picard').balance == D(' 0.00') + + def test_pt_clips_to_available(self): + self.make_member('alice', 350) + self.make_member('bruiser', 250) + self.make_member('crusher', 150) + self.make_member('zorro', 450) + self.run_through_takes() + assert P('crusher').balance == D('150.00') + assert P('bruiser').balance == D('250.00') + assert P('alice').balance == D('100.00') + assert P('zorro').balance == D(' 0.00') + assert P('picard').balance == D(' 0.00') + + def test_pt_clips_to_balance_when_less_than_available(self): + self.set_available(1000) + self.make_member('alice', 350) + self.make_member('bruiser', 250) + self.make_member('crusher', 150) + self.make_member('zorro', 450) + self.run_through_takes() + assert P('crusher').balance == D('150.00') + assert P('bruiser').balance == D('250.00') + assert P('alice').balance == D('137.00') + assert P('zorro').balance == D(' 0.00') + assert P('picard').balance == D(' 0.00') + + def test_pt_is_NOT_happy_to_deal_the_owner_in(self): + self.make_member('crusher', 150) + self.make_member('bruiser', 250) + self.enterprise.set_take_for(self.picard, D('0.01'), self.picard) + with pytest.raises(NotAllowed): + self.enterprise.set_take_for(self.picard, 50, self.picard) + return # XXX allow owners to join teams! + self.run_through_takes() + assert P('crusher').balance == D('150.00') + assert P('bruiser').balance == D('250.00') + assert P('picard').balance == D(' 50.00') + + class TestNotifyParticipants(EmailHarness): def test_it_notifies_participants(self): From ee5bd7a586e89beeb888ce655381dac30f5823d7 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 11 May 2016 13:06:37 -0400 Subject: [PATCH 3/7] Update pre-existing test suite --- tests/py/test_billing_payday.py | 107 +++++--------------------------- 1 file changed, 15 insertions(+), 92 deletions(-) diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index 55dd75abf2..aac9cadb9d 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -2,7 +2,6 @@ import os -import balanced import braintree import mock import pytest @@ -186,20 +185,6 @@ def test_nusers_includes_dues(self, fch): nusers = self.db.one("SELECT nusers FROM paydays") assert nusers == 1 - @pytest.mark.xfail(reason="haven't migrated transfer_takes yet") - @mock.patch.object(Payday, 'fetch_card_holds') - @mock.patch('gratipay.billing.payday.create_card_hold') - def test_ncc_failing(self, cch, fch): - self.janet.set_tip_to(self.homer, 24) - fch.return_value = {} - cch.return_value = (None, 'oops') - payday = Payday.start() - before = self.fetch_payday() - assert before['ncc_failing'] == 0 - payday.payin() - after = self.fetch_payday() - assert after['ncc_failing'] == 1 - @mock.patch('gratipay.billing.payday.log') def test_start_prepare(self, log): self.clear_tables() @@ -373,29 +358,7 @@ def test_payin_cancels_existing_holds_of_insufficient_amounts(self, fch): assert holds[self.obama.id] is fake_hold assert hold.status == 'voided' - @pytest.mark.xfail(reason="Don't think we'll need this anymore since we aren't using balanced, " - "leaving it here till I'm sure.") - @mock.patch('gratipay.billing.payday.CardHold') - @mock.patch('gratipay.billing.payday.cancel_card_hold') - def test_fetch_card_holds_handles_extra_holds(self, cancel, CardHold): - fake_hold = mock.MagicMock() - fake_hold.meta = {'participant_id': 0} - fake_hold.amount = 1061 - fake_hold.save = mock.MagicMock() - CardHold.query.filter.return_value = [fake_hold] - for attr, state in (('failure_reason', 'failed'), - ('voided_at', 'cancelled'), - ('debit_href', 'captured')): - holds = Payday.fetch_card_holds(set()) - assert fake_hold.meta['state'] == state - fake_hold.save.assert_called_with() - assert len(holds) == 0 - setattr(fake_hold, attr, None) - holds = Payday.fetch_card_holds(set()) - cancel.assert_called_with(fake_hold) - assert len(holds) == 0 - - @pytest.mark.xfail(reason="haven't migrated transfer_takes yet") + @pytest.mark.xfail(reason="turned this off during Gratipocalypse; turn back on!") @mock.patch('gratipay.billing.payday.log') def test_payin_cancels_uncaptured_holds(self, log): self.janet.set_tip_to(self.homer, 42) @@ -459,39 +422,6 @@ def test_process_payment_instructions(self): 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) - alice = self.make_participant('alice', claimed_time='now') - a_team.add_member(alice) - a_team.add_member(self.make_participant('bob', claimed_time='now')) - a_team.set_take_for(alice, D('1.00'), alice) - - payday = Payday.start() - - # Test that payday ignores takes set after it started - a_team.set_take_for(alice, D('2.00'), alice) - - # Run the transfer multiple times to make sure we ignore takes that - # have already been processed - for i in range(3): - with self.db.get_cursor() as cursor: - payday.prepare(cursor) - payday.transfer_takes(cursor, payday.ts_start) - payday.update_balances(cursor) - - participants = self.db.all("SELECT username, balance FROM participants") - - for p in participants: - if p.username == 'a_team': - assert p.balance == D('18.99') - elif p.username == 'alice': - assert p.balance == D('1.00') - elif p.username == 'bob': - assert p.balance == D('0.01') - else: - assert p.balance == 0 - def test_process_remainder(self): alice = self.make_participant('alice', claimed_time='now', balance=1) picard = self.make_participant('picard', claimed_time='now', last_paypal_result='') @@ -515,38 +445,31 @@ def test_process_remainder(self): 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): - hold = balanced.CardHold(amount=1500, meta={'participant_id': self.janet.id}, - card_href=self.card_href) - hold.capture = lambda *a, **kw: None - hold.save = lambda *a, **kw: None - fch.return_value = {self.janet.id: hold} - self.janet.update_number('plural') - self.janet.set_tip_to(self.homer, 10) - self.janet.add_member(self.david) - Payday.start().payin() - assert P('david').balance == 0 - assert P('homer').balance == 10 - assert P('janet').balance == 0 - - @pytest.mark.xfail(reason="haven't migrated take_over_balances yet") + @pytest.mark.xfail(reason="team owners can't be taken over because of #3602") def test_take_over_during_payin(self): alice = self.make_participant('alice', claimed_time='now', balance=50) - bob = self.make_participant('bob', claimed_time='now', elsewhere='twitter') - alice.set_tip_to(bob, 18) + enterprise = self.make_team('The Enterprise', is_approved=True) + picard = Participant.from_username(enterprise.owner) + self.make_participant('bob', claimed_time='now', elsewhere='twitter') + alice.set_payment_instruction(enterprise, 18) payday = Payday.start() with self.db.get_cursor() as cursor: payday.prepare(cursor) + + # bruce takes over picard bruce = self.make_participant('bruce', claimed_time='now') - bruce.take_over(('twitter', str(bob.id)), have_confirmation=True) + bruce.take_over(('github', str(picard.id)), have_confirmation=True) payday.process_payment_instructions(cursor) - bruce.delete_elsewhere('twitter', str(bob.id)) + + # billy takes over bruce + bruce.delete_elsewhere('twitter', str(picard.id)) billy = self.make_participant('billy', claimed_time='now') billy.take_over(('github', str(bruce.id)), have_confirmation=True) + payday.update_balances(cursor) payday.take_over_balances() + + # billy ends up with the money assert P('bob').balance == 0 assert P('bruce').balance == 0 assert P('billy').balance == 18 From 0ed1d311207bff90eca77686c177bc0e94657a8d Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Wed, 31 Aug 2016 22:15:21 +0530 Subject: [PATCH 4/7] Increase surface area of process_remainder test --- tests/py/test_billing_payday.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index aac9cadb9d..ce990d07aa 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -423,27 +423,38 @@ def test_process_payment_instructions(self): assert payment.direction == 'to-team' def test_process_remainder(self): - alice = self.make_participant('alice', claimed_time='now', balance=1) - picard = self.make_participant('picard', claimed_time='now', last_paypal_result='') - Enterprise = self.make_team('The Enterprise', picard, is_approved=True) - alice.set_payment_instruction(Enterprise, D('0.51')) + alice = self.make_participant('alice', claimed_time='now', balance=100) + picard = self.make_participant('picard', claimed_time='now', last_paypal_result='', verified_in='TT', email_address='picard@x.y') + crusher = self.make_participant('crusher', claimed_time='now', verified_in='TT', email_address='crusher@x.y') + + Enterprise = self.make_team('The Enterprise', picard, is_approved=True, available=100) + Enterprise.add_member(crusher, picard) + Enterprise.set_take_for(crusher, 10, crusher) + alice.set_payment_instruction(Enterprise, D('80')) payday = Payday.start() with self.db.get_cursor() as cursor: payday.prepare(cursor) payday.process_payment_instructions(cursor) - payday.process_remainder(cursor) payday.process_takes(cursor, payday.ts_start) + payday.process_remainder(cursor) assert cursor.one("select new_balance from payday_participants " - "where username='picard'") == D('0.51') + "where username='picard'") == D('70') assert cursor.one("select balance from payday_teams where slug='TheEnterprise'") == 0 payday.update_balances(cursor) - assert P('alice').balance == D('0.49') - assert P('picard').balance == D('0.51') + assert P('alice').balance == D('20') # Alice had $100 and gave away $80 + assert P('crusher').balance == D('10') # Crusher had set their take to $10 + assert P('picard').balance == D('70') # Picard is the owner of the team, recieves what is leftover - payment = self.db.one("SELECT * FROM payments WHERE direction='to-participant'") - assert payment.amount == D('0.51') + payment_to_team = self.db.one("SELECT amount FROM payments WHERE direction='to-team'") + assert payment_to_team == D('80') + + payments_to_participant = self.db.all("SELECT participant, amount FROM payments WHERE direction='to-participant' ORDER BY amount DESC") + assert payments_to_participant[0].participant == 'picard' + assert payments_to_participant[0].amount == D('70') + assert payments_to_participant[1].participant == 'crusher' + assert payments_to_participant[1].amount == D('10') @pytest.mark.xfail(reason="team owners can't be taken over because of #3602") def test_take_over_during_payin(self): From 51af22746bd7595170125f2f6298bc2755e0fa1e Mon Sep 17 00:00:00 2001 From: Paul Kuruvilla Date: Wed, 31 Aug 2016 23:48:24 +0530 Subject: [PATCH 5/7] Simplify takes logic by adding available_today --- gratipay/billing/payday.py | 2 ++ sql/payday.sql | 25 ++++++++++--------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index 90930b7fd6..f67f9b81f5 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -265,6 +265,8 @@ def process_takes(cursor, ts_start): log("Processing takes.") cursor.run(""" + UPDATE payday_teams SET available_today = LEAST(available, balance); + INSERT INTO payday_takes SELECT team_id, participant_id, amount FROM ( SELECT DISTINCT ON (team_id, participant_id) diff --git a/sql/payday.sql b/sql/payday.sql index 14dc274172..7c81916642 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -29,8 +29,9 @@ CREATE TABLE payday_teams AS SELECT t.id , slug , owner - , available + , available -- The maximum amount that can be distributed to members (ever) , 0::numeric(35, 2) AS balance + , 0::numeric(35, 2) AS available_today -- The maximum amount that can be distributed to members in this payday , false AS is_drained FROM teams t JOIN participants p @@ -208,34 +209,28 @@ CREATE TRIGGER process_payment_instruction BEFORE UPDATE OF is_funded ON payday_ -- Create a trigger to process distributions based on takes -CREATE OR REPLACE FUNCTION process_distribution() RETURNS trigger AS $$ +CREATE OR REPLACE FUNCTION process_take() RETURNS trigger AS $$ DECLARE amount numeric(35,2); - balance_ numeric(35,2); - available_ numeric(35,2); + available_today_ numeric(35,2); BEGIN amount := NEW.amount; - balance_ := (SELECT balance FROM payday_teams WHERE id = NEW.team_id); - IF balance_ < amount THEN - amount := balance_; - END IF; - - available_ := (SELECT available FROM payday_teams WHERE id = NEW.team_id); - IF available_ < amount THEN - amount := available_; + available_today_ := (SELECT available_today FROM payday_teams WHERE id = NEW.team_id); + IF amount > available_today_ THEN + amount := available_today_; END IF; IF amount > 0 THEN - UPDATE payday_teams SET available = (available - amount) WHERE id = NEW.team_id; + UPDATE payday_teams SET available_today = (available_today - amount) WHERE id = NEW.team_id; EXECUTE pay(NEW.participant_id, NEW.team_id, amount, 'to-participant'); END IF; RETURN NULL; END; $$ LANGUAGE plpgsql; -CREATE TRIGGER process_takes AFTER INSERT ON payday_takes - FOR EACH ROW EXECUTE PROCEDURE process_distribution(); +CREATE TRIGGER process_take AFTER INSERT ON payday_takes + FOR EACH ROW EXECUTE PROCEDURE process_take(); -- Create a trigger to process draws From 241bc51168acaa809e92bd47556b572ae5f6d97c Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 31 Aug 2016 16:01:12 -0400 Subject: [PATCH 6/7] Minor formatting cleanups --- sql/payday.sql | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/sql/payday.sql b/sql/payday.sql index 7c81916642..4310c2b354 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -31,7 +31,7 @@ CREATE TABLE payday_teams AS , owner , available -- The maximum amount that can be distributed to members (ever) , 0::numeric(35, 2) AS balance - , 0::numeric(35, 2) AS available_today -- The maximum amount that can be distributed to members in this payday + , 0::numeric(35, 2) AS available_today -- The max that can be distributed this payday , false AS is_drained FROM teams t JOIN participants p @@ -207,22 +207,24 @@ CREATE TRIGGER process_payment_instruction BEFORE UPDATE OF is_funded ON payday_ EXECUTE PROCEDURE process_payment_instruction(); --- Create a trigger to process distributions based on takes +-- Create a trigger to process takes CREATE OR REPLACE FUNCTION process_take() RETURNS trigger AS $$ DECLARE - amount numeric(35,2); - available_today_ numeric(35,2); + amount numeric(35,2); + available_today_ numeric(35,2); BEGIN amount := NEW.amount; - available_today_ := (SELECT available_today FROM payday_teams WHERE id = NEW.team_id); + IF amount > available_today_ THEN amount := available_today_; END IF; IF amount > 0 THEN - UPDATE payday_teams SET available_today = (available_today - amount) WHERE id = NEW.team_id; + UPDATE payday_teams + SET available_today = (available_today - amount) + WHERE id = NEW.team_id; EXECUTE pay(NEW.participant_id, NEW.team_id, amount, 'to-participant'); END IF; RETURN NULL; From 3ac7fee75e10d8c6dd3d867b1a14c9b623060d09 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 31 Aug 2016 20:47:21 -0400 Subject: [PATCH 7/7] Allow owners to add themselves --- gratipay/models/team/mixins/takes.py | 11 ++++++++--- tests/py/test_billing_payday.py | 11 ++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/gratipay/models/team/mixins/takes.py b/gratipay/models/team/mixins/takes.py index a7d2f558d3..7357ea8e2e 100644 --- a/gratipay/models/team/mixins/takes.py +++ b/gratipay/models/team/mixins/takes.py @@ -82,10 +82,15 @@ def vet(p): vet(participant) vet(recorder) - if recorder.username == self.owner: - if take not in (ZERO, PENNY): + owner_recording = recorder.username == self.owner + owner_taking = participant.username == self.owner + taker_recording = recorder == participant + adding_or_removing = take in (ZERO, PENNY) + + if owner_recording: + if not adding_or_removing and not owner_taking: raise NotAllowed("owner can only add and remove members, not otherwise set takes") - elif recorder != participant: + elif not taker_recording: raise NotAllowed("can only set own take") with self.db.get_cursor(cursor) as cursor: diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index ce990d07aa..785832adaa 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -10,7 +10,6 @@ from gratipay.billing.payday import NoPayday, Payday from gratipay.exceptions import NegativeBalance from gratipay.models.participant import Participant -from gratipay.models.team.mixins.takes import NotAllowed from gratipay.testing import Foobar, D,P from gratipay.testing.billing import BillingHarness from gratipay.testing.emails import EmailHarness @@ -609,17 +608,15 @@ def test_pt_clips_to_balance_when_less_than_available(self): assert P('zorro').balance == D(' 0.00') assert P('picard').balance == D(' 0.00') - def test_pt_is_NOT_happy_to_deal_the_owner_in(self): + def test_pt_is_happy_to_deal_the_owner_in(self): self.make_member('crusher', 150) self.make_member('bruiser', 250) self.enterprise.set_take_for(self.picard, D('0.01'), self.picard) - with pytest.raises(NotAllowed): - self.enterprise.set_take_for(self.picard, 50, self.picard) - return # XXX allow owners to join teams! + self.enterprise.set_take_for(self.picard, 200, self.picard) self.run_through_takes() assert P('crusher').balance == D('150.00') - assert P('bruiser').balance == D('250.00') - assert P('picard').balance == D(' 50.00') + assert P('bruiser').balance == D('150.00') # Sorry, bruiser. + assert P('picard').balance == D('200.00') class TestNotifyParticipants(EmailHarness):