Merge pull request #7226 from edx/christina/update-accounts-api

Updates to account and profile APIs based on arch council feedback

common/djangoapps/student/tests/test_enrollment.py

@@ -130,7 +130,7 @@ class EnrollmentTest(UrlResetMixin, ModuleStoreTestCase):
         # Verify that the profile API has been called as expected
         if email_opt_in is not None:
             opt_in = email_opt_in == 'true'
-            mock_update_email_opt_in.assert_called_once_with(self.USERNAME, self.course.org, opt_in)
+            mock_update_email_opt_in.assert_called_once_with(self.user, self.course.org, opt_in)
         else:
             self.assertFalse(mock_update_email_opt_in.called)
 

common/djangoapps/student/views.py

@@ -797,7 +797,7 @@ def try_change_enrollment(request):
             log.exception(u"Exception automatically enrolling after login: %s", exc)
 
 
-def _update_email_opt_in(request, username, org):
+def _update_email_opt_in(request, org):
     """Helper function used to hit the profile API if email opt-in is enabled."""
 
     # TODO: remove circular dependency on openedx from common
@@ -806,7 +806,7 @@ def _update_email_opt_in(request, username, org):
     email_opt_in = request.POST.get('email_opt_in')
     if email_opt_in is not None:
         email_opt_in_boolean = email_opt_in == 'true'
-        profile_api.update_email_opt_in(username, org, email_opt_in_boolean)
+        profile_api.update_email_opt_in(request.user, org, email_opt_in_boolean)
 
 
 @require_POST
@@ -878,7 +878,7 @@ def change_enrollment(request, check_access=True):
 
         # Record the user's email opt-in preference
         if settings.FEATURES.get('ENABLE_MKTG_EMAIL_OPT_IN'):
-            _update_email_opt_in(request, user.username, course_id.org)
+            _update_email_opt_in(request, course_id.org)
 
         available_modes = CourseMode.modes_for_course_dict(course_id)
 
@@ -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()

common/djangoapps/third_party_auth/pipeline.py

@@ -672,7 +672,7 @@ def change_enrollment(strategy, user=None, is_dashboard=False, *args, **kwargs):
             # TODO: remove circular dependency on openedx from common
             from openedx.core.djangoapps.user_api.api import profile
             opt_in = email_opt_in.lower() == 'true'
-            profile.update_email_opt_in(user.username, course_id.org, opt_in)
+            profile.update_email_opt_in(user, course_id.org, opt_in)
 
         # Check whether we're blocked from enrolling by a
         # country access rule.

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
@@ -1167,7 +1167,7 @@ class TestSubmitPhotosForVerification(TestCase):
             AssertionError
 
         """
-        account_settings = AccountView.get_serialized_account(self.user.username)
+        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
@@ -714,16 +714,14 @@ def submit_photos_for_verification(request):
     if SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user):
         return HttpResponseBadRequest(_("You already have a valid or pending verification."))
 
-    username = request.user.username
-
     # If the user wants to change his/her full name,
     # then try to do that before creating the attempt.
     if request.POST.get('full_name'):
         try:
-            AccountView.update_account(request.user, username, {"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)
@@ -743,7 +741,7 @@ def submit_photos_for_verification(request):
     attempt.mark_ready()
     attempt.submit()
 
-    account_settings = AccountView.get_serialized_account(username)
+    account_settings = get_account_settings(request.user)
 
     # Send a confirmation email to the user
     context = {

lms/envs/common.py

@@ -2053,14 +2053,14 @@ SEARCH_ENGINE = None
 # Use the LMS specific result processor
 SEARCH_RESULT_PROCESSOR = "lms.lib.courseware_search.lms_result_processor.LmsSearchResultProcessor"
 
-# The configuration for learner profiles
-PROFILE_CONFIGURATION = {
+# The configuration visibility of account fields.
+ACCOUNT_VISIBILITY_CONFIGURATION = {
     # Default visibility level for accounts without a specified value
     # The value is one of: 'all_users', 'private'
     "default_visibility": "private",
 
-    # The list of all fields that can be shown on a learner's profile
-    "all_fields": [
+    # The list of all fields that can be shared with other users
+    "shareable_fields": [
         'username',
         'profile_image',
         'country',
@@ -2069,7 +2069,7 @@ PROFILE_CONFIGURATION = {
         'bio',
     ],
 
-    # The list of fields that are always public on a learner's profile
+    # The list of account fields that are always public
     "public_fields": [
         'username',
         'profile_image',

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

@@ -1,2 +1,14 @@
+"""
+Account constants
+"""
+
 # The minimum acceptable length for the name account field
 NAME_MIN_LENGTH = 2
+
+ACCOUNT_VISIBILITY_PREF_KEY = 'account_privacy'
+
+# Indicates the user's preference that all users can view the shareable fields in their account information.
+ALL_USERS_VISIBILITY = 'all_users'
+
+# Indicates the user's preference that all their account information be private.
+PRIVATE_VISIBILITY = 'private'

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

@@ -2,6 +2,7 @@
 import unittest
 import ddt
 import json
+from mock import patch
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
@@ -9,6 +10,9 @@ from rest_framework.test import APITestCase, APIClient
 
 from student.tests.factories import UserFactory
 from student.models import UserProfile, PendingEmailChange
+from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY
+from openedx.core.djangoapps.user_api.models import UserPreference
+from .. import PRIVATE_VISIBILITY, ALL_USERS_VISIBILITY
 
 TEST_PASSWORD = "test"
 
@@ -73,24 +77,66 @@ class TestAccountAPI(UserAPITestCase):
     """
     Unit tests for the Account API.
     """
-
     def setUp(self):
         super(TestAccountAPI, self).setUp()
 
         self.url = reverse("accounts_api", kwargs={'username': self.user.username})
 
-    def test_get_account_anonymous_user(self):
+    def _verify_full_shareable_account_response(self, response):
         """
-        Test that an anonymous client (not logged in) cannot call get.
+        Verify that the shareable fields from the account are returned
+        """
+        data = response.data
+        self.assertEqual(6, len(data))
+        self.assertEqual(self.user.username, data["username"])
+        self.assertEqual("US", data["country"])
+        self.assertIsNone(data["profile_image"])
+        self.assertIsNone(data["time_zone"])
+        self.assertIsNone(data["languages"])
+        self.assertIsNone(data["bio"])
+
+    def _verify_private_account_response(self, response):
+        """
+        Verify that only the public fields are returned if a user does not want to share account fields
+        """
+        data = response.data
+        self.assertEqual(2, len(data))
+        self.assertEqual(self.user.username, data["username"])
+        self.assertIsNone(data["profile_image"])
+
+    def _verify_full_account_response(self, response):
+        """
+        Verify that all account fields are returned (even those that are not shareable).
+        """
+        data = response.data
+        self.assertEqual(11, len(data))
+        self.assertEqual(self.user.username, data["username"])
+        self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
+        self.assertEqual("US", data["country"])
+        self.assertEqual("", data["language"])
+        self.assertEqual("m", data["gender"])
+        self.assertEqual(1900, data["year_of_birth"])
+        self.assertEqual("m", data["level_of_education"])
+        self.assertEqual("world peace", data["goals"])
+        self.assertEqual("Park Ave", data['mailing_address'])
+        self.assertEqual(self.user.email, data["email"])
+        self.assertIsNotNone(data["date_joined"])
+
+    def test_anonymous_access(self):
+        """
+        Test that an anonymous client (not logged in) cannot call GET or PATCH.
         """
         self.send_get(self.anonymous_client, expected_status=401)
+        self.send_patch(self.anonymous_client, {}, expected_status=401)
 
-    def test_get_account_different_user(self):
+    def test_unsupported_methods(self):
         """
-        Test that a client (logged in) cannot get the account information for a different client.
+        Test that DELETE, POST, and PUT are not supported.
         """
-        self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
-        self.send_get(self.different_client, expected_status=404)
+        self.client.login(username=self.user.username, password=TEST_PASSWORD)
+        self.assertEqual(405, self.client.put(self.url).status_code)
+        self.assertEqual(405, self.client.post(self.url).status_code)
+        self.assertEqual(405, self.client.delete(self.url).status_code)
 
     @ddt.data(
         ("client", "user"),
@@ -105,6 +151,69 @@ class TestAccountAPI(UserAPITestCase):
         response = client.get(reverse("accounts_api", kwargs={'username': "does_not_exist"}))
         self.assertEqual(404, response.status_code)
 
+    # Note: using getattr so that the patching works even if there is no configuration.
+    # This is needed when testing CMS as the patching is still executed even though the
+    # suite is skipped.
+    @patch.dict(getattr(settings, "ACCOUNT_VISIBILITY_CONFIGURATION", {}), {"default_visibility": "all_users"})
+    def test_get_account_different_user_visible(self):
+        """
+        Test that a client (logged in) can only get the shareable fields for a different user.
+        This is the case when default_visibility is set to "all_users".
+        """
+        self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
+        self.create_mock_profile(self.user)
+        response = self.send_get(self.different_client)
+        self._verify_full_shareable_account_response(response)
+
+    # Note: using getattr so that the patching works even if there is no configuration.
+    # This is needed when testing CMS as the patching is still executed even though the
+    # suite is skipped.
+    @patch.dict(getattr(settings, "ACCOUNT_VISIBILITY_CONFIGURATION", {}), {"default_visibility": "private"})
+    def test_get_account_different_user_private(self):
+        """
+        Test that a client (logged in) can only get the shareable fields for a different user.
+        This is the case when default_visibility is set to "private".
+        """
+        self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
+        self.create_mock_profile(self.user)
+        response = self.send_get(self.different_client)
+        self._verify_private_account_response(response)
+
+    @ddt.data(
+        ("client", "user", PRIVATE_VISIBILITY),
+        ("different_client", "different_user", PRIVATE_VISIBILITY),
+        ("staff_client", "staff_user", PRIVATE_VISIBILITY),
+        ("client", "user", ALL_USERS_VISIBILITY),
+        ("different_client", "different_user", ALL_USERS_VISIBILITY),
+        ("staff_client", "staff_user", ALL_USERS_VISIBILITY),
+    )
+    @ddt.unpack
+    def test_get_account_private_visibility(self, api_client, requesting_username, preference_visibility):
+        """
+        Test the return from GET based on user visibility setting.
+        """
+        def verify_fields_visible_to_all_users(response):
+            if preference_visibility == PRIVATE_VISIBILITY:
+                self._verify_private_account_response(response)
+            else:
+                self._verify_full_shareable_account_response(response)
+
+        client = self.login_client(api_client, requesting_username)
+
+        # Update user account visibility setting.
+        UserPreference.set_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, preference_visibility)
+        self.create_mock_profile(self.user)
+        response = self.send_get(client)
+
+        if requesting_username == "different_user":
+            verify_fields_visible_to_all_users(response)
+        else:
+            self._verify_full_account_response(response)
+
+        # Verify how the view parameter changes the fields that are returned.
+        response = self.send_get(client, query_parameters='view=shared')
+        verify_fields_visible_to_all_users(response)
+
     def test_get_account_default(self):
         """
         Test that a client (logged in) can get her own account information (using default legacy profile information,
@@ -126,33 +235,6 @@ class TestAccountAPI(UserAPITestCase):
         self.assertEqual(self.user.email, data["email"])
         self.assertIsNotNone(data["date_joined"])
 
-    @ddt.data(
-        ("client", "user"),
-        ("staff_client", "staff_user"),
-    )
-    @ddt.unpack
-    def test_get_account(self, api_client, user):
-        """
-        Test that a client (logged in) can get her own account information. Also verifies that a "is_staff"
-        user can get the account information for other users.
-        """
-        self.create_mock_profile(self.user)
-        client = self.login_client(api_client, user)
-        response = self.send_get(client)
-        data = response.data
-        self.assertEqual(11, len(data))
-        self.assertEqual(self.user.username, data["username"])
-        self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
-        self.assertEqual("US", data["country"])
-        self.assertEqual("", data["language"])
-        self.assertEqual("m", data["gender"])
-        self.assertEqual(1900, data["year_of_birth"])
-        self.assertEqual("m", data["level_of_education"])
-        self.assertEqual("world peace", data["goals"])
-        self.assertEqual("Park Ave", data['mailing_address'])
-        self.assertEqual(self.user.email, data["email"])
-        self.assertIsNotNone(data["date_joined"])
-
     def test_get_account_empty_string(self):
         """
         Test the conversion of empty strings to None for certain fields.
@@ -168,18 +250,18 @@ class TestAccountAPI(UserAPITestCase):
         for empty_field in ("level_of_education", "gender", "country"):
             self.assertIsNone(response.data[empty_field])
 
-    def test_patch_account_anonymous_user(self):
+    @ddt.data(
+        ("different_client", "different_user"),
+        ("staff_client", "staff_user"),
+    )
+    @ddt.unpack
+    def test_patch_account_disallowed_user(self, api_client, user):
         """
-        Test that an anonymous client (not logged in) cannot call patch.
+        Test that a client cannot call PATCH on a different client's user account (even with
+        is_staff access).
         """
-        self.send_patch(self.anonymous_client, {}, expected_status=401)
-
-    def test_patch_account_different_user(self):
-        """
-        Test that a client (logged in) cannot update the account information for a different client.
-        """
-        self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
-        self.send_patch(self.different_client, {}, expected_status=404)
+        client = self.login_client(api_client, user)
+        self.send_patch(client, {}, expected_status=404)
 
     @ddt.data(
         ("client", "user"),
@@ -198,34 +280,23 @@ class TestAccountAPI(UserAPITestCase):
         self.assertEqual(404, response.status_code)
 
     @ddt.data(
-        (
-            "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", "XY", "Select a valid choice. XY is not one of the available choices."),
-        ("client", "user", "year_of_birth", 2009, "not_an_int", "Enter a whole number."),
-        ("client", "user", "name", "bob", "z" * 256, "Ensure this value has at most 255 characters (it has 256)."),
-        ("client", "user", "name", u"ȻħȺɍłɇs", "z   ", "The name field must be at least 2 characters long."),
-        ("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"),
+        ("gender", "f", "not a gender", "Select a valid choice. not a gender is not one of the available choices."),
+        ("level_of_education", "none", "x", "Select a valid choice. x is not one of the available choices."),
+        ("country", "GB", "XY", "Select a valid choice. XY is not one of the available choices."),
+        ("year_of_birth", 2009, "not_an_int", "Enter a whole number."),
+        ("name", "bob", "z" * 256, "Ensure this value has at most 255 characters (it has 256)."),
+        ("name", u"ȻħȺɍłɇs", "z   ", "The name field must be at least 2 characters long."),
+        ("language", "Creole"),
+        ("goals", "Smell the roses"),
+        ("mailing_address", "Sesame Street"),
         # Note that email is tested below, as it is not immediately updated.
     )
     @ddt.unpack
-    def test_patch_account(
-            self, api_client, user, field, value, fails_validation_value=None, developer_validation_message=None
-    ):
+    def test_patch_account(self, field, value, fails_validation_value=None, developer_validation_message=None):
         """
         Test the behavior of patch, when using the correct content_type.
         """
-        client = self.login_client(api_client, user)
+        client = self.login_client("client", "user")
         self.send_patch(client, {field: value})
 
         get_response = self.send_get(client)
@@ -248,16 +319,12 @@ class TestAccountAPI(UserAPITestCase):
             get_response = self.send_get(client)
             self.assertEqual("", get_response.data[field])
 
-    @ddt.data(
-        ("client", "user"),
-        ("staff_client", "staff_user"),
-    )
     @ddt.unpack
-    def test_patch_account_noneditable(self, api_client, user):
+    def test_patch_account_noneditable(self):
         """
         Tests the behavior of patch when a read-only field is attempted to be edited.
         """
-        client = self.login_client(api_client, user)
+        client = self.login_client("client", "user")
 
         def verify_error_response(field_name, data):
             self.assertEqual(
@@ -336,25 +403,19 @@ class TestAccountAPI(UserAPITestCase):
         name_change_info = get_name_change_info(1)
         verify_change_info(name_change_info[0], old_name, self.user.username, "Mickey Mouse")
 
-        # Now change the name as a different (staff) user and verify meta information.
-        self.staff_client.login(username=self.staff_user.username, password=TEST_PASSWORD)
-        self.send_patch(self.staff_client, {"name": "Donald Duck"})
+        # Now change the name again and verify meta information.
+        self.send_patch(self.client, {"name": "Donald Duck"})
         name_change_info = get_name_change_info(2)
         verify_change_info(name_change_info[0], old_name, self.user.username, "Donald Duck",)
-        verify_change_info(name_change_info[1], "Mickey Mouse", self.staff_user.username, "Donald Duck")
+        verify_change_info(name_change_info[1], "Mickey Mouse", self.user.username, "Donald Duck")
 
-    @ddt.data(
-        ("client", "user"),
-        ("staff_client", "staff_user"),
-    )
-    @ddt.unpack
-    def test_patch_email(self, api_client, user):
+    def test_patch_email(self):
         """
-        Test that the user (and anyone with an is_staff account) can request an email change through the accounts API.
+        Test that the user can request an email change through the accounts API.
         Full testing of the helper method used (do_email_change_request) exists in the package with the code.
         Here just do minimal smoke testing.
         """
-        client = self.login_client(api_client, user)
+        client = self.login_client("client", "user")
         old_email = self.user.email
         new_email = "newemail@example.com"
         self.send_patch(client, {"email": new_email, "goals": "change my email"})
@@ -379,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,24 +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 _
-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
+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 .api import get_account_settings, update_account_settings
 
 
 class AccountView(APIView):
@@ -32,54 +25,78 @@ 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, IsUserInUrlOrStaff)
+    permission_classes = (permissions.IsAuthenticated,)
     parser_classes = (MergePatchParser,)
 
     def get(self, request, username):
@@ -87,35 +104,12 @@ class AccountView(APIView):
         GET /api/user/v0/accounts/{username}/
         """
         try:
-            account_settings = AccountView.get_serialized_account(username)
+            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(username):
-        """Returns the user's account information serialized as JSON.
-
-        Note:
-          This method does not perform authentication so it is up to the caller
-          to ensure that only the user themselves or staff can access the account.
-
-        Args:
-          username (str): The username for the desired account.
-
-        Returns:
-           A dict containing each of the account's fields.
-
-        Raises:
-           AccountUserNotFound: raised if there is no account for the specified username.
-        """
-        existing_user, existing_user_profile = AccountView._get_user_and_profile(username)
-        user_serializer = AccountUserSerializer(existing_user)
-        legacy_profile_serializer = AccountLegacyProfileSerializer(existing_user_profile)
-
-        return dict(user_serializer.data, **legacy_profile_serializer.data)
-
     def patch(self, request, username):
         """
         PATCH /api/user/v0/accounts/{username}/
@@ -125,128 +119,18 @@ class AccountView(APIView):
         else an error response with status code 415 will be returned.
         """
         try:
-            AccountView.update_account(request.user, username, request.DATA)
-        except AccountUserNotFound:
+            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, username, update):
-        """Update the account for the given username.
-
-        Note:
-            No authorization or permissions checks are done in this method. It is up to the caller
-            of this method to enforce the contract that this method is only called if
-            requesting_user.username == username or requesting_user.is_staff == True.
-
-        Arguments:
-            requesting_user (User): the user who initiated the request
-            username (string): the username associated with the account to change
-            update (dict): the updated account field values
-
-        Raises:
-            AccountUserNotFound: no user exists with the specified 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)
-        """
-        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
 
@@ -90,14 +90,14 @@ def update_preferences(username, **kwargs):
 
 
 @intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError])
-def update_email_opt_in(username, org, optin):
+def update_email_opt_in(user, org, optin):
     """Updates a user's preference for receiving org-wide emails.
 
     Sets a User Org Tag defining the choice to opt in or opt out of organization-wide
     emails.
 
     Arguments:
-        username (str): The user to set a preference for.
+        user (User): The user to set a preference for.
         org (str): The org is used to determine the organization this setting is related to.
         optin (Boolean): True if the user is choosing to receive emails for this organization. If the user is not
             the correct age to receive emails, email-optin is set to False regardless.
@@ -105,11 +105,8 @@ def update_email_opt_in(username, org, optin):
     Returns:
         None
 
-    Raises:
-        AccountUserNotFound: Raised when the username specified is not associated with a user.
-
     """
-    account_settings = AccountView.get_serialized_account(username)
+    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.
@@ -118,7 +115,6 @@ def update_email_opt_in(username, org, optin):
     )
 
     try:
-        user = User.objects.get(username=username)
         preference, _ = UserOrgTag.objects.get_or_create(
             user=user, org=org, key='email-optin'
         )

openedx/core/djangoapps/user_api/management/tests/test_email_opt_in_list.py

@@ -297,7 +297,7 @@ class EmailOptInListTest(ModuleStoreTestCase):
             None
 
         """
-        profile_api.update_email_opt_in(user.username, org, is_opted_in)
+        profile_api.update_email_opt_in(user, org, is_opted_in)
 
     def _latest_pref_set_datetime(self, user):
         """Retrieve the latest opt-in preference for the user,

openedx/core/djangoapps/user_api/profiles/__init__.py

@@ -1,11 +0,0 @@
-"""
-Profile constants
-"""
-
-PROFILE_VISIBILITY_PREF_KEY = 'profile_privacy'
-
-# Indicates the user's preference that all users can view their profile.
-ALL_USERS_VISIBILITY = 'all_users'
-
-# Indicates the user's preference that their profile be private.
-PRIVATE_VISIBILITY = 'private'

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

@@ -1,137 +0,0 @@
-"""
-Unit tests for profile APIs.
-"""
-
-import ddt
-import unittest
-
-from django.conf import settings
-from django.core.urlresolvers import reverse
-from mock import patch
-
-from openedx.core.djangoapps.user_api.accounts.tests.test_views import UserAPITestCase
-from openedx.core.djangoapps.user_api.models import UserPreference
-from openedx.core.djangoapps.user_api.profiles import PROFILE_VISIBILITY_PREF_KEY
-from .. import PRIVATE_VISIBILITY
-
-
-@ddt.ddt
-@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile APIs are only supported in LMS')
-class TestProfileAPI(UserAPITestCase):
-    """
-    Unit tests for the profile API.
-    """
-
-    def setUp(self):
-        super(TestProfileAPI, self).setUp()
-        self.url = reverse("profiles_api", kwargs={'username': self.user.username})
-
-    def test_get_profile_anonymous_user(self):
-        """
-        Test that an anonymous client (not logged in) cannot call get.
-        """
-        self.send_get(self.anonymous_client, expected_status=401)
-
-    def _verify_full_profile_response(self, response):
-        """
-        Verify that all of the profile's fields are returned
-        """
-        data = response.data
-        self.assertEqual(6, len(data))
-        self.assertEqual(self.user.username, data["username"])
-        self.assertEqual("US", data["country"])
-        self.assertIsNone(data["profile_image"])
-        self.assertIsNone(data["time_zone"])
-        self.assertIsNone(data["languages"])
-        self.assertIsNone(data["bio"])
-
-    def _verify_private_profile_response(self, response):
-        """
-        Verify that only the public fields are returned for a private user's profile
-        """
-        data = response.data
-        self.assertEqual(2, len(data))
-        self.assertEqual(self.user.username, data["username"])
-        self.assertIsNone(data["profile_image"])
-
-    @ddt.data(
-        ("client", "user"),
-        ("different_client", "different_user"),
-        ("staff_client", "staff_user"),
-    )
-    @ddt.unpack
-    # Note: using getattr so that the patching works even if there is no configuration.
-    # This is needed when testing CMS as the patching is still executed even though the
-    # suite is skipped.
-    @patch.dict(getattr(settings, "PROFILE_CONFIGURATION", {}), {"default_visibility": "all_users"})
-    def test_get_default_profile(self, api_client, username):
-        """
-        Test that any logged in user can get the main test user's public profile information.
-        """
-        client = self.login_client(api_client, username)
-        self.create_mock_profile(self.user)
-        response = self.send_get(client)
-        self._verify_full_profile_response(response)
-
-    @ddt.data(
-        ("client", "user"),
-        ("different_client", "different_user"),
-        ("staff_client", "staff_user"),
-    )
-    @ddt.unpack
-    # Note: using getattr so that the patching works even if there is no configuration.
-    # This is needed when testing CMS as the patching is still executed even though the
-    # suite is skipped.
-    @patch.dict(getattr(settings, "PROFILE_CONFIGURATION", {}), {"default_visibility": "private"})
-    def test_get_default_private_profile(self, api_client, username):
-        """
-        Test that any logged in user gets only the public fields for a profile
-        if the default visibility is 'private'.
-        """
-        client = self.login_client(api_client, username)
-        self.create_mock_profile(self.user)
-        response = self.send_get(client)
-        self._verify_private_profile_response(response)
-
-    @ddt.data(
-        ("client", "user"),
-        ("different_client", "different_user"),
-        ("staff_client", "staff_user"),
-    )
-    @ddt.unpack
-    def test_get_private_profile(self, api_client, requesting_username):
-        """
-        Test that private profile information is only available to the test user themselves.
-        """
-        client = self.login_client(api_client, requesting_username)
-
-        # Verify that a user with a private profile only returns the public fields
-        UserPreference.set_preference(self.user, PROFILE_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY)
-        self.create_mock_profile(self.user)
-        response = self.send_get(client)
-        self._verify_private_profile_response(response)
-
-        # Verify that only the public fields are returned if 'include_all' parameter is specified as false
-        response = self.send_get(client, query_parameters='include_all=false')
-        self._verify_private_profile_response(response)
-
-        # Verify that all fields are returned for the user themselves if
-        # the 'include_all' parameter is specified as true.
-        response = self.send_get(client, query_parameters='include_all=true')
-        if requesting_username == "user":
-            self._verify_full_profile_response(response)
-        else:
-            self._verify_private_profile_response(response)
-
-    @ddt.data(
-        ("client", "user"),
-        ("staff_client", "staff_user"),
-    )
-    @ddt.unpack
-    def test_get_profile_unknown_user(self, api_client, username):
-        """
-        Test that requesting a user who does not exist returns a 404.
-        """
-        client = self.login_client(api_client, username)
-        response = client.get(reverse("profiles_api", kwargs={'username': "does_not_exist"}))
-        self.assertEqual(404, response.status_code)

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

@@ -1,123 +0,0 @@
-"""
-NOTE: this API is WIP and has not yet been approved. Do not use this API without talking to Christina or Andy.
-
-For more information, see:
-https://openedx.atlassian.net/wiki/display/TNL/User+API
-"""
-from django.conf import settings
-from django.contrib.auth.models import User
-
-from rest_framework import status
-from rest_framework.views import APIView
-from rest_framework.response import Response
-from rest_framework.authentication import OAuth2Authentication, SessionAuthentication
-from rest_framework import permissions
-
-from ..accounts.views import AccountView
-from ..api.account import AccountUserNotFound
-from ..models import UserPreference
-
-from . import PROFILE_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY
-
-
-class ProfileView(APIView):
-    """
-        **Use Cases**
-
-            Get the user's public profile information.
-
-        **Example Requests**:
-
-            GET /api/user/v0/profiles/{username}/[?include_all={true | false}]
-
-        **Response Values for GET**
-
-            Returns the same responses as for the AccountView API for the
-            subset of fields that have been configured to be in a profile.
-            The fields are additionally filtered based upon the user's
-            specified privacy permissions.
-
-            If the parameter 'include_all' is passed as 'true' then a user
-            can receive all fields for their own account, ignoring
-            any field visibility preferences. If the parameter is not
-            specified or if the user is requesting information for a
-            different account then the privacy filtering will be applied.
-
-    """
-    authentication_classes = (OAuth2Authentication, SessionAuthentication)
-    permission_classes = (permissions.IsAuthenticated,)
-
-    def get(self, request, username):
-        """
-        GET /api/user/v0/profiles/{username}/[?include_all={true | false}]
-
-        Note:
-          The include_all query parameter will only be honored if the user is making
-          the request for their own username. It defaults to false, but if true
-          then all the profile fields will be returned even for a user with
-          a private profile.
-        """
-        if request.user.username == username:
-            include_all_fields = self.request.QUERY_PARAMS.get('include_all') == 'true'
-        else:
-            include_all_fields = False
-        try:
-            profile_settings = ProfileView.get_serialized_profile(
-                username,
-                settings.PROFILE_CONFIGURATION,
-                include_all_fields=include_all_fields,
-            )
-        except AccountUserNotFound:
-            return Response(status=status.HTTP_404_NOT_FOUND)
-        return Response(profile_settings)
-
-    @staticmethod
-    def get_serialized_profile(username, configuration=None, include_all_fields=False):
-        """Returns the user's public profile settings serialized as JSON.
-
-        The fields returned are by default governed by the user's privacy preference.
-        If the user has a private profile, then only the fields that are always
-        public are returned. If the user is sharing their profile with all users
-        then all profile fields are returned.
-
-        Note:
-          This method does not perform authentication so it is up to the caller
-          to ensure that only edX users can access the profile. In addition, only
-          the user themselves should be able to access all fields of a private
-          profile through 'include_all_fields' being true.
-
-        Args:
-          username (str): The username for the desired account.
-          configuration (dict): A dictionary specifying three profile configuration settings:
-            default_visibility: the default visibility level for user's with no preference
-            all_fields: the list of all fields that can be shown on a profile
-            public_fields: the list of profile fields that are public
-          include_all_fields (bool): If true, ignores the user's privacy setting.
-
-        Returns:
-           A dict containing each of the user's profile fields.
-
-        Raises:
-           AccountUserNotFound: raised if there is no account for the specified username.
-        """
-        if not configuration:
-            configuration = settings.PROFILE_CONFIGURATION
-        account_settings = AccountView.get_serialized_account(username)
-        profile = {}
-        privacy_setting = ProfileView._get_user_profile_privacy(username, configuration)
-        if include_all_fields or privacy_setting == ALL_USERS_VISIBILITY:
-            field_names = configuration.get('all_fields')
-        else:
-            field_names = configuration.get('public_fields')
-        for field_name in field_names:
-            profile[field_name] = account_settings.get(field_name, None)
-        return profile
-
-    @staticmethod
-    def _get_user_profile_privacy(username, configuration):
-        """
-        Returns the profile privacy preference for the specified user.
-        """
-        user = User.objects.get(username=username)
-        profile_privacy = UserPreference.get_preference(user, PROFILE_VISIBILITY_PREF_KEY)
-        return profile_privacy if profile_privacy else configuration.get('default_visibility')

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
@@ -29,7 +29,8 @@ class ProfileApiTest(ModuleStoreTestCase):
         account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
 
         # Retrieve the account settings
-        account_settings = AccountView.get_serialized_account(self.USERNAME)
+        user = User.objects.get(username=self.USERNAME)
+        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'])
@@ -87,7 +88,7 @@ class ProfileApiTest(ModuleStoreTestCase):
         profile.year_of_birth = year_of_birth
         profile.save()
 
-        profile_api.update_email_opt_in(self.USERNAME, course.id.org, option)
+        profile_api.update_email_opt_in(user, course.id.org, option)
         result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin')
         self.assertEqual(result_obj.value, expected_result)
 
@@ -99,7 +100,7 @@ class ProfileApiTest(ModuleStoreTestCase):
 
         user = User.objects.get(username=self.USERNAME)
 
-        profile_api.update_email_opt_in(self.USERNAME, course.id.org, True)
+        profile_api.update_email_opt_in(user, course.id.org, True)
         result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin')
         self.assertEqual(result_obj.value, u"True")
 
@@ -130,8 +131,8 @@ class ProfileApiTest(ModuleStoreTestCase):
         profile.year_of_birth = year_of_birth
         profile.save()
 
-        profile_api.update_email_opt_in(self.USERNAME, course.id.org, option)
-        profile_api.update_email_opt_in(self.USERNAME, course.id.org, second_option)
+        profile_api.update_email_opt_in(user, course.id.org, option)
+        profile_api.update_email_opt_in(user, course.id.org, second_option)
 
         result_obj = UserOrgTag.objects.get(user=user, org=course.id.org, key='email-optin')
         self.assertEqual(result_obj.value, expected_result)

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

@@ -8,6 +8,7 @@ import re
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.core import mail
+from django.contrib.auth.models import User
 from django.test import TestCase
 from django.test.utils import override_settings
 from unittest import skipUnless
@@ -23,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
@@ -1247,7 +1248,8 @@ class RegistrationViewTest(ApiTestCase):
         )
 
         # Verify that the user's full name is set
-        account_settings = AccountView.get_serialized_account(self.USERNAME)
+        user = User.objects.get(username=self.USERNAME)
+        account_settings = get_account_settings(user)
         self.assertEqual(account_settings["name"], self.NAME)
 
         # Verify that we've been logged in
@@ -1280,7 +1282,8 @@ class RegistrationViewTest(ApiTestCase):
         self.assertHttpOK(response)
 
         # Verify the user's account
-        account_settings = AccountView.get_serialized_account(self.USERNAME)
+        user = User.objects.get(username=self.USERNAME)
+        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))

openedx/core/djangoapps/user_api/urls.py

@@ -3,7 +3,6 @@ Defines the URL routes for this app.
 """
 
 from .accounts.views import AccountView
-from .profiles.views import ProfileView
 
 from django.conf.urls import patterns, url
 
@@ -16,9 +15,4 @@ urlpatterns = patterns(
         AccountView.as_view(),
         name="accounts_api"
     ),
-    url(
-        r'^v0/profiles/' + USERNAME_PATTERN + '$',
-        ProfileView.as_view(),
-        name="profiles_api"
-    ),
 )