launchpad-reviewers team mailing list archive

[Martin Pool <mbp@xxxxxxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/855150-mail-disabled into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~mbp/launchpad/855150-mail-disabled/+merge/76876

This handles part of bug 855150, which is that Launchpad sends mail to people who have deactivated accounts.  I addressed this by changing get_recipients to check against the account table too, and adding a unit test for this.  I also checked the uses of get_recipients and none seems like it would actually want to be mailing deactivated accounts - perhaps the only case for that would be if people wanted to reactivate them, and I think they have to log in first.
-- 
https://code.launchpad.net/~mbp/launchpad/855150-mail-disabled/+merge/76876
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/855150-mail-disabled into lp:launchpad.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-09-23 07:49:54 +0000
+++ lib/lp/registry/model/person.py	2011-09-25 00:45:28 +0000
@@ -4674,7 +4674,8 @@
     If <person> has a preferred email, the set will contain only that
     person.  If <person> doesn't have a preferred email but is a team,
     the set will contain the preferred email address of each member of
-    <person>, including indirect members.
+    <person>, including indirect members, that has an active account and an
+    preferred (active) address.
 
     Finally, if <person> doesn't have a preferred email and is not a team,
     the set will be empty.
@@ -4682,7 +4683,7 @@
     if person.preferredemail:
         return [person]
     elif person.is_team:
-        # Get transitive members of team without a preferred email.
+        # Get transitive members of a team that does not itself have a preferred email.
         return _get_recipients_for_team(person)
     else:
         return []
@@ -4701,13 +4702,15 @@
                                   And(
                                       EmailAddress.person == Person.id,
                                       EmailAddress.status ==
-                                        EmailAddressStatus.PREFERRED)))
+                                        EmailAddressStatus.PREFERRED)),
+                         LeftJoin(Account,
+                             Person.accountID == Account.id))
     pending_team_ids = [team.id]
     recipient_ids = set()
     seen = set()
     while pending_team_ids:
-        # Find Persons that have a preferred email address, or are a
-        # team, or both.
+        # Find Persons that have a preferred email address and an active
+        # account, or are a team, or both.
         intermediate_transitive_results = source.find(
             (TeamMembership.personID, EmailAddress.personID),
             In(TeamMembership.status,
@@ -4715,7 +4718,8 @@
                 TeamMembershipStatus.APPROVED.value]),
             In(TeamMembership.teamID, pending_team_ids),
             Or(
-                EmailAddress.personID != None,
+                And(EmailAddress.personID != None,
+                    Account.status == AccountStatus.ACTIVE),
                 Person.teamownerID != None)).config(distinct=True)
         next_ids = []
         for (person_id,

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2011-09-23 07:49:54 +0000
+++ lib/lp/registry/tests/test_person.py	2011-09-25 00:45:28 +0000
@@ -1719,3 +1719,13 @@
                               super_team_member_person,
                               super_team_member_team]),
                          set(recipients))
+
+    def test_get_recipients_team_with_disabled_account(self):
+        """Mail is not sent to teams containing people with a non-active account.
+
+        See <https://bugs.launchpad.net/launchpad/+bug/855150>
+        """
+        owner = self.factory.makePerson(email='foo@xxxxxxx')
+        team = self.factory.makeTeam(owner)
+        owner.account.status = AccountStatus.DEACTIVATED
+        self.assertContentEqual([], get_recipients(team))