Integrate profile images into Accounts API

TNL-1545

cms/envs/common.py

@@ -36,7 +36,12 @@ import lms.envs.common
 # Although this module itself may not use these imported variables, other dependent modules may.
 from lms.envs.common import (
     USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, BUGS_EMAIL, DOC_STORE_CONFIG, DATA_DIR, ALL_LANGUAGES, WIKI_ENABLED,
-    update_module_store_settings, ASSET_IGNORE_REGEX, COPYRIGHT_YEAR
+    update_module_store_settings, ASSET_IGNORE_REGEX, COPYRIGHT_YEAR,
+    # The following PROFILE_IMAGE_* settings are included as they are
+    # indirectly accessed through the email opt-in API, which is
+    # technically accessible through the CMS via legacy URLs.
+    PROFILE_IMAGE_BACKEND, PROFILE_IMAGE_DOMAIN, PROFILE_IMAGE_URL_PATH, PROFILE_IMAGE_DEFAULT_FILENAME,
+    PROFILE_IMAGE_SECRET_KEY
 )
 from path import path
 from warnings import simplefilter

common/djangoapps/student/models.py

@@ -250,6 +250,7 @@ class UserProfile(models.Model):
     goals = models.TextField(blank=True, null=True)
     allow_certificate = models.BooleanField(default=1)
     bio = models.TextField(blank=True, null=True)
+    has_profile_image = models.BooleanField(default=0)
 
     def get_meta(self):  # pylint: disable=missing-docstring
         js_str = self.meta

lms/envs/common.py

@@ -318,7 +318,7 @@ FEATURES = {
     'ENABLE_COURSE_SORTING_BY_START_DATE': True,
 
     # Flag to enable new user account APIs.
-    'ENABLE_USER_REST_API': False,
+    'ENABLE_USER_REST_API': True,
 
     # Expose Mobile REST API. Note that if you use this, you must also set
     # ENABLE_OAUTH2_PROVIDER to True
@@ -2248,3 +2248,20 @@ CHECKPOINT_PATTERN = r'(?P<checkpoint_name>\w+)'
 # 'courseware.student_field_overrides.IndividualStudentOverrideProvider' to
 # this setting.
 FIELD_OVERRIDE_PROVIDERS = ()
+
+# PROFILE IMAGE CONFIG
+# TODO: add these settings to aws.py as well
+PROFILE_IMAGE_BACKEND = 'django.core.files.storage.FileSystemStorage'
+# PROFILE_IMAGE_DOMAIN points to the domain from which we serve image
+# files from.  When this is '/', it refers to the same domain as the
+# app server.  If serving from a different domain, specify that here
+# i.e. 'http://www.example-image-server.com/'
+PROFILE_IMAGE_DOMAIN = '/'
+PROFILE_IMAGE_URL_PATH = 'media/profile_images/'
+PROFILE_IMAGE_DEFAULT_FILENAME = 'default_profile_image'  # TODO: determine final name
+# This secret key is used in generating unguessable URLs to users'
+# profile images.  Once it has been set, changing it will make the
+# platform unaware of current image URLs, resulting in reverting all
+# users' profile images to the default placeholder image.
+PROFILE_IMAGE_SECRET_KEY = 'placeholder secret key'
+

lms/envs/test.py

@@ -473,3 +473,10 @@ FEATURES['CERTIFICATES_HTML_VIEW'] = True
 INSTALLED_APPS += ('ccx',)
 MIDDLEWARE_CLASSES += ('ccx.overrides.CcxMiddleware',)
 FEATURES['CUSTOM_COURSES_EDX'] = True
+
+# Set dummy values for profile image settings.
+PROFILE_IMAGE_BACKEND = 'django.core.files.storage.FileSystemStorage'
+PROFILE_IMAGE_DOMAIN = 'http://example-storage.com/'
+PROFILE_IMAGE_URL_PATH = 'profile_images/'
+PROFILE_IMAGE_DEFAULT_FILENAME = 'default'
+PROFILE_IMAGE_SECRET_KEY = 'secret'

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

@@ -144,7 +144,7 @@ def update_account_settings(requesting_user, update, username=None):
 
     # 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
+        AccountUserSerializer.get_read_only_fields() + AccountLegacyProfileSerializer.get_read_only_fields()
     )
 
     # Build up all field errors, whether read-only, validation, or email errors.

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

+"""
+Helper functions for the accounts API.
+"""
+import hashlib
+
+from django.conf import settings
+from django.core.files.storage import FileSystemStorage, get_storage_class
+
+PROFILE_IMAGE_SIZES_MAP = {
+    'full': 500,
+    'large': 120,
+    'medium': 50,
+    'small': 30
+}
+_PROFILE_IMAGE_SIZES = PROFILE_IMAGE_SIZES_MAP.values()
+PROFILE_IMAGE_FORMAT = 'jpg'
+
+
+def get_profile_image_url_for_user(user, size):
+    """Return the URL to a user's profile image for a given size.
+    Note that based on the value of
+    django.conf.settings.PROFILE_IMAGE_DOMAIN, the URL may be relative,
+    and in that case the caller is responsible for constructing the full
+    URL.
+
+    If the user has not yet uploaded a profile image, return the URL to
+    the default edX user profile image.
+
+    Arguments:
+        user (django.auth.User): The user for whom we're generating a
+        profile image URL.
+
+    Returns:
+        string: The URL for the user's profile image.
+
+    Raises:
+        ValueError: The caller asked for an unsupported image size.
+    """
+    if size not in _PROFILE_IMAGE_SIZES:
+        raise ValueError('Unsupported profile image size: {size}'.format(size=size))
+
+    if user.profile.has_profile_image:
+        name = hashlib.md5(settings.PROFILE_IMAGE_SECRET_KEY + user.username).hexdigest()
+    else:
+        name = settings.PROFILE_IMAGE_DEFAULT_FILENAME
+
+    filename = '{name}_{size}.{format}'.format(name=name, size=size, format=PROFILE_IMAGE_FORMAT)
+
+    # Note that, for now, the backend will be FileSystemStorage.  When
+    # we eventually support s3 storage, we'll need to pass a parameter
+    # to the storage class indicating the s3 bucket which we're using
+    # for profile picture uploads.
+    storage_class = get_storage_class(settings.PROFILE_IMAGE_BACKEND)
+    if storage_class == FileSystemStorage:
+        kwargs = {'base_url': (settings.PROFILE_IMAGE_DOMAIN + settings.PROFILE_IMAGE_URL_PATH)}
+    storage = storage_class(**kwargs)
+    return storage.url(filename)

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

 from rest_framework import serializers
 from django.contrib.auth.models import User
-from student.models import UserProfile
 from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH
+from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin
+
+from student.models import UserProfile
+from .helpers import get_profile_image_url_for_user, PROFILE_IMAGE_SIZES_MAP
 
+PROFILE_IMAGE_KEY_PREFIX = 'image_url'
 
-class AccountUserSerializer(serializers.HyperlinkedModelSerializer):
+
+class AccountUserSerializer(serializers.HyperlinkedModelSerializer, ReadOnlyFieldsSerializerMixin):
     """
     Class that serializes the portion of User model needed for account information.
     """
@@ -12,20 +17,24 @@ class AccountUserSerializer(serializers.HyperlinkedModelSerializer):
         model = User
         fields = ("username", "email", "date_joined", "is_active")
         read_only_fields = ("username", "email", "date_joined", "is_active")
+        explicit_read_only_fields = ()
 
 
-class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer):
+class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, ReadOnlyFieldsSerializerMixin):
     """
     Class that serializes the portion of UserProfile model needed for account information.
     """
+    profile_image = serializers.SerializerMethodField("get_profile_image")
+
     class Meta:
         model = UserProfile
         fields = (
             "name", "gender", "goals", "year_of_birth", "level_of_education", "language", "country",
-            "mailing_address", "bio"
+            "mailing_address", "bio", "profile_image"
         )
         # Currently no read-only field, but keep this so view code doesn't need to know.
         read_only_fields = ()
+        explicit_read_only_fields = ("profile_image",)
 
     def validate_name(self, attrs, source):
         """ Enforce minimum length for name. """
@@ -55,3 +64,13 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer):
     def convert_empty_to_None(value):
         """ Helper method to convert empty string to None (other values pass through). """
         return None if value == "" else value
+
+    def get_profile_image(self, obj):
+        """ Returns metadata about a user's profile image. """
+        data = {'has_image': obj.has_profile_image}
+        data.update({
+            '{image_key_prefix}_{size}'.format(image_key_prefix=PROFILE_IMAGE_KEY_PREFIX, size=size_display_name):
+            get_profile_image_url_for_user(obj.user, size_value)
+            for size_display_name, size_value in PROFILE_IMAGE_SIZES_MAP.items()
+        })
+        return data

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

@@ -117,6 +117,12 @@ class TestAccountApi(TestCase):
         with self.assertRaises(AccountValidationError):
             update_account_settings(self.user, {"gender": "undecided"})
 
+        with self.assertRaises(AccountValidationError):
+            update_account_settings(
+                self.user,
+                {"profile_image": {"has_image": "not_allowed", "image_url": "not_allowed"}}
+            )
+
     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.

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

+"""
+Tests for helpers.py
+"""
+from ddt import ddt, data
+import hashlib
+from mock import patch
+from unittest import skipUnless
+
+from django.conf import settings
+from django.test import TestCase
+
+from openedx.core.djangoapps.user_api.accounts.helpers import get_profile_image_url_for_user
+from student.tests.factories import UserFactory
+
+
+@ddt
+@patch('openedx.core.djangoapps.user_api.accounts.helpers._PROFILE_IMAGE_SIZES', [50, 10])
+@patch.dict(
+    'openedx.core.djangoapps.user_api.accounts.helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
+)
+@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+class ProfileImageUrlTestCase(TestCase):
+    """
+    Tests for `get_profile_image_url_for_user`.
+    """
+    def setUp(self):
+        super(ProfileImageUrlTestCase, self).setUp()
+        self.user = UserFactory()
+
+    def verify_url(self, user, pixels, filename):
+        """
+        Helper method to verify that we're correctly generating profile
+        image URLs.
+        """
+        self.assertEqual(
+            get_profile_image_url_for_user(user, pixels),
+            'http://example-storage.com/profile_images/{filename}_{pixels}.jpg'.format(filename=filename, pixels=pixels)
+        )
+
+    @data(10, 50)
+    def test_profile_image_urls(self, pixels):
+        """
+        Verify we get the URL to the default image if the user does not
+        have a profile image.
+        """
+        # By default new users will have no profile image.
+        self.verify_url(self.user, pixels, 'default')
+        # A user can add an image, then remove it.  We should get the
+        # default image URL in that case.
+        self.user.profile.has_profile_image = True
+        self.user.profile.save()
+        self.verify_url(self.user, pixels, hashlib.md5('secret' + self.user.username).hexdigest())
+        self.user.profile.has_profile_image = False
+        self.user.profile.save()
+        self.verify_url(self.user, pixels, 'default')
+
+    @data(1, 5000)
+    def test_unsupported_sizes(self, image_size):
+        """
+        Verify that we cannot ask for image sizes which are unsupported.
+        """
+        with self.assertRaises(ValueError):
+            get_profile_image_url_for_user(self.user, image_size)

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

 # -*- coding: utf-8 -*-
-import unittest
 import ddt
+import hashlib
 import json
 from mock import patch
+import unittest
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.test.testcases import TransactionTestCase
+from django.test.utils import override_settings
 from rest_framework.test import APITestCase, APIClient
 
 from student.tests.factories import UserFactory
@@ -86,11 +88,16 @@ class UserAPITestCase(APITestCase):
         legacy_profile.goals = "world peace"
         legacy_profile.mailing_address = "Park Ave"
         legacy_profile.bio = "Tired mother of twins"
+        legacy_profile.has_profile_image = True
         legacy_profile.save()
 
 
 @ddt.ddt
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
+@patch('openedx.core.djangoapps.user_api.accounts.helpers._PROFILE_IMAGE_SIZES', [50, 10])
+@patch.dict(
+    'openedx.core.djangoapps.user_api.accounts.helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
+)
 class TestAccountAPI(UserAPITestCase):
     """
     Unit tests for the Account API.
@@ -100,6 +107,25 @@ class TestAccountAPI(UserAPITestCase):
 
         self.url = reverse("accounts_api", kwargs={'username': self.user.username})
 
+    def _verify_profile_image_data(self, data, has_profile_image):
+        """
+        Verify the profile image data in a GET response for self.user
+        corresponds to whether the user has or hasn't set a profile
+        image.
+        """
+        if has_profile_image:
+            filename = hashlib.md5('secret' + self.user.username).hexdigest()
+        else:
+            filename = 'default'
+        self.assertEqual(
+            data['profile_image'],
+            {
+                'has_image': has_profile_image,
+                'image_url_full': 'http://example-storage.com/profile_images/{}_50.jpg'.format(filename),
+                'image_url_small': 'http://example-storage.com/profile_images/{}_10.jpg'.format(filename)
+            }
+        )
+
     def _verify_full_shareable_account_response(self, response):
         """
         Verify that the shareable fields from the account are returned
@@ -108,7 +134,7 @@ class TestAccountAPI(UserAPITestCase):
         self.assertEqual(6, len(data))
         self.assertEqual(self.user.username, data["username"])
         self.assertEqual("US", data["country"])
-        self.assertIsNone(data["profile_image"])
+        self._verify_profile_image_data(data, True)
         self.assertIsNone(data["time_zone"])
         self.assertIsNone(data["languages"])
         self.assertEqual("Tired mother of twins", data["bio"])
@@ -120,14 +146,14 @@ class TestAccountAPI(UserAPITestCase):
         data = response.data
         self.assertEqual(2, len(data))
         self.assertEqual(self.user.username, data["username"])
-        self.assertIsNone(data["profile_image"])
+        self._verify_profile_image_data(data, True)
 
     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(12, len(data))
+        self.assertEqual(13, 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"])
@@ -141,6 +167,7 @@ class TestAccountAPI(UserAPITestCase):
         self.assertTrue(data["is_active"])
         self.assertIsNotNone(data["date_joined"])
         self.assertEqual("Tired mother of twins", data["bio"])
+        self._verify_profile_image_data(data, True)
 
     def test_anonymous_access(self):
         """
@@ -242,7 +269,7 @@ class TestAccountAPI(UserAPITestCase):
         def verify_get_own_information():
             response = self.send_get(self.client)
             data = response.data
-            self.assertEqual(12, len(data))
+            self.assertEqual(13, len(data))
             self.assertEqual(self.user.username, data["username"])
             self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
             for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"):
@@ -255,6 +282,7 @@ class TestAccountAPI(UserAPITestCase):
             self.assertEqual(self.user.email, data["email"])
             self.assertIsNotNone(data["date_joined"])
             self.assertEqual(self.user.is_active, data["is_active"])
+            self._verify_profile_image_data(data, False)
 
         self.client.login(username=self.user.username, password=self.test_password)
         verify_get_own_information()
@@ -516,6 +544,25 @@ class TestAccountAPI(UserAPITestCase):
             error_response.data["developer_message"]
         )
         self.assertIsNone(error_response.data["user_message"])
+        
+    @override_settings(PROFILE_IMAGE_DOMAIN='/')
+    def test_convert_relative_profile_url(self):
+        """
+        Test that when PROFILE_IMAGE_DOMAIN is set to '/', the API
+        generates the full URL to profile images based on the URL
+        of the request.
+        """
+        self.client.login(username=self.user.username, password=self.test_password)
+        response = self.send_get(self.client)
+        # pylint: disable=no-member
+        self.assertEqual(
+            response.data["profile_image"],
+            {
+                "has_image": False,
+                "image_url_full": "http://testserver/profile_images/default_50.jpg",
+                "image_url_small": "http://testserver/profile_images/default_10.jpg"
+            }
+        )     
 
 
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')

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

@@ -14,6 +14,7 @@ from rest_framework import permissions
 from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError
 from openedx.core.lib.api.parsers import MergePatchParser
 from .api import get_account_settings, update_account_settings
+from .serializers import PROFILE_IMAGE_KEY_PREFIX
 
 
 class AccountView(APIView):
@@ -131,6 +132,10 @@ class AccountView(APIView):
         """
         try:
             account_settings = get_account_settings(request.user, username, view=request.QUERY_PARAMS.get('view'))
+            # Account for possibly relative URLs.
+            for key, value in account_settings['profile_image'].items():
+                if key.startswith(PROFILE_IMAGE_KEY_PREFIX):
+                    account_settings['profile_image'][key] = request.build_absolute_uri(value)
         except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
 

openedx/core/djangoapps/user_api/serializers.py

@@ -39,3 +39,21 @@ class RawUserPreferenceSerializer(serializers.ModelSerializer):
     class Meta:
         model = UserPreference
         depth = 1
+
+
+class ReadOnlyFieldsSerializerMixin(object):
+    """
+    Mixin for use with Serializers that provides a method
+    `get_read_only_fields`, which returns a tuple of all read-only
+    fields on the Serializer.
+    """
+    @classmethod
+    def get_read_only_fields(cls):
+        """
+        Return all fields on this Serializer class which are read-only.
+        Expects sub-classes implement Meta.explicit_read_only_fields,
+        which is a tuple declaring read-only fields which were declared
+        explicitly and thus could not be added to the usual
+        cls.Meta.read_only_fields tuple.
+        """
+        return getattr(cls.Meta, 'read_only_fields', '') + getattr(cls.Meta, 'explicit_read_only_fields', '')

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

@@ -1693,7 +1693,6 @@ class TestGoogleRegistrationView(
     """Tests the User API registration endpoint with Google authentication."""
     pass
 
-
 @ddt.ddt
 class UpdateEmailOptInTestCase(ApiTestCase, ModuleStoreTestCase):
     """Tests the UpdateEmailOptInPreference view. """