launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/person-getBranchVisibilityInfo-api into lp:launchpad with lp:~wallyworld/launchpad/chained-personpicker-validators as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/person-getBranchVisibilityInfo-api/+merge/90069

Part 3 of the work to improve nominating a mp reviewer.

== Implementation ==

This branch adds a new exported API to IPerson:
- getBranchVisibilityInfo

You pass in a list of unique branch names and it returns a dict containing the person displaname and the branch names which are invisible to the person. This API is used by an XHR call when nominating a reviewer so that the user can be warned that previously invisible branches will be made visible.

== Tests ==

Add tests to test_person and also test_person_webservice

== Lint==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/tests/test_person_webservice.py
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/model/person.py
  lib/lp/registry/tests/test_person.py



-- 
https://code.launchpad.net/~wallyworld/launchpad/person-getBranchVisibilityInfo-api/+merge/90069
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/person-getBranchVisibilityInfo-api into lp:launchpad.

=== modified file 'lib/lp/registry/browser/tests/test_person_webservice.py'
--- lib/lp/registry/browser/tests/test_person_webservice.py	2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_person_webservice.py	2012-01-25 10:57:44 +0000
@@ -74,3 +74,26 @@
         self.assertEquals(
             rendered_comment,
             '<a href="/~test-person" class="sprite person">Test Person</a>')
+
+
+class TestPersonWebService(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_getBranchVisibilityInfo(self):
+        """Test the test_getBranchVisibilityInfo API."""
+        self.factory.makePerson(name='fred')
+        owner = self.factory.makePerson()
+        visible_branch = self.factory.makeBranch()
+        invisible_branch = self.factory.makeBranch(owner=owner, private=True)
+        invisible_name = removeSecurityProxy(invisible_branch).unique_name
+        branches = [
+            visible_branch.unique_name,
+            invisible_name]
+        endInteraction()
+
+        lp = launchpadlib_for("test", person=owner)
+        person = lp.people['fred']
+        info = person.getBranchVisibilityInfo(branch_names=branches)
+        self.assertEqual('Fred', info['person_name'])
+        self.assertEqual([invisible_name], info['invisible_branches'])

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2012-01-20 06:29:10 +0000
+++ lib/lp/registry/interfaces/person.py	2012-01-25 10:57:44 +0000
@@ -1632,6 +1632,25 @@
         If no orderby is provided, Person.sortingColumns is used.
         """
 
+    @operation_parameters(
+        branch_names=List(value_type=Text(),
+            title=_('List of branch unique names'), required=True))
+    @export_read_operation()
+    @operation_for_version("devel")
+    def getBranchVisibilityInfo(branch_names):
+        """Can this person see the specified named branches?
+
+        :param branch_names: The unique names of the branches to check.
+
+        Return a dict with the following values:
+        person_name: the displayname of this person.
+        invisible_branches: a list of the unique names of the branches
+        the person cannot see.
+
+        This API call is provided for use by the client Javascript where the
+        calling context only has a person web_link and unique branch names.
+        """
+
 
 class IPersonEditRestricted(Interface):
     """IPerson attributes that require launchpad.Edit permission."""

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-01-20 06:29:10 +0000
+++ lib/lp/registry/model/person.py	2012-01-25 10:57:44 +0000
@@ -137,6 +137,7 @@
 from lp.bugs.model.bugtask import get_related_bugtasks_search_params
 from lp.bugs.model.structuralsubscription import StructuralSubscription
 from lp.code.interfaces.branchcollection import IBranchCollection
+from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.model.hasbranches import (
     HasBranchesMixin,
     HasMergeProposalsMixin,
@@ -1750,6 +1751,18 @@
                     "The team subscription policy cannot be %s because one "
                     "or more if its member teams are Open." % policy)
 
+    def getBranchVisibilityInfo(self, branch_names):
+        """See `IPerson`."""
+        branch_set = getUtility(IBranchLookup)
+        invisible_branches = []
+        for name in branch_names:
+            branch = branch_set.getByUniqueName(name)
+            if branch is not None and not branch.visibleByUser(self):
+                invisible_branches.append(branch.unique_name)
+        return {
+            'person_name': self.displayname,
+            'invisible_branches': invisible_branches}
+
     @property
     def wiki_names(self):
         """See `IPerson`."""

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2012-01-19 10:09:32 +0000
+++ lib/lp/registry/tests/test_person.py	2012-01-25 10:57:44 +0000
@@ -536,6 +536,30 @@
         self.assertTrue(contact.isAnySecurityContact())
         self.assertFalse(person.isAnySecurityContact())
 
+    def test_getBranchVisibilityInfo_empty_branch_names(self):
+        """Test the test_getBranchVisibilityInfo API with no branch names."""
+        person = self.factory.makePerson(name='fred')
+        info = person.getBranchVisibilityInfo(branch_names=[])
+        self.assertEqual('Fred', info['person_name'])
+        self.assertEqual([], info['invisible_branches'])
+
+    def test_getBranchVisibilityInfo(self):
+        """Test the test_getBranchVisibilityInfo API."""
+        person = self.factory.makePerson(name='fred')
+        owner = self.factory.makePerson()
+        visible_branch = self.factory.makeBranch()
+        invisible_branch = self.factory.makeBranch(owner=owner, private=True)
+        invisible_name = removeSecurityProxy(invisible_branch).unique_name
+        branches = [
+            visible_branch.unique_name,
+            invisible_name,
+            'invalid_branch_name']
+
+        login_person(owner)
+        info = person.getBranchVisibilityInfo(branch_names=branches)
+        self.assertEqual('Fred', info['person_name'])
+        self.assertEqual([invisible_name], info['invisible_branches'])
+
 
 class TestPersonStates(TestCaseWithFactory):