Merge pull request #4018 from open-formulieren/issue/4016-wrong-maxle…

…ngth-type Fix wrong type for validation properties
open-formulieren · Mar 19, 2024 · 8b6ff27 · 8b6ff27
2 parents 4690a5d + 757b9fb
commit 8b6ff27
Show file tree

Hide file tree

Showing 12 changed files with 342 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();
   }