Cherry-pick #4023

gratipay/models/participant/__init__.py

@@ -37,6 +37,7 @@
 from gratipay.models.account_elsewhere import AccountElsewhere
 from gratipay.models.exchange_route import ExchangeRoute
 from gratipay.models.team import Team
+from gratipay.models.team.mixins.takes import ZERO
 from gratipay.models.participant import mixins
 from gratipay.security.crypto import constant_time_compare
 from gratipay.utils import (
@@ -336,9 +337,8 @@ def clear_payment_instructions(self, cursor):
     def clear_takes(self, cursor):
         """Leave all teams by zeroing all takes.
         """
-        for team, nmembers in self.get_old_teams():
-            t = Participant.from_username(team)
-            t.set_take_for(self, Decimal(0), self, cursor)
+        for team in self.get_teams():
+            team.set_take_for(self, ZERO, cursor=cursor)
 
 
     def clear_personal_information(self, cursor):
@@ -1070,10 +1070,17 @@ def profile_url(self):
 
 
     def get_teams(self, only_approved=False, cursor=None):
-        """Return a list of teams this user is the owner of.
+        """Return a list of teams this user is an owner or member of.
         """
-        teams = (cursor or self.db).all( "SELECT teams.*::teams FROM teams WHERE owner=%s"
-                                       , (self.username,)
+        teams = (cursor or self.db).all("""
+            SELECT teams.*::teams FROM teams WHERE owner=%s
+
+            UNION
+
+            SELECT teams.*::teams FROM teams WHERE id IN (
+                SELECT team_id FROM current_takes WHERE participant_id=%s
+            )
+        """, (self.username, self.id)
                                         )
         if only_approved:
             teams = [t for t in teams if t.is_approved]
@@ -1089,22 +1096,6 @@ def member_of(self, team):
         return False
 
 
-    def get_old_teams(self):
-        """Return a list of old-style teams this user was a member of.
-        """
-        return self.db.all("""
-
-            SELECT team AS name
-                 , ( SELECT count(*)
-                       FROM current_takes
-                      WHERE team=x.team
-                    ) AS nmembers
-              FROM current_takes x
-             WHERE member=%s;
-
-        """, (self.username,))
-
-
     def insert_into_communities(self, is_member, name, slug):
         participant_id = self.id
         self.db.run("""
@@ -1188,14 +1179,14 @@ def get_age_in_seconds(self):
         return out
 
 
-    class StillATeamOwner(Exception): pass
+    class StillOnATeam(Exception): pass
     class BalanceIsNotZero(Exception): pass
 
     def final_check(self, cursor):
         """Sanity-check that teams and balance have been dealt with.
         """
         if self.get_teams(cursor=cursor):
-            raise self.StillATeamOwner
+            raise self.StillOnATeam
         if self.balance != 0:
             raise self.BalanceIsNotZero
 

gratipay/models/team/__init__.py

@@ -9,6 +9,7 @@
 from aspen import json, log
 from gratipay.exceptions import InvalidTeamName
 from gratipay.models import add_event
+from gratipay.models.team import mixins
 from postgres.orm import Model
 
 from gratipay.billing.exchanges import MINIMUM_CHARGE
@@ -50,6 +51,23 @@ def __ne__(self, other):
         return self.id != other.id
 
 
+    # Computed Values
+    # ===============
+
+    #: The total amount of money this team receives during payday. Read-only;
+    #: modified by
+    #: :py:meth:`~gratipay.models.participant.Participant.set_payment_instruction`.
+
+    receiving = 0
+
+
+    #: The number of participants that are giving to this team. Read-only;
+    #: modified by
+    #: :py:meth:`~gratipay.models.participant.Participant.set_payment_instruction`.
+
+    nreceiving_from = 0
+
+
     # Constructors
     # ============
 

gratipay/models/team/mixins/takes.py

@@ -9,9 +9,29 @@
 
 
 class TakesMixin(object):
-    """Members can take money from a Team.
+    """:py:class:`~gratipay.models.participant.Participant` s who are members
+    of a :py:class:`~gratipay.models.team.Team` may take money from the team
+    during :py:class:`~gratipay.billing.payday.Payday`. Only the team owner may
+    add a new member, by setting their take to a penny, but team owners may
+    *only* set their take to a penny---no more. Team owners may also remove
+    members, by setting their take to zero, as may the members themselves, who
+    may also set their take to whatever they wish.
     """
 
+    #: The total amount of money the team distributes to participants
+    #: (including the owner) during payday. Read-only; equal to
+    #: :py:attr:`~gratipay.models.team.Team.receiving`.
+
+    distributing = 0
+
+
+    #: The number of participants (including the owner) that the team
+    #: distributes money to during payday. Read-only; modified by
+    #: :py:meth:`set_take_for`.
+
+    ndistributing_to = 0
+
+
     def get_take_last_week_for(self, member):
         """Get the user's nominal take last week.
         """
@@ -33,54 +53,99 @@ def get_take_last_week_for(self, member):
         """, (self.username, membername), default=ZERO)
 
 
-    def get_take_for(self, member):
-        """Return a Decimal representation of the take for this member, or 0.
-        """
-        return self.db.one( "SELECT amount FROM current_takes "
-                            "WHERE member=%s AND team=%s"
-                          , (member.username, self.username)
-                          , default=ZERO
-                           )
+    def set_take_for(self, participant, take, recorder, cursor=None):
+        """Set the amount a participant wants to take from this team during payday.
 
+        :param Participant participant: the participant to set the take for
+        :param int take: the amount the participant wants to take
+        :param Participant recorder: the participant making the change
+
+        :return: ``None``
+        :raises: :py:exc:`NotAllowed`
+
+        It is a bug to pass in a ``participant`` or ``recorder`` that is
+        suspicious, unclaimed, or without a verified email and identity.
+        Furthermore, :py:exc:`NotAllowed` is raised in the following circumstances:
+
+        - ``recorder`` is neither ``participant`` nor the team owner
+        - ``recorder`` is the team owner and ``take`` is neither zero nor $0.01
+        - ``recorder`` is ``participant``, but ``participant`` isn't already on the team
 
-    def set_take_for(self, member, amount, recorder, cursor=None):
-        """Sets member's take from the team pool.
         """
+        def vet(p):
+            assert not p.is_suspicious, p.id
+            assert p.is_claimed, p.id
+            assert p.email_address, p.id
+            assert p.has_verified_identity, p.id
+
+        vet(participant)
+        vet(recorder)
+
+        if recorder.username == self.owner:
+            if take not in (ZERO, PENNY):
+                raise NotAllowed('owner can only add and remove members, not otherwise set takes')
+        elif recorder != participant:
+            raise NotAllowed('can only set own take')
+
         with self.db.get_cursor(cursor) as cursor:
-            # Lock to avoid race conditions
-            cursor.run("LOCK TABLE takes IN EXCLUSIVE MODE")
+            cursor.run("LOCK TABLE takes IN EXCLUSIVE MODE")  # avoid race conditions
+
             # Compute the current takes
             old_takes = self.compute_actual_takes(cursor)
-            # Insert the new take
-            cursor.run("""
-
-                INSERT INTO takes (ctime, member, team, amount, recorder)
-                 VALUES ( COALESCE (( SELECT ctime
-                                        FROM takes
-                                       WHERE member=%(member)s
-                                         AND team=%(team)s
-                                       LIMIT 1
-                                     ), CURRENT_TIMESTAMP)
-                        , %(member)s
-                        , %(team)s
-                        , %(amount)s
-                        , %(recorder)s
-                         )
-
-            """, dict(member=member.username, team=self.username, amount=amount,
-                      recorder=recorder.username))
+
+            old_take = self.get_take_for(participant, cursor=cursor)
+            if recorder.username != self.owner:
+                if recorder == participant and not old_take:
+                    raise NotAllowed('can only set take if already a member of the team')
+
+            cursor.one( """
+
+                INSERT INTO takes
+                            (ctime, participant_id, team_id, amount, recorder_id)
+                     VALUES ( COALESCE (( SELECT ctime
+                                            FROM takes
+                                           WHERE (participant_id=%(participant_id)s
+                                                  AND team_id=%(team_id)s)
+                                           LIMIT 1
+                                         ), CURRENT_TIMESTAMP)
+                            , %(participant_id)s, %(team_id)s, %(amount)s, %(recorder_id)s
+                             )
+                  RETURNING *
+
+            """, { 'participant_id': participant.id
+                 , 'team_id': self.id
+                 , 'amount': take
+                 , 'recorder_id': recorder.id
+                  })
+
             # Compute the new takes
             new_takes = self.compute_actual_takes(cursor)
-            # Update receiving amounts in the participants table
-            self.update_taking(old_takes, new_takes, cursor, member)
-            # Update is_funded on member's tips
-            member.update_giving(cursor)
+
+            # Update computed values
+            self.update_taking(old_takes, new_takes, cursor, participant)
+            self.update_distributing(old_takes, new_takes, cursor, participant)
+
+
+    def get_take_for(self, participant, cursor=None):
+        """
+        :param Participant participant: the participant to get the take for
+        :param GratipayDB cursor: a database cursor; if ``None``, a new cursor will be used
+        :return: a :py:class:`~decimal.Decimal`: the ``participant``'s take from this team, or 0.
+        """
+        return (cursor or self.db).one("""
+
+            SELECT amount
+              FROM current_takes
+             WHERE team_id=%s AND participant_id=%s
+
+        """, (self.id, participant.id), default=ZERO)
 
 
     def update_taking(self, old_takes, new_takes, cursor=None, member=None):
         """Update `taking` amounts based on the difference between `old_takes`
         and `new_takes`.
         """
+        # XXX Deal with owner as well as members
         for username in set(old_takes.keys()).union(new_takes.keys()):
             if username == self.username:
                 continue
@@ -127,3 +192,9 @@ def compute_actual_takes(self, cursor=None):
             take['percentage'] = (actual_amount / budget) if budget > 0 else 0
             actual_takes[take['member']] = take
         return actual_takes
+
+
+class NotAllowed(Exception):
+    """Raised by :py:meth:`set_take_for` if ``recorder`` is not allowed to set
+    the take for ``participant``.
+    """

gratipay/testing/__init__.py

@@ -166,7 +166,14 @@ def make_team(self, *a, **kw):
             _kw['is_approved'] = False
 
         if Participant.from_username(_kw['owner']) is None:
-            self.make_participant(_kw['owner'], claimed_time='now', last_paypal_result='')
+            owner = self.make_participant( _kw['owner']
+                                         , claimed_time='now'
+                                         , last_paypal_result=''
+                                         , email_address=_kw['owner']+'@example.com'
+                                          )
+            TT = self.db.one("SELECT id FROM countries WHERE code='TT'")
+            owner.store_identity_info(TT, 'nothing-enforced', {'name': 'Owner'})
+            owner.set_identity_verification(TT, True)
 
         team = self.db.one("""
             INSERT INTO teams

sql/branch.sql

@@ -6,9 +6,41 @@ BEGIN;
 END;
 
 
--- https://github.com/gratipay/gratipay.com/pull/????
+-- https://github.com/gratipay/gratipay.com/pull/4072
 
 BEGIN;
     ALTER TABLE teams ADD COLUMN available numeric(35,2) NOT NULL DEFAULT 0;
     ALTER TABLE teams ADD CONSTRAINT available_not_negative CHECK ((available >= (0)::numeric));
 END;
+
+
+-- https://github.com/gratipay/gratipay.com/pull/????
+
+BEGIN;
+    DROP VIEW current_takes;
+    DROP TABLE takes;
+
+    -- takes - how participants express membership in teams
+    CREATE TABLE takes
+    ( id                bigserial                   PRIMARY KEY
+    , ctime             timestamp with time zone    NOT NULL
+    , mtime             timestamp with time zone    NOT NULL DEFAULT now()
+    , participant_id    bigint                      NOT NULL REFERENCES participants(id)
+    , team_id           bigint                      NOT NULL REFERENCES teams(id)
+    , amount            numeric(35,2)               NOT NULL
+    , recorder_id       bigint                      NOT NULL REFERENCES participants(id)
+    , CONSTRAINT not_negative CHECK (amount >= 0)
+     );
+
+    CREATE VIEW current_takes AS
+        SELECT * FROM (
+             SELECT DISTINCT ON (participant_id, team_id) t.*
+               FROM takes t
+               JOIN participants p ON p.id = t.participant_id
+              WHERE p.is_suspicious IS NOT TRUE
+           ORDER BY participant_id
+                  , team_id
+                  , mtime DESC
+        ) AS anon WHERE amount > 0;
+
+END;

tests/py/test_close.py

@@ -29,7 +29,7 @@ def test_close_fails_if_still_a_balance(self):
     def test_close_fails_if_still_owns_a_team(self):
         alice = self.make_participant('alice', claimed_time='now')
         self.make_team(owner=alice)
-        with pytest.raises(alice.StillATeamOwner):
+        with pytest.raises(alice.StillOnATeam):
             alice.close()
 
     def test_close_page_is_usually_available(self):

tests/py/test_participant.py

@@ -691,7 +691,7 @@ def test_archive_fails_for_team_owner(self):
         alice = self.make_participant('alice')
         self.make_team(owner=alice)
         with self.db.get_cursor() as cursor:
-            pytest.raises(alice.StillATeamOwner, alice.archive, cursor)
+            pytest.raises(alice.StillOnATeam, alice.archive, cursor)
 
     def test_archive_fails_if_balance_is_positive(self):
         alice = self.make_participant('alice', balance=2)