SendGrid: simplify personalizations processing; stop using "sections"

* Rework and simplify personalizations code (that had grown convoluted
  through several feature additions).

* Stop putting merge_global_data in legacy template "sections"; instead
  just merge it into individual personalization substitutions like we
  do for dynamic templates. (The "sections" version didn't add any
  functionality, had the potential for conflicts with the user's own
  template section tags, and was needlessly complex.)
This commit is contained in:
medmunds
2019-02-23 14:07:01 -08:00
parent f89d92bc37
commit d2d568b6d3
3 changed files with 67 additions and 116 deletions

View File

@@ -60,6 +60,9 @@ Fixes
(but no To addresses). Also, `message.anymail_status.recipients[email]` now includes (but no To addresses). Also, `message.anymail_status.recipients[email]` now includes
send status for Cc and Bcc recipients. (Thanks to `@ailionx`_ for reporting the error.) send status for Cc and Bcc recipients. (Thanks to `@ailionx`_ for reporting the error.)
* **SendGrid:** With legacy templates, stop (ab)using "sections" for merge_global_data.
This avoids potential conflicts with a template's own use of SendGrid section tags.
v5.0 v5.0
---- ----

View File

@@ -75,9 +75,9 @@ class SendGridPayload(RequestsPayload):
self.use_dynamic_template = False # how to represent merge_data self.use_dynamic_template = False # how to represent merge_data
self.message_id = None # Message-ID -- assigned in serialize_data unless provided in headers self.message_id = None # Message-ID -- assigned in serialize_data unless provided in headers
self.merge_field_format = backend.merge_field_format self.merge_field_format = backend.merge_field_format
self.merge_data = None # late-bound per-recipient data self.merge_data = {} # late-bound per-recipient data
self.merge_global_data = None self.merge_global_data = {}
self.merge_metadata = None self.merge_metadata = {}
http_headers = kwargs.pop('headers', {}) http_headers = kwargs.pop('headers', {})
http_headers['Authorization'] = 'Bearer %s' % backend.api_key http_headers['Authorization'] = 'Bearer %s' % backend.api_key
@@ -101,6 +101,8 @@ class SendGridPayload(RequestsPayload):
if self.generate_message_id: if self.generate_message_id:
self.set_anymail_id() self.set_anymail_id()
if self.is_batch():
self.expand_personalizations_for_batch()
self.build_merge_data() self.build_merge_data()
self.build_merge_metadata() self.build_merge_metadata()
@@ -115,91 +117,57 @@ class SendGridPayload(RequestsPayload):
self.message_id = str(uuid.uuid4()) self.message_id = str(uuid.uuid4())
self.data.setdefault("custom_args", {})["anymail_id"] = self.message_id self.data.setdefault("custom_args", {})["anymail_id"] = self.message_id
def expand_personalizations_for_batch(self):
"""Split data["personalizations"] into individual message for each recipient"""
assert len(self.data["personalizations"]) == 1
base_personalization = self.data["personalizations"].pop()
to_list = base_personalization.pop("to") # {email, name?} for each message.to
for recipient in to_list:
personalization = base_personalization.copy()
personalization["to"] = [recipient]
self.data["personalizations"].append(personalization)
def build_merge_data(self): def build_merge_data(self):
if self.use_dynamic_template: if self.merge_data or self.merge_global_data:
self.build_merge_data_dynamic() # Always build dynamic_template_data first,
else: # then convert it to legacy template format if needed
self.build_merge_data_legacy() for personalization in self.data["personalizations"]:
assert len(personalization["to"]) == 1
recipient_email = personalization["to"][0]["email"]
dynamic_template_data = self.merge_global_data.copy()
dynamic_template_data.update(self.merge_data.get(recipient_email, {}))
if dynamic_template_data:
personalization["dynamic_template_data"] = dynamic_template_data
def build_merge_data_dynamic(self): if not self.use_dynamic_template:
"""Set personalizations[...]['dynamic_template_data']""" self.convert_dynamic_template_data_to_legacy_substitutions()
if self.merge_global_data is not None:
assert len(self.data["personalizations"]) == 1
self.data["personalizations"][0].setdefault(
"dynamic_template_data", {}).update(self.merge_global_data)
if self.merge_data is not None: def convert_dynamic_template_data_to_legacy_substitutions(self):
# Burst apart each to-email in personalizations[0] into a separate """Change personalizations[...]['dynamic_template_data'] to ...['substitutions]"""
# personalization, and add merge_data 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, merge_global_data, esp_extra
personalization["to"] = [recipient]
try:
recipient_data = self.merge_data[recipient["email"]]
except KeyError:
pass # no merge_data for this recipient
else:
if "dynamic_template_data" in personalization:
# merge per-recipient data into (copy of) merge_global_data
personalization["dynamic_template_data"] = personalization["dynamic_template_data"].copy()
personalization["dynamic_template_data"].update(recipient_data)
else:
personalization["dynamic_template_data"] = recipient_data
self.data["personalizations"].append(personalization)
def build_merge_data_legacy(self):
"""Set personalizations[...]['substitutions'] and data['sections']"""
merge_field_format = self.merge_field_format or '{}' merge_field_format = self.merge_field_format or '{}'
if self.merge_data is not None: all_merge_fields = set()
# Burst apart each to-email in personalizations[0] into a separate for personalization in self.data["personalizations"]:
# personalization, and add merge_data for that recipient try:
assert len(self.data["personalizations"]) == 1 dynamic_template_data = personalization.pop("dynamic_template_data")
base_personalizations = self.data["personalizations"].pop() except KeyError:
to_list = base_personalizations.pop("to") # {email, name?} for each message.to pass # no substitutions for this recipient
all_fields = set() else:
for recipient in to_list: # Convert dynamic_template_data keys for substitutions, using merge_field_format
personalization = base_personalizations.copy() # captures cc, bcc, and any esp_extra personalization["substitutions"] = {
personalization["to"] = [recipient] merge_field_format.format(field): data
try: for field, data in dynamic_template_data.items()}
recipient_data = self.merge_data[recipient["email"]] all_merge_fields.update(dynamic_template_data.keys())
personalization["substitutions"] = {merge_field_format.format(field): data
for field, data in recipient_data.items()}
all_fields = all_fields.union(recipient_data.keys())
except KeyError:
pass # no merge_data for this recipient
self.data["personalizations"].append(personalization)
if self.merge_field_format is None and len(all_fields) and all(field.isalnum() for field in all_fields): if self.merge_field_format is None:
if all_merge_fields and all(field.isalnum() for field in all_merge_fields):
warnings.warn( warnings.warn(
"Your SendGrid merge fields don't seem to have delimiters, " "Your SendGrid merge fields don't seem to have delimiters, "
"which can cause unexpected results with Anymail's merge_data. " "which can cause unexpected results with Anymail's merge_data. "
"Search SENDGRID_MERGE_FIELD_FORMAT in the Anymail docs for more info.", "Search SENDGRID_MERGE_FIELD_FORMAT in the Anymail docs for more info.",
AnymailWarning) AnymailWarning)
if self.merge_global_data is not None: if self.merge_global_data and all(field.isalnum() for field in self.merge_global_data.keys()):
# (merge into any existing 'sections' from esp_extra)
self.data.setdefault("sections", {}).update({
merge_field_format.format(field): data
for field, data in self.merge_global_data.items()
})
# Confusingly, "Section tags have to be contained within a Substitution tag"
# (https://sendgrid.com/docs/API_Reference/SMTP_API/section_tags.html),
# so we need to insert a "-field-": "-field-" identity fallback for each
# missing global field in the recipient substitutions...
global_fields = [merge_field_format.format(field)
for field in self.merge_global_data.keys()]
for personalization in self.data["personalizations"]:
substitutions = personalization.setdefault("substitutions", {})
substitutions.update({field: field for field in global_fields
if field not in substitutions})
if (self.merge_field_format is None and
all(field.isalnum() for field in self.merge_global_data.keys())):
warnings.warn( warnings.warn(
"Your SendGrid global merge fields don't seem to have delimiters, " "Your SendGrid global merge fields don't seem to have delimiters, "
"which can cause unexpected results with Anymail's merge_data. " "which can cause unexpected results with Anymail's merge_data. "
@@ -207,26 +175,14 @@ class SendGridPayload(RequestsPayload):
AnymailWarning) AnymailWarning)
def build_merge_metadata(self): def build_merge_metadata(self):
if self.merge_metadata is None: if self.merge_metadata:
return for personalization in self.data["personalizations"]:
assert len(personalization["to"]) == 1
if self.merge_data is None: recipient_email = personalization["to"][0]["email"]
# Burst apart each to-email in personalizations[0] into a separate recipient_metadata = self.merge_metadata.get(recipient_email)
# personalization, and add merge_metadata for that recipient if recipient_metadata:
assert len(self.data["personalizations"]) == 1 recipient_custom_args = self.transform_metadata(recipient_metadata)
base_personalizations = self.data["personalizations"].pop() personalization["custom_args"] = recipient_custom_args
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)
personalization["custom_args"] = recipient_custom_args
# #
# Payload construction # Payload construction

View File

@@ -515,18 +515,15 @@ class SendGridBackendAnymailFeatureTests(SendGridBackendMockAPITestCase):
{'to': [{'email': 'alice@example.com'}], {'to': [{'email': 'alice@example.com'}],
'cc': [{'email': 'cc@example.com'}], # all recipients get the cc 'cc': [{'email': 'cc@example.com'}], # all recipients get the cc
'substitutions': {':name': "Alice", ':group': "Developers", 'substitutions': {':name': "Alice", ':group': "Developers",
':site': ":site"}}, # tell SG to look for global field in 'sections' ':site': "ExampleCo"}}, # merge_global_data merged
{'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}],
'cc': [{'email': 'cc@example.com'}], 'cc': [{'email': 'cc@example.com'}],
'substitutions': {':name': "Bob", ':group': ":group", ':site': ":site"}}, 'substitutions': {':name': "Bob", ':group': "Users", ':site': "ExampleCo"}},
{'to': [{'email': 'celia@example.com'}], {'to': [{'email': 'celia@example.com'}],
'cc': [{'email': 'cc@example.com'}], 'cc': [{'email': 'cc@example.com'}],
'substitutions': {':group': ":group", ':site': ":site"}}, # look for global fields in 'sections' 'substitutions': {':group': "Users", ':site': "ExampleCo"}},
]) ])
self.assertEqual(data['sections'], { self.assertNotIn('sections', data) # 'sections' no longer used for merge_global_data
':group': "Users",
':site': "ExampleCo",
})
@override_settings(ANYMAIL_SENDGRID_MERGE_FIELD_FORMAT=":{}") # :field as shown in SG examples @override_settings(ANYMAIL_SENDGRID_MERGE_FIELD_FORMAT=":{}") # :field as shown in SG examples
def test_legacy_merge_field_format_setting(self): def test_legacy_merge_field_format_setting(self):
@@ -541,11 +538,11 @@ class SendGridBackendAnymailFeatureTests(SendGridBackendMockAPITestCase):
data = self.get_api_call_json() data = self.get_api_call_json()
self.assertEqual(data['personalizations'], [ self.assertEqual(data['personalizations'], [
{'to': [{'email': 'alice@example.com'}], {'to': [{'email': 'alice@example.com'}],
'substitutions': {':name': "Alice", ':group': "Developers", ':site': ":site"}}, # keys changed to :field 'substitutions': {':name': "Alice", ':group': "Developers", # keys changed to :field
':site': "ExampleCo"}},
{'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}],
'substitutions': {':name': "Bob", ':site': ":site"}} 'substitutions': {':name': "Bob", ':site': "ExampleCo"}}
]) ])
self.assertEqual(data['sections'], {':site': "ExampleCo"})
def test_legacy_merge_field_format_esp_extra(self): def test_legacy_merge_field_format_esp_extra(self):
# Provide merge field delimiters for an individual message # Provide merge field delimiters for an individual message
@@ -560,11 +557,10 @@ class SendGridBackendAnymailFeatureTests(SendGridBackendMockAPITestCase):
data = self.get_api_call_json() data = self.get_api_call_json()
self.assertEqual(data['personalizations'], [ self.assertEqual(data['personalizations'], [
{'to': [{'email': 'alice@example.com'}], {'to': [{'email': 'alice@example.com'}],
'substitutions': {'*|name|*': "Alice", '*|group|*': "Developers", '*|site|*': "*|site|*"}}, 'substitutions': {'*|name|*': "Alice", '*|group|*': "Developers", '*|site|*': "ExampleCo"}},
{'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}],
'substitutions': {'*|name|*': "Bob", '*|site|*': "*|site|*"}} 'substitutions': {'*|name|*': "Bob", '*|site|*': "ExampleCo"}}
]) ])
self.assertEqual(data['sections'], {'*|site|*': "ExampleCo"})
# Make sure our esp_extra merge_field_format doesn't get sent to SendGrid API: # Make sure our esp_extra merge_field_format doesn't get sent to SendGrid API:
self.assertNotIn('merge_field_format', data) self.assertNotIn('merge_field_format', data)
@@ -682,20 +678,16 @@ class SendGridBackendAnymailFeatureTests(SendGridBackendMockAPITestCase):
{'to': [{'email': 'alice@example.com'}], {'to': [{'email': 'alice@example.com'}],
'cc': [{'email': 'cc@example.com'}], # all recipients get the cc 'cc': [{'email': 'cc@example.com'}], # all recipients get the cc
'custom_args': {'order_id': '123'}, 'custom_args': {'order_id': '123'},
'substitutions': {':name': "Alice", ':group': "Developers", ':site': ":site"}}, 'substitutions': {':name': "Alice", ':group': "Developers", ':site': "ExampleCo"}},
{'to': [{'email': 'bob@example.com', 'name': '"Bob"'}], {'to': [{'email': 'bob@example.com', 'name': '"Bob"'}],
'cc': [{'email': 'cc@example.com'}], 'cc': [{'email': 'cc@example.com'}],
'custom_args': {'order_id': '678', 'tier': 'premium'}, 'custom_args': {'order_id': '678', 'tier': 'premium'},
'substitutions': {':name': "Bob", ':group': ":group", ':site': ":site"}}, 'substitutions': {':name': "Bob", ':group': "Users", ':site': "ExampleCo"}},
{'to': [{'email': 'celia@example.com'}], {'to': [{'email': 'celia@example.com'}],
'cc': [{'email': 'cc@example.com'}], 'cc': [{'email': 'cc@example.com'}],
# no custom_args # no custom_args
'substitutions': {':group': ":group", ':site': ":site"}}, 'substitutions': {':group': "Users", ':site': "ExampleCo"}},
]) ])
self.assertEqual(data['sections'], {
':group': "Users",
':site': "ExampleCo",
})
@override_settings(ANYMAIL_SENDGRID_GENERATE_MESSAGE_ID=False) # else we force custom_args @override_settings(ANYMAIL_SENDGRID_GENERATE_MESSAGE_ID=False) # else we force custom_args
def test_default_omits_options(self): def test_default_omits_options(self):