-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix sendgrid cc and bcc #33
Conversation
espwrap/adaptors/sendgrid_v3.py
Outdated
@@ -193,6 +183,15 @@ def message_constructor(self, to_send): | |||
|
|||
message.add_to(to_emails, is_multiple=True) | |||
|
|||
#CC and BCC | |||
for personalization in message.personalizations: | |||
if self.cc_list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to the the source for MassEmail
(linked here), self.cc_list
and self.bcc_list
are always initialized as empty lists. I think you can just simplify this to:
for cc_email in self.cc_list:
personalization.add_cc(...)
for bcc_email in self.bcc_list:
personalization.add_bcc(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooo nice catch 👍
Resolves a few interrelated issues with adding CC and BCC addresses to messages using the sendgrid V3 adapter:
add_bcc()
, messages are never delivered to the bcc addressadd_cc()
, sendgrid returns a400
Bad Response error with the following:Each email address in the personalization block should be unique between to, cc, and bcc. We found the first duplicate instance of [email@example.com] in the personalizations.1.bcc field
The two issues above come from a tiny typo, where the section for handling bccs in the
message_constructor
leads to pulling emails from the cc list instead of the bcc list (line 124)Fixing those issues then leads to:
add_cc()
oradd_bcc()
, sendgrid returns a400
Bad Request error with the following:The to array is required for all personalization objects, and must have at least one email object with a valid email address.
This is caused by using the
add_to
function withis_multiple=True
, which causes sendgrid to create a newPersonalization
for each recipient of the email message. In that case then, the bcc and cc fields need to be added on to each of the personalization objects, not the message object, so this reworks the logic to do that (semi-related StackOverflow example.)Also for now chose to keep the
is_multiple=True
behavior and work around it rather than another approach, to keep things as consistent as possible to how it works today (just with bcc and cc working); there certainly may be a more elegant sendgrid-approved approach! Also added a quick little cc/bcc addendum to the existing test formessage_constructor
(For SpotOn internal users, this is for jira ticket CSVC-520, discovered while working on CSVC-381 )