diff --git a/openedx/api.py b/openedx/api.py index 0862b00c9d..1b767fe4e8 100644 --- a/openedx/api.py +++ b/openedx/api.py @@ -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 ( @@ -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. @@ -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 @@ -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, @@ -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 @@ -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 diff --git a/openedx/api_test.py b/openedx/api_test.py index f136f8e318..0c41076586 100644 --- a/openedx/api_test.py +++ b/openedx/api_test.py @@ -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, ) @@ -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() diff --git a/poetry.lock b/poetry.lock index dd60901b95..e054a0a167 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1659,10 +1659,8 @@ description = "Python interface to the edX REST APIs" optional = false python-versions = "*" groups = ["main"] -files = [ - {file = "edx_api_client-1.13.0-py2.py3-none-any.whl", hash = "sha256:60a949eb31ddf3f8b0ddb63189d58bc9e7057117dfad032c9416a1ff1a401fae"}, - {file = "edx_api_client-1.13.0.tar.gz", hash = "sha256:d52f3267e0af0d4bb60982d31fd792c4edf10bb935ab0389c7b47c9dd8737e27"}, -] +files = [] +develop = false [package.dependencies] python-dateutil = ">=2.5.3,<4.0.0" @@ -1671,6 +1669,12 @@ requests = ">=2.20.0" [package.extras] dev = ["flake8"] +[package.source] +type = "git" +url = "https://github.com/mitodl/edx-api-client" +reference = "arslan/8853-fix-lti-dup-users" +resolved_reference = "8a960039efe823ef7647cb530824ebf8360e1c58" + [[package]] name = "edx-opaque-keys" version = "2.2.2" @@ -6065,4 +6069,4 @@ cffi = ["cffi (>=1.17,<2.0) ; platform_python_implementation != \"PyPy\" and pyt [metadata] lock-version = "2.1" python-versions = "^3.10" -content-hash = "44a47cb0e0b13e55a51419a5d16738e2419426e1f7e5214a3aca3e7ab2e5bbee" +content-hash = "61c6c8202dd15922a558b2eac7fe3e820b5ee9e285db063bf7b712b2d5c8c214" diff --git a/pyproject.toml b/pyproject.toml index bca8b2b5ef..b9c9a2cfac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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" } hubspot-api-client = "^6.1.0" ipython = "^8.0.0" mitol-django-apigateway = "2025.8.14"