From a0bb77a6d88a2bd6f1e3ce57604eabe66caa52fe Mon Sep 17 00:00:00 2001 From: KEVYN SUAREZ <91025555+efortish@users.noreply.github.com> Date: Thu, 3 Jul 2025 05:36:14 -0500 Subject: [PATCH] fix: commerce and enrollment error handling and ux (#36612) Returns HTTP 400 for disallowed enrollments instead of HTTP 500. Prevents infinite loading spinners on the enrollment page. Displays clear error messages to users before redirection. Ensures consistent and meaningful responses from enrollment API endpoints. * fix: commerce and enrollment apis return 403 when enrollment not allowed * fix: now both apis send the right message and http code when enrollment fails * fix: InvalidEnrollmentAtribute as final exception to catch and HTTP 400 returned * style: the message is displayed as a popup instead of creating a div at the end * fix: import not used removed for pylint checks * style: popup now use utility classes * refactor: use const instead of let for existing const * refactor: textContent const structure changed due check failed * refactor: SetTimeout settled as arrow function * feat: button incorporated to bring users enough time to read the message * refactor: ErrorStatuses defined at the top of the file to use it in conditionals * style: typo fixed Co-authored-by: Diana Olarte * refactor: double validation of redirectUrl eliminated and better styling of the message Co-authored-by: Diana Olarte * refactor: redirectUrl param eliminated in showmessage function, close button redirects to dashboard always * docs: remove unused redirectUrl param from JSDoc and explain hardcoded URL * style: endline added * feat: enrollmentNotAllowed exception added in views and the js * docs: comment added to especify exception * style: endline added * refactor: error statuses velidation changed to one single validation instead of two * refactor: function added to handle enrollment errors * feat: enrollmentNotAllowed exception added for API coherence and consistency * style: empty line added * style: pylint check line too long disabled --------- Co-authored-by: Diana Olarte --- lms/djangoapps/commerce/api/v0/views.py | 46 +++++++++++++- lms/static/js/student_account/enrollment.js | 67 ++++++++++++++++++-- openedx/core/djangoapps/enrollments/views.py | 21 +++++- 3 files changed, 124 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/commerce/api/v0/views.py b/lms/djangoapps/commerce/api/v0/views.py index 5238b842e9..a6cadadb43 100644 --- a/lms/djangoapps/commerce/api/v0/views.py +++ b/lms/djangoapps/commerce/api/v0/views.py @@ -9,17 +9,18 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from requests.exceptions import HTTPError from rest_framework.permissions import IsAuthenticated -from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_409_CONFLICT +from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_409_CONFLICT, HTTP_400_BAD_REQUEST, HTTP_403_FORBIDDEN from rest_framework.views import APIView from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.entitlements.models import CourseEntitlement -from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.models import CourseEnrollment, EnrollmentNotAllowed from common.djangoapps.util.json_request import JsonResponse from lms.djangoapps.courseware import courses from openedx.core.djangoapps.commerce.utils import get_ecommerce_api_base_url, get_ecommerce_api_client from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.enrollments.api import add_enrollment +from openedx.core.djangoapps.enrollments.errors import InvalidEnrollmentAttribute from openedx.core.djangoapps.enrollments.views import EnrollmentCrossDomainSessionAuth from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser @@ -149,7 +150,28 @@ class BasketsView(APIView): announcement=course_announcement ) log.info(msg) - self._enroll(course_key, user, default_enrollment_mode.slug) + + try: + self._enroll(course_key, user, default_enrollment_mode.slug) + except InvalidEnrollmentAttribute as e: + # Exception handling for InvalidEnrollmentAttribute + return self._handle_enrollment_error( + e, + user, + course_id, + "Invalid enrollment attribute ", + HTTP_400_BAD_REQUEST + ) + except EnrollmentNotAllowed as e: + # Exception handling for EnrollmentNotAllowed + return self._handle_enrollment_error( + e, + user, + course_id, + "Enrollment not allowed ", + HTTP_403_FORBIDDEN + ) + mode = CourseMode.AUDIT if audit_mode else CourseMode.HONOR # lint-amnesty, pylint: disable=unused-variable self._handle_marketing_opt_in(request, course_key, user) return DetailResponse(msg) @@ -157,6 +179,24 @@ class BasketsView(APIView): msg = Messages.NO_DEFAULT_ENROLLMENT_MODE.format(course_id=course_id) return DetailResponse(msg, status=HTTP_406_NOT_ACCEPTABLE) + def _handle_enrollment_error(self, exception, user, course_id, log_message, status_code): + """ + Helper function to handle enrollment exceptions. + + Args: + exception (Exception): The exception raised. + user (User): The user attempting to enroll. + course_id (str): The course ID. + log_message (str): The log message template. + status_code (int): The HTTP status code to return. + + Returns: + DetailResponse: The response with the error message and status code. + """ + log.exception(log_message, str(exception)) + error_msg = f"{log_message.format(str(exception))} for user {user.username} in course {course_id}: {str(exception)}" # lint-amnesty, pylint: disable=line-too-long + return DetailResponse(error_msg, status=status_code) + class BasketOrderView(APIView): """ diff --git a/lms/static/js/student_account/enrollment.js b/lms/static/js/student_account/enrollment.js index 1ce8fb0f20..68b4319027 100644 --- a/lms/static/js/student_account/enrollment.js +++ b/lms/static/js/student_account/enrollment.js @@ -2,6 +2,11 @@ 'use strict'; define(['jquery', 'jquery.cookie'], function($) { + const ErrorStatuses = { + forbidden: 403, + badRequest: 400 + }; + var EnrollmentInterface = { urls: { @@ -30,12 +35,14 @@ context: this }).fail(function(jqXHR) { var responseData = JSON.parse(jqXHR.responseText); - if (jqXHR.status === 403 && responseData.user_message_url) { - // Check if we've been blocked from the course - // because of country access rules. - // If so, redirect to a page explaining to the user - // why they were blocked. - this.redirect(responseData.user_message_url); + if (jqXHR.status === ErrorStatuses.forbidden) { + if (responseData.user_message_url) { + this.redirect(responseData.user_message_url); + } else { + this.showMessage(responseData); + } + } else if (jqXHR.status === ErrorStatuses.badRequest) { + this.showMessage(responseData); } else { // Otherwise, redirect the user to the next page. if (redirectUrl) { @@ -52,7 +59,54 @@ } }); }, + /** + * Show a message in the frontend. + * @param {Object} message The message to display. + */ + showMessage: function(message) { + const componentId = 'student-enrollment-feedback-error'; + const existing = document.getElementById(componentId); + if (existing) { + existing.remove(); + } + // Using a fixed dashboard URL as the redirect destination since this is the most logical + // place for users to go after encountering an enrollment error. The URL is hardcoded + // because environment variables are not injected into the HTML/JavaScript context. + const DASHBOARD_URL = '/dashboard'; + const textContent = (message && message.detail) ? message.detail : String(message); + const messageDiv = document.createElement('div'); + messageDiv.setAttribute('id', componentId); + messageDiv.setAttribute('class', 'fixed-top d-flex justify-content-center align-items-center'); + messageDiv.style.cssText = [ + 'width:100vw', + 'height:100vh', + 'background:rgba(0,0,0,0.5)', + 'z-index:9999' + ].join(';'); + const buttonText = typeof gettext === 'function' ? gettext('Close') : 'Close'; + + messageDiv.innerHTML = ` +
+ +
+ `; + const actionContainer = messageDiv.querySelector('.nav-actions'); + actionContainer.classList.replace('d-none', 'd-flex'); + actionContainer.querySelector('button').addEventListener('click', () => this.redirect(DASHBOARD_URL) ) + document.body.appendChild(messageDiv); + + }, /** * Redirect to a URL. Mainly useful for mocking out in tests. * @param {string} url The URL to redirect to. @@ -65,3 +119,4 @@ return EnrollmentInterface; }); }).call(this, define || RequireJS.define); + diff --git a/openedx/core/djangoapps/enrollments/views.py b/openedx/core/djangoapps/enrollments/views.py index e857a0d02d..dc3423245e 100644 --- a/openedx/core/djangoapps/enrollments/views.py +++ b/openedx/core/djangoapps/enrollments/views.py @@ -29,7 +29,7 @@ from rest_framework.views import APIView # lint-amnesty, pylint: disable=wrong- from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.auth import user_has_role -from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, User +from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, EnrollmentNotAllowed, User from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf @@ -41,6 +41,7 @@ from openedx.core.djangoapps.enrollments.errors import ( CourseEnrollmentError, CourseEnrollmentExistsError, CourseModeNotFoundError, + InvalidEnrollmentAttribute, ) from openedx.core.djangoapps.enrollments.forms import CourseEnrollmentsApiListForm from openedx.core.djangoapps.enrollments.paginators import CourseEnrollmentsApiListPagination @@ -869,6 +870,23 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): log.info("The user [%s] has already been enrolled in course run [%s].", username, course_id) return Response(response) + + except InvalidEnrollmentAttribute as error: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": str(error), + "localizedMessage": str(error), + } + ) + except EnrollmentNotAllowed as error: + return Response( + status=status.HTTP_403_FORBIDDEN, + data={ + "message": str(error), + "localizedMessage": str(error), + } + ) except CourseModeNotFoundError as error: return Response( status=status.HTTP_400_BAD_REQUEST, @@ -901,6 +919,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ).format(username=username, course_id=course_id) }, ) + except CourseUserGroup.DoesNotExist: log.exception("Missing cohort [%s] in course run [%s]", cohort_name, course_id) return Response(