launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/export-getSharedArtifacts-1046022 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1046022 in Launchpad itself: "Cannot get the artifacts shared with a person over the API"
  https://bugs.launchpad.net/launchpad/+bug/1046022

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/export-getSharedArtifacts-1046022/+merge/122787

== Implementation ==

Export some more ISharingService methods to the web service:

- getSharedArtifacts
- getVisibleArtifacts

== Tests ==

Add a new test for each exported method.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/interfaces/sharingservice.py
  lib/lp/registry/services/sharingservice.py
  lib/lp/registry/services/tests/test_sharingservice.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/export-getSharedArtifacts-1046022/+merge/122787
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/export-getSharedArtifacts-1046022 into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-09-04 04:27:53 +0000
+++ lib/lp/bugs/model/bug.py	2012-09-05 04:19:19 +0000
@@ -811,7 +811,8 @@
         # there is at least one bugtask for which access can be checked.
         if self.default_bugtask:
             service = getUtility(IService, 'sharing')
-            bugs, ignored = service.getVisibleArtifacts(person, bugs=[self])
+            bugs, ignored = service.getVisibleArtifacts(
+                person, bugs=[self], ignore_permissions=True)
             if not bugs:
                 service.ensureAccessGrants(
                     [person], subscribed_by, bugs=[self],

=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py	2012-09-04 00:10:23 +0000
+++ lib/lp/registry/interfaces/sharingservice.py	2012-09-05 04:19:19 +0000
@@ -62,6 +62,12 @@
         information type.
         """
 
+    @export_read_operation()
+    @call_with(user=REQUEST_USER)
+    @operation_parameters(
+        pillar=Reference(IPillar, title=_('Pillar'), required=True),
+        person=Reference(IPerson, title=_('Person'), required=True))
+    @operation_for_version('devel')
     def getSharedArtifacts(pillar, person, user):
         """Return the artifacts shared between the pillar and person.
 
@@ -75,6 +81,14 @@
         :return: a (bugtasks, branches) tuple
         """
 
+    @export_read_operation()
+    @operation_parameters(
+        person=Reference(IPerson, title=_('Person'), required=True),
+        branches=List(
+            Reference(schema=IBranch), title=_('Branches'), required=False),
+        bugs=List(
+            Reference(schema=IBug), title=_('Bugs'), required=False))
+    @operation_for_version('devel')
     def getVisibleArtifacts(person, branches=None, bugs=None):
         """Return the artifacts shared with person.
 

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-09-04 22:25:36 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-09-05 04:19:19 +0000
@@ -112,6 +112,7 @@
             AccessPolicy.id.is_in(ids)
         )
 
+    @available_with_permission('launchpad.Driver', 'pillar')
     def getSharedArtifacts(self, pillar, person, user):
         """See `ISharingService`."""
         policies = getUtility(IAccessPolicySource).findByPillar([pillar])
@@ -140,13 +141,20 @@
 
         return bugtasks, branches
 
-    def getVisibleArtifacts(self, person, branches=None, bugs=None):
+    def getVisibleArtifacts(self, person, branches=None, bugs=None,
+                            ignore_permissions=False):
         """See `ISharingService`."""
         bugs_by_id = {}
         branches_by_id = {}
         for bug in bugs or []:
+            if (not ignore_permissions
+                and not check_permission('launchpad.View', bug)):
+                raise Unauthorized
             bugs_by_id[bug.id] = bug
         for branch in branches or []:
+            if (not ignore_permissions
+                and not check_permission('launchpad.View', branch)):
+                raise Unauthorized
             branches_by_id[branch.id] = branch
 
         # Load the bugs.

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-09-04 00:02:07 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-09-05 04:19:19 +0000
@@ -1423,7 +1423,9 @@
                         if d['name'] != 'thundercat']
         self.assertEqual('grantee', grantee_data['name'])
         self.assertEqual(
-            {InformationType.USERDATA.name: SharingPermission.ALL.name},
+            {InformationType.USERDATA.name: SharingPermission.ALL.name,
+             InformationType.PRIVATESECURITY.name:
+                 SharingPermission.SOME.name},
             grantee_data['permissions'])
 
 
@@ -1479,6 +1481,19 @@
         super(TestLaunchpadlib, self).setUp()
         self.launchpad = self.factory.makeLaunchpadService(person=self.owner)
         self.service = self.launchpad.load('+services/sharing')
+        self.bug = self.factory.makeBug(
+            owner=self.owner,
+            target=self.pillar,
+            information_type=InformationType.PRIVATESECURITY)
+        self.branch = self.factory.makeBranch(
+            owner=self.owner,
+            product=self.pillar,
+            information_type=InformationType.PRIVATESECURITY)
+        login_person(self.owner)
+        self.bug.subscribe(self.grantee, self.owner)
+        self.branch.subscribe(
+            self.grantee, BranchSubscriptionNotificationLevel.NOEMAIL,
+            None, CodeReviewNotificationLevel.NOEMAIL, self.owner)
         transaction.commit()
         self._sharePillarInformation()
 
@@ -1494,3 +1509,29 @@
             permissions={
                 InformationType.USERDATA.title: SharingPermission.ALL.title}
         )
+
+    def test_getSharedArtifacts(self):
+        # Test the exported getSharedArtifacts() method.
+        ws_pillar = ws_object(self.launchpad, self.pillar)
+        ws_grantee = ws_object(self.launchpad, self.grantee)
+        (bugtasks, branches) = self.service.getSharedArtifacts(
+            pillar=ws_pillar, person=ws_grantee)
+        self.assertEqual(1, len(bugtasks))
+        self.assertEqual(1, len(branches))
+        self.assertEqual(bugtasks[0]['title'], self.bug.default_bugtask.title)
+        self.assertEqual(branches[0]['unique_name'], self.branch.unique_name)
+
+    def test_getVisibleArtifacts(self):
+        # Test the exported getVisibleArtifacts() method.
+        ws_grantee = ws_object(self.launchpad, self.grantee)
+        # Sadly lazr.restful doesn't know how to marshall lists of entities
+        # so we have to use links directly.
+        ws_bug_link = ws_object(self.launchpad, self.bug).self_link
+        ws_branch_link = ws_object(self.launchpad, self.branch).self_link
+        (bugs, branches) = self.service.getVisibleArtifacts(
+            person=ws_grantee,
+            branches=[ws_branch_link], bugs=[ws_bug_link])
+        self.assertEqual(1, len(bugs))
+        self.assertEqual(1, len(branches))
+        self.assertEqual(bugs[0]['title'], self.bug.title)
+        self.assertEqual(branches[0]['unique_name'], self.branch.unique_name)