launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/bp_default_1062207 into lp:launchpad.

Commit message:
Add initial value date for the information_type and default to PUBLIC vs None

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1062207 in Launchpad itself: "Blueprint information type has no default expressed"
  https://bugs.launchpad.net/launchpad/+bug/1062207

For more details, see:
https://code.launchpad.net/~rharding/launchpad/bp_default_1062207/+merge/129001

= Summary =

The information type input widget was not getting a default value so when you
submitted the form you had an error that the field was not set.

This branch forces a default initial value.

== Implementation Notes ==

If there was no initial value from the context, the code set information type
to None. This caused the input to not have a set value on render. This changes
that None value to PUBLIC since it's the usual default.

It adds a test for all of the test cases extending the base that checks this
default value is set. It also overrides for the one test case where PUBLIC is
not a valid information type.

There's a drive by lint and setting of this default on the product
registration as well.

Note: I've filed bug #1065161 about the issue that the UX issues presented by
the choice edit widget that complicated this issue.


== Q/A ==

Per the bug, going to the +addspec page and submitting a new spec should work
without changing the information type field at all. It should get a default
value of PUBLIC.

== Tests ==

browser/tests/test_specification.py

-- 
https://code.launchpad.net/~rharding/launchpad/bp_default_1062207/+merge/129001
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/bp_default_1062207 into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2012-09-28 19:59:35 +0000
+++ lib/lp/blueprints/browser/specification.py	2012-10-10 17:43:21 +0000
@@ -102,7 +102,10 @@
     DateTimeFormatterAPI,
     format_link,
     )
-from lp.app.enums import PUBLIC_PROPRIETARY_INFORMATION_TYPES
+from lp.app.enums import (
+    InformationType,
+    PUBLIC_PROPRIETARY_INFORMATION_TYPES,
+    )
 from lp.app.utilities import json_dump_information_types
 from lp.app.vocabularies import InformationTypeVocabulary
 from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
@@ -309,7 +312,7 @@
     @property
     def initial_values(self):
         """Set initial values to honor sharing policy default value."""
-        information_type = None
+        information_type = InformationType.PUBLIC
         if (IProduct.providedBy(self.context) or
             IProductSeries.providedBy(self.context)):
             information_type = (

=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py	2012-10-08 10:07:11 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-10-10 17:43:21 +0000
@@ -352,6 +352,14 @@
         self.assertIsNot(None, info_data)
         self.assertEqual(self.expected_keys, set(info_data.keys()))
 
+    def test_default_info_type(self):
+        # The default selected information type needs to be PUBLIC for new
+        # specifications.
+        view = self.createInitializedView()
+        self.assertEqual(
+            InformationType.PUBLIC,
+            view.initial_values['information_type'])
+
 
 class TestNewSpecificationFromRootView(TestCaseWithFactory,
                                        NewSpecificationTests):
@@ -396,6 +404,14 @@
             specification_sharing_policy=policy)
         return create_initialized_view(product, '+addspec')
 
+    def test_default_info_type(self):
+        # In this case the default info type cannot be PUBlIC as it's not
+        # among the allowed types.
+        view = self.createInitializedView()
+        self.assertEqual(
+            InformationType.EMBARGOED,
+            view.initial_values['information_type'])
+
 
 class TestNewSpecificationFromDistributionView(TestCaseWithFactory,
                                                NewSpecificationTests):
@@ -619,7 +635,6 @@
         self.assertEqual(spec.information_type, InformationType.EMBARGOED)
 
 
-
 class TestNewSpecificationDefaultInformationTypeProduct(
     BrowserTestCase, BaseNewSpecificationInformationTypeDefaultMixin):
 

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2012-10-08 11:56:56 +0000
+++ lib/lp/registry/browser/product.py	2012-10-10 17:43:21 +0000
@@ -2008,7 +2008,7 @@
         return {
             'driver': self.user.name,
             'bug_supervisor': self.user.name,
-            'owner': self.user.name,
+            'information_type': InformationType.PUBLIC,
         }
 
     def setUpFields(self):