diff --git a/.gitignore b/.gitignore index 27598510..1cecfa1c 100644 --- a/.gitignore +++ b/.gitignore @@ -64,6 +64,9 @@ target/ # Vim *.swp +# Emacs +*~ + # Local configuration overrides private.py diff --git a/ecommerce_worker/configuration/devstack.py b/ecommerce_worker/configuration/devstack.py index de906dfe..59e434da 100644 --- a/ecommerce_worker/configuration/devstack.py +++ b/ecommerce_worker/configuration/devstack.py @@ -1,5 +1,6 @@ import logging from logging.config import dictConfig +import os import yaml @@ -15,12 +16,13 @@ dictConfig(logger_config) # END LOGGING -filename = get_overrides_filename('ECOMMERCE_WORKER_CFG') -with open(filename) as f: - config_from_yaml = yaml.load(f) +if not os.environ.get('IGNORE_YAML_OVERRIDES'): + filename = get_overrides_filename('ECOMMERCE_WORKER_CFG') + with open(filename) as f: + config_from_yaml = yaml.load(f) -# Override base configuration with values from disk. -vars().update(config_from_yaml) + # Override base configuration with values from disk. + vars().update(config_from_yaml) # Apply any developer-defined overrides. try: diff --git a/ecommerce_worker/email/v1/braze/client.py b/ecommerce_worker/email/v1/braze/client.py index 02d75d1f..224eebdc 100644 --- a/ecommerce_worker/email/v1/braze/client.py +++ b/ecommerce_worker/email/v1/braze/client.py @@ -10,6 +10,8 @@ import requests from celery.utils.log import get_task_logger +from braze import client as edx_braze_client + from ecommerce_worker.email.v1.braze.exceptions import ( ConfigurationError, BrazeNotEnabled, @@ -22,7 +24,7 @@ log = get_task_logger(__name__) -def is_braze_enabled(site_code) -> bool: # pylint: disable=missing-function-docstring +def is_braze_enabled(site_code) -> bool: config = get_braze_configuration(site_code) return bool(config.get('BRAZE_ENABLE')) @@ -38,6 +40,23 @@ def get_braze_configuration(site_code): return config +def validate_braze_config(config, site_code): + """ + Raises if braze is not enabled or if either the + Rest or Webapp API keys are missing from the configuration. + """ + # Return if Braze integration disabled + if not config.get('BRAZE_ENABLE'): + msg = f'Braze is not enabled for site {site_code}' + log.error(msg) + raise BrazeNotEnabled(msg) + + if not (config.get('BRAZE_REST_API_KEY') and config.get('BRAZE_WEBAPP_API_KEY')): + msg = f'Required keys missing for site {site_code}' + log.error(msg) + raise ConfigurationError(msg) + + def get_braze_client(site_code): """ Returns a Braze client for the specified site. @@ -52,44 +71,21 @@ def get_braze_client(site_code): BrazeNotEnabled: If Braze is not enabled for the specified site. ConfigurationError: If either the Braze API key or Webapp key are not set for the site. """ - # Get configuration config = get_braze_configuration(site_code) - - # Return if Braze integration disabled - if not config.get('BRAZE_ENABLE'): - msg = f'Braze is not enabled for site {site_code}' - log.debug(msg) - raise BrazeNotEnabled(msg) - - rest_api_key = config.get('BRAZE_REST_API_KEY') - webapp_api_key = config.get('BRAZE_WEBAPP_API_KEY') - rest_api_url = config.get('REST_API_URL') - messages_send_endpoint = config.get('MESSAGES_SEND_ENDPOINT') - email_bounce_endpoint = config.get('EMAIL_BOUNCE_ENDPOINT') - new_alias_endpoint = config.get('NEW_ALIAS_ENDPOINT') - users_track_endpoint = config.get('USERS_TRACK_ENDPOINT') - export_id_endpoint = config.get('EXPORT_ID_ENDPOINT') - campaign_send_endpoint = config.get('CAMPAIGN_SEND_ENDPOINT') - enterprise_campaign_id = config.get('ENTERPRISE_CAMPAIGN_ID') - from_email = config.get('FROM_EMAIL') - - if not rest_api_key or not webapp_api_key: - msg = f'Required keys missing for site {site_code}' - log.error(msg) - raise ConfigurationError(msg) + validate_braze_config(config, site_code) return BrazeClient( - rest_api_key=rest_api_key, - webapp_api_key=webapp_api_key, - rest_api_url=rest_api_url, - messages_send_endpoint=messages_send_endpoint, - email_bounce_endpoint=email_bounce_endpoint, - new_alias_endpoint=new_alias_endpoint, - users_track_endpoint=users_track_endpoint, - export_id_endpoint=export_id_endpoint, - campaign_send_endpoint=campaign_send_endpoint, - enterprise_campaign_id=enterprise_campaign_id, - from_email=from_email, + rest_api_key=config.get('BRAZE_REST_API_KEY'), + webapp_api_key=config.get('BRAZE_WEBAPP_API_KEY'), + rest_api_url=config.get('REST_API_URL'), + messages_send_endpoint=config.get('MESSAGES_SEND_ENDPOINT'), + email_bounce_endpoint=config.get('EMAIL_BOUNCE_ENDPOINT'), + new_alias_endpoint=config.get('NEW_ALIAS_ENDPOINT'), + users_track_endpoint=config.get('USERS_TRACK_ENDPOINT'), + export_id_endpoint=config.get('EXPORT_ID_ENDPOINT'), + campaign_send_endpoint=config.get('CAMPAIGN_SEND_ENDPOINT'), + enterprise_campaign_id=config.get('ENTERPRISE_CAMPAIGN_ID'), + from_email=config.get('FROM_EMAIL'), ) @@ -526,3 +522,56 @@ def get_braze_external_id( return response["users"][0]["external_id"] return None + + +class EdxBrazeClient(edx_braze_client.BrazeClient): + """ + Wrapper around the edx-braze-client library BrazeClient class. + TODO: Deprecate ``BrazeClient`` above and use only this class + for Braze interactions. + """ + def __init__(self, site_code): + config = get_braze_configuration(site_code) + validate_braze_config(config, site_code) + + super().__init__( + api_key=config.get('BRAZE_REST_API_KEY'), + api_url=config.get('REST_API_URL'), + app_id=config.get('BRAZE_WEBAPP_API_KEY'), + ) + + def create_recipient( + self, + user_email, + lms_user_id, + trigger_properties=None, + ): + """ + Create a recipient object using the given user_email and lms_user_id. + """ + + user_alias = { + 'alias_label': 'Enterprise', + 'alias_name': user_email, + } + + # Identify the user alias in case it already exists. This is necessary so + # we don't accidently create a duplicate Braze profile. + self.identify_users([{ + 'external_id': lms_user_id, + 'user_alias': user_alias, + }]) + + attributes = { + "user_alias": user_alias, + "email": user_email, + "_update_existing_only": False, + } + + return { + 'external_user_id': lms_user_id, + 'attributes': attributes, + # If a profile does not already exist, Braze will create a new profile before sending a message. + 'send_to_existing_only': False, + 'trigger_properties': trigger_properties or {}, + } diff --git a/ecommerce_worker/email/v1/braze/tasks.py b/ecommerce_worker/email/v1/braze/tasks.py index 44e097e0..02192e78 100644 --- a/ecommerce_worker/email/v1/braze/tasks.py +++ b/ecommerce_worker/email/v1/braze/tasks.py @@ -1,15 +1,25 @@ """ This file contains celery task functionality for braze. """ +from operator import itemgetter +import braze.exceptions as edx_braze_exceptions from celery.utils.log import get_task_logger -from ecommerce_worker.email.v1.braze.client import get_braze_client, get_braze_configuration +from ecommerce_worker.email.v1.braze.client import ( + get_braze_client, + get_braze_configuration, + EdxBrazeClient, +) from ecommerce_worker.email.v1.braze.exceptions import BrazeError, BrazeRateLimitError, BrazeInternalServerError from ecommerce_worker.email.v1.utils import update_assignment_email_status logger = get_task_logger(__name__) +# Use a smaller countdown for the offer usage task, +# since the mgmt command that executes it blocks until the task is done/failed. +OFFER_USAGE_RETRY_DELAY_SECONDS = 10 + def send_offer_assignment_email_via_braze(self, user_email, offer_assignment_id, subject, email_body, sender_alias, reply_to, attachments, site_code): @@ -105,41 +115,54 @@ def send_offer_update_email_via_braze(self, user_email, subject, email_body, sen ) -def send_offer_usage_email_via_braze(self, emails, subject, email_body, reply_to, attachments, site_code): +def send_offer_usage_email_via_braze( + self, lms_user_ids_by_email, subject, email_body_variables, site_code, campaign_id=None +): """ Sends the offer usage email via braze. Args: self: Ignore. - emails (str): comma separated emails. + lms_user_ids_by_email (dict): Map of recipient email addresses to LMS user ids. subject (str): Email subject. - email_body (str): The body of the email. - reply_to (str): Enterprise Customer reply to address for email reply. - attachments (list): File attachment list with dicts having 'file_name' and 'url' keys. + email_body_variables (dict): key-value pairs that are injected into Braze email template for personalization. site_code (str): Identifier of the site sending the email. + campaign_id (str): Identifier of Braze API-triggered campaign to send message through; defaults + to config.ENTERPRISE_CODE_USAGE_CAMPAIGN_ID """ config = get_braze_configuration(site_code) try: - user_emails = list(emails.strip().split(",")) - braze_client = get_braze_client(site_code) - _send_braze_message( - braze_client, - email_ids=user_emails, - subject=subject, - body=email_body, - reply_to=reply_to, - attachments=attachments, - campaign_id=config.get('ENTERPRISE_CODE_USAGE_CAMPAIGN_ID'), - message_variation_id=config.get('ENTERPRISE_CODE_USAGE_MESSAGE_VARIATION_ID'), - ) - except (BrazeRateLimitError, BrazeInternalServerError) as exc: - raise self.retry(countdown=config.get('BRAZE_RETRY_SECONDS'), - max_retries=config.get('BRAZE_RETRY_ATTEMPTS')) from exc - except BrazeError: + braze_client = EdxBrazeClient(site_code) + + message_kwargs = { + 'campaign_id': campaign_id or config.get('ENTERPRISE_CODE_USAGE_CAMPAIGN_ID'), + 'recipients': [], + 'emails': [], + 'trigger_properties': { + 'subject': subject, + **email_body_variables, + }, + } + + for user_email, lms_user_id in sorted(lms_user_ids_by_email.items(), key=itemgetter(0)): + if lms_user_id: + recipient = braze_client.create_recipient(user_email, lms_user_id) + message_kwargs['recipients'].append(recipient) + else: + message_kwargs['emails'].append(user_email) + + braze_client.send_campaign_message(**message_kwargs) + except (edx_braze_exceptions.BrazeRateLimitError, edx_braze_exceptions.BrazeInternalServerError) as exc: + raise self.retry( + countdown=OFFER_USAGE_RETRY_DELAY_SECONDS, + max_retries=config.get('BRAZE_RETRY_ATTEMPTS') + ) from exc + except edx_braze_exceptions.BrazeError: logger.exception( '[Offer Usage] Error in offer usage notification with message --- ' - '{message}'.format(message=email_body) + '{message}'.format(message=email_body_variables) ) + raise def send_code_assignment_nudge_email_via_braze(self, email, subject, email_body, sender_alias, reply_to, # pylint: disable=invalid-name diff --git a/ecommerce_worker/email/v1/braze/tests/test_client.py b/ecommerce_worker/email/v1/braze/tests/test_client.py index 4b636c3f..3e4cd2a1 100644 --- a/ecommerce_worker/email/v1/braze/tests/test_client.py +++ b/ecommerce_worker/email/v1/braze/tests/test_client.py @@ -9,7 +9,7 @@ import ddt import responses -from ecommerce_worker.email.v1.braze.client import get_braze_client +from ecommerce_worker.email.v1.braze.client import get_braze_client, EdxBrazeClient from ecommerce_worker.email.v1.braze.exceptions import ( BrazeClientError, BrazeNotEnabled, @@ -19,32 +19,32 @@ ) +SITE_CODE = 'test' +BRAZE_OVERRIDES = { + SITE_CODE: { + 'BRAZE': { + 'BRAZE_ENABLE': True, + 'BRAZE_REST_API_KEY': 'rest_api_key', + 'BRAZE_WEBAPP_API_KEY': 'webapp_api_key', + 'REST_API_URL': 'https://rest.iad-06.braze.com', + 'MESSAGES_SEND_ENDPOINT': '/messages/send', + 'EMAIL_BOUNCE_ENDPOINT': '/email/hard_bounces', + 'NEW_ALIAS_ENDPOINT': '/users/alias/new', + 'USERS_TRACK_ENDPOINT': '/users/track', + 'EXPORT_ID_ENDPOINT': '/users/export/ids', + 'CAMPAIGN_SEND_ENDPOINT': '/campaigns/trigger/send', + 'ENTERPRISE_CAMPAIGN_ID': '', + 'FROM_EMAIL': '', + } + } +} + + @ddt.ddt class BrazeClientTests(TestCase): """ Tests for Braze Client. """ - SITE_CODE = 'test' - SITE_OVERRIDES_MODULE = 'ecommerce_worker.configuration.test.SITE_OVERRIDES' - BRAZE_OVERRIDES = { - SITE_CODE: { - 'BRAZE': { - 'BRAZE_ENABLE': True, - 'BRAZE_REST_API_KEY': 'rest_api_key', - 'BRAZE_WEBAPP_API_KEY': 'webapp_api_key', - 'REST_API_URL': 'https://rest.iad-06.braze.com', - 'MESSAGES_SEND_ENDPOINT': '/messages/send', - 'EMAIL_BOUNCE_ENDPOINT': '/email/hard_bounces', - 'NEW_ALIAS_ENDPOINT': '/users/alias/new', - 'USERS_TRACK_ENDPOINT': '/users/track', - 'EXPORT_ID_ENDPOINT': '/users/export/ids', - 'CAMPAIGN_SEND_ENDPOINT': '/campaigns/trigger/send', - 'ENTERPRISE_CAMPAIGN_ID': '', - 'FROM_EMAIL': '', - } - } - } - def mock_braze_user_endpoints(self, users=[]): # pylint: disable=dangerous-default-value """ Mock POST requests to the user alias, track and export endpoints. """ host = 'https://rest.iad-06.braze.com/users/track' @@ -75,17 +75,17 @@ def assert_get_braze_client_raises(self, exc_class, config): """ with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=config)): with self.assertRaises(exc_class): - get_braze_client(self.SITE_CODE) + get_braze_client(SITE_CODE) def test_get_braze_client_with_braze_disabled(self): """ Verify the method raises a BrazeNotEnabled if Braze is not enabled for the site. """ - with mock.patch('ecommerce_worker.email.v1.braze.client.log.debug') as mock_log: + with mock.patch('ecommerce_worker.email.v1.braze.client.log.error') as mock_log_error: self.assert_get_braze_client_raises(BrazeNotEnabled, {'BRAZE_ENABLE': False}) - mock_log.assert_called_once_with(f'Braze is not enabled for site {self.SITE_CODE}') + mock_log_error.assert_called_once_with(f'Braze is not enabled for site {SITE_CODE}') @ddt.data( {}, @@ -101,15 +101,15 @@ def test_get_braze_client_without_credentials(self, braze_config): with mock.patch('ecommerce_worker.email.v1.braze.client.log.error') as mock_log: self.assert_get_braze_client_raises(ConfigurationError, braze_config) - mock_log.assert_called_once_with(f'Required keys missing for site {self.SITE_CODE}') + mock_log.assert_called_once_with(f'Required keys missing for site {SITE_CODE}') def test_create_braze_alias(self): """ Asserts an error is raised by a call to create_braze_alias. """ - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(BrazeClientError): client.create_braze_alias(recipient_emails=[]) @@ -132,9 +132,9 @@ def test_send_braze_message_success(self): json=success_response, status=201 ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) response = client.send_message( email_ids=['test1@example.com', 'test2@example.com'], subject='Test Subject', @@ -162,9 +162,9 @@ def test_send_braze_message_success_with_external_ids(self): json=success_response, status=201 ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) response = client.send_message( ['test1@example.com', 'test2@example.com'], 'Test Subject', @@ -196,9 +196,9 @@ def test_send_braze_message_failure(self, status_code, error): json=failure_response, status=status_code ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(error): response = client.send_message( # pylint: disable=unused-variable ['test1@example.com', 'test2@example.com'], @@ -216,9 +216,9 @@ def test_send_braze_message_failure_missing_parameters(self, email, subject, bod """ Verify that an error is raised for missing email parameters. """ - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(BrazeClientError): response = client.send_message(email, subject, body) # pylint: disable=unused-variable @@ -241,9 +241,9 @@ def test_send_braze_campaign_message_success(self): json=success_response, status=201 ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) response = client.send_campaign_message( ['test1@example.com', 'test2@example.com'], 'Test Subject', @@ -275,9 +275,9 @@ def test_send_braze_campaign_message_failure(self, status_code, error): json=failure_response, status=status_code ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(error): response = client.send_campaign_message( # pylint: disable=unused-variable ['test1@example.com', 'test2@example.com'], @@ -295,9 +295,9 @@ def test_send_braze_campaign_message_failure_missing_parameters(self, email, sub """ Verify that an error is raised for missing email parameters. """ - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(BrazeClientError): response = client.send_campaign_message(email, subject, body) # pylint: disable=unused-variable @@ -339,9 +339,9 @@ def test_bounced_email(self, response, did_bounce): json=response, status=200 ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) bounce = client.did_email_bounce(bounced_email) self.assertEqual(bounce, did_bounce) @@ -370,9 +370,9 @@ def test_bounce_message_failure(self, status_code, error): json=failure_response, status=status_code ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(error): client.did_email_bounce(bounced_email) @@ -380,9 +380,9 @@ def test_did_email_bounce_failure_missing_parameters(self): """ Verify that an error is raised for missing parameters. """ - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(BrazeClientError): client.did_email_bounce(email_id=None) @@ -390,9 +390,9 @@ def test_get_braze_external_id(self): """ Asserts an error is raised by a call to get_braze_external_id. """ - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) with self.assertRaises(BrazeClientError): client.get_braze_external_id(email_id=None) @@ -408,8 +408,77 @@ def test_get_braze_external_id_success(self): json={"users": [{"external_id": "5261613"}], "message": "success"}, status=201 ) - braze = self.BRAZE_OVERRIDES[self.SITE_CODE]['BRAZE'] + braze = BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] with patch('ecommerce_worker.email.v1.braze.client.get_braze_configuration', Mock(return_value=braze)): - client = get_braze_client(self.SITE_CODE) + client = get_braze_client(SITE_CODE) external_id = client.get_braze_external_id(email_id='test@example.com') self.assertEqual(external_id, '5261613') + + +@ddt.ddt +class EdxBrazeClientTests(TestCase): + """ + Tests for the EdxBrazeClient wrapper class. + """ + @ddt.data( + ( + {'BRAZE_ENABLE': False}, + BrazeNotEnabled, + ), + ( + { + 'BRAZE_REST_API_KEY': 'some-key', + 'BRAZE_ENABLE': True, + }, + ConfigurationError, + ), + ) + @ddt.unpack + def test_braze_config_invalid(self, mock_config, expected_exception): + """ + Asserts an error is raised when initializing an EdxBrazeClient for a config + that has Braze disabled or is missing a required key. + """ + with patch( + 'ecommerce_worker.email.v1.braze.client.get_braze_configuration', + Mock(return_value=mock_config), + ) as mock_braze_config: + with self.assertRaises(expected_exception): + EdxBrazeClient(SITE_CODE) + + mock_braze_config.assert_called_once_with(SITE_CODE) + + @patch( + 'ecommerce_worker.email.v1.braze.client.get_braze_configuration', + return_value=BRAZE_OVERRIDES[SITE_CODE]['BRAZE'] + ) + def test_create_recipient(self, mock_get_braze_config): # pylint: disable=unused-argument + """ + Tests that create_recipient() calls the underlying identify_users() method + and returns a dictionary with expected keys and values. + """ + user_email = 'curly@stooges.org' + lms_user_id = 777 + trigger_properties = {'foo': 12} + with patch( + 'ecommerce_worker.email.v1.braze.client.EdxBrazeClient.identify_users' + ) as mock_identify_users: + client = EdxBrazeClient(SITE_CODE) + actual_recipient = client.create_recipient(user_email, lms_user_id, trigger_properties) + + expected_recipient = { + 'attributes': { + '_update_existing_only': False, + 'email': 'curly@stooges.org', + 'user_alias': { + 'alias_label': 'Enterprise', + 'alias_name': 'curly@stooges.org' + } + }, + 'external_user_id': 777, + 'send_to_existing_only': False, + 'trigger_properties': { + 'foo': 12, + }, + } + self.assertEqual(actual_recipient, expected_recipient) diff --git a/ecommerce_worker/email/v1/braze/tests/test_tasks.py b/ecommerce_worker/email/v1/braze/tests/test_tasks.py index 470059af..cad0690e 100644 --- a/ecommerce_worker/email/v1/braze/tests/test_tasks.py +++ b/ecommerce_worker/email/v1/braze/tests/test_tasks.py @@ -2,8 +2,9 @@ import logging from unittest import TestCase -from unittest.mock import patch, Mock +from unittest.mock import patch, MagicMock, Mock +import braze.exceptions as edx_braze_exceptions import ddt import responses from celery.exceptions import Retry @@ -23,42 +24,51 @@ ) +LOG_NAME = 'ecommerce_worker.email.v1.braze.tasks' +LOG_TASK_NAME = 'ecommerce_worker.email.v1.braze.tasks' +USER_EMAIL = 'user@unknown.com' +EMAILS = 'user@unknown.com, user1@example.com' +LMS_USER_IDS_BY_EMAIL = { + 'user@unknown.com': None, + 'user1@example.com': 456, +} +OFFER_ASSIGNMENT_ID = '555' +SEND_ID = '66cdc28f8f082bc3074c0c79f' +SUBJECT = 'New edX course assignment' +EMAIL_BODY = 'Template message with johndoe@unknown.com GIL7RUEOU7VHBH7Q ' \ + 'https://tempurl.url/enroll 3 2012-04-23' +USAGE_EMAIL_BODY_VARIABLES = { + 'FOO': 123, + 'BAR': 555, +} +SENDER_ALIAS = 'edx Support Team' +REPLY_TO = 'reply@edx.org' +SITE_CODE = 'test' +BRAZE_CONFIG = { + 'BRAZE_ENABLE': True, + 'BRAZE_REST_API_KEY': 'rest_api_key', + 'BRAZE_WEBAPP_API_KEY': 'webapp_api_key', + 'REST_API_URL': 'https://rest.iad-06.braze.com', + 'MESSAGES_SEND_ENDPOINT': '/messages/send', + 'EMAIL_BOUNCE_ENDPOINT': '/email/hard_bounces', + 'NEW_ALIAS_ENDPOINT': '/users/alias/new', + 'USERS_TRACK_ENDPOINT': '/users/track', + 'EXPORT_ID_ENDPOINT': '/users/export/ids', + 'CAMPAIGN_SEND_ENDPOINT': '/campaigns/trigger/send', + 'ENTERPRISE_CAMPAIGN_ID': '', + 'ENTERPRISE_CODE_ASSIGNMENT_CAMPAIGN_ID': 'the-code-assignment-campaign-id', + 'ENTERPRISE_CODE_UPDATE_CAMPAIGN_ID': 'the-code-update-campaign-id', + 'ENTERPRISE_CODE_USAGE_CAMPAIGN_ID': 'the-code-usage-campaign-id', + 'ENTERPRISE_CODE_NUDGE_CAMPAIGN_ID': '', # cover case where a `campaign_id` kwarg is falsey. + 'FROM_EMAIL': '', + 'BRAZE_RETRY_SECONDS': 3600, + 'BRAZE_RETRY_ATTEMPTS': 6, +} + + @ddt.ddt class SendEmailsViaBrazeTests(TestCase): """ Validates the email sending tasks with Braze api. """ - LOG_NAME = 'ecommerce_worker.email.v1.braze.tasks' - LOG_TASK_NAME = 'ecommerce_worker.email.v1.braze.tasks' - USER_EMAIL = 'user@unknown.com' - EMAILS = 'user@unknown.com, user1@example.com' - OFFER_ASSIGNMENT_ID = '555' - SEND_ID = '66cdc28f8f082bc3074c0c79f' - SUBJECT = 'New edX course assignment' - EMAIL_BODY = 'Template message with johndoe@unknown.com GIL7RUEOU7VHBH7Q ' \ - 'https://tempurl.url/enroll 3 2012-04-23' - SENDER_ALIAS = 'edx Support Team' - REPLY_TO = 'reply@edx.org' - SITE_CODE = 'test' - BRAZE_CONFIG = { - 'BRAZE_ENABLE': True, - 'BRAZE_REST_API_KEY': 'rest_api_key', - 'BRAZE_WEBAPP_API_KEY': 'webapp_api_key', - 'REST_API_URL': 'https://rest.iad-06.braze.com', - 'MESSAGES_SEND_ENDPOINT': '/messages/send', - 'EMAIL_BOUNCE_ENDPOINT': '/email/hard_bounces', - 'NEW_ALIAS_ENDPOINT': '/users/alias/new', - 'USERS_TRACK_ENDPOINT': '/users/track', - 'EXPORT_ID_ENDPOINT': '/users/export/ids', - 'CAMPAIGN_SEND_ENDPOINT': '/campaigns/trigger/send', - 'ENTERPRISE_CAMPAIGN_ID': '', - 'ENTERPRISE_CODE_ASSIGNMENT_CAMPAIGN_ID': 'the-code-assignment-campaign-id', - 'ENTERPRISE_CODE_UPDATE_CAMPAIGN_ID': 'the-code-update-campaign-id', - 'ENTERPRISE_CODE_USAGE_CAMPAIGN_ID': 'the-code-usage-campaign-id', - 'ENTERPRISE_CODE_NUDGE_CAMPAIGN_ID': '', # cover case where a `campaign_id` kwarg is falsey. - 'FROM_EMAIL': '', - 'BRAZE_RETRY_SECONDS': 3600, - 'BRAZE_RETRY_ATTEMPTS': 6, - } - ASSIGNMENT_TASK_KWARGS = { 'user_email': USER_EMAIL, 'offer_assignment_id': OFFER_ASSIGNMENT_ID, @@ -78,14 +88,6 @@ class SendEmailsViaBrazeTests(TestCase): 'site_code': SITE_CODE, } - USAGE_TASK_KWARGS = { - 'emails': EMAILS, - 'subject': SUBJECT, - 'email_body': EMAIL_BODY, - 'reply_to': REPLY_TO, - 'site_code': SITE_CODE, - } - NUDGE_TASK_KWARGS = { 'email': USER_EMAIL, 'subject': SUBJECT, @@ -97,29 +99,26 @@ class SendEmailsViaBrazeTests(TestCase): def execute_task(self): """ Execute the send_offer_assignment_email task. """ - with patch('ecommerce_worker.configuration.test.BRAZE', self.BRAZE_CONFIG): + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): send_offer_assignment_email(**self.ASSIGNMENT_TASK_KWARGS) def mock_braze_user_endpoints(self): """ Mock POST requests to the user alias and track endpoints. """ - host = 'https://rest.iad-06.braze.com/users/track' responses.add( responses.POST, - host, + 'https://rest.iad-06.braze.com/users/track', json={'message': 'success'}, status=201 ) - host = 'https://rest.iad-06.braze.com/users/alias/new' responses.add( responses.POST, - host, + 'https://rest.iad-06.braze.com/users/alias/new', json={'message': 'success'}, status=201 ) - host = 'https://rest.iad-06.braze.com/users/export/ids' responses.add( responses.POST, - host, + 'https://rest.iad-06.braze.com/users/export/ids', json={"users": [], "message": "success"}, status=201 ) @@ -128,24 +127,23 @@ def mock_braze_user_endpoints(self): @ddt.data( (send_offer_assignment_email, ASSIGNMENT_TASK_KWARGS, "Offer Assignment", 'assignment'), (send_offer_update_email, UPDATE_TASK_KWARGS, "Offer Assignment", 'update'), - (send_offer_usage_email, USAGE_TASK_KWARGS, "Offer Usage", 'usage'), (send_code_assignment_nudge_email, NUDGE_TASK_KWARGS, "Code Assignment Nudge Email", 'nudge'), ) @ddt.unpack def test_client_instantiation_error(self, task, task_kwargs, logger_prefix, log_message): """ Verify no message is sent if an error occurs while instantiating the Braze API client. """ - with patch('ecommerce_worker.configuration.test.BRAZE', self.BRAZE_CONFIG): + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): with LogCapture(level=logging.INFO) as log: task(**task_kwargs) log.check( ( - self.LOG_NAME, + LOG_NAME, 'ERROR', '[{logger_prefix}] Error in offer {log_message} notification with message --- ' '{message}'.format( logger_prefix=logger_prefix, log_message=log_message, - message=self.EMAIL_BODY + message=EMAIL_BODY ) ), ) @@ -154,20 +152,19 @@ def test_client_instantiation_error(self, task, task_kwargs, logger_prefix, log_ @ddt.data( (send_offer_assignment_email, ASSIGNMENT_TASK_KWARGS, "Offer Assignment", 'assignment'), (send_offer_update_email, UPDATE_TASK_KWARGS, "Offer Assignment", 'update'), - (send_offer_usage_email, USAGE_TASK_KWARGS, "Offer Usage", 'usage'), (send_code_assignment_nudge_email, NUDGE_TASK_KWARGS, "Code Assignment Nudge Email", 'nudge'), ) @ddt.unpack def test_api_client_error(self, task, task_kwargs, logger_prefix, log_message, mock_log): """ Verify API client errors are logged. """ - with patch('ecommerce_worker.configuration.test.BRAZE', self.BRAZE_CONFIG): + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): task(**task_kwargs) mock_log.assert_called_once_with( '[{logger_prefix}] Error in offer {log_message} notification with message --- ' '{message}'.format( logger_prefix=logger_prefix, log_message=log_message, - message=self.EMAIL_BODY + message=EMAIL_BODY ) ) @@ -175,7 +172,6 @@ def test_api_client_error(self, task, task_kwargs, logger_prefix, log_message, m @ddt.data( (send_offer_assignment_email, ASSIGNMENT_TASK_KWARGS), (send_offer_update_email, UPDATE_TASK_KWARGS), - (send_offer_usage_email, USAGE_TASK_KWARGS), (send_code_assignment_nudge_email, NUDGE_TASK_KWARGS), ) @ddt.unpack @@ -193,7 +189,7 @@ def test_api_429_error_with_retry(self, task, task_kwargs): json=failure_response, status=429 ) - with patch('ecommerce_worker.configuration.test.BRAZE', self.BRAZE_CONFIG): + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): with self.assertRaises((BrazeRateLimitError, Retry)): task(**task_kwargs) @@ -201,7 +197,6 @@ def test_api_429_error_with_retry(self, task, task_kwargs): @ddt.data( (send_offer_assignment_email, ASSIGNMENT_TASK_KWARGS), (send_offer_update_email, UPDATE_TASK_KWARGS), - (send_offer_usage_email, USAGE_TASK_KWARGS), (send_code_assignment_nudge_email, NUDGE_TASK_KWARGS), ) @ddt.unpack @@ -219,14 +214,13 @@ def test_api_500_error_with_retry(self, task, task_kwargs): json=failure_response, status=500 ) - with patch('ecommerce_worker.configuration.test.BRAZE', self.BRAZE_CONFIG): + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): with self.assertRaises((BrazeInternalServerError, Retry)): task(**task_kwargs) @responses.activate @ddt.data( (send_offer_update_email, UPDATE_TASK_KWARGS), - (send_offer_usage_email, USAGE_TASK_KWARGS), (send_code_assignment_nudge_email, NUDGE_TASK_KWARGS), ) @ddt.unpack @@ -247,7 +241,7 @@ def test_api(self, task, task_kwargs): host, json=success_response, status=201) - with patch('ecommerce_worker.configuration.test.BRAZE', self.BRAZE_CONFIG): + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): task(**task_kwargs) self.assertIn('success', responses.calls[0].response.text) @@ -272,10 +266,10 @@ def test_message_sent(self, mock_update_assignment): self.execute_task() log.check_present( ( - self.LOG_TASK_NAME, + LOG_TASK_NAME, 'INFO', '[Offer Assignment] Offer assignment notification sent with message --- ' - '{message}'.format(message=self.EMAIL_BODY) + '{message}'.format(message=EMAIL_BODY) ), ) self.assertEqual(mock_update_assignment.call_count, 1) @@ -302,12 +296,111 @@ def test_update_assignment_exception(self, mock_get_ecommerce_client): self.execute_task() log.check_present( ( - self.LOG_TASK_NAME, + LOG_TASK_NAME, 'ERROR', '[Offer Assignment] An error occurred while updating email status data for offer {token_offer} and ' 'email {token_email} via the ecommerce API.'.format( - token_offer=self.OFFER_ASSIGNMENT_ID, - token_email=self.USER_EMAIL + token_offer=OFFER_ASSIGNMENT_ID, + token_email=USER_EMAIL ) ) ) + + def test_braze_not_enabled(self): + """ + Test that tasks do nothing when Braze is not enabled. + """ + # pylint: disable=unnecessary-comprehension + disabled_config = {key: value for key, value in BRAZE_CONFIG.items()} + disabled_config['BRAZE_ENABLE'] = False + for task, braze_task_name, kwargs in ( + (send_code_assignment_nudge_email, 'send_code_assignment_nudge_email_via_braze', self.NUDGE_TASK_KWARGS), + (send_offer_assignment_email, 'send_offer_assignment_email_via_braze', self.ASSIGNMENT_TASK_KWARGS), + (send_offer_update_email, 'send_offer_update_email_via_braze', self.UPDATE_TASK_KWARGS), + ): + task_path = 'ecommerce_worker.email.v1.tasks.{}'.format(braze_task_name) + with patch(task_path) as mock_braze_task: + task(**kwargs) + self.assertFalse(mock_braze_task.called) + + +@ddt.ddt +class OfferUsageEmailTests(TestCase): + """ + Separate test class for the offer usage email task, since it utilizes + edx-braze-client. + """ + USAGE_TASK_KWARGS = { + 'lms_user_ids_by_email': LMS_USER_IDS_BY_EMAIL, + 'subject': SUBJECT, + 'email_body_variables': USAGE_EMAIL_BODY_VARIABLES, + 'site_code': SITE_CODE, + } + + def test_braze_not_enabled(self): + """ + Test that this task does nothing when Braze is not enabled. + """ + # pylint: disable=unnecessary-comprehension + disabled_config = {key: value for key, value in BRAZE_CONFIG.items()} + disabled_config['BRAZE_ENABLE'] = False + task_path = 'ecommerce_worker.email.v1.tasks.send_offer_usage_email_via_braze' + with patch(task_path) as mock_braze_task: + send_offer_usage_email(**self.USAGE_TASK_KWARGS) + self.assertFalse(mock_braze_task.called) + + @patch('ecommerce_worker.email.v1.braze.tasks.EdxBrazeClient', return_value=MagicMock()) + @ddt.data( + edx_braze_exceptions.BrazeRateLimitError, + edx_braze_exceptions.BrazeInternalServerError, + ) + def test_api_error_with_retry(self, exception, mock_client): + """ Verify the task is rescheduled if an expected API error occurs, and the request can be retried. """ + mock_client.return_value.send_campaign_message.side_effect = exception('msg') + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): + with self.assertRaises((exception, Retry)): + send_offer_usage_email(**self.USAGE_TASK_KWARGS) + + @patch('ecommerce_worker.email.v1.braze.tasks.EdxBrazeClient', return_value=MagicMock()) + def test_api_generic_braze_error(self, mock_client): + """ Verify the task raises a BrazeError for other types of exceptions. """ + mock_client.return_value.send_campaign_message.side_effect = edx_braze_exceptions.BrazeError('msg') + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): + with self.assertRaises(edx_braze_exceptions.BrazeError): + send_offer_usage_email(**self.USAGE_TASK_KWARGS) + + @patch('ecommerce_worker.email.v1.braze.tasks.EdxBrazeClient', return_value=MagicMock(), autospec=True) + @ddt.data( + None, + 'custom-campaign-id', + ) + def test_successful_usage_email(self, campaign_id, mock_client): + """ Verify the task successfully calls send_campaign_message() and create_recipient(). """ + mock_client.reset_mock() + + def mock_create_recipient(user_email, lms_user_id): + return {'user_email': user_email, 'lms_user_id': lms_user_id} + + client = mock_client.return_value + client.create_recipient.side_effect = mock_create_recipient + + with patch('ecommerce_worker.configuration.test.BRAZE', BRAZE_CONFIG): + send_offer_usage_email(campaign_id=campaign_id, **self.USAGE_TASK_KWARGS) + + client.create_recipient.assert_called_once_with( + 'user1@example.com', 456, + ) + + expected_trigger_properties = { + 'subject': 'New edX course assignment', + **USAGE_EMAIL_BODY_VARIABLES, + } + expected_campaign_id = campaign_id or BRAZE_CONFIG['ENTERPRISE_CODE_USAGE_CAMPAIGN_ID'] + client.send_campaign_message.assert_called_once_with( + campaign_id=expected_campaign_id, + recipients=[ + {'user_email': 'user1@example.com', 'lms_user_id': 456} + ], + emails=['user@unknown.com'], + trigger_properties=expected_trigger_properties, + ) diff --git a/ecommerce_worker/email/v1/tasks.py b/ecommerce_worker/email/v1/tasks.py index 25523cfe..4a0a0072 100644 --- a/ecommerce_worker/email/v1/tasks.py +++ b/ecommerce_worker/email/v1/tasks.py @@ -1,6 +1,7 @@ """ This file contains celery tasks for email marketing signal handler. """ +import logging from celery import shared_task @@ -17,6 +18,8 @@ # regardless, they are intentionally there as part of the public API, to be used or not by implementations. # pylint: disable=unused-argument +logger = logging.getLogger(__name__) + @shared_task(bind=True, ignore_result=True) def send_offer_assignment_email(self, user_email, offer_assignment_id, subject, email_body, sender_alias, # pylint: disable=dangerous-default-value @@ -36,9 +39,14 @@ def send_offer_assignment_email(self, user_email, offer_assignment_id, subject, site_code (str): Identifier of the site sending the email. base_enterprise_url (str): Url for the enterprise learner portal. """ - if is_braze_enabled(site_code): - send_offer_assignment_email_via_braze( - self, user_email, offer_assignment_id, subject, email_body, sender_alias, reply_to, attachments, site_code) + if not is_braze_enabled(site_code): + logger.error('Braze not enabled for site code {}'.format(site_code)) + return + + send_offer_assignment_email_via_braze( + self, user_email, offer_assignment_id, subject, email_body, + sender_alias, reply_to, attachments, site_code + ) @shared_task(bind=True, ignore_result=True) @@ -58,29 +66,38 @@ def send_offer_update_email(self, user_email, subject, email_body, sender_alias, attachments (list): File attachment list with dicts having 'file_name' and 'url' keys. base_enterprise_url (str): Enterprise learner portal url. """ - if is_braze_enabled(site_code): - send_offer_update_email_via_braze(self, user_email, subject, email_body, sender_alias, reply_to, attachments, - site_code) + if not is_braze_enabled(site_code): + logger.error('Braze not enabled for site code {}'.format(site_code)) + return + + send_offer_update_email_via_braze( + self, user_email, subject, email_body, sender_alias, reply_to, attachments, site_code + ) @shared_task(bind=True, ignore_result=True) -def send_offer_usage_email(self, emails, subject, email_body, reply_to='', attachments=[], site_code=None, # pylint: disable=dangerous-default-value - base_enterprise_url='') -> None: +def send_offer_usage_email( + self, lms_user_ids_by_email, subject, email_body_variables, site_code=None, campaign_id=None +) -> None: """ Sends the offer usage email. Args: self: Ignore. - emails (str): comma separated emails. + lms_user_ids_by_email (dict): Map of user email to LMS user ids. subject (str): Email subject. - email_body (str): The body of the email. - reply_to (str): Enterprise Customer reply to address for email reply. - attachments (list): File attachment list with dicts having 'file_name' and 'url' keys. + email_body_variables (dict): key-value pairs that are injected into Braze email template for personalization. site_code (str): Identifier of the site sending the email. - base_enterprise_url (str): Url of the enterprise's learner portal + campaign_id (str): Identifier of Braze API-triggered campaign to send message through; defaults + to config.ENTERPRISE_CODE_USAGE_CAMPAIGN_ID """ - if is_braze_enabled(site_code): - send_offer_usage_email_via_braze(self, emails, subject, email_body, reply_to, attachments, site_code) + if not is_braze_enabled(site_code): + logger.error('Braze not enabled for site code {}'.format(site_code)) + return + + send_offer_usage_email_via_braze( + self, lms_user_ids_by_email, subject, email_body_variables, site_code, campaign_id, + ) @shared_task(bind=True, ignore_result=True) @@ -100,6 +117,10 @@ def send_code_assignment_nudge_email(self, email, subject, email_body, sender_al site_code (str): Identifier of the site sending the email. base_enterprise_url (str): Enterprise learner portal url. """ - if is_braze_enabled(site_code): - send_code_assignment_nudge_email_via_braze(self, email, subject, email_body, sender_alias, reply_to, - attachments, site_code) + if not is_braze_enabled(site_code): + logger.error('Braze not enabled for site code {}'.format(site_code)) + return + + send_code_assignment_nudge_email_via_braze( + self, email, subject, email_body, sender_alias, reply_to, attachments, site_code + ) diff --git a/requirements/base.in b/requirements/base.in index 63f19d8e..94e8148b 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -1,6 +1,7 @@ -c constraints.txt celery +edx-braze-client edx-rest-api-client redis six diff --git a/requirements/base.txt b/requirements/base.txt index 3aecce4c..fff644f4 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -36,6 +36,8 @@ django-crum==0.7.9 # via edx-django-utils django-waffle==2.5.0 # via edx-django-utils +edx-braze-client==0.1.4 + # via -r requirements/base.in edx-django-utils==5.0.0 # via edx-rest-api-client edx-rest-api-client==5.5.0 diff --git a/requirements/production.txt b/requirements/production.txt index 53d50af3..04e0aa0e 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -59,6 +59,8 @@ django-waffle==2.5.0 # via # -r requirements/base.txt # edx-django-utils +edx-braze-client==0.1.4 + # via -r requirements/base.txt edx-django-utils==5.0.0 # via # -r requirements/base.txt diff --git a/requirements/test.txt b/requirements/test.txt index ec06bfdb..dbc0a6c9 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -80,6 +80,8 @@ django-waffle==2.5.0 # via # -r requirements/base.txt # edx-django-utils +edx-braze-client==0.1.4 + # via -r requirements/base.txt edx-django-utils==5.0.0 # via # -r requirements/base.txt diff --git a/setup.py b/setup.py index c6d7398b..290141be 100644 --- a/setup.py +++ b/setup.py @@ -36,7 +36,7 @@ def is_requirement(line): setup( name='edx-ecommerce-worker', - version='3.1.2', + version='3.2.0', description='Celery tasks supporting the operations of edX\'s ecommerce service', long_description=long_description, classifiers=[