launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/private-blueprint-dependencies into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1081773 in Launchpad itself: "Unauthorized on public blueprint which is a dependancy to an embargoed one"
  https://bugs.launchpad.net/launchpad/+bug/1081773

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/private-blueprint-dependencies/+merge/136756

Summary
=======
The dependencies between blueprints can lead to a mix of public and non public
blueprints that are dependent on each other. To make this safe, we need to be
able to redact the private portion of the dependencies on public blueprint
pages for users who can't see them. Additionally, as a sensible reduction in
complexity, we will not allow public blueprints to depend on private ones.

This branch covers that latter portion; a subsequent branch will handle the
redaction.

Preimp
======
Spoke with Orange squad.

Implementation
==============
An additional check is made in the form validation in +linkdependency,
preventing setting a private blueprint as a dependency for a public one.

Tests
=====
bin/test -vvct test_add_forbids_private_as_dependency_for_public

QA
==
Attempt to set a private blueprint as a dependency for a public one. You will
receive an error message informing you it's not allowed.

LoC
===
Part of private projects.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/blueprints/browser/tests/test_specificationdependency.py
  lib/lp/blueprints/browser/specificationdependency.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/private-blueprint-dependencies/+merge/136756
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/private-blueprint-dependencies into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/specificationdependency.py'
--- lib/lp/blueprints/browser/specificationdependency.py	2012-01-01 02:58:52 +0000
+++ lib/lp/blueprints/browser/specificationdependency.py	2012-11-28 18:47:24 +0000
@@ -15,6 +15,7 @@
 from zope.interface import Interface
 
 from lp import _
+from lp.app.enums import InformationType
 from lp.app.browser.launchpadform import (
     action,
     LaunchpadFormView,
@@ -53,7 +54,11 @@
         Because it's too hard to set a good error message from inside the
         widget -- it will be the infamously inscrutable 'Invalid Value' -- we
         replace it here.
+
+        Also forbid setting a private blueprint as a dependency for a public.
         """
+        dependency = data.get('dependency')
+        # Check for a bad error message
         if self.getFieldError('dependency'):
             token = self.request.form.get(self.widgets['dependency'].name)
             self.setFieldError(
@@ -61,6 +66,11 @@
                 'There is no blueprint named "%s" in %s, or '
                 '%s isn\'t valid dependency of that blueprint.' %
                 (token, self.context.target.name, self.context.name))
+        # Check for private set as dependency for public
+        elif (self.context.information_type == InformationType.PUBLIC and
+            dependency.information_type != InformationType.PUBLIC):
+            error = "A public blueprint may not depend on a private one."
+            self.setFieldError('dependency', error)
 
     @action(_('Continue'), name='linkdependency')
     def linkdependency_action(self, action, data):

=== modified file 'lib/lp/blueprints/browser/tests/test_specificationdependency.py'
--- lib/lp/blueprints/browser/tests/test_specificationdependency.py	2012-08-20 16:38:10 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationdependency.py	2012-11-28 18:47:24 +0000
@@ -8,17 +8,40 @@
 
 __metaclass__ = type
 
+from lp.app.enums import InformationType
+from lp.registry.enums import SpecificationSharingPolicy
 from lp.services.webapp import canonical_url
 from lp.testing import (
     BrowserTestCase,
     person_logged_in,
     )
+from lp.testing.views import create_initialized_view
 from lp.testing.layers import DatabaseFunctionalLayer
 
 
 class TestAddDependency(BrowserTestCase):
+
     layer = DatabaseFunctionalLayer
 
+    def test_add_forbids_private_as_dependency_for_public(self):
+        policy = SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY
+        product = self.factory.makeProduct(
+            specification_sharing_policy=policy, owner=self.user)
+        spec = self.factory.makeSpecification(
+            product=product, owner=self.user,
+            information_type=InformationType.PUBLIC)
+        self.factory.makeSpecification(
+            product=product, owner=self.user,
+            information_type=InformationType.PROPRIETARY, name='private')
+        form = {
+            'field.dependency': 'private',
+            'field.actions.linkdependency': 'Continue',
+            }
+        view = create_initialized_view(
+            spec, name="+linkdependency", form=form)
+        errors = [u'A public blueprint may not depend on a private one.']
+        self.assertEqual(errors, view.errors)
+
     def test_add_dependency_by_url(self):
         # It is possible to use the URL of a specification in the "Depends On"
         # field of the form to add a dependency to a spec.