From 753c8953012c7333e02f36639924da38787b14ed Mon Sep 17 00:00:00 2001 From: medmunds Date: Wed, 5 Sep 2018 12:41:33 -0700 Subject: [PATCH] Postmark: Fix Postmark error on empty subject/body with `template_id`. Postmark issues an error if Django's default empty strings are used with template sends. Include template send in Postmark integration tests. (Requires real Postmark API token -- templates aren't testable with Postmark's sandbox token.) Fixes #121 --- CHANGELOG.rst | 11 +++++++++++ anymail/backends/postmark.py | 5 +++++ docs/esps/postmark.rst | 8 +++----- tests/test_postmark_backend.py | 29 +++++++++++++---------------- tests/test_postmark_integration.py | 21 +++++++++++++++++++++ 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1507202..03ba543 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -25,6 +25,17 @@ Release history ^^^^^^^^^^^^^^^ .. This extra heading level keeps the ToC from becoming unmanageably long +v4.2 +---- + +*In development* + +Fixes +~~~~~ + +* **Postmark:** When using `template_id`, ignore empty subject and body. (Postmark + issues an error if Django's default empty strings are used with template sends.) + v4.1 ---- diff --git a/anymail/backends/postmark.py b/anymail/backends/postmark.py index 653fd01..e716bbb 100644 --- a/anymail/backends/postmark.py +++ b/anymail/backends/postmark.py @@ -198,6 +198,11 @@ class PostmarkPayload(RequestsPayload): def set_template_id(self, template_id): self.data["TemplateId"] = template_id + # Subject, TextBody, and HtmlBody aren't allowed with TemplateId; + # delete Django default subject and body empty strings: + for field in ("Subject", "TextBody", "HtmlBody"): + if field in self.data and not self.data[field]: + del self.data[field] # merge_data: Postmark doesn't support per-recipient substitutions diff --git a/docs/esps/postmark.rst b/docs/esps/postmark.rst index 606ffa2..5714d36 100644 --- a/docs/esps/postmark.rst +++ b/docs/esps/postmark.rst @@ -142,8 +142,8 @@ message attribute: .. code-block:: python message = EmailMessage( - ... - subject=None, # use template subject + # (subject and body come from the template, so don't include those) + from_email="from@example.com", to=["alice@example.com"] # single recipient... # ...multiple to emails would all get the same message # (and would all see each other's emails in the "to" header) @@ -159,9 +159,7 @@ message attribute: ], } -Set the EmailMessage's subject to `None` to use the subject from -your Postmark template, or supply a subject with the message to override -the template value. +Postmark does not allow overriding the message's subject or body with a template. See this `Postmark blog post on templates`_ for more information. diff --git a/tests/test_postmark_backend.py b/tests/test_postmark_backend.py index 55210f5..c72bc9a 100644 --- a/tests/test_postmark_backend.py +++ b/tests/test_postmark_backend.py @@ -13,7 +13,7 @@ from django.test.utils import override_settings from anymail.exceptions import ( AnymailAPIError, AnymailSerializationError, AnymailUnsupportedFeature, AnymailRecipientsRefused, AnymailInvalidAddress) -from anymail.message import attach_inline_image_file +from anymail.message import attach_inline_image_file, AnymailMessage from .mock_requests_backend import RequestsBackendMockAPITestCase, SessionSharingTestCasesMixin from .utils import sample_image_content, sample_image_path, SAMPLE_IMAGE_FILENAME, AnymailTestMixin, decode_att @@ -368,14 +368,22 @@ class PostmarkBackendAnymailFeatureTests(PostmarkBackendMockAPITestCase): self.assertEqual(data['TrackLinks'], 'None') def test_template(self): - self.message.template_id = 1234567 - # Postmark doesn't support per-recipient merge_data - self.message.merge_global_data = {'name': "Alice", 'group': "Developers"} - self.message.send() + message = AnymailMessage( + # Omit subject and body (Postmark prohibits them with templates) + from_email='from@example.com', to=['to@example.com'], + template_id=1234567, + # Postmark doesn't support per-recipient merge_data + merge_global_data={'name': "Alice", 'group': "Developers"}, + ) + message.send() self.assert_esp_called('/email/withTemplate/') data = self.get_api_call_json() self.assertEqual(data['TemplateId'], 1234567) self.assertEqual(data['TemplateModel'], {'name': "Alice", 'group': "Developers"}) + # Make sure Django default subject and body didn't end up in the payload: + self.assertNotIn('Subject', data) + self.assertNotIn('HtmlBody', data) + self.assertNotIn('TextBody', data) def test_merge_data(self): self.message.merge_data = { @@ -384,17 +392,6 @@ class PostmarkBackendAnymailFeatureTests(PostmarkBackendMockAPITestCase): with self.assertRaisesMessage(AnymailUnsupportedFeature, 'merge_data'): self.message.send() - def test_missing_subject(self): - """Make sure a missing subject omits Subject from API call. - - (Allows use of template subject) - """ - self.message.template_id = 1234567 - self.message.subject = None - self.message.send() - data = self.get_api_call_json() - self.assertNotIn('Subject', data) - def test_default_omits_options(self): """Make sure by default we don't send any ESP-specific options. diff --git a/tests/test_postmark_integration.py b/tests/test_postmark_integration.py index 79e5908..9ad80e7 100644 --- a/tests/test_postmark_integration.py +++ b/tests/test_postmark_integration.py @@ -1,3 +1,4 @@ +import os import unittest from django.test import SimpleTestCase @@ -9,6 +10,12 @@ from anymail.message import AnymailMessage from .utils import AnymailTestMixin, sample_image_path, RUN_LIVE_TESTS +# For most integration tests, Postmark's sandboxed "POSTMARK_API_TEST" token is used. +# But to test template sends, a real Postmark server token and template id are needed: +POSTMARK_TEST_SERVER_TOKEN = os.getenv('POSTMARK_TEST_SERVER_TOKEN') +POSTMARK_TEST_TEMPLATE_ID = os.getenv('POSTMARK_TEST_TEMPLATE_ID') + + @unittest.skipUnless(RUN_LIVE_TESTS, "RUN_LIVE_TESTS disabled in this environment") @override_settings(ANYMAIL_POSTMARK_SERVER_TOKEN="POSTMARK_API_TEST", EMAIL_BACKEND="anymail.backends.postmark.EmailBackend") @@ -76,6 +83,20 @@ class PostmarkBackendIntegrationTests(SimpleTestCase, AnymailTestMixin): self.assertEqual(err.status_code, 422) self.assertIn("Invalid 'From' address", str(err)) + @unittest.skipUnless(POSTMARK_TEST_SERVER_TOKEN and POSTMARK_TEST_TEMPLATE_ID, + "Set POSTMARK_TEST_SERVER_TOKEN and POSTMARK_TEST_TEMPLATE_ID " + "environment variables to run Postmark template integration tests") + @override_settings(ANYMAIL_POSTMARK_SERVER_TOKEN=POSTMARK_TEST_SERVER_TOKEN) + def test_template(self): + message = AnymailMessage( + from_email="from@test-pm.anymail.info", + to=["test+to1@anymail.info"], + template_id=POSTMARK_TEST_TEMPLATE_ID, + merge_global_data={"name": "Test Recipient", "order_no": "12345"}, + ) + message.send() + self.assertEqual(message.anymail_status.status, {'sent'}) + @override_settings(ANYMAIL_POSTMARK_SERVER_TOKEN="Hey, that's not a server token!") def test_invalid_server_token(self): with self.assertRaises(AnymailAPIError) as cm: