launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1035288 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1035288 in Launchpad itself: "ValidPersonOrTeamVocabulary doesn't know that commercial admins can see private teams"
  https://bugs.launchpad.net/launchpad/+bug/1035288

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1035288/+merge/119125

Commercial admins hold launchpad.View on all private teams, but ValidPersonOrTeamVocabulary thinks that only admins and participants can see them. This means commercial admins can find private teams, but not use them in any of the normal person fields.

Some of the fragments of the main vocab doctest used commercial-member as an unprivileged user. I switched them to use no-priv instead.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-1035288/+merge/119125
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1035288 into lp:launchpad.

=== modified file 'lib/lp/registry/doc/vocabularies.txt'
--- lib/lp/registry/doc/vocabularies.txt	2012-07-27 13:12:41 +0000
+++ lib/lp/registry/doc/vocabularies.txt	2012-08-10 12:52:23 +0000
@@ -801,13 +801,13 @@
 A PRIVATE team is displayed when the logged in user is a member of the
 team.
 
-    >>> commercial = person_set.getByEmail('commercial-member@xxxxxxxxxxxxx')
-    >>> vocab = get_naked_vocab(commercial, "ValidPersonOrTeam")
-    >>> login('commercial-member@xxxxxxxxxxxxx')
+    >>> no_priv = person_set.getByEmail('no-priv@xxxxxxxxxxxxx')
+    >>> vocab = get_naked_vocab(no_priv, "ValidPersonOrTeam")
+    >>> login('no-priv@xxxxxxxxxxxxx')
     >>> priv_team = factory.makeTeam(
     ...     name='private-team',
     ...     displayname='Private Team',
-    ...     owner=commercial,
+    ...     owner=no_priv,
     ...     visibility=PersonVisibility.PRIVATE)
     >>> sorted(person.name for person in vocab.search('team'))
     [u'hwdb-team', u'name18', u'name20', u'name21',
@@ -815,7 +815,8 @@
      u'simple-team', u'testing-spanish-team', u'ubuntu-security',
      u'ubuntu-team', u'warty-gnome']
 
-The PRIVATE team is also displayed for Launchpad admins.
+The PRIVATE team is also displayed for Launchpad admins or commercial
+admins.
 
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> sorted(person.name for person in vocab.search('team'))
@@ -824,6 +825,14 @@
      u'private-team', u'public-team', u'simple-team',
      u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
      u'warty-gnome']
+    >>> logout()
+    >>> login('commercial-member@xxxxxxxxxxxxx')
+    >>> sorted(person.name for person in vocab.search('team'))
+    [u'hwdb-team', u'myteam', u'name18', u'name20', u'name21',
+     u'no-team-memberships', u'otherteam',
+     u'private-team', u'public-team', u'simple-team',
+     u'testing-spanish-team', u'ubuntu-security', u'ubuntu-team',
+     u'warty-gnome']
 
 The PRIVATE team can be looked up via getTermByToken for a member of the
 team.
@@ -862,7 +871,7 @@
 Searching for all teams, which requires monkey-patching the
 `allow_null_search` property, will also return the private team.
 
-    >>> login('commercial-member@xxxxxxxxxxxxx')
+    >>> login('no-priv@xxxxxxxxxxxxx')
     >>> vocab.allow_null_search = True
     >>> sorted(person.name for person in vocab.search(''))
     [...u'private-team'...]
@@ -1085,13 +1094,13 @@
 A user who is a member of a private team will see that team in his
 search.
 
-    >>> login('commercial-member@xxxxxxxxxxxxx')
+    >>> login('no-priv@xxxxxxxxxxxxx')
     >>> sorted((team.displayname, team.teamowner.displayname)
     ...        for team in vocab.search('team'))
     [(u'HWDB Team', u'Foo Bar'),
      (u'Hoary Gnome Team', u'Mark Shuttleworth'),
      (u'Other Team', u'Owner'),
-     (u'Private Team', u'Commercial Member'),
+     (u'Private Team', u'No Privileges Person'),
      (u'Simple Team', u'One Membership'),
      (u'Ubuntu Gnome Team', u'Mark Shuttleworth'),
      (u'Ubuntu Security Team', u'Colin Watson'),

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-07-30 00:05:44 +0000
+++ lib/lp/registry/vocabularies.py	2012-08-10 12:52:23 +0000
@@ -101,7 +101,6 @@
     removeSecurityProxy,
     )
 
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.blueprints.interfaces.specification import ISpecification
 from lp.bugs.interfaces.bugtask import IBugTask
 from lp.registry.interfaces.accesspolicy import IAccessPolicySource
@@ -141,6 +140,7 @@
     )
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.projectgroup import IProjectGroup
+from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.interfaces.sourcepackagename import ISourcePackageName
 from lp.registry.model.distribution import Distribution
@@ -569,10 +569,10 @@
         tables = []
         logged_in_user = getUtility(ILaunchBag).user
         if logged_in_user is not None:
-            celebrities = getUtility(ILaunchpadCelebrities)
-            if logged_in_user.inTeam(celebrities.admin):
-                # If the user is a LP admin we allow all private teams to be
-                # visible.
+            roles = IPersonRoles(logged_in_user)
+            if roles.in_admin or roles.in_commercial_admin:
+                # If the user is a LP admin or commercial admin we allow
+                # all private teams to be visible.
                 private_query = AND(
                     Not(Person.teamowner == None),
                     Person.visibility == PersonVisibility.PRIVATE)