SparkPost: error on features incompatible with template_id

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 <https://anymail.dev/en/latest/esps/sparkpost/#sparkpost-template-limitations>`__.
+
 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 <https://anymail.dev/en/stable/esps/sendinblue/#sendinblue-inbound>`_.)
+  `docs <https://anymail.dev/en/stable/esps/sendinblue/#sendinblue-inbound>`__.)
 
 * **SendGrid:** Support multiple ``reply_to`` addresses.
   (Thanks to `@gdvalderrama`_ for pointing out the new API.)

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.<field> -> 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"]:

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 <batch-send>` 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 <sparkpost-template-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=["[email protected]", "Bob <[email protected]>"]
       )
       message.template_id = "11806290401558530"  # SparkPost id
+      message.from_email = None  # must set after constructor (see below)
       message.merge_data = {
           '[email protected]': {'name': "Alice", 'order_no': "12345"},
           '[email protected]': {'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`_,

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="[email protected]", to=["[email protected]"]
-        )
+        message = mail.EmailMultiAlternatives(to=["[email protected]"])
+        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=["[email protected]"], 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=["[email protected]"],
+            from_email="[email protected]",
+            reply_to=["[email protected]"],
+            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'

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(