Merge pull request #4439 from gratipay/fix-export-csv-bug

Fix export CSV bug (#4399)

docs/index.rst

@@ -8,25 +8,46 @@ Welcome! This is the documentation for programmers working on `gratipay.com`_
 .. _web API: https://github.com/gratipay/gratipay.com#api
 
 
+.. _db-schema:
+
 DB Schema
 ---------
 
-is_suspipicous on participant can be None, True or False. It represents unknown,
-blacklisted or whitelisted user.
+Users
+^^^^^
+
+``is_suspicious`` on a participant can be ``None`` (unknown), ``True``
+(blacklisted) or ``False`` (whitelisted)
 
     * whitelisted can transfer money out of gratipay
     * unknown can move money within gratipay
     * blacklisted cannot do anything
 
+Money
+^^^^^
+
+- ``transfers``
+
+Used under Gratipay 1.0, when users were allowed to tip each other (without
+having to setup a team). ``transfers`` models money moving **within** Gratipay,
+from one participant (``tipper``) to another (``tippee``).
 
-The exchanges table records movements of money into and out of Gratipay. The
-``amount`` column shows a positive amount for payins and a negative amount for
-payouts. The ``fee`` column is always positive. For both payins and payouts,
+- ``payments``
+
+The replacement for ``transfers``, used in Gratipay 2.0. ``payments`` are
+between a Team and a Participant, in either direction (``to-team``, or
+``to-participant``)
+
+- ``exchanges``
+
+Records money moving into and out of Gratipay. Every ``exchange`` is linked to a
+participant. The ``amount`` column shows a positive amount for money flowing
+into gratipay (payins), and a negative amount for money flowing out of Gratipay
+(payouts). The ``fee`` column is always positive. For both payins and payouts,
 the ``amount`` does not include the ``fee`` (e.g., a $10 payin would result in
 an ``amount`` of ``9.41`` and a ``fee`` of ``0.59``, and a $100 payout with a
 2% fee would result in an ``amount`` of ``-98.04`` and a fee of ``1.96``).
 
-
 Contents
 --------
 

gratipay/utils/history.py

@@ -1,3 +1,9 @@
+"""Helpers to fetch logs of payments made to/from a participant.
+
+Data is fetched from 3 tables: `transfers`, `payments` and `exchanges`. For
+details on what these tables represent, see :ref:`db-schema`.
+"""
+
 from datetime import datetime
 from decimal import Decimal
 
@@ -23,6 +29,7 @@ def get_end_of_year_balance(db, participant, year, current_year):
 
     username = participant.username
     start_balance = get_end_of_year_balance(db, participant, year-1, current_year)
+
     delta = db.one("""
         SELECT (
                   SELECT COALESCE(sum(amount), 0) AS a
@@ -48,6 +55,18 @@ def get_end_of_year_balance(db, participant, year, current_year):
                     FROM transfers
                    WHERE tippee = %(username)s
                      AND extract(year from timestamp) = %(year)s
+               ) + (
+                  SELECT COALESCE(sum(amount), 0) AS a
+                    FROM payments
+                   WHERE participant = %(username)s
+                     AND direction = 'to-participant'
+                     AND extract(year from timestamp) = %(year)s
+               ) + (
+                  SELECT COALESCE(sum(-amount), 0) AS a
+                    FROM payments
+                   WHERE participant = %(username)s
+                     AND direction = 'to-team'
+                     AND extract(year from timestamp) = %(year)s
                ) AS delta
     """, locals())
     balance = start_balance + delta
@@ -160,57 +179,37 @@ def iter_payday_events(db, participant, year=None):
     yield dict(kind='day-close', balance=balance)
 
 
-def export_history(participant, year, mode, key, back_as='namedtuple', require_key=False):
+def export_history(participant, year, key, back_as='namedtuple', require_key=False):
     db = participant.db
     params = dict(username=participant.username, year=year)
     out = {}
-    if mode == 'aggregate':
-        out['given'] = lambda: db.all("""
-            SELECT tippee, sum(amount) AS amount
-              FROM transfers
-             WHERE tipper = %(username)s
-               AND extract(year from timestamp) = %(year)s
-          GROUP BY tippee
-        """, params, back_as=back_as)
-        out['taken'] = lambda: db.all("""
-            SELECT tipper AS team, sum(amount) AS amount
-              FROM transfers
-             WHERE tippee = %(username)s
-               AND context = 'take'
-               AND extract(year from timestamp) = %(year)s
-          GROUP BY tipper
-        """, params, back_as=back_as)
-    else:
-        out['exchanges'] = lambda: db.all("""
-            SELECT timestamp, amount, fee, status, note
-              FROM exchanges
-             WHERE participant = %(username)s
-               AND extract(year from timestamp) = %(year)s
-          ORDER BY timestamp ASC
-        """, params, back_as=back_as)
-        out['given'] = lambda: db.all("""
-            SELECT timestamp, tippee, amount, context
-              FROM transfers
-             WHERE tipper = %(username)s
-               AND extract(year from timestamp) = %(year)s
-          ORDER BY timestamp ASC
-        """, params, back_as=back_as)
-        out['taken'] = lambda: db.all("""
-            SELECT timestamp, tipper AS team, amount
-              FROM transfers
-             WHERE tippee = %(username)s
-               AND context = 'take'
-               AND extract(year from timestamp) = %(year)s
-          ORDER BY timestamp ASC
-        """, params, back_as=back_as)
-        out['received'] = lambda: db.all("""
-            SELECT timestamp, amount, context
-              FROM transfers
-             WHERE tippee = %(username)s
-               AND context NOT IN ('take', 'take-over')
-               AND extract(year from timestamp) = %(year)s
-          ORDER BY timestamp ASC
-        """, params, back_as=back_as)
+
+    out['given'] = lambda: db.all("""
+        SELECT CONCAT('~', tippee) as tippee, sum(amount) AS amount
+          FROM transfers
+         WHERE tipper = %(username)s
+           AND extract(year from timestamp) = %(year)s
+      GROUP BY tippee
+
+         UNION
+
+        SELECT team as tippee, sum(amount) AS amount
+          FROM payments
+         WHERE participant = %(username)s
+           AND direction = 'to-team'
+           AND extract(year from timestamp) = %(year)s
+      GROUP BY tippee
+    """, params, back_as=back_as)
+
+    # FIXME: Include values from the `payments` table
+    out['taken'] = lambda: db.all("""
+        SELECT tipper AS team, sum(amount) AS amount
+          FROM transfers
+         WHERE tippee = %(username)s
+           AND context = 'take'
+           AND extract(year from timestamp) = %(year)s
+      GROUP BY tipper
+    """, params, back_as=back_as)
 
     if key:
         try:

sql/branch.sql

@@ -0,0 +1,3 @@
+-- A bug was discovered in https://github.com/gratipay/gratipay.com/pull/4439
+-- Let's delete all cached entries in the table, they'll be regenerated with proper values.
+DELETE FROM balances_at;

tests/py/test_history.py

@@ -6,33 +6,71 @@
 from mock import patch
 
 from gratipay.billing.payday import Payday
+from gratipay.models.participant import Participant
 from gratipay.testing import Harness, D,P
 from gratipay.testing.billing import BillingHarness
 from gratipay.utils.history import get_end_of_year_balance, iter_payday_events
 
 
 def make_history(harness):
-    alice = harness.make_participant('alice', claimed_time=datetime(2001, 1, 1, 0, 0, 0))
-    harness.alice = alice
-    harness.make_exchange('braintree-cc', 50, 0, alice)
+    alice = harness.make_participant('alice', claimed_time=datetime(2001, 1, 1))
+
+    # Exchanges for the previous year
+    harness.make_exchange('braintree-cc', 60, 0, alice)
     harness.make_exchange('braintree-cc', 12, 0, alice, status='failed')
     harness.make_exchange('paypal', -40, 0, alice)
     harness.make_exchange('paypal', -5, 0, alice, status='failed')
-    harness.db.run("""
+    past_year = harness.db.all("""
         UPDATE exchanges
            SET timestamp = "timestamp" - interval '1 year'
-    """)
-    harness.past_year = int(harness.db.one("""
-        SELECT extract(year from timestamp)
-          FROM exchanges
-      ORDER BY timestamp ASC
-         LIMIT 1
-    """))
-    harness.make_exchange('braintree-cc', 35, 0, alice)
+     RETURNING extract(year from timestamp)::int
+    """)[0]
+
+    # Exchanges for the current year
+    harness.make_exchange('braintree-cc', 45, 0, alice)
     harness.make_exchange('braintree-cc', 49, 0, alice, status='failed')
     harness.make_exchange('paypal', -15, 0, alice)
     harness.make_exchange('paypal', -7, 0, alice, status='failed')
 
+    # Tips under Gratipay 1.0
+    harness.make_participant('bob')
+    tips = [
+        {'timestamp': datetime(past_year, 1, 1), 'amount': 10},
+        {'timestamp': datetime(past_year+1, 1, 1), 'amount': 20}
+    ]
+    for tip in tips:
+        harness.db.run("""
+            INSERT INTO transfers (tipper, tippee, amount, context, timestamp)
+                 VALUES ('alice', 'bob', %(amount)s, 'tip', %(timestamp)s);
+
+                 UPDATE participants
+                    SET balance = (balance - %(amount)s)
+                  WHERE username = 'alice';
+
+                 UPDATE participants
+                    SET balance = (balance + %(amount)s)
+                  WHERE username = 'bob';
+        """, tip)
+
+    # Payments under Gratipay 2.x
+    Enterprise = harness.make_team(is_approved=True)
+    payday_id = harness.db.one("""
+        INSERT INTO paydays
+                    (ts_start, ts_end)
+             VALUES (CURRENT_TIMESTAMP, CURRENT_TIMESTAMP + interval '1 hour')
+          RETURNING id
+    """)
+    alice.set_payment_instruction(Enterprise, '10.00')  # >= MINIMUM_CHARGE!
+    harness.make_payment(alice, Enterprise, 10, 'to-team', payday_id)
+    harness.db.run("""
+        UPDATE participants
+           SET balance = balance - 10
+         WHERE username = 'alice'
+    """)
+
+    harness.alice = Participant.from_username('alice')
+    harness.past_year = past_year
+
 
 class TestHistory(BillingHarness):
 
@@ -116,8 +154,17 @@ def test_iter_payday_events_with_failed_exchanges(self):
 
     def test_get_end_of_year_balance(self):
         make_history(self)
-        balance = get_end_of_year_balance(self.db, self.alice, self.past_year, datetime.now().year)
-        assert balance == 10
+
+        past_year, current_year = self.past_year, self.past_year+1
+
+        balance = get_end_of_year_balance(self.db, self.alice, past_year, current_year+1)
+        assert balance == 10 # +60(payin), -40(payout), -10(bob)
+
+        balance = get_end_of_year_balance(self.db, self.alice, current_year, current_year+1)
+        assert balance == 10 # +45(payin), -15(payout), -20(bob), -10(enterprise)
+
+        balance = get_end_of_year_balance(self.db, self.alice, current_year+1, current_year+1)
+        assert balance == 10 # Should return balance directly
 
 
 class TestHistoryPage(Harness):
@@ -130,11 +177,11 @@ def test_participant_can_view_history(self):
         assert self.client.GET('/~alice/history/', auth_as='alice').code == 200
 
     def test_admin_can_view_closed_participant_history(self):
-        self.make_exchange('braintree-cc', -30, 0, self.alice)
+        self.make_exchange('paypal', -10, 0, self.alice)
         self.alice.close()
 
-        self.make_participant('bob', claimed_time='now', is_admin=True)
-        response = self.client.GET('/~alice/history/?year=%s' % self.past_year, auth_as='bob')
+        self.make_participant('carl', claimed_time='now', is_admin=True)
+        response = self.client.GET('/~alice/history/?year=%s' % self.past_year, auth_as='carl')
         assert "automatic charge" in response.body
 
 class TestExport(Harness):
@@ -145,16 +192,31 @@ def setUp(self):
 
     def test_export_json(self):
         r = self.client.GET('/~alice/history/export.json', auth_as='alice')
-        assert json.loads(r.body)
 
-    def test_export_json_aggregate(self):
-        r = self.client.GET('/~alice/history/export.json?mode=aggregate', auth_as='alice')
-        assert json.loads(r.body)
+        response = json.loads(r.body)
 
-    def test_export_json_past_year(self):
-        r = self.client.GET('/~alice/history/export.json?year=%s' % self.past_year, auth_as='alice')
-        assert len(json.loads(r.body)['exchanges']) == 4
+        assert len(response['given']) == 2
+        assert {'amount': 10, 'tippee': 'TheEnterprise'} in response['given']
+        assert {'amount': 20, 'tippee': '~bob'} in response['given']
+
+        # TODO: Add tests for 'taken'
+
+    def test_export_json_for_year(self):
+        r = self.client.GET('/~alice/history/export.json?year=%s' % (self.past_year), auth_as='alice')
+        expected = {
+            'given': [{'amount': 10, 'tippee': '~bob'}],
+            'taken': []
+        }
+        actual = json.loads(r.body)
+        assert expected == actual
 
     def test_export_csv(self):
-        r = self.client.GET('/~alice/history/export.csv?key=exchanges', auth_as='alice')
-        assert r.body.count('\n') == 5
+        r = self.client.GET('/~alice/history/export.csv?key=given', auth_as='alice')
+
+        lines = r.body.strip().split('\n')
+        assert len(lines) == 3 # header + 2
+        assert lines.pop(0).strip() == 'tippee,amount'
+
+        lines.sort()
+        assert lines[0].strip() == 'TheEnterprise,10.00'
+        assert lines[1].strip() == '~bob,20.00'

www/~/%username/history/export.spt

@@ -18,10 +18,9 @@ except ValueError:
     raise Response(400, "bad year")
 
 key = request.qs.get('key')
-mode = request.qs.get('mode')
 
 [---] text/csv via csv_dump
-export_history(participant, year, mode, key, require_key=True)
+export_history(participant, year, key, require_key=True)
 
 [---] application/json via json_dump
-export_history(participant, year, mode, key, back_as=dict)
+export_history(participant, year, key, back_as=dict)

www/~/%username/history/index.html.spt

@@ -58,7 +58,7 @@ else:
     <tr><td colspan="8" class="totals">
         {{ _("Total given: {0}", format_currency(event['given'], "USD")) }}
             {% if event['given'] %}
-            (<a href="export.csv?year={{ year }}&amp;key=given&amp;mode=aggregate">{{
+            (<a href="export.csv?year={{ year }}&amp;key=given">{{
                 _("Export as CSV")
             }}</a>)
             {% endif %}