launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/product-specifications-privacy into lp:launchpad with lp:~abentley/launchpad/product-specifications-storm as a prerequisite.

Commit message:
Enable privacy for Product.specifications.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1058411 in Launchpad itself: "private blueprints break product pages with specification listings"
  https://bugs.launchpad.net/launchpad/+bug/1058411

For more details, see:
https://code.launchpad.net/~abentley/launchpad/product-specifications-privacy/+merge/130003

= Summary =
Fix bug #1058411: private blueprints break product pages with specification listings

== Proposed fix ==
Update Product.specifications to repect privacy using get_specification_privacy_filter

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of Private Projects.

== Implementation details ==
None

== Tests ==
bin/test -t TestPrivacy.test_product_specs -t TestProductView.test_index_proprietary_specification -t test_product.TestSpecifications


== Demo and Q/A ==
Add a proprietary specification to a project.  It should be listed in the portlet and in +specs.  Using the +sharing page, remove your ability to see proprietary information.  It should be gone from the portlet and +specs, but you should be able to see everything else.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/testing/factory.py
  lib/lp/blueprints/model/specification.py
  lib/lp/registry/browser/tests/test_product.py
  lib/lp/blueprints/model/sprint.py
  lib/lp/registry/model/product.py
  lib/lp/registry/model/person.py
  lib/lp/registry/tests/test_person.py
  lib/lp/blueprints/browser/tests/test_specificationtarget.py
  lib/lp/registry/tests/test_product.py
-- 
https://code.launchpad.net/~abentley/launchpad/product-specifications-privacy/+merge/130003
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/product-specifications-privacy into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py'
--- lib/lp/blueprints/browser/tests/test_specificationtarget.py	2012-06-04 16:13:51 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationtarget.py	2012-10-16 22:37:30 +0000
@@ -9,7 +9,10 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-from lp.app.enums import ServiceUsage
+from lp.app.enums import (
+    InformationType,
+    ServiceUsage,
+    )
 from lp.blueprints.browser.specificationtarget import HasSpecificationsView
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.blueprints.interfaces.specificationtarget import (
@@ -18,7 +21,9 @@
     )
 from lp.blueprints.publisher import BlueprintsLayer
 from lp.testing import (
+    BrowserTestCase,
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -315,3 +320,26 @@
         self.assertIn(picker_vocab, text)
         focus_script = "setFocusByName('field.search_text')"
         self.assertIn(focus_script, text)
+
+
+class TestPrivacy(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_product_specs(self):
+        # Proprietary specs are only listed for users who can see them.
+        # Other users see the page, but not the private specs.
+        proprietary = self.factory.makeSpecification(
+            information_type=InformationType.PROPRIETARY)
+        product = proprietary.product
+        public = self.factory.makeSpecification(product=product)
+        with person_logged_in(product.owner):
+            product.blueprints_usage = ServiceUsage.LAUNCHPAD
+            browser = self.getViewBrowser(product, '+specs')
+        self.assertIn(public.name, browser.contents)
+        self.assertNotIn(proprietary.name, browser.contents)
+        with person_logged_in(None):
+            browser = self.getViewBrowser(product, '+specs',
+                                          user=product.owner)
+        self.assertIn(public.name, browser.contents)
+        self.assertIn(proprietary.name, browser.contents)

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2012-10-16 00:57:45 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2012-10-16 22:37:30 +0000
@@ -296,7 +296,7 @@
                 InformationType.PROPRIETARY, product.information_type)
 
 
-class TestProductView(TestCaseWithFactory):
+class TestProductView(BrowserTestCase):
     """Tests the ProductView."""
 
     layer = DatabaseFunctionalLayer
@@ -431,6 +431,24 @@
             team_membership_policy_data,
             cache.objects['team_membership_policy_data'])
 
+    def test_index_proprietary_specification(self):
+        # Ordinary users can see page, but proprietary specs are only listed
+        # for users with access to them.
+        proprietary = self.factory.makeSpecification(
+            information_type=InformationType.PROPRIETARY)
+        product = proprietary.product
+        with person_logged_in(product.owner):
+            product.blueprints_usage = ServiceUsage.LAUNCHPAD
+            public = self.factory.makeSpecification(product=product)
+            browser = self.getViewBrowser(product, '+index')
+        self.assertIn(public.name, browser.contents)
+        self.assertNotIn(proprietary.name, browser.contents)
+        with person_logged_in(None):
+            browser = self.getViewBrowser(product, '+index',
+                                          user=product.owner)
+        self.assertIn(public.name, browser.contents)
+        self.assertIn(proprietary.name, browser.contents)
+
 
 class TestProductEditView(BrowserTestCase):
     """Tests for the ProductEditView"""

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-10-16 22:37:27 +0000
+++ lib/lp/registry/model/product.py	2012-10-16 22:37:30 +0000
@@ -96,6 +96,7 @@
     )
 from lp.blueprints.model.specification import (
     get_specification_filters,
+    get_specification_privacy_filter,
     HasSpecificationsMixin,
     Specification,
     SPECIFICATION_POLICY_ALLOWED_TYPES,
@@ -1355,7 +1356,8 @@
         #  - completeness.
         #  - informational.
         #
-        clauses = [Specification.product == self]
+        clauses = [Specification.product == self,
+                   get_specification_privacy_filter(user)]
         clauses.extend(get_specification_filters(filter))
         results = Store.of(self).find(Specification, *clauses)
         results.order_by(order)

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-10-16 22:37:27 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-16 22:37:30 +0000
@@ -1758,6 +1758,34 @@
         result = list_result(product, ['def'])
         self.assertEqual([blueprint2], result)
 
+    def test_proprietary_not_listed(self):
+        # Proprietary blueprints are not listed for random users
+        blueprint1 = self.makeSpec(
+            information_type=InformationType.PROPRIETARY)
+        self.assertEqual([], list_result(blueprint1.product))
+
+    def test_proprietary_listed_for_artifact_grant(self):
+        # Proprietary blueprints are listed for users with an artifact grant.
+        blueprint1 = self.makeSpec(
+            information_type=InformationType.PROPRIETARY)
+        grant = self.factory.makeAccessArtifactGrant(
+            concrete_artifact=blueprint1)
+        self.assertEqual(
+            [blueprint1],
+            list_result(blueprint1.product, user=grant.grantee))
+
+    def test_proprietary_listed_for_policy_grant(self):
+        # Proprietary blueprints are listed for users with a policy grant.
+        blueprint1 = self.makeSpec(
+            information_type=InformationType.PROPRIETARY)
+        policy_source = getUtility(IAccessPolicySource)
+        (policy,) = policy_source.find(
+            [(blueprint1.product, InformationType.PROPRIETARY)])
+        grant = self.factory.makeAccessPolicyGrant(policy)
+        self.assertEqual(
+            [blueprint1],
+            list_result(blueprint1.product, user=grant.grantee))
+
 
 class TestWebService(WebServiceTestCase):