Cherry-pick #4024

Conflicts:
	gratipay/models/team/__init__.py
	gratipay/models/team/mixins/__init__.py
	gratipay/models/team/mixins/membership.py

gratipay/models/team/mixins/__init__.py

@@ -3,4 +3,4 @@
 from .takes import TakesMixin as Takes
 from .tip_migration import TipMigrationMixin as TipMigration
 
-__all__ = ['Available', 'Takes', 'Membership', 'TipMigration']
+__all__ = ['Available', 'Membership', 'Takes', 'TipMigration']

gratipay/models/team/mixins/membership.py

@@ -3,24 +3,28 @@
 from .takes import ZERO, PENNY
 
 
-class StubParticipantAdded(Exception): pass
-
 class MembershipMixin(object):
-    """Teams can distribute money to their members.
+    """Teams may have zero or more members, who are participants that take money from the team.
     """
 
-    def add_member(self, member):
-        """Add a member to this team.
+    def add_member(self, participant, recorder):
+        """Add a participant to this team.
+
+        :param Participant participant: the participant to add
+        :param Participant recorder: the participant making the change
+
         """
-        if not member.is_claimed:
-            raise StubParticipantAdded
-        self.set_take_for(member, PENNY, self)
+        self.set_take_for(participant, PENNY, recorder)
 
 
-    def remove_member(self, member):
-        """Remove a member from this team.
+    def remove_member(self, participant, recorder):
+        """Remove a participant from this team.
+
+        :param Participant participant: the participant to remove
+        :param Participant recorder: the participant making the change
+
         """
-        self.set_take_for(member, ZERO, self)
+        self.set_take_for(participant, ZERO, recorder)
 
 
     def remove_all_members(self, cursor=None):
@@ -36,21 +40,21 @@ def remove_all_members(self, cursor=None):
 
     @property
     def nmembers(self):
-        return self.db.one("""
-            SELECT COUNT(*)
-              FROM current_takes
-             WHERE team=%s
-        """, (self.username, ))
+        """The number of members. Read-only and computed (not in the db); equal to
+        :py:attr:`~gratipay.models.team.mixins.takes.ndistributing_to`.
+        """
+        return self.ndistributing_to
 
 
-    def get_members(self, current_participant=None):
+    def get_memberships(self, current_participant=None):
         """Return a list of member dicts.
         """
         takes = self.compute_actual_takes()
         members = []
         for take in takes.values():
             member = {}
-            member['username'] = take['member']
+            member['participant_id'] = take['participant'].id
+            member['username'] = take['participant'].username
             member['take'] = take['nominal_amount']
             member['balance'] = take['balance']
             member['percentage'] = take['percentage']
@@ -65,6 +69,6 @@ def get_members(self, current_participant=None):
                         # current user, but not the team itself
                         member['editing_allowed']= True
 
-            member['last_week'] = self.get_take_last_week_for(member)
+            member['last_week'] = self.get_take_last_week_for(member['participant_id'])
             members.append(member)
         return members

gratipay/models/team/mixins/takes.py

@@ -32,16 +32,14 @@ class TakesMixin(object):
     ndistributing_to = 0
 
 
-    def get_take_last_week_for(self, member):
-        """Get the user's nominal take last week.
+    def get_take_last_week_for(self, participant_id):
+        """Get the participant's nominal take last week.
         """
-        membername = member.username if hasattr(member, 'username') \
-                                                        else member['username']
         return self.db.one("""
 
             SELECT amount
               FROM takes
-             WHERE team=%s AND member=%s
+             WHERE team_id=%s AND participant_id=%s
                AND mtime < (
                        SELECT ts_start
                          FROM paydays
@@ -50,7 +48,7 @@ def get_take_last_week_for(self, member):
                    )
           ORDER BY mtime DESC LIMIT 1
 
-        """, (self.username, membername), default=ZERO)
+        """, (self.id, participant_id), default=ZERO)
 
 
     def set_take_for(self, participant, take, recorder, cursor=None):
@@ -182,8 +180,11 @@ def get_current_takes(self, cursor=None):
         """Return a list of member takes for a team.
         """
         TAKES = """
-            SELECT participant_id, amount, ctime, mtime
-              FROM current_takes
+            SELECT p.*::participants AS participant
+                 , ct.amount, ct.ctime, ct.mtime
+              FROM current_takes ct
+              JOIN participants p
+                ON ct.participant_id = p.id
              WHERE team_id=%(team_id)s
           ORDER BY amount ASC, ctime ASC
         """
@@ -202,7 +203,7 @@ def compute_actual_takes(self, cursor=None):
             actual_amount = take['actual_amount'] = min(nominal_amount, balance)
             take['balance'] = balance
             take['percentage'] = actual_amount / available
-            actual_takes[take['participant_id']] = take
+            actual_takes[take['participant'].id] = take
         return actual_takes
 
 

tests/py/test_team_membership.py

@@ -0,0 +1,72 @@
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+from test_team_takes import TeamTakesHarness
+from gratipay.models.team import mixins
+
+
+class Tests(TeamTakesHarness):
+
+    def setUp(self):
+        TeamTakesHarness.setUp(self)
+
+    def assert_memberships(self, *expected):
+        actual = self.enterprise.get_memberships()
+        assert [m['username'] for m in actual] == list(expected)
+
+
+    def test_team_object_subclasses_takes_mixin(self):
+        assert isinstance(self.enterprise, mixins.Membership)
+
+
+    # gm - get_memberships
+
+    def test_gm_returns_an_empty_list_when_there_are_no_members(self):
+        assert self.enterprise.get_memberships() == []
+
+    def test_gm_returns_memberships_when_there_are_members(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        assert len(self.enterprise.get_memberships()) == 1
+
+    def test_gm_returns_more_memberships_when_there_are_more_members(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.enterprise.add_member(self.bruiser, self.picard)
+        assert len(self.enterprise.get_memberships()) == 2
+
+
+    # am - add_member
+
+    def test_am_adds_a_member(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.assert_memberships('crusher')
+
+    def test_am_adds_another_member(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.enterprise.add_member(self.bruiser, self.picard)
+        self.assert_memberships('crusher', 'bruiser')
+
+    def test_am_affects_computed_values_as_expected(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.enterprise.add_member(self.bruiser, self.picard)
+        assert self.enterprise.nmembers == 2
+
+
+    # rm - remove_member
+
+    def test_rm_removes_a_member(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.enterprise.add_member(self.bruiser, self.picard)
+        self.enterprise.remove_member(self.crusher, self.crusher)
+        self.assert_memberships('bruiser')
+
+    def test_rm_removes_another_member(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.enterprise.add_member(self.bruiser, self.picard)
+        self.enterprise.remove_member(self.crusher, self.crusher)
+        self.enterprise.remove_member(self.bruiser, self.picard)
+        self.assert_memberships()
+
+    def test_rm_affects_computed_values_as_expected(self):
+        self.enterprise.add_member(self.crusher, self.picard)
+        self.enterprise.add_member(self.bruiser, self.picard)
+        self.enterprise.remove_member(self.crusher, self.crusher)
+        assert self.enterprise.nmembers == 1

www/%team/distributing/index.json.spt

@@ -9,4 +9,4 @@ if team.available == 0:
     website.redirect('..', base_url='')
 
 [--------------------] application/json via json_dump
-team.get_members(user.participant)
+team.get_memberships(user.participant)