launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/no-team-admins-fallback into lp:launchpad.

Commit message:
Fall back to emailing the team owner if the team has no admins.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1270141 in Launchpad itself: "Oops when attempting register new project against team with no admins"
  https://bugs.launchpad.net/launchpad/+bug/1270141

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/no-team-admins-fallback/+merge/343363
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/no-team-admins-fallback into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2017-06-01 12:58:53 +0000
+++ lib/lp/registry/interfaces/person.py	2018-04-17 01:07:45 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Person interfaces."""
@@ -1344,9 +1344,14 @@
         member with admin privilege, or member of a team with such
         privileges.  It excludes teams which have been merged.
         """
+
     def getTeamAdminsEmailAddresses():
         """Return a set containing the email addresses of all administrators
         of this team.
+
+        If the team has no administrators, fall back to the team owner.
+        This shouldn't normally happen, but a team can end up in this state
+        after deactivations, and there's no good way to prevent it entirely.
         """
 
     def getLatestApprovedMembershipsForPerson(limit=5):

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2018-04-10 11:10:09 +0000
+++ lib/lp/registry/model/person.py	2018-04-17 01:07:45 +0000
@@ -1595,6 +1595,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_subscribers.py'
--- lib/lp/registry/tests/test_subscribers.py	2017-12-19 17:15:21 +0000
+++ lib/lp/registry/tests/test_subscribers.py	2018-04-17 01:07:45 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test subscruber classes and functions."""
@@ -190,6 +190,25 @@
         self.assertEqual(1, len(notifications))
         self.assertEqual('admin@xxxxxx,owner@xxxxxx', notifications[0]['To'])
 
+    def test_send_other_proprietary_no_team_admins_falls_back_to_owner(self):
+        # If there are no team admins, an Other/Proprietary licence falls
+        # back to sending email to the team owner.
+        product, user = self.make_product_user([License.OTHER_PROPRIETARY])
+        owner = self.factory.makePerson(email='owner@xxxxxx')
+        team = self.factory.makeTeam(
+            owner=owner, membership_policy=TeamMembershipPolicy.RESTRICTED)
+        with person_logged_in(team.teamowner):
+            team.teamowner.leave(team)
+        with person_logged_in(product.owner):
+            product.owner = team
+        pop_notifications()
+        notification = LicenseNotification(product)
+        result = notification.send()
+        self.assertIs(True, result)
+        notifications = pop_notifications()
+        self.assertEqual(1, len(notifications))
+        self.assertEqual('owner@xxxxxx', notifications[0]['To'])
+
     def test_display_no_request(self):
         # If there is no request, there is no reason to show a message in
         # the browser.

=== modified file 'lib/lp/registry/tests/test_team.py'
--- lib/lp/registry/tests/test_team.py	2016-01-26 15:47:37 +0000
+++ lib/lp/registry/tests/test_team.py	2018-04-17 01:07:45 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for PersonSet."""
@@ -165,9 +165,10 @@
         self.assertEqual([email], self.team.getTeamAdminsEmailAddresses())
 
     def test_no_admins(self):
-        # A team without admins has no email addresses.
+        # A team without admins falls back to the owner's email address.
+        email = self.team.teamowner.preferredemail.email
         self.team.teamowner.leave(self.team)
-        self.assertEqual([], self.team.getTeamAdminsEmailAddresses())
+        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.