Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions openedx/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ def _is_duplicate_username_error(resp, data):
)


def _is_duplicate_email_error(resp, data):
"""Check if the response indicates a duplicate email error."""
return (
resp.status_code == status.HTTP_409_CONFLICT
and data.get("error_code") == "duplicate-email"
)


def _is_bad_request(resp):
"""Check if the response indicates a bad request."""
return resp.status_code in (
Expand Down Expand Up @@ -257,7 +265,7 @@ def _set_edx_error(open_edx_user, data):
open_edx_user.save()


def _create_edx_user_request(open_edx_user, user, access_token):
def _create_edx_user_request(open_edx_user, user, access_token): # noqa: C901, PLR0915
"""
Handle the actual user creation request to Open edX with retry logic for duplicate usernames.

Expand Down Expand Up @@ -306,7 +314,7 @@ def _create_edx_user_request(open_edx_user, user, access_token):
try:
resp = None
data = None

tried_duplicate_email_fix = False
while attempt < max_attempts:
attempt += 1

Expand All @@ -327,6 +335,16 @@ def _create_edx_user_request(open_edx_user, user, access_token):

data = _parse_openedx_response(resp)

# Only try for LTI user duplicate email error fix if the response error was duplicate-email
if _is_duplicate_email_error(resp, data) and not tried_duplicate_email_fix:
tried_duplicate_email_fix = True
client = get_edx_api_lti_dup_email_client()
dup_email_fix_resp = client.fix_lti_user(email=user.email)
# If the LTI API fixed the duplicate email issue, Retry account creation
if dup_email_fix_resp.status_code == status.HTTP_200_OK:
continue
break

new_username, should_continue, should_reset_attempts = (
_handle_username_collision(
resp,
Expand All @@ -343,8 +361,7 @@ def _create_edx_user_request(open_edx_user, user, access_token):
if should_reset_attempts:
attempt = 0
continue
else:
break
break

if _is_bad_request(resp):
# this is a known type of error dependent on user input
Expand Down Expand Up @@ -923,6 +940,17 @@ def get_edx_api_course_detail_client():
return edx_client.course_detail


def get_edx_api_lti_dup_email_client():
"""
Gets an edx api client instance for use with the grades api

Returns:
CourseDetails: edx api course client instance
"""
edx_client = get_edx_api_service_client()
return edx_client.lti_tools


def get_edx_api_course_mode_client():
"""
Gets an edx api client instance for use with the grades api
Expand Down
35 changes: 32 additions & 3 deletions openedx/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,20 @@ def test_create_edx_user( # noqa: PLR0913
},
],
)
def test_create_edx_user_409_errors(settings, error_data):
"""Test that create_edx_user handles a 409 response from the edX API"""
@pytest.mark.parametrize(
"lti_fix_response_status",
[
status.HTTP_200_OK,
status.HTTP_400_BAD_REQUEST,
status.HTTP_500_INTERNAL_SERVER_ERROR,
],
)
def test_create_edx_user_409_errors(settings, error_data, lti_fix_response_status):
"""Test that create_edx_user handles a 409 response from the edX API
1. If the error is duplicate-email, it should call the LTI fix endpoint
2. If the LTI fix endpoint returns 200, the user should be marked as synced
3. If the LTI fix endpoint returns non-200, the user should be marked as having a sync error
"""
user = UserFactory.create(
openedx_user__has_been_synced=False,
)
Expand All @@ -262,11 +274,28 @@ def test_create_edx_user_409_errors(settings, error_data):
json=error_data,
status=status.HTTP_409_CONFLICT,
)
resp3 = responses.add(
responses.POST,
f"{settings.OPENEDX_API_BASE_URL}/api/lti-user-fix/",
json={},
status=lti_fix_response_status,
)

create_edx_user(user)

is_duplicate_email = error_data.get("error_code") == "duplicate-email"

assert resp1.call_count == 0
assert resp2.call_count == 1
if is_duplicate_email:
assert (
resp2.call_count == 2
if lti_fix_response_status == status.HTTP_200_OK
else 1
)
assert resp3.call_count == 1
else:
assert resp2.call_count == 1
assert resp3.call_count == 0

user.refresh_from_db()

Expand Down
14 changes: 9 additions & 5 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ djangorestframework = "3.16.1"
djoser = "^2.1.0"
drf-extensions = "^0.8.0"
drf-spectacular = "^0.28.0"
edx-api-client = "^1.13.0"
edx-api-client = { git = "https://github.com/mitodl/edx-api-client", branch = "arslan/8853-fix-lti-dup-users" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be replaced by an actual pin once mitodl/edx-api-client#133 is merged and released.

hubspot-api-client = "^6.1.0"
ipython = "^8.0.0"
mitol-django-apigateway = "2025.8.14"
Expand Down