Merge lp:~adeuring/launchpad/revokeAccessGrants_specs into lp:launchpad

[Abel Deuring]

SharingService.revokeAccessGrants() extended for specifications.

This branch extends the method SharingService.revokeAccessGrants() to
handle specifications too.

The changes mostly copy the pattern already used for bugs and branches.

test: ./bin/test -vvt lp.registry.services.tests.test_sharingservice

no lint

[Richard Harding (rharding)]

Thanks Abel, just one typo in #164 of the diff. Subscibing...

review: Approve

=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py	2012-09-25 06:16:36 +0000
+++ lib/lp/registry/interfaces/sharingservice.py	2012-09-26 08:35:24 +0000
@@ -306,7 +306,8 @@
         branches=List(
             Reference(schema=IBranch), title=_('Branches'), required=False))
     @operation_for_version('devel')
-    def revokeAccessGrants(pillar, grantee, user, branches=None, bugs=None):
+    def revokeAccessGrants(pillar, grantee, user, branches=None, bugs=None,
+                           specifications=None):
         """Remove a grantee's access to the specified artifacts.
 
         :param pillar: the pillar from which to remove access
@@ -314,6 +315,7 @@
         :param user: the user making the request
         :param bugs: the bugs for which to revoke access
         :param branches: the branches for which to revoke access
+        :param specifications: the specifications for which to revoke access
         """
 
     @export_write_operation()

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-09-25 17:47:36 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-09-26 08:35:24 +0000
@@ -670,17 +670,20 @@
 
     @available_with_permission('launchpad.Edit', 'pillar')
     def revokeAccessGrants(self, pillar, grantee, user, branches=None,
-                           bugs=None):
+                           bugs=None, specifications=None):
         """See `ISharingService`."""
 
-        if not branches and not bugs:
-            raise ValueError("Either bugs or branches must be specified")
+        if not branches and not bugs and not specifications:
+            raise ValueError(
+                "Either bugs, branches or specifications must be specified")
 
         artifacts = []
         if branches:
             artifacts.extend(branches)
         if bugs:
             artifacts.extend(bugs)
+        if specifications:
+            artifacts.extend(specifications)
         # Find the access artifacts associated with the bugs and branches.
         accessartifact_source = getUtility(IAccessArtifactSource)
         artifacts_to_delete = accessartifact_source.find(artifacts)

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-09-25 17:47:36 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-09-26 08:35:24 +0000
@@ -888,12 +888,15 @@
         self._assert_deleteGranteeRemoveSubscriptions(
             [InformationType.USERDATA])
 
-    def _assert_revokeAccessGrants(self, pillar, bugs, branches):
+    def _assert_revokeAccessGrants(self, pillar, bugs, branches,
+                                   specifications):
         artifacts = []
         if bugs:
             artifacts.extend(bugs)
         if branches:
             artifacts.extend(branches)
+        if specifications:
+            artifacts.extend(specifications)
         policy = self.factory.makeAccessPolicy(pillar=pillar)
         # Grant access to a grantee and another person.
         grantee = self.factory.makePerson()
@@ -918,6 +921,8 @@
                 branch.subscribe(person,
                     BranchSubscriptionNotificationLevel.NOEMAIL, None,
                     CodeReviewNotificationLevel.NOEMAIL, pillar.owner)
+            for spec in specifications or []:
+                spec.subscribe(person)
 
         # Check that grantee has expected access grants.
         accessartifact_grant_source = getUtility(IAccessArtifactGrantSource)
@@ -927,7 +932,8 @@
         self.assertEqual(1, grants.count())
 
         self.service.revokeAccessGrants(
-            pillar, grantee, pillar.owner, bugs=bugs, branches=branches)
+            pillar, grantee, pillar.owner, bugs=bugs, branches=branches,
+            specifications=specifications)
         with block_on_job(self):
             transaction.commit()
 
@@ -941,6 +947,8 @@
             self.assertNotIn(grantee, bug.getDirectSubscribers())
         for branch in branches or []:
             self.assertNotIn(grantee, branch.subscribers)
+        for spec in specifications or []:
+            self.assertNotIn(grantee, spec.subscribers)
 
         # Someone else still has access to the bugs and branches.
         grants = accessartifact_grant_source.findByArtifact(
@@ -951,6 +959,8 @@
             self.assertIn(someone, bug.getDirectSubscribers())
         for branch in branches or []:
             self.assertIn(someone, branch.subscribers)
+        for spec in specifications or []:
+            self.assertIn(someone, spec.subscribers)
 
     def test_revokeAccessGrantsBugs(self):
         # Users with launchpad.Edit can delete all access for a grantee.
@@ -960,7 +970,7 @@
         bug = self.factory.makeBug(
             target=distro, owner=owner,
             information_type=InformationType.USERDATA)
-        self._assert_revokeAccessGrants(distro, [bug], None)
+        self._assert_revokeAccessGrants(distro, [bug], None, None)
 
     def test_revokeAccessGrantsBranches(self):
         owner = self.factory.makePerson()
@@ -969,14 +979,30 @@
         branch = self.factory.makeBranch(
             product=product, owner=owner,
             information_type=InformationType.USERDATA)
-        self._assert_revokeAccessGrants(product, None, [branch])
-
-    def _assert_revokeTeamAccessGrants(self, pillar, bugs, branches):
+        self._assert_revokeAccessGrants(product, None, [branch], None)
+
+    def test_revokeAccessGrantsSpecifications(self):
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            owner=owner, specification_sharing_policy=(
+                SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY))
+        self.factory.makeAccessPolicy(
+            pillar=product, type=InformationType.EMBARGOED)
+        login_person(owner)
+        specification = self.factory.makeSpecification(
+            product=product, owner=owner,
+            information_type=InformationType.EMBARGOED)
+        self._assert_revokeAccessGrants(product, None, None, [specification])
+
+    def _assert_revokeTeamAccessGrants(self, pillar, bugs, branches,
+                                       specifications):
         artifacts = []
         if bugs:
             artifacts.extend(bugs)
         if branches:
             artifacts.extend(branches)
+        if specifications:
+            artifacts.extend(specifications)
         policy = self.factory.makeAccessPolicy(pillar=pillar)
 
         person_grantee = self.factory.makePerson()
@@ -984,7 +1010,7 @@
         team_grantee = self.factory.makeTeam(
             owner=team_owner,
             membership_policy=TeamMembershipPolicy.RESTRICTED,
-            members=[person_grantee])
+            members=[person_grantee], email='[e-mail]')
 
         # Subscribe the team and person grantees to the artifacts.
         for person in [team_grantee, person_grantee]:
@@ -1000,19 +1026,29 @@
                 branch.subscribe(
                     person, BranchSubscriptionNotificationLevel.NOEMAIL,
                     None, CodeReviewNotificationLevel.NOEMAIL, pillar.owner)
+            # Subscribing somebody to a specification does not yet imply
+            # granting access to this person.
+            if specifications:
+                self.service.ensureAccessGrants(
+                    [person], pillar.owner, specifications=specifications)
+            for spec in specifications or []:
+                spec.subscribe(person)
 
         # Check that grantees have expected access grants and subscriptions.
         for person in [team_grantee, person_grantee]:
             visible_bugs, visible_branches, visible_specs = (
-                self.service.getVisibleArtifacts(person, branches, bugs))
+                self.service.getVisibleArtifacts(
+                    person, branches, bugs, specifications))
             self.assertContentEqual(bugs or [], visible_bugs)
             self.assertContentEqual(branches or [], visible_branches)
+            self.assertContentEqual(specifications or [], visible_specs)
         for person in [team_grantee, person_grantee]:
             for bug in bugs or []:
                 self.assertIn(person, bug.getDirectSubscribers())
 
         self.service.revokeAccessGrants(
-            pillar, team_grantee, pillar.owner, bugs=bugs, branches=branches)
+            pillar, team_grantee, pillar.owner, bugs=bugs, branches=branches,
+            specifications=specifications)
         with block_on_job(self):
             transaction.commit()
 
@@ -1031,6 +1067,7 @@
                 self.service.getVisibleArtifacts(person, branches, bugs))
             self.assertContentEqual([], visible_bugs)
             self.assertContentEqual([], visible_branches)
+            self.assertContentEqual([], visible_specs)
 
     def test_revokeTeamAccessGrantsBugs(self):
         # Users with launchpad.Edit can delete all access for a grantee.
@@ -1040,7 +1077,7 @@
         bug = self.factory.makeBug(
             target=distro, owner=owner,
             information_type=InformationType.USERDATA)
-        self._assert_revokeTeamAccessGrants(distro, [bug], None)
+        self._assert_revokeTeamAccessGrants(distro, [bug], None, None)
 
     def test_revokeTeamAccessGrantsBranches(self):
         # Users with launchpad.Edit can delete all access for a grantee.
@@ -1049,7 +1086,21 @@
         login_person(owner)
         branch = self.factory.makeBranch(
             owner=owner, information_type=InformationType.USERDATA)
-        self._assert_revokeTeamAccessGrants(product, None, [branch])
+        self._assert_revokeTeamAccessGrants(product, None, [branch], None)
+
+    def test_revokeTeamAccessGrantsSpecifications(self):
+        # Users with launchpad.Edit can delete all access for a grantee.
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            owner=owner, specification_sharing_policy=(
+                SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY))
+        self.factory.makeAccessPolicy(product, InformationType.EMBARGOED)
+        login_person(owner)
+        specification = self.factory.makeSpecification(
+            product=product, owner=owner,
+            information_type=InformationType.EMBARGOED)
+        self._assert_revokeTeamAccessGrants(
+            product, None, None, [specification])
 
     def _assert_revokeAccessGrantsUnauthorized(self):
         # revokeAccessGrants raises an Unauthorized exception if the user