From 0d24af5ac279e18c1adaeca519530ef473bb1ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Mon, 6 Jul 2026 16:50:33 -0300 Subject: [PATCH 1/2] Fix EmailFormProcessor ignoring block settings The processor read block fields under the wrong keys (e.g. `email_template` instead of `mail_template`), so the selected mail template, admin info, header/footer and sender/recipient addresses were silently dropped and the defaults used instead. - Type the block payload (`SchemaFormBlock`) and the submission context so a wrong key is visible rather than silently defaulted. - Extract the email helpers (`substitute_variables`, `format_property`, `addresses_from_block`, template lookup, mailhost check) into `utils.email`; the processor now delegates to them. - Resolve sender/recipient/bcc addresses once, up front, and pass them through the submission context. - Move the mail templates registry setting to a JSONField (`schemaform.mail_templates_json`), deprecating `schemaform.mail_templates`, with an upgrade step (profile 1000 -> 1001) migrating existing templates. - Add unit tests for the `utils.email` helpers. Refs #19 --- backend/news/19.bugfix | 1 + backend/news/19.internal | 1 + backend/src/plone/formblock/interfaces.py | 68 ++++++- .../src/plone/formblock/processors/email.py | 166 +++++----------- .../formblock/profiles/default/metadata.xml | 2 +- .../restapi/services/submit_form/post.py | 4 +- .../src/plone/formblock/upgrades/__init__.py | 19 ++ .../plone/formblock/upgrades/configure.zcml | 15 ++ backend/src/plone/formblock/utils/email.py | 91 +++++++++ .../plone/formblock/vocabularies/templates.py | 4 +- backend/tests/setup/test_setup_install.py | 2 +- backend/tests/utils/test_email.py | 185 +++++++++++++++++- 12 files changed, 425 insertions(+), 133 deletions(-) create mode 100644 backend/news/19.bugfix create mode 100644 backend/news/19.internal diff --git a/backend/news/19.bugfix b/backend/news/19.bugfix new file mode 100644 index 0000000..882caba --- /dev/null +++ b/backend/news/19.bugfix @@ -0,0 +1 @@ +Fixed the form email processor ignoring the block's configured settings: the selected mail template, admin info, mail header/footer and sender/recipient addresses are now honored when a form is submitted. @ericof diff --git a/backend/news/19.internal b/backend/news/19.internal new file mode 100644 index 0000000..23540a5 --- /dev/null +++ b/backend/news/19.internal @@ -0,0 +1 @@ +Moved the mail templates setting to a ``JSONField`` (``schemaform.mail_templates_json``), deprecating ``schemaform.mail_templates``, with an upgrade step that migrates existing templates. @ericof diff --git a/backend/src/plone/formblock/interfaces.py b/backend/src/plone/formblock/interfaces.py index a7e8503..b06c873 100644 --- a/backend/src/plone/formblock/interfaces.py +++ b/backend/src/plone/formblock/interfaces.py @@ -1,9 +1,12 @@ from plone.dexterity.content import DexterityContent +from plone.schema import JSONField +from typing import Any +from typing import TypedDict from zope import schema from zope.interface import Attribute from zope.interface import Interface +from zope.publisher.http import HTTPRequest from zope.publisher.interfaces.browser import IDefaultBrowserLayer -from ZPublisher.BaseRequest import BaseRequest import dataclasses @@ -46,12 +49,65 @@ def verify(data): """ +class FormSchema(TypedDict): + """A JSON Schema definition for a form block.""" + + properties: dict[str, dict] + fieldsets: list[dict] + required: list[str] + + +class RichTextField(TypedDict): + data: str + + +class SchemaFormBlock(TypedDict): + """A form block schema definition.""" + + schema: FormSchema + title: str + description: str + submit_label: str + show_cancel: bool + cancel_label: str + forward_user_to: str + success: str + thankyou: str + captcha: str + send: bool + recipients: str + bcc: str + admin_info: str + sender: str + sender_name: str + subject: str + mail_header: RichTextField + mail_footer: RichTextField + store: bool + data_wipe: int + send_confirmation: bool + confirmation_recipients: str + fixed_attachment: Any + mail_template: str + + +@dataclasses.dataclass +class AddressesFromBlock: + """Addresses extracted from a form block, with variable substitution.""" + + sender: str + admin_recipients: str + confirmation_recipients: str + bcc: str + + @dataclasses.dataclass class FormSubmissionContext: context: DexterityContent - block: dict + block: SchemaFormBlock form_data: dict - request: BaseRequest + request: HTTPRequest + addresses: AddressesFromBlock def get_records(self) -> list: """ @@ -109,8 +165,12 @@ def __call__(): class IFormSettings(Interface): mail_templates = schema.Dict( - title="Email templates", + title="Email templates (Deprecated)", key_type=schema.TextLine(), value_type=schema.Text(), + default={}, + ) + mail_templates_json = JSONField( + title="Email templates", default={"default": DEFAULT_TEMPLATE}, ) diff --git a/backend/src/plone/formblock/processors/email.py b/backend/src/plone/formblock/processors/email.py index ac2f7f4..3caf0f4 100644 --- a/backend/src/plone/formblock/processors/email.py +++ b/backend/src/plone/formblock/processors/email.py @@ -1,41 +1,26 @@ from email.message import EmailMessage -from email.utils import formataddr from plone import api -from plone.base.interfaces.controlpanel import IMailSchema -from plone.formblock import _ from plone.formblock.interfaces import FormSubmissionContext from plone.formblock.interfaces import IFormSubmissionProcessor -from plone.formblock.utils.email import create_message -from plone.registry.registry import Registry +from plone.formblock.interfaces import SchemaFormBlock +from plone.formblock.utils import email as utils from Products.MailHost.MailHost import MailHost from zExceptions import BadRequest from zope.component import adapter -from zope.component import getMultiAdapter from zope.interface import implementer -import re +def template_vars_from_block(block: SchemaFormBlock, form_data: dict) -> dict: + """Extract template variables from a form block. -def is_mailhost_configured() -> bool: - smtp_host = api.portal.get_registry_record("plone.smtp_host") - email_from_address = api.portal.get_registry_record("plone.email_from_address") - return bool(smtp_host and email_from_address) - - -def substitute_variables( - value: str, context: dict | None = None, form_data: dict | None = None -) -> str: - if context is None and form_data is not None: - context = form_data - elif context is None: - context = {} - - def replace(match): - name = match.group(1) - return context.get(name, "") - - pattern = r"\$\{([^}]+)\}" - return re.sub(pattern, replace, value) + Substituting variables if necessary. + """ + attrs = ("mail_header", "mail_footer") + template_vars = {} + for attr in attrs: + value = block.get(attr, {}).get("data", "") + template_vars[attr] = utils.substitute_variables(value, context=form_data) + return template_vars @implementer(IFormSubmissionProcessor) @@ -50,39 +35,19 @@ class EmailFormProcessor: def __init__(self, context: FormSubmissionContext): self.context = context.context self.request = context.request + self.addresses = context.addresses self.block = context.block + self.template = utils.get_template_from_block(self.block) self.form_data = context.form_data self.records = context.get_records() self.attachments = context.get_attachments() self.portal_transforms = api.portal.get_tool(name="portal_transforms") - self.templates: dict = ( - api.portal.get_registry_record("schemaform.mail_templates") or {} - ) - - registry: Registry = api.portal.get_tool("portal_registry") - self.mail_settings = registry.forInterface(IMailSchema, prefix="plone") - self.charset: str = registry.get("plone.email_charset", "utf-8") def _body_as_plain_text(self, html: str) -> str: tool = self.portal_transforms text = tool.convertTo("text/plain", html, mimetype="text/html").getData() return text.strip() - def get_sender(self) -> str: - sender = self.block.get("sender", "") - sender = ( - substitute_variables(sender, context=self.form_data) - or self.mail_settings.email_from_address - ) - - sender_name = self.block.get("sender_name", "") - sender_name = ( - substitute_variables(sender_name, context=self.form_data) - or self.mail_settings.email_from_name - ) - - return formataddr((sender_name, sender)) - def get_subject(self) -> str: subject = self.block.get("subject") schema_properties = self.block.get("schema", {}).get("properties", {}) @@ -91,72 +56,24 @@ def get_subject(self) -> str: subject = "${subject}" else: subject = self.block.get("title") or "Form Submission" - subject = substitute_variables(subject, context=self.form_data) + subject = utils.substitute_variables(subject, context=self.form_data) return subject def get_value(self, field_id, default=None): return self.form_data.get(field_id, default) - def get_bcc(self) -> str: - bcc: str = self.block.get("bcc", "") - bcc = substitute_variables(bcc, context=self.form_data) - return bcc or "" - - def get_confirmation_recipients(self) -> str: - confirmation_recipients = self.block.get("confirmation_recipients", "") - confirmation_recipients = substitute_variables( - confirmation_recipients, context=self.form_data - ) - return confirmation_recipients - - def prepare_message(self, admin=False): # noqa: C901 - template_name = self.block.get("email_template", "default") - admin_info = self.block.get("admin_info", "") - properties = self.block.get("schema").get("properties") - template = self.templates.get(template_name, "") - plone = getMultiAdapter((self.context, self.request), name="plone") - template_vars = { - "mail_header": substitute_variables( - self.block.get("mail_header", {}).get("data", ""), - context=self.form_data, - ), - "mail_footer": substitute_variables( - self.block.get("mail_footer", {}).get("data", ""), - context=self.form_data, - ), - } - form_fields = "" - if admin: - form_fields += admin_info.replace("\n", "
") + "

" - - def format_property(factory, value): - if factory == "label_boolean_field" or factory == "termsAccepted": - if value == True: # noqa: E712 - return self.context.translate( - _("Yes"), - context=self.request, - ) - else: - return self.context.translate( - _("No"), - context=self.request, - ) - elif factory == "checkbox_group": - if isinstance(value, list): - return "
".join(value) - else: - return str(value) - elif factory == "label_date_field": - return plone.toLocalizedTime(value) - elif factory == "label_datetime_field": - return plone.toLocalizedTime(value, True) - else: - return str(value) - + def prepare_message(self, admin: bool = False) -> str: + block = self.block + form_data = self.form_data + template = self.template + template_vars = template_vars_from_block(block, form_data) + admin_info = block.get("admin_info", "") + properties = block.get("schema", {}).get("properties") + form_fields = admin_info.replace("\n", "
") + "

" if admin else "" form_fields += "\n" for record in self.records: factory = properties[record["field_id"]].get("factory", "") - value = format_property(factory, record["value"]) + value = utils.format_property(factory, record["value"]) template_vars[record["field_id"]] = value if factory == "hidden" and not admin: @@ -169,7 +86,7 @@ def format_property(factory, value): ) form_fields += "\n
\n" template_vars["form_fields"] = form_fields - message = substitute_variables(template, template_vars) + message = utils.substitute_variables(template, template_vars) return message def send_mail(self, msg: EmailMessage, charset: str) -> None: @@ -178,7 +95,9 @@ def send_mail(self, msg: EmailMessage, charset: str) -> None: # by default (False) exceptions are handled by MailHost and we can't catch them. host.send(msg, charset=charset, immediate=True) - def _prepare_msg_admin(self, mfrom: str, mto: str, subject: str) -> EmailMessage: + def _prepare_msg_admin( + self, mfrom: str, mto: str, subject: str, bcc: str + ) -> EmailMessage: body = self.prepare_message(True) body_text = self._body_as_plain_text(body) @@ -189,20 +108,20 @@ def _prepare_msg_admin(self, mfrom: str, mto: str, subject: str) -> EmailMessage if header_value: headers[header] = header_value - return create_message( + return utils.create_message( mfrom=mfrom, mto=mto, subject=subject, body=body, body_txt=body_text, reply_to=mfrom, - bcc=self.get_bcc(), + bcc=bcc, headers=headers, attachments=self.attachments, ) def _prepare_msg_confirmation( - self, mfrom: str, mto: str, subject: str + self, mfrom: str, mto: str, subject: str, bcc: str = "" ) -> EmailMessage: body = self.prepare_message() body_text = self._body_as_plain_text(body) @@ -211,38 +130,43 @@ def _prepare_msg_confirmation( if self.block.get("fixed_attachment"): attachments["fixed_attachment"] = self.block["fixed_attachment"] - return create_message( + return utils.create_message( mfrom=mfrom, mto=mto, subject=subject, body=body, body_txt=body_text, reply_to="", + bcc=bcc, headers={}, attachments=attachments, ) def __call__(self) -> None: queued: list[EmailMessage] = [] - send_to_admin = bool(self.block.get("send")) - send_confirmation = bool(self.block.get("send_confirmation")) + block = self.block + addresses = self.addresses + send_to_admin = bool(block.get("send")) + send_confirmation = bool(block.get("send_confirmation")) if not send_to_admin and not send_confirmation: return - if not is_mailhost_configured(): + if not utils.is_mailhost_configured(): raise BadRequest("MailHost is not configured.") subject = self.get_subject() - mfrom = self.get_sender() + mfrom = addresses.sender if send_to_admin: - mto = self.block.get("recipients", self.mail_settings.email_from_address) - msg = self._prepare_msg_admin(mfrom, mto, subject) + mto = addresses.admin_recipients + bcc = addresses.bcc + msg = self._prepare_msg_admin(mfrom, mto, subject, bcc) queued.append(msg) - if send_confirmation and (mto := self.get_confirmation_recipients()): + if send_confirmation and (mto := addresses.confirmation_recipients.strip()): msg = self._prepare_msg_confirmation(mfrom, mto, subject) queued.append(msg) + charset = api.portal.get_registry_record("plone.email_charset", default="utf-8") for msg in queued: - self.send_mail(msg=msg, charset=self.charset) + self.send_mail(msg=msg, charset=charset) diff --git a/backend/src/plone/formblock/profiles/default/metadata.xml b/backend/src/plone/formblock/profiles/default/metadata.xml index 2d4b5fc..fa3beaa 100644 --- a/backend/src/plone/formblock/profiles/default/metadata.xml +++ b/backend/src/plone/formblock/profiles/default/metadata.xml @@ -1,6 +1,6 @@ - 1000 + 1001 profile-plone.restapi:default profile-plone.volto:default diff --git a/backend/src/plone/formblock/restapi/services/submit_form/post.py b/backend/src/plone/formblock/restapi/services/submit_form/post.py index 87b7d89..3a39896 100644 --- a/backend/src/plone/formblock/restapi/services/submit_form/post.py +++ b/backend/src/plone/formblock/restapi/services/submit_form/post.py @@ -6,6 +6,7 @@ from plone.formblock.restapi.services.base import BaseService from plone.formblock.utils import fix_block_schema from plone.formblock.utils import get_blocks +from plone.formblock.utils.email import addresses_from_block from plone.protect.interfaces import IDisableCSRFProtection from plone.restapi.deserializer import json_body from zExceptions import BadRequest @@ -174,18 +175,19 @@ def _process_post(self) -> None: self.block_id = "" self.block = {} schema = {} - if block_id := body.get("block_id", ""): self.block_id = block_id self.block = self.get_block_data(block_id=block_id) schema = self.block.get("schema", {}) self.form_data = self.cleanup_data(schema, body.get("data", {})) + addresses = addresses_from_block(self.block, self.form_data) self.submission_context = FormSubmissionContext( context=self.context, request=self.request, block=self.block, form_data=self.form_data, + addresses=addresses, ) def reply(self) -> dict: diff --git a/backend/src/plone/formblock/upgrades/__init__.py b/backend/src/plone/formblock/upgrades/__init__.py index e69de29..a2d3755 100644 --- a/backend/src/plone/formblock/upgrades/__init__.py +++ b/backend/src/plone/formblock/upgrades/__init__.py @@ -0,0 +1,19 @@ +from plone import api +from plone.formblock import logger +from plone.formblock.interfaces import DEFAULT_TEMPLATE +from Products.GenericSetup.tool import SetupTool + + +def migrate_mail_templates(context: SetupTool) -> None: + """Migrate mail templates from the old registry record to the new one.""" + default_templates = {"default": DEFAULT_TEMPLATE} + old_templates: dict[str, str] = api.portal.get_registry_record( + "schemaform.mail_templates", default=default_templates + ) + api.portal.set_registry_record("schemaform.mail_templates_json", old_templates) + # Set the old registry record to an empty dict to avoid confusion + api.portal.set_registry_record("schemaform.mail_templates", {}) + logger.info( + "Migrated mail templates from 'schemaform.mail_templates' to " + "'schemaform.mail_templates_json'." + ) diff --git a/backend/src/plone/formblock/upgrades/configure.zcml b/backend/src/plone/formblock/upgrades/configure.zcml index 56cc451..45915e9 100644 --- a/backend/src/plone/formblock/upgrades/configure.zcml +++ b/backend/src/plone/formblock/upgrades/configure.zcml @@ -3,6 +3,21 @@ xmlns:genericsetup="http://namespaces.zope.org/genericsetup" > + + + + + diff --git a/backend/src/plone/formblock/utils/email.py b/backend/src/plone/formblock/utils/email.py index 6f75e46..6424908 100644 --- a/backend/src/plone/formblock/utils/email.py +++ b/backend/src/plone/formblock/utils/email.py @@ -1,13 +1,104 @@ from email import policy from email.message import EmailMessage +from email.utils import formataddr +from plone import api +from plone.formblock import _ +from plone.formblock.interfaces import AddressesFromBlock +from plone.formblock.interfaces import DEFAULT_TEMPLATE +from plone.formblock.interfaces import SchemaFormBlock import codecs import os +import re CTE = os.environ.get("MAIL_CONTENT_TRANSFER_ENCODING", None) +def is_mailhost_configured() -> bool: + """Check if MailHost is configured with SMTP host and email from address.""" + smtp_host = api.portal.get_registry_record("plone.smtp_host") + email_from_address = api.portal.get_registry_record("plone.email_from_address") + return bool(smtp_host and email_from_address) + + +def available_templates() -> dict[str, str]: + """Return a dictionary of available email templates.""" + templates: dict[str, str] = api.portal.get_registry_record( + "schemaform.mail_templates_json" + ) + return templates or {} + + +def get_template_from_block(block: SchemaFormBlock) -> str: + """Return the email template for the given template name.""" + templates: dict[str, str] = available_templates() + template_name = block.get("mail_template", "default") + return templates.get(template_name) or DEFAULT_TEMPLATE + + +def substitute_variables( + value: str, context: dict | None = None, form_data: dict | None = None +) -> str: + """Substitute variables in the form ${variable_name} with values from the context + or form_data. + """ + if context is None and form_data is not None: + context = form_data + elif context is None: + context = {} + + def replace(match): + name = match.group(1) + return context.get(name, "") + + pattern = r"\$\{([^}]+)\}" + return re.sub(pattern, replace, value) + + +def format_property(factory: str, value: str | bool | list | None) -> str: + response = str(value) + if factory == "label_boolean_field" or factory == "termsAccepted": + response = ( + api.portal.translate(_("Yes")) + if value is True + else api.portal.translate(_("No")) + ) + elif factory == "checkbox_group" and isinstance(value, list): + response = "
".join(value) + elif factory in ("label_date_field", "label_datetime_field"): + util = api.portal.get_tool("translation_service") + response = util.toLocalizedTime( + value, long_format=factory == "label_datetime_field" + ) + return response + + +def addresses_from_block(block: SchemaFormBlock, form_data: dict) -> AddressesFromBlock: + """Extract addresses from a form block, substituting variables if necessary.""" + default_addr: str = api.portal.get_registry_record("plone.email_from_address") + default_name: str = api.portal.get_registry_record("plone.email_from_name") + admin_recipients: str = block.get("recipients", default_addr) + _sender = ( + substitute_variables(block.get("sender", ""), context=form_data) or default_addr + ) + _sender_name = ( + substitute_variables(block.get("sender_name", ""), context=form_data) + or default_name + ) + sender: str = formataddr((_sender_name, _sender)) + bcc: str = substitute_variables(block.get("bcc", ""), context=form_data) or "" + rcpts: str = substitute_variables( + block.get("confirmation_recipients", ""), context=form_data + ) + return AddressesFromBlock( + sender=sender, + admin_recipients=admin_recipients, + confirmation_recipients=rcpts, + bcc=bcc, + ) + + def add_attachaments_to_msg( msg: EmailMessage, attachments: dict[str, dict | str] ) -> None: diff --git a/backend/src/plone/formblock/vocabularies/templates.py b/backend/src/plone/formblock/vocabularies/templates.py index 40a518d..39fd3cd 100644 --- a/backend/src/plone/formblock/vocabularies/templates.py +++ b/backend/src/plone/formblock/vocabularies/templates.py @@ -5,8 +5,8 @@ @provider(IVocabularyFactory) -def mail_templates_vocabulary_factory(context): - name = "schemaform.mail_templates" +def mail_templates_vocabulary_factory(context) -> SimpleVocabulary: + name: str = "schemaform.mail_templates_json" registry_record_value = api.portal.get_registry_record(name) items = list(registry_record_value.keys()) return SimpleVocabulary.fromItems([[item, item, item] for item in items]) diff --git a/backend/tests/setup/test_setup_install.py b/backend/tests/setup/test_setup_install.py index 8ee8cae..7bb7a13 100644 --- a/backend/tests/setup/test_setup_install.py +++ b/backend/tests/setup/test_setup_install.py @@ -14,4 +14,4 @@ def test_browserlayer(self, browser_layers): def test_latest_version(self, profile_last_version): """Test latest version of default profile.""" - assert profile_last_version(f"{PACKAGE_NAME}:default") == "1000" + assert profile_last_version(f"{PACKAGE_NAME}:default") == "1001" diff --git a/backend/tests/utils/test_email.py b/backend/tests/utils/test_email.py index ff0fdac..795b1b3 100644 --- a/backend/tests/utils/test_email.py +++ b/backend/tests/utils/test_email.py @@ -1,14 +1,19 @@ """Unit tests for plone.formblock.utils.email. -These cover the two pure helpers used to build outgoing form e-mails: -``add_attachaments_to_msg`` and ``create_message``. They do not need a Plone -site, so they run as plain unit tests with fixtures and parametrization. +The pure helpers (``add_attachaments_to_msg``, ``create_message`` and +``substitute_variables``) need no Plone site and run as plain unit tests with +fixtures and parametrization. The helpers that read the registry or translate +strings (``is_mailhost_configured``, ``available_templates``, +``get_template_from_block``, ``format_property`` and ``addresses_from_block``) +need ``api`` access and live in :class:`TestApiSettings`. """ from email import policy from email.message import EmailMessage +from plone import api from plone.formblock.utils.email import add_attachaments_to_msg from plone.formblock.utils.email import create_message +from plone.formblock.utils.email import substitute_variables import base64 import pytest @@ -224,3 +229,177 @@ def test_with_attachments(self, b64_attachment): assert len(attachments) == 1 assert attachments[0].get_filename() == "x.png" assert attachments[0].get_content_type() == "image/png" + + +class TestSubstituteVariables: + """``substitute_variables`` is a pure ``${var}`` interpolation helper.""" + + def test_no_variables_returns_unchanged(self): + assert substitute_variables("plain text") == "plain text" + + @pytest.mark.parametrize( + "template,context,expected", + [ + ("hi ${name}", {"name": "Bob"}, "hi Bob"), + ("${a}-${b}", {"a": "1", "b": "2"}, "1-2"), + ("${g}, ${name}!", {"g": "Hello", "name": "Ana"}, "Hello, Ana!"), + ], + ) + def test_substitution_with_context(self, template, context, expected): + assert substitute_variables(template, context=context) == expected + + def test_missing_variable_becomes_empty(self): + assert substitute_variables("a${x}b", context={}) == "ab" + + def test_form_data_used_when_context_is_none(self): + assert substitute_variables("hi ${name}", form_data={"name": "X"}) == "hi X" + + def test_context_takes_precedence_over_form_data(self): + # When both are given, ``form_data`` is ignored entirely. + result = substitute_variables( + "${a}", context={"a": "ctx"}, form_data={"a": "fd"} + ) + assert result == "ctx" + + def test_both_none_treats_all_variables_as_missing(self): + assert substitute_variables("x${y}z") == "xz" + + def test_non_string_substitution_value_raises(self): + # ``re.sub`` requires the replacement to be a string, so a non-string + # value in the context surfaces as a TypeError. Documents a latent + # constraint: callers must pass stringified values. + with pytest.raises(TypeError): + substitute_variables("${n}", context={"n": 5}) + + +class TestApiSettings: + @pytest.fixture(autouse=True) + def _setup(self, portal_class): + self.portal = portal_class + + def test_is_mailhost_configured(self): + from plone.formblock.utils.email import is_mailhost_configured + + assert is_mailhost_configured() is True + + def test_available_templates(self): + from plone.formblock.utils.email import available_templates + + templates = available_templates() + assert isinstance(templates, dict) + assert "default" in templates + assert isinstance(templates["default"], str) + + def test_get_template_from_block_defaults_to_default_template(self): + from plone.formblock.interfaces import DEFAULT_TEMPLATE + from plone.formblock.utils.email import get_template_from_block + + # No ``mail_template`` key -> resolves the "default" entry. + assert get_template_from_block({}) == DEFAULT_TEMPLATE + + def test_get_template_from_block_unknown_name_falls_back(self): + from plone.formblock.interfaces import DEFAULT_TEMPLATE + from plone.formblock.utils.email import get_template_from_block + + block = {"mail_template": "does-not-exist"} + assert get_template_from_block(block) == DEFAULT_TEMPLATE + + def test_get_template_from_block_returns_selected_template(self): + from plone.formblock.utils.email import get_template_from_block + + record = "schemaform.mail_templates_json" + original = api.portal.get_registry_record(record) + try: + api.portal.set_registry_record( + record, {**original, "custom": "CUSTOM BODY"} + ) + block = {"mail_template": "custom"} + assert get_template_from_block(block) == "CUSTOM BODY" + finally: + api.portal.set_registry_record(record, original) + + @pytest.mark.parametrize( + "factory,value,expected", + [ + ("label_boolean_field", True, "Yes"), + ("label_boolean_field", False, "No"), + ("termsAccepted", True, "Yes"), + ("termsAccepted", False, "No"), + # Identity check: only ``True`` is "Yes"; a truthy non-True is "No". + ("label_boolean_field", "true", "No"), + ], + ) + def test_format_property_boolean(self, factory, value, expected): + from plone.formblock.utils.email import format_property + + assert format_property(factory, value) == expected + + def test_format_property_checkbox_group_joins_list(self): + from plone.formblock.utils.email import format_property + + assert format_property("checkbox_group", ["a", "b", "c"]) == "a
b
c" + + def test_format_property_checkbox_group_non_list_is_stringified(self): + from plone.formblock.utils.email import format_property + + # Only list values are joined; anything else falls through to ``str``. + assert format_property("checkbox_group", "single") == "single" + + @pytest.mark.parametrize("factory", ["label_date_field", "label_datetime_field"]) + def test_format_property_dates_are_localized(self, factory): + from plone.formblock.utils.email import format_property + + result = format_property(factory, "2026-07-06") + assert "2026" in result + + @pytest.mark.parametrize( + "value,expected", + [(42, "42"), ("plain", "plain"), (None, "None")], + ) + def test_format_property_default_stringifies(self, value, expected): + from plone.formblock.utils.email import format_property + + assert format_property("text", value) == expected + + def test_addresses_from_block_defaults(self): + from plone.formblock.utils.email import addresses_from_block + + addresses = addresses_from_block({}, {}) + assert addresses.sender == "Plone test site " + assert addresses.admin_recipients == "site_addr@plone.com" + assert addresses.bcc == "" + assert addresses.confirmation_recipients == "" + + def test_addresses_from_block_explicit_values(self): + from plone.formblock.utils.email import addresses_from_block + + block = { + "sender": "me@x.com", + "sender_name": "Me", + "recipients": "admin@x.com", + "bcc": "b@x.com", + "confirmation_recipients": "c@x.com", + } + addresses = addresses_from_block(block, {}) + assert addresses.sender == "Me " + assert addresses.admin_recipients == "admin@x.com" + assert addresses.bcc == "b@x.com" + assert addresses.confirmation_recipients == "c@x.com" + + def test_addresses_from_block_substitutes_variables(self): + from plone.formblock.utils.email import addresses_from_block + + block = {"sender": "${email}", "confirmation_recipients": "${email}"} + form_data = {"email": "user@x.com"} + addresses = addresses_from_block(block, form_data) + # sender_name falls back to the site default; the address is substituted. + assert addresses.sender == "Plone test site " + assert addresses.confirmation_recipients == "user@x.com" + + def test_addresses_from_block_empty_recipients_not_defaulted(self): + from plone.formblock.utils.email import addresses_from_block + + # ``recipients`` defaults to the site address only when the key is + # absent; an explicit empty string is preserved as-is. + addresses = addresses_from_block({"recipients": ""}, {}) + assert addresses.admin_recipients == "" From 74fd67644c8232e93384caec8500d18ad37ed76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Andrei?= Date: Mon, 6 Jul 2026 16:50:41 -0300 Subject: [PATCH 2/2] Add GenericSetup testing profile Replace the imperative registry setup in the test layer with a declarative `plone.formblock.testing:testing` profile that configures the mail settings and installs the captcha add-ons (norobots, hcaptcha, recaptcha). Update the captcha vocabulary test for the now-available norobots adapter. --- backend/news/+testing-profile.tests | 1 + backend/src/plone/formblock/testing/__init__.py | 9 +-------- backend/src/plone/formblock/testing/configure.zcml | 14 +++++++++++++- .../formblock/testing/profiles/testing/actions.xml | 9 +++++++++ .../testing/profiles/testing/metadata.xml | 9 +++++++++ ...ne.base.interfaces.controlpanel.IMailSchema.xml | 12 ++++++++++++ .../plone.formblock.interfaces.IFormSettings.xml | 9 +++++++++ .../tests/functional/captcha/test_vocabulary.py | 10 +++++++--- 8 files changed, 61 insertions(+), 12 deletions(-) create mode 100644 backend/news/+testing-profile.tests create mode 100644 backend/src/plone/formblock/testing/profiles/testing/actions.xml create mode 100644 backend/src/plone/formblock/testing/profiles/testing/metadata.xml create mode 100644 backend/src/plone/formblock/testing/profiles/testing/registry/plone.base.interfaces.controlpanel.IMailSchema.xml create mode 100644 backend/src/plone/formblock/testing/profiles/testing/registry/plone.formblock.interfaces.IFormSettings.xml diff --git a/backend/news/+testing-profile.tests b/backend/news/+testing-profile.tests new file mode 100644 index 0000000..d5bb1c9 --- /dev/null +++ b/backend/news/+testing-profile.tests @@ -0,0 +1 @@ +Added a dedicated GenericSetup testing profile to configure mail and captcha settings for the test suite. @ericof diff --git a/backend/src/plone/formblock/testing/__init__.py b/backend/src/plone/formblock/testing/__init__.py index 09e1aa0..dbd6e26 100644 --- a/backend/src/plone/formblock/testing/__init__.py +++ b/backend/src/plone/formblock/testing/__init__.py @@ -1,4 +1,3 @@ -from plone import api from plone.app.contenttypes.testing import PLONE_APP_CONTENTTYPES_FIXTURE from plone.app.testing import applyProfile from plone.app.testing import FunctionalTesting @@ -24,16 +23,10 @@ def setUpZope(self, app, configurationContext): def setUpPloneSite(self, portal): applyProfile(portal, "plone.restapi:blocks") applyProfile(portal, "plone.formblock:default") + applyProfile(portal, "plone.formblock.testing:testing") quickInstallProduct(portal, "collective.MockMailHost") applyProfile(portal, "collective.MockMailHost:default") - # Set the email from address and name to avoid validation errors - # when sending emails - api.portal.set_registry_record( - "plone.email_from_address", "site_addr@plone.com" - ) - api.portal.set_registry_record("plone.email_from_name", "Plone test site") - # Mock the validate email token function def validate_email_token_mock(*args, **kwargs): return True diff --git a/backend/src/plone/formblock/testing/configure.zcml b/backend/src/plone/formblock/testing/configure.zcml index a3196c2..633cdf3 100644 --- a/backend/src/plone/formblock/testing/configure.zcml +++ b/backend/src/plone/formblock/testing/configure.zcml @@ -1,4 +1,7 @@ - + @@ -7,4 +10,13 @@ for="plone.formblock.interfaces.IPostEvent" handler="plone.formblock.testing.event_handler.event_handler" /> + + + diff --git a/backend/src/plone/formblock/testing/profiles/testing/actions.xml b/backend/src/plone/formblock/testing/profiles/testing/actions.xml new file mode 100644 index 0000000..45453ac --- /dev/null +++ b/backend/src/plone/formblock/testing/profiles/testing/actions.xml @@ -0,0 +1,9 @@ + + + + + False + + + + diff --git a/backend/src/plone/formblock/testing/profiles/testing/metadata.xml b/backend/src/plone/formblock/testing/profiles/testing/metadata.xml new file mode 100644 index 0000000..76bffff --- /dev/null +++ b/backend/src/plone/formblock/testing/profiles/testing/metadata.xml @@ -0,0 +1,9 @@ + + + 1000 + + profile-collective.z3cform.norobots:default + profile-plone.formwidget.hcaptcha:default + profile-plone.formwidget.recaptcha:default + + diff --git a/backend/src/plone/formblock/testing/profiles/testing/registry/plone.base.interfaces.controlpanel.IMailSchema.xml b/backend/src/plone/formblock/testing/profiles/testing/registry/plone.base.interfaces.controlpanel.IMailSchema.xml new file mode 100644 index 0000000..f5e8e40 --- /dev/null +++ b/backend/src/plone/formblock/testing/profiles/testing/registry/plone.base.interfaces.controlpanel.IMailSchema.xml @@ -0,0 +1,12 @@ + + + + utf-8 + site_addr@plone.com + Plone test site + localhost + 25 + + diff --git a/backend/src/plone/formblock/testing/profiles/testing/registry/plone.formblock.interfaces.IFormSettings.xml b/backend/src/plone/formblock/testing/profiles/testing/registry/plone.formblock.interfaces.IFormSettings.xml new file mode 100644 index 0000000..15564be --- /dev/null +++ b/backend/src/plone/formblock/testing/profiles/testing/registry/plone.formblock.interfaces.IFormSettings.xml @@ -0,0 +1,9 @@ + + + + + {'default': '\n${mail_header}\n<hr />\n${form_fields}\n<hr />\n${mail_footer}\n'} + + diff --git a/backend/tests/functional/captcha/test_vocabulary.py b/backend/tests/functional/captcha/test_vocabulary.py index ecdfcf8..7b80636 100644 --- a/backend/tests/functional/captcha/test_vocabulary.py +++ b/backend/tests/functional/captcha/test_vocabulary.py @@ -38,15 +38,19 @@ def test_no_adapters(self, manager_request): # no adapters configured assert data["@id"].endswith(self.endpoint) - # honeypot is always active if it's in buildout - assert data["items_total"] == 1 - assert data["items"] == [{"title": "Honeypot Support", "token": "honeypot"}] + assert data["items_total"] == 2 + assert data["items"][0] == { + "title": "NoRobots ReCaptcha Support", + "token": "norobots-captcha", + } + assert data["items"][1] == {"title": "Honeypot Support", "token": "honeypot"} @pytest.mark.parametrize( "token,title", [ ["recaptcha", "Google ReCaptcha"], ["hcaptcha", "HCaptcha"], + ["norobots-captcha", "NoRobots ReCaptcha Support"], ], ) def test_adapters(self, manager_request, configure_captcha_adapters, token, title):