Address code review comments

Author: collinpreston

openedx/api.py

@@ -101,13 +101,14 @@ def create_edx_user(user):
         if not created and open_edx_user.has_been_synced:
             # Here we should check with edx that the user exists on that end.
             try:
-                get_edx_api_client(user)
+                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 created
+                return False
 
         # a non-200 status here will ensure we rollback creation of the OpenEdxUser and try again
         req_session = requests.Session()
@@ -135,7 +136,7 @@ def create_edx_user(user):
             )
         open_edx_user.has_been_synced = True
         open_edx_user.save()
-        return created
+        return True
 
 
 @transaction.atomic

openedx/api_test.py

@@ -205,17 +205,18 @@ def test_create_edx_user_for_user_not_synced_with_edx(
     OpenEdxUserFactory.create(
         user=user, has_been_synced=open_edx_user_record_has_been_synced
     )
-    mocker.patch(
-        "openedx.api.get_edx_api_client",
-        side_effect=ValueError("Unexpected error")
-        if not open_edx_user_record_exists
-        else None,
+    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 False if open_edx_user_record_exists else True
+    assert user_created_in_edx is not (
+        open_edx_user_record_exists and open_edx_user_record_has_been_synced
+    )
 
 
 @responses.activate