launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/product-limitedview-with-team-aag into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1095982 in Launchpad itself: "Person.getAffiliatedPillars doesn't filter out inaccessible private projects"
  https://bugs.launchpad.net/launchpad/+bug/1095982

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/product-limitedview-with-team-aag/+merge/142441

Currently, the LimitedView security adapter for IProduct checks if a user has an AccessArtifactGrant on a bug, branch or specification for the private project. This is fine, and works well, except that it doesn't work if a team the user is a member has the AAG. As such, I've rewritten ISharingService.userHasGrantsOnPillar() to respect TeamParticipation.

I have done some drive-by on some asserts, using assert{True,False} rather than assertEqual({True,False}, and fixed one indentation error I noticed.

I have more than enough LoC credit for the +19 gain.
-- 
https://code.launchpad.net/~stevenk/launchpad/product-limitedview-with-team-aag/+merge/142441
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/product-limitedview-with-team-aag into lp:launchpad.

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-12-02 03:30:18 +0000
+++ lib/lp/registry/services/sharingservice.py	2013-01-09 05:21:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes for pillar and artifact sharing service."""
@@ -229,8 +229,18 @@
 
     def userHasGrantsOnPillar(self, pillar, user):
         """See `ISharingService`."""
-        return not self.getArtifactGrantsForPersonOnPillar(
-            pillar, user).is_empty()
+        tables = [
+            AccessPolicyGrantFlat,
+            Join(
+                TeamParticipation,
+                TeamParticipation.teamID == AccessPolicyGrantFlat.grantee_id),
+            Join(
+                AccessPolicy,
+                AccessPolicy.id == AccessPolicyGrantFlat.policy_id)]
+        return not IStore(AccessPolicyGrantFlat).using(*tables).find(
+            AccessPolicyGrantFlat,
+            AccessPolicy.product_id == pillar.id,
+            TeamParticipation.personID == user.id).is_empty()
 
     @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedBugs(self, pillar, person, user):

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-11-29 20:27:19 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2013-01-09 05:21:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -1762,19 +1762,28 @@
             self.service.sharePillarInformation(
                 product, wrong_person, product.owner,
                 {InformationType.PRIVATESECURITY: SharingPermission.ALL})
-        self.assertEqual(
-            False,
+        self.assertFalse(
             self.service.checkPillarAccess(
                 [product], InformationType.USERDATA, wrong_person))
-        self.assertEqual(
-            True,
+        self.assertTrue(
             self.service.checkPillarAccess(
                 [product], InformationType.USERDATA, right_person))
 
+    def test_userHasGrantsOnPillar_respects_teams(self):
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY, owner=owner)
+        person = self.factory.makePerson()
+        team = self.factory.makeTeam(
+            membership_policy=TeamMembershipPolicy.MODERATED, members=[person])
+        with person_logged_in(owner):
+            bug = self.factory.makeBug(target=product)
+            bug.subscribe(team, owner)
+        self.assertTrue(self.service.userHasGrantsOnPillar(product, person))
+
     def test_checkPillarAccess_no_policy(self):
         # checkPillarAccess returns False if there's no policy.
-        self.assertEqual(
-            False,
+        self.assertFalse(
             self.service.checkPillarAccess(
                 [self.factory.makeProduct()], InformationType.PUBLIC,
                 self.factory.makePerson()))

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2013-01-03 03:41:13 +0000
+++ lib/lp/registry/tests/test_product.py	2013-01-09 05:21:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -1184,7 +1184,7 @@
                 # The second access does not require another query.
                 product.description
                 self.assertEqual(
-                queries_for_first_user_access, len(recorder.queries))
+                    queries_for_first_user_access, len(recorder.queries))
 
     def test_userCanView_works_with_IPersonRoles(self):
         # userCanView() maintains a cache of users known to have the