From 7346f999e83d5039358e39b223eea05be7cc8aa3 Mon Sep 17 00:00:00 2001 From: Hamzah Ullah Date: Wed, 8 May 2024 19:34:01 +0000 Subject: [PATCH] feat: updates tasks usage of create_recipient to create_recipients --- CHANGELOG.rst | 4 ++ enterprise/__init__.py | 2 +- enterprise/api_client/braze.py | 41 +---------- enterprise/tasks.py | 85 +++++++++++++++++------ enterprise/utils.py | 17 +++++ requirements/dev.txt | 2 +- requirements/doc.txt | 2 +- requirements/edx-platform-constraints.txt | 2 +- requirements/test-master.txt | 2 +- requirements/test.txt | 2 +- test_utils/fake_user_id_by_emails.py | 18 +++++ tests/test_enterprise/test_tasks.py | 50 +++++++++++-- tests/test_enterprise/test_utils.py | 37 ++++++++++ 13 files changed, 194 insertions(+), 70 deletions(-) create mode 100644 test_utils/fake_user_id_by_emails.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index acf6167721..d564d7e004 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * nothing unreleased +[4.18.0] +-------- +* feat: updates tasks usage of create_recipient to create_recipients + [4.17.5] -------- * fix: hard deleting expired group memberships diff --git a/enterprise/__init__.py b/enterprise/__init__.py index 9c598a0fc6..6569bd003d 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.17.5" +__version__ = "4.18.0" diff --git a/enterprise/api_client/braze.py b/enterprise/api_client/braze.py index 3549743a35..ca4f33c782 100644 --- a/enterprise/api_client/braze.py +++ b/enterprise/api_client/braze.py @@ -10,6 +10,8 @@ logger = logging.getLogger(__name__) ENTERPRISE_BRAZE_ALIAS_LABEL = 'Enterprise' # Do Not change this, this is consistent with other uses across edX repos. +# https://www.braze.com/docs/api/endpoints/user_data/post_user_identify/ +MAX_NUM_IDENTIFY_USERS_ALIASES = 50 class BrazeAPIClient(BrazeClient): @@ -41,45 +43,6 @@ def generate_mailto_link(self, emails): return None - 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. - Identifies the given email address with any existing Braze alias records - via the provided ``lms_user_id``. - """ - - user_alias = { - 'alias_label': ENTERPRISE_BRAZE_ALIAS_LABEL, - 'alias_name': user_email, - } - - # Identify the user alias in case it already exists. This is necessary so - # we don't accidentally 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, - "is_enterprise_learner": True, - "_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 {}, - } - def create_recipient_no_external_id(self, user_email): """ Create a Braze recipient dict identified only by an alias based on their email. diff --git a/enterprise/tasks.py b/enterprise/tasks.py index bfd7328631..c7db032464 100644 --- a/enterprise/tasks.py +++ b/enterprise/tasks.py @@ -13,10 +13,10 @@ from django.core import mail from django.db import IntegrityError -from enterprise.api_client.braze import ENTERPRISE_BRAZE_ALIAS_LABEL, BrazeAPIClient +from enterprise.api_client.braze import ENTERPRISE_BRAZE_ALIAS_LABEL, MAX_NUM_IDENTIFY_USERS_ALIASES, BrazeAPIClient from enterprise.api_client.enterprise_catalog import EnterpriseCatalogApiClient from enterprise.constants import SSO_BRAZE_CAMPAIGN_ID -from enterprise.utils import get_enterprise_customer, send_email_notification_message +from enterprise.utils import batch_dict, get_enterprise_customer, send_email_notification_message LOGGER = getLogger(__name__) @@ -209,6 +209,58 @@ def send_sso_configured_email( raise exc +def _recipients_for_identified_users( + user_id_by_email, + maximum_aliases_per_batch=MAX_NUM_IDENTIFY_USERS_ALIASES, + alias_label=ENTERPRISE_BRAZE_ALIAS_LABEL +): + """ + Helper function for create_recipients that takes a dictionary of user_email keys and + user_id values, batches them in groups of the maximum number of users alias based + on the braze documentation, and destructures the individual recipient values from + recipients_by_email and returns a list of recipients. + + Arguments: + * user_id_by_email (dict): A dictionary of user_email key and user_id values + * maximum_aliases_per_batch (int): An integer denoting the max allowable aliases to identify + per create_recipients call to braze. + Default is MAX_NUM_IDENTIFY_USERS_ALIASES + * alias_label (string): A string denoting the alias label requried by braze. + Default is ENTERPRISE_BRAZE_ALIAS_LABEL + + Return: + * recipients (list): A list of dictionary recipients + + Example: + Input: + user_id_by_email = { + 'test@gmail.com': 12345 + } + maximum_aliases_per_batch: 50 + alias_label: Titans + Output: [ + { + 'external_user_id: 12345, + 'attributes: { + 'user_alias': { + 'external_id': 12345, + 'alias_label': 'Titans' + }, + }, + }, + ] + """ + braze_client_instance = BrazeAPIClient() + recipients = [] + for user_id_by_email_chunk in batch_dict(user_id_by_email, maximum_aliases_per_batch): + recipients_by_email = braze_client_instance.create_recipients( + alias_label, + user_id_by_email=user_id_by_email_chunk + ) + recipients.extend(recipients_by_email.values()) + return recipients + + @shared_task @set_code_owner_attribute def send_group_membership_invitation_notification( @@ -240,16 +292,15 @@ def send_group_membership_invitation_notification( braze_trigger_properties['act_by_date'] = act_by_date.strftime('%B %d, %Y') pecu_emails = [] - ecus = [] + user_id_by_email = {} membership_records = enterprise_group_membership_model().objects.filter(uuid__in=membership_uuids) for group_membership in membership_records: if group_membership.pending_enterprise_customer_user is not None: pecu_emails.append(group_membership.pending_enterprise_customer_user.user_email) else: - ecus.append({ - 'user_email': group_membership.enterprise_customer_user.user_email, - 'user_id': group_membership.enterprise_customer_user.user_id - }) + user_id_by_email[ + group_membership.enterprise_customer_user.user_email + ] = group_membership.enterprise_customer_user.user_id recipients = [] for pecu_email in pecu_emails: recipients.append(braze_client_instance.create_recipient_no_external_id(pecu_email)) @@ -257,10 +308,7 @@ def send_group_membership_invitation_notification( [pecu_emails], ENTERPRISE_BRAZE_ALIAS_LABEL, ) - for ecu in ecus: - recipients.append(braze_client_instance.create_recipient( - user_email=ecu['user_email'], - lms_user_id=ecu['user_id'])) + recipients.extend(_recipients_for_identified_users(user_id_by_email)) try: braze_client_instance.send_campaign_message( settings.BRAZE_GROUPS_INVITATION_EMAIL_CAMPAIGN_ID, @@ -298,16 +346,15 @@ def send_group_membership_removal_notification(enterprise_customer_uuid, members 'catalog_content_count' ] = enterprise_catalog_client.get_catalog_content_count(catalog_uuid) pecu_emails = [] - ecus = [] + user_id_by_email = {} membership_records = enterprise_group_membership_model().objects.filter(uuid__in=membership_uuids) for group_membership in membership_records: if group_membership.pending_enterprise_customer_user is not None: pecu_emails.append(group_membership.pending_enterprise_customer_user.user_email) else: - ecus.append({ - 'user_email': group_membership.enterprise_customer_user.user_email, - 'user_id': group_membership.enterprise_customer_user.user_id - }) + user_id_by_email[ + group_membership.enterprise_customer_user.user_email + ] = group_membership.enterprise_customer_user.user_id recipients = [] for pecu_email in pecu_emails: @@ -316,11 +363,7 @@ def send_group_membership_removal_notification(enterprise_customer_uuid, members [pecu_emails], ENTERPRISE_BRAZE_ALIAS_LABEL, ) - for ecu in ecus: - recipients.append(braze_client_instance.create_recipient( - user_email=ecu['user_email'], - lms_user_id=ecu['user_id'] - )) + recipients.extend(_recipients_for_identified_users(user_id_by_email)) try: braze_client_instance.send_campaign_message( settings.BRAZE_GROUPS_REMOVAL_EMAIL_CAMPAIGN_ID, diff --git a/enterprise/utils.py b/enterprise/utils.py index 435796453b..b2768148dc 100644 --- a/enterprise/utils.py +++ b/enterprise/utils.py @@ -7,6 +7,7 @@ import os import re from collections import OrderedDict +from itertools import islice from urllib.parse import parse_qs, quote, urlencode, urljoin, urlparse, urlsplit, urlunsplit from uuid import UUID, uuid4 @@ -2297,6 +2298,22 @@ def batch(iterable, batch_size=1): yield iterable[index:min(index + batch_size, iterable_len)] +def batch_dict(dict_data, chunk_size=1): + """ + Breaks up a dictionary into equal-sized chunks. + No fillers values are added for any 'remainder' chunks + + Arguments: + dict (dict): A dictionary to chunk + chunk_size (int): the size of each chunk. Defaults to 1. + Returns: + generator: iterates through each chunk of a dictionary + """ + it = iter(dict_data.items()) + for _ in range(0, len(dict_data), chunk_size): + yield dict(islice(it, chunk_size)) + + def get_best_mode_from_course_key(course_key): """ Helper method to retrieve a list of enrollments for a given course and select the one most applicable to enroll an diff --git a/requirements/dev.txt b/requirements/dev.txt index 435655137a..c792bb9536 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -351,7 +351,7 @@ edx-api-doc-tools==1.8.0 # -r requirements/doc.txt # -r requirements/test-master.txt # -r requirements/test.txt -edx-braze-client==0.2.2 +edx-braze-client==0.2.5 # via # -r requirements/doc.txt # -r requirements/test-master.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 0374344a8b..5559e3da42 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -217,7 +217,7 @@ drf-yasg==1.21.5 # edx-api-doc-tools edx-api-doc-tools==1.8.0 # via -r requirements/test-master.txt -edx-braze-client==0.2.2 +edx-braze-client==0.2.5 # via -r requirements/test-master.txt edx-django-utils==5.13.0 # via diff --git a/requirements/edx-platform-constraints.txt b/requirements/edx-platform-constraints.txt index 9828fffb8a..355b0cc2bb 100644 --- a/requirements/edx-platform-constraints.txt +++ b/requirements/edx-platform-constraints.txt @@ -411,7 +411,7 @@ edx-auth-backends==4.3.0 # via # -r requirements/edx/kernel.in # openedx-blockstore -edx-braze-client==0.2.2 +edx-braze-client==0.2.5 # via # -r requirements/edx/bundled.in # edx-enterprise diff --git a/requirements/test-master.txt b/requirements/test-master.txt index 932e08a15f..b6d6cdb4c7 100644 --- a/requirements/test-master.txt +++ b/requirements/test-master.txt @@ -218,7 +218,7 @@ edx-api-doc-tools==1.8.0 # via # -c requirements/edx-platform-constraints.txt # -r requirements/test-master.in -edx-braze-client==0.2.2 +edx-braze-client==0.2.5 # via # -c requirements/edx-platform-constraints.txt # -r requirements/base.in diff --git a/requirements/test.txt b/requirements/test.txt index 39c22077ca..88fce3a46e 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -200,7 +200,7 @@ drf-yasg==1.21.5 # edx-api-doc-tools edx-api-doc-tools==1.8.0 # via -r requirements/test-master.txt -edx-braze-client==0.2.2 +edx-braze-client==0.2.5 # via -r requirements/test-master.txt edx-django-utils==5.13.0 # via diff --git a/test_utils/fake_user_id_by_emails.py b/test_utils/fake_user_id_by_emails.py new file mode 100644 index 0000000000..6a92d9d7a2 --- /dev/null +++ b/test_utils/fake_user_id_by_emails.py @@ -0,0 +1,18 @@ +""" +Utility functions for tests +""" +import math +import random +import string + + +def generate_emails_and_ids(num_emails): + """ + Generates random emails with random uuids used primarily to test length constraints + """ + emails_and_ids = { + ''.join(random.choices(string.ascii_uppercase + string.digits, k=8)) + + '@gmail.com': math.floor(random.random() * 1000) + for _ in range(num_emails) + } + return emails_and_ids diff --git a/tests/test_enterprise/test_tasks.py b/tests/test_enterprise/test_tasks.py index eaa460b706..bc54fbc59a 100644 --- a/tests/test_enterprise/test_tasks.py +++ b/tests/test_enterprise/test_tasks.py @@ -213,8 +213,29 @@ def test_send_group_membership_invitation_notification(self, mock_braze_api_clie activated_at=datetime.now() ) admin_email = 'edx@example.org' - mock_recipients = [self.pending_enterprise_customer_user.user_email, - mock_braze_api_client().create_recipient.return_value] + mock_braze_api_client().create_recipients.return_value = { + self.user.email: { + "external_user_id": self.user.id, + "attributes": { + "user_alias": { + "external_id": self.user.id, + "user_alias": self.user.email, + }, + } + } + } + mock_recipients = [ + self.pending_enterprise_customer_user.user_email, + { + "external_user_id": self.user.id, + "attributes": { + "user_alias": { + "external_id": self.user.id, + "user_alias": self.user.email, + }, + } + } + ] mock_catalog_content_count = 5 mock_admin_mailto = f'mailto:{admin_email}' mock_braze_api_client().generate_mailto_link.return_value = mock_admin_mailto @@ -261,8 +282,29 @@ def test_send_group_membership_removal_notification(self, mock_braze_api_client, activated_at=datetime.now() ) admin_email = 'edx@example.org' - mock_recipients = [self.pending_enterprise_customer_user.user_email, - mock_braze_api_client().create_recipient.return_value] + mock_braze_api_client().create_recipients.return_value = { + self.user.email: { + "external_user_id": self.user.id, + "attributes": { + "user_alias": { + "external_id": self.user.id, + "user_alias": self.user.email, + }, + } + } + } + mock_recipients = [ + self.pending_enterprise_customer_user.user_email, + { + "external_user_id": self.user.id, + "attributes": { + "user_alias": { + "external_id": self.user.id, + "user_alias": self.user.email, + }, + } + } + ] mock_admin_mailto = f'mailto:{admin_email}' mock_braze_api_client().generate_mailto_link.return_value = mock_admin_mailto mock_braze_api_client().create_recipient_no_external_id.return_value = ( diff --git a/tests/test_enterprise/test_utils.py b/tests/test_enterprise/test_utils.py index fcc2fa793f..487b0ae88d 100644 --- a/tests/test_enterprise/test_utils.py +++ b/tests/test_enterprise/test_utils.py @@ -1,6 +1,7 @@ """ Tests for the `edx-enterprise` utils module. """ +import math import unittest from datetime import timedelta from unittest import mock @@ -16,6 +17,7 @@ from enterprise.constants import MAX_ALLOWED_TEXT_LENGTH from enterprise.models import EnterpriseCourseEnrollment, LicensedEnterpriseCourseEnrollment from enterprise.utils import ( + batch_dict, enroll_subsidy_users_in_courses, get_default_invite_key_expiration_date, get_idiff_list, @@ -27,6 +29,7 @@ truncate_string, ) from test_utils import FAKE_UUIDS, TEST_PASSWORD, TEST_USERNAME, factories +from test_utils.fake_user_id_by_emails import generate_emails_and_ids LMS_BASE_URL = 'https://lms.base.url' @@ -650,3 +653,37 @@ def test_truncate_string(self): (truncated_string, was_truncated) = truncate_string(test_string_2) self.assertTrue(was_truncated) self.assertEqual(len(truncated_string), MAX_ALLOWED_TEXT_LENGTH) + + @ddt.unpack + @ddt.data( + ( + 50, + 200 + ), + ( + 50, + 56 + ), + ( + 50, + 24 + ), + ( + 1, + 200 + ) + ) + def test_batch_dict_multiple_batches(self, items_per_batch, generated_emails_count): + """ + Test that `batch_dict` returns a set of batched dictionaries + """ + fake_user_ids_by_emails = generate_emails_and_ids(generated_emails_count) + for fake_user_id_by_email_chunk in batch_dict(fake_user_ids_by_emails, items_per_batch): + if len(fake_user_id_by_email_chunk) != items_per_batch: + assert len(fake_user_id_by_email_chunk) == generated_emails_count % items_per_batch + else: + assert len(fake_user_id_by_email_chunk) == items_per_batch + assert isinstance(fake_user_id_by_email_chunk, dict) + assert sum( + 1 for _ in batch_dict(fake_user_ids_by_emails, items_per_batch) + ) == math.ceil(generated_emails_count / items_per_batch )