Merge lp:~stevenk/launchpad/branch-unsubscribe-revokes into lp:launchpad

[Steve Kowalik]

Fix IBranch.unsubscribe() to remove AccessArtifactGrants, like it should have when subscribe() added them.

In https://code.launchpad.net/~stevenk/launchpad/branch-subscribe-aag/+merge/108881 I changed Branch.subscribe() to add an AccessArtifactGrant if required. Unfortunately, I neglected to change Branch.unsubscribe() to remove the AAG, hence this branch.

[Curtis Hovey (sinzui)]

I think the unsubscribe case needs a test.

review: Needs Information (code)

[Curtis Hovey (sinzui)]

review: Approve (code)

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2012-06-18 01:04:30 +0000
+++ lib/lp/code/model/branch.py	2012-06-27 22:34:23 +0000
@@ -137,6 +137,10 @@
     PRIVATE_INFORMATION_TYPES,
     PUBLIC_INFORMATION_TYPES,
     )
+from lp.registry.interfaces.accesspolicy import (
+    IAccessArtifactGrantSource,
+    IAccessArtifactSource,
+    )
 from lp.registry.interfaces.person import (
     validate_person,
     validate_public_person,
@@ -878,6 +882,9 @@
                     person.displayname))
         store = Store.of(subscription)
         store.remove(subscription)
+        artifact = getUtility(IAccessArtifactSource).find([self])
+        getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+            artifact, [person])
         store.flush()
 
     def getBranchRevision(self, sequence=None, revision=None,

=== modified file 'lib/lp/code/model/tests/test_branchsubscription.py'
--- lib/lp/code/model/tests/test_branchsubscription.py	2012-06-06 05:58:44 +0000
+++ lib/lp/code/model/tests/test_branchsubscription.py	2012-06-27 22:34:23 +0000
@@ -95,6 +95,20 @@
                 None, CodeReviewNotificationLevel.NOEMAIL, owner)
             self.assertTrue(branch.visibleByUser(subscribee))
 
+    def test_unsubscribe_gives_access(self):
+        """Unsubscibing a user to a branch removes their access."""
+        owner = self.factory.makePerson()
+        branch = self.factory.makeBranch(
+            information_type=InformationType.USERDATA, owner=owner)
+        subscribee = self.factory.makePerson()
+        with person_logged_in(owner):
+            branch.subscribe(
+                subscribee, BranchSubscriptionNotificationLevel.NOEMAIL,
+                None, CodeReviewNotificationLevel.NOEMAIL, owner)
+            self.assertTrue(branch.visibleByUser(subscribee))
+            branch.unsubscribe(subscribee, owner)
+            self.assertFalse(branch.visibleByUser(subscribee))
+
 
 class TestBranchSubscriptionCanBeUnsubscribedbyUser(TestCaseWithFactory):
     """Tests for BranchSubscription.canBeUnsubscribedByUser."""