Launchpad itself

Merge lp:~wgrant/launchpad/gPPFID-email-argh into lp:launchpad

  1. gPPFID-email-argh
  2. Merge into devel
Proposed by William Grant
Status: Merged
Merged at revision: 18471
Proposed branch: lp:~wgrant/launchpad/gPPFID-email-argh
Merge into: lp:launchpad
Diff against target: 79 lines (+40/-6)
2 files modified
lib/lp/registry/model/person.py (+5/-5)
lib/lp/registry/tests/test_personset.py (+35/-1)
To merge this branch: bzr merge lp:~wgrant/launchpad/gPPFID-email-argh
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+331517@code.launchpad.net

Commit message

Fix getPrecachedPersonsFromIDs to handle teams with mailing lists.

Description of the change

Fix getPrecachedPersonsFromIDs to handle teams with mailing lists.

Previously, when need_preferred_email was true, a team with a mailing list but
no contact address (an email address, but only a non-preferred one) would be
filtered out by the WHERE, resulting in no precaching, and indeed not being
returned by the method at all.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

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 2017-06-09 00:03:35 +0000
+++ lib/lp/registry/model/person.py 2017-09-28 18:04:26 +0000
@@ -4012,12 +4012,12 @@
4012 # Checking validity requires having a preferred email.4012 # Checking validity requires having a preferred email.
4013 if not need_api and need_preferred_email and not need_validity:4013 if not need_api and need_preferred_email and not need_validity:
4014 # Teams don't have email, so a left join4014 # Teams don't have email, so a left join
4015 origin.append(4015 origin.append(LeftJoin(
4016 LeftJoin(EmailAddress, EmailAddress.person == Person.id))4016 EmailAddress,
4017 And(
4018 EmailAddress.person == Person.id,
4019 EmailAddress.status == EmailAddressStatus.PREFERRED)))
4017 columns.append(EmailAddress)4020 columns.append(EmailAddress)
4018 conditions = And(conditions,
4019 Or(EmailAddress.status == None,
4020 EmailAddress.status == EmailAddressStatus.PREFERRED))
4021 if need_validity or need_api:4021 if need_validity or need_api:
4022 valid_stuff = Person._validity_queries()4022 valid_stuff = Person._validity_queries()
4023 origin.extend(valid_stuff["joins"])4023 origin.extend(valid_stuff["joins"])
40244024
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py 2016-05-25 06:41:01 +0000
+++ lib/lp/registry/tests/test_personset.py 2017-09-28 18:04:26 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2016 Canonical Ltd. This software is licensed under the1# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Tests for PersonSet."""4"""Tests for PersonSet."""
@@ -7,6 +7,7 @@
77
88
9from testtools.matchers import (9from testtools.matchers import (
10 Equals,
10 GreaterThan,11 GreaterThan,
11 LessThan,12 LessThan,
12 )13 )
@@ -154,6 +155,39 @@
154 person.teamowner155 person.teamowner
155 self.assertThat(recorder, HasQueryCount(LessThan(1)))156 self.assertThat(recorder, HasQueryCount(LessThan(1)))
156157
158 def test_getPrecachedPersonsFromIDs_preferred_email(self):
159 # getPrecachedPersonsFromIDs() sets preferredemail to the preferred
160 # address if it exists, but otherwise leaves it as none.
161 team_no_contact = self.factory.makeTeam(email=None)
162 team_contact = self.factory.makeTeam(email=u'team@example.com')
163 team_list = self.factory.makeTeam(email=None)
164 self.factory.makeMailingList(team_list, team_list.teamowner)
165 person_normal = self.factory.makePerson()
166 person_unactivated = self.factory.makePerson(
167 account_status=AccountStatus.NOACCOUNT)
168 person_ids = [
169 t.id for t in [
170 team_no_contact, team_contact, team_list, person_normal,
171 person_unactivated]]
172 transaction.commit()
173
174 with StormStatementRecorder() as recorder:
175 list(self.person_set.getPrecachedPersonsFromIDs(
176 person_ids, need_preferred_email=True))
177 self.assertThat(recorder, HasQueryCount(Equals(1)))
178
179 with StormStatementRecorder() as recorder:
180 self.assertIsNone(team_no_contact.preferredemail)
181 self.assertEqual(
182 EmailAddressStatus.PREFERRED,
183 team_contact.preferredemail.status)
184 self.assertIsNone(team_list.preferredemail)
185 self.assertIsNone(person_unactivated.preferredemail)
186 self.assertEqual(
187 EmailAddressStatus.PREFERRED,
188 person_normal.preferredemail.status)
189 self.assertThat(recorder, HasQueryCount(Equals(0)))
190
157 def test_getPrecachedPersonsFromIDs_is_ubuntu_coc_signer(self):191 def test_getPrecachedPersonsFromIDs_is_ubuntu_coc_signer(self):
158 # getPrecachedPersonsFromIDs() sets is_ubuntu_coc_signer192 # getPrecachedPersonsFromIDs() sets is_ubuntu_coc_signer
159 # correctly.193 # correctly.