Resolve issue when repairing user's edx synchronised records

openedx/api.py

@@ -94,12 +94,21 @@ def create_edx_user(user):
     )
 
     with transaction.atomic():
-        _, created = OpenEdxUser.objects.select_for_update().get_or_create(
+        open_edx_user, created = OpenEdxUser.objects.select_for_update().get_or_create(
             user=user, platform=PLATFORM_EDX
         )
 
-        if not created:
-            return
+        if not created and open_edx_user.has_been_synced:
+            # Here we should check with edx that the user exists on that end.
+            try:
+                client = get_edx_api_client(user)
+                client.user_info.get_user_info()
+            except:
+                pass
+            else:
+                open_edx_user.has_been_synced = True
+                open_edx_user.save()
+                return False
 
         # a non-200 status here will ensure we rollback creation of the OpenEdxUser and try again
         req_session = requests.Session()
@@ -125,6 +134,9 @@ def create_edx_user(user):
             raise OpenEdxUserCreateError(
                 f"Error creating Open edX user. {get_error_response_summary(resp)}"
             )
+        open_edx_user.has_been_synced = True
+        open_edx_user.save()
+        return True
 
 
 @transaction.atomic
@@ -290,14 +302,7 @@ def repair_faulty_edx_user(user):
     """
     created_user, created_auth_token = False, False
     try:
-        if (
-            find_object_with_matching_attr(
-                user.openedx_users.all(), "platform", value=PLATFORM_EDX
-            )
-            is None
-        ):
-            create_edx_user(user)
-            created_user = True
+        created_user = create_edx_user(user)
     except Exception as e:
         # 409 means we have a username conflict - pass in that case so we can
         # try to create the api auth tokens; re-raise otherwise

openedx/api_test.py

@@ -182,6 +182,43 @@ def test_create_edx_user_conflict(settings, user):
     assert OpenEdxUser.objects.count() == 0
 
 
+@responses.activate
+@pytest.mark.parametrize(
+    "open_edx_user_record_exists,open_edx_user_record_has_been_synced",
+    itertools.product([True, False], [True, False]),
+)
+def test_create_edx_user_for_user_not_synced_with_edx(
+    mocker,
+    settings,
+    user,
+    open_edx_user_record_exists,
+    open_edx_user_record_has_been_synced,
+):
+    """Test that create_edx_user validates the user record on Edx if an OpenEdxUser record already exists."""
+
+    responses.add(
+        responses.POST,
+        f"{settings.OPENEDX_API_BASE_URL}/user_api/v1/account/registration/",
+        json=dict(success=True),
+        status=status.HTTP_200_OK,
+    )
+    OpenEdxUserFactory.create(
+        user=user, has_been_synced=open_edx_user_record_has_been_synced
+    )
+    mock_client = mocker.MagicMock()
+    mock_client.user_info.get_user_info = mocker.Mock(
+        side_effect=Exception if not open_edx_user_record_exists else None,
+    )
+    mocker.patch("openedx.api.get_edx_api_client", return_value=mock_client)
+
+    user_created_in_edx = create_edx_user(user)
+
+    assert OpenEdxUser.objects.get(user=user).has_been_synced is True
+    assert user_created_in_edx is not (
+        open_edx_user_record_exists and open_edx_user_record_has_been_synced
+    )
+
+
 @responses.activate
 def test_validate_edx_username_conflict(settings, user):
     """Test that validate_username_with_edx handles a username validation conflict"""
@@ -502,19 +539,16 @@ def test_repair_faulty_edx_user(mocker, user, no_openedx_user, no_edx_auth):
     Tests that repair_faulty_edx_user creates OpenEdxUser/OpenEdxApiAuth objects as necessary and
     returns flags that indicate what was created
     """
-    patched_create_edx_user = mocker.patch("openedx.api.create_edx_user")
     patched_create_edx_auth_token = mocker.patch("openedx.api.create_edx_auth_token")
-    openedx_user = OpenEdxUserFactory.create(user=user)
-    patched_find_object = mocker.patch(
-        "openedx.api.find_object_with_matching_attr",
-        return_value=None if no_openedx_user else openedx_user,
+    OpenEdxUserFactory.create(user=user)
+    mocker.patch(
+        "openedx.api.create_edx_user",
+        return_value=True if no_openedx_user else False,
     )
     openedx_api_auth = None if no_edx_auth else OpenEdxApiAuthFactory.build()
     user.openedx_api_auth = openedx_api_auth
 
     created_user, created_auth_token = repair_faulty_edx_user(user)
-    patched_find_object.assert_called()
-    assert patched_create_edx_user.called is no_openedx_user
     assert patched_create_edx_auth_token.called is no_edx_auth
     assert created_user is no_openedx_user
     assert created_auth_token is no_edx_auth

openedx/migrations/0005_alter_openedxuser_has_been_synced.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.18 on 2023-03-17 20:49
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("openedx", "0004_add_openedx_related_names"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="openedxuser",
+            name="has_been_synced",
+            field=models.BooleanField(
+                default=False,
+                help_text="Indicates whether a corresponding user has been created on the openedx platform",
+            ),
+        ),
+    ]

openedx/models.py

@@ -18,7 +18,7 @@ class OpenEdxUser(TimestampedModel):
         max_length=20, choices=OPENEDX_PLATFORM_CHOICES, default=PLATFORM_EDX
     )
     has_been_synced = models.BooleanField(
-        default=True,
+        default=False,
         help_text="Indicates whether a corresponding user has been created on the openedx platform",
     )