diff --git a/osf/email/__init__.py b/osf/email/__init__.py index 1cf39af809b..4193d9e5212 100644 --- a/osf/email/__init__.py +++ b/osf/email/__init__.py @@ -238,7 +238,33 @@ def send_email_over_smtp(to_email, notification_type, context, email_context): email.attach(attachment_name, attachment_content) email.send() -def send_email_with_send_grid(to_addr, notification_type, context, email_context=None): +def _build_sendgrid_personalizations(to_list, email_context=None, is_multiple=False): + """Build SendGrid personalizations for one shared message or one message per recipient. + + When ``is_multiple`` is True, each address gets its own personalization (separate + delivery; recipients do not see each other). CC/BCC are omitted in that mode to + avoid duplicating copies per recipient. + """ + email_context = email_context or {} + + if is_multiple: + # If we attached the same CC/BCC to every personalization in is_multiple mode, + # a batch of N recipients would produce N separate emails each including that CC + # so the CC address would get N copies of the same message + # TODO: decide if it's safe to add the CC/BCC to each personalization + return [{'to': [{'email': addr}]} for addr in to_list] + + personalization = {'to': [{'email': addr} for addr in to_list]} + cc_addr = email_context.get('cc_addr') + if cc_addr: + personalization['cc'] = [{'email': a} for a in ([cc_addr] if isinstance(cc_addr, str) else cc_addr)] + bcc_addr = email_context.get('bcc_addr') + if bcc_addr: + personalization['bcc'] = [{'email': a} for a in ([bcc_addr] if isinstance(bcc_addr, str) else bcc_addr)] + return [personalization] + + +def send_email_with_send_grid(to_addr, notification_type, context, email_context=None, *, is_multiple=False): email_context = email_context or {} to_list = [to_addr] if isinstance(to_addr, str) else [a for a in (to_addr or []) if a] @@ -256,18 +282,12 @@ def send_email_with_send_grid(to_addr, notification_type, context, email_context subject_tpl = getattr(notification_type, 'subject', None) subject = subject_tpl.format(**context) if subject_tpl else f'Notification: {getattr(notification_type, "name", "OSF")}' - personalization = {'to': [{'email': addr} for addr in to_list]} - cc_addr = email_context.get('cc_addr') - if cc_addr: - personalization['cc'] = [{'email': a} for a in ([cc_addr] if isinstance(cc_addr, str) else cc_addr)] - bcc_addr = email_context.get('bcc_addr') - if bcc_addr: - personalization['bcc'] = [{'email': a} for a in ([bcc_addr] if isinstance(bcc_addr, str) else bcc_addr)] - payload = { 'from': {'email': from_email}, 'subject': subject, - 'personalizations': [personalization], + 'personalizations': _build_sendgrid_personalizations( + to_list, email_context=email_context, is_multiple=is_multiple + ), 'content': [ {'type': 'text/html', 'value': html}, ], diff --git a/osf/email/notification_campaign.py b/osf/email/notification_campaign.py index fc62cc85810..ba516f59cbc 100644 --- a/osf/email/notification_campaign.py +++ b/osf/email/notification_campaign.py @@ -383,7 +383,12 @@ def send_campaign_batch( # NOTE: sendgrid bulk send feature has not been fully implemented and tested recipient_emails = list(valid_emails_qs.values_list('recipient_address', flat=True)) try: - send_email_with_send_grid(to_addr=recipient_emails, notification_type=notification_type, context=context) + send_email_with_send_grid( + to_addr=recipient_emails, + notification_type=notification_type, + context=context, + is_multiple=True, + ) valid_emails_qs.update(status=NotificationCampaignRecipientStatus.SENT, error_message=None) except Exception as exc: message = (f'[Notification Campaign #{campaign_id}] ERROR: ' diff --git a/osf_tests/test_notification_campaign.py b/osf_tests/test_notification_campaign.py index a26604afe43..9548e6e6cb8 100644 --- a/osf_tests/test_notification_campaign.py +++ b/osf_tests/test_notification_campaign.py @@ -6,6 +6,7 @@ from django.utils import timezone from django.db.models import Q +from osf.email import _build_sendgrid_personalizations from osf.email.notification_campaign import ( create_campaign_recipients, get_campaign_recipient_batches, @@ -573,10 +574,33 @@ def test_send_campaign_batch_sendgrid_bulk_success(self, mock_sendgrid, running_ recipient.refresh_from_db() running_campaign.refresh_from_db() mock_sendgrid.assert_called_once() + assert mock_sendgrid.call_args.kwargs.get('is_multiple') is True assert recipient.status == NotificationCampaignRecipientStatus.SENT assert running_campaign.sent_count == 1 assert running_campaign.failed_count == 0 + def test_build_sendgrid_personalizations_shared_to_list(self): + personalizations = _build_sendgrid_personalizations( + ['a@example.com', 'b@example.com'], + is_multiple=False, + ) + assert personalizations == [{ + 'to': [ + {'email': 'a@example.com'}, + {'email': 'b@example.com'}, + ], + }] + + def test_build_sendgrid_personalizations_one_per_recipient(self): + personalizations = _build_sendgrid_personalizations( + ['a@example.com', 'b@example.com'], + is_multiple=True, + ) + assert personalizations == [ + {'to': [{'email': 'a@example.com'}]}, + {'to': [{'email': 'b@example.com'}]}, + ] + @mock.patch( 'osf.email.notification_campaign.send_email_with_send_grid', side_effect=Exception('bulk failed'),