Merge lp:~lifeless/launchpad/features into lp:launchpad

[Robert Collins]

Add a team: scope.

Not much to say. Team based scope. \o/. Drive by fix in inTeam to handle nonexistant teams more gracefully. (We could raise, but the existing handling for e.g. teams that are Persons is to return False, so I followed that style, for now).

[Jeroen T. Vermeulen (jtv)]

As discussed on IRC. Note that existing behaviour is for teams that are Persons is not actually to return False as opposed to raising an exception; it is to return False if the "team" is not the same person as self (which is the only circumstance under which there could be a membership).

Since you made a change in the interface where a mistake that would previously raise an error will now be quietly accepted, you're adding a test and documentation for the change.

review: Approve (code)

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2010-11-04 00:18:34 +0000
+++ lib/lp/registry/interfaces/person.py	2010-11-04 07:27:52 +0000
@@ -1074,8 +1074,11 @@
         since it inherits from `IPerson`) is a member of himself
         (i.e. `person1.inTeam(person1)`).
 
-        :param team: An object providing `IPerson`, the name of a
-            team, or `None` (in which case `False` is returned).
+        :param team: One of an object providing `IPerson`, the string name of a
+            team or `None`. If a string was supplied the team is looked up.
+        :return: A bool with the result of the membership lookup. When looking
+            up the team from a string finds nothing or team was `None` then 
+            `False` is returned.
         """
 
     def clearInTeamCache():
 
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-11-03 20:25:27 +0000
+++ lib/lp/registry/model/person.py	2010-11-04 07:27:52 +0000
@@ -1191,6 +1191,9 @@
         # Translate the team name to an ITeam if we were passed a team.
         if isinstance(team, (str, unicode)):
             team = PersonSet().getByName(team)
+            if team is None:
+                # No team, no membership.
+                return False
 
         if self.id == team.id:
             # A team is always a member of itself.
 
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2010-11-03 13:58:34 +0000
+++ lib/lp/registry/tests/test_person.py	2010-11-04 07:27:52 +0000
@@ -220,6 +220,13 @@
             {},
             removeSecurityProxy(self.user)._inTeam_cache)
 
+    def test_inTeam_person_string_missing_team(self):
+        # If a check against a string is done, the team lookup is implicit:
+        # treat a missing team as an empty team so that any pages that choose
+        # to do this don't blow up unnecessarily. Similarly feature flags 
+        # team: scopes depend on this.
+        self.assertFalse(self.user.inTeam('does-not-exist'))
+
 
 class TestPerson(TestCaseWithFactory):
 
 
=== modified file 'lib/lp/services/features/tests/test_webapp.py'
--- lib/lp/services/features/tests/test_webapp.py	2010-09-13 00:55:15 +0000
+++ lib/lp/services/features/tests/test_webapp.py	2010-11-04 07:27:52 +0000
@@ -7,7 +7,11 @@
 
 from canonical.testing import layers
 from lp.services.features import webapp
-from lp.testing import TestCase
+from lp.testing import (
+    login_as,
+    TestCase,
+    TestCaseWithFactory,
+    )
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 
 
@@ -38,4 +42,23 @@
         scopes = webapp.ScopesFromRequest(request)
         self.assertTrue(scopes.lookup('pageid:'))
         self.assertFalse(scopes.lookup('pageid:foo'))
-        self.assertFalse(scopes.lookup('pageid:foo'))
+        self.assertFalse(scopes.lookup('pageid:foo:bar'))
+
+
+class TestDBScopes(TestCaseWithFactory):
+
+    layer = layers.DatabaseFunctionalLayer
+
+    def test_team_scope_outside_team(self):
+        request = LaunchpadTestRequest()
+        scopes = webapp.ScopesFromRequest(request)
+        self.factory.loginAsAnyone()
+        self.assertFalse(scopes.lookup('team:nonexistent'))
+
+    def test_team_scope_in_team(self):
+        request = LaunchpadTestRequest()
+        scopes = webapp.ScopesFromRequest(request)
+        member = self.factory.makePerson()
+        team = self.factory.makeTeam(members=[member])
+        login_as(member)
+        self.assertTrue(scopes.lookup('team:%s' % team.name))
 
=== modified file 'lib/lp/services/features/webapp.py'
--- lib/lp/services/features/webapp.py	2010-10-25 17:29:07 +0000
+++ lib/lp/services/features/webapp.py	2010-11-04 07:27:52 +0000
@@ -7,7 +7,10 @@
 
 __metaclass__ = type
 
+from zope.component import getUtility
+
 import canonical.config
+from canonical.launchpad.webapp.interfaces import ILaunchBag
 from lp.services.features import per_thread
 from lp.services.features.flags import FeatureController
 from lp.services.features.rulesource import StormFeatureRuleSource
@@ -34,11 +37,16 @@
              - pageid:SomeType
              - pageid:SomeType:+view
              - pageid:SomeType:+view#subselector
+         - team:
+           This scope looks up a team. For instance
+             - team:launchpad-beta-users
         """
         if scope_name == 'default':
             return True
         if scope_name.startswith('pageid:'):
             return self._lookup_pageid(scope_name[len('pageid:'):])
+        if scope_name.startswith('team:'):
+            return self._lookup_team(scope_name[len('team:'):])
         parts = scope_name.split('.')
         if len(parts) == 2:
             if parts[0] == 'server':
@@ -65,6 +73,23 @@
                 return False
         return True
 
+    def _lookup_team(self, team_name):
+        """Lookup a team membership as a scope.
+
+        This will do a two queries, so we probably want to keep the number of
+        team based scopes in use to a small number. (Person.inTeam could be
+        fixed to reduce this to one query).
+
+        teamid scopes are written as 'team:' + the team name to match.
+
+        E.g. the scope 'team:launchpad-beta-users' will match members of
+        the team 'launchpad-beta-users'.
+        """
+        person = getUtility(ILaunchBag).user
+        if person is None:
+            return False
+        return person.inTeam(team_name)
+
     def _pageid_to_namespace(self, pageid):
         """Return a list of namespace elements for pageid."""
         # Normalise delimiters.