launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13034
[Merge] lp:~rharding/launchpad/edit_product_info_type into lp:launchpad
Richard Harding has proposed merging lp:~rharding/launchpad/edit_product_info_type into lp:launchpad.
Commit message:
Add the information type choice widget to project edit if the feature flag is enabled.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rharding/launchpad/edit_product_info_type/+merge/128081
= Summary =
There will be mistakes and users need to be able to change the information
type on their project using the ProjectEditView. This branch adds the
information type field and adds the JS to setup the choice widget and dealing
with the license setting in the same way it does for new project registration.
== Implementation Notes ==
In order to get the JS onto the page we needed to make sure we use
project-edit.pt.
The field is added in the same location as when registering a project. It adds
the choice widget.
There's a drive by moving the @property next_url up the class to be with the
other @property definitions.
The field is only available if the feature flag is set so we adjust the field
using setUpFields and make sure to omit the information_type if the feature
flag isn't set.
Large chunks of the JS are moved out into shared functions so that each View
class can share the code that watches the information type for changes and
respond with showing/hiding the license fields and such.
We add a new JS view EditProduct that is figured for the edit page. This only
sets up events if the information type field is set and is safe if the feature
flag is not set. It'll just be inert JS.
_information_type_change is changed to a public method since the new shared
bind_information_type method calls into it directly.
The tests are setup to share setUp, tearDown, and the set of asserts for the
license changes. The product edit view doesn't have bug_supervisor or driver
fields to update.
== Q/A ==
Register a new project with a private information type. Then, once it's
created, "Change details" and select a Public information type.
== Known Issues ==
If the project starts out as public you cannot currently change it to a private type as you don't have a commercial subscription. A follow up branch will add that into place.
== Tests ==
test_product_views.html
--
https://code.launchpad.net/~rharding/launchpad/edit_product_info_type/+merge/128081
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/edit_product_info_type into lp:launchpad.
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2012-08-15 16:05:54 +0000
+++ lib/lp/registry/browser/configure.zcml 2012-10-05 14:06:23 +0000
@@ -1496,7 +1496,7 @@
for="lp.registry.interfaces.product.IProduct"
class="lp.registry.browser.product.ProductEditView"
permission="launchpad.Edit"
- template="../../app/templates/generic-edit.pt"/>
+ template="../templates/project-edit.pt"/>
<browser:page
for="lp.registry.interfaces.product.IProduct"
permission="launchpad.Edit"
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-10-05 13:29:39 +0000
+++ lib/lp/registry/browser/product.py 2012-10-05 14:06:23 +0000
@@ -1412,6 +1412,7 @@
"description",
"project",
"homepageurl",
+ "information_type",
"sourceforgeproject",
"freshmeatproject",
"wikiurl",
@@ -1424,12 +1425,41 @@
]
custom_widget('licenses', LicenseWidget)
custom_widget('license_info', GhostWidget)
+ custom_widget('information_type', LaunchpadRadioWidgetWithDescription)
+
+ @property
+ def next_url(self):
+ """See `LaunchpadFormView`."""
+ if self.context.active:
+ return canonical_url(self.context)
+ else:
+ return canonical_url(getUtility(IProductSet))
+
+ cancel_url = next_url
@property
def page_title(self):
"""The HTML page title."""
return "Change %s's details" % self.context.title
+ def initialize(self):
+ # The JSON cache must be populated before the super call, since
+ # the form is rendered during LaunchpadFormView's initialize()
+ # when an action is invoked.
+ cache = IJSONRequestCache(self.request)
+ json_dump_information_types(cache,
+ PUBLIC_PROPRIETARY_INFORMATION_TYPES)
+ super(ProductEditView, self).initialize()
+
+ def setUpFields(self):
+ """See `LaunchpadFormView`."""
+ super(ProductEditView, self).setUpFields()
+
+ private_projects_flag = 'disclosure.private_projects.enabled'
+ private_projects = bool(getFeatureFlag(private_projects_flag))
+ if not private_projects:
+ self.form_fields = self.form_fields.omit('information_type')
+
def showOptionalMarker(self, field_name):
"""See `LaunchpadFormView`."""
# This has the effect of suppressing the ": (Optional)" stuff for the
@@ -1444,16 +1474,6 @@
def change_action(self, action, data):
self.updateContextFromData(data)
- @property
- def next_url(self):
- """See `LaunchpadFormView`."""
- if self.context.active:
- return canonical_url(self.context)
- else:
- return canonical_url(getUtility(IProductSet))
-
- cancel_url = next_url
-
class ProductValidationMixin:
=== modified file 'lib/lp/registry/javascript/product_views.js'
--- lib/lp/registry/javascript/product_views.js 2012-10-04 13:14:50 +0000
+++ lib/lp/registry/javascript/product_views.js 2012-10-05 14:06:23 +0000
@@ -12,6 +12,109 @@
var COMMERCIAL_LICENSE = "OTHER_PROPRIETARY";
/**
+ * Handle binding up events for our information type choice widget.
+ *
+ * @function bind_information_type
+ * @param {View} view
+ * @param {String} value
+ */
+ var bind_information_type = function (view) {
+ var widget = Y.lp.app.choice.addPopupChoiceForRadioButtons(
+ 'information_type',
+ info_type.cache_to_choicesource(
+ LP.cache.information_type_data));
+
+ // When the information type changes, check if we should show/hide
+ // UI components.
+ widget.on('save', function (ev) {
+ view.information_type_change(ev.target.get('value'));
+ });
+
+ // If we don't have a value to start with, always default to PUBLIC.
+ if (widget.get('value')) {
+ view.information_type_change(widget.get('value'));
+ } else {
+ view.information_type_change('PUBLIC');
+ }
+
+ // Hold onto the reference to the information type widget so we
+ // can test with it.
+ view._information_type_widget = widget;
+ };
+
+ /**
+ * Update the visiblity and settings of the license field based on the
+ * value of the information type.
+ *
+ * @function toggle_license_field
+ * @param {String} value of the information type
+ *
+ */
+ var toggle_license_field = function (value) {
+ // The license code is nested in another table.
+ var license = Y.one('input[name="field.licenses"]');
+ var license_cont = license.ancestor('td').ancestor('td');
+
+ if (info_type.get_cache_data_from_key(value, 'value',
+ 'is_private')) {
+ // Hide the license field and set it to commercial.
+ license_cont.hide();
+
+ // We have to set both checked and the value because value is
+ // a DOM object property, but the UI is updated based on the
+ // 'checked' attribute.
+ 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 commercial license');
+ } else {
+ // Show the license field.
+ license_cont.show();
+ }
+ };
+
+
+ /**
+ * View handler for the Project Edit view.
+ *
+ * @class registry.views.EditProduct
+ * @extends Y.View
+ *
+ */
+ ns.EditProduct = Y.Base.create('edit-project-view', Y.View, [], {
+ /**
+ * Update the UI when the information type changes.
+ *
+ * @method _information_type_change
+ * @param {String} information_type value
+ */
+ information_type_change: function (value) {
+ toggle_license_field(value);
+ },
+
+ /**
+ * Render out the View into the active page.
+ *
+ * @method render
+ */
+ render: function (information_type_value) {
+ // Only bind the information type if it's available due to the
+ // feature flag.
+ if (Y.one('input[name="field.information_type"]')) {
+ // get the original value, if there is one set.
+ bind_information_type(this);
+ }
+ }
+ }, {
+ ATTRS: {
+ container: {
+ value: '.yui-main'
+ }
+ }
+ });
+
+ /**
* Handle setting up the JS for the new project View
*
* @class registry.views.NewProduct
@@ -29,33 +132,6 @@
valid_char: new RegExp('^[a-z0-9][-.+a-z0-9]*$', 'i'),
/**
- * Process binding the UI for the information type choice widget.
- *
- * @method _bind_information_type
- * @private
- */
- _bind_information_type: function () {
- var that = this;
- var widget = Y.lp.app.choice.addPopupChoiceForRadioButtons(
- 'information_type',
- info_type.cache_to_choicesource(
- LP.cache.information_type_data));
-
- // When the information type changes, check if we should show/hide
- // UI components.
- widget.on('save', function (ev) {
- that._information_type_change(ev.target.get('value'));
- });
-
- // And on first render, the information type is PUBLIC.
- that._information_type_change('PUBLIC');
-
- // Hold onto the reference to the information type widget so we
- // can test with it.
- this._information_type_widget = widget;
- },
-
- /**
* Bind the url/name field interaction
*
* @method _bind_name_field
@@ -190,49 +266,6 @@
},
/**
- * Update the UI when the information type changes.
- *
- * @method _information_type_change
- * @param {String} information_type value
- *
- */
- _information_type_change: function (value) {
- var driver_cont = Y.one(
- 'input[name="field.driver"]').ancestor('td');
- var bug_super_cont = Y.one(
- 'input[name="field.bug_supervisor"]').ancestor('td');
-
- // The license code is nested in another table.
- var license = Y.one('input[name="field.licenses"]');
- var license_cont = license.ancestor('td').ancestor('td');
-
- if (info_type.get_cache_data_from_key(value, 'value',
- 'is_private')) {
- // Hide the driver and bug supervisor fields.
- driver_cont.show();
- bug_super_cont.show();
-
- // Hide the license field and set it to commercial.
- license_cont.hide();
-
- // We have to set both checked and the value because value is
- // a DOM object property, but the UI is updated based on the
- // 'checked' attribute.
- 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 commercial license');
- } else {
- driver_cont.hide();
- bug_super_cont.hide();
-
- // Show the license field.
- license_cont.show();
- }
- },
-
- /**
* Handle the reveals when there are search results.
*
* @method _show_separator
@@ -290,7 +323,7 @@
*/
bindUI: function () {
if (Y.one('input[name="field.information_type"]')) {
- this._bind_information_type();
+ bind_information_type(this);
}
if(Y.one('input[id="field.name"]')) {
@@ -305,6 +338,28 @@
},
/**
+ * Update the UI when the information type changes.
+ *
+ * @method _information_type_change
+ * @param {String} information_type value
+ */
+ information_type_change: function (value) {
+ toggle_license_field(value);
+ var driver_cont = Y.one('input[name="field.driver"]').ancestor('td');
+ var bug_super_cont = Y.one('input[name="field.bug_supervisor"]').ancestor('td');
+
+ if (info_type.get_cache_data_from_key(value, 'value',
+ 'is_private')) {
+ // Hide the driver and bug supervisor fields.
+ driver_cont.show();
+ bug_super_cont.show();
+ } else {
+ driver_cont.hide();
+ bug_super_cont.hide();
+ }
+ },
+
+ /**
* Standard YUI init.
*
* @method initialize
@@ -428,6 +483,6 @@
});
}, '0.1', {
- 'requires': ['base', 'node', 'lp.ui.effects', 'lp.app.choice',
+ 'requires': ['base', 'node', 'view', 'lp.ui.effects', 'lp.app.choice',
'lp.ui.choiceedit', 'lp.app.information_type']
});
=== modified file 'lib/lp/registry/javascript/tests/test_product_views.js'
--- lib/lp/registry/javascript/tests/test_product_views.js 2012-10-04 13:14:50 +0000
+++ lib/lp/registry/javascript/tests/test_product_views.js 2012-10-05 14:06:23 +0000
@@ -6,9 +6,8 @@
var ns = Y.namespace('registry.views');
tests.suite = new Y.Test.Suite('registry.product-views Tests');
- tests.suite.add(new Y.Test.Case({
- name: 'registry.product-views.new_tests',
-
+ // Share methods used in various test suites for the Views.
+ var shared = {
setUp: function () {
window.LP = {
cache: {
@@ -38,7 +37,6 @@
var tpl = Y.one('#tpl_information_type');
var html = Y.lp.mustache.to_html(tpl.getContent());
Y.one('#testdom').setContent(html);
-
},
tearDown: function () {
@@ -46,6 +44,31 @@
LP.cache = {};
},
+ assert_license_updates: function () {
+ var licenses = Y.one('input[name="field.licenses"]');
+ var licenses_cont = licenses.ancestor('td').ancestor('td');
+ Y.Assert.areEqual('none',
+ licenses_cont.getComputedStyle('display'),
+ 'License is hidden when EMBARGOED is selected.');
+
+ 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 commercial license',
+ license_info.get('value'));
+ }
+ };
+
+ tests.suite.add(new Y.Test.Case({
+ name: 'registry.product-views.new_tests',
+ setUp: shared.setUp,
+ tearDown: shared.tearDown,
+
test_library_exists: function () {
Y.Assert.isObject(ns.NewProduct,
"Could not locate the registry.views.NewProduct module");
@@ -110,13 +133,7 @@
// Force the value to change to a private value and make sure the
// UI is updated.
widget._saveData('EMBARGOED');
-
- var licenses = Y.one('input[name="field.licenses"]');
- var licenses_cont = licenses.ancestor('td').ancestor('td');
- Y.Assert.areEqual('none',
- licenses_cont.getComputedStyle('display'),
- 'License is hidden when EMBARGOED is selected.');
-
+ shared.assert_license_updates();
var bug_super = Y.one('input[name="field.bug_supervisor"]');
var bug_super_cont = bug_super.ancestor('td');
@@ -131,17 +148,39 @@
'none',
driver_cont.getComputedStyle('display'),
'Driver is shown when EMBARGOED is selected.');
-
- 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 commercial license',
- license_info.get('value'));
+ }
+ }));
+
+
+ tests.suite.add(new Y.Test.Case({
+ name: 'registry.product-views.edit_tests',
+ setUp: shared.setUp,
+ tearDown: shared.tearDown,
+
+ test_library_exists: function () {
+ Y.Assert.isObject(ns.EditProduct,
+ "Could not locate the registry.views.EditProduct module");
+ },
+
+ test_information_type_widget: function () {
+ // Render will give us a pretty JS choice widget.
+ var view = new ns.EditProduct();
+ view.render();
+ Y.Assert.isNotNull(Y.one('#testdom .yui3-ichoicesource'));
+ },
+
+ test_information_type_choose_non_public: function () {
+ // Selecting an information type not-public hides the license,
+ // sets it to commercial
+ var view = new ns.EditProduct();
+ view.render();
+
+ var widget = view._information_type_widget;
+
+ // Force the value to change to a private value and make sure the
+ // UI is updated.
+ widget._saveData('EMBARGOED');
+ shared.assert_license_updates();
}
}));
=== modified file 'lib/lp/registry/templates/project-edit.pt'
--- lib/lp/registry/templates/project-edit.pt 2009-08-05 19:26:37 +0000
+++ lib/lp/registry/templates/project-edit.pt 2012-10-05 14:06:23 +0000
@@ -6,9 +6,20 @@
metal:use-macro="view/macro:page/main_only"
i18n:domain="launchpad"
>
+ <head>
+ <tal:head-epilogue metal:fill-slot="head_epilogue">
+ <script type="text/javascript">
+ LPJS.use('registry.product-views', function(Y) {
+ Y.on('domready', function() {
+ var view = new Y.registry.views.EditProduct();
+ view.render();
+ });
+ });
+ </script>
+ </tal:head-epilogue>
+ </head>
<body>
<div metal:fill-slot="main">
-
<div class="top-portlet"
metal:use-macro="context/@@launchpad_form/form">
Follow ups
