feat: set traverse pagination to False (#32269)

* For enrollment email task, while getting course uuid and
owner data don't traverse the discovery endpoint.
* Update log message
This commit is contained in:
Zainab Amir
2023-05-19 19:46:33 +05:00
committed by GitHub
parent 0fd36f97f3
commit fa7953ca24
4 changed files with 109 additions and 19 deletions

View File

@@ -590,7 +590,7 @@ class CourseEnrollment(models.Model):
segment_traits['email'] = self.user.email
if event_name == EVENT_NAME_ENROLLMENT_ACTIVATED:
if should_send_enrollment_email():
if should_send_enrollment_email() and self.course_id:
course_pacing_type = 'self-paced' if self.course_overview.self_paced else 'instructor-paced'
send_course_enrollment_email.apply_async((self.user.id, str(self.course_id),
self.course_overview.display_name,

View File

@@ -79,7 +79,7 @@ def send_course_enrollment_email(
if not course_ended:
course_date_blocks = get_course_dates_for_email(user, course_key, request=None)
except Exception as err: # pylint: disable=broad-except
log.exception(err)
log.exception(f'[Enrollment email] Failed to get course dates with error: {err}')
is_course_date_missing = True
canvas_entry_properties.update(
@@ -89,17 +89,16 @@ def send_course_enrollment_email(
}
)
if course_id is None:
raise ValueError('missing course_id')
try:
course_uuid = get_course_uuid_for_course(course_id)
if course_uuid is None:
raise ValueError('missing course_uuid')
owners = get_owners_for_course(course_uuid=course_uuid) or [{}]
raise ValueError('Missing course_uuid')
owners = get_owners_for_course(course_uuid=course_uuid)
course_run = get_course_run_details(course_id, course_run_fields)
if course_run is None:
raise ValueError('missing course_run')
if not course_run:
raise ValueError('Missing course_run')
marketing_root_url = settings.MKTG_URLS.get("ROOT")
instructors = get_instructors(course_run, marketing_root_url)
enrollment_count = int(course_run.get("enrollment_count")) if course_run.get("enrollment_count") else 0
@@ -115,12 +114,12 @@ def send_course_enrollment_email(
"course_title": course_run.get("title"),
"short_description": course_run.get("short_description"),
"pacing_type": course_run.get("pacing_type"),
"partner_image_url": owners[0].get("logo_image_url") or "",
"partner_image_url": owners[0].get("logo_image_url") if owners else "",
}
)
except Exception as err: # pylint: disable=broad-except
is_course_run_missing = True
log.warning(f"[Course Enrollment] Course run call failed for user: {user_id} course: {course_id} err: {err}")
log.warning(f"[Course Enrollment] Course run call failed for user: {user_id} course: {course_id} error: {err}")
if is_course_run_missing or is_course_date_missing:
segment_properties = {

View File

@@ -15,6 +15,7 @@ from common.djangoapps.student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
TASK_LOGGER = 'common.djangoapps.student.tasks.log'
BRAZE_COURSE_ENROLLMENT_CANVAS_ID = "braze-canvas-id"
@@ -247,7 +248,7 @@ class TestCourseEnrollmentEmailTask(ModuleStoreTestCase):
"common.djangoapps.student.tasks.get_course_run_details",
Mock(side_effect=Exception),
)
def test_canvas_properties_without_discovery_call(
def test_canvas_properties_on_get_course_run_details_failure(
self,
mock_get_braze_client,
mock_get_course_dates_for_email,
@@ -255,8 +256,8 @@ class TestCourseEnrollmentEmailTask(ModuleStoreTestCase):
mock_get_course_uuid_for_course,
):
"""
Test to verify the "canvas entry properties" for enrollment email when
course run call is failed.
Test to verify the "canvas entry properties" in the enrollment email when
get_course_run_details fails.
"""
mock_get_course_uuid_for_course.return_value = self.course_uuid
mock_get_owners_for_course.return_value = self._get_course_owners()
@@ -277,6 +278,84 @@ class TestCourseEnrollmentEmailTask(ModuleStoreTestCase):
),
)
@patch("common.djangoapps.student.tasks.get_course_uuid_for_course")
@patch("common.djangoapps.student.tasks.get_course_dates_for_email")
@patch("common.djangoapps.student.tasks.get_braze_client")
@patch(TASK_LOGGER)
def test_email_task_when_course_uuid_is_missing(
self,
mocked_logger,
mock_get_braze_client,
mock_get_course_dates_for_email,
mock_get_course_uuid_for_course,
):
"""
Test that exception is logged when course_uuid returned by
get_course_uuid_for_course is None and that email is sent with
appropriate canvas properties.
"""
mock_get_course_uuid_for_course.return_value = None
mock_get_course_dates_for_email.return_value = self._get_course_dates()
send_course_enrollment_email.apply_async(
kwargs=self.send_course_enrollment_email_kwargs
)
mocked_logger.warning.assert_called_once_with(
f"[Course Enrollment] Course run call failed for "
f"user: {self.user.id} course: {self.course.id} error: Missing course_uuid"
)
mock_get_braze_client.return_value.send_canvas_message.assert_called_with(
canvas_id=BRAZE_COURSE_ENROLLMENT_CANVAS_ID,
recipients=[
{
"external_user_id": self.user.id,
}
],
canvas_entry_properties=self._get_canvas_properties(add_course_run_details=False),
)
@patch("common.djangoapps.student.tasks.get_course_uuid_for_course")
@patch("common.djangoapps.student.tasks.get_owners_for_course")
@patch("common.djangoapps.student.tasks.get_course_run_details")
@patch("common.djangoapps.student.tasks.get_course_dates_for_email")
@patch("common.djangoapps.student.tasks.get_braze_client")
@patch(TASK_LOGGER)
def test_email_task_when_course_run_is_missing(
self,
mocked_logger,
mock_get_braze_client,
mock_get_course_dates_for_email,
mock_get_course_run_details,
mock_get_owners_for_course,
mock_get_course_uuid_for_course,
):
"""
Test that exception is logged when course_run returned by
get_course_run_details is an empty dictionary and that email is sent with
appropriate canvas properties.
"""
mock_get_course_dates_for_email.return_value = self._get_course_dates()
mock_get_course_uuid_for_course.return_value = self.course_uuid
mock_get_owners_for_course.return_value = []
mock_get_course_run_details.return_value = {}
send_course_enrollment_email.apply_async(
kwargs=self.send_course_enrollment_email_kwargs
)
mocked_logger.warning.assert_called_once_with(
f"[Course Enrollment] Course run call failed for "
f"user: {self.user.id} course: {self.course.id} error: Missing course_run"
)
mock_get_braze_client.return_value.send_canvas_message.assert_called_with(
canvas_id=BRAZE_COURSE_ENROLLMENT_CANVAS_ID,
recipients=[
{
"external_user_id": self.user.id,
}
],
canvas_entry_properties=self._get_canvas_properties(add_course_run_details=False),
)
@patch("common.djangoapps.student.tasks.get_course_uuid_for_course")
@patch("common.djangoapps.student.tasks.get_owners_for_course")
@patch("common.djangoapps.student.tasks.get_course_run_details")

View File

@@ -469,9 +469,16 @@ def get_course_runs_for_course(course_uuid): # lint-amnesty, pylint: disable=mi
return []
def get_owners_for_course(course_uuid): # lint-amnesty, pylint: disable=missing-function-docstring
def get_owners_for_course(course_uuid):
"""
Retrieves the course owner given a course uuid.
Arguments
course_uuid (string): Course UUID
"""
if course_uuid is None:
raise ValueError("missing course_uuid")
return []
user, catalog_integration = check_catalog_integration_and_get_user(error_message_field='Owners')
if user:
cache_key = f"{catalog_integration.CACHE_KEY}.course.{course_uuid}.course_runs"
@@ -483,6 +490,7 @@ def get_owners_for_course(course_uuid): # lint-amnesty, pylint: disable=missing
api_client=get_catalog_api_client(user),
base_api_url=get_catalog_api_base_url(),
cache_key=cache_key if catalog_integration.is_cache_enabled else None,
traverse_pagination=False,
long_term_cache=True,
many=False
)
@@ -503,7 +511,8 @@ def get_course_uuid_for_course(course_run_key):
UUID: Course UUID and None if it was not retrieved.
"""
if course_run_key is None:
raise ValueError("missing course_run_key")
return None
user, catalog_integration = check_catalog_integration_and_get_user(error_message_field='Course UUID')
if user:
api_client = get_catalog_api_client(user)
@@ -520,6 +529,7 @@ def get_course_uuid_for_course(course_run_key):
cache_key=run_cache_key if catalog_integration.is_cache_enabled else None,
long_term_cache=True,
many=False,
traverse_pagination=False,
)
course_key_str = course_run_data.get('course', None)
@@ -536,6 +546,7 @@ def get_course_uuid_for_course(course_run_key):
cache_key=run_cache_key if catalog_integration.is_cache_enabled else None,
long_term_cache=True,
many=False,
traverse_pagination=False,
)
uuid_str = data.get('uuid', None)
if uuid_str:
@@ -617,9 +628,10 @@ def get_course_run_details(course_run_key, fields):
Returns:
dict with language, start date, end date, and max_effort details about specified course run
"""
if course_run_key is None:
raise ValueError("missing course_run_key")
course_run_details = {}
if course_run_key is None:
return course_run_details
user, catalog_integration = check_catalog_integration_and_get_user(
error_message_field=f'Data for course_run {course_run_key}'
)