launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/milestone-spec-privacy into lp:launchpad.

Commit message:
Milestone.getSpecifications respects spec privacy.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1067531 in Launchpad itself: " private blueprints break milestone listings"
  https://bugs.launchpad.net/launchpad/+bug/1067531

For more details, see:
https://code.launchpad.net/~abentley/launchpad/milestone-spec-privacy/+merge/131560

= Summary =
Fix bug #1067531: private blueprints break milestone listings

== Proposed fix ==
Use get_specification_privacy_filter to filter out specifications not visible to the user.

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of private projects

== Implementation details ==
None

== Tests ==
bin/test -t test_private_specifications -t test_getSpecifications_specification_privacy


== Demo and Q/A ==
Create a public project with a "Proprietary" specification sharing policy.  Create a milestone.  Create a blueprint targeted at that milestone.  View the milestone index page.  You should see it listed.

Log in as an unprivileged user.  View the milestone index page.  The specification should not be listed.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/tests/test_milestone.py
  lib/lp/registry/browser/tests/test_milestone.py
  lib/lp/registry/model/milestone.py
-- 
https://code.launchpad.net/~abentley/launchpad/milestone-spec-privacy/+merge/131560
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/milestone-spec-privacy into lp:launchpad.

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2012-10-18 16:56:54 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2012-10-26 09:28:23 +0000
@@ -8,12 +8,12 @@
 import soupmatchers
 from testtools.matchers import LessThan
 from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
+from lp.registry.enums import SpecificationSharingPolicy
 from lp.registry.interfaces.accesspolicy import (
     IAccessPolicyGrantSource,
     IAccessPolicySource,
@@ -101,6 +101,22 @@
             browser.contents, soupmatchers.HTMLContains(
             privacy_portlet_proprietary))
 
+    def test_private_specifications(self):
+        # Only specifications visible to the browser user are listed.
+        owner = self.factory.makePerson()
+        enum = SpecificationSharingPolicy
+        product = self.factory.makeProduct(
+            owner=owner, specification_sharing_policy=enum.PROPRIETARY)
+        milestone = self.factory.makeMilestone(product=product)
+        specification = self.factory.makeSpecification(
+            information_type=InformationType.PROPRIETARY,
+            milestone=milestone)
+        with person_logged_in(None):
+            browser = self.getViewBrowser(milestone, '+index', user=owner)
+        self.assertIn(specification.name, browser.contents)
+        with person_logged_in(None):
+            browser = self.getViewBrowser(milestone, '+index')
+
 
 class TestAddMilestoneViews(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py	2012-10-22 13:55:44 +0000
+++ lib/lp/registry/model/milestone.py	2012-10-26 09:28:23 +0000
@@ -38,7 +38,10 @@
 from zope.interface import implements
 
 from lp.app.errors import NotFoundError
-from lp.blueprints.model.specification import Specification
+from lp.blueprints.model.specification import (
+    get_specification_privacy_filter,
+    Specification,
+    )
 from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugtarget import IHasBugs
@@ -172,7 +175,8 @@
                             SpecificationWorkItem.milestone_id.is_in(
                                 milestones),
                             SpecificationWorkItem.deleted == False)),
-                    all=True)))
+                    all=True)),
+            get_specification_privacy_filter(user))
         results.config(distinct=True)
         ordered_results = results.order_by(Desc(Specification.priority),
                                            Specification.definition_status,

=== modified file 'lib/lp/registry/tests/test_milestone.py'
--- lib/lp/registry/tests/test_milestone.py	2012-10-22 10:09:48 +0000
+++ lib/lp/registry/tests/test_milestone.py	2012-10-26 09:28:23 +0000
@@ -23,6 +23,7 @@
 from lp.registry.enums import (
     BugSharingPolicy,
     SharingPermission,
+    SpecificationSharingPolicy,
     )
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.milestone import (
@@ -504,6 +505,21 @@
         self.assertContentEqual([spec],
                                 milestone.getSpecifications(owner))
 
+    def test_getSpecifications_specification_privacy(self):
+        # Only specifications visible to the specified user are listed.
+        owner = self.factory.makePerson()
+        enum = SpecificationSharingPolicy
+        product = self.factory.makeProduct(
+            owner=owner, specification_sharing_policy=enum.PROPRIETARY)
+        milestone = self.factory.makeMilestone(product=product)
+        specification = self.factory.makeSpecification(
+            information_type=InformationType.PROPRIETARY,
+            milestone=milestone)
+        self.assertIn(
+            specification, list(milestone.getSpecifications(owner)))
+        self.assertNotIn(
+            specification, list(milestone.getSpecifications(None)))
+
     def test_bugtasks_milestone_privacy(self):
         # Ensure bugtasks respects milestone privacy.
         # This looks wrong, because the bugtask is actually public, and we