Skip to content

Commit a0bb77a

Browse files
efortishdcoa
efortish
and
dcoa
authored
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 <[email protected]> * refactor: double validation of redirectUrl eliminated and better styling of the message Co-authored-by: Diana Olarte <[email protected]> * 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 <[email protected]>
1 parent 113f0ff commit a0bb77a

3 files changed

Lines changed: 124 additions & 10 deletions

File tree

‎lms/djangoapps/commerce/api/v0/views.py‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,18 @@
99
from opaque_keys.edx.keys import CourseKey
1010
from requests.exceptions import HTTPError
1111
from rest_framework.permissions import IsAuthenticated
12-
from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_409_CONFLICT
12+
from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_409_CONFLICT, HTTP_400_BAD_REQUEST, HTTP_403_FORBIDDEN
1313
from rest_framework.views import APIView
1414

1515
from common.djangoapps.course_modes.models import CourseMode
1616
from common.djangoapps.entitlements.models import CourseEntitlement
17-
from common.djangoapps.student.models import CourseEnrollment
17+
from common.djangoapps.student.models import CourseEnrollment, EnrollmentNotAllowed
1818
from common.djangoapps.util.json_request import JsonResponse
1919
from lms.djangoapps.courseware import courses
2020
from openedx.core.djangoapps.commerce.utils import get_ecommerce_api_base_url, get_ecommerce_api_client
2121
from openedx.core.djangoapps.embargo import api as embargo_api
2222
from openedx.core.djangoapps.enrollments.api import add_enrollment
23+
from openedx.core.djangoapps.enrollments.errors import InvalidEnrollmentAttribute
2324
from openedx.core.djangoapps.enrollments.views import EnrollmentCrossDomainSessionAuth
2425
from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in
2526
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
@@ -149,14 +150,53 @@ def post(self, request, *args, **kwargs): # lint-amnesty, pylint: disable=unuse
149150
announcement=course_announcement
150151
)
151152
log.info(msg)
152-
self._enroll(course_key, user, default_enrollment_mode.slug)
153+
154+
try:
155+
self._enroll(course_key, user, default_enrollment_mode.slug)
156+
except InvalidEnrollmentAttribute as e:
157+
# Exception handling for InvalidEnrollmentAttribute
158+
return self._handle_enrollment_error(
159+
e,
160+
user,
161+
course_id,
162+
"Invalid enrollment attribute ",
163+
HTTP_400_BAD_REQUEST
164+
)
165+
except EnrollmentNotAllowed as e:
166+
# Exception handling for EnrollmentNotAllowed
167+
return self._handle_enrollment_error(
168+
e,
169+
user,
170+
course_id,
171+
"Enrollment not allowed ",
172+
HTTP_403_FORBIDDEN
173+
)
174+
153175
mode = CourseMode.AUDIT if audit_mode else CourseMode.HONOR # lint-amnesty, pylint: disable=unused-variable
154176
self._handle_marketing_opt_in(request, course_key, user)
155177
return DetailResponse(msg)
156178
else:
157179
msg = Messages.NO_DEFAULT_ENROLLMENT_MODE.format(course_id=course_id)
158180
return DetailResponse(msg, status=HTTP_406_NOT_ACCEPTABLE)
159181

182+
def _handle_enrollment_error(self, exception, user, course_id, log_message, status_code):
183+
"""
184+
Helper function to handle enrollment exceptions.
185+
186+
Args:
187+
exception (Exception): The exception raised.
188+
user (User): The user attempting to enroll.
189+
course_id (str): The course ID.
190+
log_message (str): The log message template.
191+
status_code (int): The HTTP status code to return.
192+
193+
Returns:
194+
DetailResponse: The response with the error message and status code.
195+
"""
196+
log.exception(log_message, str(exception))
197+
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
198+
return DetailResponse(error_msg, status=status_code)
199+
160200

161201
class BasketOrderView(APIView):
162202
"""

‎lms/static/js/student_account/enrollment.js‎

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
'use strict';
33

44
define(['jquery', 'jquery.cookie'], function($) {
5+
const ErrorStatuses = {
6+
forbidden: 403,
7+
badRequest: 400
8+
};
9+
510
var EnrollmentInterface = {
611

712
urls: {
@@ -30,12 +35,14 @@
3035
context: this
3136
}).fail(function(jqXHR) {
3237
var responseData = JSON.parse(jqXHR.responseText);
33-
if (jqXHR.status === 403 && responseData.user_message_url) {
34-
// Check if we've been blocked from the course
35-
// because of country access rules.
36-
// If so, redirect to a page explaining to the user
37-
// why they were blocked.
38-
this.redirect(responseData.user_message_url);
38+
if (jqXHR.status === ErrorStatuses.forbidden) {
39+
if (responseData.user_message_url) {
40+
this.redirect(responseData.user_message_url);
41+
} else {
42+
this.showMessage(responseData);
43+
}
44+
} else if (jqXHR.status === ErrorStatuses.badRequest) {
45+
this.showMessage(responseData);
3946
} else {
4047
// Otherwise, redirect the user to the next page.
4148
if (redirectUrl) {
@@ -52,7 +59,54 @@
5259
}
5360
});
5461
},
62+
/**
63+
* Show a message in the frontend.
64+
* @param {Object} message The message to display.
65+
*/
66+
showMessage: function(message) {
67+
const componentId = 'student-enrollment-feedback-error';
68+
const existing = document.getElementById(componentId);
69+
if (existing) {
70+
existing.remove();
71+
}
72+
// Using a fixed dashboard URL as the redirect destination since this is the most logical
73+
// place for users to go after encountering an enrollment error. The URL is hardcoded
74+
// because environment variables are not injected into the HTML/JavaScript context.
75+
const DASHBOARD_URL = '/dashboard';
76+
const textContent = (message && message.detail) ? message.detail : String(message);
77+
const messageDiv = document.createElement('div');
78+
messageDiv.setAttribute('id', componentId);
79+
messageDiv.setAttribute('class', 'fixed-top d-flex justify-content-center align-items-center');
80+
messageDiv.style.cssText = [
81+
'width:100vw',
82+
'height:100vh',
83+
'background:rgba(0,0,0,0.5)',
84+
'z-index:9999'
85+
].join(';');
5586

87+
const buttonText = typeof gettext === 'function' ? gettext('Close') : 'Close';
88+
89+
messageDiv.innerHTML = `
90+
<div class="page-banner w-75 has-actions">
91+
<div class="alert alert-warning" role="alert">
92+
<div class="row w-100">
93+
<div class="col d-flex align-items-center">
94+
<span class="icon icon-alert fa fa-warning me-2" aria-hidden="true"></span>
95+
<span class="message-content" style="min-width: 0; overflow-wrap: anywhere;">${textContent}</span>
96+
</div>
97+
<div class="nav-actions mt-3 flex-row-reverse d-none">
98+
<button type="button" class="action-primary" id="enrollment-redirect-btn">${buttonText}</button>
99+
</div>
100+
</div>
101+
</div>
102+
</div>
103+
`;
104+
const actionContainer = messageDiv.querySelector('.nav-actions');
105+
actionContainer.classList.replace('d-none', 'd-flex');
106+
actionContainer.querySelector('button').addEventListener('click', () => this.redirect(DASHBOARD_URL) )
107+
document.body.appendChild(messageDiv);
108+
109+
},
56110
/**
57111
* Redirect to a URL. Mainly useful for mocking out in tests.
58112
* @param {string} url The URL to redirect to.
@@ -65,3 +119,4 @@
65119
return EnrollmentInterface;
66120
});
67121
}).call(this, define || RequireJS.define);
122+

‎openedx/core/djangoapps/enrollments/views.py‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
from common.djangoapps.course_modes.models import CourseMode
3131
from common.djangoapps.student.auth import user_has_role
32-
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, User
32+
from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, EnrollmentNotAllowed, User
3333
from common.djangoapps.student.roles import CourseStaffRole, GlobalStaff
3434
from common.djangoapps.util.disable_rate_limit import can_disable_rate_limit
3535
from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf
@@ -41,6 +41,7 @@
4141
CourseEnrollmentError,
4242
CourseEnrollmentExistsError,
4343
CourseModeNotFoundError,
44+
InvalidEnrollmentAttribute,
4445
)
4546
from openedx.core.djangoapps.enrollments.forms import CourseEnrollmentsApiListForm
4647
from openedx.core.djangoapps.enrollments.paginators import CourseEnrollmentsApiListPagination
@@ -869,6 +870,23 @@ def post(self, request):
869870

870871
log.info("The user [%s] has already been enrolled in course run [%s].", username, course_id)
871872
return Response(response)
873+
874+
except InvalidEnrollmentAttribute as error:
875+
return Response(
876+
status=status.HTTP_400_BAD_REQUEST,
877+
data={
878+
"message": str(error),
879+
"localizedMessage": str(error),
880+
}
881+
)
882+
except EnrollmentNotAllowed as error:
883+
return Response(
884+
status=status.HTTP_403_FORBIDDEN,
885+
data={
886+
"message": str(error),
887+
"localizedMessage": str(error),
888+
}
889+
)
872890
except CourseModeNotFoundError as error:
873891
return Response(
874892
status=status.HTTP_400_BAD_REQUEST,
@@ -901,6 +919,7 @@ def post(self, request):
901919
).format(username=username, course_id=course_id)
902920
},
903921
)
922+
904923
except CourseUserGroup.DoesNotExist:
905924
log.exception("Missing cohort [%s] in course run [%s]", cohort_name, course_id)
906925
return Response(

0 commit comments

Comments
 (0)