diff --git a/license_manager/apps/api/tasks.py b/license_manager/apps/api/tasks.py index 97755b6c..d29b0d22 100644 --- a/license_manager/apps/api/tasks.py +++ b/license_manager/apps/api/tasks.py @@ -50,6 +50,8 @@ SOFT_TIME_LIMIT = 900 MAX_TIME_LIMIT = 960 +LICENSE_DEBUG_PREFIX = '[LICENSE DEBUGGING]' + class LoggedTaskWithRetry(LoggedTask): # pylint: disable=abstract-method """ @@ -117,10 +119,12 @@ def send_assignment_email_task(custom_template_text, email_recipient_list, subsc enterprise_sender_alias = get_enterprise_sender_alias(enterprise_customer) enterprise_contact_email = enterprise_customer.get('contact_email') + pending_license_by_email = {} # We need to send these emails individually, because each email's text must be # generated for every single user/activation_key for pending_license in pending_licenses: user_email = pending_license.user_email + pending_license_by_email[user_email] = pending_license license_activation_key = str(pending_license.activation_key) braze_campaign_id = settings.BRAZE_ASSIGNMENT_EMAIL_CAMPAIGN braze_trigger_properties = { @@ -141,6 +145,10 @@ def send_assignment_email_task(custom_template_text, email_recipient_list, subsc recipients=[recipient], trigger_properties=braze_trigger_properties, ) + logger.info( + f'{LICENSE_DEBUG_PREFIX} Sent license assignment email ' + f'braze campaign {braze_campaign_id} to {recipient}' + ) except BrazeClientError as exc: message = ( 'License manager activation email sending received an ' @@ -149,6 +157,13 @@ def send_assignment_email_task(custom_template_text, email_recipient_list, subsc logger.exception(message) raise exc + emails_with_no_assignments = [ + _email for _email in email_recipient_list + if _email not in pending_license_by_email + ] + if emails_with_no_assignments: + logger.warning(f'{LICENSE_DEBUG_PREFIX} No assignment email sent for {emails_with_no_assignments}') + @shared_task(base=LoggedTaskWithRetry, soft_time_limit=SOFT_TIME_LIMIT, time_limit=MAX_TIME_LIMIT) def link_learners_to_enterprise_task(learner_emails, enterprise_customer_uuid): @@ -189,10 +204,12 @@ def send_reminder_email_task(custom_template_text, email_recipient_list, subscri enterprise_sender_alias = get_enterprise_sender_alias(enterprise_customer) enterprise_contact_email = enterprise_customer.get('contact_email') + pending_license_by_email = {} # We need to send these emails individually, because each email's text must be # generated for every single user/activation_key for pending_license in pending_licenses: user_email = pending_license.user_email + pending_license_by_email[user_email] = pending_license license_activation_key = str(pending_license.activation_key) braze_campaign_id = settings.BRAZE_REMIND_EMAIL_CAMPAIGN braze_trigger_properties = { @@ -217,7 +234,10 @@ def send_reminder_email_task(custom_template_text, email_recipient_list, subscri recipients=[recipient], trigger_properties=braze_trigger_properties, ) - + logger.info( + f'{LICENSE_DEBUG_PREFIX} Sent license reminder email ' + f'braze campaign {braze_campaign_id} to {recipient}' + ) except BrazeClientError as exc: message = ( 'Error hitting Braze API. ' @@ -228,6 +248,13 @@ def send_reminder_email_task(custom_template_text, email_recipient_list, subscri License.set_date_fields_to_now(pending_licenses, ['last_remind_date']) + emails_with_no_assignments = [ + _email for _email in email_recipient_list + if _email not in pending_license_by_email + ] + if emails_with_no_assignments: + logger.warning(f'{LICENSE_DEBUG_PREFIX} No reminder email sent for {emails_with_no_assignments}') + @shared_task(base=LoggedTaskWithRetry) def send_post_activation_email_task(enterprise_customer_uuid, user_email): diff --git a/license_manager/apps/api/tests/test_tasks.py b/license_manager/apps/api/tests/test_tasks.py index c62de71b..bd61014f 100644 --- a/license_manager/apps/api/tests/test_tasks.py +++ b/license_manager/apps/api/tests/test_tasks.py @@ -105,12 +105,20 @@ def test_activation_task(self, mock_braze_client, mock_enterprise_client): 'contact_email': self.contact_email, } + # Add an extra recipient with no associated license + # to hit debug-logging test coverage tasks.send_assignment_email_task( self.custom_template_text, - self.email_recipient_list, + self.email_recipient_list + ['no-license@foo.com'], self.subscription_plan.uuid, ) + called_emails = [ + _call.kwargs['recipients'][0]['attributes']['email'] + for _call in mock_braze_client().send_campaign_message.call_args_list + ] + self.assertNotIn('no-license@foo.com', called_emails) + for user_email in self.email_recipient_list: expected_license_key = str(self.subscription_plan.licenses.get( user_email=user_email @@ -179,12 +187,20 @@ def test_send_reminder_email_task(self, mock_enterprise_client, mock_braze_clien 'contact_email': self.contact_email, } with freeze_time(localized_utcnow()): + # Add an extra recipient with no associated license + # to hit debug-logging test coverage tasks.send_reminder_email_task( self.custom_template_text, - self.email_recipient_list, + self.email_recipient_list + ['no-license@foo.com'], self.subscription_plan.uuid ) + called_emails = [ + _call.kwargs['recipients'][0]['attributes']['email'] + for _call in mock_braze_client().send_campaign_message.call_args_list + ] + self.assertNotIn('no-license@foo.com', called_emails) + for user_email in self.email_recipient_list: expected_license_key = str(self.subscription_plan.licenses.get( user_email=user_email diff --git a/license_manager/apps/api/v1/views.py b/license_manager/apps/api/v1/views.py index 154369a3..8c90f93d 100644 --- a/license_manager/apps/api/v1/views.py +++ b/license_manager/apps/api/v1/views.py @@ -721,11 +721,6 @@ def _assign_new_licenses(self, subscription_plan, user_emails): batch_size=10, ) - license_uuid_strs = [str(_license.uuid) for _license in licenses] - track_license_changes_task.delay( - license_uuid_strs, - constants.SegmentEvents.LICENSE_ASSIGNED, - ) return licenses def _set_source_for_assigned_licenses(self, assigned_licenses, emails_and_sfids): @@ -844,13 +839,20 @@ def _assign(self, request, subscription_plan): logger.exception(error_message) return Response(error_message, status=status.HTTP_422_UNPROCESSABLE_ENTITY) else: - notify_users = request.data.get('notify_users', True) - custom_template_text = utils.get_custom_text(request.data) + # Track license changes and do any other tasks that may want to + # read from the License DB table outside of the transaction.atomic() block, + # so that they can read the committed and updated versions + # of the now-assigned license records. + track_license_changes_task.delay( + [str(_license.uuid) for _license in assigned_licenses], + constants.SegmentEvents.LICENSE_ASSIGNED, + ) + self._link_and_notify_assigned_emails( - user_emails, - subscription_plan, - notify_users, - custom_template_text + user_emails=user_emails, + subscription_plan=subscription_plan, + notify_users=request.data.get('notify_users', True), + custom_template_text=utils.get_custom_text(request.data), ) else: logger.info('All given emails are already associated with a license.') diff --git a/requirements/base.txt b/requirements/base.txt index c2d4242e..cce7f985 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -148,7 +148,7 @@ edx-api-doc-tools==1.7.0 # via -r requirements/base.in edx-auth-backends==4.2.0 # via -r requirements/base.in -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via -r requirements/base.in edx-celeryutils==1.2.3 # via -r requirements/base.in diff --git a/requirements/dev.txt b/requirements/dev.txt index 799f1042..08b1abf1 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -226,7 +226,7 @@ edx-api-doc-tools==1.7.0 # via -r requirements/validation.txt edx-auth-backends==4.2.0 # via -r requirements/validation.txt -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via -r requirements/validation.txt edx-celeryutils==1.2.3 # via -r requirements/validation.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 8a989ab6..f9c4cef1 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -225,7 +225,7 @@ edx-api-doc-tools==1.7.0 # via -r requirements/test.txt edx-auth-backends==4.2.0 # via -r requirements/test.txt -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via -r requirements/test.txt edx-celeryutils==1.2.3 # via -r requirements/test.txt diff --git a/requirements/production.txt b/requirements/production.txt index 6febee7e..ccb763a2 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -181,7 +181,7 @@ edx-api-doc-tools==1.7.0 # via -r requirements/base.txt edx-auth-backends==4.2.0 # via -r requirements/base.txt -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via -r requirements/base.txt edx-celeryutils==1.2.3 # via -r requirements/base.txt diff --git a/requirements/quality.txt b/requirements/quality.txt index 60c38276..69b41aa2 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -193,7 +193,7 @@ edx-api-doc-tools==1.7.0 # via -r requirements/base.txt edx-auth-backends==4.2.0 # via -r requirements/base.txt -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via -r requirements/base.txt edx-celeryutils==1.2.3 # via -r requirements/base.txt diff --git a/requirements/test.txt b/requirements/test.txt index b596031c..7c9feafd 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -202,7 +202,7 @@ edx-api-doc-tools==1.7.0 # via -r requirements/base.txt edx-auth-backends==4.2.0 # via -r requirements/base.txt -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via -r requirements/base.txt edx-celeryutils==1.2.3 # via -r requirements/base.txt diff --git a/requirements/validation.txt b/requirements/validation.txt index cd290fff..6ab38dda 100644 --- a/requirements/validation.txt +++ b/requirements/validation.txt @@ -264,7 +264,7 @@ edx-auth-backends==4.2.0 # via # -r requirements/quality.txt # -r requirements/test.txt -edx-braze-client==0.1.7 +edx-braze-client==0.1.8 # via # -r requirements/quality.txt # -r requirements/test.txt