From 6f6fdbfb5f24e7b1ef9ddabc3e92b9210c8375d2 Mon Sep 17 00:00:00 2001 From: Daniel Friedman <dfriedman58@gmail.com> Date: Thu, 12 Mar 2015 18:07:56 -0400 Subject: [PATCH] Integrate profile images into Accounts API TNL-1545 --- cms/envs/common.py | 7 +- .../0048_add_has_profile_image_boolean.py | 195 ++++++++++++++++++ common/djangoapps/student/models.py | 1 + lms/envs/common.py | 19 +- lms/envs/test.py | 7 + .../core/djangoapps/user_api/accounts/api.py | 2 +- .../djangoapps/user_api/accounts/helpers.py | 57 +++++ .../user_api/accounts/serializers.py | 27 ++- .../user_api/accounts/tests/test_api.py | 6 + .../user_api/accounts/tests/test_helpers.py | 63 ++++++ .../user_api/accounts/tests/test_views.py | 57 ++++- .../djangoapps/user_api/accounts/views.py | 5 + .../core/djangoapps/user_api/serializers.py | 18 ++ .../djangoapps/user_api/tests/test_views.py | 1 - 14 files changed, 452 insertions(+), 13 deletions(-) create mode 100644 common/djangoapps/student/migrations/0048_add_has_profile_image_boolean.py create mode 100644 openedx/core/djangoapps/user_api/accounts/helpers.py create mode 100644 openedx/core/djangoapps/user_api/accounts/tests/test_helpers.py diff --git a/cms/envs/common.py b/cms/envs/common.py index f9c7d84065e..6aff9c2546f 100644 --- a/cms/envs/common.py +++ b/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 diff --git a/common/djangoapps/student/migrations/0048_add_has_profile_image_boolean.py b/common/djangoapps/student/migrations/0048_add_has_profile_image_boolean.py new file mode 100644 index 00000000000..a5f3ebf4a11 --- /dev/null +++ b/common/djangoapps/student/migrations/0048_add_has_profile_image_boolean.py @@ -0,0 +1,195 @@ +# -*- coding: utf-8 -*- +from south.utils import datetime_utils as datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding field 'UserProfile.has_profile_image' + db.add_column('auth_userprofile', 'has_profile_image', + self.gf('django.db.models.fields.BooleanField')(default=False), + keep_default=False) + + + def backwards(self, orm): + # Deleting field 'UserProfile.has_profile_image' + db.delete_column('auth_userprofile', 'has_profile_image') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'student.anonymoususerid': { + 'Meta': {'object_name': 'AnonymousUserId'}, + 'anonymous_user_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32'}), + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.courseaccessrole': { + 'Meta': {'unique_together': "(('user', 'org', 'course_id', 'role'),)", 'object_name': 'CourseAccessRole'}, + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'org': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '64', 'blank': 'True'}), + 'role': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.courseenrollment': { + 'Meta': {'ordering': "('user', 'course_id')", 'unique_together': "(('user', 'course_id'),)", 'object_name': 'CourseEnrollment'}, + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '100'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.courseenrollmentallowed': { + 'Meta': {'unique_together': "(('email', 'course_id'),)", 'object_name': 'CourseEnrollmentAllowed'}, + 'auto_enroll': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}) + }, + 'student.dashboardconfiguration': { + 'Meta': {'object_name': 'DashboardConfiguration'}, + 'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}), + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'recent_enrollment_time_delta': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}) + }, + 'student.entranceexamconfiguration': { + 'Meta': {'unique_together': "(('user', 'course_id'),)", 'object_name': 'EntranceExamConfiguration'}, + 'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}), + 'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'skip_entrance_exam': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'updated': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'db_index': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.linkedinaddtoprofileconfiguration': { + 'Meta': {'object_name': 'LinkedInAddToProfileConfiguration'}, + 'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}), + 'company_identifier': ('django.db.models.fields.TextField', [], {}), + 'dashboard_tracking_code': ('django.db.models.fields.TextField', [], {'default': "''", 'blank': 'True'}), + 'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'trk_partner_name': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '10', 'blank': 'True'}) + }, + 'student.loginfailures': { + 'Meta': {'object_name': 'LoginFailures'}, + 'failure_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'lockout_until': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.passwordhistory': { + 'Meta': {'object_name': 'PasswordHistory'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'time_set': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.pendingemailchange': { + 'Meta': {'object_name': 'PendingEmailChange'}, + 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_email': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.pendingnamechange': { + 'Meta': {'object_name': 'PendingNameChange'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'new_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}), + 'rationale': ('django.db.models.fields.CharField', [], {'max_length': '1024', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.registration': { + 'Meta': {'object_name': 'Registration', 'db_table': "'auth_registration'"}, + 'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'}) + }, + 'student.userprofile': { + 'Meta': {'object_name': 'UserProfile', 'db_table': "'auth_userprofile'"}, + 'allow_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'bio': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'city': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'null': 'True', 'blank': 'True'}), + 'courseware': ('django.db.models.fields.CharField', [], {'default': "'course.xml'", 'max_length': '255', 'blank': 'True'}), + 'gender': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}), + 'goals': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'has_profile_image': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'language': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'level_of_education': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}), + 'location': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'mailing_address': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}), + 'meta': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'profile'", 'unique': 'True', 'to': "orm['auth.User']"}), + 'year_of_birth': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'}) + }, + 'student.usersignupsource': { + 'Meta': {'object_name': 'UserSignupSource'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'site': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}) + }, + 'student.userstanding': { + 'Meta': {'object_name': 'UserStanding'}, + 'account_status': ('django.db.models.fields.CharField', [], {'max_length': '31', 'blank': 'True'}), + 'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'standing_last_changed_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'standing'", 'unique': 'True', 'to': "orm['auth.User']"}) + }, + 'student.usertestgroup': { + 'Meta': {'object_name': 'UserTestGroup'}, + 'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}), + 'users': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.User']", 'db_index': 'True', 'symmetrical': 'False'}) + } + } + + complete_apps = ['student'] \ No newline at end of file diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 3080181a6d4..a42a06fee56 100644 --- a/common/djangoapps/student/models.py +++ b/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 diff --git a/lms/envs/common.py b/lms/envs/common.py index 1930da143c7..d399640d354 100644 --- a/lms/envs/common.py +++ b/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' + diff --git a/lms/envs/test.py b/lms/envs/test.py index a74d84514ec..7658867fef2 100644 --- a/lms/envs/test.py +++ b/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' diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index a11eab68e88..7913aecd18e 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/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. diff --git a/openedx/core/djangoapps/user_api/accounts/helpers.py b/openedx/core/djangoapps/user_api/accounts/helpers.py new file mode 100644 index 00000000000..836ace7d942 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/helpers.py @@ -0,0 +1,57 @@ +""" +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) diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index 40b572cc9cb..2de8ac03196 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -1,10 +1,15 @@ 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 diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 0fb92da9e93..6f2d709f2f3 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/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. diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_helpers.py b/openedx/core/djangoapps/user_api/accounts/tests/test_helpers.py new file mode 100644 index 00000000000..af8e2150e98 --- /dev/null +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_helpers.py @@ -0,0 +1,63 @@ +""" +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) 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 38e62f25772..fa23742aec3 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -1,12 +1,14 @@ # -*- 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') diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 7d969127a59..efaab726ff0 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/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) diff --git a/openedx/core/djangoapps/user_api/serializers.py b/openedx/core/djangoapps/user_api/serializers.py index 12ba2c1d920..fefee20e53c 100644 --- a/openedx/core/djangoapps/user_api/serializers.py +++ b/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', '') diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 0337b2cbcf0..464b5c9ed7b 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/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. """ -- GitLab