Merge lp:~mbp/launchpad/855150-mail-disabled into lp:launchpad

Proposed by Martin Pool
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 14050
Proposed branch: lp:~mbp/launchpad/855150-mail-disabled
Merge into: lp:launchpad
Diff against target: 92 lines (+43/-6)
2 files modified
lib/lp/registry/model/person.py (+10/-6)
lib/lp/registry/tests/test_person.py (+33/-0)
To merge this branch: bzr merge lp:~mbp/launchpad/855150-mail-disabled
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+76876@code.launchpad.net

Commit message

[r=sinzui] [bug=858605] don't send mail to individuals without an active account

Description of the change

This handles bug 858605 (split from 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.

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

W00t! Thank you very much.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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-26 10:09:24 +0000
@@ -4674,7 +4674,8 @@
4674 If <person> has a preferred email, the set will contain only that4674 If <person> has a preferred email, the set will contain only that
4675 person. If <person> doesn't have a preferred email but is a team,4675 person. If <person> doesn't have a preferred email but is a team,
4676 the set will contain the preferred email address of each member of4676 the set will contain the preferred email address of each member of
4677 <person>, including indirect members.4677 <person>, including indirect members, that has an active account and an
4678 preferred (active) address.
46784679
4679 Finally, if <person> doesn't have a preferred email and is not a team,4680 Finally, if <person> doesn't have a preferred email and is not a team,
4680 the set will be empty.4681 the set will be empty.
@@ -4682,7 +4683,7 @@
4682 if person.preferredemail:4683 if person.preferredemail:
4683 return [person]4684 return [person]
4684 elif person.is_team:4685 elif person.is_team:
4685 # Get transitive members of team without a preferred email.4686 # Get transitive members of a team that does not itself have a preferred email.
4686 return _get_recipients_for_team(person)4687 return _get_recipients_for_team(person)
4687 else:4688 else:
4688 return []4689 return []
@@ -4701,13 +4702,15 @@
4701 And(4702 And(
4702 EmailAddress.person == Person.id,4703 EmailAddress.person == Person.id,
4703 EmailAddress.status ==4704 EmailAddress.status ==
4704 EmailAddressStatus.PREFERRED)))4705 EmailAddressStatus.PREFERRED)),
4706 LeftJoin(Account,
4707 Person.accountID == Account.id))
4705 pending_team_ids = [team.id]4708 pending_team_ids = [team.id]
4706 recipient_ids = set()4709 recipient_ids = set()
4707 seen = set()4710 seen = set()
4708 while pending_team_ids:4711 while pending_team_ids:
4709 # Find Persons that have a preferred email address, or are a4712 # Find Persons that have a preferred email address and an active
4710 # team, or both.4713 # account, or are a team, or both.
4711 intermediate_transitive_results = source.find(4714 intermediate_transitive_results = source.find(
4712 (TeamMembership.personID, EmailAddress.personID),4715 (TeamMembership.personID, EmailAddress.personID),
4713 In(TeamMembership.status,4716 In(TeamMembership.status,
@@ -4715,7 +4718,8 @@
4715 TeamMembershipStatus.APPROVED.value]),4718 TeamMembershipStatus.APPROVED.value]),
4716 In(TeamMembership.teamID, pending_team_ids),4719 In(TeamMembership.teamID, pending_team_ids),
4717 Or(4720 Or(
4718 EmailAddress.personID != None,4721 And(EmailAddress.personID != None,
4722 Account.status == AccountStatus.ACTIVE),
4719 Person.teamownerID != None)).config(distinct=True)4723 Person.teamownerID != None)).config(distinct=True)
4720 next_ids = []4724 next_ids = []
4721 for (person_id,4725 for (person_id,
47224726
=== 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-26 10:09:24 +0000
@@ -1719,3 +1719,36 @@
1719 super_team_member_person,1719 super_team_member_person,
1720 super_team_member_team]),1720 super_team_member_team]),
1721 set(recipients))1721 set(recipients))
1722
1723 def test_get_recipients_team_with_disabled_owner_account(self):
1724 """Mail is not sent to a team owner whose account is disabled.
1725
1726 See <https://bugs.launchpad.net/launchpad/+bug/855150>
1727 """
1728 owner = self.factory.makePerson(email='foo@bar.com')
1729 team = self.factory.makeTeam(owner)
1730 owner.account.status = AccountStatus.DEACTIVATED
1731 self.assertContentEqual([], get_recipients(team))
1732
1733 def test_get_recipients_team_with_disabled_member_account(self):
1734 """Mail is not sent to a team member whose account is disabled.
1735
1736 See <https://bugs.launchpad.net/launchpad/+bug/855150>
1737 """
1738 person = self.factory.makePerson(email='foo@bar.com')
1739 person.account.status = AccountStatus.DEACTIVATED
1740 team = self.factory.makeTeam(members=[person])
1741 self.assertContentEqual([team.teamowner], get_recipients(team))
1742
1743 def test_get_recipients_team_with_nested_disabled_member_account(self):
1744 """Mail is not sent to transitive team member with disabled account.
1745
1746 See <https://bugs.launchpad.net/launchpad/+bug/855150>
1747 """
1748 person = self.factory.makePerson(email='foo@bar.com')
1749 person.account.status = AccountStatus.DEACTIVATED
1750 team1 = self.factory.makeTeam(members=[person])
1751 team2 = self.factory.makeTeam(members=[team1])
1752 self.assertContentEqual(
1753 [team2.teamowner],
1754 get_recipients(team2))