launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-subscription-traversal into lp:launchpad.

Commit message:
Add GitRepository:+subscription traversal.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1503749 in Launchpad itself: "Can not remove subscribers from git repos"
  https://bugs.launchpad.net/launchpad/+bug/1503749

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-subscription-traversal/+merge/273720

Add GitRepository:+subscription traversal.  This was an oversight in https://code.launchpad.net/~cjwatson/launchpad/git-subscriptions-browser/+merge/256900, and fixing it makes it possible to unsubscribe a person from a repository using the web UI (the API already works fine for this).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-subscription-traversal into lp:launchpad.

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2015-09-24 09:58:26 +0000
+++ lib/lp/code/browser/gitrepository.py	2015-10-07 16:12:34 +0000
@@ -26,6 +26,7 @@
     copy_field,
     use_template,
     )
+from zope.component import getUtility
 from zope.event import notify
 from zope.formlib import form
 from zope.interface import (
@@ -64,7 +65,10 @@
 from lp.code.interfaces.gitnamespace import get_git_namespace
 from lp.code.interfaces.gitref import IGitRefBatchNavigator
 from lp.code.interfaces.gitrepository import IGitRepository
-from lp.registry.interfaces.person import IPerson
+from lp.registry.interfaces.person import (
+    IPerson,
+    IPersonSet,
+    )
 from lp.registry.vocabularies import UserTeamsParticipationPlusSelfVocabulary
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
@@ -149,6 +153,14 @@
                 return ref
         raise NotFoundError
 
+    @stepthrough("+subscription")
+    def traverse_subscription(self, name):
+        """Traverses to an `IGitSubcription`."""
+        person = getUtility(IPersonSet).getByName(name)
+
+        if person is not None:
+            return self.context.getSubscription(person)
+
     @stepthrough("+merge")
     def traverse_merge_proposal(self, id):
         """Traverse to an `IBranchMergeProposal`."""

=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2015-09-18 15:41:08 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2015-10-07 16:12:34 +0000
@@ -27,6 +27,7 @@
 from lp.code.interfaces.githosting import IGitHostingClient
 from lp.code.interfaces.revision import IRevisionSet
 from lp.registry.enums import BranchSharingPolicy
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.person import PersonVisibility
 from lp.services.database.constants import UTC_NOW
 from lp.services.webapp.publisher import canonical_url
@@ -133,6 +134,7 @@
 
     A repository may be associated with a private team as follows:
     - the owner is a private team
+    - a subscriber is a private team
 
     A logged in user who is not authorised to see the private team(s) still
     needs to be able to view the repository.  The private team will be
@@ -176,6 +178,85 @@
         browser = self._getBrowser()
         self.assertRaises(NotFound, browser.open, url)
 
+    def test_view_repository_with_private_subscriber(self):
+        # A repository with a private subscriber is rendered.
+        private_subscriber = self.factory.makeTeam(
+            name="privateteam", visibility=PersonVisibility.PRIVATE)
+        repository = self.factory.makeGitRepository()
+        with person_logged_in(repository.owner):
+            self.factory.makeGitSubscription(
+                repository, private_subscriber, repository.owner)
+        # Ensure the repository subscriber is rendered.
+        url = canonical_url(repository, rootsite='code')
+        user = self.factory.makePerson()
+        browser = self._getBrowser(user)
+        browser.open(url)
+        soup = BeautifulSoup(browser.contents)
+        self.assertIsNotNone(
+            soup.find('div', attrs={'id': 'subscriber-privateteam'}))
+
+    def test_anonymous_view_repository_with_private_subscriber(self):
+        # Private repository subscribers are not rendered for anon users.
+        private_subscriber = self.factory.makeTeam(
+            name="privateteam", visibility=PersonVisibility.PRIVATE)
+        repository = self.factory.makeGitRepository()
+        with person_logged_in(private_subscriber):
+            self.factory.makeGitSubscription(
+                repository, private_subscriber, repository.owner)
+        # Viewing the repository doesn't show the private subscriber.
+        url = canonical_url(repository, rootsite='code')
+        browser = self._getBrowser()
+        browser.open(url)
+        soup = BeautifulSoup(browser.contents)
+        self.assertIsNone(
+            soup.find('div', attrs={'id': 'subscriber-privateteam'}))
+
+    def test_unsubscribe_private_repository(self):
+        # Unsubscribing from a repository with a policy grant still allows
+        # the repository to be seen.
+        project = self.factory.makeProduct()
+        owner = self.factory.makePerson()
+        subscriber = self.factory.makePerson()
+        [ap] = getUtility(IAccessPolicySource).find(
+            [(project, InformationType.USERDATA)])
+        self.factory.makeAccessPolicyGrant(
+            policy=ap, grantee=subscriber, grantor=project.owner)
+        repository = self.factory.makeGitRepository(
+            target=project, owner=owner, name=u"repo",
+            information_type=InformationType.USERDATA)
+        with person_logged_in(owner):
+            self.factory.makeGitSubscription(repository, subscriber, owner)
+            base_url = canonical_url(repository, rootsite='code')
+            expected_title = '%s : Git : Code : %s' % (
+                repository.identity, project.displayname)
+        url = '%s/+subscription/%s' % (base_url, subscriber.name)
+        browser = self._getBrowser(user=subscriber)
+        browser.open(url)
+        browser.getControl('Unsubscribe').click()
+        self.assertEqual(base_url, browser.url)
+        self.assertEqual(expected_title, browser.title)
+
+    def test_unsubscribe_private_repository_no_access(self):
+        # Unsubscribing from a repository with no access will redirect to
+        # the context of the repository.
+        project = self.factory.makeProduct()
+        owner = self.factory.makePerson()
+        subscriber = self.factory.makePerson()
+        repository = self.factory.makeGitRepository(
+            target=project, owner=owner,
+            information_type=InformationType.USERDATA)
+        with person_logged_in(owner):
+            self.factory.makeGitSubscription(repository, subscriber, owner)
+            base_url = canonical_url(repository, rootsite='code')
+            project_url = canonical_url(project, rootsite='code')
+        url = '%s/+subscription/%s' % (base_url, subscriber.name)
+        expected_title = "Code : %s" % project.displayname
+        browser = self._getBrowser(user=subscriber)
+        browser.open(url)
+        browser.getControl('Unsubscribe').click()
+        self.assertEqual(project_url, browser.url)
+        self.assertEqual(expected_title, browser.title)
+
 
 class TestGitRepositoryBranches(BrowserTestCase):
     """Test the listing of branches in a Git repository."""