From 8d660e6d10daee999bd7a8358964e9acda137a3f Mon Sep 17 00:00:00 2001 From: vasileios Date: Tue, 28 Jan 2025 16:35:05 +0100 Subject: [PATCH] [#5006] Updated AddressValueSerializer to adapt manual filling of streetname and city Updated street name and city serializer fields in order to be able to require these fields in case the component is required. --- package-lock.json | 15 +- package.json | 2 +- src/openforms/contrib/brk/constants.py | 1 + src/openforms/formio/components/custom.py | 60 +++++++- src/openforms/formio/formatters/custom.py | 1 + .../formio/tests/validation/test_addressnl.py | 133 +++++++++++++++++- src/openforms/submissions/api/serializers.py | 2 +- .../tests/e2e/test_input_validation.py | 10 +- 8 files changed, 206 insertions(+), 18 deletions(-) diff --git a/package-lock.json b/package-lock.json index a8ff5f5dda..030fffde11 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "dependencies": { "@fortawesome/fontawesome-free": "^6.1.1", "@open-formulieren/design-tokens": "^0.53.0", - "@open-formulieren/formio-builder": "^0.35.0", + "@open-formulieren/formio-builder": "^0.36.0", "@open-formulieren/leaflet-tools": "^1.0.0", "@open-formulieren/monaco-json-editor": "^0.2.0", "@tinymce/tinymce-react": "^4.3.2", @@ -4498,11 +4498,10 @@ "integrity": "sha512-3Pv32ULCuFOJZ2GaqcpvB45u6xScr0lmW5ETB9P1Ox9TG5nvMcVSwuwYe/GwxbzmvtZgiMQRMKRFT9lNYLeREQ==" }, "node_modules/@open-formulieren/formio-builder": { - "version": "0.35.0", - "resolved": "https://registry.npmjs.org/@open-formulieren/formio-builder/-/formio-builder-0.35.0.tgz", - "integrity": "sha512-/qu3BgbqLnIE/YUpP7sjbI/p/xtZbMOLBNyFzUEdZbEM6st4a3vCC0A7xAb8uGFbl67qLOwunUN4Te0D6VsorA==", + "version": "0.36.0", + "resolved": "https://registry.npmjs.org/@open-formulieren/formio-builder/-/formio-builder-0.36.0.tgz", + "integrity": "sha512-ExclSZX9XdNPQIoxCRYlkYE681C0dDHJRBbNwZeU3j6oeFesnzISmvXPUSAUr3S61b36mFbjwLvF+FSTouE/qw==", "hasInstallScript": true, - "license": "EUPL-1.2", "dependencies": { "@ckeditor/ckeditor5-react": "^6.2.0", "@floating-ui/react": "^0.26.4", @@ -21524,9 +21523,9 @@ "integrity": "sha512-3Pv32ULCuFOJZ2GaqcpvB45u6xScr0lmW5ETB9P1Ox9TG5nvMcVSwuwYe/GwxbzmvtZgiMQRMKRFT9lNYLeREQ==" }, "@open-formulieren/formio-builder": { - "version": "0.35.0", - "resolved": "https://registry.npmjs.org/@open-formulieren/formio-builder/-/formio-builder-0.35.0.tgz", - "integrity": "sha512-/qu3BgbqLnIE/YUpP7sjbI/p/xtZbMOLBNyFzUEdZbEM6st4a3vCC0A7xAb8uGFbl67qLOwunUN4Te0D6VsorA==", + "version": "0.36.0", + "resolved": "https://registry.npmjs.org/@open-formulieren/formio-builder/-/formio-builder-0.36.0.tgz", + "integrity": "sha512-ExclSZX9XdNPQIoxCRYlkYE681C0dDHJRBbNwZeU3j6oeFesnzISmvXPUSAUr3S61b36mFbjwLvF+FSTouE/qw==", "requires": { "@ckeditor/ckeditor5-react": "^6.2.0", "@floating-ui/react": "^0.26.4", diff --git a/package.json b/package.json index bdd3f5227d..4c087d900c 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "dependencies": { "@fortawesome/fontawesome-free": "^6.1.1", "@open-formulieren/design-tokens": "^0.53.0", - "@open-formulieren/formio-builder": "^0.35.0", + "@open-formulieren/formio-builder": "^0.36.0", "@open-formulieren/leaflet-tools": "^1.0.0", "@open-formulieren/monaco-json-editor": "^0.2.0", "@tinymce/tinymce-react": "^4.3.2", diff --git a/src/openforms/contrib/brk/constants.py b/src/openforms/contrib/brk/constants.py index 9fef1d3f96..493ade886a 100644 --- a/src/openforms/contrib/brk/constants.py +++ b/src/openforms/contrib/brk/constants.py @@ -9,3 +9,4 @@ class AddressValue(TypedDict): city: NotRequired[str] streetName: NotRequired[str] secretStreetCity: NotRequired[str] + autoPopulated: NotRequired[bool] diff --git a/src/openforms/formio/components/custom.py b/src/openforms/formio/components/custom.py index fb47f1b617..e38ca80f80 100644 --- a/src/openforms/formio/components/custom.py +++ b/src/openforms/formio/components/custom.py @@ -490,12 +490,36 @@ class AddressValueSerializer(serializers.Serializer): required=False, allow_blank=True, ) + autoPopulated = serializers.BooleanField( + label=_("city and street name auto populated"), + help_text=_("Whether city and street name have been retrieved from the API"), + default=False, + ) def __init__(self, **kwargs): self.derive_address = kwargs.pop("derive_address", None) self.component = kwargs.pop("component", None) super().__init__(**kwargs) + def get_fields(self): + fields = super().get_fields() + + # Some fields have to be treated as required or not dynamically and based on + # specific situations. + if self.component and (validate := self.component.get("validate")): + if validate["required"] is True: + if self.derive_address: + fields["city"].required = True + fields["city"].allow_blank = False + fields["streetName"].required = True + fields["streetName"].allow_blank = False + elif validate["required"] is False: + fields["postcode"].required = False + fields["postcode"].allow_blank = True + fields["houseNumber"].required = False + fields["houseNumber"].allow_blank = True + return fields + def validate_city(self, value: str) -> str: if city_regex := glom( self.component, "openForms.components.city.validate.pattern", default="" @@ -522,18 +546,46 @@ def validate_postcode(self, value: str) -> str: def validate(self, attrs): attrs = super().validate(attrs) + auto_populated = attrs.get("autoPopulated", False) + postcode = attrs.get("postcode", "") + house_number = attrs.get("houseNumber", "") city = attrs.get("city", "") street_name = attrs.get("streetName", "") + # Allow users to save(pause) the form even if one of the fields is missing. + # We validate the combination of them only during the subission of the form. + if self.context.get("validate_on_complete", False): + if postcode and not house_number: + raise serializers.ValidationError( + { + "houseNumber": _( + 'This field is required if "postcode" is provided' + ) + }, + code="required", + ) + + if not postcode and house_number: + raise serializers.ValidationError( + { + "postcode": _( + 'This field is required if "house number" is provided' + ) + }, + code="required", + ) + if self.derive_address: - existing_hmac = attrs.get("secretStreetCity", "") - postcode = attrs.get("postcode", "") - number = attrs.get("houseNumber", "") + # When the user fills in manually the city and the street name we do not + # need to check the secret city - street name combination + if not auto_populated: + return attrs + existing_hmac = attrs.get("secretStreetCity", "") computed_hmac = salt_location_message( { "postcode": postcode, - "number": number, + "number": house_number, "city": city, "street_name": street_name, } diff --git a/src/openforms/formio/formatters/custom.py b/src/openforms/formio/formatters/custom.py index e42c3eafb2..991f817b59 100644 --- a/src/openforms/formio/formatters/custom.py +++ b/src/openforms/formio/formatters/custom.py @@ -50,6 +50,7 @@ class AddressValue(TypedDict): city: NotRequired[str] streetName: NotRequired[str] secretStreetCity: NotRequired[str] + autoPopulated: NotRequired[bool] class AddressNLFormatter(FormatterBase): diff --git a/src/openforms/formio/tests/validation/test_addressnl.py b/src/openforms/formio/tests/validation/test_addressnl.py index 365dd49a83..8915c72978 100644 --- a/src/openforms/formio/tests/validation/test_addressnl.py +++ b/src/openforms/formio/tests/validation/test_addressnl.py @@ -106,12 +106,51 @@ def test_addressNL_field_regex_pattern_success(self): self.assertTrue(is_valid) - def test_missing_keys(self): + def test_missing_keys_when_component_optional(self): component: AddressNLComponent = { "key": "addressNl", "type": "addressNL", "label": "AddressNL missing keys", "deriveAddress": False, + "validate": {"required": False}, + } + + data = { + "addressNl": { + "houseLetter": "A", + } + } + + is_valid, _ = validate_formio_data(component, data) + + self.assertTrue(is_valid) + + def test_missing_keys_when_autofill_enabled_and_component_optional(self): + component: AddressNLComponent = { + "key": "addressNl", + "type": "addressNL", + "label": "AddressNL missing keys", + "deriveAddress": True, + "validate": {"required": False}, + } + + data = { + "addressNl": { + "houseLetter": "A", + } + } + + is_valid, _ = validate_formio_data(component, data) + + self.assertTrue(is_valid) + + def test_missing_keys_when_component_required(self): + component: AddressNLComponent = { + "key": "addressNl", + "type": "addressNL", + "label": "AddressNL missing keys", + "deriveAddress": True, + "validate": {"required": True}, } invalid_values = { @@ -124,10 +163,14 @@ def test_missing_keys(self): postcode_error = extract_error(errors["addressNl"], "postcode") house_number_error = extract_error(errors["addressNl"], "houseNumber") + street_name_error = extract_error(errors["addressNl"], "streetName") + city_error = extract_error(errors["addressNl"], "city") self.assertFalse(is_valid) self.assertEqual(postcode_error.code, "required") self.assertEqual(house_number_error.code, "required") + self.assertEqual(street_name_error.code, "required") + self.assertEqual(city_error.code, "required") def test_plugin_validator(self): with replace_validators_registry() as register: @@ -138,7 +181,7 @@ def test_plugin_validator(self): "type": "addressNL", "label": "AddressNL plugin validator", "deriveAddress": False, - "validate": {"plugins": ["postcode_validator"]}, + "validate": {"required": False, "plugins": ["postcode_validator"]}, } with self.subTest("valid value"): @@ -150,6 +193,8 @@ def test_plugin_validator(self): "houseNumber": "3", "houseLetter": "A", "houseNumberAddition": "", + "streetName": "Keizersgracht", + "city": "Amsterdam", } }, ) @@ -171,12 +216,66 @@ def test_plugin_validator(self): self.assertFalse(is_valid) + def test_non_required_postcode_is_required_if_houseNumber_is_provided( + self, + ): + component: AddressNLComponent = { + "key": "addressNl", + "type": "addressNL", + "label": "AddressNL", + "deriveAddress": False, + } + + invalid_values = { + "addressNl": { + "postcode": "", + "houseNumber": "117", + "houseLetter": "", + "houseNumberAddition": "", + "city": "Amsterdam", + "streetName": "", + } + } + + is_valid, errors = validate_formio_data(component, invalid_values) + postcode_error = extract_error(errors["addressNl"], "postcode") + + self.assertFalse(is_valid) + self.assertEqual(postcode_error.code, "blank") + + def test_non_required_house_number_is_required_if_postcode_is_provided( + self, + ): + component: AddressNLComponent = { + "key": "addressNl", + "type": "addressNL", + "label": "AddressNL", + "deriveAddress": False, + } + + invalid_values = { + "addressNl": { + "postcode": "1234 AB", + "houseNumber": "", + "houseLetter": "", + "houseNumberAddition": "", + "city": "Amsterdam", + "streetName": "", + } + } + + is_valid, errors = validate_formio_data(component, invalid_values) + house_number_error = extract_error(errors["addressNl"], "houseNumber") + + self.assertFalse(is_valid) + self.assertEqual(house_number_error.code, "blank") + def test_addressNL_field_secret_success(self): component: AddressNLComponent = { "key": "addressNl", "type": "addressNL", "label": "AddressNL secret success", - "deriveAddress": False, + "deriveAddress": True, } message = "1015CJ/117/Amsterdam/Keizersgracht" @@ -190,6 +289,7 @@ def test_addressNL_field_secret_success(self): "city": "Amsterdam", "streetName": "Keizersgracht", "secretStreetCity": secret, + "autoPopulated": True, } } @@ -214,6 +314,7 @@ def test_addressNL_field_secret_failure(self): "city": "Amsterdam", "streetName": "Keizersgracht", "secretStreetCity": "invalid secret", + "autoPopulated": True, } } @@ -224,6 +325,32 @@ def test_addressNL_field_secret_failure(self): self.assertFalse(is_valid) self.assertEqual(secret_error.code, "invalid") + def test_addressNL_field_secret_not_used_when_manual_address(self): + component: AddressNLComponent = { + "key": "addressNl", + "type": "addressNL", + "label": "AddressNL secret failure", + "deriveAddress": True, + "validate": {"required": False}, + } + + data = { + "addressNl": { + "postcode": "1015CJ", + "houseNumber": "117", + "houseLetter": "", + "houseNumberAddition": "", + "city": "Amsterdam", + "streetName": "Keizersgracht", + "secretStreetCity": "a secret", + "autoPopulated": False, + } + } + + is_valid, _ = validate_formio_data(component, data) + + self.assertTrue(is_valid) + def test_addressNL_field_missing_city(self): component: AddressNLComponent = { "key": "addressNl", diff --git a/src/openforms/submissions/api/serializers.py b/src/openforms/submissions/api/serializers.py index 0e19267a0f..3c4e71939c 100644 --- a/src/openforms/submissions/api/serializers.py +++ b/src/openforms/submissions/api/serializers.py @@ -300,7 +300,7 @@ def _run_formio_validation(self, data: dict) -> None: step_data_serializer = build_serializer( configuration["components"], data=data, - context={"submission": submission}, + context={"submission": submission, "validate_on_complete": False}, ) step_data_serializer.is_valid(raise_exception=True) diff --git a/src/openforms/tests/e2e/test_input_validation.py b/src/openforms/tests/e2e/test_input_validation.py index 3eeb26f631..6a77348591 100644 --- a/src/openforms/tests/e2e/test_input_validation.py +++ b/src/openforms/tests/e2e/test_input_validation.py @@ -981,6 +981,7 @@ def assertAddressNLValidationIsAligned( ui_inputs: dict[str, str], expected_ui_error: str, api_value: dict[str, Any], + expected_error_keys: list[str] = [], ) -> None: form = create_form(component) @@ -988,7 +989,13 @@ def assertAddressNLValidationIsAligned( self._assertAddressNLFrontendValidation(form, ui_inputs, expected_ui_error) with self.subTest("backend validation"): - self._assertBackendValidation(form, component["key"], api_value) + if not expected_error_keys: + self._assertBackendValidation(form, component["key"], api_value) + else: + for key in expected_error_keys: + self._assertBackendValidation( + form, f"{component['key']}.{key}", api_value + ) @async_to_sync async def _assertAddressNLFrontendValidation( @@ -1029,6 +1036,7 @@ def test_required_field(self): "houseNumberAddition": "", }, expected_ui_error="Dit veld mag niet leeg zijn.", + expected_error_keys=["postcode", "houseNumber"], ) def test_regex_failure(self):