Skip to content

Commit

Permalink
🐛 Radio component defaultValue automatically being set to null in a…
Browse files Browse the repository at this point in the history
…dmin

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: #5104
  • Loading branch information
robinmolen committed Mar 6, 2025
1 parent d02e1f0 commit 870fc20
Show file tree
Hide file tree
Showing 10 changed files with 262 additions and 2 deletions.
59 changes: 59 additions & 0 deletions bin/fix_radio_component_default_values.py
Original file line number Diff line number Diff line change
@@ -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()
182 changes: 182 additions & 0 deletions src/openforms/forms/tests/e2e_tests/test_form_designer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions src/openforms/js/components/form/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class EmailField extends FormioEmail {
static schema(...extend) {
const schema = FormioEmail.schema(
{
defaultValue: '',
validateOn: 'blur',
},
...extend
Expand All @@ -32,6 +33,10 @@ class EmailField extends FormioEmail {

patchValidateDefaults(this);
}

get defaultSchema() {
return EmailField.schema();
}
}

export default EmailField;
1 change: 1 addition & 0 deletions src/openforms/js/components/form/iban.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class IbanField extends TextField {
static schema(...extend) {
return TextField.schema(
{
defaultValue: '',
type: 'iban',
label: 'IBAN',
key: 'iban',
Expand Down
1 change: 1 addition & 0 deletions src/openforms/js/components/form/licenseplate.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class LicensePlate extends TextField {
static schema(...extend) {
const schema = TextField.schema(
{
defaultValue: '',
type: 'licenseplate',
label: 'License plate',
key: 'licenseplate',
Expand Down
1 change: 1 addition & 0 deletions src/openforms/js/components/form/phoneNumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class PhoneNumberField extends PhoneNumber {
static schema(...extend) {
const schema = PhoneNumber.schema(
{
defaultValue: '',
inputMask: null,
},
...extend
Expand Down
4 changes: 4 additions & 0 deletions src/openforms/js/components/form/radio.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class RadioField extends RadioFormio {

return super.setSelectedClasses();
}

get defaultSchema() {
return RadioField.schema();
}
}

export default RadioField;
4 changes: 3 additions & 1 deletion src/openforms/js/components/form/textarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 5 additions & 1 deletion src/openforms/js/components/form/textfield.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -45,6 +45,10 @@ class TextField extends FormioTextField {

patchValidateDefaults(this);
}

get defaultSchema() {
return TextField.schema();
}
}

export default TextField;
1 change: 1 addition & 0 deletions src/openforms/js/components/form/time.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class TimeField extends Time {
minTime: null,
maxTime: null,
},
defaultValue: '',
},
...extend
);
Expand Down

0 comments on commit 870fc20

Please sign in to comment.