From a96d095c3af4851891174b1fe8096e5d14fb52e3 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 | 92 +++++++++++++++++++ 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 +- 9 files changed, 171 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 685c5553c6..2c1b60731f 100644 --- a/src/openforms/forms/tests/e2e_tests/test_form_designer.py +++ b/src/openforms/forms/tests/e2e_tests/test_form_designer.py @@ -510,6 +510,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() diff --git a/src/openforms/js/components/form/email.js b/src/openforms/js/components/form/email.js index cdbb062e7f..d01402cab0 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 @@ -38,6 +39,10 @@ class EmailField extends FormioEmail { this.component.defaultValue = ''; } } + + 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 43b4f09a32..06c3a5a48e 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 ebf038d560..fcf612e224 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 a0a8266945..6466885cc7 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 9fda017466..0b59463aaa 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 4945d818d6..51b65a8703 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() { @@ -51,6 +51,10 @@ class TextField extends FormioTextField { this.component.defaultValue = ''; } } + + get defaultSchema() { + return TextField.schema(); + } } export default TextField;