From f95be248ec466f025be19910284f5edbe21b2d19 Mon Sep 17 00:00:00 2001 From: medmunds Date: Fri, 24 Jun 2016 12:13:32 -0700 Subject: [PATCH] SparkPost: remove empty content params with template_id When using a stored template, SparkPost disallows subject, text, and html. Django's EmailMessage default empty strings are enough to provoke "Both content object and template_id are specified" from SparkPost, so remove them (if empty) when using stored templates. Update docs and tests; add integration test for template_id. Fixes #24 --- anymail/backends/sparkpost.py | 9 +++++++++ docs/esps/sparkpost.rst | 7 +++++-- tests/test_sparkpost_backend.py | 9 +++++++-- tests/test_sparkpost_integration.py | 17 +++++++++++++++++ 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/anymail/backends/sparkpost.py b/anymail/backends/sparkpost.py index 83605ec..dad7dfb 100644 --- a/anymail/backends/sparkpost.py +++ b/anymail/backends/sparkpost.py @@ -104,6 +104,15 @@ class SparkPostPayload(BasePayload): if recipients: self.params['recipients'] = recipients + # Must remove empty string "content" params when using stored template + if self.params.get('template', None): + for content_param in ['subject', 'text', 'html']: + try: + if not self.params[content_param]: + del self.params[content_param] + except KeyError: + pass + return self.params def set_from_email(self, email): diff --git a/docs/esps/sparkpost.rst b/docs/esps/sparkpost.rst index 57166e9..066ae8e 100644 --- a/docs/esps/sparkpost.rst +++ b/docs/esps/sparkpost.rst @@ -140,8 +140,11 @@ 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. Alternatively, you can refer to merge fields -directly in an EmailMessage---the message itself is used as an +template's unique id. (When using a stored template, SparkPost prohibits +setting the EmailMessage's subject, text body, or html body.) + +Alternatively, you can refer to merge fields directly in an EmailMessage's +subject, body, and other fields---the message itself is used as an on-the-fly template. In either case, supply the merge data values with Anymail's diff --git a/tests/test_sparkpost_backend.py b/tests/test_sparkpost_backend.py index 942f229..2333be1 100644 --- a/tests/test_sparkpost_backend.py +++ b/tests/test_sparkpost_backend.py @@ -390,10 +390,15 @@ class SparkPostBackendAnymailFeatureTests(SparkPostBackendMockAPITestCase): self.assertEqual(params['track_clicks'], True) def test_template_id(self): - self.message.template_id = "welcome_template" - self.message.send() + message = mail.EmailMultiAlternatives(from_email='from@example.com', to=['to@example.com']) + message.template_id = "welcome_template" + message.send() params = self.get_send_params() self.assertEqual(params['template'], "welcome_template") + # SparkPost disallows all content (even empty strings) with stored template: + self.assertNotIn('subject', params) + self.assertNotIn('text', params) + self.assertNotIn('html', params) def test_merge_data(self): self.set_mock_response(accepted=2) diff --git a/tests/test_sparkpost_integration.py b/tests/test_sparkpost_integration.py index 482c6f8..74d9c30 100644 --- a/tests/test_sparkpost_integration.py +++ b/tests/test_sparkpost_integration.py @@ -107,6 +107,23 @@ class SparkPostBackendIntegrationTests(SimpleTestCase, AnymailTestMixin): self.assertEqual(recipient_status['to1@test.sink.sparkpostmail.com'].status, 'queued') self.assertEqual(recipient_status['to2@test.sink.sparkpostmail.com'].status, 'queued') + def test_stored_template(self): + message = AnymailMessage( + template_id='test-template', # a real template in our SparkPost test account + to=["to1@test.sink.sparkpostmail.com"], + merge_data={ + 'to1@test.sink.sparkpostmail.com': { + 'name': "Test Recipient", + } + }, + merge_global_data={ + 'order': '12345', + }, + ) + message.send() + recipient_status = message.anymail_status.recipients + self.assertEqual(recipient_status['to1@test.sink.sparkpostmail.com'].status, 'queued') + @override_settings(ANYMAIL_SPARKPOST_API_KEY="Hey, that's not an API key!") def test_invalid_api_key(self): with self.assertRaises(AnymailAPIError) as cm: