launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/actually-save-info-type-on-project-create into lp:launchpad.

Commit message:
Ensure the project creation web UI doesn't error when saving, and ensure that it also saves information_type correctly, when the new private projects feature UI is enabled.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1061152 in Launchpad itself: "Project creation needs updating to save information_type"
  https://bugs.launchpad.net/launchpad/+bug/1061152
  Bug #1061155 in Launchpad itself: "Project creation for private projects errors mysteriously"
  https://bugs.launchpad.net/launchpad/+bug/1061155

For more details, see:
https://code.launchpad.net/~deryck/launchpad/actually-save-info-type-on-project-create/+merge/127863

This branch fixes a couple bugs discovered in the new private projects UI for project creation.

The JavaScript needed to be updated to fill in a value for license_info if the "Other/Proprietary" option was selected.  This will prevent the form validation from throwing an error.  I added a test inside the excellent test coverage for this new js code.

Also, the project creation web UI was not saving information type.  This changes the form to save PUBLIC unless the feature flag is set, then we will save the value returned in form data.  I added a couple tests here, too, that ensure that information_type is saved correctly whether the private projects feature flag is set or not/

There are some long lines of lint in the js file, which I'll cleanup before landing.
-- 
https://code.launchpad.net/~deryck/launchpad/actually-save-info-type-on-project-create/+merge/127863
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/actually-save-info-type-on-project-create into lp:launchpad.

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2012-09-28 06:15:58 +0000
+++ lib/lp/registry/browser/product.py	2012-10-03 20:42:22 +0000
@@ -2118,6 +2118,13 @@
             owner = getUtility(ILaunchpadCelebrities).registry_experts
         else:
             owner = data.get('owner')
+
+        information_type = InformationType.PUBLIC
+        private_projects_flag = 'disclosure.private_projects.enabled'
+        private_projects = bool(getFeatureFlag(private_projects_flag))
+        if private_projects:
+            information_type = data.get('information_type')
+
         return getUtility(IProductSet).createProduct(
             registrant=self.user,
             bug_supervisor=data.get('bug_supervisor', None),
@@ -2131,6 +2138,7 @@
             homepageurl=data.get('homepageurl'),
             licenses=data['licenses'],
             license_info=data['license_info'],
+            information_type=information_type,
             project=project)
 
     def link_source_package(self, product, data):

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2012-09-10 13:24:03 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2012-10-03 20:42:22 +0000
@@ -11,7 +11,10 @@
 from zope.schema.vocabulary import SimpleVocabulary
 
 from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
-from lp.app.enums import ServiceUsage
+from lp.app.enums import (
+    InformationType,
+    ServiceUsage,
+    )
 from lp.registry.browser.product import (
     ProjectAddStepOne,
     ProjectAddStepTwo,
@@ -25,6 +28,7 @@
     License,
     )
 from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.publisher import canonical_url
 from lp.testing import (
     BrowserTestCase,
@@ -106,6 +110,7 @@
                 'field.licenses': ['MIT'],
                 'field.license_info': '',
                 'field.disclaim_maintainer': 'off',
+                'field.information_type': 0,
                 }
 
     def test_view_data_model(self):
@@ -211,6 +216,38 @@
         product = self.product_set.getByName('fnord')
         self.assertEqual('registry', product.owner.name)
 
+    def test_information_type_saved_default(self):
+        # information_type should only ever be PUBLIC if the private
+        # projects feature flag is not set.
+        registrant = self.factory.makePerson()
+        login_person(registrant)
+        form = self.makeForm(action=2)
+        form['field.information_type'] = 'PROPRIETARY'
+        form['field.owner'] = registrant.name
+        view = create_initialized_view(self.product_set, '+new', form=form)
+        self.assertEqual(0, len(view.view.errors))
+        product = self.product_set.getByName('fnord')
+        self.assertEqual(InformationType.PUBLIC, product.information_type)
+
+    def test_information_type_saved_private_projects(self):
+        # information_type should only ever be PUBLIC if the private
+        # projects feature flag is not set.
+        with FeatureFixture({u'disclosure.private_projects.enabled': u'on'}):
+            registrant = self.factory.makePerson()
+            login_person(registrant)
+            form = self.makeForm(action=2)
+            form['field.information_type'] = 'PROPRIETARY'
+            form['field.owner'] = registrant.name
+            form['field.driver'] = registrant.name
+            form['field.maintainer'] = registrant.name
+            form['field.bug_supervisor'] = registrant.name
+            view = create_initialized_view(
+                self.product_set, '+new', form=form)
+            self.assertEqual(0, len(view.view.errors))
+            product = self.product_set.getByName('fnord')
+            self.assertEqual(
+                InformationType.PROPRIETARY, product.information_type)
+
 
 class TestProductView(TestCaseWithFactory):
     """Tests the ProductView."""

=== modified file 'lib/lp/registry/javascript/product_views.js'
--- lib/lp/registry/javascript/product_views.js	2012-09-24 14:10:10 +0000
+++ lib/lp/registry/javascript/product_views.js	2012-10-03 20:42:22 +0000
@@ -218,6 +218,8 @@
                 var commercial = 'input[value="' + COMMERCIAL_LICENSE + '"]';
                 Y.one(commercial).set('checked', 'checked');
                 license.set('value', COMMERCIAL_LICENSE);
+                Y.one('textarea[name="field.license_info"]').set(
+                    'value', 'Launchpad 30-day trial commerical license');
             } else {
                 driver_cont.hide();
                 bug_super_cont.hide();

=== modified file 'lib/lp/registry/javascript/tests/test_product_views.js'
--- lib/lp/registry/javascript/tests/test_product_views.js	2012-09-24 14:10:10 +0000
+++ lib/lp/registry/javascript/tests/test_product_views.js	2012-10-03 20:42:22 +0000
@@ -135,6 +135,13 @@
             var new_license = Y.one('input[name="field.licenses"]');
             Y.Assert.areEqual('OTHER_PROPRIETARY', new_license.get('value'),
                               'License is updated to a commercial selection');
+
+            // license_info must also be filled in to ensure we don't
+            // get form validation errors.
+            var license_info = Y.one('textarea[name="field.license_info"]');
+            Y.Assert.areEqual(
+                'Launchpad 30-day trial commerical license',
+                license_info.get('value'));
         }
     }));