launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/person-vocab-1006244 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1006244 in Launchpad itself: "sharee picker shows suspended/invalid people"
  https://bugs.launchpad.net/launchpad/+bug/1006244

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/person-vocab-1006244/+merge/108124

== Implementation ==

This branch filters people with inactive accounts from person vocab results.

Add a new left join to Account table main person query and filter result on active accounts or teams.
Query was checked on dogfood and its all good.

== Tests ==

Add new test to test_person_vocabularies:
- test_inactive_people_ignored

Run existing vocab tests to check that other queries still work.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/vocabularies.py
  lib/lp/registry/tests/test_person_vocabularies.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/person-vocab-1006244/+merge/108124
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/person-vocab-1006244 into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_person_vocabularies.py'
--- lib/lp/registry/tests/test_person_vocabularies.py	2012-04-11 02:15:10 +0000
+++ lib/lp/registry/tests/test_person_vocabularies.py	2012-05-31 08:49:20 +0000
@@ -20,6 +20,7 @@
     TeamSubscriptionPolicy,
     )
 from lp.registry.vocabularies import ValidPersonOrTeamVocabulary
+from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.webapp.vocabulary import FilteredVocabularyBase
 from lp.testing import (
     login_person,
@@ -155,6 +156,18 @@
         results = self.searchVocabulary(None, u'fred', 'TEAM')
         self.assertContentEqual(teams, list(results))
 
+    def test_inactive_people_ignored(self):
+        # Only people with active accounts (or teams) are returned.
+        for status in AccountStatus:
+            if status.value != AccountStatus.ACTIVE:
+                self.factory.makePerson(
+                    name='fred' + status.token.lower(),
+                    account_status=status.value)
+        active_person = self.factory.makePerson(name='fredactive')
+        team = self.factory.makePerson(name='fredteam')
+        results = self.searchVocabulary(None, 'fred')
+        self.assertContentEqual([active_person, team], list(results))
+
 
 class TestValidPersonOrTeamVocabulary(ValidPersonOrTeamVocabularyMixin,
                                       TestCaseWithFactory):

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-05-29 03:16:43 +0000
+++ lib/lp/registry/vocabularies.py	2012-05-31 08:49:20 +0000
@@ -180,7 +180,9 @@
     ensure_unicode,
     shortlist,
     )
+from lp.services.identity.interfaces.account import AccountStatus
 from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
+from lp.services.identity.model.account import Account
 from lp.services.identity.model.emailaddress import EmailAddress
 from lp.services.propertycache import (
     cachedproperty,
@@ -701,6 +703,7 @@
                 SQL("MatchingPerson"),
                 Person,
                 LeftJoin(EmailAddress, EmailAddress.person == Person.id),
+                LeftJoin(Account, Account.id == Person.accountID),
                 ]
 
             # If private_tables is empty, we are searching for all private
@@ -721,6 +724,9 @@
                 And(
                     SQL("Person.id = MatchingPerson.id"),
                     Or(
+                        Account.status == AccountStatus.ACTIVE,
+                        Person.teamowner != None),
+                    Or(
                         And(  # A public person or team
                             Person.visibility == PersonVisibility.PUBLIC,
                             Person.merged == None,