Modify CourseOverviews to create course image thumbnails.

Course teams occasionally upload very large files as their course
image. Before this commit, those images would be used directly in
the student's dashboard, sometimes leading to MBs worth of image
data on that page. With this commit, we now auto-generate small
and large thumbnails of configurable size. The Student Dashboard
and Course About pages will make use of this new functionality
(CourseOverview.image_urls), but the behavior of
CourseOverview.course_image_url will not change.

Note that the thumbnails are still created in the contentstore,
and sit alongside their originals.

What's included:

1. Multiple sizes, currently starting with "raw", "small", and
   "large". This falls back to the current behavior automatically in
   the case where thumbnails don't exist or this feature has been
   disabled in configuration.

2. Django admin based configuration for image sizes and whether
   to enable the functionality at all. Note that to regenerate
   images, you'd need to wipe the CourseOverviewImageSet model
   rows -- it doesn't do that automatically. This is partly because
   it's a very rare operation, and partly because I'm not entirely
   sure what the longer term invalidation strategy should be in a
   world where we might potentially have multiple themes. The
   flexible configuration was intended to allow better customization
   and theming.

3. The Course About pages also use the new thumbnail functionality,
   as an example of "large". This is in addition to the "small"
   used on the student dashboard.

Things I'm punting on for now (followup PRs welcome!):

1. Bringing the thumbnails to course discovery. A quick attempt
   to do so showed that it wasn't getting properly invalidated
   and updated when publishes happen (so the old image still showed
   up). It probably has something to do with when we do the
   re-indexing because it stores this data in elasticsearch, but
   I'm not going to chase it down right now.

2. Center-cropping. While this is a nice-to-have feature, the
   behavior in this PR is no worse than what already exists in
   master in terms of image distortion (letting the browser handle
   it).

3. Automated invalidation of the images when a new config is
   created.

common/lib/xmodule/xmodule/contentstore/content.py

@@ -39,13 +39,23 @@ class StaticContent(object):
         return self.location.category == 'thumbnail'
 
     @staticmethod
-    def generate_thumbnail_name(original_name):
+    def generate_thumbnail_name(original_name, dimensions=None):
+        """
+        - original_name: Name of the asset (typically its location.name)
+        - dimensions: `None` or a tuple of (width, height) in pixels
+        """
         name_root, ext = os.path.splitext(original_name)
         if not ext == XASSET_THUMBNAIL_TAIL_NAME:
             name_root = name_root + ext.replace(u'.', u'-')
+
+        if dimensions:
+            width, height = dimensions  # pylint: disable=unpacking-non-sequence
+            name_root += "-{}x{}".format(width, height)
+
         return u"{name_root}{extension}".format(
             name_root=name_root,
-            extension=XASSET_THUMBNAIL_TAIL_NAME,)
+            extension=XASSET_THUMBNAIL_TAIL_NAME,
+        )
 
     @staticmethod
     def compute_location(course_key, path, revision=None, is_thumbnail=False):
@@ -248,11 +258,25 @@ class ContentStore(object):
         """
         raise NotImplementedError
 
-    def generate_thumbnail(self, content, tempfile_path=None):
+    def generate_thumbnail(self, content, tempfile_path=None, dimensions=None):
+        """Create a thumbnail for a given image.
+
+        Returns a tuple of (StaticContent, AssetKey)
+
+        `content` is the StaticContent representing the image you want to make a
+        thumbnail out of.
+
+        `tempfile_path` is a string path to the location of a file to read from
+        in order to grab the image data, instead of relying on `content.data`
+
+        `dimensions` is an optional param that represents (width, height) in
+        pixels. It defaults to None.
+        """
         thumbnail_content = None
         # use a naming convention to associate originals with the thumbnail
-        thumbnail_name = StaticContent.generate_thumbnail_name(content.location.name)
-
+        thumbnail_name = StaticContent.generate_thumbnail_name(
+            content.location.name, dimensions=dimensions
+        )
         thumbnail_file_location = StaticContent.compute_location(
             content.location.course_key, thumbnail_name, is_thumbnail=True
         )
@@ -273,8 +297,11 @@ class ContentStore(object):
                 # I've seen some exceptions from the PIL library when trying to save palletted
                 # PNG files to JPEG. Per the google-universe, they suggest converting to RGB first.
                 im = im.convert('RGB')
-                size = 128, 128
-                im.thumbnail(size, Image.ANTIALIAS)
+
+                if not dimensions:
+                    dimensions = (128, 128)
+
+                im.thumbnail(dimensions, Image.ANTIALIAS)
                 thumbnail_file = StringIO.StringIO()
                 im.save(thumbnail_file, 'JPEG')
                 thumbnail_file.seek(0)

lms/djangoapps/courseware/views.py

@@ -54,6 +54,7 @@ from openedx.core.djangoapps.credit.api import (
     is_user_eligible_for_credit,
     is_credit_course
 )
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from courseware.models import StudentModuleHistory
 from courseware.model_data import FieldDataCache, ScoresClient
 from .module_render import toc_for_course, get_module_for_descriptor, get_module, get_module_by_usage_id
@@ -866,6 +867,9 @@ def course_about(request, course_id):
         # get prerequisite courses display names
         pre_requisite_courses = get_prerequisite_courses_display(course)
 
+        # Overview
+        overview = CourseOverview.get_from_id(course.id)
+
         return render_to_response('courseware/course_about.html', {
             'course': course,
             'staff_access': staff_access,
@@ -887,7 +891,8 @@ def course_about(request, course_id):
             'disable_courseware_header': True,
             'can_add_course_to_cart': can_add_course_to_cart,
             'cart_link': reverse('shoppingcart.views.show_cart'),
-            'pre_requisite_courses': pre_requisite_courses
+            'pre_requisite_courses': pre_requisite_courses,
+            'course_image_urls': overview.image_urls,
         })
 
 

lms/templates/courseware/course_about.html

@@ -173,14 +173,14 @@ from openedx.core.lib.courses import course_image_url
       % if get_course_about_section(request, course, "video"):
       <a href="#video-modal" class="media" rel="leanModal">
         <div class="hero">
-          <img src="${course_image_url(course)}" alt="" />
+          <img src="${course_image_urls['large']}" alt="" />
           <div class="play-intro"></div>
         </div>
       </a>
       %else:
       <div class="media">
         <div class="hero">
-          <img src="${course_image_url(course)}" alt="" />
+          <img src="${course_image_urls['large']}" alt="" />
         </div>
       </div>
       % endif

lms/templates/dashboard/_dashboard_course_listing.html

@@ -63,16 +63,16 @@ from student.helpers import (
       % if show_courseware_link:
         % if not is_course_blocked:
             <a href="${course_target}" data-course-key="${enrollment.course_id}" class="cover">
-              <img src="${course_overview.course_image_url}" class="course-image" alt="${_('{course_number} {course_name} Home Page').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default) |h}" />
+              <img src="${course_overview.image_urls['small']}" class="course-image" alt="${_('{course_number} {course_name} Home Page').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default) |h}" />
             </a>
         % else:
             <a class="fade-cover">
-              <img src="${course_overview.course_image_url}" class="course-image" alt="${_('{course_number} {course_name} Cover Image').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default) |h}" />
+              <img src="${course_overview.image_urls['small']}" class="course-image" alt="${_('{course_number} {course_name} Cover Image').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default) |h}" />
             </a>
         % endif
       % else:
         <a class="cover">
-          <img src="${course_overview.course_image_url}" class="course-image" alt="${_('{course_number} {course_name} Cover Image').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default) | h}" />
+          <img src="${course_overview.image_urls['small']}" class="course-image" alt="${_('{course_number} {course_name} Cover Image').format(course_number=course_overview.number, course_name=course_overview.display_name_with_default) | h}" />
         </a>
       % endif
       % if settings.FEATURES.get('ENABLE_VERIFIED_CERTIFICATES') and course_verified_certs.get('display_mode') != 'audit':
@@ -281,7 +281,7 @@ from student.helpers import (
 
       % if credit_status is not None:
         <%include file="_dashboard_credit_info.html" args="credit_status=credit_status"/>
-      % endif 
+      % endif
 
         % if verification_status.get('status') in [VERIFY_STATUS_NEED_TO_VERIFY, VERIFY_STATUS_SUBMITTED, VERIFY_STATUS_APPROVED, VERIFY_STATUS_NEED_TO_REVERIFY] and not is_course_blocked:
         <div class="message message-status wrapper-message-primary is-shown">

openedx/core/djangoapps/content/course_overviews/admin.py

@@ -5,7 +5,8 @@ name, and start dates, but don't actually need to crawl into course content.
 """
 from django.contrib import admin
 
-from .models import CourseOverview
+from config_models.admin import ConfigurationModelAdmin
+from .models import CourseOverview, CourseOverviewImageConfig, CourseOverviewImageSet
 
 
 class CourseOverviewAdmin(admin.ModelAdmin):
@@ -35,4 +36,51 @@ class CourseOverviewAdmin(admin.ModelAdmin):
     search_fields = ['id', 'display_name']
 
 
+class CourseOverviewImageConfigAdmin(ConfigurationModelAdmin):
+    """
+    Basic configuration for CourseOverview Image thumbnails.
+
+    By default this is disabled. If you change the dimensions of the images with
+    a new config after thumbnails have already been generated, you need to clear
+    the entries in CourseOverviewImageSet manually for new entries to be
+    created.
+    """
+    list_display = [
+        'change_date',
+        'changed_by',
+        'enabled',
+        'large_width',
+        'large_height',
+        'small_width',
+        'small_height'
+    ]
+
+    def get_list_display(self, request):
+        """
+        Restore default list_display behavior.
+
+        ConfigurationModelAdmin overrides this, but in a way that doesn't
+        respect the ordering. This lets us customize it the usual Django admin
+        way.
+        """
+        return self.list_display
+
+
+class CourseOverviewImageSetAdmin(admin.ModelAdmin):
+    """
+    Thumbnail images associated with CourseOverviews. This should be used for
+    debugging purposes only -- e.g. don't edit these values.
+    """
+    list_display = [
+        'course_overview',
+        'small_url',
+        'large_url',
+    ]
+    search_fields = ['course_overview__id']
+    readonly_fields = ['course_overview_id']
+    fields = ('course_overview_id', 'small_url', 'large_url')
+
+
 admin.site.register(CourseOverview, CourseOverviewAdmin)
+admin.site.register(CourseOverviewImageConfig, CourseOverviewImageConfigAdmin)
+admin.site.register(CourseOverviewImageSet, CourseOverviewImageSetAdmin)

openedx/core/djangoapps/content/course_overviews/migrations/0006_courseoverviewimageset.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.utils.timezone
+import model_utils.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_overviews', '0005_delete_courseoverviewgeneratedhistory'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CourseOverviewImageSet',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, verbose_name='created', editable=False)),
+                ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, verbose_name='modified', editable=False)),
+                ('small_url', models.TextField(default=b'', blank=True)),
+                ('large_url', models.TextField(default=b'', blank=True)),
+                ('course_overview', models.OneToOneField(related_name='image_set', to='course_overviews.CourseOverview')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+    ]

openedx/core/djangoapps/content/course_overviews/migrations/0007_courseoverviewimageconfig.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+from django.conf import settings
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('course_overviews', '0006_courseoverviewimageset'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CourseOverviewImageConfig',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')),
+                ('enabled', models.BooleanField(default=False, verbose_name='Enabled')),
+                ('small_width', models.IntegerField(default=375)),
+                ('small_height', models.IntegerField(default=200)),
+                ('large_width', models.IntegerField(default=750)),
+                ('large_height', models.IntegerField(default=400)),
+                ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')),
+            ],
+            options={
+                'ordering': ('-change_date',),
+                'abstract': False,
+            },
+        ),
+    ]

openedx/core/lib/courses.py

@@ -3,8 +3,10 @@ Common utility functions related to courses.
 """
 from django.conf import settings
 
-from xmodule.modulestore.django import modulestore
+from xmodule.assetstore.assetmgr import AssetManager
 from xmodule.contentstore.content import StaticContent
+from xmodule.contentstore.django import contentstore
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
 
 
@@ -27,4 +29,18 @@ def course_image_url(course):
     else:
         loc = StaticContent.compute_location(course.id, course.course_image)
         url = StaticContent.serialize_asset_key_with_slash(loc)
+
     return url
+
+
+def create_course_image_thumbnail(course, dimensions):
+    """Create a course image thumbnail and return the URL.
+
+    - dimensions is a tuple of (width, height)
+    """
+    course_image_asset_key = StaticContent.compute_location(course.id, course.course_image)
+    course_image = AssetManager.find(course_image_asset_key)  # a StaticContent obj
+
+    _content, thumb_loc = contentstore().generate_thumbnail(course_image, dimensions=dimensions)
+
+    return StaticContent.serialize_asset_key_with_slash(thumb_loc)