Switch to api directory and improve error handling.

common/djangoapps/student/views.py

@@ -1909,7 +1909,8 @@ def reactivation_email_for_user(user):
     return JsonResponse({"success": True})
 
 
-# TODO: delete this method and redirect unit tests to do_email_change_request after accounts page work is done.
+# TODO: delete this method and redirect unit tests to validate_new_email and do_email_change_request
+# after accounts page work is done.
 @ensure_csrf_cookie
 def change_email_request(request):
     """ AJAX call from the profile page. User wants a new e-mail.
@@ -1928,6 +1929,7 @@ def change_email_request(request):
 
     new_email = request.POST['new_email']
     try:
+        validate_new_email(request.user, new_email)
         do_email_change_request(request.user, new_email)
     except ValueError as err:
         return JsonResponse({
@@ -1937,11 +1939,10 @@ def change_email_request(request):
     return JsonResponse({"success": True})
 
 
-def do_email_change_request(user, new_email, activation_key=uuid.uuid4().hex):
+def validate_new_email(user, new_email):
     """
-    Given a new email for a user, does some basic verification of the new address and sends an activation message
-    to the new address. If any issues are encountered with verification or sending the message, a ValueError will
-    be thrown.
+    Given a new email for a user, does some basic verification of the new address If any issues are encountered
+    with verification a ValueError will be thrown.
     """
     try:
         validate_email(new_email)
@@ -1954,6 +1955,13 @@ def do_email_change_request(user, new_email, activation_key=uuid.uuid4().hex):
     if User.objects.filter(email=new_email).count() != 0:
         raise ValueError(_('An account with this e-mail already exists.'))
 
+
+def do_email_change_request(user, new_email, activation_key=uuid.uuid4().hex):
+    """
+    Given a new email for a user, does some basic verification of the new address and sends an activation message
+    to the new address. If any issues are encountered with verification or sending the message, a ValueError will
+    be thrown.
+    """
     pec_list = PendingEmailChange.objects.filter(user=user)
     if len(pec_list) == 0:
         pec = PendingEmailChange()

lms/djangoapps/verify_student/tests/test_views.py

@@ -19,7 +19,7 @@ from django.core.exceptions import ObjectDoesNotExist
 from django.core import mail
 from bs4 import BeautifulSoup
 
-from openedx.core.djangoapps.user_api.accounts.views import AccountView
+from openedx.core.djangoapps.user_api.accounts.api import get_account_settings
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.django import modulestore
@@ -1149,7 +1149,7 @@ class TestSubmitPhotosForVerification(TestCase):
             AssertionError
 
         """
-        account_settings = AccountView.get_serialized_account(self.user)
+        account_settings = get_account_settings(self.user)
         self.assertEqual(account_settings['name'], full_name)
 
 

lms/djangoapps/verify_student/views.py

@@ -27,9 +27,9 @@ from django.utils.translation import ugettext as _, ugettext_lazy
 from django.contrib.auth.decorators import login_required
 from django.core.mail import send_mail
 
-from openedx.core.djangoapps.user_api.accounts.views import AccountView
+from openedx.core.djangoapps.user_api.accounts.api import get_account_settings, update_account_settings
 from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH
-from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountUpdateError
+from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountValidationError
 
 from course_modes.models import CourseMode
 from student.models import CourseEnrollment
@@ -718,10 +718,10 @@ def submit_photos_for_verification(request):
     # then try to do that before creating the attempt.
     if request.POST.get('full_name'):
         try:
-            AccountView.update_account(request.user, {"name": request.POST.get('full_name')})
+            update_account_settings(request.user, {"name": request.POST.get('full_name')})
         except AccountUserNotFound:
             return HttpResponseBadRequest(_("No profile found for user"))
-        except AccountUpdateError:
+        except AccountValidationError:
             msg = _(
                 "Name must be at least {min_length} characters long."
             ).format(min_length=NAME_MIN_LENGTH)
@@ -741,7 +741,7 @@ def submit_photos_for_verification(request):
     attempt.mark_ready()
     attempt.submit()
 
-    account_settings = AccountView.get_serialized_account(request.user)
+    account_settings = get_account_settings(request.user)
 
     # Send a confirmation email to the user
     context = {

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

@@ -0,0 +1,231 @@
+from django.contrib.auth.models import User
+from django.utils.translation import ugettext as _
+import datetime
+from pytz import UTC
+from django.core.exceptions import ObjectDoesNotExist
+from django.conf import settings
+
+from openedx.core.djangoapps.user_api.api.account import (
+    AccountUserNotFound, AccountUpdateError, AccountNotAuthorized, AccountValidationError
+)
+from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer
+from student.models import UserProfile
+from student.views import validate_new_email, do_email_change_request
+from ..models import UserPreference
+from . import ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY
+
+
+def get_account_settings(requesting_user, username=None, configuration=None, view=None):
+    """Returns account information for a user serialized as JSON.
+
+    Note:
+        If `requesting_user.username` != `username`, this method will return differing amounts of information
+        based on who `requesting_user` is and the privacy settings of the user associated with `username`.
+
+    Args:
+        requesting_user (User): The user requesting the account information. Only the user with username
+            `username` or users with "is_staff" privileges can get full account information.
+            Other users will get the account fields that the user has elected to share.
+        username (str): Optional username for the desired account information. If not specified,
+            `requesting_user.username` is assumed.
+        configuration (dict): an optional configuration specifying which fields in the account
+            can be shared, and the default visibility settings. If not present, the setting value with
+            key ACCOUNT_VISIBILITY_CONFIGURATION is used.
+        view (str): An optional string allowing "is_staff" users and users requesting their own
+            account information to get just the fields that are shared with everyone. If view is
+            "shared", only shared account information will be returned, regardless of `requesting_user`.
+
+    Returns:
+         A dict containing account fields.
+
+    Raises:
+         AccountUserNotFound: no user with username `username` exists (or `requesting_user.username` if
+            `username` is not specified)
+    """
+    if username is None:
+        username = requesting_user.username
+
+    has_full_access = requesting_user.username == username or requesting_user.is_staff
+    return_all_fields = has_full_access and view != 'shared'
+
+    existing_user, existing_user_profile = _get_user_and_profile(username)
+
+    user_serializer = AccountUserSerializer(existing_user)
+    legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile)
+
+    account_settings = dict(user_serializer.data, **legacy_profile_serializer.data)
+
+    if return_all_fields:
+        return account_settings
+
+    if not configuration:
+        configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION
+
+    visible_settings = {}
+
+    profile_privacy = UserPreference.get_preference(existing_user, ACCOUNT_VISIBILITY_PREF_KEY)
+    privacy_setting = profile_privacy if profile_privacy else configuration.get('default_visibility')
+
+    if privacy_setting == ALL_USERS_VISIBILITY:
+        field_names = configuration.get('shareable_fields')
+    else:
+        field_names = configuration.get('public_fields')
+
+    for field_name in field_names:
+        visible_settings[field_name] = account_settings.get(field_name, None)
+
+    return visible_settings
+
+
+def update_account_settings(requesting_user, update, username=None):
+    """Update user account information.
+
+    Note:
+        It is up to the caller of this method to enforce the contract that this method is only called
+        with the user who made the request.
+
+    Arguments:
+        requesting_user (User): The user requesting to modify account information. Only the user with username
+            'username' has permissions to modify account information.
+        update (dict): The updated account field values.
+        username (string): Optional username specifying which account should be updated. If not specified,
+            `requesting_user.username` is assumed.
+
+    Raises:
+        AccountUserNotFound: no user with username `username` exists (or `requesting_user.username` if
+            `username` is not specified)
+        AccountNotAuthorized: the requesting_user does not have access to change the account
+            associated with `username`
+        AccountValidationError: the update was not attempted because validation errors were found with
+            the supplied update
+        AccountUpdateError: the update could not be completed. Note that if multiple fields are updated at the same
+            time, some parts of the update may have been successful, even if an AccountUpdateError is returned;
+            in particular, the user account (not including e-mail address) may have successfully been updated,
+            but then the e-mail change request, which is processed last, may throw an error.
+
+    """
+    if username is None:
+        username = requesting_user.username
+
+    existing_user, existing_user_profile = _get_user_and_profile(username)
+
+    if requesting_user.username != username:
+        raise AccountNotAuthorized()
+
+    # If user has requested to change email, we must call the multi-step process to handle this.
+    # It is not handled by the serializer (which considers email to be read-only).
+    new_email = None
+    if "email" in update:
+        new_email = update["email"]
+        del update["email"]
+
+    # If user has requested to change name, store old name because we must update associated metadata
+    # after the save process is complete.
+    old_name = None
+    if "name" in update:
+        old_name = existing_user_profile.name
+
+    # Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
+    read_only_fields = set(update.keys()).intersection(
+        AccountUserSerializer.Meta.read_only_fields + AccountLegacyProfileSerializer.Meta.read_only_fields
+    )
+
+    # Build up all field errors, whether read-only, validation, or email errors.
+    field_errors = {}
+
+    if read_only_fields:
+        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))
+            }
+            del update[read_only_field]
+
+    user_serializer = AccountUserSerializer(existing_user, data=update)
+    legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile, data=update)
+
+    for serializer in user_serializer, legacy_profile_serializer:
+        field_errors = _add_serializer_errors(update, serializer, field_errors)
+
+    # If the user asked to change email, validate it.
+    if new_email:
+        try:
+            validate_new_email(existing_user, new_email)
+        except ValueError as err:
+            field_errors["email"] = {
+                "developer_message": "Error thrown from validate_new_email: '{}'".format(err.message),
+                "user_message": err.message
+            }
+
+    # If we have encountered any validation errors, return them to the user.
+    if field_errors:
+        raise AccountValidationError(field_errors)
+
+    try:
+        # If everything validated, go ahead and save the serializers.
+        for serializer in user_serializer, legacy_profile_serializer:
+            serializer.save()
+
+        # If the name was changed, store information about the change operation. This is outside of the
+        # serializer so that we can store who requested the change.
+        if old_name:
+            meta = existing_user_profile.get_meta()
+            if 'old_names' not in meta:
+                meta['old_names'] = []
+            meta['old_names'].append([
+                old_name,
+                "Name change requested through account API by {0}".format(requesting_user.username),
+                datetime.datetime.now(UTC).isoformat()
+            ])
+            existing_user_profile.set_meta(meta)
+            existing_user_profile.save()
+
+    except Exception as err:
+        raise AccountUpdateError(
+            "Error thrown when saving account updates: '{}'".format(err.message)
+        )
+
+    # And try to send the email change request if necessary.
+    if new_email:
+        try:
+            do_email_change_request(existing_user, new_email)
+        except ValueError as err:
+            raise AccountUpdateError(
+                "Error thrown from do_email_change_request: '{}'".format(err.message),
+                user_message=err.message
+            )
+
+
+def _get_user_and_profile(username):
+    """
+    Helper method to return the legacy user and profile objects based on username.
+    """
+    try:
+        existing_user = User.objects.get(username=username)
+        existing_user_profile = UserProfile.objects.get(user=existing_user)
+    except ObjectDoesNotExist:
+        raise AccountUserNotFound()
+
+    return existing_user, existing_user_profile
+
+
+def _add_serializer_errors(update, serializer, field_errors):
+    """
+    Helper method that adds any validation errors that are present in the serializer to
+    the supplied field_errors dict.
+    """
+    if not serializer.is_valid():
+        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)
+                )
+            }
+
+    return field_errors

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

@@ -0,0 +1,177 @@
+# -*- coding: utf-8 -*-
+"""
+Unit tests for behavior that is specific to the api methods (vs. the view methods).
+Most of the functionality is covered in test_views.py.
+"""
+
+from mock import Mock, patch
+from django.test import TestCase
+import unittest
+from student.tests.factories import UserFactory
+from django.conf import settings
+from student.models import PendingEmailChange
+from openedx.core.djangoapps.user_api.api.account import (
+    AccountUserNotFound, AccountUpdateError, AccountNotAuthorized, AccountValidationError
+)
+from ..api import get_account_settings, update_account_settings
+from ..serializers import AccountUserSerializer
+
+
+def mock_render_to_string(template_name, context):
+    """Return a string that encodes template_name and context"""
+    return str((template_name, sorted(context.iteritems())))
+
+
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
+class TestAccountApi(TestCase):
+    """
+    These tests specifically cover the parts of the API methods that are not covered by test_views.py.
+    This includes the specific types of error raised, and default behavior when optional arguments
+    are not specified.
+    """
+    password = "test"
+
+    def setUp(self):
+        super(TestAccountApi, self).setUp()
+        self.user = UserFactory.create(password=self.password)
+        self.different_user = UserFactory.create(password=self.password)
+        self.staff_user = UserFactory(is_staff=True, password=self.password)
+
+    def test_get_username_provided(self):
+        """Test the difference in behavior when a username is supplied to get_account_settings."""
+        account_settings = get_account_settings(self.user)
+        self.assertEqual(self.user.username, account_settings["username"])
+
+        account_settings = get_account_settings(self.user, username=self.user.username)
+        self.assertEqual(self.user.username, account_settings["username"])
+
+        account_settings = get_account_settings(self.user, username=self.different_user.username)
+        self.assertEqual(self.different_user.username, account_settings["username"])
+
+    def test_get_configuration_provided(self):
+        """Test the difference in behavior when a configuration is supplied to get_account_settings."""
+        config = {
+            "default_visibility": "private",
+
+            "shareable_fields": [
+                'name',
+            ],
+
+            "public_fields": [
+                'email',
+            ],
+        }
+
+        # With default configuration settings, email is not shared with other (non-staff) users.
+        account_settings = get_account_settings(self.user, self.different_user.username)
+        self.assertFalse("email" in account_settings)
+
+        account_settings = get_account_settings(self.user, self.different_user.username, configuration=config)
+        self.assertEqual(self.different_user.email, account_settings["email"])
+
+    def test_get_user_not_found(self):
+        """Test that AccountUserNotFound is thrown if there is no user with username."""
+        with self.assertRaises(AccountUserNotFound):
+            get_account_settings(self.user, username="does_not_exist")
+
+        self.user.username = "does_not_exist"
+        with self.assertRaises(AccountUserNotFound):
+            get_account_settings(self.user)
+
+    def test_update_username_provided(self):
+        """Test the difference in behavior when a username is supplied to update_account_settings."""
+        update_account_settings(self.user, {"name": "Mickey Mouse"})
+        account_settings = get_account_settings(self.user)
+        self.assertEqual("Mickey Mouse", account_settings["name"])
+
+        update_account_settings(self.user, {"name": "Donald Duck"}, username=self.user.username)
+        account_settings = get_account_settings(self.user)
+        self.assertEqual("Donald Duck", account_settings["name"])
+
+        with self.assertRaises(AccountNotAuthorized):
+            update_account_settings(self.different_user, {"name": "Pluto"}, username=self.user.username)
+
+    def test_update_user_not_found(self):
+        """Test that AccountUserNotFound is thrown if there is no user with username."""
+        with self.assertRaises(AccountUserNotFound):
+            update_account_settings(self.user, {}, username="does_not_exist")
+
+        self.user.username = "does_not_exist"
+        with self.assertRaises(AccountUserNotFound):
+            update_account_settings(self.user, {})
+
+    def test_update_error_validating(self):
+        """Test that AccountValidationError is thrown if incorrect values are supplied."""
+        with self.assertRaises(AccountValidationError):
+            update_account_settings(self.user, {"username": "not_allowed"})
+
+        with self.assertRaises(AccountValidationError):
+            update_account_settings(self.user, {"gender": "undecided"})
+
+    def test_update_multiple_validation_errors(self):
+        """Test that all validation errors are built up and returned at once"""
+        # Send a read-only error, serializer error, and email validation error.
+        naughty_update = {
+            "username": "not_allowed",
+            "gender": "undecided",
+            "email": "not an email address"
+        }
+
+        error_thrown = False
+        try:
+            update_account_settings(self.user, naughty_update)
+        except AccountValidationError as response:
+            error_thrown = True
+            field_errors = response.field_errors
+            self.assertEqual(3, len(field_errors))
+            self.assertEqual("This field is not editable via this API", field_errors["username"]["developer_message"])
+            self.assertIn("Select a valid choice", field_errors["gender"]["developer_message"])
+            self.assertIn("Valid e-mail address required.", field_errors["email"]["developer_message"])
+
+        self.assertTrue(error_thrown, "No AccountValidationError was thrown")
+
+    @patch('django.core.mail.send_mail')
+    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
+    def test_update_sending_email_fails(self, send_mail):
+        """Test what happens if all validation checks pass, but sending the email for email change fails."""
+        send_mail.side_effect = [Exception, None]
+        less_naughty_update = {
+            "name": "Mickey Mouse",
+            "email": "seems_ok@sample.com"
+        }
+        error_thrown = False
+        try:
+            update_account_settings(self.user, less_naughty_update)
+        except AccountUpdateError as response:
+            error_thrown = True
+            self.assertIn("Error thrown from do_email_change_request", response.developer_message)
+
+        self.assertTrue(error_thrown, "No AccountUpdateError was thrown")
+
+        # Verify that the name change happened, even though the attempt to send the email failed.
+        account_settings = get_account_settings(self.user)
+        self.assertEqual("Mickey Mouse", account_settings["name"])
+
+    @patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save')
+    def test_serializer_save_fails(self, serializer_save):
+        """
+        Test the behavior of one of the serializers failing to save. Note that email request change
+        won't be processed in this case.
+        """
+        serializer_save.side_effect = [Exception, None]
+        update_will_fail = {
+            "name": "Mickey Mouse",
+            "email": "ok@sample.com"
+        }
+        error_thrown = False
+        try:
+            update_account_settings(self.user, update_will_fail)
+        except AccountUpdateError as response:
+            error_thrown = True
+            self.assertIn("Error thrown when saving account updates", response.developer_message)
+
+        self.assertTrue(error_thrown, "No AccountUpdateError was thrown")
+
+        # Verify that no email change request was initiated.
+        pending_change = PendingEmailChange.objects.filter(user=self.user)
+        self.assertEqual(0, len(pending_change))

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

@@ -440,8 +440,23 @@ class TestAccountAPI(UserAPITestCase):
 
         # Finally, try changing to an invalid email just to make sure error messages are appropriately returned.
         error_response = self.send_patch(client, {"email": "not_an_email"}, expected_status=400)
+        field_errors = error_response.data["field_errors"]
         self.assertEqual(
-            "Error thrown from do_email_change_request: 'Valid e-mail address required.'",
+            "Error thrown from validate_new_email: 'Valid e-mail address required.'",
+            field_errors["email"]["developer_message"]
+        )
+        self.assertEqual("Valid e-mail address required.", field_errors["email"]["user_message"])
+
+    @patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save')
+    def test_patch_serializer_save_fails(self, serializer_save):
+        """
+        Test that AccountUpdateErrors are passed through to the response.
+        """
+        serializer_save.side_effect = [Exception("bummer"), None]
+        self.client.login(username=self.user.username, password=TEST_PASSWORD)
+        error_response = self.send_patch(self.client, {"goals": "save an account field"}, expected_status=400)
+        self.assertEqual(
+            "Error thrown when saving account updates: 'bummer'",
             error_response.data["developer_message"]
         )
-        self.assertEqual("Valid e-mail address required.", error_response.data["user_message"])
+        self.assertIsNone(error_response.data["user_message"])

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

@@ -4,28 +4,17 @@ NOTE: this API is WIP and has not yet been approved. Do not use this API without
 For more information, see:
 https://openedx.atlassian.net/wiki/display/TNL/User+API
 """
-from django.core.exceptions import ObjectDoesNotExist
-from django.contrib.auth.models import User
-from django.utils.translation import ugettext as _
-from django.conf import settings
-import datetime
-from pytz import UTC
-
 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 openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer, AccountUserSerializer
-from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountUpdateError, AccountNotAuthorized
+from openedx.core.djangoapps.user_api.api.account import (
+    AccountUserNotFound, AccountUpdateError, AccountNotAuthorized, AccountValidationError
+)
 from openedx.core.lib.api.parsers import MergePatchParser
-from openedx.core.lib.api.permissions import IsUserInUrlOrStaff
-from student.models import UserProfile
-from student.views import do_email_change_request
-from ..models import UserPreference
-
-from . import ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY
+from .api import get_account_settings, update_account_settings
 
 
 class AccountView(APIView):
@@ -36,51 +25,75 @@ class AccountView(APIView):
 
         **Example Requests**:
 
-            GET /api/user/v0/accounts/{username}/
+            GET /api/user/v0/accounts/{username}/[?view=shared]
 
             PATCH /api/user/v0/accounts/{username}/ with content_type "application/merge-patch+json"
 
         **Response Values for GET**
 
-            * username: username associated with the account (not editable)
+            If the user making the request has username "username", or has "is_staff" access, the following
+            fields will be returned:
 
-            * name: full name of the user (must be at least two characters)
+                * username: username associated with the account (not editable)
 
-            * email: email for the user (the new email address must be confirmed via a confirmation email, so GET will
-                not reflect the change until the address has been confirmed)
+                * name: full name of the user (must be at least two characters)
 
-            * date_joined: date this account was created (not editable), in the string format provided by
-                datetime (for example, "2014-08-26T17:52:11Z")
+                * email: email for the user (the new email address must be confirmed via a confirmation email, so GET will
+                    not reflect the change until the address has been confirmed)
 
-            * gender: null (not set), "m", "f", or "o"
+                * date_joined: date this account was created (not editable), in the string format provided by
+                    datetime (for example, "2014-08-26T17:52:11Z")
 
-            * year_of_birth: null or integer year
+                * gender: null (not set), "m", "f", or "o"
 
-            * level_of_education: null (not set), or one of the following choices:
+                * year_of_birth: null or integer year
 
-                * "p" signifying "Doctorate"
-                * "m" signifying "Master's or professional degree"
-                * "b" signifying "Bachelor's degree"
-                * "a" signifying "Associate's degree"
-                * "hs" signifying "Secondary/high school"
-                * "jhs" signifying "Junior secondary/junior high/middle school"
-                * "el" signifying "Elementary/primary school"
-                * "none" signifying "None"
-                * "o" signifying "Other"
+                * level_of_education: null (not set), or one of the following choices:
 
-             * language: null or name of preferred language
+                    * "p" signifying "Doctorate"
+                    * "m" signifying "Master's or professional degree"
+                    * "b" signifying "Bachelor's degree"
+                    * "a" signifying "Associate's degree"
+                    * "hs" signifying "Secondary/high school"
+                    * "jhs" signifying "Junior secondary/junior high/middle school"
+                    * "el" signifying "Elementary/primary school"
+                    * "none" signifying "None"
+                    * "o" signifying "Other"
 
-             * country: null (not set), or a Country corresponding to one of the ISO 3166-1 countries
+                * language: null or name of preferred language
 
-             * mailing_address: null or textual representation of mailing address
+                * country: null (not set), or a Country corresponding to one of the ISO 3166-1 countries
 
-             * goals: null or textual representation of goals
+                * mailing_address: null or textual representation of mailing address
+
+                * goals: null or textual representation of goals
+
+            If a user without "is_staff" access has requested account information for a different user,
+            only a subset of these fields will be returned. The actual fields returned depend on the configuration
+            setting ACCOUNT_VISIBILITY_CONFIGURATION, and the visibility preference of the user with username
+            "username".
+
+            Note that a user can view which account fields they have shared with other users by requesting their
+            own username and providing the url parameter "view=shared".
+
+            This method will return a 404 if no user exists with username "username".
 
         **Response for PATCH**
 
-             Returns a 204 status if successful, with no additional content.
-             If "application/merge-patch+json" is not the specified content_type, returns a 415 status.
+            Users can only modify their own account information. If the requesting user does not have username
+            "username", this method will return with a status of 404.
 
+            This method will also return a 404 if no user exists with username "username".
+
+            If "application/merge-patch+json" is not the specified content_type, this method returns a 415 status.
+
+            If the update could not be completed due to validation errors, this method returns a 400 with all
+            field-specific error messages in the "field_errors" field of the returned JSON.
+
+            If the update could not be completed due to failure at the time of update, this method returns a 400 with
+            specific errors in the returned JSON.
+
+            If the updated is successful, a 204 status is returned with no additional content.
     """
     authentication_classes = (OAuth2Authentication, SessionAuthentication)
     permission_classes = (permissions.IsAuthenticated,)
@@ -91,76 +104,12 @@ class AccountView(APIView):
         GET /api/user/v0/accounts/{username}/
         """
         try:
-            account_settings = AccountView.get_serialized_account(
-                request.user, username,
-                view=request.QUERY_PARAMS.get('view')
-            )
+            account_settings = get_account_settings(request.user, username, view=request.QUERY_PARAMS.get('view'))
         except AccountUserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
 
         return Response(account_settings)
 
-    @staticmethod
-    def get_serialized_account(requesting_user, username=None, configuration=None, view=None):
-        """Returns account information for a user serialized as JSON.
-
-        Note:
-            If `requesting_user.username` != `username`, this method will return differing amounts of information
-            based on who `requesting_user` is and the privacy settings of the user associated with `username`.
-
-        Args:
-            requesting_user (User): The user requesting the account information. Only the user with username
-                `username` or users with "is_staff" privileges can get full account information.
-                Other users will get the account fields that the user has elected to share.
-            username (str): Optional username for the desired account information. If not specified,
-                `requesting_user.username` is assumed.
-            configuration (dict): an optional configuration specifying which fields in the account
-                can be shared, and the default visibility settings. If not present, the setting value with
-                key ACCOUNT_VISIBILITY_CONFIGURATION is used.
-            view (str): An optional string allowing "is_staff" users and users requesting their own
-                account information to get just the fields that are shared with everyone. If view is
-                "shared", only shared account information will be returned, regardless of `requesting_user`.
-
-        Returns:
-             A dict containing account fields.
-
-        Raises:
-             AccountUserNotFound: `username` was specified, but no user exists with that username.
-        """
-        if username is None:
-            username = requesting_user.username
-
-        has_full_access = requesting_user.username == username or requesting_user.is_staff
-        return_all_fields = has_full_access and view != 'shared'
-
-        existing_user, existing_user_profile = AccountView._get_user_and_profile(username)
-
-        user_serializer = AccountUserSerializer(existing_user)
-        legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile)
-
-        account_settings = dict(user_serializer.data, **legacy_profile_serializer.data)
-
-        if return_all_fields:
-            return account_settings
-
-        if not configuration:
-            configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION
-
-        visible_settings = {}
-
-        profile_privacy = UserPreference.get_preference(existing_user, ACCOUNT_VISIBILITY_PREF_KEY)
-        privacy_setting = profile_privacy if profile_privacy else configuration.get('default_visibility')
-
-        if privacy_setting == ALL_USERS_VISIBILITY:
-            field_names = configuration.get('shareable_fields')
-        else:
-            field_names = configuration.get('public_fields')
-
-        for field_name in field_names:
-            visible_settings[field_name] = account_settings.get(field_name, None)
-
-        return visible_settings
-
     def patch(self, request, username):
         """
         PATCH /api/user/v0/accounts/{username}/
@@ -170,137 +119,18 @@ class AccountView(APIView):
         else an error response with status code 415 will be returned.
         """
         try:
-            AccountView.update_account(request.user, request.DATA, username=username)
+            update_account_settings(request.user, request.DATA, username=username)
         except (AccountUserNotFound, AccountNotAuthorized):
             return Response(status=status.HTTP_404_NOT_FOUND)
+        except AccountValidationError as err:
+            return Response({"field_errors": err.field_errors}, status=status.HTTP_400_BAD_REQUEST)
         except AccountUpdateError as err:
-            return Response(err.error_info, status=status.HTTP_400_BAD_REQUEST)
+            return Response(
+                {
+                    "developer_message": err.developer_message,
+                    "user_message": err.user_message
+                },
+                status=status.HTTP_400_BAD_REQUEST
+            )
 
         return Response(status=status.HTTP_204_NO_CONTENT)
-
-    @staticmethod
-    def update_account(requesting_user, update, username=None):
-        """Update user account information.
-
-        Note:
-            It is up to the caller of this method to enforce the contract that this method is only called
-            with the user who made the request.
-
-        Arguments:
-            requesting_user (User): The user requesting to modify account information. Only the user with username
-                'username' has permissions to modify account information.
-            update (dict): The updated account field values.
-            username (string): Optional username specifying which account should be updated. If not specified,
-                `requesting_user.username` is assumed.
-
-        Raises:
-            AccountUserNotFound: `username` was specified, but no user exists with that username.
-            AccountUpdateError: the update could not be completed, usually due to validation errors
-                (for example, read-only fields were specified or field values are not legal)
-            AccountNotAuthorized: the requesting_user does not have access to change the account
-                associated with `username`.
-        """
-        if username is None:
-            username = requesting_user.username
-
-        if requesting_user.username != username:
-            raise AccountNotAuthorized()
-
-        existing_user, existing_user_profile = AccountView._get_user_and_profile(username)
-
-        # If user has requested to change email, we must call the multi-step process to handle this.
-        # It is not handled by the serializer (which considers email to be read-only).
-        new_email = None
-        if "email" in update:
-            new_email = update["email"]
-            del update["email"]
-
-        # If user has requested to change name, store old name because we must update associated metadata
-        # after the save process is complete.
-        old_name = None
-        if "name" in update:
-            old_name = existing_user_profile.name
-
-        # Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
-        read_only_fields = set(update.keys()).intersection(
-            AccountUserSerializer.Meta.read_only_fields + AccountLegacyProfileSerializer.Meta.read_only_fields
-        )
-        if 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))
-                }
-            raise AccountUpdateError({"field_errors": field_errors})
-
-        # If the user asked to change email, send the request now.
-        if new_email:
-            try:
-                do_email_change_request(existing_user, new_email)
-            except ValueError as err:
-                response_data = {
-                    "developer_message": "Error thrown from do_email_change_request: '{}'".format(err.message),
-                    "user_message": err.message
-                }
-                raise AccountUpdateError(response_data)
-
-        user_serializer = AccountUserSerializer(existing_user, data=update)
-        legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile, data=update)
-
-        for serializer in user_serializer, legacy_profile_serializer:
-            validation_errors = AccountView._get_validation_errors(update, serializer)
-            if validation_errors:
-                raise AccountUpdateError(validation_errors)
-            serializer.save()
-
-        # If the name was changed, store information about the change operation. This is outside of the
-        # serializer so that we can store who requested the change.
-        if old_name:
-            meta = existing_user_profile.get_meta()
-            if 'old_names' not in meta:
-                meta['old_names'] = []
-            meta['old_names'].append([
-                old_name,
-                "Name change requested through account API by {0}".format(requesting_user.username),
-                datetime.datetime.now(UTC).isoformat()
-            ])
-            existing_user_profile.set_meta(meta)
-            existing_user_profile.save()
-
-    @staticmethod
-    def _get_user_and_profile(username):
-        """
-        Helper method to return the legacy user and profile objects based on username.
-        """
-        try:
-            existing_user = User.objects.get(username=username)
-            existing_user_profile = UserProfile.objects.get(user=existing_user)
-        except ObjectDoesNotExist:
-            raise AccountUserNotFound()
-
-        return existing_user, existing_user_profile
-
-    @staticmethod
-    def _get_validation_errors(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

openedx/core/djangoapps/user_api/api/account.py

@@ -67,11 +67,23 @@ class AccountNotAuthorized(AccountRequestError):
 
 class AccountUpdateError(AccountRequestError):
     """
-    An update to the account failed. More detailed information is present in error_info (a dict
-    with at least a developer_message, though possibly also a nested field_errors dict).
+    An update to the account failed. More detailed information is present in developer_message,
+    and depending on the type of error encountered, there may also be a non-null user_message field.
     """
-    def __init__(self, error_info):
-        self.error_info = error_info
+    def __init__(self, developer_message, user_message=None):
+        self.developer_message = developer_message
+        self.user_message = user_message
+
+
+class AccountValidationError(AccountRequestError):
+    """
+    Validation issues were found with the supplied data. More detailed information is present in field_errors,
+    a dict with specific information about each field that failed validation. For each field,
+    there will be at least a developer_message describing the validation issue, and possibly
+    also a user_message.
+    """
+    def __init__(self, field_errors):
+        self.field_errors = field_errors
 
 
 @intercept_errors(AccountInternalError, ignore_errors=[AccountRequestError])

openedx/core/djangoapps/user_api/api/profile.py

@@ -15,7 +15,7 @@ import analytics
 
 from eventtracking import tracker
 from ..accounts import NAME_MIN_LENGTH
-from ..accounts.views import AccountView
+from ..accounts.api import get_account_settings
 from ..models import User, UserPreference, UserOrgTag
 from ..helpers import intercept_errors
 
@@ -106,7 +106,7 @@ def update_email_opt_in(user, org, optin):
         None
 
     """
-    account_settings = AccountView.get_serialized_account(user)
+    account_settings = get_account_settings(user)
     year_of_birth = account_settings['year_of_birth']
     of_age = (
         year_of_birth is None or  # If year of birth is not set, we assume user is of age.

openedx/core/djangoapps/user_api/tests/test_profile_api.py

@@ -11,7 +11,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 import datetime
 
-from ..accounts.views import AccountView
+from ..accounts.api import get_account_settings
 from ..api import account as account_api
 from ..api import profile as profile_api
 from ..models import UserProfile, UserOrgTag
@@ -30,7 +30,7 @@ class ProfileApiTest(ModuleStoreTestCase):
 
         # Retrieve the account settings
         user = User.objects.get(username=self.USERNAME)
-        account_settings = AccountView.get_serialized_account(user)
+        account_settings = get_account_settings(user)
 
         # Expect a date joined field but remove it to simplify the following comparison
         self.assertIsNotNone(account_settings['date_joined'])

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

@@ -24,7 +24,7 @@ from django_comment_common import models
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from third_party_auth.tests.testutil import simulate_running_pipeline
 
-from ..accounts.views import AccountView
+from ..accounts.api import get_account_settings
 from ..api import account as account_api, profile as profile_api
 from ..models import UserOrgTag
 from ..tests.factories import UserPreferenceFactory
@@ -1249,7 +1249,7 @@ class RegistrationViewTest(ApiTestCase):
 
         # Verify that the user's full name is set
         user = User.objects.get(username=self.USERNAME)
-        account_settings = AccountView.get_serialized_account(user)
+        account_settings = get_account_settings(user)
         self.assertEqual(account_settings["name"], self.NAME)
 
         # Verify that we've been logged in
@@ -1283,7 +1283,7 @@ class RegistrationViewTest(ApiTestCase):
 
         # Verify the user's account
         user = User.objects.get(username=self.USERNAME)
-        account_settings = AccountView.get_serialized_account(user)
+        account_settings = get_account_settings(user)
         self.assertEqual(account_settings["level_of_education"], self.EDUCATION)
         self.assertEqual(account_settings["mailing_address"], self.ADDRESS)
         self.assertEqual(account_settings["year_of_birth"], int(self.YEAR_OF_BIRTH))