launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08553
[Merge] lp:~rvb/maas/multifield-ui into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/multifield-ui into lp:maas with lp:~rvb/maas/multifield-api as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/multifield-ui/+merge/108979
This branch is a preparation branch for the follow-up branch which adds the power_parameter field to the UI on a node edit page.
Because of the way Django handles empty values in forms, I had to adapt how DictCharField and the related widget deal with empty values to be able to use the same form in the API and in the UI. The value is considered empty if the the value itself is in validators.EMPTY_VALUES or if all the subfields are None. This allows us to set the subfields to the empty string (see test test_PUT_updates_power_parameters_empty_string).
Also, the boolean field used to control whether or not the data should be checked should be displayed (this is a control field used only in the API). This is tested by the changed test in src/maasserver/tests/test_config_forms.py.
I've also added "required=False" to all the subfields in power_parameters.py because at this stage I want the subfields to be very flexible.
Drive-by fix: Change templates/test_module.js to have a real test method.
--
https://code.launchpad.net/~rvb/maas/multifield-ui/+merge/108979
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/multifield-ui into lp:maas.
=== modified file 'src/maasserver/config_forms.py'
--- src/maasserver/config_forms.py 2012-06-05 10:55:21 +0000
+++ src/maasserver/config_forms.py 2012-06-06 16:07:21 +0000
@@ -143,12 +143,11 @@
value = value[:-1]
if not value or isinstance(value, (list, tuple)):
# value is considered empty if it is in
- # validators.EMPTY_VALUES, or if each of the subvalues is in
- # validators.EMPTY_VALUES.
+ # validators.EMPTY_VALUES, or if each of the subvalues is
+ # None.
is_empty = (
value in validators.EMPTY_VALUES or
- len(filter(
- lambda x: x not in validators.EMPTY_VALUES, value)) == 0)
+ len(filter(lambda x: x is not None, value)) == 0)
if is_empty:
if self.required:
raise ValidationError(self.error_messages['required'])
@@ -253,12 +252,18 @@
def render(self, name, value, attrs=None):
# value is a list of values, each corresponding to a widget
# in self.widgets.
+ # Do not display the 'skip_check' boolean widget.
+ if self.skip_check:
+ widgets = self.widgets[:-1]
+ else:
+ widgets = self.widgets
if not isinstance(value, list):
value = self.decompress(value)
output = ['<fieldset>']
final_attrs = self.build_attrs(attrs)
id_ = final_attrs.get('id', None)
- for index, widget in enumerate(self.widgets):
+
+ for index, widget in enumerate(widgets):
try:
widget_value = value[index]
except IndexError:
@@ -328,7 +333,7 @@
"""Returns a list of decompressed values for the given compressed
value. The given value can be assumed to be valid, but not
necessarily non-empty."""
- if value is not None:
+ if value not in validators.EMPTY_VALUES:
return [value.get(name, None) for name in self.names]
else:
return [None] * len(self.names)
=== modified file 'src/maasserver/power_parameters.py'
--- src/maasserver/power_parameters.py 2012-05-31 15:51:58 +0000
+++ src/maasserver/power_parameters.py 2012-06-06 16:07:21 +0000
@@ -36,38 +36,60 @@
POWER_TYPE_PARAMETERS = {
+ POWER_TYPE.DEFAULT:
+ DictCharField([], required=False, skip_check=True),
POWER_TYPE.WAKE_ON_LAN:
DictCharField(
[
- ('power_address', forms.CharField(label="Address")),
+ (
+ 'power_address',
+ forms.CharField(label="Address", required=False)),
],
required=False,
skip_check=True),
POWER_TYPE.VIRSH:
DictCharField(
[
- ('driver', forms.CharField(label="Driver")),
- ('username', forms.CharField(label="Username")),
- ('power_address', forms.CharField(label="Address")),
- ('power_id', forms.CharField(label="Power ID")),
+ ('driver', forms.CharField(label="Driver", required=False)),
+ (
+ 'username',
+ forms.CharField(label="Username", required=False)),
+ (
+ 'power_address',
+ forms.CharField(label="Address", required=False)),
+ (
+ 'power_id',
+ forms.CharField(label="Power ID", required=False)),
],
required=False,
skip_check=True),
POWER_TYPE.IPMI:
DictCharField(
[
- ('power_address', forms.CharField(label="Address")),
- ('power_user', forms.CharField(label="User")),
- ('power_pass', forms.CharField(label="Password")),
+ (
+ 'power_address',
+ forms.CharField(label="Address", required=False)),
+ (
+ 'power_user',
+ forms.CharField(label="User", required=False)),
+ (
+ 'power_pass',
+ forms.CharField(label="Password", required=False)),
],
required=False,
skip_check=True),
POWER_TYPE.IPMI_LAN:
DictCharField(
[
- ('power_user', forms.CharField(label="User")),
- ('power_pass', forms.CharField(label="Password")),
- ('power_id', forms.CharField(label="Power ID")),
+ (
+ 'power_user',
+ forms.CharField(label="User", required=False)),
+ (
+ 'power_pass',
+ forms.CharField(label="Password", required=False)),
+ (
+ 'power_id',
+ forms.CharField(label="Power ID", required=False)),
],
required=False,
skip_check=True),
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-06-06 16:07:21 +0000
+++ src/maasserver/tests/test_api.py 2012-06-06 16:07:21 +0000
@@ -1029,6 +1029,21 @@
self.assertEqual(
{new_param: new_value}, reload_object(node).power_parameters)
+ def test_PUT_updates_power_parameters_empty_string(self):
+ self.become_admin()
+ node = factory.make_node(
+ owner=self.logged_in_user,
+ power_type=POWER_TYPE.WAKE_ON_LAN,
+ power_parameters=factory.getRandomString())
+ response = self.client.put(
+ self.get_node_uri(node),
+ {'power_parameters_power_address': ''})
+
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(
+ {'power_address': ''},
+ reload_object(node).power_parameters)
+
def test_DELETE_deletes_node(self):
# The api allows to delete a Node.
self.become_admin()
=== modified file 'src/maasserver/tests/test_config_forms.py'
--- src/maasserver/tests/test_config_forms.py 2012-06-05 09:23:50 +0000
+++ src/maasserver/tests/test_config_forms.py 2012-06-06 16:07:21 +0000
@@ -200,7 +200,8 @@
labels = [factory.getRandomString(), factory.getRandomString()]
values = [factory.getRandomString(), factory.getRandomString()]
widget = DictCharWidget(
- [widgets.TextInput, widgets.TextInput], names, labels)
+ [widgets.TextInput, widgets.TextInput, widgets.CheckboxInput],
+ names, labels, skip_check=True)
name = factory.getRandomString()
self.assertEqual(
'<fieldset>'
@@ -238,3 +239,13 @@
self.assertEqual(
[field_1_value, field_2_value],
widget.value_from_datadict(data, None, name))
+
+ def test_DictCharWidget_renders_with_empty_string_as_input_data(self):
+ names = [factory.getRandomString(), factory.getRandomString()]
+ labels = [factory.getRandomString(), factory.getRandomString()]
+ widget = DictCharWidget(
+ [widgets.TextInput, widgets.TextInput, widgets.CheckboxInput],
+ names, labels, skip_check=True)
+ name = factory.getRandomString()
+ widget.render(name, '')
+ # No exception.
=== modified file 'templates/test_module.js'
--- templates/test_module.js 2012-04-30 13:10:04 +0000
+++ templates/test_module.js 2012-06-06 16:07:21 +0000
@@ -15,7 +15,7 @@
name: 'test-sample',
testMe: function() {
- Y.Assert.areTrue(true);
+ Y.Assert.isTrue(true);
}
}));
