From dfe0057b796ec239e0d05fe1dd1776a99f5b54c4 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Thu, 26 Feb 2015 14:00:47 -0500 Subject: [PATCH] Implement profile API TNL-1491 See the API design here: https://openedx.atlassian.net/wiki/display/TNL/User+API --- cms/urls.py | 11 +- common/djangoapps/student/views.py | 14 +- .../djangoapps/third_party_auth/pipeline.py | 4 +- .../verify_student/tests/test_views.py | 17 ++- lms/djangoapps/verify_student/views.py | 25 ++-- lms/envs/common.py | 23 ++++ lms/urls.py | 6 +- .../user_api/accounts/tests/__init__.py | 0 .../user_api/accounts/tests/test_views.py | 101 ++++++++------ .../core/djangoapps/user_api/accounts/urls.py | 14 -- .../djangoapps/user_api/accounts/views.py | 34 +++-- .../core/djangoapps/user_api/api/profile.py | 69 ++-------- .../core/djangoapps/user_api/legacy_urls.py | 43 ++++++ .../djangoapps/user_api/profiles/__init__.py | 11 ++ .../user_api/profiles/tests/__init__.py | 0 .../user_api/profiles/tests/test_views.py | 112 ++++++++++++++++ .../djangoapps/user_api/profiles/views.py | 123 ++++++++++++++++++ .../user_api/tests/test_profile_api.py | 24 ++-- .../djangoapps/user_api/tests/test_views.py | 24 ++-- openedx/core/djangoapps/user_api/urls.py | 37 ++---- 20 files changed, 484 insertions(+), 208 deletions(-) create mode 100644 openedx/core/djangoapps/user_api/accounts/tests/__init__.py delete mode 100644 openedx/core/djangoapps/user_api/accounts/urls.py create mode 100644 openedx/core/djangoapps/user_api/legacy_urls.py create mode 100644 openedx/core/djangoapps/user_api/profiles/__init__.py create mode 100644 openedx/core/djangoapps/user_api/profiles/tests/__init__.py create mode 100644 openedx/core/djangoapps/user_api/profiles/tests/test_views.py create mode 100644 openedx/core/djangoapps/user_api/profiles/views.py diff --git a/cms/urls.py b/cms/urls.py index a505636afb..a0cf873fdb 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -5,6 +5,8 @@ from django.conf.urls import patterns, include, url from ratelimitbackend import admin admin.autodiscover() +# pylint: disable=bad-continuation + # Pattern to match a course key or a library key COURSELIKE_KEY_PATTERN = r'(?P({}|{}))'.format( r'[^/]+/[^/]+/[^/]+', r'[^/:]+:[^/+]+\+[^/+]+(\+[^/]+)?' @@ -47,7 +49,7 @@ urlpatterns = patterns( url(r'^xmodule/', include('pipeline_js.urls')), url(r'^heartbeat$', include('heartbeat.urls')), - url(r'^user_api/', include('openedx.core.djangoapps.user_api.urls')), + url(r'^user_api/', include('openedx.core.djangoapps.user_api.legacy_urls')), url(r'^lang_pref/', include('lang_pref.urls')), ) @@ -87,7 +89,8 @@ urlpatterns += patterns( name='course_search_index_handler' ), url(r'^course/{}?$'.format(settings.COURSE_KEY_PATTERN), 'course_handler', name='course_handler'), - url(r'^course_notifications/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), 'course_notifications_handler'), + url(r'^course_notifications/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), + 'course_notifications_handler'), url(r'^course_rerun/{}$'.format(settings.COURSE_KEY_PATTERN), 'course_rerun_handler', name='course_rerun_handler'), url(r'^container/{}$'.format(settings.USAGE_KEY_PATTERN), 'container_handler'), url(r'^checklists/{}/(?P\d+)?$'.format(settings.COURSE_KEY_PATTERN), 'checklists_handler'), @@ -115,7 +118,7 @@ urlpatterns += patterns( url(r'^api/val/v0/', include('edxval.urls')), ) -js_info_dict = { +JS_INFO_DICT = { 'domain': 'djangojs', # We need to explicitly include external Django apps that are not in LOCALE_PATHS. 'packages': ('openassessment',), @@ -124,7 +127,7 @@ js_info_dict = { urlpatterns += patterns( '', # Serve catalog of localized strings to be rendered by Javascript - url(r'^i18n.js$', 'django.views.i18n.javascript_catalog', js_info_dict), + url(r'^i18n.js$', 'django.views.i18n.javascript_catalog', JS_INFO_DICT), ) if settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES'): diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index d2649b60c4..6697b48417 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -20,7 +20,7 @@ from django.contrib import messages from django.core.context_processors import csrf from django.core import mail from django.core.urlresolvers import reverse -from django.core.validators import validate_email, validate_slug, ValidationError +from django.core.validators import validate_email, ValidationError from django.db import IntegrityError, transaction from django.http import (HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, Http404) @@ -84,7 +84,6 @@ from external_auth.login_and_register import ( from bulk_email.models import Optout, CourseAuthorization import shoppingcart -from openedx.core.djangoapps.user_api.models import UserPreference from lang_pref import LANGUAGE_KEY from notification_prefs.views import enable_notifications @@ -113,7 +112,6 @@ from student.helpers import ( ) from xmodule.error_module import ErrorDescriptor from shoppingcart.models import DonationConfiguration, CourseRegistrationCode -from openedx.core.djangoapps.user_api.api import profile as profile_api from embargo import api as embargo_api @@ -649,6 +647,9 @@ def dashboard(request): # Re-alphabetize language options language_options.sort() + # TODO: remove circular dependency on openedx from common + from openedx.core.djangoapps.user_api.models import UserPreference + # try to get the prefered language for the user cur_pref_lang_code = UserPreference.get_preference(request.user, LANGUAGE_KEY) # try and get the current language of the user @@ -813,6 +814,10 @@ def try_change_enrollment(request): def _update_email_opt_in(request, username, org): """Helper function used to hit the profile API if email opt-in is enabled.""" + + # TODO: remove circular dependency on openedx from common + from openedx.core.djangoapps.user_api.api import profile as profile_api + email_opt_in = request.POST.get('email_opt_in') if email_opt_in is not None: email_opt_in_boolean = email_opt_in == 'true' @@ -1401,6 +1406,9 @@ def _do_create_account(form): log.exception("UserProfile creation failed for user {id}.".format(id=user.id)) raise + # TODO: remove circular dependency on openedx from common + from openedx.core.djangoapps.user_api.models import UserPreference + UserPreference.set_preference(user, LANGUAGE_KEY, get_language()) return (user, profile, registration) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 6701ee372f..28081c1b8b 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -103,8 +103,6 @@ from . import provider # `AUTH_ENROLL_COURSE_ID_KEY` provides the course ID that a student # is trying to enroll in, used to generate analytics events # and auto-enroll students. -from openedx.core.djangoapps.user_api.api import profile - AUTH_ENTRY_KEY = 'auth_entry' AUTH_REDIRECT_KEY = 'next' AUTH_ENROLL_COURSE_ID_KEY = 'enroll_course_id' @@ -671,6 +669,8 @@ def change_enrollment(strategy, user=None, is_dashboard=False, *args, **kwargs): # If the email opt in parameter is found, set the preference. email_opt_in = strategy.session_get(AUTH_EMAIL_OPT_IN_KEY) if email_opt_in: + # 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) diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 4bd7108c89..31e8053a26 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -13,15 +13,14 @@ from datetime import timedelta, datetime import ddt from django.test.client import Client from django.test import TestCase -from django.test.utils import override_settings from django.conf import settings from django.core.urlresolvers import reverse from django.core.exceptions import ObjectDoesNotExist from django.core import mail from bs4 import BeautifulSoup -from openedx.core.djangoapps.user_api.api import profile as profile_api -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config +from openedx.core.djangoapps.user_api.accounts.views import AccountView +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum @@ -1057,7 +1056,7 @@ class TestSubmitPhotosForVerification(TestCase): self.assertEqual(attempt.status, "submitted") # Verify that the user's name wasn't changed - self._assert_full_name(self.user.profile.name) + self._assert_user_name(self.user.profile.name) def test_submit_photos_and_change_name(self): # Submit the photos, along with a name change @@ -1068,7 +1067,7 @@ class TestSubmitPhotosForVerification(TestCase): ) # Check that the user's name was changed in the database - self._assert_full_name(self.FULL_NAME) + self._assert_user_name(self.FULL_NAME) @ddt.data('face_image', 'photo_id_image') def test_invalid_image_data(self, invalid_param): @@ -1140,8 +1139,8 @@ class TestSubmitPhotosForVerification(TestCase): return response - def _assert_full_name(self, full_name): - """Check the user's full name. + def _assert_user_name(self, full_name): + """Check the user's name. Arguments: full_name (unicode): The user's full name. @@ -1150,8 +1149,8 @@ class TestSubmitPhotosForVerification(TestCase): AssertionError """ - info = profile_api.profile_info(self.user.username) - self.assertEqual(info['full_name'], full_name) + account_settings = AccountView.get_serialized_account(self.user.username) + self.assertEqual(account_settings['name'], full_name) class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 0feb69944a..a245b30658 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -27,7 +27,6 @@ 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.api import profile as profile_api from openedx.core.djangoapps.user_api.accounts.views import AccountView from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountUpdateError @@ -744,20 +743,20 @@ def submit_photos_for_verification(request): attempt.mark_ready() attempt.submit() - profile_dict = profile_api.profile_info(username) - if profile_dict: - # Send a confirmation email to the user - context = { - 'full_name': profile_dict.get('full_name'), - 'platform_name': settings.PLATFORM_NAME - } + account_settings = AccountView.get_serialized_account(username) - subject = _("Verification photos received") - message = render_to_string('emails/photo_submission_confirmation.txt', context) - from_address = microsite.get_value('default_from_email', settings.DEFAULT_FROM_EMAIL) - to_address = profile_dict.get('email') + # Send a confirmation email to the user + context = { + 'full_name': account_settings['name'], + 'platform_name': settings.PLATFORM_NAME + } - send_mail(subject, message, from_address, [to_address], fail_silently=False) + subject = _("Verification photos received") + message = render_to_string('emails/photo_submission_confirmation.txt', context) + from_address = microsite.get_value('default_from_email', settings.DEFAULT_FROM_EMAIL) + to_address = account_settings['email'] + + send_mail(subject, message, from_address, [to_address], fail_silently=False) return HttpResponse(200) diff --git a/lms/envs/common.py b/lms/envs/common.py index fdf7b6aaef..7b5e5f68a1 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2047,3 +2047,26 @@ PDF_RECEIPT_COBRAND_LOGO_HEIGHT_MM = 12 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 = { + # Default visibility level for accounts without a specified value + # The value is one of: 'all_users', 'private' + "default_visibility": 'all_users', + + # The list of all fields that can be shown on a learner's profile + "all_fields": [ + 'username', + 'profile_image', + 'country', + 'time_zone', + 'languages', + 'bio', + ], + + # The list of fields that are always public on a learner's profile + "public_fields": [ + 'username', + 'profile_image', + ], +} diff --git a/lms/urls.py b/lms/urls.py index 2bd3d051ef..99d11f5392 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -60,9 +60,11 @@ urlpatterns = ( url(r'^heartbeat$', include('heartbeat.urls')), - url(r'^user_api/', include('openedx.core.djangoapps.user_api.urls')), + url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')), - url(r'^api/user/', include('openedx.core.djangoapps.user_api.accounts.urls')), + # Note: these are older versions of the User API that will eventually be + # subsumed by api/user. + url(r'^user_api/', include('openedx.core.djangoapps.user_api.legacy_urls')), url(r'^notifier_api/', include('notifier_api.urls')), diff --git a/openedx/core/djangoapps/user_api/accounts/tests/__init__.py b/openedx/core/djangoapps/user_api/accounts/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 1d58b60fe1..cec245f01d 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -2,32 +2,81 @@ import unittest import ddt import json -from datetime import datetime -from django.test import TestCase -from django.core.urlresolvers import reverse from django.conf import settings +from django.core.urlresolvers import reverse from rest_framework.test import APITestCase, APIClient from student.tests.factories import UserFactory from student.models import UserProfile, PendingEmailChange -from student.views import confirm_email_change TEST_PASSWORD = "test" -@ddt.ddt -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class TestAccountAPI(APITestCase): - +class UserAPITestCase(APITestCase): + """ + The base class for all tests of the User API + """ def setUp(self): - super(TestAccountAPI, self).setUp() + super(UserAPITestCase, self).setUp() + self.anonymous_client = APIClient() self.different_user = UserFactory.create(password=TEST_PASSWORD) self.different_client = APIClient() self.staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD) self.staff_client = APIClient() self.user = UserFactory.create(password=TEST_PASSWORD) + + def login_client(self, api_client, user): + """Helper method for getting the client and user and logging in. Returns client. """ + client = getattr(self, api_client) + user = getattr(self, user) + client.login(username=user.username, password=TEST_PASSWORD) + return client + + def send_patch(self, client, json_data, content_type="application/merge-patch+json", expected_status=204): + """ + Helper method for sending a patch to the server, defaulting to application/merge-patch+json content_type. + Verifies the expected status and returns the response. + """ + # pylint: disable=no-member + response = client.patch(self.url, data=json.dumps(json_data), content_type=content_type) + self.assertEqual(expected_status, response.status_code) + return response + + def send_get(self, client, query_parameters=None, expected_status=200): + """ + Helper method for sending a GET to the server. Verifies the expected status and returns the response. + """ + url = self.url + '?' + query_parameters if query_parameters else self.url # pylint: disable=no-member + response = client.get(url) + self.assertEqual(expected_status, response.status_code) + return response + + def create_mock_profile(self, user): + """ + Helper method that creates a mock profile for the specified user + :return: + """ + legacy_profile = UserProfile.objects.get(id=user.id) + legacy_profile.country = "US" + legacy_profile.level_of_education = "m" + legacy_profile.year_of_birth = 1900 + legacy_profile.goals = "world peace" + legacy_profile.mailing_address = "Park Ave" + legacy_profile.save() + + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +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): @@ -87,15 +136,7 @@ class TestAccountAPI(APITestCase): 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. """ - # Create some test profile values. - legacy_profile = UserProfile.objects.get(id=self.user.id) - legacy_profile.country = "US" - legacy_profile.level_of_education = "m" - legacy_profile.year_of_birth = 1900 - legacy_profile.goals = "world peace" - legacy_profile.mailing_address = "Park Ave" - legacy_profile.save() - + self.create_mock_profile(self.user) client = self.login_client(api_client, user) response = self.send_get(client) data = response.data @@ -343,27 +384,3 @@ class TestAccountAPI(APITestCase): error_response.data["developer_message"] ) self.assertEqual("Valid e-mail address required.", error_response.data["user_message"]) - - def login_client(self, api_client, user): - """Helper method for getting the client and user and logging in. Returns client. """ - client = getattr(self, api_client) - user = getattr(self, user) - client.login(username=user.username, password=TEST_PASSWORD) - return client - - def send_patch(self, client, json_data, content_type="application/merge-patch+json", expected_status=204): - """ - Helper method for sending a patch to the server, defaulting to application/merge-patch+json content_type. - Verifies the expected status and returns the response. - """ - response = client.patch(self.url, data=json.dumps(json_data), content_type=content_type) - self.assertEqual(expected_status, response.status_code) - return response - - def send_get(self, client, expected_status=200): - """ - Helper method for sending a GET to the server. Verifies the expected status and returns the response. - """ - response = client.get(self.url) - self.assertEqual(expected_status, response.status_code) - return response diff --git a/openedx/core/djangoapps/user_api/accounts/urls.py b/openedx/core/djangoapps/user_api/accounts/urls.py deleted file mode 100644 index 05d94ec7a2..0000000000 --- a/openedx/core/djangoapps/user_api/accounts/urls.py +++ /dev/null @@ -1,14 +0,0 @@ -from .views import AccountView - -from django.conf.urls import include, patterns, url - -USERNAME_PATTERN = r'(?P[\w.+-]+)' - -urlpatterns = patterns( - '', - url( - r'^v0/accounts/' + USERNAME_PATTERN + '$', - AccountView.as_view(), - name="accounts_api" - ) -) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 78c878d229..358ed3cb36 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -15,15 +15,13 @@ from rest_framework.response import Response from rest_framework import status from rest_framework.authentication import OAuth2Authentication, SessionAuthentication from rest_framework import permissions -from rest_framework import parsers +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.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 openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH -from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer, AccountUserSerializer -from openedx.core.lib.api.permissions import IsUserInUrlOrStaff -from openedx.core.lib.api.parsers import MergePatchParser -from openedx.core.djangoapps.user_api.api.account import AccountUserNotFound, AccountUpdateError class AccountView(APIView): @@ -89,14 +87,34 @@ class AccountView(APIView): GET /api/user/v0/accounts/{username}/ """ try: - existing_user, existing_user_profile = self._get_user_and_profile(username) + account_settings = AccountView.get_serialized_account(username) 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 Response(dict(user_serializer.data, **legacy_profile_serializer.data)) + return dict(user_serializer.data, **legacy_profile_serializer.data) def patch(self, request, username): """ diff --git a/openedx/core/djangoapps/user_api/api/profile.py b/openedx/core/djangoapps/user_api/api/profile.py index 2697e2b499..28de57068c 100644 --- a/openedx/core/djangoapps/user_api/api/profile.py +++ b/openedx/core/djangoapps/user_api/api/profile.py @@ -14,7 +14,8 @@ from pytz import UTC import analytics from eventtracking import tracker -from ..models import User, UserProfile, UserPreference, UserOrgTag +from ..accounts.views import AccountView +from ..models import User, UserPreference, UserOrgTag from ..helpers import intercept_errors log = logging.getLogger(__name__) @@ -30,62 +31,11 @@ class ProfileUserNotFound(ProfileRequestError): pass -class ProfileInvalidField(ProfileRequestError): - """ The proposed value for a field is not in a valid format. """ - - def __init__(self, field, value): - super(ProfileInvalidField, self).__init__() - self.field = field - self.value = value - - def __str__(self): - return u"Invalid value '{value}' for profile field '{field}'".format( - value=self.value, - field=self.field - ) - - class ProfileInternalError(Exception): """ An error occurred in an API call. """ pass -@intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError]) -def profile_info(username): - """Retrieve a user's profile information. - - Searches either by username or email. - - At least one of the keyword args must be provided. - - Arguments: - username (unicode): The username of the account to retrieve. - - Returns: - dict: If profile information was found. - None: If the provided username did not match any profiles. - - """ - try: - profile = UserProfile.objects.get(user__username=username) - except UserProfile.DoesNotExist: - return None - - profile_dict = { - "username": profile.user.username, - "email": profile.user.email, - "full_name": profile.name, - "level_of_education": profile.level_of_education, - "mailing_address": profile.mailing_address, - "year_of_birth": profile.year_of_birth, - "goals": profile.goals, - "city": profile.city, - "country": unicode(profile.country), - } - - return profile_dict - - @intercept_errors(ProfileInternalError, ignore_errors=[ProfileRequestError]) def preference_info(username): """Retrieve information about a user's preferences. @@ -151,22 +101,19 @@ def update_email_opt_in(username, org, optin): None Raises: - ProfileUserNotFound: Raised when the username specified is not associated with a user. + AccountUserNotFound: Raised when the username specified is not associated with a user. """ - try: - user = User.objects.get(username=username) - except User.DoesNotExist: - raise ProfileUserNotFound - - profile = UserProfile.objects.get(user=user) + account_settings = AccountView.get_serialized_account(username) + year_of_birth = account_settings['year_of_birth'] of_age = ( - profile.year_of_birth is None or # If year of birth is not set, we assume user is of age. - datetime.datetime.now(UTC).year - profile.year_of_birth > # pylint: disable=maybe-no-member + year_of_birth is None or # If year of birth is not set, we assume user is of age. + datetime.datetime.now(UTC).year - year_of_birth > # pylint: disable=maybe-no-member getattr(settings, 'EMAIL_OPTIN_MINIMUM_AGE', 13) ) try: + user = User.objects.get(username=username) preference, _ = UserOrgTag.objects.get_or_create( user=user, org=org, key='email-optin' ) diff --git a/openedx/core/djangoapps/user_api/legacy_urls.py b/openedx/core/djangoapps/user_api/legacy_urls.py new file mode 100644 index 0000000000..3bfb1af025 --- /dev/null +++ b/openedx/core/djangoapps/user_api/legacy_urls.py @@ -0,0 +1,43 @@ +""" +Defines the URL routes for this app. +""" + +from django.conf import settings +from django.conf.urls import include, patterns, url +from rest_framework import routers + +from . import views as user_api_views +from .models import UserPreference + +USER_API_ROUTER = routers.DefaultRouter() +USER_API_ROUTER.register(r'users', user_api_views.UserViewSet) +USER_API_ROUTER.register(r'user_prefs', user_api_views.UserPreferenceViewSet) +urlpatterns = patterns( + '', + url(r'^v1/', include(USER_API_ROUTER.urls)), + url( + r'^v1/preferences/(?P{})/users/$'.format(UserPreference.KEY_REGEX), + user_api_views.PreferenceUsersListView.as_view() + ), + url( + r'^v1/forum_roles/(?P[a-zA-Z]+)/users/$', + user_api_views.ForumRoleUsersListView.as_view() + ), + + url( + r'^v1/preferences/email_opt_in/$', + user_api_views.UpdateEmailOptInPreference.as_view(), + name="preferences_email_opt_in" + ), +) + +if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): + urlpatterns += patterns( + '', + url(r'^v1/account/login_session/$', user_api_views.LoginSessionView.as_view(), + name="user_api_login_session"), + url(r'^v1/account/registration/$', user_api_views.RegistrationView.as_view(), + name="user_api_registration"), + url(r'^v1/account/password_reset/$', user_api_views.PasswordResetView.as_view(), + name="user_api_password_reset"), + ) diff --git a/openedx/core/djangoapps/user_api/profiles/__init__.py b/openedx/core/djangoapps/user_api/profiles/__init__.py new file mode 100644 index 0000000000..b3d5698513 --- /dev/null +++ b/openedx/core/djangoapps/user_api/profiles/__init__.py @@ -0,0 +1,11 @@ +""" +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' diff --git a/openedx/core/djangoapps/user_api/profiles/tests/__init__.py b/openedx/core/djangoapps/user_api/profiles/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/user_api/profiles/tests/test_views.py b/openedx/core/djangoapps/user_api/profiles/tests/test_views.py new file mode 100644 index 0000000000..8341988d74 --- /dev/null +++ b/openedx/core/djangoapps/user_api/profiles/tests/test_views.py @@ -0,0 +1,112 @@ +""" +Unit tests for profile APIs. +""" + +import ddt +import unittest + +from django.conf import settings +from django.core.urlresolvers import reverse + +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 + 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 + 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) diff --git a/openedx/core/djangoapps/user_api/profiles/views.py b/openedx/core/djangoapps/user_api/profiles/views.py new file mode 100644 index 0000000000..2d4038e276 --- /dev/null +++ b/openedx/core/djangoapps/user_api/profiles/views.py @@ -0,0 +1,123 @@ +""" +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') diff --git a/openedx/core/djangoapps/user_api/tests/test_profile_api.py b/openedx/core/djangoapps/user_api/tests/test_profile_api.py index 7ad09a5ec7..c5cab80744 100644 --- a/openedx/core/djangoapps/user_api/tests/test_profile_api.py +++ b/openedx/core/djangoapps/user_api/tests/test_profile_api.py @@ -11,6 +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 ..api import account as account_api from ..api import profile as profile_api from ..models import UserProfile, UserOrgTag @@ -27,24 +28,27 @@ class ProfileApiTest(ModuleStoreTestCase): # Create a new account, which should have an empty profile by default. account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) - # Retrieve the profile, expecting default values - profile = profile_api.profile_info(username=self.USERNAME) - self.assertEqual(profile, { + # Retrieve the account settings + account_settings = AccountView.get_serialized_account(self.USERNAME) + + # Expect a date joined field but remove it to simplify the following comparison + self.assertIsNotNone(account_settings['date_joined']) + del account_settings['date_joined'] + + # Expect all the values to be defaulted + self.assertEqual(account_settings, { 'username': self.USERNAME, 'email': self.EMAIL, - 'full_name': u'', + 'name': u'', + 'gender': None, + 'language': u'', 'goals': None, 'level_of_education': None, 'mailing_address': None, 'year_of_birth': None, - 'country': '', - 'city': None, + 'country': None, }) - def test_retrieve_profile_no_user(self): - profile = profile_api.profile_info('does not exist') - self.assertIs(profile, None) - def test_update_and_retrieve_preference_info(self): account_api.create_account(self.USERNAME, self.PASSWORD, self.EMAIL) diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 8f43b8a97b..c9c841527e 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -10,7 +10,7 @@ from django.core.urlresolvers import reverse from django.core import mail from django.test import TestCase from django.test.utils import override_settings -from unittest import SkipTest, skipUnless +from unittest import skipUnless import ddt from pytz import UTC import mock @@ -23,6 +23,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 ..api import account as account_api, profile as profile_api from ..models import UserOrgTag from ..tests.factories import UserPreferenceFactory @@ -1247,8 +1248,8 @@ class RegistrationViewTest(ApiTestCase): ) # Verify that the user's full name is set - profile_info = profile_api.profile_info(self.USERNAME) - self.assertEqual(profile_info["full_name"], self.NAME) + account_settings = AccountView.get_serialized_account(self.USERNAME) + self.assertEqual(account_settings["name"], self.NAME) # Verify that we've been logged in # by trying to access a page that requires authentication @@ -1261,7 +1262,6 @@ class RegistrationViewTest(ApiTestCase): "year_of_birth": "optional", "mailing_address": "optional", "goals": "optional", - "city": "optional", "country": "required", }) def test_register_with_profile_info(self): @@ -1275,20 +1275,18 @@ class RegistrationViewTest(ApiTestCase): "mailing_address": self.ADDRESS, "year_of_birth": self.YEAR_OF_BIRTH, "goals": self.GOALS, - "city": self.CITY, "country": self.COUNTRY, "honor_code": "true", }) self.assertHttpOK(response) - # Verify the profile information - profile_info = profile_api.profile_info(self.USERNAME) - self.assertEqual(profile_info["level_of_education"], self.EDUCATION) - self.assertEqual(profile_info["mailing_address"], self.ADDRESS) - self.assertEqual(profile_info["year_of_birth"], int(self.YEAR_OF_BIRTH)) - self.assertEqual(profile_info["goals"], self.GOALS) - self.assertEqual(profile_info["city"], self.CITY) - self.assertEqual(profile_info["country"], self.COUNTRY) + # Verify the user's account + account_settings = AccountView.get_serialized_account(self.USERNAME) + 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)) + self.assertEqual(account_settings["goals"], self.GOALS) + self.assertEqual(account_settings["country"], self.COUNTRY) def test_activation_email(self): # Register, which should trigger an activation email diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 3b840cc6b5..795deffb73 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -2,40 +2,23 @@ Defines the URL routes for this app. """ -from django.conf import settings -from django.conf.urls import include, patterns, url -from rest_framework import routers +from .accounts.views import AccountView +from .profiles.views import ProfileView -from . import views as user_api_views -from .models import UserPreference +from django.conf.urls import patterns, url +USERNAME_PATTERN = r'(?P[\w.+-]+)' -USER_API_ROUTER = routers.DefaultRouter() -USER_API_ROUTER.register(r'users', user_api_views.UserViewSet) -USER_API_ROUTER.register(r'user_prefs', user_api_views.UserPreferenceViewSet) urlpatterns = patterns( '', - url(r'^v1/', include(USER_API_ROUTER.urls)), url( - r'^v1/preferences/(?P{})/users/$'.format(UserPreference.KEY_REGEX), - user_api_views.PreferenceUsersListView.as_view() + r'^v0/accounts/' + USERNAME_PATTERN + '$', + AccountView.as_view(), + name="accounts_api" ), url( - r'^v1/forum_roles/(?P[a-zA-Z]+)/users/$', - user_api_views.ForumRoleUsersListView.as_view() - ), - - url( - r'^v1/preferences/email_opt_in/$', - user_api_views.UpdateEmailOptInPreference.as_view(), - name="preferences_email_opt_in" + r'^v0/profiles/' + USERNAME_PATTERN + '$', + ProfileView.as_view(), + name="profiles_api" ), ) - -if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): - urlpatterns += patterns( - '', - url(r'^v1/account/login_session/$', user_api_views.LoginSessionView.as_view(), name="user_api_login_session"), - url(r'^v1/account/registration/$', user_api_views.RegistrationView.as_view(), name="user_api_registration"), - url(r'^v1/account/password_reset/$', user_api_views.PasswordResetView.as_view(), name="user_api_password_reset"), - )