refactor: notification for full_name and some minor fixes

authentication/api_test.py

@@ -17,15 +17,12 @@ def test_create_user_session(user):
     assert session.session_key is not None
 
 
+@pytest.mark.usefixtures("mock_validate_user_registration")
 def test_create_user_with_generated_username(mocker, valid_address_dict):
     """
     Integration test to assert that create_user_with_generated_username tries to find an available
     username and try again to save a User if there was a username collision
     """
-    mock_client = mocker.MagicMock()
-    mock_client.user_info.validate_user_registration.return_value = {"validation_decisions": {"name": ""}}
-    mocker.patch("courseware.api.get_edx_api_registration_client", return_value=mock_client)
-
     username = "testuser"
     # Create a user with the desired username before calling the function so we get a collision
     UserFactory.create(username=username)

authentication/pipeline/user_test.py

@@ -257,6 +257,7 @@ def test_create_user_via_email_exit(mocker, backend_name, flow):
 
 
 @pytest.mark.django_db
+@pytest.mark.usefixtures("mock_validate_user_registration")
 def test_create_user_via_email(mocker, mock_email_backend, mock_create_user_strategy):
     """
     Tests that create_user_via_email creates a user via social_core.pipeline.user.create_user_via_email,
@@ -272,9 +273,6 @@ def test_create_user_via_email(mocker, mock_email_backend, mock_create_user_stra
         "authentication.pipeline.user.create_user_with_generated_username",
         return_value=fake_user,
     )
-    mock_client = mocker.MagicMock()
-    mock_client.user_info.validate_user_registration.return_value = {"validation_decisions": {"name": ""}}
-    mocker.patch("courseware.api.get_edx_api_registration_client", return_value=mock_client)
 
     response = user_actions.create_user_via_email(
         mock_create_user_strategy,
@@ -384,6 +382,7 @@ def test_create_user_via_email_with_email_case_insensitive_existing_user(
     "create_user_return_val,create_user_exception",  # noqa: PT006
     [[None, None], [UserFactory.build(), ValueError("bad value")]],  # noqa: PT007
 )
+@pytest.mark.usefixtures("mock_validate_user_registration")
 def test_create_user_via_email_create_fail(
     mocker,
     mock_email_backend,
@@ -397,10 +396,6 @@ def test_create_user_via_email_create_fail(
         return_value=create_user_return_val,
         side_effect=create_user_exception,
     )
-    mock_client = mocker.MagicMock()
-    mock_client.user_info.validate_user_registration.return_value = {"validation_decisions": {"name": ""}}
-    mocker.patch("courseware.api.get_edx_api_registration_client", return_value=mock_client)
-
     with pytest.raises(UserCreationFailedException):
         user_actions.create_user_via_email(
             mock_create_user_strategy,
@@ -413,6 +408,7 @@ def test_create_user_via_email_create_fail(
 
 
 @pytest.mark.django_db
+@pytest.mark.usefixtures("mock_validate_user_registration")
 def test_create_user_via_email_affiliate(
     mocker, mock_create_user_strategy, mock_email_backend
 ):
@@ -426,9 +422,6 @@ def test_create_user_via_email_affiliate(
         "authentication.pipeline.user.create_user_with_generated_username",
         return_value=UserFactory.build(),
     )
-    mock_client = mocker.MagicMock()
-    mock_client.user_info.validate_user_registration.return_value = {"validation_decisions": {"name": ""}}
-    mocker.patch("courseware.api.get_edx_api_registration_client", return_value=mock_client)
     user_actions.create_user_via_email(
         mock_create_user_strategy,
         mock_email_backend,

authentication/serializers.py

@@ -27,6 +27,7 @@
     UserTryAgainLaterException,
 )
 from authentication.utils import SocialAuthState
+from users.constants import USER_REGISTRATION_FAILED_MSG
 
 PARTIAL_PIPELINE_TOKEN_KEY = "partial_pipeline_token"  # noqa: S105
 
@@ -177,7 +178,7 @@ def save(self, **kwargs):  # noqa: C901
         except UserTryAgainLaterException:
             result = SocialAuthState(
                 SocialAuthState.STATE_ERROR_TEMPORARY,
-                errors=["Unable to register at this time, please try again later"],
+                errors=[USER_REGISTRATION_FAILED_MSG],
             )
         except AuthException as exc:
             log.exception("Received unexpected AuthException")

courseware/api.py

@@ -542,11 +542,14 @@ def get_edx_api_registration_client():
     Returns:
          EdxApi: edx api registration client instance
     """
-    if settings.MITXPRO_REGISTRATION_ACCESS_TOKEN is None:
-        raise ImproperlyConfigured("MITXPRO_REGISTRATION_ACCESS_TOKEN is not set")  # noqa: EM101
+    registration_access_token = (
+        settings.MITXPRO_REGISTRATION_ACCESS_TOKEN
+        if settings.MITXPRO_REGISTRATION_ACCESS_TOKEN
+        else ""
+    )
 
     return EdxApi(
-        {"access_token": settings.MITXPRO_REGISTRATION_ACCESS_TOKEN},
+        {"access_token": registration_access_token},
         settings.OPENEDX_API_BASE_URL,
         timeout=settings.EDX_API_CLIENT_TIMEOUT,
     )
@@ -844,16 +847,14 @@ def validate_name_with_edx(name):
         name (str): The full name
 
     Raises:
-        EdxApiRegistrationValidationException: Raised if response status is not OK.
+        EdxApiRegistrationValidationException: Raised if response status is not 200.
     """
     edx_client = get_edx_api_registration_client()
     try:
-        resp = edx_client.user_info.validate_user_registration(
-            data=dict(  # noqa: C408
+        return edx_client.user_validation.validate_user_registration(
+            registration_information=dict(  # noqa: C408
                 name=name,
             )
-        )
-    except Exception as exc:  # noqa: BLE001
+        ).name
+    except Exception as exc:
         raise EdxApiRegistrationValidationException(name, exc.response) from exc
-
-    return resp["validation_decisions"]["name"]

courseware/api_test.py

@@ -112,24 +112,17 @@ def update_token_response_error(settings):
     return SimpleNamespace(refresh_token=refresh_token, access_token=access_token)
 
 
-def test_validate_name_with_edx_success(mocker):
+def test_validate_name_with_edx_success(mock_validate_user_registration):
     """
     Test that validate_name_with_edx successfully returns the validation message
     from edX API when the name is valid.
     """
     name = "Test User"
 
-    mock_client = mocker.MagicMock()
-    mock_client.user_info.validate_user_registration.return_value = {"validation_decisions": {"name": ""}}
-    mocker.patch(
-        "courseware.api.get_edx_api_registration_client",
-        return_value=mock_client
-    )
-
     result = validate_name_with_edx(name)
     assert result == ""
-    mock_client.user_info.validate_user_registration.assert_called_once_with(
-        data={"name": name}
+    mock_validate_user_registration.user_validation.validate_user_registration.assert_called_once_with(
+        registration_information={"name": name}
     )
 
 
@@ -142,27 +135,33 @@ def test_validate_name_with_edx_failure(mocker):
 
     class MockApiException(Exception):  # noqa: N818
         """Mock exception for API errors with a response attribute."""
+
         def __init__(self, message, response):
             super().__init__(message)
             self.response = response
 
     mock_response = mocker.MagicMock()
     mock_response.status_code = 403
     mock_response.headers = {"Content-Type": "application/json"}
+    mock_response.text = "Some error details"
 
     mock_client = mocker.MagicMock()
-    mock_client.user_info.validate_user_registration.side_effect = MockApiException(
-        "API error", response=mock_response
+    mock_client.user_validation.validate_user_registration.side_effect = (
+        MockApiException("API error", response=mock_response)
     )
     mocker.patch(
-        "courseware.api.get_edx_api_registration_client",
-        return_value=mock_client
+        "courseware.api.get_edx_api_registration_client", return_value=mock_client
     )
 
-    with pytest.raises(EdxApiRegistrationValidationException):
+    with pytest.raises(EdxApiRegistrationValidationException) as exc_info:
         validate_name_with_edx(name)
-    mock_client.user_info.validate_user_registration.assert_called_once_with(
-        data={"name": name}
+    mock_client.user_validation.validate_user_registration.assert_called_once_with(
+        registration_information={"name": name}
+    )
+    assert (
+        str(exc_info.value)
+        == f"EdX API error validating registration name {name}.\nResponse - code: {mock_response.status_code}, "
+        f"content: {mock_response.text}"
     )
 
 

courseware/exceptions.py

@@ -68,7 +68,7 @@ class UserNameUpdateFailedException(Exception):  # noqa: N818
 class EdxApiRegistrationValidationException(Exception):  # noqa: N818
     """An edX Registration Validation API call resulted in an error response"""
 
-    def __init__(self, name, response, msg=None):
+    def __init__(self, name, error_response, msg=None):
         """
         Sets exception properties and adds a default message
 
@@ -77,7 +77,7 @@ def __init__(self, name, response, msg=None):
             response (requests.Response): edX response for name validation
         """
         self.name = name
-        self.response = response
+        self.response = error_response
         if msg is None:
             # Set some default useful error message
             msg = f"EdX API error validating registration name {self.name}.\n{get_error_response_summary(self.response)}"

fixtures/common.py

@@ -110,3 +110,18 @@ def valid_address_dict():
 def nplusone_fail(settings):  # noqa: PT004
     """Configures the nplusone app to raise errors"""
     settings.NPLUSONE_RAISE = True
+
+
+@pytest.fixture
+def mock_validate_user_registration(mocker):
+    """Fixture to mock validate_user_registration method."""
+    mock_response = mocker.MagicMock()
+    mock_response.name = ""
+
+    mock_client = mocker.MagicMock()
+    mock_client.user_validation.validate_user_registration.return_value = mock_response
+    mocker.patch(
+        "courseware.api.get_edx_api_registration_client", return_value=mock_client
+    )
+
+    return mock_client

static/js/containers/pages/profile/EditProfilePage.js

@@ -87,11 +87,8 @@ export class EditProfilePage extends React.Component<Props, State> {
         /* eslint-disable camelcase */
         body: { errors },
       }: { body: Object } = await editProfile(payload);
-
-      if (errors) {
-        const nonFieldErrors = errors.non_field_errors || [];
-        const fieldErrors = { ...errors };
-        delete fieldErrors.non_field_errors;
+      if (errors && errors.non_field_errors) {
+        const nonFieldErrors = errors.non_field_errors;
 
         nonFieldErrors.forEach((error) => {
           addUserNotification({
@@ -104,8 +101,10 @@ export class EditProfilePage extends React.Component<Props, State> {
             },
           });
         });
-
-        setErrors(fieldErrors);
+      } else if (errors && errors.length > 0) {
+        setErrors({
+          email: errors[0],
+        });
       } else {
         history.push(routes.profile.view);
       }

static/js/containers/pages/register/RegisterDetailsPage.js

@@ -21,7 +21,6 @@ import {
   STATE_ERROR,
   STATE_EXISTING_ACCOUNT,
   handleAuthResponse,
-  STATE_REGISTER_DETAILS,
 } from "../../../lib/auth";
 import queries from "../../../lib/queries";
 import { qsPartialTokenSelector } from "../../../lib/selectors";
@@ -109,13 +108,6 @@ export class RegisterDetailsPage extends React.Component<Props, State> {
         /* eslint-disable camelcase */
         [STATE_ERROR]: ({ field_errors }: AuthResponse) =>
           setErrors(field_errors),
-        [STATE_REGISTER_DETAILS]: ({ field_errors }: AuthResponse) => {
-          // Validation failures will result in a 200 API response that still points to this page but contains
-          // field errors.
-          if (field_errors) {
-            setErrors(field_errors);
-          }
-        },
       });
     } finally {
       setSubmitting(false);

static/js/containers/pages/register/RegisterDetailsPage_test.js

@@ -13,7 +13,6 @@ import {
   STATE_ERROR_TEMPORARY,
   FLOW_REGISTER,
   STATE_EXISTING_ACCOUNT,
-  STATE_REGISTER_DETAILS,
 } from "../../../lib/auth";
 import { routes } from "../../../lib/urls";
 import { makeRegisterAuthResponse } from "../../../factories/auth";
@@ -170,31 +169,4 @@ describe("RegisterDetailsPage", () => {
       "You already have an xPRO account. Please .",
     );
   });
-
-  it("shows field errors from the auth response if they exist", async () => {
-    const { inner } = await renderPage();
-
-    helper.handleRequestStub.returns({
-      body: makeRegisterAuthResponse({
-        state: STATE_REGISTER_DETAILS,
-        field_errors: { name: "Invalid" },
-        partial_token: "new_partial_token",
-      }),
-    });
-    const onSubmit = inner.find("RegisterDetailsForm").prop("onSubmit");
-
-    await onSubmit(detailsData, {
-      setSubmitting: setSubmittingStub,
-      setErrors: setErrorsStub,
-    });
-
-    sinon.assert.calledWith(
-      helper.handleRequestStub,
-      "/api/register/details/",
-      "POST",
-      { body, headers: undefined, credentials: undefined },
-    );
-    sinon.assert.calledOnce(setErrorsStub);
-    sinon.assert.calledWith(setErrorsStub, { name: "Invalid" });
-  });
 });

users/serializers.py

@@ -34,11 +34,6 @@
     """,
     flags=re.I | re.VERBOSE | re.MULTILINE,
 )
-NAME_CONTAINS_HTML_URL_MSG = (
-    "Full name can not contain HTML or URL. Please try a different one."
-)
-
-OPENEDX_NAME_VALIDATION_MSGS_MAP = {"Enter a valid name": NAME_CONTAINS_HTML_URL_MSG}
 
 
 class LegalAddressSerializer(serializers.ModelSerializer):
@@ -278,14 +273,11 @@ def get_unused_coupons(self, instance):
         return []
 
     def validate(self, data):
-        """Validate an existing user"""
+        """Validate user data"""
         name = data.get("name")
         if name:
             try:
                 openedx_validation_msg = validate_name_with_edx(name)
-                openedx_validation_msg = OPENEDX_NAME_VALIDATION_MSGS_MAP.get(
-                    openedx_validation_msg, openedx_validation_msg
-                )
             except (
                 HTTPError,
                 RequestsConnectionError,
@@ -295,7 +287,7 @@ def validate(self, data):
                 raise serializers.ValidationError(USER_REGISTRATION_FAILED_MSG)  # noqa: B904
 
             if openedx_validation_msg:
-                raise serializers.ValidationError({"name": openedx_validation_msg})
+                raise serializers.ValidationError(USER_REGISTRATION_FAILED_MSG)
 
         return data