From 1b3a3730e35bf5d30a00afecb8396d14dde504f5 Mon Sep 17 00:00:00 2001 From: rorepo Date: Wed, 19 Aug 2015 08:56:07 +0100 Subject: [PATCH] Refined and added tests --- gratipay/billing/payday.py | 43 ++++++--------------------------- sql/payday.sql | 23 +++++++++++++++--- tests/py/test_billing_payday.py | 40 ++++++++++++++++++++++-------- tests/py/test_history.py | 15 ++++++------ 4 files changed, 66 insertions(+), 55 deletions(-) diff --git a/gratipay/billing/payday.py b/gratipay/billing/payday.py index 66ce3bb479..939f93f9e2 100644 --- a/gratipay/billing/payday.py +++ b/gratipay/billing/payday.py @@ -236,12 +236,14 @@ def f(p): # not up to minimum charge level. cancel the hold cancel_card_hold(holds.pop(p.id)) return - hold, error = create_card_hold(self.db, p, amount) - if error: - return 1 - else: - holds[p.id] = hold + if amount >= MINIMUM_CHARGE: + hold, error = create_card_hold(self.db, p, amount) + if error: + return 1 + else: + holds[p.id] = hold n_failures = sum(filter(None, threaded_map(f, participants))) + # Record the number of failures cursor.one(""" @@ -302,36 +304,6 @@ def park_payment_instructions(cursor): """) - @staticmethod - def park_payment_instructions(cursor): - """In the case of participants in whose case the amount to be charged to their cc's - in order to meet all outstanding and current subscriptions does not reach the minimum - charge threshold, park this for the next week by adding the current subscription amount - to giving_due - """ - cursor.run(""" - - INSERT INTO participants_payments_uncharged - SELECT participant - FROM payday_payment_instructions - GROUP BY participant - HAVING SUM(amount + giving_due) < %(MINIMUM_CHARGE)s - - """, dict(MINIMUM_CHARGE=MINIMUM_CHARGE)) - - cursor.run(""" - - UPDATE payment_instructions - SET giving_due = amount + giving_due - WHERE id IN ( SELECT id - FROM payday_payment_instructions ppi - , participants_payments_uncharged ppu - WHERE ppi.participant = ppu.participant - ) - - """) - - @staticmethod def transfer_takes(cursor, ts_start): return # XXX Bring me back! @@ -426,6 +398,7 @@ def update_balances(cursor): FROM payday_payment_instructions pi2 WHERE pi.id = pi2.id """) + log("Updated the balances of %i participants." % len(participants)) diff --git a/sql/payday.sql b/sql/payday.sql index 72aaf4053a..c932ec30e5 100644 --- a/sql/payday.sql +++ b/sql/payday.sql @@ -86,16 +86,16 @@ UPDATE payday_payment_instructions ppi DROP TABLE IF EXISTS participants_payments_uncharged; CREATE TABLE participants_payments_uncharged AS - SELECT participant + SELECT id, giving_due FROM payday_payment_instructions WHERE 1 = 2; 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 + giving_due) FROM payday_payment_instructions - WHERE participant = username + WHERE participant = pp.username ), 0); DROP TABLE IF EXISTS payday_takes; @@ -160,6 +160,20 @@ RETURNS void AS $$ END; $$ LANGUAGE plpgsql; +-- Add payments that were not met on to giving_due + +CREATE OR REPLACE FUNCTION park(text, text, numeric) +RETURNS void AS $$ + BEGIN + IF ($3 = 0) THEN RETURN; END IF; + + UPDATE payday_payment_instructions + SET giving_due = $3 + WHERE participant = $1 + AND team = $2; + END; +$$ LANGUAGE plpgsql; + -- Create a trigger to process payment_instructions @@ -175,6 +189,9 @@ CREATE OR REPLACE FUNCTION process_payment_instruction() RETURNS trigger AS $$ IF (NEW.amount + NEW.giving_due <= participant.new_balance OR participant.card_hold_ok) THEN EXECUTE pay(NEW.participant, NEW.team, NEW.amount + NEW.giving_due, 'to-team'); RETURN NEW; + ELSE + EXECUTE park(NEW.participant, NEW.team, NEW.amount + NEW.giving_due); + RETURN NULL; END IF; RETURN NULL; END; diff --git a/tests/py/test_billing_payday.py b/tests/py/test_billing_payday.py index b2da17eb37..7197180a76 100644 --- a/tests/py/test_billing_payday.py +++ b/tests/py/test_billing_payday.py @@ -8,7 +8,7 @@ import mock import pytest -from gratipay.billing.exchanges import create_card_hold +from gratipay.billing.exchanges import create_card_hold, MINIMUM_CHARGE from gratipay.billing.payday import NoPayday, Payday from gratipay.exceptions import NegativeBalance from gratipay.models.participant import Participant @@ -19,18 +19,38 @@ class TestPayday(BillingHarness): - def test_payday_moves_money(self): + def test_payday_moves_money_above_min_charge(self): A = self.make_team(is_approved=True) - self.obama.set_payment_instruction(A, '6.00') # under $10! + self.obama.set_payment_instruction(A, MINIMUM_CHARGE) # must be >= MINIMUM_CHARGE with mock.patch.object(Payday, 'fetch_card_holds') as fch: fch.return_value = {} Payday.start().run() obama = Participant.from_username('obama') hannibal = Participant.from_username('hannibal') + + assert hannibal.balance == D(MINIMUM_CHARGE) + assert obama.balance == D('0.00') + + @mock.patch.object(Payday, 'fetch_card_holds') + def test_payday_does_not_move_money_below_min_charge(self, fch): + A = self.make_team(is_approved=True) + self.obama.set_payment_instruction(A, '6.00') # not enough to reach MINIMUM_CHARGE + fch.return_value = {} + Payday.start().run() + + obama = Participant.from_username('obama') + hannibal = Participant.from_username('hannibal') + giving_due = self.db.one("""SELECT giving_due + FROM payment_instructions ppi + WHERE ppi.participant = 'obama' + AND ppi.team = 'TheATeam' + """) + + assert hannibal.balance == D('0.00') + assert obama.balance == D('0.00') + assert giving_due == D('6.00') - assert hannibal.balance == D('6.00') - assert obama.balance == D('3.41') @mock.patch.object(Payday, 'fetch_card_holds') def test_payday_doesnt_move_money_from_a_suspicious_account(self, fch): @@ -40,7 +60,7 @@ def test_payday_doesnt_move_money_from_a_suspicious_account(self, fch): WHERE username = 'obama' """) team = self.make_team(owner=self.homer, is_approved=True) - self.obama.set_payment_instruction(team, '6.00') # under $10! + self.obama.set_payment_instruction(team, MINIMUM_CHARGE) # >= MINIMUM_CHARGE! fch.return_value = {} Payday.start().run() @@ -58,7 +78,7 @@ def test_payday_doesnt_move_money_to_a_suspicious_account(self, fch): WHERE username = 'homer' """) team = self.make_team(owner=self.homer, is_approved=True) - self.obama.set_payment_instruction(team, '6.00') # under $10! + self.obama.set_payment_instruction(team, MINIMUM_CHARGE) # >= MINIMUM_CHARGE! fch.return_value = {} Payday.start().run() @@ -247,10 +267,10 @@ def create_card_holds(self): def test_payin_pays_in(self, sale, sfs, fch): fch.return_value = {} team = self.make_team('Gratiteam', is_approved=True) - self.obama.set_payment_instruction(team, 1) + self.obama.set_payment_instruction(team, MINIMUM_CHARGE) # >= MINIMUM_CHARGE txn_attrs = { - 'amount': 1, + 'amount': MINIMUM_CHARGE, 'tax_amount': 0, 'status': 'authorized', 'custom_fields': {'participant_id': self.obama.id}, @@ -268,7 +288,7 @@ def test_payin_pays_in(self, sale, sfs, fch): Payday.start().payin() payments = self.db.all("SELECT amount, direction FROM payments") - assert payments == [(1, 'to-team'), (1, 'to-participant')] + assert payments == [(MINIMUM_CHARGE, 'to-team'), (MINIMUM_CHARGE, 'to-participant')] @mock.patch('braintree.Transaction.sale') def test_payin_doesnt_try_failed_cards(self, sale): diff --git a/tests/py/test_history.py b/tests/py/test_history.py index f2ce682ecc..bddfc7cef0 100644 --- a/tests/py/test_history.py +++ b/tests/py/test_history.py @@ -3,6 +3,7 @@ from datetime import datetime from decimal import Decimal as D import json +import time from mock import patch @@ -40,9 +41,8 @@ class TestHistory(BillingHarness): def test_iter_payday_events(self): Payday().start().run() - A = self.make_team(is_approved=True) - self.obama.set_payment_instruction(A, '6.00') # under $10! + self.obama.set_payment_instruction(A, '10.00') # >= MINIMUM_CHARGE! for i in range(2): with patch.object(Payday, 'fetch_card_holds') as fch: fch.return_value = {} @@ -57,11 +57,12 @@ def test_iter_payday_events(self): SET timestamp = "timestamp" - interval '1 week'; """) + obama = Participant.from_username('obama') hannibal = Participant.from_username('hannibal') - assert obama.balance == D('6.82') - assert hannibal.balance == D('12.00') + assert obama.balance == D('0.00') + assert hannibal.balance == D('20.00') Payday().start() # to demonstrate that we ignore any open payday? @@ -69,15 +70,15 @@ def test_iter_payday_events(self): assert len(events) == 7 assert events[0]['kind'] == 'totals' assert events[0]['given'] == 0 - assert events[0]['received'] == 12 + assert events[0]['received'] == 20 assert events[1]['kind'] == 'day-open' assert events[1]['payday_number'] == 2 - assert events[2]['balance'] == 12 + assert events[2]['balance'] == 20 assert events[-1]['kind'] == 'day-close' assert events[-1]['balance'] == 0 events = list(iter_payday_events(self.db, obama)) - assert events[0]['given'] == 12 + assert events[0]['given'] == 20 assert len(events) == 11 def test_iter_payday_events_with_failed_exchanges(self):