From 06923c794832aa3164b4177473d2130e0c72adca Mon Sep 17 00:00:00 2001 From: Mohammad Ahtasham ul Hassan <60315450+aht007@users.noreply.github.com> Date: Mon, 17 Jul 2023 20:17:12 +0500 Subject: [PATCH] feat: use username for course completion API instead of lms_user_id (#32761) --- .../rest_api/v1/tests/test_views.py | 2 +- .../entitlements/rest_api/v1/views.py | 24 +++++++++++-------- common/djangoapps/entitlements/tasks.py | 18 +++++++------- common/djangoapps/entitlements/utils.py | 14 +++++------ openedx/core/djangoapps/credentials/utils.py | 20 ++++++++-------- 5 files changed, 41 insertions(+), 37 deletions(-) diff --git a/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py b/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py index f36d29249a43..34abc39c0096 100644 --- a/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py @@ -1392,4 +1392,4 @@ def test_course_completion_exception_triggers_task(self, mock_get_courses_comple assert response.status_code == 204 mock_task.assert_called_once_with(args=([str(course_entitlement.uuid)], [str(enrollment.course_id)], - self.user.id)) + self.user.username)) diff --git a/common/djangoapps/entitlements/rest_api/v1/views.py b/common/djangoapps/entitlements/rest_api/v1/views.py index 3ca538eb0e1b..6eaf24c3da9e 100644 --- a/common/djangoapps/entitlements/rest_api/v1/views.py +++ b/common/djangoapps/entitlements/rest_api/v1/views.py @@ -4,6 +4,7 @@ import logging +from django.contrib.auth import get_user_model from django.db import IntegrityError, transaction from django.db.models import Q from django.http import HttpResponseBadRequest @@ -42,6 +43,7 @@ from openedx.core.djangoapps.credentials.utils import get_courses_completion_status from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in +User = get_user_model() log = logging.getLogger(__name__) @@ -552,36 +554,38 @@ def _process_revoke_and_downgrade_to_audit(self, course_entitlements, user_id, r course completion status. """ entitled_course_ids = [] + user = User.objects.get(id=user_id) + username = user.username for course_entitlement in course_entitlements: if course_entitlement.enrollment_course_run is not None: entitled_course_ids.append(str(course_entitlement.enrollment_course_run.course_id)) - log.info('B2C_SUBSCRIPTIONS: Getting course completion status for user %s and entitled_course_ids %s', - user_id, + log.info('B2C_SUBSCRIPTIONS: Getting course completion status for user [%s] and entitled_course_ids %s', + username, entitled_course_ids) - awarded_cert_course_ids, is_exception = get_courses_completion_status(user_id, entitled_course_ids) + awarded_cert_course_ids, is_exception = get_courses_completion_status(username, entitled_course_ids) - log.info('B2C_SUBSCRIPTIONS: Got course completion response %s for user %s and entitled_course_ids %s', + log.info('B2C_SUBSCRIPTIONS: Got course completion response %s for user [%s] and entitled_course_ids %s', awarded_cert_course_ids, - user_id, + username, entitled_course_ids) if is_exception: # Trigger the retry task asynchronously log.exception('B2C_SUBSCRIPTIONS: Exception occurred while getting course completion status for user %s ' 'and entitled_course_ids %s', - user_id, + username, entitled_course_ids) retry_revoke_subscriptions_verified_access.apply_async(args=(revocable_entitlement_uuids, entitled_course_ids, - user_id)) + username)) return - log.info('B2C_SUBSCRIPTIONS: Starting revoke_entitlements_and_downgrade_courses_to_audit for user %s and ' + log.info('B2C_SUBSCRIPTIONS: Starting revoke_entitlements_and_downgrade_courses_to_audit for user [%s] and ' 'awarded_cert_course_ids %s and revocable_entitlement_uuids %s', - user_id, + username, awarded_cert_course_ids, revocable_entitlement_uuids) - revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, user_id, awarded_cert_course_ids, + revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, username, awarded_cert_course_ids, revocable_entitlement_uuids) def post(self, request): diff --git a/common/djangoapps/entitlements/tasks.py b/common/djangoapps/entitlements/tasks.py index 47bc0cfd2650..0adb2c333867 100644 --- a/common/djangoapps/entitlements/tasks.py +++ b/common/djangoapps/entitlements/tasks.py @@ -157,7 +157,7 @@ def expire_and_create_entitlements(self, entitlement_ids, support_username): @shared_task(bind=True) -def retry_revoke_subscriptions_verified_access(self, revocable_entitlement_uuids, entitled_course_ids, user_id): +def retry_revoke_subscriptions_verified_access(self, revocable_entitlement_uuids, entitled_course_ids, username): """ Task to process course access revoke and move to audit. This is called only if call to get_courses_completion_status fails due to any exception. @@ -165,7 +165,7 @@ def retry_revoke_subscriptions_verified_access(self, revocable_entitlement_uuids course_entitlements = CourseEntitlement.objects.filter(uuid__in=revocable_entitlement_uuids) course_entitlements = course_entitlements.select_related('user').select_related('enrollment_course_run') if course_entitlements.exists(): - awarded_cert_course_ids, is_exception = get_courses_completion_status(user_id, entitled_course_ids) + awarded_cert_course_ids, is_exception = get_courses_completion_status(username, entitled_course_ids) if is_exception: try: countdown = 2 ** self.request.retries @@ -173,20 +173,20 @@ def retry_revoke_subscriptions_verified_access(self, revocable_entitlement_uuids except MaxRetriesExceededError: log.exception( 'B2C_SUBSCRIPTIONS: Failed to process retry_revoke_subscriptions_verified_access ' - 'for user_id %s and entitlement_uuids %s', - user_id, + 'for user [%s] and entitlement_uuids %s', + username, revocable_entitlement_uuids ) return - log.info('B2C_SUBSCRIPTIONS: Starting revoke_entitlements_and_downgrade_courses_to_audit for user %s and ' + log.info('B2C_SUBSCRIPTIONS: Starting revoke_entitlements_and_downgrade_courses_to_audit for user [%s] and ' 'awarded_cert_course_ids %s and revocable_entitlement_uuids %s from retry task', - user_id, + username, awarded_cert_course_ids, revocable_entitlement_uuids) - revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, user_id, awarded_cert_course_ids, + revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, username, awarded_cert_course_ids, revocable_entitlement_uuids) else: log.info('B2C_SUBSCRIPTIONS: Entitlements not found for the provided entitlements uuids %s ' - 'for user_id %s duing the retry_revoke_subscriptions_verified_access task', + 'for user [%s] duing the retry_revoke_subscriptions_verified_access task', revocable_entitlement_uuids, - user_id) + username) diff --git a/common/djangoapps/entitlements/utils.py b/common/djangoapps/entitlements/utils.py index 397b0cd1c05d..89a00914de6f 100644 --- a/common/djangoapps/entitlements/utils.py +++ b/common/djangoapps/entitlements/utils.py @@ -61,7 +61,7 @@ def is_course_run_entitlement_fulfillable( return course_overview.start and can_upgrade and (is_enrolled or can_enroll) -def revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, user_id, awarded_cert_course_ids, +def revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, username, awarded_cert_course_ids, revocable_entitlement_uuids): """ This method expires the entitlements for provided course_entitlements and also moves the enrollments @@ -69,8 +69,8 @@ def revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, user """ log.info('B2C_SUBSCRIPTIONS: Starting revoke_entitlements_and_downgrade_courses_to_audit for ' - 'user: %s, course_entitlements_uuids: %s, awarded_cert_course_ids: %s', - user_id, + 'user: [%s], course_entitlements_uuids: %s, awarded_cert_course_ids: %s', + username, revocable_entitlement_uuids, awarded_cert_course_ids) for course_entitlement in course_entitlements: @@ -87,11 +87,11 @@ def revoke_entitlements_and_downgrade_courses_to_audit(course_entitlements, user course_entitlement.expire_entitlement() update_enrollment(username, str(course_id), CourseMode.AUDIT, include_expired=True) else: - log.warning('B2C_SUBSCRIPTIONS: Enrollment mode mismatch for user_id: %s and course_id: %s', - user_id, + log.warning('B2C_SUBSCRIPTIONS: Enrollment mode mismatch for user: %s and course_id: %s', + username, course_id) log.info('B2C_SUBSCRIPTIONS: Completed revoke_entitlements_and_downgrade_courses_to_audit for ' - 'user: %s, course_entitlements_uuids: %s, awarded_cert_course_ids: %s', - user_id, + 'user: [%s], course_entitlements_uuids: %s, awarded_cert_course_ids: %s', + username, revocable_entitlement_uuids, awarded_cert_course_ids) diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index 893aba787189..9e7f61e1897e 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -110,11 +110,11 @@ def get_credentials(user, program_uuid=None, credential_type=None): ) -def get_courses_completion_status(lms_user_id, course_run_ids): +def get_courses_completion_status(username, course_run_ids): """ - Given the lms_user_id and course run ids, checks for course completion status + Given the username and course run ids, checks for course completion status Arguments: - lms_user_id (User): The user to authenticate as when requesting credentials. + username (User): Username of the user whose credentials are being requested. course_run_ids(List): list of course run ids for which we need to check the completion status Returns: list of course_run_ids for which user has completed the course @@ -134,7 +134,7 @@ def get_courses_completion_status(lms_user_id, course_run_ids): api_response = api_client.post( completion_status_url, json={ - 'lms_user_id': lms_user_id, + 'username': username, 'course_runs': course_run_ids, } ) @@ -142,16 +142,16 @@ def get_courses_completion_status(lms_user_id, course_run_ids): course_completion_response = api_response.json() except Exception as exc: # pylint: disable=broad-except log.exception("An unexpected error occurred while reqeusting course completion statuses " - "for lms_user_id [%s] for course_run_ids [%s] with exc [%s]:", - lms_user_id, + "for user [%s] for course_run_ids [%s] with exc [%s]:", + username, course_run_ids, exc ) return [], True # Yes, This is course_credentials_data. The key is named status but # it contains all the courses data from credentials. - log.info("Course completion status response for lms_user_id [%s] for course_run_ids [%s] is [%s]", - lms_user_id, + log.info("Course completion status response for user [%s] for course_run_ids [%s] is [%s]", + username, course_run_ids, course_completion_response) course_credentials_data = course_completion_response.get('status', []) @@ -159,8 +159,8 @@ def get_courses_completion_status(lms_user_id, course_run_ids): filtered_records = [course_data['course_run']['key'] for course_data in course_credentials_data if course_data['course_run']['key'] in course_run_ids and course_data['status'] == settings.CREDENTIALS_COURSE_COMPLETION_STATE] - log.info("Filtered course completion status response for lms_user_id [%s] for course_run_ids [%s] is [%s]", - lms_user_id, + log.info("Filtered course completion status response for user [%s] for course_run_ids [%s] is [%s]", + username, course_run_ids, filtered_records) return filtered_records, False