Merge pull request #1307 from ogayot/subforms-recurse-validation

Check child forms status when disabling/enabling Done button of a form

subiquity/ui/views/filesystem/guided.py

@@ -80,6 +80,7 @@ class LVMOptionsForm(SubForm):
 
     def _toggle(self, sender, val):
         self.luks_options.enabled = val
+        self.validated()
 
     encrypt = BooleanField(_("Encrypt the LVM group with LUKS"), help=NO_HELP)
     luks_options = SubFormField(LUKSOptionsForm, "", help=NO_HELP)
@@ -146,6 +147,7 @@ class GuidedChoiceForm(SubForm):
 
     def _toggle(self, sender, val):
         self.lvm_options.enabled = val
+        self.validated()
 
 
 class GuidedForm(Form):
@@ -165,6 +167,7 @@ class GuidedForm(Form):
 
     def _toggle_guided(self, sender, new_value):
         self.guided_choice.enabled = new_value
+        self.validated()
 
 
 HELP = _("""

subiquitycore/ui/form.py

@@ -111,27 +111,6 @@ class _Validator(WidgetWrap):
         self.field.validate()
 
 
-class FormField(abc.ABC):
-
-    next_index = 0
-    takes_default_style = True
-    caption_first = True
-
-    def __init__(self, caption=None, help=None):
-        self.caption = caption
-        self.help = help
-        self.index = FormField.next_index
-        FormField.next_index += 1
-
-    @abc.abstractmethod
-    def _make_widget(self, form):
-        pass
-
-    def bind(self, form):
-        widget = self._make_widget(form)
-        return BoundFormField(self, form, widget)
-
-
 class WantsToKnowFormField(object):
     """A marker class."""
     def set_bound_form_field(self, bff):
@@ -160,6 +139,10 @@ class BoundFormField(object):
         if isinstance(widget, WantsToKnowFormField):
             widget.set_bound_form_field(self)
 
+    def is_in_error(self) -> bool:
+        """ Tells whether this field is in error. """
+        return self.in_error
+
     def _build_table(self):
         widget = self.widget
         if self.field.takes_default_style:
@@ -298,6 +281,42 @@ class BoundFormField(object):
             row.enabled = val
 
 
+class BoundSubFormField(BoundFormField):
+    def is_in_error(self):
+        """ Tells whether this field is in error. We will also check if the
+        subform (if enabled) reports an error.
+        """
+        if super().is_in_error():
+            return True
+
+        if not self._enabled:
+            return False
+
+        return self.widget.form.has_validation_error()
+
+
+class FormField(abc.ABC):
+
+    next_index = 0
+    takes_default_style = True
+    caption_first = True
+    bound_field_class = BoundFormField
+
+    def __init__(self, caption=None, help=None):
+        self.caption = caption
+        self.help = help
+        self.index = FormField.next_index
+        FormField.next_index += 1
+
+    @abc.abstractmethod
+    def _make_widget(self, form):
+        pass
+
+    def bind(self, form):
+        widget = self._make_widget(form)
+        return self.bound_field_class(self, form, widget)
+
+
 def simple_field(widget_maker):
     class Field(FormField):
         def _make_widget(self, form):
@@ -504,13 +523,12 @@ class Form(object, metaclass=MetaForm):
             focus_buttons=focus_buttons, excerpt=excerpt,
             narrow_rows=narrow_rows)
 
+    def has_validation_error(self) -> bool:
+        """ Tells if any field is in error. """
+        return any(map(lambda f: f.is_in_error(), self._fields))
+
     def validated(self):
-        in_error = False
-        for f in self._fields:
-            if f.in_error:
-                in_error = True
-                break
-        if in_error:
+        if self.has_validation_error():
             self.buttons.base_widget.contents[0][0].enabled = False
             self.buttons.base_widget.focus_position = 1
         else:
@@ -543,6 +561,7 @@ class SubFormWidget(WidgetWrap):
 class SubFormField(FormField):
 
     takes_default_style = False
+    bound_field_class = BoundSubFormField
 
     def __init__(self, form_cls, caption=None, help=None):
         super().__init__(caption=caption, help=help)
@@ -558,3 +577,8 @@ class SubForm(Form):
     def __init__(self, parent, **kw):
         self.parent = parent
         super().__init__(**kw)
+
+    def validated(self):
+        """ Propagate the validation to the parent. """
+        self.parent.validated()
+        super().validated()

subiquitycore/ui/tests/test_form.py

@@ -0,0 +1,151 @@
+# Copyright 2022 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from unittest import TestCase
+
+from subiquitycore.ui.form import (
+    Form,
+    StringField,
+    SubForm,
+    SubFormField,
+    )
+
+
+class TestForm(TestCase):
+
+    def test_has_validation_error(self):
+        """ Make sure Form.has_validation_form() returns:
+        * True if any field is in error
+        * False otherwise. """
+        class DummyForm(Form):
+            field1 = StringField("DummyStringOne", help="")
+            field2 = StringField("DummyStringTwo", help="")
+
+        form = DummyForm()
+
+        form.field1.in_error = False
+        form.field2.in_error = False
+        self.assertFalse(form.has_validation_error())
+
+        form.field1.in_error = True
+        form.field2.in_error = True
+        self.assertTrue(form.has_validation_error())
+
+        form.field1.in_error = True
+        form.field2.in_error = False
+        self.assertTrue(form.has_validation_error())
+
+    def test_has_validation_error_with_subform(self):
+        """ Make sure Form.has_validation_form() is affected by fields from
+        child forms (only if the child form is enabled). """
+        class DummySubForm(SubForm):
+            field1 = StringField("DummyString", help="")
+
+        class DummyForm(Form):
+            field1 = StringField("DummyString", help="")
+            dummy_subform = SubFormField(DummySubForm, "", help="")
+
+        form = DummyForm()
+        subform = form.dummy_subform.widget.form
+
+        form.field1.in_error = False
+        subform.field1.in_error = False
+        self.assertFalse(form.has_validation_error())
+
+        form.field1.in_error = True
+        subform.field1.in_error = False
+        self.assertTrue(form.has_validation_error())
+
+        form.field1.in_error = False
+        subform.field1.in_error = True
+        self.assertTrue(form.has_validation_error())
+
+        form.field1.in_error = True
+        subform.field1.in_error = True
+        self.assertTrue(form.has_validation_error())
+
+        # Make sure fields in disabled subforms are ignored.
+        form.field1.in_error = False
+        subform.field1.in_error = True
+        form.dummy_subform.enabled = False
+        self.assertFalse(form.has_validation_error())
+
+    def test_has_validation_error_with_subsubform(self):
+        """ Make sure enabling/disabling parent forms also acts as if sub forms
+        are disabled. """
+        class DummySubSubForm(SubForm):
+            field1 = StringField("DummyString", help="")
+
+        class DummySubForm(SubForm):
+            dummy_subform = SubFormField(DummySubSubForm, "", help="")
+
+        class DummyForm(Form):
+            dummy_subform = SubFormField(DummySubForm, "", help="")
+
+        form = DummyForm()
+        subform = form.dummy_subform.widget.form
+        subsubform = subform.dummy_subform.widget.form
+
+        subsubform.field1.in_error = True
+        self.assertTrue(form.has_validation_error())
+
+        # If subsubform is disabled, it should be ignored.
+        subsubform.field1.in_error = True
+        subform.dummy_subform.enabled = False
+        self.assertFalse(form.has_validation_error())
+
+        # If subform is disabled, it should also be ignored.
+        subsubform.field1.in_error = True
+        subform.dummy_subform.enabled = True
+        form.dummy_subform.enabled = False
+        self.assertFalse(form.has_validation_error())
+
+    def test_done_button_auto_toggle(self):
+        """ Make sure calling validated() enables or disables the Done button.
+        """
+        class DummyForm(Form):
+            field1 = StringField("DummyString", help="")
+
+        form = DummyForm()
+        done_button = form.buttons.base_widget.contents[0][0]
+
+        form.field1.in_error = False
+        form.validated()
+        self.assertTrue(done_button.enabled)
+
+        form.field1.in_error = True
+        form.validated()
+        self.assertFalse(done_button.enabled)
+
+    def test_subform_validated_propagates(self):
+        """ Make sure calling validated() in a subform affects the Done button
+        in the parent form. """
+        class DummySubForm(SubForm):
+            field1 = StringField("DummyString", help="")
+
+        class DummyForm(Form):
+            dummy_subform = SubFormField(DummySubForm, "", help="")
+
+        form = DummyForm()
+        subform = form.dummy_subform.widget.form
+        done_button = form.buttons.base_widget.contents[0][0]
+
+        subform.field1.in_error = False
+        subform.validated()
+        self.assertTrue(done_button.enabled)
+
+        subform.field1.in_error = True
+        subform.validated()
+        self.assertFalse(done_button.enabled)