Rest of PATCH implementation.

openedx/core/djangoapps/user_api/accounts/tests/test_views.py

@@ -14,17 +14,6 @@ TEST_PASSWORD = "test"
 @ddt.ddt
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
 class TestAccountAPI(APITestCase):
-    USERNAME = "Christina"
-    EMAIL = "christina@example.com"
-    PASSWORD = TEST_PASSWORD
-
-    BAD_USERNAME = "Bad"
-    BAD_EMAIL = "bad@example.com"
-    BAD_PASSWORD = TEST_PASSWORD
-
-    STAFF_USERNAME = "Staff"
-    STAFF_EMAIL = "staff@example.com"
-    STAFF_PASSWORD = TEST_PASSWORD
 
     def setUp(self):
         super(TestAccountAPI, self).setUp()
@@ -45,7 +34,6 @@ class TestAccountAPI(APITestCase):
         legacy_profile.year_of_birth = 1900
         legacy_profile.level_of_education = "m"
         legacy_profile.goals = "world peace"
-        legacy_profile.mailing_address = "North Pole"
         legacy_profile.save()
         
         self.accounts_base_uri = reverse("accounts_api", kwargs={'username': self.user.username})
@@ -82,21 +70,61 @@ class TestAccountAPI(APITestCase):
         self.assertEqual(1900, data["year_of_birth"])
         self.assertEqual("m", data["level_of_education"])
         self.assertEqual("world peace", data["goals"])
-        self.assertEqual("North Pole", data['mailing_address'])
+        # Default value for mailing address is None, nothing assigned in setup.
+        self.assertIsNone(None, data['mailing_address'])
         self.assertEqual(self.user.email, data["email"])
         self.assertIsNotNone(data["date_joined"])
 
     @ddt.data(
-        ("client", "user"),
-        ("staff_client", "staff_user"),
+        (
+            "client", "user", "gender", "f", "not a gender",
+            "Select a valid choice. not a gender is not one of the available choices."
+        ),
+        (
+            "client", "user", "level_of_education", "none", "x",
+            "Select a valid choice. x is not one of the available choices."
+        ),
+        ("client", "user", "country", "GB", "UK", "Select a valid choice. UK is not one of the available choices."),
+        ("client", "user", "year_of_birth", 2009, "not_an_int", "Enter a whole number."),
+        ("client", "user", "city", "Knoxville"),
+        ("client", "user", "language", "Creole"),
+        ("client", "user", "goals", "Smell the roses"),
+        ("client", "user", "mailing_address", "Sesame Street"),
+        # All of the fields can be edited by is_staff, but iterating through all of them again seems like overkill.
+        # Just test a representative field.
+        ("staff_client", "staff_user", "goals", "Smell the roses"),
     )
     @ddt.unpack
-    def test_patch_account(self, api_client, user):
+    def test_patch_account(
+            self, api_client, user, field, value, fails_validation_value=None, developer_validation_message=None
+    ):
         client = self.login_client(api_client, user)
-        response = client.patch(self.accounts_base_uri, data={"gender": "f"})
-        self.assert_status_code(200, response)
-        data = response.data
-        self.assertEqual("f", data["gender"])
+        patch_response = client.patch(self.accounts_base_uri, data={field: value})
+        self.assert_status_code(204, patch_response)
+
+        get_response = client.get(self.accounts_base_uri)
+        self.assert_status_code(200, get_response)
+        self.assertEqual(value, get_response.data[field])
+
+        if fails_validation_value:
+            error_response = client.patch(self.accounts_base_uri, data={field: fails_validation_value})
+            self.assert_status_code(400, error_response)
+            self.assertEqual(
+                "Value '{0}' is not valid for field '{1}'.".format(fails_validation_value, field),
+                error_response.data["field_errors"][field]["user_message"]
+            )
+            self.assertEqual(
+                developer_validation_message,
+                error_response.data["field_errors"][field]["developer_message"]
+            )
+        else:
+            # If there are no values that would fail validation, then empty string should be supported.
+            patch_response = client.patch(self.accounts_base_uri, data={field: ""})
+            self.assert_status_code(204, patch_response)
+
+            get_response = client.get(self.accounts_base_uri)
+            self.assert_status_code(200, get_response)
+            self.assertEqual("", get_response.data[field])
 
     @ddt.data(
         ("client", "user"),
@@ -106,20 +134,29 @@ class TestAccountAPI(APITestCase):
     def test_patch_account_noneditable(self, api_client, user):
         client = self.login_client(api_client, user)
 
+        def verify_error_response(field_name, data):
+            self.assertEqual(
+                "This field is not editable via this API", data["field_errors"][field_name]["developer_message"]
+            )
+            self.assertEqual(
+                "Field '{0}' cannot be edited.".format(field_name), data["field_errors"][field_name]["user_message"]
+            )
+
         for field_name in ["username", "email", "date_joined", "name"]:
-            response = client.patch(self.accounts_base_uri, data={field_name: "willbeignored", "gender": "f"})
+            response = client.patch(self.accounts_base_uri, data={field_name: "will_error", "gender": "f"})
             self.assert_status_code(400, response)
-            data = response.data
-            self.assertEqual("The following fields are not editable: " + field_name, data["message"])
+            verify_error_response(field_name, response.data)
 
         # Make sure that gender did not change.
         response = client.get(self.accounts_base_uri)
         self.assertEqual("m", response.data["gender"])
 
         # Test error message with multiple read-only items
-        response = client.patch(self.accounts_base_uri, data={"username": "willbeignored", "email": "xx"})
+        response = client.patch(self.accounts_base_uri, data={"username": "will_error", "email": "xx"})
         self.assert_status_code(400, response)
-        self.assertEqual("The following fields are not editable: username, email", response.data["message"])
+        self.assertEqual(2, len(response.data["field_errors"]))
+        verify_error_response("username", response.data)
+        verify_error_response("email", response.data)
 
     def assert_status_code(self, expected_status_code, response):
         """Assert that the given response has the expected status code"""

openedx/core/djangoapps/user_api/accounts/views.py

-from rest_framework.views import APIView
 from django.core.exceptions import ObjectDoesNotExist
 from django.contrib.auth.models import User
+from django.utils.translation import ugettext as _
+
+from rest_framework.views import APIView
 from rest_framework.response import Response
 from rest_framework import status
+from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
+from rest_framework import permissions
 
 from student.models import UserProfile
 from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer, AccountUserSerializer
 from openedx.core.lib.api.permissions import IsUserInUrlOrStaff
 
-from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
-from rest_framework import permissions
-from rest_framework import status
-
 
 class AccountView(APIView):
     """
         **Use Cases**
 
-            Get the user's account information.
+            Get or update the user's account information.
 
         **Example Requests**:
 
             GET /api/user/v0/accounts/{username}/
 
-        **Response Values**
+            PATCH /api/user/v0/accounts/{username}/
+
+        **Response Values for GET**
 
             * username: The username associated with the account (not editable).
 
@@ -59,6 +61,10 @@ class AccountView(APIView):
 
              * goals: null or textual representation of goals
 
+        **Response for PATCH**
+
+             Returns a 204 status if successful, with no additional content.
+
     """
     authentication_classes = (OAuth2Authentication, SessionAuthentication)
     permission_classes = (permissions.IsAuthenticated, IsUserInUrlOrStaff)
@@ -85,19 +91,25 @@ class AccountView(APIView):
             AccountUserSerializer.Meta.read_only_fields + AccountLegacyProfileSerializer.Meta.read_only_fields
         )
         if read_only_fields:
-            response_data = {}
-            response_data['message'] = "The following fields are not editable: " + ", ".join(str(e) for e in read_only_fields)
+            field_errors = {}
+            for read_only_field in read_only_fields:
+                field_errors[read_only_field] = {
+                    "developer_message": "This field is not editable via this API",
+                    "user_message": _("Field '{field_name}' cannot be edited.".format(field_name=read_only_field))
+                }
+            response_data = {"field_errors": field_errors}
             return Response(response_data, status=status.HTTP_400_BAD_REQUEST)
 
         user_serializer = AccountUserSerializer(existing_user, data=update)
-        user_serializer.is_valid()
-        user_serializer.save()
-
         legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile, data=update)
-        legacy_profile_serializer.is_valid()
-        legacy_profile_serializer.save()
 
-        return Response(dict(user_serializer.data, **legacy_profile_serializer.data))
+        for serializer in user_serializer, legacy_profile_serializer:
+            validation_errors = self._get_validation_errors(update, serializer)
+            if validation_errors:
+                return Response(validation_errors, status=status.HTTP_400_BAD_REQUEST)
+            serializer.save()
+
+        return Response(status=status.HTTP_204_NO_CONTENT)
 
     def _get_user_and_profile(self, username):
         """
@@ -110,3 +122,26 @@ class AccountView(APIView):
         existing_user_profile = UserProfile.objects.get(id=existing_user.id)
 
         return existing_user, existing_user_profile
+
+    def _get_validation_errors(self, update, serializer):
+        """
+        Helper method that returns any validation errors that are present.
+        """
+        validation_errors = {}
+        if not serializer.is_valid():
+            field_errors = {}
+            errors = serializer.errors
+            for key, value in errors.iteritems():
+                if isinstance(value, list) and len(value) > 0:
+                    developer_message = value[0]
+                else:
+                    developer_message = "Invalid value: {field_value}'".format(field_value=update[key])
+                field_errors[key] = {
+                    "developer_message": developer_message,
+                    "user_message": _("Value '{field_value}' is not valid for field '{field_name}'.".format(
+                        field_value=update[key], field_name=key)
+                    )
+                }
+
+            validation_errors['field_errors'] = field_errors
+        return validation_errors
\ No newline at end of file