From 870fc20731342032d08036b1a9b36fdfbd7a82e8 Mon Sep 17 00:00:00 2001 From: robinvandermolen Date: Wed, 26 Feb 2025 15:41:21 +0100 Subject: [PATCH] :bug: Radio component defaultValue automatically being set to `null` in admin This bug is a combination of problems that result in the defaultValue being wrongly set by formio. The problem comes from our radio component, which defines a defaultValue in the schema. This schema is not set on the formio component, unless we specify it with `get defaultSchema`. When calculating the defaultValue, formio starts with the defaultValue on the schema, which isn't defined in the regular formio radio component schema. If the defaultValue defined on the component (editable via the admin) is truety, this will be used as the defaultValue. Because the defaultValue of the component was an empty string, which is falsey, and the defaultValue wasn't set by the component schema, the defaultValue was set to null. If you then would edit the component and save it, the defaultValue `null` would be saved in the database. Backport-Of: open-formulieren/open-forms#5104 --- bin/fix_radio_component_default_values.py | 59 ++++++ .../tests/e2e_tests/test_form_designer.py | 182 ++++++++++++++++++ src/openforms/js/components/form/email.js | 5 + src/openforms/js/components/form/iban.js | 1 + .../js/components/form/licenseplate.js | 1 + .../js/components/form/phoneNumber.js | 1 + src/openforms/js/components/form/radio.js | 4 + src/openforms/js/components/form/textarea.js | 4 +- src/openforms/js/components/form/textfield.js | 6 +- src/openforms/js/components/form/time.js | 1 + 10 files changed, 262 insertions(+), 2 deletions(-) create mode 100755 bin/fix_radio_component_default_values.py diff --git a/bin/fix_radio_component_default_values.py b/bin/fix_radio_component_default_values.py new file mode 100755 index 0000000000..cea58104ed --- /dev/null +++ b/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() diff --git a/src/openforms/forms/tests/e2e_tests/test_form_designer.py b/src/openforms/forms/tests/e2e_tests/test_form_designer.py index 930baae231..91be2c1dda 100644 --- a/src/openforms/forms/tests/e2e_tests/test_form_designer.py +++ b/src/openforms/forms/tests/e2e_tests/test_form_designer.py @@ -415,6 +415,188 @@ def setUpTestData(): key_input = page.get_by_label("Property Name") await expect(key_input).to_have_value("textField1") + async def test_textfields_default_value_empty_string(self): + @sync_to_async + def setUpTestData(): + # set up a form + form = FormFactory.create( + name="Test textfields default value empty string", + name_nl="Test textfields default value empty string", + generate_minimal_setup=False, + ) + 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 add_new_step(page) + step_name_input = page.get_by_role( + "textbox", name="Step name", exact=True + ) + await step_name_input.click() + await step_name_input.fill("Step 1") + + basic_components = [ + "Tekstveld", + "E-mail", + "Tijd", + "Telefoonnummer", + "Tekstvlak", + ] + for component in basic_components: + await drag_and_drop_component(page, component, "group-panel-custom") + await close_modal(page, "Save", exact=True) + + basic_components_with_multiple = [ + "Tekstveld", + "E-mail", + "Tijd", + "Telefoonnummer", + "Tekstvlak", + ] + for component in basic_components_with_multiple: + await drag_and_drop_component(page, component, "group-panel-custom") + await page.get_by_label("Multiple values", exact=True).check() + await close_modal(page, "Save", exact=True) + + # Open the special fields list + await page.get_by_role( + "button", name="Speciale velden", exact=True + ).click() + + special_components = ["IBAN", "Kenteken", "Mede-ondertekenen"] + for component in special_components: + await drag_and_drop_component(page, component) + await close_modal(page, "Save", exact=True) + + special_components_with_multiple = ["IBAN", "Kenteken"] + for component in special_components_with_multiple: + await drag_and_drop_component(page, component) + await page.get_by_label("Multiple values", exact=True).check() + await close_modal(page, "Save", exact=True) + + # Save form + await page.get_by_role( + "button", name="Save and continue editing", exact=True + ).click() + await page.wait_for_url(admin_url) + + with phase("Validate default values"): + + @sync_to_async + def assertFormValues(): + for component in form.iter_components(): + expected = [""] if component.get("multiple", False) else "" + + self.assertEqual( + component["defaultValue"], + expected, + msg=f"Test failed for component {component['key']} with multiple set to {component.get('multiple', False)}", + ) + + 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() diff --git a/src/openforms/js/components/form/email.js b/src/openforms/js/components/form/email.js index 265ebdb487..90a22ecdd4 100644 --- a/src/openforms/js/components/form/email.js +++ b/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 @@ -32,6 +33,10 @@ class EmailField extends FormioEmail { patchValidateDefaults(this); } + + get defaultSchema() { + return EmailField.schema(); + } } export default EmailField; diff --git a/src/openforms/js/components/form/iban.js b/src/openforms/js/components/form/iban.js index 499c668ff7..ac5c5a6aa2 100644 --- a/src/openforms/js/components/form/iban.js +++ b/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', diff --git a/src/openforms/js/components/form/licenseplate.js b/src/openforms/js/components/form/licenseplate.js index 3f77c89e2c..ad4cfb894e 100644 --- a/src/openforms/js/components/form/licenseplate.js +++ b/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', diff --git a/src/openforms/js/components/form/phoneNumber.js b/src/openforms/js/components/form/phoneNumber.js index 64de132058..f60739ba4f 100644 --- a/src/openforms/js/components/form/phoneNumber.js +++ b/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 diff --git a/src/openforms/js/components/form/radio.js b/src/openforms/js/components/form/radio.js index e0a44edef7..3c1ede38e3 100644 --- a/src/openforms/js/components/form/radio.js +++ b/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; diff --git a/src/openforms/js/components/form/textarea.js b/src/openforms/js/components/form/textarea.js index e1ed20eaf2..fb38e053fa 100644 --- a/src/openforms/js/components/form/textarea.js +++ b/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() { diff --git a/src/openforms/js/components/form/textfield.js b/src/openforms/js/components/form/textfield.js index 61035031ee..d70bee80be 100644 --- a/src/openforms/js/components/form/textfield.js +++ b/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() { @@ -45,6 +45,10 @@ class TextField extends FormioTextField { patchValidateDefaults(this); } + + get defaultSchema() { + return TextField.schema(); + } } export default TextField; diff --git a/src/openforms/js/components/form/time.js b/src/openforms/js/components/form/time.js index 314c4f2f62..20729b295c 100644 --- a/src/openforms/js/components/form/time.js +++ b/src/openforms/js/components/form/time.js @@ -20,6 +20,7 @@ class TimeField extends Time { minTime: null, maxTime: null, }, + defaultValue: '', }, ...extend );