From f500b701179ab6c81ef1fa8dd89742ebd88b988f Mon Sep 17 00:00:00 2001 From: robinvandermolen Date: Tue, 24 Dec 2024 15:06:21 +0100 Subject: [PATCH 01/12] :sparkles: [#4650] Add variable select input to email registration Either the `to_emails` or the `to_email_from_variable` fields should used when registering the email backend plugin. When a variable is selected, the resulting email address will be used for mailings. When the variable doesn't contain an email address, the `to_emails` will be used as fallback. --- .../registrations/email/EmailOptionsForm.js | 1 + .../email/EmailOptionsFormFields.js | 2 + .../fields/EmailRecipientsFromVariable.js | 46 +++++++++++++++ .../registrations/contrib/email/config.py | 59 +++++++++++++------ .../tests/test_registration_hook.py | 44 +++++++++++++- 5 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipientsFromVariable.js diff --git a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js index fb6a8d8608..3871a0c8d8 100644 --- a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js +++ b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js @@ -51,6 +51,7 @@ EmailOptionsForm.propTypes = { emailSubject: PropTypes.string, paymentEmails: PropTypes.arrayOf(PropTypes.string), toEmails: PropTypes.arrayOf(PropTypes.string), + toEmailsFromVariable: PropTypes.string, }), onChange: PropTypes.func.isRequired, }; diff --git a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js index ae83aadfbd..d6692f05b3 100644 --- a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js +++ b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js @@ -17,6 +17,7 @@ import EmailHasAttachmentSelect from './fields/EmailHasAttachmentSelect'; import EmailPaymentSubject from './fields/EmailPaymentSubject'; import EmailPaymentUpdateRecipients from './fields/EmailPaymentUpdateRecipients'; import EmailRecipients from './fields/EmailRecipients'; +import EmailRecipientsFromVariable from './fields/EmailRecipientsFromVariable'; import EmailSubject from './fields/EmailSubject'; const EmailOptionsFormFields = ({name, schema}) => { @@ -37,6 +38,7 @@ const EmailOptionsFormFields = ({name, schema}) => {
+ diff --git a/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipientsFromVariable.js b/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipientsFromVariable.js new file mode 100644 index 0000000000..565092b7a6 --- /dev/null +++ b/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipientsFromVariable.js @@ -0,0 +1,46 @@ +import {useField} from 'formik'; +import React from 'react'; +import {FormattedMessage} from 'react-intl'; + +import Field from 'components/admin/forms/Field'; +import FormRow from 'components/admin/forms/FormRow'; +import VariableSelection from 'components/admin/forms/VariableSelection'; + +const EmailRecipientsFromVariable = () => { + const [fieldProps, , fieldHelpers] = useField('toEmailsFromVariable'); + const {setValue} = fieldHelpers; + return ( + + + } + helpText={ + + } + > + { + setValue(event.target.value); + }} + /> + + + ); +}; + +EmailRecipientsFromVariable.propTypes = {}; + +export default EmailRecipientsFromVariable; diff --git a/src/openforms/registrations/contrib/email/config.py b/src/openforms/registrations/contrib/email/config.py index 7cda616e80..f309d2a8a3 100644 --- a/src/openforms/registrations/contrib/email/config.py +++ b/src/openforms/registrations/contrib/email/config.py @@ -13,11 +13,40 @@ from .constants import AttachmentFormat +class Options(TypedDict): + """ + Shape of the email registration plugin options. + + This describes the shape of :attr:`EmailOptionsSerializer.validated_data`, after + the input data has been cleaned/validated. + """ + + to_emails: NotRequired[list[str]] + to_emails_from_variable: NotRequired[str] + attachment_formats: NotRequired[list[AttachmentFormat | str]] + payment_emails: NotRequired[list[str]] + attach_files_to_email: bool | None + email_subject: NotRequired[str] + email_payment_subject: NotRequired[str] + email_content_template_html: NotRequired[str] + email_content_template_text: NotRequired[str] + + class EmailOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): to_emails = serializers.ListField( child=serializers.EmailField(), label=_("The email addresses to which the submission details will be sent"), - required=True, + required=False, # Either to_emails or to_emails_from_variable should be required + ) + to_emails_from_variable = serializers.CharField( + label=_("Key of the target variable containing the email address"), + required=False, + allow_blank=True, + help_text=_( + "Key of the target variable whose value will be used for the mailing. " + "When using this field, the mailing will only be send to this email address. " + "The email addresses field would then be ignored. " + ), ) attachment_formats = serializers.ListField( child=serializers.ChoiceField(choices=AttachmentFormat.choices), @@ -99,23 +128,19 @@ class Meta: ), ] + def validate(self, attrs: Options) -> Options: + # The email registration requires either `to_emails` or `to_emails_from_variable` + # to determine which email address to use. + # Both may be set - in that case, `to_emails_from_variable` is preferred. + if not attrs.get("to_emails") and not attrs.get("to_emails_from_variable"): + raise serializers.ValidationError( + { + "to_emails": _("This field is required."), + }, + code="required", + ) -class Options(TypedDict): - """ - Shape of the email registration plugin options. - - This describes the shape of :attr:`EmailOptionsSerializer.validated_data`, after - the input data has been cleaned/validated. - """ - - to_emails: list[str] - attachment_formats: NotRequired[list[AttachmentFormat | str]] - payment_emails: NotRequired[list[str]] - attach_files_to_email: bool | None - email_subject: NotRequired[str] - email_payment_subject: NotRequired[str] - email_content_template_html: NotRequired[str] - email_content_template_text: NotRequired[str] + return attrs # sanity check for development - keep serializer and type definitions in sync diff --git a/src/openforms/registrations/tests/test_registration_hook.py b/src/openforms/registrations/tests/test_registration_hook.py index 65791261ec..201130f7c5 100644 --- a/src/openforms/registrations/tests/test_registration_hook.py +++ b/src/openforms/registrations/tests/test_registration_hook.py @@ -312,7 +312,7 @@ def test_registration_backend_invalid_options(self): completed=True, form__registration_backend="email", form__registration_backend_options={}, - ) # Missing "to_email" option + ) # Missing "to_emails" and "to_emails_from_variable" option with ( self.subTest("On completion - does NOT raise"), @@ -334,6 +334,48 @@ def test_registration_backend_invalid_options(self): ): register_submission(submission.id, PostSubmissionEvents.on_retry) + def test_email_registration_backend_with_to_emails(self): + submission = SubmissionFactory.create( + completed=True, + form__registration_backend="email", + form__registration_backend_options={"to_emails": ["foo@example.com"]}, + ) + + with ( + self.subTest("On completion - logs as successful"), + self.assertLogs(level="INFO") as logs, + ): + register_submission(submission.id, PostSubmissionEvents.on_completion) + + submission.refresh_from_db() + self.assertIn( + "Registration using plugin '%r' for submission '%s' succeeded", + logs.records[-1].msg, + ) + self.assertFalse(submission.needs_on_completion_retry) + + def test_email_registration_backend_with_to_emails_from_variable(self): + submission = SubmissionFactory.create( + completed=True, + form__registration_backend="email", + form__registration_backend_options={ + "to_emails_from_variable": "email_example_variable" + }, + ) + + with ( + self.subTest("On completion - logs as successful"), + self.assertLogs(level="INFO") as logs, + ): + register_submission(submission.id, PostSubmissionEvents.on_completion) + + submission.refresh_from_db() + self.assertIn( + "Registration using plugin '%r' for submission '%s' succeeded", + logs.records[-1].msg, + ) + self.assertFalse(submission.needs_on_completion_retry) + def test_calling_registration_task_with_serialized_args(self): submission = SubmissionFactory.create( completed=True, From 163c4a682fb74627ee257e21733044799a03f5b1 Mon Sep 17 00:00:00 2001 From: robinvandermolen Date: Tue, 24 Dec 2024 16:31:07 +0100 Subject: [PATCH 02/12] :sparkles: [#4650] Using variable value as email recipient The variable chosen in the email registration (`to_email_from_variable`) will now actually be used for the mailing. `to_emails` is used as fallback, in case that the variable doesn't return a valid email address. The variable will also be used for the payment status update mailing. If `payment_emails` is defined, these will be used as recipients. Otherwise it will use `to_email_from_variable`, and as a last resort the `to_emails` --- .../registrations/contrib/email/plugin.py | 37 ++- .../contrib/email/tests/test_backend.py | 217 ++++++++++++++++++ 2 files changed, 252 insertions(+), 2 deletions(-) diff --git a/src/openforms/registrations/contrib/email/plugin.py b/src/openforms/registrations/contrib/email/plugin.py index 1e0e7b6e69..92679273ab 100644 --- a/src/openforms/registrations/contrib/email/plugin.py +++ b/src/openforms/registrations/contrib/email/plugin.py @@ -3,6 +3,8 @@ from typing import Any from django.conf import settings +from django.core.exceptions import ValidationError +from django.core.validators import validate_email from django.urls import reverse from django.utils import timezone from django.utils.translation import get_language_info, gettext_lazy as _ @@ -42,11 +44,41 @@ class EmailRegistration(BasePlugin[Options]): verbose_name = _("Email registration") configuration_options = EmailOptionsSerializer + def get_recipients(self, submission: Submission, options: Options) -> list[str]: + state = submission.load_submission_value_variables_state() + recipients = [] + + # If the 'recipients from variable' is used and it exists + if ( + (variable_key := options.get("to_emails_from_variable")) + and variable_key in state.variables + and (variable_value := state.variables[variable_key].value) + ): + # To simplify things, treat all variable values as lists + if type(variable_value) != list: + variable_value = [variable_value] + + # Only if all email addresses are valid, they will be used as recipients + try: + for value in variable_value: + validate_email(value) + except ValidationError: + pass + else: + recipients = variable_value + + # If the variable cannot be used, fallback to the general email addresses + if not recipients and "to_emails" in options: + recipients = options["to_emails"] + + return recipients + def register_submission(self, submission: Submission, options: Options) -> None: config = EmailConfig.get_solo() config.apply_defaults_to(options) - self.send_registration_email(options["to_emails"], submission, options) + recipients = self.get_recipients(submission, options) + self.send_registration_email(recipients, submission, options) # ensure that the payment email is also sent if registration is deferred until # payment is completed @@ -187,8 +219,9 @@ def send_registration_email( def update_payment_status(self, submission: "Submission", options: Options): recipients = options.get("payment_emails") + if not recipients: - recipients = options["to_emails"] + recipients = self.get_recipients(submission, options) order_ids = submission.payments.get_completed_public_order_ids() extra_context = { diff --git a/src/openforms/registrations/contrib/email/tests/test_backend.py b/src/openforms/registrations/contrib/email/tests/test_backend.py index 3fd74b4a4a..ceaee381c6 100644 --- a/src/openforms/registrations/contrib/email/tests/test_backend.py +++ b/src/openforms/registrations/contrib/email/tests/test_backend.py @@ -249,6 +249,158 @@ def test_submission_with_email_backend(self): self.assertIn(f"{expected_download_url_1} (my-foo.bin)", message_text) self.assertIn(f"{expected_download_url_2} (my-bar.txt)", message_text) + def test_submission_with_email_backend_using_to_emails_from_variable(self): + submission = SubmissionFactory.from_components( + completed=True, + components_list=[ + {"key": "foo", "type": "textfield", "label": "foo"}, + ], + submitted_data={"foo": "bar"}, + form__registration_backend="email", + ) + SubmissionValueVariableFactory.create( + form_variable__source=FormVariableSources.user_defined, + form_variable__name="User defined var 1", + submission=submission, + key="email_recipient_variable", + value="foo@example.com", + ) + email_form_options = dict( + to_emails_from_variable="email_recipient_variable", + ) + email_submission = EmailRegistration("email") + + set_submission_reference(submission) + + with patch("openforms.registrations.contrib.email.utils.EmailConfig.get_solo"): + email_submission.register_submission(submission, email_form_options) + + # Verify that email was sent + self.assertEqual(len(mail.outbox), 1) + + message = mail.outbox[0] + self.assertEqual(message.to, ["foo@example.com"]) + + def test_submission_with_email_backend_using_to_emails_from_variable_with_multiple_email_addresses( + self, + ): + submission = SubmissionFactory.from_components( + completed=True, + components_list=[ + {"key": "foo", "type": "textfield", "label": "foo"}, + ], + submitted_data={"foo": "bar"}, + form__registration_backend="email", + ) + SubmissionValueVariableFactory.create( + form_variable__source=FormVariableSources.user_defined, + form_variable__name="User defined var 1", + submission=submission, + key="email_recipient_variable", + value=["foo@example.com", "bar@example.com"], + ) + email_form_options = dict( + to_emails_from_variable="email_recipient_variable", + ) + email_submission = EmailRegistration("email") + + set_submission_reference(submission) + + with patch("openforms.registrations.contrib.email.utils.EmailConfig.get_solo"): + email_submission.register_submission(submission, email_form_options) + + # Verify that email was sent + self.assertEqual(len(mail.outbox), 1) + + message = mail.outbox[0] + self.assertEqual(message.to, ["foo@example.com", "bar@example.com"]) + + def test_submission_with_email_backend_unknown_to_emails_from_variable(self): + submission = SubmissionFactory.from_components( + completed=True, + components_list=[ + {"key": "foo", "type": "textfield", "label": "foo"}, + ], + submitted_data={"foo": "bar"}, + form__registration_backend="email", + ) + email_form_options = dict( + to_emails_from_variable="email_recipient_variable", + ) + email_submission = EmailRegistration("email") + + set_submission_reference(submission) + + with patch("openforms.registrations.contrib.email.utils.EmailConfig.get_solo"): + email_submission.register_submission(submission, email_form_options) + + # Verify that email wasn't sent + self.assertEqual(len(mail.outbox), 0) + + def test_submission_with_email_backend_invalid_to_emails_from_variable(self): + submission = SubmissionFactory.from_components( + completed=True, + components_list=[ + {"key": "foo", "type": "textfield", "label": "foo"}, + ], + submitted_data={"foo": "bar"}, + form__registration_backend="email", + ) + SubmissionValueVariableFactory.create( + form_variable__source=FormVariableSources.user_defined, + form_variable__name="User defined var 1", + submission=submission, + key="email_recipient_variable", + value="foo.com", + ) + email_form_options = dict( + to_emails_from_variable="email_recipient_variable", + ) + email_submission = EmailRegistration("email") + + set_submission_reference(submission) + + with patch("openforms.registrations.contrib.email.utils.EmailConfig.get_solo"): + email_submission.register_submission(submission, email_form_options) + + # Verify that email wasn't sent + self.assertEqual(len(mail.outbox), 0) + + def test_submission_with_email_backend_invalid_to_emails_from_variable_with_fallback( + self, + ): + submission = SubmissionFactory.from_components( + completed=True, + components_list=[ + {"key": "foo", "type": "textfield", "label": "foo"}, + ], + submitted_data={"foo": "bar"}, + form__registration_backend="email", + ) + SubmissionValueVariableFactory.create( + form_variable__source=FormVariableSources.user_defined, + form_variable__name="User defined var 1", + submission=submission, + key="email_recipient_variable", + value="foo.com", + ) + email_form_options = dict( + to_emails_from_variable="email_recipient_variable", + to_emails=["bar@example.com"], + ) + email_submission = EmailRegistration("email") + + set_submission_reference(submission) + + with patch("openforms.registrations.contrib.email.utils.EmailConfig.get_solo"): + email_submission.register_submission(submission, email_form_options) + + # Verify that email was sent + self.assertEqual(len(mail.outbox), 1) + + message = mail.outbox[0] + self.assertEqual(message.to, ["bar@example.com"]) + def test_submission_with_email_backend_strip_out_urls(self): config = GlobalConfiguration.get_solo() config.email_template_netloc_allowlist = [] @@ -477,6 +629,71 @@ def test_register_and_update_paid_product_with_payment_email_recipient(self): # check we used the payment_emails self.assertEqual(message.to, ["payment@bar.nl", "payment@foo.nl"]) + def test_register_and_update_paid_product_with_payment_email_recipient_and_variable_email_recipient( + self, + ): + submission = SubmissionFactory.from_data( + {"voornaam": "Foo"}, + form__product__price=Decimal("11.35"), + form__payment_backend="demo", + registration_success=True, + public_registration_reference="XYZ", + ) + SubmissionValueVariableFactory.create( + form_variable__source=FormVariableSources.user_defined, + form_variable__name="User defined var 1", + submission=submission, + key="email_recipient_variable", + value="foo@example.com", + ) + + email_form_options = dict( + to_emails=["foo@bar.nl", "bar@foo.nl"], + to_emails_from_variable="email_recipient_variable", + # payment_emails would override to_emails and to_emails_from_variable + payment_emails=["payment@bar.nl", "payment@foo.nl"], + ) + email_submission = EmailRegistration("email") + email_submission.update_payment_status(submission, email_form_options) + + self.assertEqual(len(mail.outbox), 1) + + message = mail.outbox[0] + # check we used the payment_emails + self.assertEqual(message.to, ["payment@bar.nl", "payment@foo.nl"]) + + def test_register_and_update_paid_product_with_variable_email_recipient( + self, + ): + submission = SubmissionFactory.from_data( + {"voornaam": "Foo"}, + form__product__price=Decimal("11.35"), + form__payment_backend="demo", + registration_success=True, + public_registration_reference="XYZ", + ) + SubmissionValueVariableFactory.create( + form_variable__source=FormVariableSources.user_defined, + form_variable__name="User defined var 1", + submission=submission, + key="email_recipient_variable", + value="foo@example.com", + ) + + email_form_options = dict( + to_emails=["foo@bar.nl", "bar@foo.nl"], + # to_emails_from_variable would override to_emails + to_emails_from_variable="email_recipient_variable", + ) + email_submission = EmailRegistration("email") + email_submission.update_payment_status(submission, email_form_options) + + self.assertEqual(len(mail.outbox), 1) + + message = mail.outbox[0] + # check we used the payment_emails + self.assertEqual(message.to, ["foo@example.com"]) + @override_settings(DEFAULT_FROM_EMAIL="info@open-forms.nl") def test_submission_with_email_backend_export_csv_xlsx(self): email_form_options = dict( From c23e2da609bc0e9de94755ed1dc8f165bd8e50c0 Mon Sep 17 00:00:00 2001 From: robinvandermolen Date: Tue, 24 Dec 2024 17:28:05 +0100 Subject: [PATCH 03/12] :globe_with_meridians: [#4650] Updated translations --- src/openforms/js/compiled-lang/en.json | 12 ++++++++++++ src/openforms/js/compiled-lang/nl.json | 12 ++++++++++++ src/openforms/js/lang/en.json | 10 ++++++++++ src/openforms/js/lang/nl.json | 10 ++++++++++ 4 files changed, 44 insertions(+) diff --git a/src/openforms/js/compiled-lang/en.json b/src/openforms/js/compiled-lang/en.json index b09003fddf..09ce4adf38 100644 --- a/src/openforms/js/compiled-lang/en.json +++ b/src/openforms/js/compiled-lang/en.json @@ -5457,6 +5457,12 @@ "value": "the variable" } ], + "mOh/pF": [ + { + "type": 0, + "value": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." + } + ], "mOjKcm": [ { "type": 0, @@ -6273,6 +6279,12 @@ "value": "Do not display the configured label and top line as the header in the fieldset." } ], + "uwCD8e": [ + { + "type": 0, + "value": "Using a variable to decide to which email address the submission details will be sent" + } + ], "vAZDY4": [ { "type": 0, diff --git a/src/openforms/js/compiled-lang/nl.json b/src/openforms/js/compiled-lang/nl.json index 6e9966d2f5..95992ff191 100644 --- a/src/openforms/js/compiled-lang/nl.json +++ b/src/openforms/js/compiled-lang/nl.json @@ -5475,6 +5475,12 @@ "value": "de variabele" } ], + "mOh/pF": [ + { + "type": 0, + "value": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." + } + ], "mOjKcm": [ { "type": 0, @@ -6291,6 +6297,12 @@ "value": "Verberg de koptekst en de lijn boven de veldengroep in het formulier." } ], + "uwCD8e": [ + { + "type": 0, + "value": "Using a variable to decide to which email address the submission details will be sent" + } + ], "vAZDY4": [ { "type": 0, diff --git a/src/openforms/js/lang/en.json b/src/openforms/js/lang/en.json index 9ec99111ef..6542ac71fb 100644 --- a/src/openforms/js/lang/en.json +++ b/src/openforms/js/lang/en.json @@ -2524,6 +2524,11 @@ "description": "\"variable\" operand type", "originalDefault": "the variable" }, + "mOh/pF": { + "defaultMessage": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option.", + "description": "Email registration options 'toEmailsFromVariable' helpText", + "originalDefault": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." + }, "mOjKcm": { "defaultMessage": "Steps and fields", "description": "Form design tab title", @@ -2914,6 +2919,11 @@ "description": "History link button", "originalDefault": "History" }, + "uwCD8e": { + "defaultMessage": "Using a variable to decide to which email address the submission details will be sent", + "description": "Email registration options 'toEmailsFromVariable' label", + "originalDefault": "Using a variable to decide to which email address the submission details will be sent" + }, "vAZDY4": { "defaultMessage": "Source path", "description": "Prefill / Objects API: column header for object type property selection", diff --git a/src/openforms/js/lang/nl.json b/src/openforms/js/lang/nl.json index 28136b3ddd..697caa3cfe 100644 --- a/src/openforms/js/lang/nl.json +++ b/src/openforms/js/lang/nl.json @@ -2545,6 +2545,11 @@ "description": "\"variable\" operand type", "originalDefault": "the variable" }, + "mOh/pF": { + "defaultMessage": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option.", + "description": "Email registration options 'toEmailsFromVariable' helpText", + "originalDefault": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." + }, "mOjKcm": { "defaultMessage": "Stappen en velden", "description": "Form design tab title", @@ -2935,6 +2940,11 @@ "description": "History link button", "originalDefault": "History" }, + "uwCD8e": { + "defaultMessage": "Using a variable to decide to which email address the submission details will be sent", + "description": "Email registration options 'toEmailsFromVariable' label", + "originalDefault": "Using a variable to decide to which email address the submission details will be sent" + }, "vAZDY4": { "defaultMessage": "Bronpad", "description": "Prefill / Objects API: column header for object type property selection", From e448544e9fad6640bf4f6ab0c8f5f5433ac53513 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 30 Dec 2024 15:28:53 +0100 Subject: [PATCH 04/12] :art: [#4650] Simplify email behaviour The possible combinations of to_emails being empty and to_emails_from_variable possibly being malformed, in combination with having to treat lists of possible email addresses leads to complicated code and a risk of no email being sent at all. Instead, it was decided that: * we keep requiring the field to_emails, to always have a well-formed email address that gives sufficient confidence emails will get delivered _somewhere_ * if a variable is provided, and it has a value, we use that to override the destination email address. This may possibly be a malformed email, or even be completely the wrong type of variable, but that will fail loudly and show up in error monitoring I've added some logging as well for when things eventually go wrong, we have some traces or debugging entrypoints to investigate. --- .../registrations/contrib/email/config.py | 11 +- .../registrations/contrib/email/plugin.py | 60 +++--- .../contrib/email/tests/test_backend.py | 191 ++++++++---------- 3 files changed, 123 insertions(+), 139 deletions(-) diff --git a/src/openforms/registrations/contrib/email/config.py b/src/openforms/registrations/contrib/email/config.py index f309d2a8a3..da4cf33784 100644 --- a/src/openforms/registrations/contrib/email/config.py +++ b/src/openforms/registrations/contrib/email/config.py @@ -7,6 +7,7 @@ from openforms.api.validators import AllOrNoneTruthyFieldsValidator from openforms.emails.validators import URLSanitationValidator +from openforms.formio.api.fields import FormioVariableKeyField from openforms.template.validators import DjangoTemplateValidator from openforms.utils.mixins import JsonSchemaSerializerMixin @@ -21,7 +22,7 @@ class Options(TypedDict): the input data has been cleaned/validated. """ - to_emails: NotRequired[list[str]] + to_emails: list[str] to_emails_from_variable: NotRequired[str] attachment_formats: NotRequired[list[AttachmentFormat | str]] payment_emails: NotRequired[list[str]] @@ -36,15 +37,17 @@ class EmailOptionsSerializer(JsonSchemaSerializerMixin, serializers.Serializer): to_emails = serializers.ListField( child=serializers.EmailField(), label=_("The email addresses to which the submission details will be sent"), - required=False, # Either to_emails or to_emails_from_variable should be required + # always required, even if using to_emails_from_variable because that may contain + # bad data/unexpectedly be empty due to reasons (failing prefill, for example) + required=True, ) - to_emails_from_variable = serializers.CharField( + to_emails_from_variable = FormioVariableKeyField( label=_("Key of the target variable containing the email address"), required=False, allow_blank=True, help_text=_( "Key of the target variable whose value will be used for the mailing. " - "When using this field, the mailing will only be send to this email address. " + "When using this field, the mailing will only be sent to this email address. " "The email addresses field would then be ignored. " ), ) diff --git a/src/openforms/registrations/contrib/email/plugin.py b/src/openforms/registrations/contrib/email/plugin.py index 92679273ab..eee1d091b4 100644 --- a/src/openforms/registrations/contrib/email/plugin.py +++ b/src/openforms/registrations/contrib/email/plugin.py @@ -1,10 +1,9 @@ import html +import logging from mimetypes import types_map from typing import Any from django.conf import settings -from django.core.exceptions import ValidationError -from django.core.validators import validate_email from django.urls import reverse from django.utils import timezone from django.utils.translation import get_language_info, gettext_lazy as _ @@ -38,38 +37,53 @@ from .models import EmailConfig from .utils import get_registration_email_templates +logger = logging.getLogger(__name__) + @register(PLUGIN_ID) class EmailRegistration(BasePlugin[Options]): verbose_name = _("Email registration") configuration_options = EmailOptionsSerializer - def get_recipients(self, submission: Submission, options: Options) -> list[str]: + @staticmethod + def get_recipients(submission: Submission, options: Options) -> list[str]: state = submission.load_submission_value_variables_state() - recipients = [] - - # If the 'recipients from variable' is used and it exists - if ( - (variable_key := options.get("to_emails_from_variable")) - and variable_key in state.variables - and (variable_value := state.variables[variable_key].value) - ): - # To simplify things, treat all variable values as lists - if type(variable_value) != list: - variable_value = [variable_value] + # ensure we have a fallback + recipients: list[str] = options["to_emails"] - # Only if all email addresses are valid, they will be used as recipients + # TODO: validate in the options that this key/variable exists, but we can't + # do that because variables get created *after* the registration options are + # submitted... + if variable_key := options.get("to_emails_from_variable"): try: - for value in variable_value: - validate_email(value) - except ValidationError: - pass + variable = state.get_variable(variable_key) + except KeyError: + logger.info( + "Variable %s does not exist in submission %r", + variable_key, + submission.uuid, + extra={ + "variable_key": variable_key, + "form": submission.form.uuid, + "submission": submission.uuid, + }, + ) else: - recipients = variable_value + if variable_value := variable.value: + # Normalize to a list of email addresses. Note that a form component + # could be used with multiple=True, then it will already be a list of + # values. + if not isinstance(variable_value, list): + variable_value = [variable_value] - # If the variable cannot be used, fallback to the general email addresses - if not recipients and "to_emails" in options: - recipients = options["to_emails"] + # do not validate that the values are emails, if they're wrong values, + # we want to see this in error monitoring. + recipients = variable_value + logger.info( + "Determined recipients from form variable %r: %r", + variable_key, + recipients, + ) return recipients diff --git a/src/openforms/registrations/contrib/email/tests/test_backend.py b/src/openforms/registrations/contrib/email/tests/test_backend.py index ceaee381c6..a49196020c 100644 --- a/src/openforms/registrations/contrib/email/tests/test_backend.py +++ b/src/openforms/registrations/contrib/email/tests/test_backend.py @@ -77,6 +77,8 @@ class EmailBackendTests(HTMLAssertMixin, TestCase): @classmethod def setUpTestData(cls): + super().setUpTestData() + fd = FormDefinitionFactory.create( configuration={ "components": [ @@ -107,10 +109,12 @@ def setUp(self): super().setUp() self.addCleanup(GlobalConfiguration.clear_cache) + self.addCleanup(EmailConfig.clear_cache) def test_submission_with_email_backend(self): submission = SubmissionFactory.from_components( completed=True, + with_public_registration_reference=True, completed_on=timezone.make_aware(datetime(2021, 1, 1, 12, 0, 0)), components_list=[ {"key": "foo", "type": "textfield", "label": "foo"}, @@ -170,12 +174,11 @@ def test_submission_with_email_backend(self): _component_configuration_path="components.3", _component_data_path="file2", ) - email_form_options = dict( - to_emails=["foo@bar.nl", "bar@foo.nl"], - ) - email_submission = EmailRegistration("email") - - set_submission_reference(submission) + email_form_options: Options = { + "to_emails": ["foo@bar.nl", "bar@foo.nl"], + "attach_files_to_email": None, + } + plugin = EmailRegistration("email") with patch( "openforms.registrations.contrib.email.utils.EmailConfig.get_solo", @@ -185,7 +188,7 @@ def test_submission_with_email_backend(self): content_text=TEST_TEMPLATE_NL, ), ): - email_submission.register_submission(submission, email_form_options) + plugin.register_submission(submission, email_form_options) # Verify that email was sent self.assertEqual(len(mail.outbox), 1) @@ -205,9 +208,7 @@ def test_submission_with_email_backend(self): self.assertIn(" Date: Mon, 30 Dec 2024 15:48:15 +0100 Subject: [PATCH 05/12] :label: [#4650] Enable type checking on entire registrations email package This should encourage people to pass the proper options and take into account optional/required configuration parameters. --- pyright.pyproject.toml | 3 +- .../registrations/contrib/email/models.py | 2 +- .../contrib/email/tests/test_backend.py | 205 +++++++++--------- .../registrations/contrib/email/views.py | 2 +- 4 files changed, 107 insertions(+), 105 deletions(-) diff --git a/pyright.pyproject.toml b/pyright.pyproject.toml index e69115a161..79555fabcf 100644 --- a/pyright.pyproject.toml +++ b/pyright.pyproject.toml @@ -36,8 +36,7 @@ include = [ "src/openforms/emails/templatetags/cosign_information.py", # Registrations "src/openforms/registrations/tasks.py", - "src/openforms/registrations/contrib/email/config.py", - "src/openforms/registrations/contrib/email/plugin.py", + "src/openforms/registrations/contrib/email/", "src/openforms/registrations/contrib/stuf_zds/options.py", "src/openforms/registrations/contrib/stuf_zds/plugin.py", "src/openforms/registrations/contrib/stuf_zds/typing.py", diff --git a/src/openforms/registrations/contrib/email/models.py b/src/openforms/registrations/contrib/email/models.py index 4097084594..0609972637 100644 --- a/src/openforms/registrations/contrib/email/models.py +++ b/src/openforms/registrations/contrib/email/models.py @@ -95,7 +95,7 @@ class EmailConfig(SingletonModel): ], ) - class Meta: + class Meta: # pyright: ignore[reportIncompatibleVariableOverride] verbose_name = _("Email registration configuration") def __str__(self): diff --git a/src/openforms/registrations/contrib/email/tests/test_backend.py b/src/openforms/registrations/contrib/email/tests/test_backend.py index a49196020c..15c3841adb 100644 --- a/src/openforms/registrations/contrib/email/tests/test_backend.py +++ b/src/openforms/registrations/contrib/email/tests/test_backend.py @@ -29,7 +29,6 @@ from openforms.submissions.attachments import attach_uploads_to_submission_step from openforms.submissions.exports import create_submission_export from openforms.submissions.models import Submission -from openforms.submissions.public_references import set_submission_reference from openforms.submissions.tests.factories import ( SubmissionFactory, SubmissionFileAttachmentFactory, @@ -69,6 +68,15 @@ """ +def _get_sent_email(index: int = 0) -> tuple[mail.EmailMultiAlternatives, str, str]: + message = mail.outbox[index] + assert isinstance(message, mail.EmailMultiAlternatives) + text_body = message.body + html_body = message.alternatives[0][0] + assert isinstance(html_body, str) + return message, str(text_body), html_body + + @override_settings( DEFAULT_FROM_EMAIL="info@open-forms.nl", BASE_URL="https://example.com", @@ -158,9 +166,12 @@ def test_submission_with_email_backend(self): }, language_code="nl", ) + step = ( + submission.submissionstep_set.get() # pyright: ignore[reportAttributeAccessIssue] + ) submission_file_attachment_1 = SubmissionFileAttachmentFactory.create( form_key="file1", - submission_step=submission.submissionstep_set.get(), + submission_step=step, file_name="my-foo.bin", content_type="application/foo", _component_configuration_path="components.2", @@ -168,7 +179,7 @@ def test_submission_with_email_backend(self): ) submission_file_attachment_2 = SubmissionFileAttachmentFactory.create( form_key="file2", - submission_step=submission.submissionstep_set.get(), + submission_step=step, file_name="my-bar.txt", content_type="text/bar", _component_configuration_path="components.3", @@ -193,7 +204,7 @@ def test_submission_with_email_backend(self): # Verify that email was sent self.assertEqual(len(mail.outbox), 1) - message = mail.outbox[0] + message, message_text, message_html = _get_sent_email() self.assertEqual( message.subject, f"Subject: MyName - submission {submission.public_registration_reference}", @@ -202,8 +213,6 @@ def test_submission_with_email_backend(self): self.assertEqual(message.to, ["foo@bar.nl", "bar@foo.nl"]) # Check that the template is used - message_text = message.body - message_html = message.alternatives[0][0] self.assertHTMLValid(message_html) self.assertIn("
  • Backend
  • Frontend
    • ", message_html) @patch("openforms.registrations.contrib.email.plugin.EmailConfig.get_solo") @@ -1006,15 +1002,18 @@ def test_with_global_config_attach_files(self, mock_get_solo): form__internal_name="MyInternalName", form__registration_backend="email", ) + step = ( + submission.submissionstep_set.get() # pyright: ignore[reportAttributeAccessIssue] + ) SubmissionFileAttachmentFactory.create( form_key="file1", - submission_step=submission.submissionstep_set.get(), + submission_step=step, file_name="my-foo.bin", content_type="application/foo", ) SubmissionFileAttachmentFactory.create( form_key="file2", - submission_step=submission.submissionstep_set.get(), + submission_step=step, file_name="my-bar.txt", content_type="text/bar", ) @@ -1071,7 +1070,10 @@ def test_user_defined_variables_included(self): value="test2", ) - email_form_options = dict(to_emails=["foo@bar.nl", "bar@foo.nl"]) + email_form_options: Options = { + "to_emails": ["foo@bar.nl", "bar@foo.nl"], + "attach_files_to_email": None, + } plugin = EmailRegistration("email") plugin.register_submission(submission, email_form_options) @@ -1079,9 +1081,7 @@ def test_user_defined_variables_included(self): # Verify that email was sent self.assertEqual(len(mail.outbox), 1) - message = mail.outbox[0] - message_text = message.body - + _, message_text, _ = _get_sent_email() self.assertIn("User defined var 1: test1", message_text) self.assertIn("User defined var 2: test2", message_text) @@ -1104,11 +1104,11 @@ def test_mime_body_parts_have_content_langauge(self): ): plugin.register_submission(submission, {"to_emails": ["foo@example.com"]}) - message = mail.outbox[0] + message, message_text, message_html = _get_sent_email() self.assertEqual(message.extra_headers["Content-Language"], "en") - self.assertIn("Engels", message.body) + self.assertIn("Engels", message_text) html_message = message.alternatives[0][0] - self.assertIn("Engels", html_message) + self.assertIn("Engels", message_html) @tag("gh-3144") def test_file_attachments_in_registration_email(self): @@ -1204,8 +1204,10 @@ def test_file_attachments_in_registration_email(self): ) attach_uploads_to_submission_step(submission_step) - subject, body_html, body_text = EmailRegistration.render_registration_email( - submission, is_payment_update=False + subject, body_html, body_text = ( + EmailRegistration.render_registration_email( # pyright: ignore[reportAttributeAccessIssue] + submission, is_payment_update=False + ) ) with self.subTest("Normal attachment"): @@ -1222,9 +1224,10 @@ def test_file_attachments_in_registration_email(self): def test_extra_headers(self): submission = SubmissionFactory.create() - email_form_options = dict( - to_emails=["foo@bar.nl", "bar@foo.nl"], - ) + email_form_options: Options = { + "to_emails": ["foo@bar.nl", "bar@foo.nl"], + "attach_files_to_email": None, + } plugin = EmailRegistration("email") with patch( diff --git a/src/openforms/registrations/contrib/email/views.py b/src/openforms/registrations/contrib/email/views.py index 1cc6b119a6..6639dcfd37 100644 --- a/src/openforms/registrations/contrib/email/views.py +++ b/src/openforms/registrations/contrib/email/views.py @@ -16,7 +16,7 @@ def get_email_content(self): subject, html_content, text_content, - ) = EmailRegistration.render_registration_email( + ) = EmailRegistration.render_registration_email( # pyright: ignore[reportAttributeAccessIssue] self.object, is_payment_update=False ) content = html_content if mode == "html" else text_content From 89682523f3b74d22046408dec5ec7598c7328a86 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 30 Dec 2024 16:11:55 +0100 Subject: [PATCH 06/12] :art: [#4650] Clean up email tests Simplified tests by removing indirection and only set up the test data that is actually required by the test. This also uses some more ergonomic factory utilities rather than hand-wiring everything. Now new tests should have better examples to follow. --- .../contrib/email/tests/test_backend.py | 108 ++++++++---------- 1 file changed, 47 insertions(+), 61 deletions(-) diff --git a/src/openforms/registrations/contrib/email/tests/test_backend.py b/src/openforms/registrations/contrib/email/tests/test_backend.py index 15c3841adb..4f73b526d8 100644 --- a/src/openforms/registrations/contrib/email/tests/test_backend.py +++ b/src/openforms/registrations/contrib/email/tests/test_backend.py @@ -19,11 +19,6 @@ EmailContentTypeChoices, EmailEventChoices, ) -from openforms.forms.tests.factories import ( - FormDefinitionFactory, - FormFactory, - FormStepFactory, -) from openforms.payments.constants import PaymentStatus from openforms.payments.tests.factories import SubmissionPaymentFactory from openforms.submissions.attachments import attach_uploads_to_submission_step @@ -32,7 +27,6 @@ from openforms.submissions.tests.factories import ( SubmissionFactory, SubmissionFileAttachmentFactory, - SubmissionReportFactory, SubmissionStepFactory, SubmissionValueVariableFactory, TemporaryFileUploadFactory, @@ -83,35 +77,6 @@ def _get_sent_email(index: int = 0) -> tuple[mail.EmailMultiAlternatives, str, s LANGUAGE_CODE="nl", ) class EmailBackendTests(HTMLAssertMixin, TestCase): - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - fd = FormDefinitionFactory.create( - configuration={ - "components": [ - { - "key": "someField", - "label": "Some Field", - "type": "textfield", - }, - { - "key": "someList", - "label": "Some list", - "type": "textfield", - "multiple": True, - }, - ], - } - ) - form = FormFactory.create( - name="MyName", - internal_name="MyInternalName", - registration_backend="email", - ) - cls.fs = FormStepFactory.create(form=form, form_definition=fd) - cls.form = form - cls.fd = fd def setUp(self): super().setUp() @@ -217,7 +182,7 @@ def test_submission_with_email_backend(self): self.assertIn(" Date: Mon, 30 Dec 2024 16:15:43 +0100 Subject: [PATCH 07/12] :art: [#4650] Revert generic test changes The test module is aimed at the generic registration mechanism and just 'happens' to use the email plugin. We do not need to repeat all the various cases/failure modes that are already covered by the plugin unit tests. --- .../tests/test_registration_hook.py | 44 +------------------ 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/src/openforms/registrations/tests/test_registration_hook.py b/src/openforms/registrations/tests/test_registration_hook.py index 201130f7c5..4fd3eba8c8 100644 --- a/src/openforms/registrations/tests/test_registration_hook.py +++ b/src/openforms/registrations/tests/test_registration_hook.py @@ -312,7 +312,7 @@ def test_registration_backend_invalid_options(self): completed=True, form__registration_backend="email", form__registration_backend_options={}, - ) # Missing "to_emails" and "to_emails_from_variable" option + ) # Missing "to_emails" option with ( self.subTest("On completion - does NOT raise"), @@ -334,48 +334,6 @@ def test_registration_backend_invalid_options(self): ): register_submission(submission.id, PostSubmissionEvents.on_retry) - def test_email_registration_backend_with_to_emails(self): - submission = SubmissionFactory.create( - completed=True, - form__registration_backend="email", - form__registration_backend_options={"to_emails": ["foo@example.com"]}, - ) - - with ( - self.subTest("On completion - logs as successful"), - self.assertLogs(level="INFO") as logs, - ): - register_submission(submission.id, PostSubmissionEvents.on_completion) - - submission.refresh_from_db() - self.assertIn( - "Registration using plugin '%r' for submission '%s' succeeded", - logs.records[-1].msg, - ) - self.assertFalse(submission.needs_on_completion_retry) - - def test_email_registration_backend_with_to_emails_from_variable(self): - submission = SubmissionFactory.create( - completed=True, - form__registration_backend="email", - form__registration_backend_options={ - "to_emails_from_variable": "email_example_variable" - }, - ) - - with ( - self.subTest("On completion - logs as successful"), - self.assertLogs(level="INFO") as logs, - ): - register_submission(submission.id, PostSubmissionEvents.on_completion) - - submission.refresh_from_db() - self.assertIn( - "Registration using plugin '%r' for submission '%s' succeeded", - logs.records[-1].msg, - ) - self.assertFalse(submission.needs_on_completion_retry) - def test_calling_registration_task_with_serialized_args(self): submission = SubmissionFactory.create( completed=True, From 77e2ddab1388701ad550c5a4c6fce3a98cd3394c Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 30 Dec 2024 16:40:41 +0100 Subject: [PATCH 08/12] :sparkles: [#4650] Improve email registration plugin options form * Added fieldsets to group related options together * Use a shorter form field label and describe the behaviour in the help text * Filter down the available variables to compatible data types (only strings and arrays can produce valid values) * Updated stories to provide some available variables in the context * Mark toEmails configuration field as required, since it always is * Clean up the formik hook/props usage * Ensure that the 'empty' value is normalized when calling the onChange behaviour to normalize formik data types to what the backend expects --- .../form_design/RegistrationFields.stories.js | 67 +++++++++++++++++++ .../registrations/email/EmailOptionsForm.js | 1 + .../email/EmailOptionsFormFields.js | 19 +++++- .../email/fields/EmailRecipients.js | 1 + .../fields/EmailRecipientsFromVariable.js | 21 +++--- src/openforms/js/lang/en.json | 15 +++++ src/openforms/js/lang/nl.json | 17 ++++- .../registrations/contrib/email/config.py | 14 ---- .../contrib/email/tests/test_backend.py | 23 +++++++ 9 files changed, 152 insertions(+), 26 deletions(-) diff --git a/src/openforms/js/components/admin/form_design/RegistrationFields.stories.js b/src/openforms/js/components/admin/form_design/RegistrationFields.stories.js index a35a3378e5..86fbd85795 100644 --- a/src/openforms/js/components/admin/form_design/RegistrationFields.stories.js +++ b/src/openforms/js/components/admin/form_design/RegistrationFields.stories.js @@ -403,6 +403,73 @@ export default { label: 'textfield2', }, }, + availableFormVariables: [ + { + dataFormat: '', + dataType: 'string', + form: 'http://localhost:8000/api/v2/forms/ae26e20c-f059-4fdf-bb82-afc377869bb5', + formDefinition: null, + initialValue: '', + isSensitiveData: false, + key: 'textField1', + name: 'textfield1', + prefillAttribute: '', + prefillPlugin: '', + source: 'component', + }, + { + dataFormat: '', + dataType: 'string', + form: 'http://localhost:8000/api/v2/forms/ae26e20c-f059-4fdf-bb82-afc377869bb5', + formDefinition: null, + initialValue: '', + isSensitiveData: false, + key: 'textField2', + name: 'textfield2', + prefillAttribute: '', + prefillPlugin: '', + source: 'component', + }, + { + dataFormat: '', + dataType: 'string', + form: 'http://localhost:8000/api/v2/forms/ae26e20c-f059-4fdf-bb82-afc377869bb5', + formDefinition: null, + initialValue: '', + isSensitiveData: false, + key: 'userDefinedVar1', + name: 'User defined string', + prefillAttribute: '', + prefillPlugin: '', + source: 'user_defined', + }, + { + dataFormat: '', + dataType: 'array', + form: 'http://localhost:8000/api/v2/forms/ae26e20c-f059-4fdf-bb82-afc377869bb5', + formDefinition: null, + initialValue: [], + isSensitiveData: false, + key: 'userDefinedVar2', + name: 'User defined array', + prefillAttribute: '', + prefillPlugin: '', + source: 'user_defined', + }, + { + dataFormat: '', + dataType: 'float', + form: 'http://localhost:8000/api/v2/forms/ae26e20c-f059-4fdf-bb82-afc377869bb5', + formDefinition: null, + initialValue: null, + isSensitiveData: false, + key: 'userDefinedVar3', + name: 'User defined float', + prefillAttribute: '', + prefillPlugin: '', + source: 'user_defined', + }, + ], registrationPluginsVariables: [ { pluginIdentifier: 'stuf-zds-create-zaak', diff --git a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js index 3871a0c8d8..ae75d34e5b 100644 --- a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js +++ b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsForm.js @@ -23,6 +23,7 @@ const EmailOptionsForm = ({name, label, schema, formData, onChange}) => { /> } initialFormData={{ + toEmailsFromVariable: '', // ensure an initial value is provided ...formData, // ensure we have a blank row initially toEmails: formData.toEmails?.length ? formData.toEmails : [''], diff --git a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js index d6692f05b3..19c7bcae38 100644 --- a/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js +++ b/src/openforms/js/components/admin/form_design/registrations/email/EmailOptionsFormFields.js @@ -36,9 +36,26 @@ const EmailOptionsFormFields = ({name, schema}) => { const relevantErrors = filterErrors(name, validationErrors); return ( -
    +
    + } + > +
    + +
    + } + > diff --git a/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipients.js b/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipients.js index 7c2ad6c6cf..ff7fcc77ee 100644 --- a/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipients.js +++ b/src/openforms/js/components/admin/form_design/registrations/email/fields/EmailRecipients.js @@ -19,6 +19,7 @@ const EmailRecipients = () => { defaultMessage="The email addresses to which the submission details will be sent" /> } + required > { - const [fieldProps, , fieldHelpers] = useField('toEmailsFromVariable'); - const {setValue} = fieldHelpers; + const [fieldProps, , {setValue}] = useField('toEmailsFromVariable'); return ( { label={ } helpText={ } > { - setValue(event.target.value); + const newValue = event.target.value; + setValue(newValue == null ? '' : newValue); }} + filter={variable => ['string', 'array'].includes(variable.dataType)} /> diff --git a/src/openforms/js/lang/en.json b/src/openforms/js/lang/en.json index 6542ac71fb..c2f7609ada 100644 --- a/src/openforms/js/lang/en.json +++ b/src/openforms/js/lang/en.json @@ -1034,6 +1034,11 @@ "description": "Objects API prefill mappings fieldset title", "originalDefault": "Mappings" }, + "Igt0Rc": { + "defaultMessage": "Skip ownership check", + "description": "Objects API registration: skipOwnershipCheck label", + "originalDefault": "Skip ownership check" + }, "IhIqdj": { "defaultMessage": "Decision definition ID", "description": "Decision definition ID label", @@ -1564,6 +1569,11 @@ "description": "Text on button to open design token editor in modal.", "originalDefault": "Open editor" }, + "T2TGaF": { + "defaultMessage": "Ownership checks", + "description": "Objects API ownership check fieldset title", + "originalDefault": "Ownership checks" + }, "TB/ku3": { "defaultMessage": "Add variable", "description": "Add user defined variable button label", @@ -2729,6 +2739,11 @@ "description": "Errored Submissions Removal Method help text", "originalDefault": "How errored submissions of this form will be removed after the limit. Leave blank to use value in General Configuration." }, + "qdV9zh": { + "defaultMessage": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!", + "description": "Objects API registration: skipOwnershipCheck helpText", + "originalDefault": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!" + }, "qo1UbS": { "defaultMessage": "Export", "description": "Export form button", diff --git a/src/openforms/js/lang/nl.json b/src/openforms/js/lang/nl.json index 697caa3cfe..6e4fa49621 100644 --- a/src/openforms/js/lang/nl.json +++ b/src/openforms/js/lang/nl.json @@ -1043,6 +1043,11 @@ "description": "Objects API prefill mappings fieldset title", "originalDefault": "Mappings" }, + "Igt0Rc": { + "defaultMessage": "Skip ownership check", + "description": "Objects API registration: skipOwnershipCheck label", + "originalDefault": "Skip ownership check" + }, "IhIqdj": { "defaultMessage": "Beslisdefinitie-ID", "description": "Decision definition ID label", @@ -1229,7 +1234,7 @@ "originalDefault": "Something went wrong while retrieving the available products defined in the selected case. Please check that the services in the selected API group are configured correctly." }, "LeVpdf": { - "defaultMessage": "Plugin-insellingen: e-mail", + "defaultMessage": "Plugin-instellingen: e-mail", "description": "Email registration options modal title", "originalDefault": "Plugin configuration: Email" }, @@ -1579,6 +1584,11 @@ "description": "Text on button to open design token editor in modal.", "originalDefault": "Open editor" }, + "T2TGaF": { + "defaultMessage": "Ownership checks", + "description": "Objects API ownership check fieldset title", + "originalDefault": "Ownership checks" + }, "TB/ku3": { "defaultMessage": "Variabele toevoegen", "description": "Add user defined variable button label", @@ -2750,6 +2760,11 @@ "description": "Errored Submissions Removal Method help text", "originalDefault": "How errored submissions of this form will be removed after the limit. Leave blank to use value in General Configuration." }, + "qdV9zh": { + "defaultMessage": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!", + "description": "Objects API registration: skipOwnershipCheck helpText", + "originalDefault": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!" + }, "qo1UbS": { "defaultMessage": "Exporteren", "description": "Export form button", diff --git a/src/openforms/registrations/contrib/email/config.py b/src/openforms/registrations/contrib/email/config.py index da4cf33784..2fb4ca3a5a 100644 --- a/src/openforms/registrations/contrib/email/config.py +++ b/src/openforms/registrations/contrib/email/config.py @@ -131,20 +131,6 @@ class Meta: ), ] - def validate(self, attrs: Options) -> Options: - # The email registration requires either `to_emails` or `to_emails_from_variable` - # to determine which email address to use. - # Both may be set - in that case, `to_emails_from_variable` is preferred. - if not attrs.get("to_emails") and not attrs.get("to_emails_from_variable"): - raise serializers.ValidationError( - { - "to_emails": _("This field is required."), - }, - code="required", - ) - - return attrs - # sanity check for development - keep serializer and type definitions in sync _serializer_fields = EmailOptionsSerializer._declared_fields.keys() diff --git a/src/openforms/registrations/contrib/email/tests/test_backend.py b/src/openforms/registrations/contrib/email/tests/test_backend.py index 4f73b526d8..47460b0de4 100644 --- a/src/openforms/registrations/contrib/email/tests/test_backend.py +++ b/src/openforms/registrations/contrib/email/tests/test_backend.py @@ -344,6 +344,29 @@ def test_submission_with_email_backend_invalid_to_emails_from_variable(self): message = mail.outbox[0] self.assertEqual(message.to, ["foo.com"]) + def test_submission_with_email_backend_empty_to_emails_from_variable(self): + submission = SubmissionFactory.from_components( + completed=True, + with_public_registration_reference=True, + components_list=[{"key": "recipient", "type": "email", "label": "foo"}], + submitted_data={"recipient": ""}, + form__registration_backend="email", + ) + email_form_options: Options = { + "to_emails": ["fallback@example.com"], + "to_emails_from_variable": "recipient", + "attach_files_to_email": None, + } + plugin = EmailRegistration("email") + + plugin.register_submission(submission, email_form_options) + + # Verify that email was queued - it will fail to deliver though and that will + # be visible in error monitoring. + self.assertEqual(len(mail.outbox), 1) + message = mail.outbox[0] + self.assertEqual(message.to, ["fallback@example.com"]) + def test_submission_with_email_backend_strip_out_urls(self): config = GlobalConfiguration.get_solo() config.email_template_netloc_allowlist = ( # pyright: ignore[reportAttributeAccessIssue] From bb61f2990394b894af10d92f547d55d50a28226b Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 30 Dec 2024 16:59:27 +0100 Subject: [PATCH 09/12] :globe_with_meridians: [#4650] Update Dutch translations --- src/openforms/js/compiled-lang/en.json | 54 +++++++++++++++++++------ src/openforms/js/compiled-lang/nl.json | 56 ++++++++++++++++++++------ src/openforms/js/lang/en.json | 30 +++++++++----- src/openforms/js/lang/nl.json | 36 +++++++++++------ 4 files changed, 128 insertions(+), 48 deletions(-) diff --git a/src/openforms/js/compiled-lang/en.json b/src/openforms/js/compiled-lang/en.json index 09ce4adf38..e95e181f36 100644 --- a/src/openforms/js/compiled-lang/en.json +++ b/src/openforms/js/compiled-lang/en.json @@ -1273,6 +1273,12 @@ "value": "Minimum value" } ], + "BHr/3M": [ + { + "type": 0, + "value": "If specified, the recipient email addresses will be taken from the selected variable. You must still specify 'regular' email addresses as a fallback, in case something is wrong with the variable." + } + ], "BKTOtD": [ { "offset": 0, @@ -1873,6 +1879,12 @@ "value": "The co-sign component requires at least one authentication plugin to be enabled." } ], + "FNT8nq": [ + { + "type": 0, + "value": "Variable containing email addresses" + } + ], "FOlQaP": [ { "type": 0, @@ -2233,6 +2245,12 @@ "value": " is unknown. We can only display the JSON definition." } ], + "Igt0Rc": [ + { + "type": 0, + "value": "Skip ownership check" + } + ], "IhIqdj": [ { "type": 0, @@ -3251,6 +3269,12 @@ "value": "Open editor" } ], + "T2TGaF": [ + { + "type": 0, + "value": "Ownership checks" + } + ], "T2dCIS": [ { "type": 0, @@ -5457,12 +5481,6 @@ "value": "the variable" } ], - "mOh/pF": [ - { - "type": 0, - "value": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." - } - ], "mOjKcm": [ { "type": 0, @@ -5577,6 +5595,12 @@ "value": "The registration result will be an object from the selected type." } ], + "nhBYT3": [ + { + "type": 0, + "value": "Recipients" + } + ], "nkr7r0": [ { "type": 0, @@ -5895,6 +5919,12 @@ "value": "How errored submissions of this form will be removed after the limit. Leave blank to use value in General Configuration." } ], + "qdV9zh": [ + { + "type": 0, + "value": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!" + } + ], "qmBAmr": [ { "type": 0, @@ -6279,12 +6309,6 @@ "value": "Do not display the configured label and top line as the header in the fieldset." } ], - "uwCD8e": [ - { - "type": 0, - "value": "Using a variable to decide to which email address the submission details will be sent" - } - ], "vAZDY4": [ { "type": 0, @@ -6309,6 +6333,12 @@ "value": "Value" } ], + "vaO0I+": [ + { + "type": 0, + "value": "Content" + } + ], "ve3rsH": [ { "type": 0, diff --git a/src/openforms/js/compiled-lang/nl.json b/src/openforms/js/compiled-lang/nl.json index 95992ff191..589bfa700a 100644 --- a/src/openforms/js/compiled-lang/nl.json +++ b/src/openforms/js/compiled-lang/nl.json @@ -1277,6 +1277,12 @@ "value": "Minimale waarde" } ], + "BHr/3M": [ + { + "type": 0, + "value": "Als een variable geselecteerd is, dan wordt de waarde gebruikt als e-mailadres(sen) van de ontvangers. Je moet nog steeds een vast adres opgeven als terugvaloptie voor het geval er iets fout gaat met de variabele." + } + ], "BKTOtD": [ { "offset": 0, @@ -1894,6 +1900,12 @@ "value": "Er moet minstens één authenticatiemethode ingeschakeld zijn voor de mede-ondertekencomponent." } ], + "FNT8nq": [ + { + "type": 0, + "value": "Variabele die adres(sen) bevat" + } + ], "FOlQaP": [ { "type": 0, @@ -2254,6 +2266,12 @@ "value": " is niet bekend. We kunnen enkel de JSON-definitie weergeven." } ], + "Igt0Rc": [ + { + "type": 0, + "value": "Sla eigenaarcontrole over" + } + ], "IhIqdj": [ { "type": 0, @@ -2597,7 +2615,7 @@ "LeVpdf": [ { "type": 0, - "value": "Plugin-insellingen: e-mail" + "value": "Plugin-instellingen: e-mail" } ], "LiXrER": [ @@ -3264,6 +3282,12 @@ "value": "Editor openen" } ], + "T2TGaF": [ + { + "type": 0, + "value": "Eigenaarcontroles" + } + ], "T2dCIS": [ { "type": 0, @@ -5475,12 +5499,6 @@ "value": "de variabele" } ], - "mOh/pF": [ - { - "type": 0, - "value": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." - } - ], "mOjKcm": [ { "type": 0, @@ -5595,6 +5613,12 @@ "value": "Het registratieresultaat zal een object van dit type zijn." } ], + "nhBYT3": [ + { + "type": 0, + "value": "Ontvangers" + } + ], "nkr7r0": [ { "type": 0, @@ -5913,6 +5937,12 @@ "value": "Geeft aan hoe niet voltooide inzendingen (door fouten in de afhandeling) worden opgeschoond na de bewaartermijn. Laat leeg om de waarde van de algemene configuratie te gebruiken." } ], + "qdV9zh": [ + { + "type": 0, + "value": "Indien aangevinkt, dan worden geen controles uitgevoerd of de (ingelogde) gebruiker toegang heeft op het gerefereerde object. Valideer dat de objecten geen privacy-gevoelige gegevens bevatten voor je dit aanvinkt!" + } + ], "qmBAmr": [ { "type": 0, @@ -6297,12 +6327,6 @@ "value": "Verberg de koptekst en de lijn boven de veldengroep in het formulier." } ], - "uwCD8e": [ - { - "type": 0, - "value": "Using a variable to decide to which email address the submission details will be sent" - } - ], "vAZDY4": [ { "type": 0, @@ -6327,6 +6351,12 @@ "value": "(Standaard)tekst" } ], + "vaO0I+": [ + { + "type": 0, + "value": "E-mailinhoud" + } + ], "ve3rsH": [ { "type": 0, diff --git a/src/openforms/js/lang/en.json b/src/openforms/js/lang/en.json index c2f7609ada..07ff822d2b 100644 --- a/src/openforms/js/lang/en.json +++ b/src/openforms/js/lang/en.json @@ -559,6 +559,11 @@ "description": "Email registration options 'emailPaymentSubject' label", "originalDefault": "Email payment subject" }, + "BHr/3M": { + "defaultMessage": "If specified, the recipient email addresses will be taken from the selected variable. You must still specify 'regular' email addresses as a fallback, in case something is wrong with the variable.", + "description": "Email registration options 'toEmailsFromVariable' helpText", + "originalDefault": "If specified, the recipient email addresses will be taken from the selected variable. You must still specify 'regular' email addresses as a fallback, in case something is wrong with the variable." + }, "BKTOtD": { "defaultMessage": "{varCount, plural, =0 {} one {(1 variable mapped)} other {({varCount} variables mapped)} }", "description": "Managed Camunda process vars state feedback", @@ -814,6 +819,11 @@ "description": "MissingAuthCosignWarning message", "originalDefault": "The co-sign component requires at least one authentication plugin to be enabled." }, + "FNT8nq": { + "defaultMessage": "Variable containing email addresses", + "description": "Email registration options 'toEmailsFromVariable' label", + "originalDefault": "Variable containing email addresses" + }, "FQ1JZc": { "defaultMessage": "Zaaktype code for newly created Zaken in StUF-ZDS", "description": "StUF-ZDS registration options 'zdsZaaktypeCode' helpText", @@ -2534,11 +2544,6 @@ "description": "\"variable\" operand type", "originalDefault": "the variable" }, - "mOh/pF": { - "defaultMessage": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option.", - "description": "Email registration options 'toEmailsFromVariable' helpText", - "originalDefault": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." - }, "mOjKcm": { "defaultMessage": "Steps and fields", "description": "Form design tab title", @@ -2594,6 +2599,11 @@ "description": "Objects API registration options 'Objecttype' helpText", "originalDefault": "The registration result will be an object from the selected type." }, + "nhBYT3": { + "defaultMessage": "Recipients", + "description": "Email registration: recipients fieldset title", + "originalDefault": "Recipients" + }, "nkr7r0": { "defaultMessage": "Amount of days when all submissions of this form will be permanently deleted. Leave blank to use value in General Configuration.", "description": "All Submissions Removal Limit help text", @@ -2934,11 +2944,6 @@ "description": "History link button", "originalDefault": "History" }, - "uwCD8e": { - "defaultMessage": "Using a variable to decide to which email address the submission details will be sent", - "description": "Email registration options 'toEmailsFromVariable' label", - "originalDefault": "Using a variable to decide to which email address the submission details will be sent" - }, "vAZDY4": { "defaultMessage": "Source path", "description": "Prefill / Objects API: column header for object type property selection", @@ -2954,6 +2959,11 @@ "description": "Create form definition tile", "originalDefault": "Create a new form definition" }, + "vaO0I+": { + "defaultMessage": "Content", + "description": "Email registration: content fieldset title", + "originalDefault": "Content" + }, "viEfGU": { "defaultMessage": "The content of the submission confirmation page. It can contain variables that will be templated from the submitted form data. If not specified, the global template will be used.", "description": "Confirmation template help text", diff --git a/src/openforms/js/lang/nl.json b/src/openforms/js/lang/nl.json index 6e4fa49621..9341ee7a7b 100644 --- a/src/openforms/js/lang/nl.json +++ b/src/openforms/js/lang/nl.json @@ -564,6 +564,11 @@ "description": "Email registration options 'emailPaymentSubject' label", "originalDefault": "Email payment subject" }, + "BHr/3M": { + "defaultMessage": "Als een variable geselecteerd is, dan wordt de waarde gebruikt als e-mailadres(sen) van de ontvangers. Je moet nog steeds een vast adres opgeven als terugvaloptie voor het geval er iets fout gaat met de variabele.", + "description": "Email registration options 'toEmailsFromVariable' helpText", + "originalDefault": "If specified, the recipient email addresses will be taken from the selected variable. You must still specify 'regular' email addresses as a fallback, in case something is wrong with the variable." + }, "BKTOtD": { "defaultMessage": "{varCount, plural, =0 {} one {(1 variabele gekoppeld)} other {({varCount} variabelen gekoppeld)} }", "description": "Managed Camunda process vars state feedback", @@ -823,6 +828,11 @@ "description": "MissingAuthCosignWarning message", "originalDefault": "The co-sign component requires at least one authentication plugin to be enabled." }, + "FNT8nq": { + "defaultMessage": "Variabele die adres(sen) bevat", + "description": "Email registration options 'toEmailsFromVariable' label", + "originalDefault": "Variable containing email addresses" + }, "FQ1JZc": { "defaultMessage": "Zaaktypecode waarmee de nieuwe zaak aangemaakt wordt.", "description": "StUF-ZDS registration options 'zdsZaaktypeCode' helpText", @@ -1044,7 +1054,7 @@ "originalDefault": "Mappings" }, "Igt0Rc": { - "defaultMessage": "Skip ownership check", + "defaultMessage": "Sla eigenaarcontrole over", "description": "Objects API registration: skipOwnershipCheck label", "originalDefault": "Skip ownership check" }, @@ -1585,7 +1595,7 @@ "originalDefault": "Open editor" }, "T2TGaF": { - "defaultMessage": "Ownership checks", + "defaultMessage": "Eigenaarcontroles", "description": "Objects API ownership check fieldset title", "originalDefault": "Ownership checks" }, @@ -2555,11 +2565,6 @@ "description": "\"variable\" operand type", "originalDefault": "the variable" }, - "mOh/pF": { - "defaultMessage": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option.", - "description": "Email registration options 'toEmailsFromVariable' helpText", - "originalDefault": "The email address described in this variable will be used for the mailing. If a variable is selected, the general registration addresses will be used as fallback option." - }, "mOjKcm": { "defaultMessage": "Stappen en velden", "description": "Form design tab title", @@ -2615,6 +2620,11 @@ "description": "Objects API registration options 'Objecttype' helpText", "originalDefault": "The registration result will be an object from the selected type." }, + "nhBYT3": { + "defaultMessage": "Ontvangers", + "description": "Email registration: recipients fieldset title", + "originalDefault": "Recipients" + }, "nkr7r0": { "defaultMessage": "Aantal dagen dat een inzending bewaard blijft voordat deze definitief verwijderd wordt. Laat leeg om de waarde van de algemene configuratie te gebruiken.", "description": "All Submissions Removal Limit help text", @@ -2761,7 +2771,7 @@ "originalDefault": "How errored submissions of this form will be removed after the limit. Leave blank to use value in General Configuration." }, "qdV9zh": { - "defaultMessage": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!", + "defaultMessage": "Indien aangevinkt, dan worden geen controles uitgevoerd of de (ingelogde) gebruiker toegang heeft op het gerefereerde object. Valideer dat de objecten geen privacy-gevoelige gegevens bevatten voor je dit aanvinkt!", "description": "Objects API registration: skipOwnershipCheck helpText", "originalDefault": "If enabled, then no access control on the referenced object is performed. Ensure that it does not contain private data before checking this!" }, @@ -2955,11 +2965,6 @@ "description": "History link button", "originalDefault": "History" }, - "uwCD8e": { - "defaultMessage": "Using a variable to decide to which email address the submission details will be sent", - "description": "Email registration options 'toEmailsFromVariable' label", - "originalDefault": "Using a variable to decide to which email address the submission details will be sent" - }, "vAZDY4": { "defaultMessage": "Bronpad", "description": "Prefill / Objects API: column header for object type property selection", @@ -2975,6 +2980,11 @@ "description": "Create form definition tile", "originalDefault": "Create a new form definition" }, + "vaO0I+": { + "defaultMessage": "E-mailinhoud", + "description": "Email registration: content fieldset title", + "originalDefault": "Content" + }, "viEfGU": { "defaultMessage": "De inhoud van bevestigingspagina na het versturen van de inzending. Dit sjabloon mag variabelen bevatten met inzendings-gegevens. Laat dit veld leeg om de standaardinstelling te gebruiken.", "description": "Confirmation template help text", From 76436ade48741a1bc021fdc4cb08efe03db2e208 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 30 Dec 2024 17:40:49 +0100 Subject: [PATCH 10/12] :sparkles: [#4650] Support optional ownership checks in objects API prefill With product prefill, the premise is that the product data is personal and likely contains sensitive/private data that gives only the person with a particular BSN access. This requires the person to authenticate and then their BSN is compared with some property in the retrieved object. However, there may be 'public' data used to prefill some variables, either user defined or even form variables. Multiple people can use those objects, even without authenticating in the first place. You can now flag a prefill to not require this kind of ownership check. --- .../contrib/objects_api/api/serializers.py | 31 +++++++++++++++++-- .../prefill/contrib/objects_api/plugin.py | 4 +++ .../prefill/contrib/objects_api/typing.py | 1 + 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/openforms/prefill/contrib/objects_api/api/serializers.py b/src/openforms/prefill/contrib/objects_api/api/serializers.py index 5470fdb307..6195409730 100644 --- a/src/openforms/prefill/contrib/objects_api/api/serializers.py +++ b/src/openforms/prefill/contrib/objects_api/api/serializers.py @@ -8,6 +8,8 @@ from openforms.formio.api.fields import FormioVariableKeyField from openforms.utils.mixins import JsonSchemaSerializerMixin +from ..typing import ObjectsAPIOptions + class PrefillTargetPathsSerializer(serializers.Serializer): target_path = serializers.ListField( @@ -63,6 +65,14 @@ class ObjectsAPIOptionsSerializer(JsonSchemaSerializerMixin, serializers.Seriali required=True, help_text=_("Version of the objecttype in the Objecttypes API."), ) + skip_ownership_check = serializers.BooleanField( + label=_("skip ownership check"), + help_text=_( + "If enabled, no authentication/ownership checks of the referenced object " + "are performed." + ), + default=False, + ) auth_attribute_path = serializers.ListField( child=serializers.CharField(label=_("Segment of a JSON path")), label=_("Path to auth attribute (e.g. BSN/KVK) in objects"), @@ -70,11 +80,28 @@ class ObjectsAPIOptionsSerializer(JsonSchemaSerializerMixin, serializers.Seriali "This is used to perform validation to verify that the authenticated " "user is the owner of the object." ), - allow_empty=False, - required=True, + required=False, + default=list, + allow_empty=True, ) variables_mapping = ObjecttypeVariableMappingSerializer( label=_("variables mapping"), many=True, required=True, ) + + def validate(self, attrs: ObjectsAPIOptions) -> ObjectsAPIOptions: + # ensure that an auth_attribute_path is specified when ownership checks are + # not skipped. + if not attrs["skip_ownership_check"] and not attrs["auth_attribute_path"]: + raise serializers.ValidationError( + { + "auth_attribute_path": _( + "You must specify which attribute to compare the authenticated " + "user identifier with." + ), + }, + code="empty", + ) + + return attrs diff --git a/src/openforms/prefill/contrib/objects_api/plugin.py b/src/openforms/prefill/contrib/objects_api/plugin.py index ee710fd688..102701afe3 100644 --- a/src/openforms/prefill/contrib/objects_api/plugin.py +++ b/src/openforms/prefill/contrib/objects_api/plugin.py @@ -31,6 +31,10 @@ class ObjectsAPIPrefill(BasePlugin[ObjectsAPIOptions]): def verify_initial_data_ownership( self, submission: Submission, prefill_options: ObjectsAPIOptions ) -> None: + if prefill_options["skip_ownership_check"]: + logger.info("Skipping ownership check for submission %r.", submission.uuid) + return + assert submission.initial_data_reference api_group = prefill_options["objects_api_group"] assert api_group, "Can't do anything useful without an API group" diff --git a/src/openforms/prefill/contrib/objects_api/typing.py b/src/openforms/prefill/contrib/objects_api/typing.py index 36fb5e7e0e..4b5ee90a58 100644 --- a/src/openforms/prefill/contrib/objects_api/typing.py +++ b/src/openforms/prefill/contrib/objects_api/typing.py @@ -13,5 +13,6 @@ class ObjectsAPIOptions(TypedDict): objects_api_group: ObjectsAPIGroupConfig objecttype_uuid: UUID objecttype_version: int + skip_ownership_check: bool auth_attribute_path: list[str] variables_mapping: list[VariableMapping] From 671145c547daee011574a7c775ff685c71674e3f Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 30 Dec 2024 17:57:04 +0100 Subject: [PATCH 11/12] :sparkles: [#4650] Add option to skip ownership checks to options UI --- .../variables/prefill/PrefillSummary.js | 2 ++ .../prefill/objects_api/ObjectsAPIFields.js | 31 ++++++++++++----- .../prefill/objects_api/SkipOwnershipCheck.js | 34 +++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 src/openforms/js/components/admin/form_design/variables/prefill/objects_api/SkipOwnershipCheck.js diff --git a/src/openforms/js/components/admin/form_design/variables/prefill/PrefillSummary.js b/src/openforms/js/components/admin/form_design/variables/prefill/PrefillSummary.js index a6265f56b4..6d072ea52d 100644 --- a/src/openforms/js/components/admin/form_design/variables/prefill/PrefillSummary.js +++ b/src/openforms/js/components/admin/form_design/variables/prefill/PrefillSummary.js @@ -94,6 +94,8 @@ const PrefillSummary = ({ } isOpen={modalOpen} closeModal={() => setModalOpen(false)} + // FIXME: push this down to the plugin-specific components, somehow + extraModifiers={plugin === 'objects_api' ? ['large'] : undefined} > { const {values, setFieldValue, setValues} = useFormikContext(); const { plugin, - options: {objecttypeUuid, objecttypeVersion, objectsApiGroup}, + options: {objecttypeUuid, objecttypeVersion, objectsApiGroup, skipOwnershipCheck}, } = values; const {showCopyButton, toggleShowCopyButton} = useStatus(); @@ -90,6 +91,7 @@ const ObjectsAPIFields = () => { objectsApiGroup: null, objecttypeUuid: '', objecttypeVersion: null, + skipOwnershipCheck: false, authAttributePath: undefined, variablesMapping: [], }; @@ -235,13 +237,26 @@ const ObjectsAPIFields = () => { objectTypeFieldName="options.objecttypeUuid" /> - +
    + +
    + } + > + + {!skipOwnershipCheck && ( + + )}
    { + const [fieldProps] = useField({name: 'options.skipOwnershipCheck', type: 'checkbox'}); + return ( + + + } + helpText={ + + } + {...fieldProps} + /> + + ); +}; + +SkipOwnershipCheck.propTypes = {}; + +export default SkipOwnershipCheck; From b8d142b62bda162764bfa81baf012461569bbf97 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 31 Dec 2024 08:46:39 +0100 Subject: [PATCH 12/12] :white_check_mark: [#4650] Add tests for optional ownership check * Added tests for configuration validation * Added tests for runtime behaviour --- .../test_initial_data_ownership_validation.py | 16 +++++++ .../tests/test_options_validation.py | 47 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/openforms/prefill/contrib/objects_api/tests/test_options_validation.py diff --git a/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py b/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py index c3dbc7b227..b2b48fb087 100644 --- a/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py +++ b/src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py @@ -159,3 +159,19 @@ def test_verify_initial_data_ownership_missing_auth_attribute_path_causes_failin logs.filter_event("object_ownership_check_success").exists() ) self.assertFalse(logs.filter_event("prefill_retrieve_success").exists()) + + def test_allow_prefill_when_ownership_check_is_skipped(self): + self.variable.prefill_options["skip_ownership_check"] = True + self.variable.save() + submission = SubmissionFactory.create( + form=self.form, + # invalid BSN + auth_info__value="000XXX000", + auth_info__attribute=AuthAttribute.bsn, + initial_data_reference=self.object_ref, + ) + + try: + prefill_variables(submission=submission) + except PermissionDenied as exc: + raise self.failureException("Ownership check should be skipped") from exc diff --git a/src/openforms/prefill/contrib/objects_api/tests/test_options_validation.py b/src/openforms/prefill/contrib/objects_api/tests/test_options_validation.py new file mode 100644 index 0000000000..0469c35aa0 --- /dev/null +++ b/src/openforms/prefill/contrib/objects_api/tests/test_options_validation.py @@ -0,0 +1,47 @@ +import uuid + +from rest_framework.test import APITestCase + +from openforms.contrib.objects_api.tests.factories import ObjectsAPIGroupConfigFactory + +from ..api.serializers import ObjectsAPIOptionsSerializer + + +class OptionValidationTests(APITestCase): + """ + Test the serializer used for options validation. + """ + + def test_auth_attribute_not_required(self): + api_group = ObjectsAPIGroupConfigFactory.create() + data = { + "objects_api_group": api_group.pk, + "objecttype_uuid": uuid.uuid4(), + "objecttype_version": 3, + "skip_ownership_check": True, + "auth_attribute_path": [], + "variables_mapping": [], + } + serializer = ObjectsAPIOptionsSerializer(data=data) + + is_valid = serializer.is_valid() + + self.assertTrue(is_valid) + + def test_auth_attribute_required(self): + api_group = ObjectsAPIGroupConfigFactory.create() + data = { + "objects_api_group": api_group.pk, + "objecttype_uuid": uuid.uuid4(), + "objecttype_version": 3, + "skip_ownership_check": False, + "auth_attribute_path": [], + "variables_mapping": [], + } + serializer = ObjectsAPIOptionsSerializer(data=data) + + is_valid = serializer.is_valid() + + self.assertFalse(is_valid) + error = serializer.errors["auth_attribute_path"][0] + self.assertEqual(error.code, "empty")