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

Charge in arrears #3675

Merged
merged 17 commits into from
Sep 7, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions gratipay/billing/payday.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import aspen.utils
from aspen import log
from gratipay.billing.exchanges import (
cancel_card_hold, capture_card_hold, create_card_hold, upcharge,
cancel_card_hold, capture_card_hold, create_card_hold, upcharge, MINIMUM_CHARGE,
)
from gratipay.exceptions import NegativeBalance
from gratipay.models import check_db
Expand Down Expand Up @@ -220,17 +220,23 @@ def f(p):
if p.old_balance < 0:
amount -= p.old_balance
if p.id in holds:
charge_amount = upcharge(amount)[0]
if holds[p.id].amount >= charge_amount:
return
if amount >= MINIMUM_CHARGE:
charge_amount = upcharge(amount)[0]
if holds[p.id].amount >= charge_amount:
return
else:
# The amount is too low, cancel the hold and make a new one
cancel_card_hold(holds.pop(p.id))
else:
# The amount is too low, cancel the hold and make a new one
# not up to minimum charge level. cancel the hold
cancel_card_hold(holds.pop(p.id))
hold, error = create_card_hold(self.db, p, amount)
if error:
return 1
else:
holds[p.id] = hold
return
if amount >= MINIMUM_CHARGE:
hold, error = create_card_hold(self.db, p, amount)
if error:
return 1
else:
holds[p.id] = hold
threaded_map(f, participants)

# Update the values of card_hold_ok in our temporary table
Expand Down Expand Up @@ -341,6 +347,7 @@ def update_balances(cursor):
SELECT *, (SELECT id FROM paydays WHERE extract(year from ts_end) = 1970)
FROM payday_payments;
""")

log("Updated the balances of %i participants." % len(participants))


Expand Down
46 changes: 46 additions & 0 deletions gratipay/models/participant.py
Original file line number Diff line number Diff line change
Expand Up @@ -854,10 +854,13 @@ def set_payment_instruction(self, team, amount, update_self=True, update_team=Tr
"""
args = dict(participant=self.username, team=team.slug, amount=amount)
t = (cursor or self.db).one(NEW_PAYMENT_INSTRUCTION, args)
t_dict = t._asdict()

if update_self:
# Update giving amount of participant
self.update_giving(cursor)
# Carry over any existing due
self.update_due(t_dict['team'], t_dict['id'], cursor)
if update_team:
# Update receiving amount of team
team.update_receiving(cursor)
Expand Down Expand Up @@ -890,6 +893,24 @@ def get_payment_instruction(self, team):
""", (self.username, team.slug), back_as=dict, default=default)


def get_due(self, team):
"""Given a slug, return a Decimal.
"""
if not isinstance(team, Team):
team, slug = Team.from_slug(team), team
if not team:
raise NoTeam(slug)

return self.db.one("""\

SELECT due
FROM current_payment_instructions
WHERE participant = %s
AND team = %s

""", (self.username, team.slug))


def get_giving_for_profile(self):
"""Return a list and a Decimal.
"""
Expand Down Expand Up @@ -984,6 +1005,31 @@ def update_giving(self, cursor=None):

return updated

def update_due(self, team, id, cursor=None):
"""Transfer existing due value to newly inserted record
"""
# Copy due to new record
(cursor or self.db).run("""
UPDATE payment_instructions p
SET due = COALESCE((
SELECT due
FROM payment_instructions s
WHERE participant=%(username)s
AND team = %(team)s
AND due > 0
), 0)
WHERE p.id = %(id)s
""", dict(username=self.username,team=team,id=id))

# Reset older due values to 0
(cursor or self.db).run("""
UPDATE payment_instructions p
SET due = 0
WHERE participant = %(username)s
AND team = %(team)s
AND due > 0
AND p.id != %(id)s
""", dict(username=self.username,team=team,id=id))

def update_taking(self, cursor=None):
(cursor or self.db).run("""
Expand Down
27 changes: 27 additions & 0 deletions sql/branch.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
BEGIN;

ALTER TABLE payment_instructions ADD COLUMN due numeric(35,2) DEFAULT 0;

-- Recreate the current_payment_instructions view to pick up due.
DROP VIEW current_payment_instructions;
CREATE VIEW current_payment_instructions AS
SELECT DISTINCT ON (participant, team) *
FROM payment_instructions
ORDER BY participant, team, mtime DESC;

-- Allow updating is_funded and due via the current_payment_instructions view for convenience.
DROP FUNCTION update_payment_instruction();
CREATE FUNCTION update_payment_instruction() RETURNS trigger AS $$
BEGIN
UPDATE payment_instructions
SET is_funded = NEW.is_funded
, due = NEW.due
WHERE id = NEW.id;
RETURN NULL;
END;
$$ LANGUAGE plpgsql;

CREATE TRIGGER update_current_payment_instruction
INSTEAD OF UPDATE ON current_payment_instructions
FOR EACH ROW EXECUTE PROCEDURE update_payment_instruction();
END;
49 changes: 42 additions & 7 deletions sql/payday.sql
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ CREATE TABLE payday_payments_done AS

DROP TABLE IF EXISTS payday_payment_instructions;
CREATE TABLE payday_payment_instructions AS
SELECT participant, team, amount
SELECT s.id, participant, team, amount, due
FROM ( SELECT DISTINCT ON (participant, team) *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This subquery can be replaced with a query against current_payment_instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current subquery has this:
WHERE mtime < %(ts_start)s
while current_payment_instructions does not.
Can this be a problem?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we'll need to move that WHERE clause up into the parent query.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, ts_start is defined at the start of payday. This would make it
difficult to incorporate this into the current_payment_instructions view as a condition that would be meaningful at other times as well as at payday.

Moving the condition into the parent query may not help, if current_payment_instructions has already picked up an incorrect record as the current one.

I'd also like to understand the situation where mtime in payment_instructions
could/would be >= payday ts_start. e.g. can a user specify a start time that is in the future,
for a subscription? Was unable to find any such provision in the dev environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made all changes except for this particular one, because I'm not sure about the filtering by ts_start. As I see it, payment_instructions can only have a record with mtime > ts_start if a new subscription is created after the payday run has been started. And if this situation comes up, we do have a problem - the current_payment_instructions view will pick up this latest record, while payday_payment_instructions (as it is now) will not. If we do update the subquery, this could have different results depending on the exact point in payday when the new subscription is created.

Once again, I've got a 'diverged' situation after rebasing with master:

Your branch and 'origin/charge-in-arrears' have diverged,
and have 196 and 14 different commits each, respectively.

However, this seems ok after checking the log - will just have to do a forced push like last time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[T]he current_payment_instructions view will pick up this latest record[.]

Good point. Withdrawn. :-)

FROM payment_instructions
WHERE mtime < %(ts_start)s
Expand All @@ -77,11 +77,11 @@ CREATE INDEX ON payday_payment_instructions (team);
ALTER TABLE payday_payment_instructions ADD COLUMN is_funded boolean;

ALTER TABLE payday_participants ADD COLUMN giving_today numeric(35,2);
UPDATE payday_participants
UPDATE payday_participants pp
SET giving_today = COALESCE((
SELECT sum(amount)
SELECT sum(amount + due)
FROM payday_payment_instructions
WHERE participant = username
WHERE participant = pp.username
), 0);

DROP TABLE IF EXISTS payday_takes;
Expand All @@ -108,6 +108,7 @@ RETURNS void AS $$
DECLARE
participant_delta numeric;
team_delta numeric;
payload json;
BEGIN
IF ($3 = 0) THEN RETURN; END IF;

Expand All @@ -125,6 +126,17 @@ RETURNS void AS $$
UPDATE payday_teams
SET balance = (balance + team_delta)
WHERE slug = $2;
UPDATE current_payment_instructions
SET due = 0
WHERE participant = $1
AND team = $2
AND due > 0;
IF ($4 = 'to-team') THEN
payload = '{"action":"pay","participant":"' || $1 || '", "team":"'
|| $2 || '", "amount":' || $3 || '}';
INSERT INTO events(type, payload)
VALUES ('payday',payload);
END IF;
INSERT INTO payday_payments
(participant, team, amount, direction)
VALUES ( ( SELECT p.username
Expand All @@ -141,6 +153,27 @@ RETURNS void AS $$
END;
$$ LANGUAGE plpgsql;

-- Add payments that were not met on to due

CREATE OR REPLACE FUNCTION park(text, text, numeric)
RETURNS void AS $$
DECLARE payload json;
BEGIN
IF ($3 = 0) THEN RETURN; END IF;

UPDATE current_payment_instructions
SET due = $3
WHERE participant = $1
AND team = $2;

payload = '{"action":"due","participant":"' || $1 || '", "team":"'
|| $2 || '", "due":' || $3 || '}';
INSERT INTO events(type, payload)
VALUES ('payday',payload);

END;
$$ LANGUAGE plpgsql;


-- Create a trigger to process payment_instructions

Expand All @@ -153,9 +186,12 @@ CREATE OR REPLACE FUNCTION process_payment_instruction() RETURNS trigger AS $$
FROM payday_participants p
WHERE username = NEW.participant
);
IF (NEW.amount <= participant.new_balance OR participant.card_hold_ok) THEN
EXECUTE pay(NEW.participant, NEW.team, NEW.amount, 'to-team');
IF (NEW.amount + NEW.due <= participant.new_balance OR participant.card_hold_ok) THEN
EXECUTE pay(NEW.participant, NEW.team, NEW.amount + NEW.due, 'to-team');
RETURN NEW;
ELSE
EXECUTE park(NEW.participant, NEW.team, NEW.amount + NEW.due);
RETURN NULL;
END IF;
RETURN NULL;
END;
Expand All @@ -166,7 +202,6 @@ 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 $$
Expand Down
Loading