Include profile image version in URLs.

TNL-1860

common/djangoapps/student/migrations/0048_add_has_profile_image_boolean.py → common/djangoapps/student/migrations/0048_add_profile_image_version.py

@@ -8,15 +8,15 @@ 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),
+        # Adding field 'UserProfile.profile_image_uploaded_at'
+        db.add_column('auth_userprofile', 'profile_image_uploaded_at',
+                      self.gf('django.db.models.fields.DateTimeField')(null=True),
                       keep_default=False)
 
 
     def backwards(self, orm):
-        # Deleting field 'UserProfile.has_profile_image'
-        db.delete_column('auth_userprofile', 'has_profile_image')
+        # Deleting field 'UserProfile.profile_image_uploaded_at'
+        db.delete_column('auth_userprofile', 'profile_image_uploaded_at')
 
 
     models = {
@@ -192,4 +192,4 @@ class Migration(SchemaMigration):
         }
     }
 
-    complete_apps = ['student']
\ No newline at end of file
+    complete_apps = ['student']

common/djangoapps/student/migrations/0049_auto__add_languageproficiency__add_unique_languageproficiency_code_use.py

@@ -173,7 +173,7 @@ class Migration(SchemaMigration):
             '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'}),
+            'profile_image_uploaded_at': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
             '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'}),

common/djangoapps/student/models.py

@@ -251,7 +251,15 @@ 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)
+    profile_image_uploaded_at = models.DateTimeField(null=True)
+
+    @property
+    def has_profile_image(self):
+        """
+        Convenience method that returns a boolean indicating whether or not
+        this user has uploaded a profile image.
+        """
+        return self.profile_image_uploaded_at is not None
 
     def get_meta(self):  # pylint: disable=missing-docstring
         js_str = self.meta
@@ -321,7 +329,7 @@ def user_profile_pre_save_callback(sender, **kwargs):
 
     # Remove profile images for users who require parental consent
     if user_profile.requires_parental_consent() and user_profile.has_profile_image:
-        user_profile.has_profile_image = False
+        user_profile.profile_image_uploaded_at = None
 
 
 class UserSignupSource(models.Model):

common/djangoapps/student/tests/test_parental_controls.py

@@ -69,14 +69,14 @@ class ProfileParentalControlsTest(TestCase):
         """Verify that a profile's image obeys parental controls."""
 
         # Verify that an image cannot be set for a user with no year of birth set
-        self.profile.has_profile_image = True
+        self.profile.profile_image_uploaded_at = datetime.datetime.now()
         self.profile.save()
         self.assertFalse(self.profile.has_profile_image)
 
         # Verify that an image can be set for an adult user
         current_year = datetime.datetime.now().year
         self.set_year_of_birth(current_year - 20)
-        self.profile.has_profile_image = True
+        self.profile.profile_image_uploaded_at = datetime.datetime.now()
         self.profile.save()
         self.assertTrue(self.profile.has_profile_image)
 

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

@@ -2,13 +2,14 @@
 Test cases for the HTTP endpoints of the profile image api.
 """
 from contextlib import closing
+import datetime
+from pytz import UTC
 import unittest
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
 import mock
 from mock import patch
-
 from PIL import Image
 from rest_framework.test import APITestCase, APIClient
 
@@ -17,13 +18,14 @@ from student.tests.factories import UserFactory
 from ...user_api.accounts.image_helpers import (
     set_has_profile_image,
     get_profile_image_names,
-    get_profile_image_storage
+    get_profile_image_storage,
 )
 from ..images import create_profile_images, ImageValidationError
 from ..views import LOG_MESSAGE_CREATE, LOG_MESSAGE_DELETE
 from .helpers import make_image_file
 
 TEST_PASSWORD = "test"
+TEST_UPLOAD_DT = datetime.datetime(2002, 1, 9, 15, 43, 01, tzinfo=UTC)
 
 
 class ProfileImageEndpointTestCase(APITestCase):
@@ -121,7 +123,8 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
         self.assertEqual(401, response.status_code)
         self.assertFalse(mock_log.info.called)
 
-    def test_upload_self(self, mock_log):
+    @patch('openedx.core.djangoapps.profile_images.views._make_upload_dt', return_value=TEST_UPLOAD_DT)
+    def test_upload_self(self, mock_make_image_version, mock_log):  # pylint: disable=unused-argument
         """
         Test that an authenticated user can POST to their own upload endpoint.
         """
@@ -244,7 +247,7 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
         with make_image_file() as image_file:
             create_profile_images(image_file, get_profile_image_names(self.user.username))
             self.check_images()
-            set_has_profile_image(self.user.username, True)
+            set_has_profile_image(self.user.username, True, TEST_UPLOAD_DT)
 
     def test_unsupported_methods(self, mock_log):
         """

openedx/core/djangoapps/profile_images/views.py

@@ -2,6 +2,7 @@
 This module implements the upload and remove endpoints of the profile image api.
 """
 from contextlib import closing
+import datetime
 import logging
 
 from django.utils.translation import ugettext as _
@@ -16,7 +17,7 @@ from openedx.core.lib.api.authentication import (
     SessionAuthenticationAllowInactiveUser,
 )
 from openedx.core.lib.api.permissions import IsUserInUrl, IsUserInUrlOrStaff
-from openedx.core.djangoapps.user_api.accounts.image_helpers import set_has_profile_image, get_profile_image_names
+from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image
 from .images import validate_uploaded_image, create_profile_images, remove_profile_images, ImageValidationError
 
 log = logging.getLogger(__name__)
@@ -25,6 +26,14 @@ LOG_MESSAGE_CREATE = 'Generated and uploaded images %(image_names)s for user %(u
 LOG_MESSAGE_DELETE = 'Deleted images %(image_names)s for user %(user_id)s'
 
 
+def _make_upload_dt():
+    """
+    Generate a server-side timestamp for the upload.  This is in a separate
+    function so its behavior can be overridden in tests.
+    """
+    return datetime.datetime.utcnow()
+
+
 class ProfileImageUploadView(APIView):
     """
     **Use Cases**
@@ -92,7 +101,7 @@ class ProfileImageUploadView(APIView):
                 create_profile_images(uploaded_file, profile_image_names)
 
                 # update the user account to reflect that a profile image is available.
-                set_has_profile_image(username, True)
+                set_has_profile_image(username, True, _make_upload_dt())
 
                 log.info(
                     LOG_MESSAGE_CREATE,

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

@@ -47,15 +47,18 @@ def _get_profile_image_filename(name, size, file_extension=PROFILE_IMAGE_FILE_EX
     return '{name}_{size}.{file_extension}'.format(name=name, size=size, file_extension=file_extension)
 
 
-def _get_profile_image_urls(name, storage, file_extension=PROFILE_IMAGE_FILE_EXTENSION):
+def _get_profile_image_urls(name, storage, file_extension=PROFILE_IMAGE_FILE_EXTENSION, version=None):
     """
     Returns a dict containing the urls for a complete set of profile images,
     keyed by "friendly" name (e.g. "full", "large", "medium", "small").
     """
-    return {
-        size_display_name: storage.url(_get_profile_image_filename(name, size, file_extension=file_extension))
-        for size_display_name, size in PROFILE_IMAGE_SIZES_MAP.items()
-    }
+    def _make_url(size):  # pylint: disable=missing-docstring
+        url = storage.url(
+            _get_profile_image_filename(name, size, file_extension=file_extension)
+        )
+        return '{}?v={}'.format(url, version) if version is not None else url
+
+    return {size_display_name: _make_url(size) for size_display_name, size in PROFILE_IMAGE_SIZES_MAP.items()}
 
 
 def get_profile_image_names(username):
@@ -88,7 +91,11 @@ def get_profile_image_urls_for_user(user):
 
     """
     if user.profile.has_profile_image:
-        return _get_profile_image_urls(_make_profile_image_name(user.username), get_profile_image_storage())
+        return _get_profile_image_urls(
+            _make_profile_image_name(user.username),
+            get_profile_image_storage(),
+            version=user.profile.profile_image_uploaded_at.strftime("%s"),
+        )
     else:
         return _get_default_profile_image_urls()
 
@@ -103,19 +110,38 @@ def _get_default_profile_image_urls():
     return _get_profile_image_urls(
         settings.PROFILE_IMAGE_DEFAULT_FILENAME,
         staticfiles_storage,
-        file_extension=settings.PROFILE_IMAGE_DEFAULT_FILE_EXTENSION
+        file_extension=settings.PROFILE_IMAGE_DEFAULT_FILE_EXTENSION,
     )
 
 
-def set_has_profile_image(username, has_profile_image=True):
+def set_has_profile_image(username, is_uploaded, upload_dt=None):
     """
     System (not user-facing) API call used to store whether the user has
-    uploaded a profile image.  Used by profile_image API.
+    uploaded a profile image, and if so, when.  Used by profile_image API.
+
+    Arguments:
+        username (django.contrib.auth.User.username): references the user who
+            uploaded an image.
+
+        is_uploaded (bool): whether or not the user has an uploaded profile
+            image.
+
+        upload_dt (datetime.datetime): If `is_uploaded` is True, this should
+            contain the server-side date+time of the upload.  If `is_uploaded`
+            is False, the parameter is optional and will be ignored.
+
+    Raises:
+        ValueError: is_uploaded was True, but no upload datetime was supplied.
+        UserNotFound: no user with username `username` exists.
     """
+    if is_uploaded and upload_dt is None:
+        raise ValueError("No upload datetime was supplied.")
+    elif not is_uploaded:
+        upload_dt = None
     try:
         profile = UserProfile.objects.get(user__username=username)
     except ObjectDoesNotExist:
         raise UserNotFound()
 
-    profile.has_profile_image = has_profile_image
+    profile.profile_image_uploaded_at = upload_dt
     profile.save()

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

 """
 Tests for helpers.py
 """
+import datetime
 import hashlib
 from mock import patch
+from pytz import UTC
 from unittest import skipUnless
 
 from django.conf import settings
@@ -12,6 +14,7 @@ from ..image_helpers import get_profile_image_urls_for_user
 from student.tests.factories import UserFactory
 
 TEST_SIZES = {'full': 50, 'small': 10}
+TEST_PROFILE_IMAGE_UPLOAD_DT = datetime.datetime(2002, 1, 9, 15, 43, 01, tzinfo=UTC)
 
 
 @patch.dict('openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', TEST_SIZES, clear=True)
@@ -25,17 +28,17 @@ class ProfileImageUrlTestCase(TestCase):
         self.user = UserFactory()
         # Ensure that parental controls don't apply to this user
         self.user.profile.year_of_birth = 1980
-        self.user.profile.has_profile_image = False
+        self.user.profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOAD_DT
         self.user.profile.save()
 
-    def verify_url(self, actual_url, expected_name, expected_pixels):
+    def verify_url(self, actual_url, expected_name, expected_pixels, expected_version):
         """
         Verify correct url structure.
         """
         self.assertEqual(
             actual_url,
-            'http://example-storage.com/profile-images/{name}_{size}.jpg'.format(
-                name=expected_name, size=expected_pixels
+            'http://example-storage.com/profile-images/{name}_{size}.jpg?v={version}'.format(
+                name=expected_name, size=expected_pixels, version=expected_version
             )
         )
 
@@ -48,7 +51,7 @@ class ProfileImageUrlTestCase(TestCase):
             '/static/default_{size}.png'.format(size=expected_pixels)
         )
 
-    def verify_urls(self, expected_name, actual_urls, is_default=False):
+    def verify_urls(self, actual_urls, expected_name, is_default=False):
         """
         Verify correct url dictionary structure.
         """
@@ -57,18 +60,20 @@ class ProfileImageUrlTestCase(TestCase):
             if is_default:
                 self.verify_default_url(url, TEST_SIZES[size_display_name])
             else:
-                self.verify_url(url, expected_name, TEST_SIZES[size_display_name])
+                self.verify_url(
+                    url, expected_name, TEST_SIZES[size_display_name], TEST_PROFILE_IMAGE_UPLOAD_DT.strftime("%s")
+                )
 
     def test_get_profile_image_urls(self):
         """
         Tests `get_profile_image_urls_for_user`
         """
-        self.user.profile.has_profile_image = True
+        self.user.profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOAD_DT
         self.user.profile.save()
         expected_name = hashlib.md5('secret' + self.user.username).hexdigest()
         actual_urls = get_profile_image_urls_for_user(self.user)
-        self.verify_urls(expected_name, actual_urls)
+        self.verify_urls(actual_urls, expected_name, is_default=False)
 
-        self.user.profile.has_profile_image = False
+        self.user.profile.profile_image_uploaded_at = None
         self.user.profile.save()
-        self.verify_urls('default', get_profile_image_urls_for_user(self.user), is_default=True)
+        self.verify_urls(get_profile_image_urls_for_user(self.user), 'default', is_default=True)

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

@@ -5,6 +5,7 @@ import ddt
 import hashlib
 import json
 from mock import patch
+from pytz import UTC
 import unittest
 
 from django.conf import settings
@@ -19,6 +20,8 @@ from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KE
 from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
 from .. import PRIVATE_VISIBILITY, ALL_USERS_VISIBILITY
 
+TEST_PROFILE_IMAGE_UPLOADED_AT = datetime.datetime(2002, 1, 9, 15, 43, 01, tzinfo=UTC)
+
 
 # this is used in one test to check the behavior of profile image url
 # generation with a relative url in the config.
@@ -97,7 +100,7 @@ class UserAPITestCase(APITestCase):
         legacy_profile.mailing_address = "Park Ave"
         legacy_profile.gender = "f"
         legacy_profile.bio = "Tired mother of twins"
-        legacy_profile.has_profile_image = True
+        legacy_profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOADED_AT
         legacy_profile.language_proficiencies.add(LanguageProficiency(code='en'))
         legacy_profile.save()
 
@@ -123,24 +126,23 @@ class TestAccountAPI(UserAPITestCase):
         corresponds to whether the user has or hasn't set a profile
         image.
         """
+        template = '{root}/{filename}_{{size}}.{extension}'
         if has_profile_image:
             url_root = 'http://example-storage.com/profile-images'
             filename = hashlib.md5('secret' + self.user.username).hexdigest()
             file_extension = 'jpg'
+            template += '?v={}'.format(TEST_PROFILE_IMAGE_UPLOADED_AT.strftime("%s"))
         else:
             url_root = 'http://testserver/static'
             filename = 'default'
             file_extension = 'png'
+        template = template.format(root=url_root, filename=filename, extension=file_extension)
         self.assertEqual(
             data['profile_image'],
             {
                 'has_image': has_profile_image,
-                'image_url_full': '{root}/{filename}_50.{extension}'.format(
-                    root=url_root, filename=filename, extension=file_extension,
-                ),
-                'image_url_small': '{root}/{filename}_10.{extension}'.format(
-                    root=url_root, filename=filename, extension=file_extension,
-                )
+                'image_url_full': template.format(size=50),
+                'image_url_small': template.format(size=10),
             }
         )