Skip to content

Commit

Permalink
fix: DB tx isolation and informative license logging
Browse files Browse the repository at this point in the history
Track license changes in a celery task outside of the DB transaction
block that updates the license state, so that task can read the
committed updates to the license records.
Also add helpful logging to our license assignment/reminder email tasks.
ENT-7795
  • Loading branch information
iloveagent57 committed Oct 5, 2023
1 parent b6fe564 commit 3790e16
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 14 deletions.
29 changes: 28 additions & 1 deletion license_manager/apps/api/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
SOFT_TIME_LIMIT = 900
MAX_TIME_LIMIT = 960

LICENSE_DEBUG_PREFIX = '[LICENSE DEBUGGING]'


class LoggedTaskWithRetry(LoggedTask): # pylint: disable=abstract-method
"""
Expand Down Expand Up @@ -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 = {
Expand All @@ -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 '
Expand All @@ -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):
Expand Down Expand Up @@ -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 = {
Expand All @@ -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. '
Expand All @@ -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):
Expand Down
20 changes: 18 additions & 2 deletions license_manager/apps/api/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 + ['[email protected]'],
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('[email protected]', called_emails)

for user_email in self.email_recipient_list:
expected_license_key = str(self.subscription_plan.licenses.get(
user_email=user_email
Expand Down Expand Up @@ -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 + ['[email protected]'],
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('[email protected]', called_emails)

for user_email in self.email_recipient_list:
expected_license_key = str(self.subscription_plan.licenses.get(
user_email=user_email
Expand Down
24 changes: 13 additions & 11 deletions license_manager/apps/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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.')
Expand Down

0 comments on commit 3790e16

Please sign in to comment.