Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Commit

Permalink
Stop Python-interpolating payday.sql
Browse files Browse the repository at this point in the history
Using Python string interpolation on the payday.sql file introduces
complexity that leads to bugs. The one I hit was using '%' for
interplation in a Postgres RAISES clause: it would've had to be escaped
to make it through the Python interpolation.
  • Loading branch information
chadwhitacre committed Jul 15, 2015
1 parent b9506a9 commit efe6a4c
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 34 deletions.
14 changes: 4 additions & 10 deletions gratipay/billing/payday.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,9 @@ def start(cls):
""", back_as=dict)
log("Starting a new payday.")
except IntegrityError: # Collision, we have a Payday already.
d = cls.db.one("""
SELECT id, (ts_start AT TIME ZONE 'UTC') AS ts_start, stage
FROM paydays
WHERE ts_end='1970-01-01T00:00:00+00'::timestamptz
""", back_as=dict)
d = cls.db.one("SELECT current_payday();", back_as=dict)
log("Picking up with an existing payday.")

d['ts_start'] = d['ts_start'].replace(tzinfo=aspen.utils.utc)

log("Payday started at %s." % d['ts_start'])

payday = Payday()
Expand Down Expand Up @@ -154,7 +148,7 @@ def payin(self):
money internally between participants.
"""
with self.db.get_cursor() as cursor:
self.prepare(cursor, self.id, self.ts_start)
self.prepare(cursor)
holds = self.create_card_holds(cursor)
self.process_subscriptions(cursor)
self.transfer_takes(cursor, self.ts_start)
Expand All @@ -177,10 +171,10 @@ def payin(self):


@staticmethod
def prepare(cursor, payday_id, ts_start):
def prepare(cursor):
"""Prepare the DB: we need temporary tables with indexes and triggers.
"""
cursor.run(PAYDAY, dict(payday_id=payday_id, ts_start=ts_start))
cursor.run(PAYDAY)
log('Prepared the DB.')


Expand Down
12 changes: 12 additions & 0 deletions sql/branch.sql
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@ BEGIN;
DROP TABLE payments;


CREATE FUNCTION current_payday() RETURNS SETOF paydays AS $$
SELECT *
FROM paydays
WHERE ts_end='1970-01-01T00:00:00+00'::timestamptz;
$$ LANGUAGE sql;

CREATE FUNCTION current_payday_id() RETURNS int AS $$
-- This is a function so we can use it in DEFAULTS for a column.
SELECT id FROM current_payday();
$$ LANGUAGE sql;


-- Accounts

CREATE TYPE account_type AS ENUM ('asset', 'liability', 'income', 'expense');
Expand Down
13 changes: 7 additions & 6 deletions sql/payday.sql
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ CREATE TABLE payday_participants AS
, braintree_customer_id
FROM participants p
WHERE is_suspicious IS NOT true
AND claimed_time < %(ts_start)s
AND claimed_time < (SELECT ts_start FROM current_payday())
ORDER BY claimed_time;

CREATE UNIQUE INDEX ON payday_participants (id);
Expand Down Expand Up @@ -49,14 +49,14 @@ CREATE TABLE payday_teams AS

DROP TABLE IF EXISTS payday_journal_so_far;
CREATE TABLE payday_journal_so_far AS
SELECT * FROM journal WHERE payday = %(payday_id)s;
SELECT * FROM journal WHERE payday = (SELECT id FROM current_payday());

DROP TABLE IF EXISTS payday_subscriptions;
CREATE TABLE payday_subscriptions AS
SELECT subscriber, team, amount
FROM ( SELECT DISTINCT ON (subscriber, team) *
FROM subscriptions
WHERE mtime < %(ts_start)s
WHERE mtime < (SELECT ts_start FROM current_payday())
ORDER BY subscriber, team, mtime DESC
) s
JOIN payday_participants p ON p.username = s.subscriber
Expand Down Expand Up @@ -93,13 +93,14 @@ CREATE TABLE payday_journal
, amount numeric(35,2) NOT NULL
, debit bigint NOT NULL
, credit bigint NOT NULL
, payday int DEFAULT %(payday_id)s
, payday int NOT NULL DEFAULT current_payday_id()
);

CREATE OR REPLACE FUNCTION payday_update_balance() RETURNS trigger AS $$
DECLARE
to_debit text;
to_credit text;
to_debit text;
to_credit text;
balance_after_debit numeric(35, 2);
BEGIN
to_debit = (SELECT participant FROM accounts WHERE id=NEW.debit);
IF to_debit IS NOT NULL THEN
Expand Down
34 changes: 16 additions & 18 deletions tests/py/test_billing_payday.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,16 @@ def test_start_prepare(self, log):
self.make_participant('carl', balance=10, claimed_time='now')

payday = Payday.start()
ts_start = payday.ts_start

get_participants = lambda c: c.all("SELECT * FROM payday_participants")

with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, ts_start)
payday.prepare(cursor)
participants = get_participants(cursor)

expected_logging_call_args = [
('Starting a new payday.'),
('Payday started at {}.'.format(ts_start)),
('Payday started at {}.'.format(payday.ts_start)),
('Prepared the DB.'),
]
expected_logging_call_args.reverse()
Expand All @@ -192,13 +191,12 @@ def test_start_prepare(self, log):
log.reset_mock()

# run a second time, we should see it pick up the existing payday
payday = Payday.start()
second_ts_start = payday.ts_start
second_payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, second_payday.id, ts_start)
payday.prepare(cursor)
second_participants = get_participants(cursor)

assert ts_start == second_ts_start
assert payday.ts_start == second_payday.ts_start
participants = list(participants)
second_participants = list(second_participants)

Expand All @@ -208,7 +206,7 @@ def test_start_prepare(self, log):

expected_logging_call_args = [
('Picking up with an existing payday.'),
('Payday started at {}.'.format(second_ts_start)),
('Payday started at {}.'.format(second_payday.ts_start)),
('Prepared the DB.'),
]
expected_logging_call_args.reverse()
Expand Down Expand Up @@ -239,7 +237,7 @@ class TestPayin(BillingHarness):
def create_card_holds(self):
payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
return payday.create_card_holds(cursor)

@mock.patch.object(Payday, 'fetch_card_holds')
Expand Down Expand Up @@ -370,7 +368,7 @@ def test_payin_cant_make_balances_more_negative(self):
""")
payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
cursor.run("""
UPDATE payday_participants
SET new_balance = -50
Expand Down Expand Up @@ -408,7 +406,7 @@ def test_payday_journal_updates_participant_and_team_balances_for_payroll(self):

payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)

cursor.run("UPDATE payday_teams SET balance=20 WHERE slug='TheATeam'")
cursor.run("""
Expand All @@ -432,7 +430,7 @@ def test_payday_journal_updates_participant_and_team_balances_for_subscriptions(

payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)

cursor.run("""
INSERT INTO payday_journal
Expand All @@ -453,7 +451,7 @@ def test_payday_journal_disallows_negative_payday_team_balance(self):

payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
cursor.run("UPDATE payday_teams SET balance=10 WHERE slug='TheATeam'")
with pytest.raises(IntegrityError):
cursor.run("""
Expand All @@ -471,7 +469,7 @@ def test_payday_journal_disallows_negative_payday_participant_balance(self):

payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
with pytest.raises(IntegrityError):
cursor.run("""
INSERT INTO payday_journal
Expand All @@ -494,7 +492,7 @@ def test_process_subscriptions(self):

payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
payday.process_subscriptions(cursor)
assert cursor.one("select balance from payday_teams where slug='TheATeam'") == D('0.51')
assert cursor.one("select balance from payday_teams where slug='TheBTeam'") == 0
Expand Down Expand Up @@ -526,7 +524,7 @@ def test_transfer_takes(self):
# have already been processed
for i in range(3):
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
payday.transfer_takes(cursor, payday.ts_start)
payday.update_balances(cursor)

Expand All @@ -550,7 +548,7 @@ def test_process_draws(self):

payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
payday.process_subscriptions(cursor)
payday.transfer_takes(cursor, payday.ts_start)
payday.process_draws(cursor)
Expand Down Expand Up @@ -588,7 +586,7 @@ def test_take_over_during_payin(self):
alice.set_tip_to(bob, 18)
payday = Payday.start()
with self.db.get_cursor() as cursor:
payday.prepare(cursor, payday.id, payday.ts_start)
payday.prepare(cursor)
bruce = self.make_participant('bruce', claimed_time='now')
bruce.take_over(('twitter', str(bob.id)), have_confirmation=True)
payday.process_subscriptions(cursor)
Expand Down

0 comments on commit efe6a4c

Please sign in to comment.