Merge pull request #2374 from gittip/fix-payday

Fix payday

gittip/billing/payday.py

@@ -84,6 +84,9 @@ def __str__(self):
         return "No payday found where one was expected."
 
 
+LOOP_PAYIN, LOOP_PACHINKO, LOOP_PAYOUT = range(3)
+
+
 class Payday(object):
     """Represent an abstract event during which money is moved.
 
@@ -100,24 +103,31 @@ def __init__(self, db):
         self.db = db
 
 
-    def genparticipants(self, ts_start, for_payday):
-        """Generator to yield participants with tips and total.
-
-        We re-fetch participants each time, because the second time through
-        we want to use the total obligations they have for next week, and
-        if we pass a non-False for_payday to get_tips_and_total then we
-        only get unfulfilled tips from prior to that timestamp, which is
-        none of them by definition.
+    def genparticipants(self, ts_start, loop):
+        """Generator to yield participants with extra info.
 
-        If someone changes tips after payout starts, and we crash during
-        payout, then their new tips_and_total will be used on the re-run.
-        That's okay.
+        The extra info varies depending on which loop we're in: tips/total for
+        payin and payout, takes for pachinko.
 
         """
-        for participant in self.get_participants(ts_start):
-            tips, total = participant.get_tips_and_total(for_payday=for_payday)
-            typecheck(total, Decimal)
-            yield(participant, tips, total)
+        teams_only = (loop == LOOP_PACHINKO)
+        for participant in self.get_participants(ts_start, teams_only):
+            if loop == LOOP_PAYIN:
+                extra = participant.get_tips_and_total(for_payday=ts_start)
+            elif loop == LOOP_PACHINKO:
+                extra = participant.get_takes(for_payday=ts_start)
+            elif loop == LOOP_PAYOUT:
+
+                # On the payout loop we want to use the total obligations they
+                # have for next week, and if we pass a non-False for_payday to
+                # get_tips_and_total then we only get unfulfilled tips from
+                # prior to that timestamp, which is none of them by definition
+                # at this point since we just recently finished payin.
+
+                extra = participant.get_tips_and_total()
+            else:
+                raise Exception  # sanity check
+            yield(participant, extra)
 
 
     def run(self):
@@ -135,11 +145,11 @@ def run(self):
         ts_start = self.start()
         self.zero_out_pending(ts_start)
 
-        self.payin(ts_start, self.genparticipants(ts_start, ts_start))
+        self.payin(ts_start, self.genparticipants(ts_start, loop=LOOP_PAYIN))
         self.move_pending_to_balance_for_teams()
-        self.pachinko(ts_start, self.genparticipants(ts_start, ts_start))
+        self.pachinko(ts_start, self.genparticipants(ts_start, loop=LOOP_PACHINKO))
         self.clear_pending_to_balance()
-        self.payout(ts_start, self.genparticipants(ts_start, False))
+        self.payout(ts_start, self.genparticipants(ts_start, loop=LOOP_PAYOUT))
         self.set_nactive(ts_start)
 
         self.end()
@@ -205,7 +215,7 @@ def zero_out_pending(self, ts_start):
         return None
 
 
-    def get_participants(self, ts_start):
+    def get_participants(self, ts_start, teams_only=False):
         """Given a timestamp, return a list of participants dicts.
         """
         PARTICIPANTS = """\
@@ -214,8 +224,9 @@ def get_participants(self, ts_start):
              WHERE claimed_time IS NOT NULL
                AND claimed_time < %s
                AND is_suspicious IS NOT true
+               {}
           ORDER BY claimed_time ASC
-        """
+        """.format(teams_only and "AND number = 'plural'" or '')
         participants = self.db.all(PARTICIPANTS, (ts_start,))
         log("Fetched participants.")
         return participants
@@ -226,7 +237,7 @@ def payin(self, ts_start, participants):
         """
         i = 0
         log("Starting payin loop.")
-        for i, (participant, tips, total) in enumerate(participants, start=1):
+        for i, (participant, (tips, total)) in enumerate(participants, start=1):
             if i % 100 == 0:
                 log("Payin done for %d participants." % i)
             self.charge_and_or_transfer(ts_start, participant, tips, total)
@@ -235,11 +246,9 @@ def payin(self, ts_start, participants):
 
     def pachinko(self, ts_start, participants):
         i = 0
-        for i, (participant, foo, bar) in enumerate(participants, start=1):
+        for i, (participant, takes) in enumerate(participants, start=1):
             if i % 100 == 0:
                 log("Pachinko done for %d participants." % i)
-            if participant.number != 'plural':
-                continue
 
             available = participant.balance
             log("Pachinko out from %s with $%s." % ( participant.username
@@ -258,7 +267,7 @@ def tip(tippee, amount):
                         , pachinko=True
                          )
 
-            for take in participant.get_current_takes():
+            for take in takes:
                 amount = min(take['amount'], available)
                 available -= amount
                 tip(take['member'], amount)
@@ -273,7 +282,7 @@ def payout(self, ts_start, participants):
         """
         i = 0
         log("Starting payout loop.")
-        for i, (participant, tips, total) in enumerate(participants, start=1):
+        for i, (participant, (tips, total)) in enumerate(participants, start=1):
             if i % 100 == 0:
                 log("Payout done for %d participants." % i)
             self.ach_credit(ts_start, participant, tips, total)

gittip/models/_mixin_team.py

@@ -27,7 +27,7 @@ def show_as_team(self, user):
             return False
         if user.ADMIN:
             return True
-        if not self.get_current_takes():
+        if not self.get_takes():
             if self == user.participant:
                 return True
             return False
@@ -37,7 +37,7 @@ def add_member(self, member):
         """Add a member to this team.
         """
         assert self.IS_PLURAL
-        if len(self.get_current_takes()) == 149:
+        if len(self.get_takes()) == 149:
             raise MemberLimitReached
         self.__set_take_for(member, Decimal('0.01'), self)
 
@@ -51,7 +51,7 @@ def member_of(self, team):
         """Given a Participant object, return a boolean.
         """
         assert team.IS_PLURAL
-        for take in team.get_current_takes():
+        for take in team.get_takes():
             if take['member'] == self.username:
                 return True
         return False
@@ -131,18 +131,56 @@ def __set_take_for(self, member, amount, recorder):
         """, (member.username, self.username, member.username, self.username, \
                                                       amount, recorder.username))
 
-    def get_current_takes(self):
+    def get_takes(self, for_payday=False):
         """Return a list of member takes for a team.
+
+        This is implemented parallel to Participant.get_tips_and_total. See
+        over there for an explanation of for_payday.
+
         """
         assert self.IS_PLURAL
-        return self.db.all("""
 
-            SELECT member, amount, ctime, mtime
-              FROM current_takes
-             WHERE team=%s
-          ORDER BY ctime DESC
+        args = dict(team=self.username)
+
+        if for_payday:
+            args['ts_start'] = for_payday
+
+            # Get the takes for this team, as they were before ts_start,
+            # filtering out the ones we've already transferred (in case payday
+            # is interrupted and restarted).
+
+            TAKES = """\
+
+                SELECT * FROM (
+                     SELECT DISTINCT ON (member) t.*
+                       FROM takes t
+                       JOIN participants p ON p.username = member
+                      WHERE team=%(team)s
+                        AND mtime < %(ts_start)s
+                        AND p.is_suspicious IS NOT true
+                        AND ( SELECT id
+                                FROM transfers
+                               WHERE tipper=t.team
+                                 AND tippee=t.member
+                                 AND as_team_member IS true
+                                 AND timestamp >= %(ts_start)s
+                             ) IS NULL
+                   ORDER BY member, mtime DESC
+                ) AS foo
+                ORDER BY ctime DESC
+
+            """
+        else:
+            TAKES = """\
+
+                SELECT member, amount, ctime, mtime
+                  FROM current_takes
+                 WHERE team=%(team)s
+              ORDER BY ctime DESC
+
+            """
 
-        """, (self.username,), back_as=dict)
+        return self.db.all(TAKES, args, back_as=dict)
 
     def get_team_take(self):
         """Return a single take for a team, the team itself's take.
@@ -162,7 +200,7 @@ def get_members(self, current_participant):
         """Return a list of member dicts.
         """
         assert self.IS_PLURAL
-        takes = self.get_current_takes()
+        takes = self.get_takes()
         takes.append(self.get_team_take())
         budget = balance = self.get_dollars_receiving()
         members = []

tests/py/test_billing_payday.py

@@ -9,7 +9,7 @@
 
 from aspen.utils import typecheck, utcnow
 from gittip import billing
-from gittip.billing.payday import Payday, skim_credit
+from gittip.billing.payday import Payday, skim_credit, LOOP_PACHINKO
 from gittip.models.participant import Participant
 from gittip.testing import Harness
 from gittip.testing.balanced import BalancedHarness
@@ -838,11 +838,60 @@ def test_get_participants_gets_participants(self):
         assert actual == expected
 
     def test_pachinko_pachinkos(self):
-        a_team = self.make_participant('a_team', claimed_time='now', number='plural', balance=20, pending=0)
+        a_team = self.make_participant('a_team', claimed_time='now', number='plural', balance=20, \
+                                                                                         pending=0)
         a_team.add_member(self.make_participant('alice', claimed_time='now', balance=0, pending=0))
         a_team.add_member(self.make_participant('bob', claimed_time='now', balance=0, pending=0))
 
         ts_start = self.payday.start()
 
-        participants = self.payday.genparticipants(ts_start, ts_start)
+        participants = self.payday.genparticipants(ts_start, LOOP_PACHINKO)
         self.payday.pachinko(ts_start, participants)
+
+        assert Participant.from_username('alice').pending == D('0.01')
+        assert Participant.from_username('bob').pending == D('0.01')
+
+    def test_pachinko_sees_current_take(self):
+        a_team = self.make_participant('a_team', claimed_time='now', number='plural', balance=20, \
+                                                                                         pending=0)
+        alice = self.make_participant('alice', claimed_time='now', balance=0, pending=0)
+        a_team.add_member(alice)
+        a_team.set_take_for(alice, D('1.00'), alice)
+
+        ts_start = self.payday.start()
+
+        participants = self.payday.genparticipants(ts_start, LOOP_PACHINKO)
+        self.payday.pachinko(ts_start, participants)
+
+        assert Participant.from_username('alice').pending == D('1.00')
+
+    def test_pachinko_ignores_take_set_after_payday_starts(self):
+        a_team = self.make_participant('a_team', claimed_time='now', number='plural', balance=20, \
+                                                                                         pending=0)
+        alice = self.make_participant('alice', claimed_time='now', balance=0, pending=0)
+        a_team.add_member(alice)
+        a_team.set_take_for(alice, D('0.33'), alice)
+
+        ts_start = self.payday.start()
+        a_team.set_take_for(alice, D('1.00'), alice)
+
+        participants = self.payday.genparticipants(ts_start, LOOP_PACHINKO)
+        self.payday.pachinko(ts_start, participants)
+
+        assert Participant.from_username('alice').pending == D('0.33')
+
+    def test_pachinko_ignores_take_thats_already_been_processed(self):
+        a_team = self.make_participant('a_team', claimed_time='now', number='plural', balance=20, \
+                                                                                         pending=0)
+        alice = self.make_participant('alice', claimed_time='now', balance=0, pending=0)
+        a_team.add_member(alice)
+        a_team.set_take_for(alice, D('0.33'), alice)
+
+        ts_start = self.payday.start()
+        a_team.set_take_for(alice, D('1.00'), alice)
+
+        for i in range(4):
+            participants = self.payday.genparticipants(ts_start, LOOP_PACHINKO)
+            self.payday.pachinko(ts_start, participants)
+
+        assert Participant.from_username('alice').pending == D('0.33')