From c4b2e08b1688da09dec6da9c4fea509672878a13 Mon Sep 17 00:00:00 2001 From: Mike Edmunds Date: Sat, 22 Jun 2024 14:30:30 -0700 Subject: [PATCH] SparkPost: error on features incompatible with template_id Raise an `AnymailUnsupportedFeature` error when trying to use a `template_id` along with other content payload fields that SparkPost silently ignores when template_id is present. --- CHANGELOG.rst | 6 +++- anymail/backends/sparkpost.py | 43 +++++++++++++++++++++++++++-- docs/esps/sparkpost.rst | 30 +++++++++++++++++++- tests/test_sparkpost_backend.py | 35 ++++++++++++++++++++--- tests/test_sparkpost_integration.py | 1 + 5 files changed, 107 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 776ac73..a43e24a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -41,6 +41,10 @@ Breaking changes (Anymail 10.0 switched to the SES v2 API by default. If your ``EMAIL_BACKEND`` setting has ``amazon_sesv2``, change that to just ``amazon_ses``.) +* **SparkPost:** When sending with a ``template_id``, Anymail now raises an + error if the message uses features that SparkPost will silently ignore. See + `docs `__. + Features ~~~~~~~~ @@ -162,7 +166,7 @@ Features should be no impact on your code. (Thanks to `@sblondon`_.) * **Brevo (Sendinblue):** Add support for inbound email. (See - `docs `_.) + `docs `__.) * **SendGrid:** Support multiple ``reply_to`` addresses. (Thanks to `@gdvalderrama`_ for pointing out the new API.) diff --git a/anymail/backends/sparkpost.py b/anymail/backends/sparkpost.py index 21c17fc..c100dbb 100644 --- a/anymail/backends/sparkpost.py +++ b/anymail/backends/sparkpost.py @@ -1,3 +1,6 @@ +from django.conf import settings +from django.utils.encoding import force_str + from ..exceptions import AnymailRequestsAPIError from ..message import AnymailRecipientStatus from ..utils import get_anymail_setting, update_deep @@ -86,6 +89,7 @@ def get_api_endpoint(self): def serialize_data(self): self._finalize_recipients() + self._check_content_options() return self.serialize_json(self.data) def _finalize_recipients(self): @@ -126,6 +130,31 @@ def _finalize_recipients(self): for email in self.cc_and_bcc ) + # SparkPost silently ignores certain "content" payload fields + # when a template_id is used. + IGNORED_WITH_TEMPLATE_ID = { + # SparkPost API content. -> feature name (for error message) + "attachments": "attachments", + "inline_images": "inline images", + "headers": "extra headers and/or cc recipients", + "from": "from_email", + "reply_to": "reply_to", + } + + def _check_content_options(self): + if "template_id" in self.data["content"]: + # subject, text, and html will cause 422 API Error: + # "message": "Both content object and template_id are specified", + # "code": "1301" + # but others are silently ignored in a template send: + ignored = [ + feature_name + for field, feature_name in self.IGNORED_WITH_TEMPLATE_ID.items() + if field in self.data["content"] + ] + if ignored: + self.unsupported_feature("template_id with %s" % ", ".join(ignored)) + # # Payload construction # @@ -138,7 +167,8 @@ def init_payload(self): } def set_from_email(self, email): - self.data["content"]["from"] = email.address + if email: + self.data["content"]["from"] = email.address def set_to(self, emails): if emails: @@ -293,13 +323,22 @@ def set_track_opens(self, track_opens): def set_template_id(self, template_id): self.data["content"]["template_id"] = template_id - # Must remove empty string "content" params when using stored template + # Must remove empty string "content" params when using stored template. + # (Non-empty params are left in place, to cause API error.) for content_param in ["subject", "text", "html"]: try: if not self.data["content"][content_param]: del self.data["content"][content_param] except KeyError: pass + # "from" is also silently ignored. Strip it if empty or DEFAULT_FROM_EMAIL, + # else leave in place to cause error in _check_content_options. + try: + from_email = self.data["content"]["from"] + if not from_email or from_email == force_str(settings.DEFAULT_FROM_EMAIL): + del self.data["content"]["from"] + except KeyError: + pass def set_merge_data(self, merge_data): for recipient in self.data["recipients"]: diff --git a/docs/esps/sparkpost.rst b/docs/esps/sparkpost.rst index d03bdc2..329e760 100644 --- a/docs/esps/sparkpost.rst +++ b/docs/esps/sparkpost.rst @@ -215,6 +215,29 @@ Limitations and quirks management headers. (The list of allowed custom headers does not seem to be documented.) +.. _sparkpost-template-limitations: + +**Features incompatible with template_id** + When sending with a :attr:`~anymail.message.AnymailMessage.template_id`, + SparkPost doesn't support attachments, inline images, extra headers, + :attr:`!reply_to`, :attr:`!cc` recipients, or overriding the + :attr:`!from_email`, :attr:`!subject`, or body (text or html) when + sending the message. Some of these can be defined in the template itself, + but SparkPost (often) silently drops them when supplied to their + Transmissions send API. + + .. versionchanged:: 11.0 + + Using features incompatible with :attr:`!template_id` will raise an + :exc:`~anymail.exceptions.AnymailUnsupportedFeature` error. In earlier + releases, Anymail would pass the incompatible content to SparkPost's + API, which in many cases would silently ignore it and send the message + anyway. + + These limitations only apply when using stored templates (with a template_id), + not when using SparkPost's template language for on-the-fly templating + in a message's subject, body, etc. + **Envelope sender may use domain only** Anymail's :attr:`~anymail.message.AnymailMessage.envelope_sender` is used to populate SparkPost's `'return_path'` parameter. Anymail supplies the full @@ -246,7 +269,8 @@ and :ref:`batch sending ` with per-recipient merge data. You can use a SparkPost stored template by setting a message's :attr:`~anymail.message.AnymailMessage.template_id` to the template's unique id. (When using a stored template, SparkPost prohibits -setting the EmailMessage's subject, text body, or html body.) +setting the EmailMessage's subject, text body, or html body, and has +:ref:`several other limitations `.) Alternatively, you can refer to merge fields directly in an EmailMessage's subject, body, and other fields---the message itself is used as an @@ -264,6 +288,7 @@ message attributes. to=["alice@example.com", "Bob "] ) message.template_id = "11806290401558530" # SparkPost id + message.from_email = None # must set after constructor (see below) message.merge_data = { 'alice@example.com': {'name': "Alice", 'order_no': "12345"}, 'bob@example.com': {'name': "Bob", 'order_no': "54321"}, @@ -279,6 +304,9 @@ message attributes. }, } +When using a :attr:`~anymail.message.AnymailMessage.template_id`, you must set the +message's :attr:`!from_email` to ``None`` as shown above. SparkPost does not permit +specifying the from address at send time when using a stored template. See `SparkPost's substitutions reference`_ for more information on templates and batch send with SparkPost. If you need the special `"dynamic" keys for nested substitutions`_, diff --git a/tests/test_sparkpost_backend.py b/tests/test_sparkpost_backend.py index 29d8e7a..bfb4402 100644 --- a/tests/test_sparkpost_backend.py +++ b/tests/test_sparkpost_backend.py @@ -19,7 +19,7 @@ AnymailSerializationError, AnymailUnsupportedFeature, ) -from anymail.message import attach_inline_image_file +from anymail.message import AnymailMessage, attach_inline_image_file from .mock_requests_backend import RequestsBackendMockAPITestCase from .utils import ( @@ -510,9 +510,8 @@ def test_tracking(self): self.assertEqual(data["options"]["click_tracking"], True) def test_template_id(self): - message = mail.EmailMultiAlternatives( - from_email="from@example.com", to=["to@example.com"] - ) + message = mail.EmailMultiAlternatives(to=["to@example.com"]) + message.from_email = None message.template_id = "welcome_template" message.send() data = self.get_api_call_json() @@ -521,6 +520,34 @@ def test_template_id(self): self.assertNotIn("subject", data["content"]) self.assertNotIn("text", data["content"]) self.assertNotIn("html", data["content"]) + self.assertNotIn("from", data["content"]) + + def test_template_id_ignores_default_from_email(self): + # No from_email, or from_email=None in constructor, + # uses settings.DEFAULT_FROM_EMAIL. We strip that with a template_id: + message = AnymailMessage(to=["to@example.com"], template_id="welcome_template") + self.assertIsNotNone(message.from_email) # because it's DEFAULT_FROM_EMAIL + message.send() + data = self.get_api_call_json() + self.assertNotIn("from", data["content"]) + + def test_unsupported_content_with_template_id(self): + # Make sure we raise an error for options that SparkPost + # silently ignores when sending with a template_id + message = AnymailMessage( + to=["to@example.com"], + from_email="non-default-from@example.com", + reply_to=["reply@example.com"], + headers={"X-Custom": "header"}, + template_id="welcome_template", + ) + message.attach(filename="test.txt", content="attachment", mimetype="text/plain") + with self.assertRaisesMessage( + AnymailUnsupportedFeature, + "template_id with attachments, extra headers and/or cc recipients," + " from_email, reply_to", + ): + message.send() def test_merge_data(self): self.set_mock_result(accepted=4) # two 'to' plus one 'cc' for each 'to' diff --git a/tests/test_sparkpost_integration.py b/tests/test_sparkpost_integration.py index b24a201..635da7f 100644 --- a/tests/test_sparkpost_integration.py +++ b/tests/test_sparkpost_integration.py @@ -153,6 +153,7 @@ def test_stored_template(self): "order": "12345", }, ) + message.from_email = None # from_email must come from stored template message.send() recipient_status = message.anymail_status.recipients self.assertEqual(