diff --git a/gratipay/models/team/__init__.py b/gratipay/models/team/__init__.py index 6e4d4711a8..f6f7b6b02f 100644 --- a/gratipay/models/team/__init__.py +++ b/gratipay/models/team/__init__.py @@ -7,13 +7,12 @@ import requests from aspen import json, log +from gratipay.billing.exchanges import MINIMUM_CHARGE 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 -from gratipay.models.team import mixins # Should have at least one letter. TEAM_NAME_PATTERN = re.compile(r'^(?=.*[A-Za-z])([A-Za-z0-9.,-_ ]+)$') diff --git a/gratipay/models/team/mixins/takes.py b/gratipay/models/team/mixins/takes.py index b94f66f8ef..8742a7d57c 100644 --- a/gratipay/models/team/mixins/takes.py +++ b/gratipay/models/team/mixins/takes.py @@ -123,7 +123,7 @@ def vet(p): # Update computed values self.update_taking(old_takes, new_takes, cursor, participant) - self.update_distributing(old_takes, new_takes, cursor, participant) + self.update_distributing(new_takes, cursor) def get_take_for(self, participant, cursor=None): @@ -145,35 +145,49 @@ 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 - old = old_takes.get(username, {}).get('actual_amount', ZERO) - new = new_takes.get(username, {}).get('actual_amount', ZERO) - diff = new - old - if diff != 0: - r = (cursor or self.db).one(""" + + for participant_id in set(old_takes.keys()).union(new_takes.keys()): + old = old_takes.get(participant_id, {}).get('actual_amount', ZERO) + new = new_takes.get(participant_id, {}).get('actual_amount', ZERO) + delta = new - old + if delta != 0: + taking = (cursor or self.db).one(""" UPDATE participants - SET taking = (taking + %(diff)s) - , receiving = (receiving + %(diff)s) - WHERE username=%(username)s - RETURNING taking, receiving - """, dict(username=username, diff=diff)) - if member and username == member.username: - member.set_attributes(**r._asdict()) + SET taking = (taking + %(delta)s) + WHERE id=%(participant_id)s + RETURNING taking + """, dict(participant_id=participant_id, delta=delta)) + if member and participant_id == member.id: + member.set_attributes(taking=taking) + + + def update_distributing(self, new_takes, cursor=None): + """Update the computed values on the team. + """ + distributing = sum(t['actual_amount'] for t in new_takes.values()) + ndistributing_to = len(tuple(t for t in new_takes.values() if t['actual_amount'] > 0)) + + r = (cursor or self.db).one(""" + UPDATE teams + SET distributing=%s, ndistributing_to=%s WHERE id=%s + RETURNING distributing, ndistributing_to + """, (distributing, ndistributing_to, self.id)) + + self.set_attributes(**r._asdict()) def get_current_takes(self, cursor=None): """Return a list of member takes for a team. """ TAKES = """ - SELECT member, amount, ctime, mtime + SELECT participant_id, amount, ctime, mtime FROM current_takes - WHERE team=%(team)s - ORDER BY ctime DESC + WHERE team_id=%(team_id)s + ORDER BY amount ASC, ctime ASC """ - records = (cursor or self.db).all(TAKES, dict(team=self.username)) + records = (cursor or self.db).all(TAKES, dict(team_id=self.id)) return [r._asdict() for r in records] @@ -182,15 +196,13 @@ def compute_actual_takes(self, cursor=None): """ actual_takes = OrderedDict() nominal_takes = self.get_current_takes(cursor=cursor) - budget = balance = self.balance + self.receiving - self.giving + available = balance = self.available for take in nominal_takes: nominal_amount = take['nominal_amount'] = take.pop('amount') actual_amount = take['actual_amount'] = min(nominal_amount, balance) - if take['member'] != self.username: - balance -= actual_amount take['balance'] = balance - take['percentage'] = (actual_amount / budget) if budget > 0 else 0 - actual_takes[take['member']] = take + take['percentage'] = actual_amount / available + actual_takes[take['participant_id']] = take return actual_takes diff --git a/tests/py/test_team_takes.py b/tests/py/test_team_takes.py index 31fdb79090..954d0b8758 100644 --- a/tests/py/test_team_takes.py +++ b/tests/py/test_team_takes.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, division, print_function, unicode_literals +from decimal import Decimal as D + from pytest import raises from gratipay.models.participant import Participant from gratipay.models.team import Team @@ -15,7 +17,7 @@ class TeamTakesHarness(Harness): # Factored out to share with membership tests ... def setUp(self): - self.enterprise = self.make_team('The Enterprise') + self.enterprise = self.make_team('The Enterprise', available=1, receiving=2) self.picard = P('picard') self.TT = self.db.one("SELECT id FROM countries WHERE code='TT'") @@ -56,7 +58,7 @@ def test_gtf_returns_correct_amount_for_multiple_members(self): def test_gtf_returns_correct_amount_for_multiple_teams(self): self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - trident = self.make_team('The Trident', owner='shelby') + trident = self.make_team('The Trident', owner='shelby', available=5) trident.set_take_for(self.crusher, PENNY, P('shelby')) trident.set_take_for(self.crusher, PENNY * 2, self.crusher) @@ -75,44 +77,17 @@ def test_stf_updates_take_for_an_existing_member(self): self.enterprise.set_take_for(self.crusher, 537, self.crusher) assert self.enterprise.get_take_for(self.crusher) == 537 - - def test_stf_can_increase_ndistributing_to(self): - self.enterprise.set_take_for(self.bruiser, PENNY, self.picard) - assert self.enterprise.ndistributing_to == 1 - self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - assert self.enterprise.ndistributing_to == 2 - - def test_stf_doesnt_increase_ndistributing_to_for_an_existing_member(self): - self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - self.enterprise.set_take_for(self.crusher, PENNY, self.crusher) - self.enterprise.set_take_for(self.crusher, 64, self.crusher) - assert self.enterprise.ndistributing_to == 1 - - def test_stf_can_decrease_ndistributing_to(self): - self.enterprise.set_take_for(self.bruiser, PENNY, self.picard) + def test_stf_calls_update_taking(self): + assert self.crusher.taking == ZERO self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - self.enterprise.set_take_for(self.crusher, 0, self.crusher) - assert self.enterprise.ndistributing_to == 1 - self.enterprise.set_take_for(self.bruiser, 0, self.bruiser) - assert self.enterprise.ndistributing_to == 0 + assert self.crusher.taking == PENNY - def test_stf_doesnt_decrease_ndistributing_to_below_zero(self): - self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - self.enterprise.set_take_for(self.crusher, 0, self.picard) - self.enterprise.set_take_for(self.crusher, 0, self.picard) - self.enterprise.set_take_for(self.crusher, 0, self.picard) - self.enterprise.set_take_for(self.crusher, 0, self.picard) + def test_stf_calls_update_distributing(self): assert self.enterprise.ndistributing_to == 0 - - def test_stf_updates_ndistributing_to_in_the_db(self): - self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - assert T('TheEnterprise').ndistributing_to == 1 - - - def test_stf_updates_taking_for_member(self): - assert self.crusher.taking == ZERO + assert self.enterprise.distributing == ZERO self.enterprise.set_take_for(self.crusher, PENNY, self.picard) - assert self.crusher.taking == PENNY + assert self.enterprise.ndistributing_to == 1 + assert self.enterprise.distributing == PENNY # stf permissions @@ -157,3 +132,81 @@ def test_stf_doesnt_let_anyone_set_a_take_who_is_not_already_on_the_team(self): def test_stf_doesnt_let_anyone_set_a_take_who_is_not_already_on_the_team_even_to_zero(self): actual = self.err(self.crusher, 0, self.crusher) assert actual == 'can only set take if already a member of the team' + + + # ut - update_taking + + def test_ut_updates_taking(self): + self.enterprise.set_take_for(self.crusher, PENNY, self.picard) + assert self.crusher.taking == PENNY + self.enterprise.update_taking( {self.crusher.id: {'actual_amount': PENNY}} + , {self.crusher.id: {'actual_amount': PENNY * 537}} + , member=self.crusher + ) + assert self.crusher.taking == PENNY * 537 + + + # ud - update_distributing + + def test_ud_can_increase_distributing(self): + self.enterprise.set_take_for(self.crusher, PENNY, self.picard) + assert self.enterprise.distributing == PENNY + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY * 80}}) + assert self.enterprise.distributing == PENNY * 80 + + + def test_ud_can_increase_ndistributing_to(self): + assert self.enterprise.ndistributing_to == 0 + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY}}) + assert self.enterprise.ndistributing_to == 1 + + def test_ud_doesnt_increase_ndistributing_to_for_an_existing_member(self): + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY * 2}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY * 40}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY * 3}}) + assert self.enterprise.ndistributing_to == 1 + + def test_ud_can_decrease_ndistributing_to(self): + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY}}) + assert self.enterprise.ndistributing_to == 1 + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': ZERO}}) + assert self.enterprise.ndistributing_to == 0 + + def test_ud_doesnt_decrease_ndistributing_to_below_zero(self): + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY}}) + assert self.enterprise.ndistributing_to == 1 + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': ZERO}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': ZERO}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': ZERO}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': ZERO}}) + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': ZERO}}) + assert self.enterprise.ndistributing_to == 0 + + def test_ud_updates_ndistributing_to_in_the_db(self): + self.enterprise.update_distributing({self.crusher.id: {'actual_amount': PENNY}}) + fresh = T('TheEnterprise') + assert fresh.distributing == PENNY + assert fresh.ndistributing_to == 1 + + + # cat - compute_actual_takes + + def test_cat_computes_actual_takes(self): + self.enterprise.set_take_for(self.crusher, PENNY, self.picard) + self.enterprise.set_take_for(self.crusher, PENNY * 80, self.crusher) + self.enterprise.set_take_for(self.bruiser, PENNY, self.picard) + self.enterprise.set_take_for(self.bruiser, PENNY * 30, self.bruiser) + takes = self.enterprise.compute_actual_takes() + + assert tuple(takes) == (self.bruiser.id, self.crusher.id) + + takes[self.bruiser.id]['actual_amount'] = PENNY * 30 + takes[self.bruiser.id]['nominal_amount'] = PENNY * 30 + takes[self.bruiser.id]['balance'] = PENNY * 70 + takes[self.bruiser.id]['percentage'] = D('0.3') + + takes[self.crusher.id]['actual_amount'] = PENNY * 70 + takes[self.crusher.id]['nominal_amount'] = PENNY * 80 + takes[self.crusher.id]['balance'] = ZERO + takes[self.crusher.id]['percentage'] = D('0.7') diff --git a/tests/py/test_www_team_distributing.py b/tests/py/test_www_team_distributing.py index aca4c61120..e226618eec 100644 --- a/tests/py/test_www_team_distributing.py +++ b/tests/py/test_www_team_distributing.py @@ -22,7 +22,7 @@ def test_json_redirects_when_no_money_is_available(self): def test_json_doesnt_redirect_when_money_is_available(self): self.make_team() self.db.run("UPDATE teams SET available=537") - assert self.client.GET('/TheEnterprise/distributing/index.json', raise_immediately=False).code == 500 + assert self.client.GET('/TheEnterprise/distributing/index.json').code == 200 def test_member_json_redirects_when_no_money_is_available(self):