diff --git a/gratipay/models/team/mixins/__init__.py b/gratipay/models/team/mixins/__init__.py index f15ef60265..90e544cf4c 100644 --- a/gratipay/models/team/mixins/__init__.py +++ b/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'] diff --git a/gratipay/models/team/mixins/membership.py b/gratipay/models/team/mixins/membership.py index 1379f23f48..d597e0c105 100644 --- a/gratipay/models/team/mixins/membership.py +++ b/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 diff --git a/gratipay/models/team/mixins/takes.py b/gratipay/models/team/mixins/takes.py index 8742a7d57c..dd71c9b490 100644 --- a/gratipay/models/team/mixins/takes.py +++ b/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 diff --git a/tests/py/test_team_membership.py b/tests/py/test_team_membership.py new file mode 100644 index 0000000000..d2c74810f9 --- /dev/null +++ b/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 diff --git a/www/%team/distributing/index.json.spt b/www/%team/distributing/index.json.spt index 2a75c73892..f95c4e53e3 100644 --- a/www/%team/distributing/index.json.spt +++ b/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)