launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/do-not-notify-deactivated-accounts into lp:launchpad.

Commit message:
Don't send email to direct recipients without active accounts.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/do-not-notify-deactivated-accounts/+merge/342923

We already avoided this for indirect recipients via teams, but emailing e.g. deactivated accounts should be a no-no for direct recipients too.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/do-not-notify-deactivated-accounts into lp:launchpad.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2018-02-24 09:11:39 +0000
+++ lib/lp/registry/model/person.py	2018-04-10 11:39:01 +0000
@@ -4397,17 +4397,21 @@
 def get_recipients(person):
     """Return a set of people who receive email for this Person (person/team).
 
-    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, that has an active account and an
+    If <person> has a preferred email, then if <person> is a team or has an
+    active account the set will contain only that person, otherwise the set
+    will be empty.  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, that has an active account and a
     preferred (active) address.
 
     Finally, if <person> doesn't have a preferred email and is not a team,
     the set will be empty.
     """
     if removeSecurityProxy(person).preferredemail:
-        return [person]
+        if person.is_team or person.account_status == AccountStatus.ACTIVE:
+            return [person]
+        else:
+            return []
     elif person.is_team:
         # Get transitive members of a team that does not itself have a
         # preferred email.

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2018-01-02 16:10:26 +0000
+++ lib/lp/registry/tests/test_person.py	2018-04-10 11:39:01 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 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).
 
 __metaclass__ = type
@@ -1314,11 +1314,18 @@
         kwargs['email_address_status'] = EmailAddressStatus.NEW
         return self.factory.makePerson(**kwargs)
 
-    def get_test_recipients_person(self):
+    def test_get_recipients_person(self):
         person = self.factory.makePerson()
         recipients = get_recipients(person)
         self.assertEqual(set([person]), set(recipients))
 
+    def test_get_recipients_person_with_disabled_account(self):
+        """Mail is not sent to a direct recipient whose account is disabled."""
+        person = self.factory.makePerson()
+        person.setAccountStatus(AccountStatus.DEACTIVATED, None, 'gone')
+        recipients = get_recipients(person)
+        self.assertEqual(set(), set(recipients))
+
     def test_get_recipients_empty(self):
         """get_recipients returns empty set for person with no preferredemail.
         """