Merge pull request #5123 from open-formulieren/bug/5104-radio-compone…

…nt-default-value-change

Components keeping original defaultValue when editing

bin/fix_radio_component_default_values.py

@@ -0,0 +1,59 @@
+#!/usr/bin/env python
+#
+# Fix the `defaultValue` of radio components.
+#
+# Due to a bug in the form editor, when a radio component had a defaultValue of ""
+# (empty string) it would automatically be set to `null`. This in turn could cause json
+# logic to stop working properly, due to the defaultValue being changed.
+#
+# This script automatically fixes all defaultValues of radio components that where set
+# to `null`.
+# Related to ticket: https://github.com/open-formulieren/open-forms/issues/5104
+from __future__ import annotations
+
+import sys
+from pathlib import Path
+
+import django
+
+SRC_DIR = Path(__file__).parent.parent / "src"
+sys.path.insert(0, str(SRC_DIR.resolve()))
+
+
+def fix_radio_component_default_values():
+    from openforms.forms.models import FormDefinition
+
+    form_definitions_to_update = []
+    fds = FormDefinition.objects.iterator()
+    for form_definition in fds:
+        # Flag to update Form Definition. Is set when child component is changed
+        should_persist_fd = False
+
+        for component in form_definition.iter_components():
+            # The fix is only needed for radio components whose defaultValue is `None`
+            if component["type"] != "radio" or component["defaultValue"] is not None:
+                continue
+
+            component["defaultValue"] = ""
+            # Set flag to update Form Definition
+            should_persist_fd = True
+
+        if should_persist_fd:
+            form_definitions_to_update.append(form_definition)
+
+    FormDefinition.objects.bulk_update(
+        form_definitions_to_update, fields=["configuration"]
+    )
+
+
+def main(**kwargs):
+    from openforms.setup import setup_env
+
+    setup_env()
+    django.setup()
+
+    return fix_radio_component_default_values(**kwargs)
+
+
+if __name__ == "__main__":
+    main()

src/openforms/formio/migration_converters.py

@@ -385,7 +385,10 @@ def fix_empty_default_value(component: Component) -> bool:
     "currency": {
         "fix_empty_validate_lengths": fix_empty_validate_lengths,
     },
-    "radio": {"set_openforms_datasrc": set_openforms_datasrc},
+    "radio": {
+        "set_openforms_datasrc": set_openforms_datasrc,
+        "fix_empty_default_value": fix_empty_default_value,
+    },
     # Special components
     "iban": {
         "fix_empty_validate_lengths": fix_empty_validate_lengths,

src/openforms/formio/tests/test_migration_converters.py

@@ -716,3 +716,41 @@ def test_missing_interactions(self):
             component["interactions"],
             {"marker": True, "polygon": False, "polyline": False},
         )
+
+
+class RadioTests(SimpleTestCase):
+    def test_no_default_value_doesnt_change(self):
+        component: Component = {
+            "type": "radio",
+            "key": "radio",
+            "label": "Radio field",
+        }
+
+        changed = fix_empty_default_value(component)
+
+        self.assertFalse(changed)
+
+    def test_none_as_default_value_does_change(self):
+        component: Component = {
+            "type": "radio",
+            "key": "radio",
+            "label": "Radio field",
+            "defaultValue": None,
+        }
+
+        changed = fix_empty_default_value(component)
+
+        self.assertTrue(changed)
+        self.assertEqual(component["defaultValue"], "")
+
+    def test_empty_string_as_default_value_doesnt_change(self):
+        component: Component = {
+            "type": "radio",
+            "key": "radio",
+            "label": "Radio field",
+            "defaultValue": "",
+        }
+
+        changed = fix_empty_default_value(component)
+
+        self.assertFalse(changed)

src/openforms/forms/migrations/0101_fix_radio_empty_default_value.py

@@ -0,0 +1,14 @@
+# Generated by Django 4.2.18 on 2025-03-05 15:25
+
+from django.db import migrations
+
+from openforms.forms.migration_operations import ConvertComponentsOperation
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("forms", "0100_add_interaction_config_to_map_component"),
+    ]
+
+    operations = [ConvertComponentsOperation("radio", "fix_empty_default_value")]

src/openforms/forms/tests/e2e_tests/test_form_designer.py

@@ -511,6 +511,98 @@ def assertFormValues():
 
                 await assertFormValues()
 
+    @tag("dh-5104")
+    async def test_radio_component_default_value_empty_string(self):
+        @sync_to_async
+        def setUpTestData():
+            form = FormFactory.create()
+            FormStepFactory.create(
+                form=form, form_definition__configuration={"components": []}
+            )
+            return form
+
+        await create_superuser()
+        form = await setUpTestData()
+        admin_url = str(
+            furl(self.live_server_url)
+            / reverse("admin:forms_form_change", args=(form.pk,))
+        )
+
+        async with browser_page() as page:
+            await self._admin_login(page)
+            await page.goto(str(admin_url))
+
+            with phase("Populate and save form"):
+                await page.get_by_role("tab", name="Steps and fields").click()
+
+                await drag_and_drop_component(page, "Radio", "group-panel-custom")
+                await page.get_by_test_id("input-label").fill("Custom Radio")
+                await page.get_by_test_id("input-values[0].label").fill("Label")
+                await page.get_by_test_id("input-values[0].value").fill("Value")
+                await close_modal(page, "Save")
+
+                # Save form
+                await page.get_by_role(
+                    "button", name="Save and continue editing"
+                ).click()
+                await page.wait_for_url(admin_url)
+
+            with phase("Validate default value after create"):
+
+                @sync_to_async
+                def assertFormValues():
+                    configuration = (
+                        form.formstep_set.get().form_definition.configuration
+                    )
+                    radio_component = configuration["components"][0]
+                    self.assertEqual(
+                        radio_component["defaultValue"],
+                        "",
+                    )
+
+                await assertFormValues()
+
+            # When creating a radio component, the defaultValue is set correctly in the
+            # database.
+            #
+            # This bug appears in the json view, where the `defaultValue` will show as
+            # `null` after `Save and continue editing`.
+            # The json view is a conditionally rendered html list of divs
+            # (only elements that are directly shown are targetable in the dom)
+            #
+            # So to test if this bug happens, we just save it again, and then check the
+            # database.
+            with phase("Edit the form"):
+                await page.get_by_role("tab", name="Steps and fields").click()
+
+                # The defaultValue in the bug is set to `null`.
+                # If we open the component, and immediately save it, the defaultValue
+                # will change from `""` (in the db) to `null` (set by bug)
+                await open_component_options_modal(page, "Custom Radio", exact=True)
+                await close_modal(page, "Save", exact=True)
+
+                # Save form
+                await page.get_by_role(
+                    "button", name="Save and continue editing"
+                ).click()
+                await page.wait_for_url(admin_url)
+
+            with phase("Validate default value after editing"):
+
+                @sync_to_async
+                def assertFormValues():
+                    configuration = (
+                        form.formstep_set.get().form_definition.configuration
+                    )
+                    radio_component = configuration["components"][0]
+
+                    self.assertEqual(
+                        radio_component["defaultValue"],
+                        "",
+                    )
+
+                await assertFormValues()
+
     @tag("gh-2805")
     async def test_enable_translations_and_create_new_step(self):
         await create_superuser()

src/openforms/js/components/form/email.js

@@ -9,6 +9,7 @@ class EmailField extends FormioEmail {
   static schema(...extend) {
     const schema = FormioEmail.schema(
       {
+        defaultValue: '',
         validateOn: 'blur',
       },
       ...extend
@@ -31,12 +32,10 @@ class EmailField extends FormioEmail {
     super(...args);
 
     patchValidateDefaults(this);
+  }
 
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
+  get defaultSchema() {
+    return EmailField.schema();
   }
 }
 

src/openforms/js/components/form/iban.js

@@ -8,6 +8,7 @@ class IbanField extends TextField {
   static schema(...extend) {
     return TextField.schema(
       {
+        defaultValue: '',
         type: 'iban',
         label: 'IBAN',
         key: 'iban',
@@ -31,12 +32,6 @@ class IbanField extends TextField {
     super(...args);
 
     patchValidateDefaults(this);
-
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
   }
 
   get defaultSchema() {

src/openforms/js/components/form/licenseplate.js

@@ -9,6 +9,7 @@ class LicensePlate extends TextField {
   static schema(...extend) {
     const schema = TextField.schema(
       {
+        defaultValue: '',
         type: 'licenseplate',
         label: 'License plate',
         key: 'licenseplate',
@@ -39,12 +40,6 @@ class LicensePlate extends TextField {
     super(...args);
 
     patchValidateDefaults(this);
-
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
   }
 
   get defaultSchema() {

src/openforms/js/components/form/phoneNumber.js

@@ -9,6 +9,7 @@ class PhoneNumberField extends PhoneNumber {
   static schema(...extend) {
     const schema = PhoneNumber.schema(
       {
+        defaultValue: '',
         inputMask: null,
       },
       ...extend
@@ -30,12 +31,6 @@ class PhoneNumberField extends PhoneNumber {
     super(...args);
 
     patchValidateDefaults(this);
-
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
   }
 
   get defaultSchema() {

src/openforms/js/components/form/radio.js

@@ -38,6 +38,10 @@ class RadioField extends RadioFormio {
 
     return super.setSelectedClasses();
   }
+
+  get defaultSchema() {
+    return RadioField.schema();
+  }
 }
 
 export default RadioField;

src/openforms/js/components/form/textarea.js

@@ -9,7 +9,9 @@ const FormioTextarea = Formio.Components.components.textarea;
 
 class TextArea extends FormioTextarea {
   static schema(...extend) {
-    return localiseSchema(FormioTextarea.schema({validate: {maxLength: 10000}}, ...extend));
+    return localiseSchema(
+      FormioTextarea.schema({defaultValue: '', validate: {maxLength: 10000}}, ...extend)
+    );
   }
 
   static get builderInfo() {
@@ -23,12 +25,6 @@ class TextArea extends FormioTextarea {
     super(...args);
 
     patchValidateDefaults(this);
-
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
   }
 
   get defaultSchema() {

src/openforms/js/components/form/textfield.js

@@ -30,7 +30,7 @@ export const patchValidateDefaults = instance => {
 
 class TextField extends FormioTextField {
   static schema(...extend) {
-    return localiseSchema(FormioTextField.schema(...extend));
+    return localiseSchema(FormioTextField.schema({defaultValue: ''}, ...extend));
   }
 
   static get builderInfo() {
@@ -44,12 +44,10 @@ class TextField extends FormioTextField {
     super(...args);
 
     patchValidateDefaults(this);
+  }
 
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
+  get defaultSchema() {
+    return TextField.schema();
   }
 }
 

src/openforms/js/components/form/time.js

@@ -1,7 +1,6 @@
 import {Formio} from 'formiojs';
 
 import {localiseSchema} from './i18n';
-import {patchValidateDefaults} from './textfield';
 
 const Time = Formio.Components.components.time;
 
@@ -13,12 +12,6 @@ const Time = Formio.Components.components.time;
 class TimeField extends Time {
   constructor(...args) {
     super(...args);
-
-    // somewhere the default emptyValue/defaultValue does not seem to be used and it forces
-    // component.defaultValue to be null, which causes issues with multiples #4659
-    if (this.component.defaultValue === null) {
-      this.component.defaultValue = '';
-    }
   }
 
   static schema(...extend) {