launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/blueprint-info-type-change into lp:launchpad with lp:~abentley/launchpad/blueprint-info-type-ui as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~abentley/launchpad/blueprint-info-type-change/+merge/122753

= Summary =
Support setting Specification.information_type via the browser

== Pre-implementation notes ==
Discussed with deryck

== LOC Rationale ==
Part of Private Projects

== Implementation details ==
This operation currently returns '', but will eventually return subscription information, as bugs do.  Modifying a Specification's information_type but returning subscriber information is page-specific functionality and makes no particular sense in an API, so it is provided via the appservers, rather than the API.

The tests use create_initialized_view because this permits setting the HTTP_X_REQUESTED_WITH header, which allows for standard error handling.

== Tests ==
bin/test test_specification

== Demo and Q/A ==
Set the 'blueprints.information_type.enabled' feature flag to "on".

Change the information_type of a blueprint.  It should succeed.  Reload the page.  The new information_type should be retained.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/blueprints/templates/specification-index.pt
  lib/lp/app/javascript/information_type.js
  lib/lp/blueprints/templates/blueprint-portlet-privacy.pt
  lib/lp/testing/factory.py
  lib/lp/app/javascript/tests/test_information_type.js
  lib/lp/testing/__init__.py
  lib/lp/blueprints/model/tests/test_specification.py
  lib/lp/blueprints/tests/test_specification.py
  lib/lp/blueprints/model/specification.py
  lib/lp/bugs/javascript/bugtask_index.js
  lib/lp/services/features/flags.py
  lib/lp/blueprints/browser/configure.zcml
  lib/lp/blueprints/browser/specification.py
  lib/lp/blueprints/browser/tests/test_specification.py
  lib/lp/app/javascript/tests/test_information_type.html
  lib/lp/blueprints/templates/bug-secrecy.pt
  lib/lp/blueprints/interfaces/specification.py
-- 
https://code.launchpad.net/~abentley/launchpad/blueprint-info-type-change/+merge/122753
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/blueprint-info-type-change into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/configure.zcml'
--- lib/lp/blueprints/browser/configure.zcml	2012-09-04 20:39:25 +0000
+++ lib/lp/blueprints/browser/configure.zcml	2012-09-04 20:39:26 +0000
@@ -276,6 +276,12 @@
                 template="../templates/specification-portlet-blocked.pt"/>
         </browser:pages>
         <browser:page
+            name="+secrecy"
+            for="lp.blueprints.interfaces.specification.ISpecification"
+            class="lp.blueprints.browser.specification.SpecificationInformationTypeEditView"
+            permission="launchpad.Edit"
+            template="../templates/bug-secrecy.pt"/>
+        <browser:page
             for="lp.blueprints.interfaces.specification.ISpecification"
             name="+portlet-subscribers"
             class="lp.blueprints.browser.specificationsubscription.SpecificationPortletSubcribersContents"

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2012-09-04 20:39:25 +0000
+++ lib/lp/blueprints/browser/specification.py	2012-09-04 20:39:26 +0000
@@ -47,7 +47,10 @@
 
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
-from lazr.restful.interface import use_template
+from lazr.restful.interface import (
+    copy_field,
+    use_template,
+    )
 from lazr.restful.interfaces import (
     IFieldHTMLRenderer,
     IWebServiceClientRequest,
@@ -98,6 +101,7 @@
     DateTimeFormatterAPI,
     format_link,
     )
+from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
 from lp.blueprints.browser.specificationtarget import HasSpecificationsView
 from lp.blueprints.enums import (
     NewSpecificationDefinitionStatus,
@@ -115,6 +119,7 @@
 from lp.registry.enums import PRIVATE_INFORMATION_TYPES
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.product import IProduct
+from lp.registry.vocabularies import InformationTypeVocabulary
 from lp.services.config import config
 from lp.services.fields import WorkItemsText
 from lp.services.propertycache import cachedproperty
@@ -768,6 +773,52 @@
     field_names = ['definition_status', 'implementation_status', 'whiteboard']
 
 
+class SpecificationInformationTypeEditView(LaunchpadFormView):
+    """Form for marking a bug as a private/public."""
+
+    @property
+    def label(self):
+        return 'Set information type'
+
+    page_title = label
+
+    field_names = ['information_type']
+
+    custom_widget('information_type', LaunchpadRadioWidgetWithDescription)
+
+    @property
+    def schema(self):
+        """Schema for editing the information type of a `IBug`."""
+        info_types = self.context.getAllowedInformationTypes(self.user)
+
+        class information_type_schema(Interface):
+            information_type_field = copy_field(
+                ISpecification['information_type'], readonly=False,
+                vocabulary=InformationTypeVocabulary(types=info_types))
+        return information_type_schema
+
+    @property
+    def next_url(self):
+        """Return the next URL to call when this call completes."""
+        return None
+
+    cancel_url = next_url
+
+    @property
+    def initial_values(self):
+        """See `LaunchpadFormView.`"""
+        return {'information_type': self.context.information_type}
+
+    @action('Change', name='change',
+        failure=LaunchpadFormView.ajax_failure_handler)
+    def change_action(self, action, data):
+        """Update the bug."""
+        data = dict(data)
+        information_type = data.pop('information_type')
+        self.context.transitionToInformationType(information_type, self.user)
+        return ''
+
+
 class SpecificationEditMilestoneView(SpecificationEditView):
     label = 'Target to a milestone'
     field_names = ['milestone', 'whiteboard']

=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py	2012-09-04 20:39:25 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-09-04 20:39:26 +0000
@@ -4,6 +4,7 @@
 __metaclass__ = type
 
 from datetime import datetime
+import json
 import unittest
 
 from BeautifulSoup import BeautifulSoup
@@ -12,8 +13,11 @@
     Equals,
     Not,
     )
+from testtools.testcase import ExpectedException
+import transaction
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 import soupmatchers
 
@@ -206,6 +210,49 @@
         self.assertThat(browser.contents,
                         soupmatchers.HTMLContains(privacy_banner))
 
+    def set_secrecy(self, spec, owner, information_type='PROPRIETARY'):
+        form = {
+            'field.actions.change': 'Change',
+            'field.information_type': information_type,
+            'field.validate_change': 'off',
+        }
+        with person_logged_in(owner):
+            view = create_initialized_view(
+                spec, '+secrecy', form, principal=owner,
+                HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+            body = view.render()
+        return view.request.response.getStatus(), body
+
+    def test_secrecy_change(self):
+        """Setting the value via '+secrecy' works."""
+        owner = self.factory.makePerson()
+        spec = self.factory.makeSpecification(owner=owner)
+        self.set_secrecy(spec, owner)
+        with person_logged_in(owner):
+            self.assertEqual(InformationType.PROPRIETARY,
+                             spec.information_type)
+
+    def test_secrecy_change_nonsense(self):
+        """Invalid values produce sane errors."""
+        owner = self.factory.makePerson()
+        spec = self.factory.makeSpecification(owner=owner)
+        transaction.commit()
+        status, body = self.set_secrecy(
+            spec, owner, information_type=self.factory.getUniqueString())
+        self.assertEqual(400, status)
+        error_data = json.loads(body)
+        self.assertEqual({u'field.information_type': u'Invalid value'},
+                         error_data['errors'])
+        self.assertEqual(InformationType.PUBLIC, spec.information_type)
+
+    def test_secrecy_change_unprivileged(self):
+        """Unprivileged users cannot change information_type."""
+        spec = self.factory.makeSpecification()
+        person = self.factory.makePerson()
+        with ExpectedException(Unauthorized):
+            self.set_secrecy(spec, person)
+        self.assertEqual(InformationType.PUBLIC, spec.information_type)
+
 
 class TestSpecificationViewPrivateArtifacts(BrowserTestCase):
     """ Tests that specifications with private team artifacts can be viewed.

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2012-09-04 20:39:25 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2012-09-04 20:39:26 +0000
@@ -627,6 +627,9 @@
         The new target must be an IProduct or IDistribution.
         """
 
+    def transitionToInformationType(information_type, who):
+        """Change the information type of the Specification."""
+
 
 class ISpecification(ISpecificationPublic, ISpecificationView,
                      ISpecificationEditRestricted, IBugLinkTarget):

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2012-09-04 20:39:25 +0000
+++ lib/lp/blueprints/model/specification.py	2012-09-04 20:39:26 +0000
@@ -71,6 +71,7 @@
     PRIVATE_INFORMATION_TYPES,
     PUBLIC_INFORMATION_TYPES,
     )
+from lp.registry.errors import CannotChangeInformationType
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.person import validate_public_person
@@ -818,6 +819,15 @@
     def getAllowedInformationTypes(self, who):
         return set(InformationType.items)
 
+    def transitionToInformationType(self, information_type, who):
+        """See `IBug`."""
+        if self.information_type == information_type:
+            return False
+        if information_type not in self.getAllowedInformationTypes(who):
+            raise CannotChangeInformationType("Forbidden by project policy.")
+        self.information_type = information_type
+        return True
+
     @property
     def private(self):
         return self.information_type in PRIVATE_INFORMATION_TYPES

=== modified file 'lib/lp/blueprints/model/tests/test_specification.py'
--- lib/lp/blueprints/model/tests/test_specification.py	2012-06-18 11:19:46 +0000
+++ lib/lp/blueprints/model/tests/test_specification.py	2012-09-04 20:39:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for blueprints here."""
@@ -11,6 +11,7 @@
     Equals,
     MatchesStructure,
     )
+from testtools.testcase import ExpectedException
 import transaction
 from zope.event import notify
 from zope.interface import providedBy
@@ -22,6 +23,8 @@
     SpecificationWorkItemStatus,
     )
 from lp.blueprints.model.specificationworkitem import SpecificationWorkItem
+from lp.registry.enums import InformationType
+from lp.registry.errors import CannotChangeInformationType
 from lp.registry.model.milestone import Milestone
 from lp.services.mail import stub
 from lp.services.webapp import canonical_url
@@ -29,6 +32,7 @@
     ANONYMOUS,
     login,
     login_person,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -601,3 +605,33 @@
         return dict(
             title=wi.title, status=wi.status, assignee=wi.assignee,
             milestone=wi.milestone, sequence=sequence)
+
+
+class TestSpecificationInformationType(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_transitionToInformationType(self):
+        """Ensure transitionToInformationType works."""
+        spec = self.factory.makeSpecification()
+        self.assertEqual(InformationType.PUBLIC, spec.information_type)
+        with person_logged_in(spec.owner):
+            result = spec.transitionToInformationType(
+                InformationType.EMBARGOED, spec.owner)
+            self.assertEqual(InformationType.EMBARGOED, spec.information_type)
+        self.assertTrue(result)
+
+    def test_transitionToInformationType_no_change(self):
+        """Return False on no change."""
+        spec = self.factory.makeSpecification()
+        with person_logged_in(spec.owner):
+            result = spec.transitionToInformationType(InformationType.PUBLIC,
+                                                      spec.owner)
+        self.assertFalse(result)
+
+    def test_transitionToInformationType_forbidden(self):
+        """Raise if specified type is not supported."""
+        spec = self.factory.makeSpecification()
+        with person_logged_in(spec.owner):
+            with ExpectedException(CannotChangeInformationType, '.*'):
+                spec.transitionToInformationType(None, spec.owner)

=== added file 'lib/lp/blueprints/templates/bug-secrecy.pt'
--- lib/lp/blueprints/templates/bug-secrecy.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/blueprints/templates/bug-secrecy.pt	2012-09-04 20:39:26 +0000
@@ -0,0 +1,16 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml";
+  xmlns:tal="http://xml.zope.org/namespaces/tal";
+  xmlns:metal="http://xml.zope.org/namespaces/metal";
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="malone"
+>
+  <body>
+    <div metal:fill-slot="main">
+      <div class="top-portlet">
+        <div metal:use-macro="context/@@launchpad_form/form" />
+      </div>
+    </div>
+  </body>
+</html>