diff --git a/src/openforms/formio/migration_converters.py b/src/openforms/formio/migration_converters.py index 504fb0ab1f..9a2c191871 100644 --- a/src/openforms/formio/migration_converters.py +++ b/src/openforms/formio/migration_converters.py @@ -149,10 +149,27 @@ def prevent_datetime_components_from_emptying_invalid_values( return True +def fix_empty_validate_lengths(component: Component) -> bool: + if not (validate := component.get("validate")): + return False + + changed = False + for key in ("minLength", "maxLength", "min", "max", "minWords", "maxWords"): + if key in validate and validate[key] == "": # type: ignore + changed = True + del validate[key] # type: ignore + + return changed + + CONVERTERS: dict[str, dict[str, ComponentConverter]] = { # Input components "textfield": { "alter_prefill_default_values": alter_prefill_default_values, + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, + "email": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, }, "date": { "alter_prefill_default_values": alter_prefill_default_values, @@ -164,22 +181,43 @@ def prevent_datetime_components_from_emptying_invalid_values( "time": { "move_time_validators": move_time_validators, }, - "select": {"set_openforms_datasrc": set_openforms_datasrc}, - "selectboxes": {"set_openforms_datasrc": set_openforms_datasrc}, - "radio": {"set_openforms_datasrc": set_openforms_datasrc}, + "phoneNumber": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, "postcode": { "alter_prefill_default_values": alter_prefill_default_values, "ensure_validate_pattern": ensure_postcode_validate_pattern, + "fix_empty_validate_lengths": fix_empty_validate_lengths, }, "file": { "fix_default_value": fix_file_default_value, }, + "textarea": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, + "number": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, + "select": {"set_openforms_datasrc": set_openforms_datasrc}, + "selectboxes": {"set_openforms_datasrc": set_openforms_datasrc}, + "currency": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, + "radio": {"set_openforms_datasrc": set_openforms_datasrc}, # Special components + "iban": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, "licenseplate": { "ensure_validate_pattern": ensure_licensplate_validate_pattern, + "fix_empty_validate_lengths": fix_empty_validate_lengths, }, "bsn": { "alter_prefill_default_values": alter_prefill_default_values, + "fix_empty_validate_lengths": fix_empty_validate_lengths, + }, + "cosign": { + "fix_empty_validate_lengths": fix_empty_validate_lengths, }, # Layout components "columns": { diff --git a/src/openforms/forms/migrations/0096_fix_invalid_validate_configuration.py b/src/openforms/forms/migrations/0096_fix_invalid_validate_configuration.py new file mode 100644 index 0000000000..9bb17edd48 --- /dev/null +++ b/src/openforms/forms/migrations/0096_fix_invalid_validate_configuration.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.10 on 2024-03-15 16:02 + +from django.db import migrations + +from openforms.forms.migration_operations import ConvertComponentsOperation + + +class Migration(migrations.Migration): + + dependencies = [ + ("forms", "0095_merge_20240313_1742"), + ] + + operations = [ + ConvertComponentsOperation("textfield", "fix_empty_validate_lengths"), + ConvertComponentsOperation("email", "fix_empty_validate_lengths"), + ConvertComponentsOperation("phoneNumber", "fix_empty_validate_lengths"), + ConvertComponentsOperation("postcode", "fix_empty_validate_lengths"), + ConvertComponentsOperation("textarea", "fix_empty_validate_lengths"), + ConvertComponentsOperation("number", "fix_empty_validate_lengths"), + ConvertComponentsOperation("currency", "fix_empty_validate_lengths"), + ConvertComponentsOperation("iban", "fix_empty_validate_lengths"), + ConvertComponentsOperation("licenseplate", "fix_empty_validate_lengths"), + ConvertComponentsOperation("bsn", "fix_empty_validate_lengths"), + ConvertComponentsOperation("cosign", "fix_empty_validate_lengths"), + ] diff --git a/src/openforms/forms/tests/test_migrations.py b/src/openforms/forms/tests/test_migrations.py index 72d42f149b..c0982bba8f 100644 --- a/src/openforms/forms/tests/test_migrations.py +++ b/src/openforms/forms/tests/test_migrations.py @@ -107,3 +107,183 @@ def test_builder_enabled(self): self.assertIsNotNone(variables[0].service_fetch_configuration) self.assertIsNone(variables[1].service_fetch_configuration) + + +class FixValidateConfigurationMigrationTests(TestMigrations): + app = "forms" + migrate_from = "0095_merge_20240313_1742" + migrate_to = "0096_fix_invalid_validate_configuration" + + def setUpBeforeMigration(self, apps): + FormDefinition = apps.get_model("forms", "FormDefinition") + + bad_config = [ + { + "type": "textfield", + "key": "textfield", + "key": "textfield", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "email", + "key": "email", + "key": "email", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "phoneNumber", + "key": "phoneNumber", + "key": "phoneNumber", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "postcode", + "key": "postcode", + "key": "postcode", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "textarea", + "key": "textarea", + "key": "textarea", + "validate": { + "minLength": "", + "maxLength": "", + "minWords": "", + "maxWords": "", + }, + }, + { + "type": "number", + "key": "number", + "key": "number", + "validate": { + "min": "", + "max": "", + }, + }, + { + "type": "currency", + "key": "currency", + "key": "currency", + "validate": { + "min": "", + "max": "", + }, + }, + { + "type": "iban", + "key": "iban", + "key": "iban", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "licenseplate", + "key": "licenseplate", + "key": "licenseplate", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "bsn", + "key": "bsn", + "key": "bsn", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + { + "type": "cosign", + "key": "cosign", + "key": "cosign", + "validate": { + "minLength": "", + "maxLength": "", + }, + }, + ] + FormDefinition.objects.create( + name="broken", configuration={"components": bad_config} + ) + + def test_migration_fixes_issues(self): + FormDefinition = self.apps.get_model("forms", "FormDefinition") + fixed_config = FormDefinition.objects.get().configuration["components"] + + ( + textfield, + email, + phone_number, + postcode, + textarea, + number, + currency, + iban, + licenseplate, + bsn, + cosign, + ) = fixed_config + + with self.subTest("textfield"): + self.assertNotIn("minLength", textfield["validate"]) + self.assertNotIn("maxLength", textfield["validate"]) + + with self.subTest("email"): + self.assertNotIn("minLength", email["validate"]) + self.assertNotIn("maxLength", email["validate"]) + + with self.subTest("phone_number"): + self.assertNotIn("minLength", phone_number["validate"]) + self.assertNotIn("maxLength", phone_number["validate"]) + + with self.subTest("postcode"): + self.assertNotIn("minLength", postcode["validate"]) + self.assertNotIn("maxLength", postcode["validate"]) + + with self.subTest("textarea"): + self.assertNotIn("minLength", textarea["validate"]) + self.assertNotIn("maxLength", textarea["validate"]) + self.assertNotIn("minWords", textarea["validate"]) + self.assertNotIn("maxWords", textarea["validate"]) + + with self.subTest("number"): + self.assertNotIn("min", number["validate"]) + self.assertNotIn("max", number["validate"]) + + with self.subTest("currency"): + self.assertNotIn("min", currency["validate"]) + self.assertNotIn("max", currency["validate"]) + + with self.subTest("iban"): + self.assertNotIn("minLength", iban["validate"]) + self.assertNotIn("maxLength", iban["validate"]) + + with self.subTest("licenseplate"): + self.assertNotIn("minLength", licenseplate["validate"]) + self.assertNotIn("maxLength", licenseplate["validate"]) + + with self.subTest("bsn"): + self.assertNotIn("minLength", bsn["validate"]) + self.assertNotIn("maxLength", bsn["validate"]) + + with self.subTest("cosign"): + self.assertNotIn("minLength", cosign["validate"]) + self.assertNotIn("maxLength", cosign["validate"]) diff --git a/src/openforms/js/components/form/cosign.js b/src/openforms/js/components/form/cosign.js index 07b88c50b5..52291deb96 100644 --- a/src/openforms/js/components/form/cosign.js +++ b/src/openforms/js/components/form/cosign.js @@ -4,6 +4,7 @@ import {AUTH_PLUGINS_ENDPOINT} from 'components/admin/form_design/constants.js'; import {get} from 'utils/fetch'; import {localiseSchema} from './i18n'; +import {patchValidateDefaults} from './textfield'; export const getAvailableAuthPlugins = async () => { const response = await get(AUTH_PLUGINS_ENDPOINT); @@ -16,6 +17,8 @@ class CoSignField extends FormioEmail { constructor(...args) { super(...args); + patchValidateDefaults(this); + // somewhere the default emptyValue/defaultValue does not seem to be used and it forces // component.defaultValue to be null, which crashes the builder. if (this.component.defaultValue === null) { diff --git a/src/openforms/js/components/form/currency.js b/src/openforms/js/components/form/currency.js index 9754577502..ae5386a9c0 100644 --- a/src/openforms/js/components/form/currency.js +++ b/src/openforms/js/components/form/currency.js @@ -3,6 +3,7 @@ import CurrencyEditData from 'formiojs/components/currency/editForm/Currency.edi import _ from 'lodash'; import {localiseSchema} from './i18n'; +import {patchValidateDefaults} from './number'; const FormioCurrency = Formio.Components.components.currency; @@ -20,6 +21,12 @@ class CurrencyField extends FormioCurrency { }; } + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } + get defaultValue() { let defaultValue = super.defaultValue; diff --git a/src/openforms/js/components/form/email.js b/src/openforms/js/components/form/email.js index 91d0da6c27..265ebdb487 100644 --- a/src/openforms/js/components/form/email.js +++ b/src/openforms/js/components/form/email.js @@ -1,6 +1,7 @@ import {Formio} from 'formiojs'; import {localiseSchema} from './i18n'; +import {patchValidateDefaults} from './textfield'; const FormioEmail = Formio.Components.components.email; @@ -25,6 +26,12 @@ class EmailField extends FormioEmail { schema: EmailField.schema(), }; } + + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } } export default EmailField; diff --git a/src/openforms/js/components/form/iban.js b/src/openforms/js/components/form/iban.js index fd75d8dc66..499c668ff7 100644 --- a/src/openforms/js/components/form/iban.js +++ b/src/openforms/js/components/form/iban.js @@ -1,5 +1,7 @@ import {Formio} from 'formiojs'; +import {patchValidateDefaults} from './textfield'; + const TextField = Formio.Components.components.textfield; class IbanField extends TextField { @@ -25,6 +27,12 @@ class IbanField extends TextField { }; } + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } + get defaultSchema() { return IbanField.schema(); } diff --git a/src/openforms/js/components/form/licenseplate.js b/src/openforms/js/components/form/licenseplate.js index 0e2812ac6f..3f77c89e2c 100644 --- a/src/openforms/js/components/form/licenseplate.js +++ b/src/openforms/js/components/form/licenseplate.js @@ -1,6 +1,7 @@ import {Formio} from 'formiojs'; import {localiseSchema} from './i18n'; +import {patchValidateDefaults} from './textfield'; const TextField = Formio.Components.components.textfield; @@ -34,6 +35,12 @@ class LicensePlate extends TextField { }; } + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } + get defaultSchema() { return LicensePlate.schema(); } diff --git a/src/openforms/js/components/form/number.js b/src/openforms/js/components/form/number.js index e3bbcfe742..13a70b9611 100644 --- a/src/openforms/js/components/form/number.js +++ b/src/openforms/js/components/form/number.js @@ -4,6 +4,19 @@ import {localiseSchema} from './i18n'; const FormioNumber = Formio.Components.components.number; +export const patchValidateDefaults = instance => { + // Similar fix to the one in the textfield component. For some reason, Formio defaults + // to empty strings for numeric values instead of using null or undefined :/ + const validate = instance.component?.validate; + + if (validate?.min === '') { + delete validate.min; + } + if (validate?.max === '') { + delete validate.max; + } +}; + class NumberField extends FormioNumber { static schema(...extend) { return localiseSchema(FormioNumber.schema(...extend)); @@ -20,6 +33,12 @@ class NumberField extends FormioNumber { }; } + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } + get defaultValue() { let defaultValue = super.defaultValue; diff --git a/src/openforms/js/components/form/phoneNumber.js b/src/openforms/js/components/form/phoneNumber.js index e414adad4f..64de132058 100644 --- a/src/openforms/js/components/form/phoneNumber.js +++ b/src/openforms/js/components/form/phoneNumber.js @@ -1,6 +1,7 @@ import {Formio} from 'formiojs'; import {localiseSchema} from './i18n'; +import {patchValidateDefaults} from './textfield'; const PhoneNumber = Formio.Components.components.phoneNumber; @@ -25,6 +26,12 @@ class PhoneNumberField extends PhoneNumber { }; } + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } + get defaultSchema() { return PhoneNumberField.schema(); } diff --git a/src/openforms/js/components/form/textarea.js b/src/openforms/js/components/form/textarea.js index 07b04c9cd9..e1ed20eaf2 100644 --- a/src/openforms/js/components/form/textarea.js +++ b/src/openforms/js/components/form/textarea.js @@ -3,6 +3,7 @@ import NativePromise from 'native-promise-only'; import {Formio} from 'react-formio'; import {localiseSchema} from './i18n'; +import {patchValidateDefaults} from './textfield'; const FormioTextarea = Formio.Components.components.textarea; @@ -18,6 +19,12 @@ class TextArea extends FormioTextarea { }; } + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } + get defaultSchema() { return TextArea.schema(); } diff --git a/src/openforms/js/components/form/textfield.js b/src/openforms/js/components/form/textfield.js index f15210f5ef..61035031ee 100644 --- a/src/openforms/js/components/form/textfield.js +++ b/src/openforms/js/components/form/textfield.js @@ -4,6 +4,30 @@ import {localiseSchema} from './i18n'; const FormioTextField = Formio.Components.components.textfield; +export const patchValidateDefaults = instance => { + // Formio.js itself doesn't respect their own typescript declarations... + // https://github.com/formio/formio.js/blob/master/src/components/textfield/TextField.js#L21 + // So we patch up these badly typed default values, letting the default behaviour of + // our own formio-builder kick in. + // Fixing this in static schema doesn't seem to apply it to component instances (?), + // so we need to patch the weird typing information here. + const validate = instance.component?.validate; + + if (validate?.minLength === '') { + delete validate.minLength; + } + if (validate?.maxLength === '') { + delete validate.maxLength; + } + + if (validate?.minWords === '') { + delete validate.minWords; + } + if (validate?.maxWords === '') { + delete validate.maxWords; + } +}; + class TextField extends FormioTextField { static schema(...extend) { return localiseSchema(FormioTextField.schema(...extend)); @@ -15,6 +39,12 @@ class TextField extends FormioTextField { schema: TextField.schema(), }; } + + constructor(...args) { + super(...args); + + patchValidateDefaults(this); + } } export default TextField;