Revision history for this message
| Curtis Hovey (sinzui) wrote : | # |
| 1 | === modified file 'lib/lp/registry/model/person.py' |
| 2 | --- lib/lp/registry/model/person.py 2011-04-01 20:26:29 +0000 |
| 3 | +++ lib/lp/registry/model/person.py 2011-04-06 16:46:14 +0000 |
| 4 | @@ -1423,15 +1423,8 @@ |
| 5 | """See `IPerson`.""" |
| 6 | assert self.is_team |
| 7 | to_addrs = set() |
| 8 | - team_or_person = self |
| 9 | - # Recurse through team ownership until an email address is found. |
| 10 | - while len(to_addrs) == 0: |
| 11 | - if team_or_person.is_team: |
| 12 | - for admin in team_or_person.getDirectAdministrators(): |
| 13 | - to_addrs.update(get_contact_email_addresses(admin)) |
| 14 | - team_or_person = team_or_person.teamowner |
| 15 | - else: |
| 16 | - to_addrs.update(get_contact_email_addresses(team_or_person)) |
| 17 | + for admin in self.adminmembers: |
| 18 | + to_addrs.update(get_contact_email_addresses(admin)) |
| 19 | return sorted(to_addrs) |
| 20 | |
| 21 | def addMember(self, person, reviewer, comment=None, force_team_add=False, |
| 22 | |
| 23 | === modified file 'lib/lp/registry/tests/test_team.py' |
| 24 | --- lib/lp/registry/tests/test_team.py 2011-03-25 17:28:15 +0000 |
| 25 | +++ lib/lp/registry/tests/test_team.py 2011-04-06 17:04:26 +0000 |
| 26 | @@ -133,23 +133,20 @@ |
| 27 | def setUp(self): |
| 28 | super(TestTeamGetTeamAdminsEmailAddresses, self).setUp() |
| 29 | self.team = self.factory.makeTeam(name='finch') |
| 30 | - self.address = self.factory.makeEmail('team@eg.dom', self.team) |
| 31 | login_celebrity('admin') |
| 32 | |
| 33 | - def test_team_direct_owner(self): |
| 34 | - # The team owner's email address is only email address. |
| 35 | + def test_admin_is_user(self): |
| 36 | + # The team owner is a user and admin who provides the email address. |
| 37 | email = self.team.teamowner.preferredemail.email |
| 38 | self.assertEqual([email], self.team.getTeamAdminsEmailAddresses()) |
| 39 | |
| 40 | - def test_team_direct_owner_not_member(self): |
| 41 | - # The team owner's email address is only email address, even when |
| 42 | - # the owner is not a team member. |
| 43 | - email = self.team.teamowner.preferredemail.email |
| 44 | + def test_no_admins(self): |
| 45 | + # A team without admins has no email addresses. |
| 46 | self.team.teamowner.leave(self.team) |
| 47 | - self.assertEqual([email], self.team.getTeamAdminsEmailAddresses()) |
| 48 | + self.assertEqual([], self.team.getTeamAdminsEmailAddresses()) |
| 49 | |
| 50 | - def test_team_direct_admin(self): |
| 51 | - # The team's owner and direct admins provide the email addresses. |
| 52 | + def test_admins_are_users_with_preferred_email_addresses(self): |
| 53 | + # The team's admins are users, and they provide the email addresses. |
| 54 | admin = self.factory.makePerson() |
| 55 | self.team.addMember(admin, self.team.teamowner) |
| 56 | for membership in self.team.member_memberships: |
| 57 | @@ -160,35 +157,37 @@ |
| 58 | self.assertEqual( |
| 59 | [email_1, email_2], self.team.getTeamAdminsEmailAddresses()) |
| 60 | |
| 61 | - def test_team_indirect_owner(self): |
| 62 | - # The team owner's of the owning team is only email address. |
| 63 | - owning_team = self.factory.makeTeam() |
| 64 | - member = self.team.teamowner |
| 65 | - self.team.teamowner = owning_team |
| 66 | - for membership in self.team.member_memberships: |
| 67 | - membership.setStatus( |
| 68 | - TeamMembershipStatus.APPROVED, owning_team.teamowner) |
| 69 | - email = owning_team.teamowner.preferredemail.email |
| 70 | - self.assertEqual([email], self.team.getTeamAdminsEmailAddresses()) |
| 71 | - |
| 72 | - def test_team_indirect_admin(self): |
| 73 | - # The team owner and admin of the owning team provide the |
| 74 | - # email addresses. |
| 75 | - owning_team = self.factory.makeTeam() |
| 76 | - member = self.team.teamowner |
| 77 | - self.team.teamowner = owning_team |
| 78 | - for membership in self.team.member_memberships: |
| 79 | - membership.setStatus( |
| 80 | - TeamMembershipStatus.APPROVED, owning_team.teamowner) |
| 81 | - admin = self.factory.makePerson() |
| 82 | - owning_team.addMember(admin, owning_team.teamowner) |
| 83 | - for membership in owning_team.member_memberships: |
| 84 | - membership.setStatus( |
| 85 | - TeamMembershipStatus.ADMIN, owning_team.teamowner) |
| 86 | - email_1 = owning_team.teamowner.preferredemail.email |
| 87 | - email_2 = admin.preferredemail.email |
| 88 | - self.assertEqual( |
| 89 | - [email_1, email_2], self.team.getTeamAdminsEmailAddresses()) |
| 90 | + def setUpAdminingTeam(self, team): |
| 91 | + """Return a new team set as the admin of the provided team.""" |
| 92 | + admin_team = self.factory.makeTeam() |
| 93 | + admin_member = self.factory.makePerson() |
| 94 | + admin_team.addMember(admin_member, admin_team.teamowner) |
| 95 | + team.addMember( |
| 96 | + admin_team, team.teamowner, force_team_add=True) |
| 97 | + for membership in team.member_memberships: |
| 98 | + membership.setStatus( |
| 99 | + TeamMembershipStatus.ADMIN, admin_team.teamowner) |
| 100 | + approved_member = self.factory.makePerson() |
| 101 | + team.addMember(approved_member, team.teamowner) |
| 102 | + team.teamowner.leave(team) |
| 103 | + return admin_team |
| 104 | + |
| 105 | + def test_admins_are_teams_with_preferred_email_addresses(self): |
| 106 | + # The team's admin is a team without a contact address. |
| 107 | + # The admin team members provide the email addresses. |
| 108 | + admin_team = self.setUpAdminingTeam(self.team) |
| 109 | + admin_team.setContactAddress( |
| 110 | + self.factory.makeEmail('team@eg.dom', admin_team)) |
| 111 | + self.assertEqual( |
| 112 | + ['team@eg.dom'], self.team.getTeamAdminsEmailAddresses()) |
| 113 | + |
| 114 | + def test_admins_are_teams_without_preferred_email_addresses(self): |
| 115 | + # The team's admin is a team with a contact address. |
| 116 | + admin_team = self.setUpAdminingTeam(self.team) |
| 117 | + emails = sorted( |
| 118 | + m.preferredemail.email for m in admin_team.activemembers) |
| 119 | + self.assertEqual( |
| 120 | + emails, self.team.getTeamAdminsEmailAddresses()) |
| 121 | |
| 122 | |
| 123 | class TestDefaultRenewalPeriodIsRequiredForSomeTeams(TestCaseWithFactory): |
| 124 |
Hi Robert
On Wed, 2011-03-23 at 01:55 +0000, Robert Collins wrote:
> + team_or_person = self
> 92 + # Recurse through team ownership until an email
> address is found.
> 93 + while len(to_addrs) == 0:
> 94 + if team_or_
> 95 + for admin in
> team_or_
> 96 +
> to_addrs.
> 97 + team_or_person = team_or_
> 98 + else:
> 99 +
> to_addrs.
I switch to team.adminmembers to avoid special casing the team owner.
I removed the recursive loop to find an email address.
The code is very simple now, mostly because
get_contact_
contact address.
I updated the tests. Notably, I added a test to show there can be no
contact address when there are no admins. I added a test to verify the
case of an admining team without a contact address. I think this case
was missing in my first effort. There were no surprises that it worked
since get_contact_
I have attached a diff, but you might want to read def
getTeamAdminsEm
verify the whole method is consistent.