diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index f7f737bc9c..f67f9b81f5 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -72,8 +72,8 @@ class Payday(object): prepare create_card_holds process_payment_instructions - transfer_takes - process_draws + process_takes + process_remainder settle_card_holds update_balances take_over_balances @@ -153,8 +153,8 @@ 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_draws(cursor) + self.process_takes(cursor, self.ts_start) + self.process_remainder(cursor) _payments_for_debugging = cursor.all(""" SELECT * FROM payments WHERE "timestamp" > %s """, (self.ts_start,)) @@ -261,37 +261,41 @@ 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(""" + UPDATE payday_teams SET available_today = LEAST(available, balance); + 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)) @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/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/sql/payday.sql b/sql/payday.sql index 38fee52285..4310c2b354 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -29,7 +29,9 @@ CREATE TABLE payday_teams AS SELECT t.id , slug , 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 max that can be distributed this payday , false AS is_drained FROM teams t JOIN participants p @@ -86,9 +88,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,24 +206,27 @@ 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 $$ DECLARE - actual_amount numeric(35,2); - team_balance numeric(35,2); + amount numeric(35,2); + available_today_ 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; + 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; + 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; diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index cb4c43b216..785832adaa 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 @@ -185,20 +184,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() @@ -372,29 +357,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) @@ -458,94 +421,65 @@ 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) + def test_process_remainder(self): + 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') - 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_draws(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')) + 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.transfer_takes(cursor, payday.ts_start) - payday.process_draws(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') - @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 + 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="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 @@ -568,6 +502,123 @@ 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_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) + 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('150.00') # Sorry, bruiser. + assert P('picard').balance == D('200.00') + + class TestNotifyParticipants(EmailHarness): def test_it_notifies_participants(self):