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
1=== modified file 'lib/lp/registry/model/person.py'
2--- lib/lp/registry/model/person.py 2017-06-09 00:03:35 +0000
3+++ lib/lp/registry/model/person.py 2017-09-28 18:04:26 +0000
4@@ -4012,12 +4012,12 @@
5 # Checking validity requires having a preferred email.
6 if not need_api and need_preferred_email and not need_validity:
7 # Teams don't have email, so a left join
8- origin.append(
9- LeftJoin(EmailAddress, EmailAddress.person == Person.id))
10+ origin.append(LeftJoin(
11+ EmailAddress,
12+ And(
13+ EmailAddress.person == Person.id,
14+ EmailAddress.status == EmailAddressStatus.PREFERRED)))
15 columns.append(EmailAddress)
16- conditions = And(conditions,
17- Or(EmailAddress.status == None,
18- EmailAddress.status == EmailAddressStatus.PREFERRED))
19 if need_validity or need_api:
20 valid_stuff = Person._validity_queries()
21 origin.extend(valid_stuff["joins"])
22
23=== modified file 'lib/lp/registry/tests/test_personset.py'
24--- lib/lp/registry/tests/test_personset.py 2016-05-25 06:41:01 +0000
25+++ lib/lp/registry/tests/test_personset.py 2017-09-28 18:04:26 +0000
26@@ -1,4 +1,4 @@
27-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
28+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
29 # GNU Affero General Public License version 3 (see the file LICENSE).
30
31 """Tests for PersonSet."""
32@@ -7,6 +7,7 @@
33
34
35 from testtools.matchers import (
36+ Equals,
37 GreaterThan,
38 LessThan,
39 )
40@@ -154,6 +155,39 @@
41 person.teamowner
42 self.assertThat(recorder, HasQueryCount(LessThan(1)))
43
44+ def test_getPrecachedPersonsFromIDs_preferred_email(self):
45+ # getPrecachedPersonsFromIDs() sets preferredemail to the preferred
46+ # address if it exists, but otherwise leaves it as none.
47+ team_no_contact = self.factory.makeTeam(email=None)
48+ team_contact = self.factory.makeTeam(email=u'team@example.com')
49+ team_list = self.factory.makeTeam(email=None)
50+ self.factory.makeMailingList(team_list, team_list.teamowner)
51+ person_normal = self.factory.makePerson()
52+ person_unactivated = self.factory.makePerson(
53+ account_status=AccountStatus.NOACCOUNT)
54+ person_ids = [
55+ t.id for t in [
56+ team_no_contact, team_contact, team_list, person_normal,
57+ person_unactivated]]
58+ transaction.commit()
59+
60+ with StormStatementRecorder() as recorder:
61+ list(self.person_set.getPrecachedPersonsFromIDs(
62+ person_ids, need_preferred_email=True))
63+ self.assertThat(recorder, HasQueryCount(Equals(1)))
64+
65+ with StormStatementRecorder() as recorder:
66+ self.assertIsNone(team_no_contact.preferredemail)
67+ self.assertEqual(
68+ EmailAddressStatus.PREFERRED,
69+ team_contact.preferredemail.status)
70+ self.assertIsNone(team_list.preferredemail)
71+ self.assertIsNone(person_unactivated.preferredemail)
72+ self.assertEqual(
73+ EmailAddressStatus.PREFERRED,
74+ person_normal.preferredemail.status)
75+ self.assertThat(recorder, HasQueryCount(Equals(0)))
76+
77 def test_getPrecachedPersonsFromIDs_is_ubuntu_coc_signer(self):
78 # getPrecachedPersonsFromIDs() sets is_ubuntu_coc_signer
79 # correctly.