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);
     }
 
 }));

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to