Merge lp:~cjohnston/launchpad/1270141 into lp:launchpad

[Chris Johnston]

When there are no team admins, find the team owner's preferred email address and mail the team owner instead

If a team has no admins, getTeamAdminsEmailAddresses() fails and causes an oops. This was seen when attempting to add a commercial subscription to a project who's team had no admins. When there are no admins, default back to using the team's owner.

[Curtis Hovey (sinzui)]

A team with admins is leaderless. The team owner is negligent, not being an admin or appointing other admins to lead. There is no one to take responsability...it is not trustworthy. I don't think it should be included in the list of valid teams.

The rules once did send emails to the team owner, but this lead to trust issue or complaints from owner who had delegated the leadership of the team to others. In general, the team owner is not to receive any emails except about admin changes.

[Chris Johnston (cjohnston)]

Curtis, this is the way William said to fix it, so I guess I will let you two debate it.

[William Grant (wgrant)]

> A team with admins is leaderless. The team owner is negligent, not being an
> admin or appointing other admins to lead. There is no one to take
> responsability...it is not trustworthy. I don't think it should be included in
> the list of valid teams.
>
> The rules once did send emails to the team owner, but this lead to trust issue
> or complaints from owner who had delegated the leadership of the team to
> others. In general, the team owner is not to receive any emails except about
> admin changes.

The team owner is negligent, indeed, so they should probably be convinced to add some admins to stop getting spammed. Do you have a suggested alternate solution for this particular case, of sending commercial project expiration emails?

[Curtis Hovey (sinzui)]

Sorry for not following up. I in the case of commercial expiration, the owner is fine given the owner is probably the one that chooses to purchase.

[William Grant (wgrant)]

On 10/06/14 03:41, Curtis Hovey wrote:
> Sorry for not following up. I in the case of commercial expiration, the owner is fine given the owner is probably the one that chooses to purchase.

It seems like a bad idea to special-case that just there. Why should the
fallback not apply everywere that the team's admins are wanted?

[William Grant (wgrant)]

Curtis, do you have any more comments, or should we go ahead and make this change?

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2014-03-11 11:34:08 +0000
+++ lib/lp/registry/model/person.py	2014-06-04 17:21:05 +0000
@@ -1593,6 +1593,8 @@
         to_addrs = set()
         for admin in self.adminmembers:
             to_addrs.update(get_contact_email_addresses(admin))
+        if not to_addrs:
+            to_addrs.update(get_contact_email_addresses(self.teamowner))
         return sorted(to_addrs)
 
     def addMember(self, person, reviewer, comment=None, force_team_add=False,

=== modified file 'lib/lp/registry/tests/test_team.py'
--- lib/lp/registry/tests/test_team.py	2013-06-20 05:50:00 +0000
+++ lib/lp/registry/tests/test_team.py	2014-06-04 17:21:05 +0000
@@ -157,9 +157,10 @@
         self.assertEqual([email], self.team.getTeamAdminsEmailAddresses())
 
     def test_no_admins(self):
-        # A team without admins has no email addresses.
+        # A team without admins returns team owners email address.
         self.team.teamowner.leave(self.team)
-        self.assertEqual([], self.team.getTeamAdminsEmailAddresses())
+        email = self.team.teamowner.preferredemail.email
+        self.assertEqual([email], self.team.getTeamAdminsEmailAddresses())
 
     def test_admins_are_users_with_preferred_email_addresses(self):
         # The team's admins are users, and they provide the email addresses.