From a737332111a817040aaf8ce86ded15f4cd91c72c Mon Sep 17 00:00:00 2001 From: Janne Thoft Date: Mon, 28 Jan 2019 11:42:22 +0100 Subject: [PATCH 1/7] Support for per-message meta data for SendGrid --- anymail/backends/base.py | 4 ++++ anymail/backends/sendgrid.py | 43 +++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/anymail/backends/base.py b/anymail/backends/base.py index 2d318794..2bcbd258 100644 --- a/anymail/backends/base.py +++ b/anymail/backends/base.py @@ -245,6 +245,7 @@ class BasePayload(object): ('template_id', last, force_non_lazy), ('merge_data', combine, force_non_lazy_dict), ('merge_global_data', combine, force_non_lazy_dict), + ('merge_metadata', combine, force_non_lazy_dict), ('esp_extra', combine, force_non_lazy_dict), ) esp_message_attrs = () # subclasses can override @@ -495,6 +496,9 @@ def set_merge_data(self, merge_data): def set_merge_global_data(self, merge_global_data): self.unsupported_feature("merge_global_data") + def set_merge_metadata(self, merge_metadata): + self.unsupported_feature("merge_metadata") + # ESP-specific payload construction def set_esp_extra(self, extra): self.unsupported_feature("esp_extra") diff --git a/anymail/backends/sendgrid.py b/anymail/backends/sendgrid.py index 91016c5f..350e5d39 100644 --- a/anymail/backends/sendgrid.py +++ b/anymail/backends/sendgrid.py @@ -101,6 +101,7 @@ def serialize_data(self): if self.generate_message_id: self.set_anymail_id() self.build_merge_data() + self.build_merge_metadata() if not self.data["headers"]: del self.data["headers"] # don't send empty headers @@ -204,6 +205,37 @@ def build_merge_data_legacy(self): "Search SENDGRID_MERGE_FIELD_FORMAT in the Anymail docs for more info.", AnymailWarning) + def build_merge_metadata(self): + if self.merge_metadata is None: + return + + if self.merge_data is not None: + # Burst apart each to-email in personalizations[0] into a separate + # personalization, and add merge_metadata for that recipient + assert len(self.data["personalizations"]) == 1 + base_personalizations = self.data["personalizations"].pop() + to_list = base_personalizations.pop("to") # {email, name?} for each message.to + for recipient in to_list: + personalization = base_personalizations.copy() # captures cc, bcc, and any esp_extra + personalization["to"] = [recipient] + self.data["personalizations"].append(personalization) + + for personalization in self.data["personalizations"]: + recipient_email = personalization["to"][0]["email"] + recipient_metadata = self.merge_metadata.get(recipient_email) + if recipient_metadata: + recipient_custom_args = self.transform_metadata(recipient_metadata) + + if self.data.get("custom_args"): + merged_custom_args = {} + merged_custom_args.update(self.data.get("custom_args")) + merged_custom_args.update(recipient_custom_args) + recipient_custom_args = merged_custom_args + del self.data["custom_args"] + + personalization["custom_args"] = recipient_custom_args + + # # Payload construction # @@ -296,11 +328,14 @@ def add_attachment(self, attachment): self.data.setdefault("attachments", []).append(att) def set_metadata(self, metadata): + self.data["custom_args"] = self.transform_metadata(metadata) + + def transform_metadata(self, metadata): # SendGrid requires custom_args values to be strings -- not integers. # (And issues the cryptic error {"field": null, "message": "Bad Request", "help": null} # if they're not.) # We'll stringify ints and floats; anything else is the caller's responsibility. - self.data["custom_args"] = { + return { k: str(v) if isinstance(v, BASIC_NUMERIC_TYPES) else v for k, v in metadata.items() } @@ -344,6 +379,12 @@ def set_merge_global_data(self, merge_global_data): # template type and merge_field_format. self.merge_global_data = merge_global_data + def set_merge_metadata(self, merge_metadata): + # Becomes personalizations[...]['custom_args'] in + # build_merge_data, after we know recipients, template type, + # and merge_field_format. + self.merge_metadata = merge_metadata + def set_esp_extra(self, extra): self.merge_field_format = extra.pop("merge_field_format", self.merge_field_format) self.use_dynamic_template = extra.pop("use_dynamic_template", self.use_dynamic_template) From 9202ad241a16fe6817395c681349ecd926257b3f Mon Sep 17 00:00:00 2001 From: Janne Thoft Date: Mon, 28 Jan 2019 12:27:49 +0100 Subject: [PATCH 2/7] Fix merge_metadata initialization bug --- anymail/backends/sendgrid.py | 1 + 1 file changed, 1 insertion(+) diff --git a/anymail/backends/sendgrid.py b/anymail/backends/sendgrid.py index 350e5d39..2c878178 100644 --- a/anymail/backends/sendgrid.py +++ b/anymail/backends/sendgrid.py @@ -77,6 +77,7 @@ def __init__(self, message, defaults, backend, *args, **kwargs): self.merge_field_format = backend.merge_field_format self.merge_data = None # late-bound per-recipient data self.merge_global_data = None + self.merge_metadata = None http_headers = kwargs.pop('headers', {}) http_headers['Authorization'] = 'Bearer %s' % backend.api_key From e1027ec95dc325eeed90268c0311caf9146ae10b Mon Sep 17 00:00:00 2001 From: Janne Thoft Date: Mon, 28 Jan 2019 12:43:34 +0100 Subject: [PATCH 3/7] Fix flake8 error --- anymail/backends/sendgrid.py | 1 - 1 file changed, 1 deletion(-) diff --git a/anymail/backends/sendgrid.py b/anymail/backends/sendgrid.py index 2c878178..c3280eb8 100644 --- a/anymail/backends/sendgrid.py +++ b/anymail/backends/sendgrid.py @@ -236,7 +236,6 @@ def build_merge_metadata(self): personalization["custom_args"] = recipient_custom_args - # # Payload construction # From 8afad8f746f8dcbc3a18feaa27e45677cf236213 Mon Sep 17 00:00:00 2001 From: medmunds Date: Tue, 5 Feb 2019 17:15:27 -0800 Subject: [PATCH 4/7] Add merge_metadata tests (Currently failing: tests uncover several issues.) --- tests/test_sendgrid_backend.py | 129 ++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/tests/test_sendgrid_backend.py b/tests/test_sendgrid_backend.py index b8d112a7..3c83c90b 100644 --- a/tests/test_sendgrid_backend.py +++ b/tests/test_sendgrid_backend.py @@ -11,6 +11,7 @@ from django.core import mail from django.test import SimpleTestCase, override_settings, tag from django.utils.timezone import get_fixed_timezone, override as override_current_timezone +from mock import patch from anymail.exceptions import (AnymailAPIError, AnymailConfigurationError, AnymailSerializationError, AnymailUnsupportedFeature, AnymailWarning) @@ -32,6 +33,13 @@ class SendGridBackendMockAPITestCase(RequestsBackendMockAPITestCase): def setUp(self): super(SendGridBackendMockAPITestCase, self).setUp() + + # Patch uuid4 to generate predictable anymail_ids for testing + patch_uuid4 = patch('anymail.backends.sendgrid.uuid.uuid4', + side_effect=["mocked-uuid-%d" % n for n in range(1, 5)]) + patch_uuid4.start() + self.addCleanup(patch_uuid4.stop) + # Simple message useful for many tests self.message = mail.EmailMultiAlternatives('Subject', 'Text Body', 'from@example.com', ['to@example.com']) @@ -57,7 +65,7 @@ def test_send_mail(self): 'to': [{'email': "to@example.com"}], }]) # make sure the backend assigned the anymail_id for event tracking and notification - self.assertUUIDIsValid(data['custom_args']['anymail_id']) + self.assertEqual(data['custom_args']['anymail_id'], 'mocked-uuid-1') def test_name_addr(self): """Make sure RFC2822 name-addr format (with display-name) is allowed @@ -118,7 +126,7 @@ def test_email_message(self): 'Message-ID': "", }) # make sure custom Message-ID also added to custom_args - self.assertUUIDIsValid(data['custom_args']['anymail_id']) + self.assertEqual(data['custom_args']['anymail_id'], 'mocked-uuid-1') def test_html_message(self): text_content = 'This is an important message.' @@ -573,6 +581,121 @@ def test_legacy_warn_if_no_global_merge_field_delimiters(self): with self.assertWarnsRegex(AnymailWarning, r'SENDGRID_MERGE_FIELD_FORMAT'): self.message.send() + def test_merge_metadata(self): + self.message.to = ['alice@example.com', 'Bob '] + self.message.merge_metadata = { + 'alice@example.com': {'order_id': 123}, + 'bob@example.com': {'order_id': 678, 'tier': 'premium'}, + } + self.message.send() + data = self.get_api_call_json() + self.assertEqual(data['personalizations'], [ + {'to': [{'email': 'alice@example.com'}], + 'custom_args': {'order_id': '123'}}, + {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], + 'custom_args': {'order_id': '678', 'tier': 'premium'}}, + ]) + self.assertEqual(data['custom_args'], {'anymail_id': 'mocked-uuid-1'}) + + def test_metadata_with_merge_metadata(self): + # Per SendGrid docs: "personalizations[x].custom_args will be merged + # with message level custom_args, overriding any conflicting keys." + # So there's no need to merge global metadata with per-recipient merge_metadata + # (like we have to for template merge_global_data and merge_data). + self.message.to = ['alice@example.com', 'Bob '] + self.message.metadata = {'tier': 'basic', 'batch': 'ax24'} + self.message.merge_metadata = { + 'alice@example.com': {'order_id': 123}, + 'bob@example.com': {'order_id': 678, 'tier': 'premium'}, + } + self.message.send() + data = self.get_api_call_json() + self.assertEqual(data['personalizations'], [ + {'to': [{'email': 'alice@example.com'}], + 'custom_args': {'order_id': '123'}}, + {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], + 'custom_args': {'order_id': '678', 'tier': 'premium'}}, + ]) + self.assertEqual(data['custom_args'], + {'tier': 'basic', 'batch': 'ax24', 'anymail_id': 'mocked-uuid-1'}) + + def test_merge_metadata_with_merge_data(self): + # (using dynamic templates) + self.message.to = ['alice@example.com', 'Bob '] + self.message.cc = ['cc@example.com'] # gets applied to *each* recipient in a merge + self.message.template_id = "d-5a963add2ec84305813ff860db277d7a" + self.message.merge_data = { + 'alice@example.com': {'name': "Alice", 'group': "Developers"}, + 'bob@example.com': {'name': "Bob"} + # and no data for celia@example.com + } + self.message.merge_global_data = { + 'group': "Users", + 'site': "ExampleCo", + } + self.message.merge_metadata = { + 'alice@example.com': {'order_id': 123}, + 'bob@example.com': {'order_id': 678, 'tier': 'premium'}, + # and no metadata for celia@example.com + } + self.message.send() + data = self.get_api_call_json() + self.assertEqual(data['personalizations'], [ + {'to': [{'email': 'alice@example.com'}], + 'cc': [{'email': 'cc@example.com'}], # all recipients get the cc + 'dynamic_template_data': { + 'name': "Alice", 'group': "Developers", 'site': "ExampleCo"}, + 'custom_args': {'order_id': '123'}}, + {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], + 'dynamic_template_data': { + 'name': "Bob", 'group': "Users", 'site': "ExampleCo"}, + 'custom_args': {'order_id': '678', 'tier': 'premium'}}, + {'to': [{'email': 'celia@example.com'}], + 'cc': [{'email': 'cc@example.com'}], + 'dynamic_template_data': { + 'group': "Users", 'site': "ExampleCo"}}, + ]) + + def test_merge_metadata_with_legacy_template(self): + self.message.to = ['alice@example.com', 'Bob '] + self.message.cc = ['cc@example.com'] # gets applied to *each* recipient in a merge + self.message.template_id = "5a963add2ec84305813ff860db277d7a" + self.message.esp_extra = {'merge_field_format': ':{}'} + self.message.merge_data = { + 'alice@example.com': {'name': "Alice", 'group': "Developers"}, + 'bob@example.com': {'name': "Bob"} + # and no data for celia@example.com + } + self.message.merge_global_data = { + 'group': "Users", + 'site': "ExampleCo", + } + self.message.merge_metadata = { + 'alice@example.com': {'order_id': 123}, + 'bob@example.com': {'order_id': 678, 'tier': 'premium'}, + # and no metadata for celia@example.com + } + self.message.send() + data = self.get_api_call_json() + self.assertEqual(data['personalizations'], [ + {'to': [{'email': 'alice@example.com'}], + 'cc': [{'email': 'cc@example.com'}], # all recipients get the cc + 'custom_args': {'order_id': '123'}, + 'substitutions': {':name': "Alice", ':group': "Developers", ':site': ":site"}}, + {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], + 'cc': [{'email': 'cc@example.com'}], + 'custom_args': {'order_id': '678', 'tier': 'premium'}, + 'substitutions': {':name': "Bob", ':group': ":group", ':site': ":site"}}, + {'to': [{'email': 'celia@example.com'}], + 'cc': [{'email': 'cc@example.com'}], + # no custom_args + 'substitutions': {':group': ":group", ':site': ":site"}}, + ]) + self.assertEqual(data['sections'], { + ':group': "Users", + ':site': "ExampleCo", + }) + @override_settings(ANYMAIL_SENDGRID_GENERATE_MESSAGE_ID=False) # else we force custom_args def test_default_omits_options(self): """Make sure by default we don't send any ESP-specific options. @@ -666,7 +789,7 @@ def test_send_attaches_anymail_status(self): sent = msg.send() self.assertEqual(sent, 1) self.assertEqual(msg.anymail_status.status, {'queued'}) - self.assertUUIDIsValid(msg.anymail_status.message_id) # don't know exactly what it'll be + self.assertEqual(msg.anymail_status.message_id, 'mocked-uuid-1') self.assertEqual(msg.anymail_status.recipients['to1@example.com'].status, 'queued') self.assertEqual(msg.anymail_status.recipients['to1@example.com'].message_id, msg.anymail_status.message_id) From 6e36f7c7f7052b6164d646cac9964e6ce86de7a1 Mon Sep 17 00:00:00 2001 From: Janne Thoft Date: Thu, 7 Feb 2019 14:23:37 +0100 Subject: [PATCH 5/7] Fix bugs from review --- anymail/backends/sendgrid.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/anymail/backends/sendgrid.py b/anymail/backends/sendgrid.py index c3280eb8..de052963 100644 --- a/anymail/backends/sendgrid.py +++ b/anymail/backends/sendgrid.py @@ -210,7 +210,7 @@ def build_merge_metadata(self): if self.merge_metadata is None: return - if self.merge_data is not None: + if self.merge_data is None: # Burst apart each to-email in personalizations[0] into a separate # personalization, and add merge_metadata for that recipient assert len(self.data["personalizations"]) == 1 @@ -221,21 +221,23 @@ def build_merge_metadata(self): personalization["to"] = [recipient] self.data["personalizations"].append(personalization) + global_custom_args = self.data.get("custom_args") for personalization in self.data["personalizations"]: recipient_email = personalization["to"][0]["email"] recipient_metadata = self.merge_metadata.get(recipient_email) if recipient_metadata: recipient_custom_args = self.transform_metadata(recipient_metadata) - if self.data.get("custom_args"): + if global_custom_args: merged_custom_args = {} - merged_custom_args.update(self.data.get("custom_args")) + merged_custom_args.update(global_custom_args) merged_custom_args.update(recipient_custom_args) recipient_custom_args = merged_custom_args - del self.data["custom_args"] personalization["custom_args"] = recipient_custom_args + del self.data["custom_args"] + # # Payload construction # From fb7784ab9d702e14a1fe1d78a4e6df963ccf2d0b Mon Sep 17 00:00:00 2001 From: medmunds Date: Thu, 21 Feb 2019 12:06:48 -0800 Subject: [PATCH 6/7] Fix copy/paste errors in two tests --- tests/test_sendgrid_backend.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_sendgrid_backend.py b/tests/test_sendgrid_backend.py index 3c83c90b..e8bf7f7a 100644 --- a/tests/test_sendgrid_backend.py +++ b/tests/test_sendgrid_backend.py @@ -621,7 +621,7 @@ def test_metadata_with_merge_metadata(self): def test_merge_metadata_with_merge_data(self): # (using dynamic templates) - self.message.to = ['alice@example.com', 'Bob '] + self.message.to = ['alice@example.com', 'Bob ', 'celia@example.com'] self.message.cc = ['cc@example.com'] # gets applied to *each* recipient in a merge self.message.template_id = "d-5a963add2ec84305813ff860db277d7a" self.message.merge_data = { @@ -647,6 +647,7 @@ def test_merge_metadata_with_merge_data(self): 'name': "Alice", 'group': "Developers", 'site': "ExampleCo"}, 'custom_args': {'order_id': '123'}}, {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], + 'cc': [{'email': 'cc@example.com'}], 'dynamic_template_data': { 'name': "Bob", 'group': "Users", 'site': "ExampleCo"}, 'custom_args': {'order_id': '678', 'tier': 'premium'}}, @@ -657,7 +658,7 @@ def test_merge_metadata_with_merge_data(self): ]) def test_merge_metadata_with_legacy_template(self): - self.message.to = ['alice@example.com', 'Bob '] + self.message.to = ['alice@example.com', 'Bob ', 'celia@example.com'] self.message.cc = ['cc@example.com'] # gets applied to *each* recipient in a merge self.message.template_id = "5a963add2ec84305813ff860db277d7a" self.message.esp_extra = {'merge_field_format': ':{}'} From d65efa3e648d5c2c5b96ba8d6feeecc6227d9f6f Mon Sep 17 00:00:00 2001 From: medmunds Date: Thu, 21 Feb 2019 12:10:11 -0800 Subject: [PATCH 7/7] Retain top-level custom_args for global `merge_data` SendGrid supports both top-level and individual custom_args, and seems to merge them properly. Use their API feature to simplify our code. --- anymail/backends/sendgrid.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/anymail/backends/sendgrid.py b/anymail/backends/sendgrid.py index de052963..31514084 100644 --- a/anymail/backends/sendgrid.py +++ b/anymail/backends/sendgrid.py @@ -221,23 +221,13 @@ def build_merge_metadata(self): personalization["to"] = [recipient] self.data["personalizations"].append(personalization) - global_custom_args = self.data.get("custom_args") for personalization in self.data["personalizations"]: recipient_email = personalization["to"][0]["email"] recipient_metadata = self.merge_metadata.get(recipient_email) if recipient_metadata: recipient_custom_args = self.transform_metadata(recipient_metadata) - - if global_custom_args: - merged_custom_args = {} - merged_custom_args.update(global_custom_args) - merged_custom_args.update(recipient_custom_args) - recipient_custom_args = merged_custom_args - personalization["custom_args"] = recipient_custom_args - del self.data["custom_args"] - # # Payload construction #