Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#4659] Fix default value for textfield #4738

Merged
merged 11 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/openforms/formio/migration_converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,17 @@ def rename_identifier_role_authorizee(component: Component) -> bool:
component["prefill"]["identifierRole"] = "authorizee"
return True

def fix_empty_default_value(component: Component) -> bool:
if component.get("multiple", False):
return False

default_value = component.get("defaultValue")
if default_value is None:
component["defaultValue"] = ""
return True

return False


DEFINITION_CONVERTERS = [
convert_simple_conditionals,
Expand All @@ -283,9 +294,11 @@ def rename_identifier_role_authorizee(component: Component) -> bool:
"alter_prefill_default_values": alter_prefill_default_values,
"fix_empty_validate_lengths": fix_empty_validate_lengths,
"rename_identifier_role_authorizee": rename_identifier_role_authorizee,
"fix_empty_default_value": fix_empty_default_value,
},
"email": {
"fix_empty_validate_lengths": fix_empty_validate_lengths,
"fix_empty_default_value": fix_empty_default_value,
},
"date": {
"alter_prefill_default_values": alter_prefill_default_values,
Expand Down
69 changes: 69 additions & 0 deletions src/openforms/formio/tests/test_migration_converters.py
robinmolen marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ensure_licensplate_validate_pattern,
ensure_postcode_validate_pattern,
fix_multiple_empty_default_value,
fix_empty_default_value,
prevent_datetime_components_from_emptying_invalid_values,
)
from ..typing import AddressNLComponent, Component
Expand Down Expand Up @@ -87,6 +88,74 @@ def test_default_value_changed(self):
self.assertEqual(component["defaultValue"], [])


class TextTests(SimpleTestCase):
def test_multiple_noop(self):
component: Component = {
"type": "textfield",
"key": "textField",
"label": "Text field",
"multiple": True,
"defaultValue": [],
}
changed = fix_empty_default_value(component)
self.assertFalse(changed)

def test_default_value_noop(self):
component: Component = {
"type": "textfield",
"key": "textField",
"label": "Text field",
"defaultValue": "",
}
changed = fix_empty_default_value(component)
self.assertFalse(changed)

def test_default_value_changed(self):
component: Component = {
"type": "textfield",
"key": "textField",
"label": "Text field",
"defaultValue": None,
}
changed = fix_empty_default_value(component)
self.assertTrue(changed)
self.assertEqual(component["defaultValue"], "")


class EmailTests(SimpleTestCase):
def test_multiple_noop(self):
component: Component = {
"type": "email",
"key": "eMailadres",
"label": "Emailadres",
"multiple": True,
"defaultValue": [],
}
changed = fix_empty_default_value(component)
self.assertFalse(changed)

def test_default_value_noop(self):
component: Component = {
"type": "email",
"key": "eMailadres",
"label": "Emailadres",
"defaultValue": "",
}
changed = fix_empty_default_value(component)
self.assertFalse(changed)

def test_default_value_changed(self):
component: Component = {
"type": "email",
"key": "eMailadres",
"label": "Emailadres",
"defaultValue": None,
}
changed = fix_empty_default_value(component)
self.assertTrue(changed)
self.assertEqual(component["defaultValue"], "")


class AddressNLTests(SimpleTestCase):
def test_existing_derive_address(self):
component: AddressNLComponent = {
Expand Down
17 changes: 17 additions & 0 deletions src/openforms/forms/migrations/0101_fix_empty_default_value.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.16 on 2024-10-07 14:36

from django.db import migrations

from openforms.forms.migration_operations import ConvertComponentsOperation


class Migration(migrations.Migration):

dependencies = [
("forms", "0100_merge_20240920_1816"),
]

operations = [
ConvertComponentsOperation("textfield", "fix_empty_default_value"),
ConvertComponentsOperation("email", "fix_empty_default_value")
]
robinmolen marked this conversation as resolved.
Show resolved Hide resolved
54 changes: 54 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 @@ -7,6 +7,7 @@
from furl import furl
from playwright.async_api import Page, expect

from openforms.formio.utils import iterate_components_with_configuration_path
from openforms.products.tests.factories import ProductFactory
from openforms.tests.e2e.base import (
E2ETestCase,
Expand All @@ -17,6 +18,7 @@
from openforms.utils.tests.cache import clear_caches
from openforms.variables.constants import FormVariableDataTypes, FormVariableSources

from ...models import Form
from ..factories import (
FormDefinitionFactory,
FormFactory,
Expand Down Expand Up @@ -415,6 +417,58 @@ 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):
await create_superuser()
admin_url = str(furl(self.live_server_url) / reverse("admin:forms_form_add"))
form_name = "Test textfields default value empty string"

async with browser_page() as page:
await self._admin_login(page)
await page.goto(str(admin_url))

with phase("Create and save form"):
form_name_input = page.get_by_role("textbox", name="Name", exact=True)
await form_name_input.click()
await form_name_input.fill(form_name)

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")

# Add textfield and save with the defaults
await drag_and_drop_component(page, "Tekstveld")
await close_modal(page, "Save", exact=True)

# Add email field and save with the defaults
await drag_and_drop_component(page, "E-mail")
await close_modal(page, "Save", exact=True)

# Add cosign field and save with the defaults
await page.get_by_role("button", name="Speciale velden", exact=True).click()
await drag_and_drop_component(page, "Mede-ondertekenen")
await close_modal(page, "Save", exact=True)

# Save form
await page.get_by_role("button", name="Save and continue editing", exact=True).click()

with phase("Validate default values"):
@sync_to_async
def assertFormValues():
form = Form.objects.get(name=form_name)
configuration = form.formstep_set.first().form_definition.configuration

for configuration_path, component in iterate_components_with_configuration_path(
configuration, recursive=False
):
self.assertEqual(
component["defaultValue"],
"",
msg="Test failed for component %s" % (component['key'])
)

await assertFormValues()
robinmolen marked this conversation as resolved.
Show resolved Hide resolved

@tag("gh-2805")
async def test_enable_translations_and_create_new_step(self):
await create_superuser()
Expand Down
6 changes: 6 additions & 0 deletions src/openforms/js/components/form/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ 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 = '';
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/openforms/js/components/form/textfield.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ 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 = '';
}
}
}

Expand Down
Loading