diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py
index 544cef6b61..9337b176e8 100644
--- a/common/lib/xmodule/xmodule/contentstore/content.py
+++ b/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)
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 72c47b6592..f061e8e937 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/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,
})
diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html
index 1d009726d4..e16259e3e4 100644
--- a/lms/templates/courseware/course_about.html
+++ b/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"):
-
})
+
+ 
% else:
-
+
% endif
% else:
-
+
% 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:
diff --git a/openedx/core/djangoapps/content/course_overviews/admin.py b/openedx/core/djangoapps/content/course_overviews/admin.py
index 17f8fa5e77..7b1c90355a 100644
--- a/openedx/core/djangoapps/content/course_overviews/admin.py
+++ b/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)
diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/0006_courseoverviewimageset.py b/openedx/core/djangoapps/content/course_overviews/migrations/0006_courseoverviewimageset.py
new file mode 100644
index 0000000000..609950bbbb
--- /dev/null
+++ b/openedx/core/djangoapps/content/course_overviews/migrations/0006_courseoverviewimageset.py
@@ -0,0 +1,30 @@
+# -*- 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,
+ },
+ ),
+ ]
diff --git a/openedx/core/djangoapps/content/course_overviews/migrations/0007_courseoverviewimageconfig.py b/openedx/core/djangoapps/content/course_overviews/migrations/0007_courseoverviewimageconfig.py
new file mode 100644
index 0000000000..8185fc1783
--- /dev/null
+++ b/openedx/core/djangoapps/content/course_overviews/migrations/0007_courseoverviewimageconfig.py
@@ -0,0 +1,34 @@
+# -*- 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,
+ },
+ ),
+ ]
diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index a4149820d5..659bf21ee2 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -9,11 +9,14 @@ from django.db.models.fields import BooleanField, DateTimeField, DecimalField, T
from django.db.utils import IntegrityError
from django.template import defaultfilters
from django.utils.translation import ugettext
-from lms.djangoapps import django_comment_client
+
+from ccx_keys.locator import CCXLocator
from model_utils.models import TimeStampedModel
from opaque_keys.edx.keys import CourseKey
-from openedx.core.djangoapps.models.course_details import CourseDetails
+from config_models.models import ConfigurationModel
+from lms.djangoapps import django_comment_client
+from openedx.core.djangoapps.models.course_details import CourseDetails
from util.date_utils import strftime_localized
from xmodule import course_metadata_utils
from xmodule.course_module import CourseDescriptor, DEFAULT_START_DATE
@@ -21,9 +24,6 @@ from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore
from xmodule_django.models import CourseKeyField, UsageKeyField
-from ccx_keys.locator import CCXLocator
-
-
log = logging.getLogger(__name__)
@@ -213,6 +213,8 @@ class CourseOverview(TimeStampedModel):
CourseOverviewTab(tab_id=tab.tab_id, course_overview=course_overview)
for tab in course.tabs
])
+ CourseOverviewImageSet.create_for_course(course_overview, course)
+
except IntegrityError:
# There is a rare race condition that will occur if
# CourseOverview.get_from_id is called while a
@@ -256,13 +258,20 @@ class CourseOverview(TimeStampedModel):
course from the module store.
"""
try:
- course_overview = cls.objects.get(id=course_id)
+ course_overview = cls.objects.select_related('image_set').get(id=course_id)
if course_overview.version < cls.VERSION:
# Throw away old versions of CourseOverview, as they might contain stale data.
course_overview.delete()
course_overview = None
except cls.DoesNotExist:
course_overview = None
+
+ # Regenerate the thumbnail images if they're missing (either because
+ # they were never generated, or because they were flushed out after
+ # a change to CourseOverviewImageConfig.
+ if course_overview and not hasattr(course_overview, 'image_set'):
+ CourseOverviewImageSet.create_for_course(course_overview)
+
return course_overview or cls.load_from_module_store(course_id)
def clean_id(self, padding_char='='):
@@ -489,6 +498,45 @@ class CourseOverview(TimeStampedModel):
return True
return False
+ @property
+ def image_urls(self):
+ """
+ Return a dict with all known URLs for this course image.
+
+ Current resolutions are:
+ raw = original upload from the user
+ small = thumbnail with dimensions CourseOverviewImageConfig.current().small
+ large = thumbnail with dimensions CourseOverviewImageConfig.current().large
+
+ If no thumbnails exist, the raw (originally uploaded) image will be
+ returned for all resolutions.
+ """
+ # This is either the raw image that the course team uploaded, or the
+ # settings.DEFAULT_COURSE_ABOUT_IMAGE_URL if they didn't specify one.
+ raw_image_url = self.course_image_url
+
+ # Default all sizes to return the raw image if there is no
+ # CourseOverviewImageSet associated with this CourseOverview. This can
+ # happen because we're disabled via CourseOverviewImageConfig.
+ urls = {
+ 'raw': raw_image_url,
+ 'small': raw_image_url,
+ 'large': raw_image_url,
+ }
+
+ # If we do have a CourseOverviewImageSet, we still default to the raw
+ # images if our thumbnails are blank (might indicate that there was a
+ # processing error of some sort while trying to generate thumbnails).
+ if hasattr(self, 'image_set') and CourseOverviewImageConfig.current().enabled:
+ urls['small'] = self.image_set.small_url or raw_image_url
+ urls['large'] = self.image_set.large_url or raw_image_url
+
+ return urls
+
+ def __unicode__(self):
+ """Represent ourselves with the course key."""
+ return unicode(self.id)
+
class CourseOverviewTab(models.Model):
"""
@@ -496,3 +544,173 @@ class CourseOverviewTab(models.Model):
"""
tab_id = models.CharField(max_length=50)
course_overview = models.ForeignKey(CourseOverview, db_index=True, related_name="tabs")
+
+
+class CourseOverviewImageSet(TimeStampedModel):
+ """
+ Model for Course overview images. Each column is an image type/size.
+
+ You should basically never use this class directly. Read from
+ CourseOverview.image_urls instead.
+
+ Special Notes on Deployment/Rollback/Changes:
+
+ 1. By default, this functionality is disabled. To turn it on, you have to
+ create a CourseOverviewImageConfig entry via Django Admin and select
+ enabled=True.
+
+ 2. If it is enabled in configuration, it will lazily create thumbnails as
+ individual CourseOverviews are requested. This is independent of the
+ CourseOverview's cls.VERSION scheme. This is to better support the use
+ case where someone might want to change the thumbnail resolutions for
+ their theme -- we didn't want to tie the code-based data schema of
+ CourseOverview to configuration changes.
+
+ 3. A CourseOverviewImageSet is automatically deleted when the CourseOverview
+ it belongs to is deleted. So it will be regenerated whenever there's a
+ new publish or the CourseOverview schema version changes. It's not
+ particularly smart about this, and will just re-write the same thumbnails
+ over and over to the same location without checking to see if there were
+ changes.
+
+ 4. Just because a CourseOverviewImageSet is successfully created does not
+ mean that any thumbnails exist. There might have been a processing error,
+ or there might simply be no source image to create a thumbnail out of.
+ In this case, accessing CourseOverview.image_urls will return the value
+ for course.course_image_url for all resolutions. CourseOverviewImageSet
+ will *not* try to regenerate if there is a model entry with blank values
+ for the URLs -- the assumption is that either there's no data there or
+ something has gone wrong and needs fixing in code.
+
+ 5. If you want to change thumbnail resolutions, you need to create a new
+ CourseOverviewImageConfig with the desired dimensions and then wipe the
+ values in CourseOverviewImageSet.
+
+ Logical next steps that I punted on for this first cut:
+
+ 1. Converting other parts of the app to use this.
+
+ Our first cut only affects About Pages and the Student Dashboard. But
+ most places that use course_image_url() should be converted -- e.g.
+ course discovery, mobile, etc.
+
+ 2. Center cropping the image before scaling.
+
+ This is desirable, but it involves a few edge cases (what the rounding
+ policy is, what to do with undersized images, etc.) The behavior that
+ we implemented is at least no worse than what was already there in terms
+ of distorting images.
+
+ 3. Automatically invalidating entries based on CourseOverviewImageConfig.
+
+ There are two basic paths I can think of for this. The first is to
+ completely wipe this table when the config changes. The second is to
+ actually tie the config as a foreign key from this model -- so you could
+ do the comparison to see if the image_set's config_id matched
+ CourseOverviewImageConfig.current() and invalidate it if they didn't
+ match. I punted on this mostly because it's just not something that
+ happens much at all in practice, there is an understood (if manual)
+ process to do it, and it can happen in a follow-on PR if anyone is
+ interested in extending this functionality.
+
+ """
+ course_overview = models.OneToOneField(CourseOverview, db_index=True, related_name="image_set")
+ small_url = models.TextField(blank=True, default="")
+ large_url = models.TextField(blank=True, default="")
+
+ @classmethod
+ def create_for_course(cls, course_overview, course=None):
+ """
+ Create thumbnail images for this CourseOverview.
+
+ This will save the CourseOverviewImageSet it creates before it returns.
+ """
+ from openedx.core.lib.courses import create_course_image_thumbnail
+
+ # If image thumbnails are not enabled, do nothing.
+ config = CourseOverviewImageConfig.current()
+ if not config.enabled:
+ return
+
+ # If a course object was provided, use that. Otherwise, pull it from
+ # CourseOverview's course_id. This happens because sometimes we are
+ # generated as part of the CourseOverview creation (course is available
+ # and passed in), and sometimes the CourseOverview already exists.
+ if not course:
+ course = modulestore().get_course(course_overview.id)
+
+ image_set = CourseOverviewImageSet(course_overview=course_overview)
+ if course.course_image:
+ # Try to create a thumbnails of the course image. If this fails for any
+ # reason (weird format, non-standard URL, etc.), the URLs will default
+ # to being blank. No matter what happens, we don't want to bubble up
+ # a 500 -- an image_set is always optional.
+ try:
+ image_set.small_url = create_course_image_thumbnail(course, config.small)
+ image_set.large_url = create_course_image_thumbnail(course, config.large)
+ except Exception: # pylint: disable=broad-except
+ log.exception(
+ "Could not create thumbnail for course %s with image %s (small=%s), (large=%s)",
+ course.id,
+ course.course_image,
+ config.small,
+ config.large
+ )
+
+ # Regardless of whether we created thumbnails or not, we need to save
+ # this record before returning. If no thumbnails were created (there was
+ # an error or the course has no source course_image), our url fields
+ # just keep their blank defaults.
+ try:
+ image_set.save()
+ course_overview.image_set = image_set
+ except (IntegrityError, ValueError):
+ # In the event of a race condition that tries to save two image sets
+ # to the same CourseOverview, we'll just silently pass on the one
+ # that fails. They should be the same data anyway.
+ #
+ # The ValueError above is to catch the following error that can
+ # happen in Django 1.8.4+ if the CourseOverview object fails to save
+ # (again, due to race condition).
+ #
+ # Example: ValueError: save() prohibited to prevent data loss due
+ # to unsaved related object 'course_overview'.")
+ pass
+
+ def __unicode__(self):
+ return u"CourseOverviewImageSet({}, small_url={}, large_url={})".format(
+ self.course_overview_id, self.small_url, self.large_url
+ )
+
+
+class CourseOverviewImageConfig(ConfigurationModel):
+ """
+ This sets the size of the thumbnail images that Course Overviews will generate
+ to display on the about, info, and student dashboard pages. If you make any
+ changes to this, you will have to regenerate CourseOverviews in order for it
+ to take effect. You might want to do this if you're doing precise theming of
+ your install of edx-platform... but really, you probably don't want to do this
+ at all at the moment, given how new this is. :-P
+ """
+ # Small thumbnail, for things like the student dashboard
+ small_width = models.IntegerField(default=375)
+ small_height = models.IntegerField(default=200)
+
+ # Large thumbnail, for things like the about page
+ large_width = models.IntegerField(default=750)
+ large_height = models.IntegerField(default=400)
+
+ @property
+ def small(self):
+ """Tuple for small image dimensions in pixels -- (width, height)"""
+ return (self.small_width, self.small_height)
+
+ @property
+ def large(self):
+ """Tuple for large image dimensions in pixels -- (width, height)"""
+ return (self.large_width, self.large_height)
+
+ def __unicode__(self):
+ return u"CourseOverviewImageConfig(enabled={}, small={}, large={})".format(
+ self.enabled, self.small, self.large
+ )
diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py
index 51666bad93..a7a15c4b0a 100644
--- a/openedx/core/djangoapps/content/course_overviews/tests.py
+++ b/openedx/core/djangoapps/content/course_overviews/tests.py
@@ -1,6 +1,7 @@
"""
Tests for course_overviews app.
"""
+from cStringIO import StringIO
import datetime
import ddt
import itertools
@@ -8,11 +9,17 @@ import math
import mock
import pytz
+from django.conf import settings
+from django.test.utils import override_settings
from django.utils import timezone
+from PIL import Image
from lms.djangoapps.certificates.api import get_active_web_certificate
from openedx.core.djangoapps.models.course_details import CourseDetails
from openedx.core.lib.courses import course_image_url
+from xmodule.assetstore.assetmgr import AssetManager
+from xmodule.contentstore.django import contentstore
+from xmodule.contentstore.content import StaticContent
from xmodule.course_metadata_utils import DEFAULT_START_DATE
from xmodule.course_module import (
CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
@@ -25,7 +32,7 @@ from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls, check_mongo_calls_range
-from .models import CourseOverview
+from .models import CourseOverview, CourseOverviewImageConfig
@ddt.ddt
@@ -340,7 +347,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
course_overview = CourseOverview._create_from_course(course) # pylint: disable=protected-access
self.assertEqual(course_overview.lowest_passing_grade, None)
- @ddt.data((ModuleStoreEnum.Type.mongo, 4, 4), (ModuleStoreEnum.Type.split, 3, 4))
+ @ddt.data((ModuleStoreEnum.Type.mongo, 5, 5), (ModuleStoreEnum.Type.split, 3, 4))
@ddt.unpack
def test_versioning(self, modulestore_type, min_mongo_calls, max_mongo_calls):
"""
@@ -485,3 +492,336 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
{c.id for c in CourseOverview.get_all_courses(org='TEST_ORG_1')},
{c.id for c in org_courses[1]},
)
+
+
+@ddt.ddt
+class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
+ """
+ Course thumbnail generation tests.
+ """
+
+ def setUp(self):
+ """Create an active CourseOverviewImageConfig with non-default values."""
+ self.set_config(True)
+ super(CourseOverviewImageSetTestCase, self).setUp()
+
+ def set_config(self, enabled):
+ """
+ Enable or disable thumbnail generation config.
+
+ Config models pick the most recent by date created, descending. I delete
+ entries here because that can sometimes screw up on MySQL, which only
+ has second-level granularity in this timestamp.
+
+ This uses non-default values for the dimensions.
+ """
+ CourseOverviewImageConfig.objects.all().delete()
+ CourseOverviewImageConfig.objects.create(
+ enabled=enabled,
+ small_width=200,
+ small_height=100,
+ large_width=400,
+ large_height=200
+ )
+
+ @override_settings(DEFAULT_COURSE_ABOUT_IMAGE_URL='default_course.png')
+ @override_settings(STATIC_URL='static/')
+ @ddt.data(
+ *itertools.product(
+ [ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split],
+ [None, '']
+ )
+ )
+ @ddt.unpack
+ def test_no_source_image(self, modulestore_type, course_image):
+ """
+ Tests that we behave as expected if no source image was specified.
+ """
+ # Because we're sending None and '', we expect to get the generic
+ # fallback URL for course images.
+ fallback_url = settings.STATIC_URL + settings.DEFAULT_COURSE_ABOUT_IMAGE_URL
+ course_overview = self._assert_image_urls_all_default(modulestore_type, course_image, fallback_url)
+
+ # Even though there was no source image to generate, we should still
+ # have a CourseOverviewImageSet object associated with this overview.
+ self.assertTrue(hasattr(course_overview, 'image_set'))
+
+ @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+ def test_disabled_no_prior_data(self, modulestore_type):
+ """
+ Test behavior when we are disabled and no entries exist.
+
+ 1. No CourseOverviewImageSet will be created.
+ 2. All resolutions should return the URL of the raw source image.
+ """
+ # Disable model generation using config models...
+ self.set_config(enabled=False)
+
+ # Since we're disabled, we should just return the raw source image back
+ # for every resolution in image_urls.
+ fake_course_image = 'sample_image.png'
+ course_overview = self._assert_image_urls_all_default(modulestore_type, fake_course_image)
+
+ # Because we are disabled, no image set should have been generated.
+ self.assertFalse(hasattr(course_overview, 'image_set'))
+
+ @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+ def test_disabled_with_prior_data(self, modulestore_type):
+ """
+ Test behavior when entries have been created but we are disabled.
+
+ This might happen because a strange bug was introduced -- e.g. we
+ corrupt the images somehow when making thumbnails. Expectations:
+
+ 1. We ignore whatever was created for the thumbnails, and image_urls
+ returns the same as if no thumbnails had ever been generated. So
+ basically, we return the raw source image for every resolution.
+ 2. We keep the CourseOverviewImageSet data around for debugging
+ purposes.
+ """
+ course_image = "my_course.jpg"
+ broken_small_url = "I am small!"
+ broken_large_url = "I am big!"
+ with self.store.default_store(modulestore_type):
+ course = CourseFactory.create(
+ default_store=modulestore_type, course_image=course_image
+ )
+ course_overview_before = CourseOverview.get_from_id(course.id)
+
+ # This initial seeding should create an entry for the image_set.
+ self.assertTrue(hasattr(course_overview_before, 'image_set'))
+
+ # Now just throw in some fake data to this image set, something that
+ # couldn't possibly work.
+ course_overview_before.image_set.small_url = broken_small_url
+ course_overview_before.image_set.large_url = broken_large_url
+ course_overview_before.image_set.save()
+
+ # Now disable the thumbnail feature
+ self.set_config(False)
+
+ # Fetch a new CourseOverview
+ course_overview_after = CourseOverview.get_from_id(course.id)
+
+ # Assert that the data still exists for debugging purposes
+ self.assertTrue(hasattr(course_overview_after, 'image_set'))
+ image_set = course_overview_after.image_set
+ self.assertEqual(image_set.small_url, broken_small_url)
+ self.assertEqual(image_set.large_url, broken_large_url)
+
+ # But because we've disabled it, asking for image_urls should give us
+ # the raw source image for all resolutions, and not our broken images.
+ expected_url = course_image_url(course)
+ self.assertEqual(
+ course_overview_after.image_urls,
+ {
+ 'raw': expected_url,
+ 'small': expected_url,
+ 'large': expected_url
+ }
+ )
+
+ @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+ def test_error_generating_thumbnails(self, modulestore_type):
+ """
+ Test a scenario where thumbnails cannot be generated.
+
+ We need to make sure that:
+
+ 1. We don't cause any 500s to leak out. A failure to generate thumbnails
+ should never cause CourseOverview generation to fail.
+ 2. We return the raw course image for all resolutions.
+ 3. We don't kill our CPU by trying over and over again.
+ """
+ with mock.patch('openedx.core.lib.courses.create_course_image_thumbnail') as patched_create_thumbnail:
+ # Strictly speaking, this would fail anyway because there's no data
+ # backing sample_image.png, but we're going to make the side-effect
+ # more dramatic. ;-)
+ fake_course_image = 'sample_image.png'
+ patched_create_thumbnail.side_effect = Exception("Kaboom!")
+
+ # This will generate a CourseOverview and verify that we get the
+ # source image back for all resolutions.
+ course_overview = self._assert_image_urls_all_default(modulestore_type, fake_course_image)
+
+ # Make sure we were called (i.e. we tried to create the thumbnail)
+ patched_create_thumbnail.assert_called()
+
+ # Now an image set does exist, even though it only has blank values for
+ # the small and large urls.
+ self.assertTrue(hasattr(course_overview, 'image_set'))
+ self.assertEqual(course_overview.image_set.small_url, '')
+ self.assertEqual(course_overview.image_set.large_url, '')
+
+ # The next time we create a CourseOverview, the images are explicitly
+ # *not* regenerated.
+ with mock.patch('openedx.core.lib.courses.create_course_image_thumbnail') as patched_create_thumbnail:
+ course_overview = CourseOverview.get_from_id(course_overview.id)
+ patched_create_thumbnail.assert_not_called()
+
+ @ddt.data(
+ *itertools.product(
+ [ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split],
+ [True, False],
+ )
+ )
+ @ddt.unpack
+ def test_happy_path(self, modulestore_type, create_after_overview):
+ """
+ What happens when everything works like we expect it to.
+
+ If `create_after_overview` is True, we will temporarily disable
+ thumbnail creation so that the initial CourseOverview is created without
+ an image_set, and the CourseOverviewImageSet is created afterwards. If
+ `create_after_overview` is False, we'll create the CourseOverviewImageSet
+ at the same time as the CourseOverview.
+ """
+ # Create a real (oversized) image...
+ image = Image.new("RGB", (800, 400), "blue")
+ image_buff = StringIO()
+ image.save(image_buff, format="JPEG")
+ image_buff.seek(0)
+ image_name = "big_course_image.jpeg"
+
+ with self.store.default_store(modulestore_type):
+ course = CourseFactory.create(
+ default_store=modulestore_type, course_image=image_name
+ )
+
+ # Save a real image here...
+ course_image_asset_key = StaticContent.compute_location(course.id, course.course_image)
+ course_image_content = StaticContent(course_image_asset_key, image_name, 'image/jpeg', image_buff)
+ contentstore().save(course_image_content)
+
+ # If create_after_overview is True, disable thumbnail generation so
+ # that the CourseOverview object is created and saved without an
+ # image_set at first (it will be lazily created later).
+ if create_after_overview:
+ self.set_config(enabled=False)
+
+ # Now generate the CourseOverview...
+ course_overview = CourseOverview.get_from_id(course.id)
+
+ # If create_after_overview is True, no image_set exists yet. Verify
+ # that, then switch config back over to True and it should lazily
+ # create the image_set on the next get_from_id() call.
+ if create_after_overview:
+ self.assertFalse(hasattr(course_overview, 'image_set'))
+ self.set_config(enabled=True)
+ course_overview = CourseOverview.get_from_id(course.id)
+
+ self.assertTrue(hasattr(course_overview, 'image_set'))
+ image_urls = course_overview.image_urls
+ config = CourseOverviewImageConfig.current()
+
+ # Make sure the thumbnail names come out as expected...
+ self.assertTrue(image_urls['raw'].endswith('big_course_image.jpeg'))
+ self.assertTrue(image_urls['small'].endswith('big_course_image-jpeg-{}x{}.jpg'.format(*config.small)))
+ self.assertTrue(image_urls['large'].endswith('big_course_image-jpeg-{}x{}.jpg'.format(*config.large)))
+
+ # Now make sure our thumbnails are of the sizes we expect...
+ for image_url, expected_size in [(image_urls['small'], config.small), (image_urls['large'], config.large)]:
+ image_key = StaticContent.get_location_from_path(image_url)
+ image_content = AssetManager.find(image_key)
+ image = Image.open(StringIO(image_content.data))
+ self.assertEqual(image.size, expected_size)
+
+ @ddt.data(
+ (800, 400), # Larger than both, correct ratio
+ (800, 600), # Larger than both, incorrect ratio
+ (300, 150), # In between small and large, correct ratio
+ (300, 180), # In between small and large, incorrect ratio
+ (100, 50), # Smaller than both, correct ratio
+ (100, 80), # Smaller than both, incorrect ratio
+ (800, 20), # Bizarrely wide
+ (20, 800), # Bizarrely tall
+ )
+ def test_different_resolutions(self, src_dimensions):
+ """
+ Test various resolutions of images to make thumbnails of.
+
+ Note that our test sizes are small=(200, 100) and large=(400, 200).
+
+ 1. Images should won't be blown up if it's too small, so a (100, 50)
+ resolution image will remain (100, 50).
+ 2. However, images *will* be converted using our format and quality
+ settings (JPEG, 75% -- the PIL default). This is because images with
+ relatively small dimensions not compressed properly.
+ 3. Image thumbnail naming will maintain the naming convention of the
+ target resolution, even if the image was not actually scaled to that
+ size (i.e. it was already smaller). This is mostly because it's
+ simpler to be consistent, but it also lets us more easily tell which
+ configuration a thumbnail was created under.
+ """
+ # Create a source image...
+ image = Image.new("RGB", src_dimensions, "blue")
+ image_buff = StringIO()
+ image.save(image_buff, format="PNG")
+ image_buff.seek(0)
+ image_name = "src_course_image.png"
+
+ course = CourseFactory.create(course_image=image_name)
+
+ # Save the image to the contentstore...
+ course_image_asset_key = StaticContent.compute_location(course.id, course.course_image)
+ course_image_content = StaticContent(course_image_asset_key, image_name, 'image/png', image_buff)
+ contentstore().save(course_image_content)
+
+ # Now generate the CourseOverview...
+ config = CourseOverviewImageConfig.current()
+ course_overview = CourseOverview.get_from_id(course.id)
+ image_urls = course_overview.image_urls
+
+ for image_url, target in [(image_urls['small'], config.small), (image_urls['large'], config.large)]:
+ image_key = StaticContent.get_location_from_path(image_url)
+ image_content = AssetManager.find(image_key)
+ image = Image.open(StringIO(image_content.data))
+
+ # Naming convention for thumbnail
+ self.assertTrue(image_url.endswith('src_course_image-png-{}x{}.jpg'.format(*target)))
+
+ # Actual thumbnail data
+ src_x, src_y = src_dimensions
+ target_x, target_y = target
+ image_x, image_y = image.size
+
+ # I'm basically going to assume the image library knows how to do
+ # the right thing in terms of handling aspect ratio. We're just
+ # going to make sure that small images aren't blown up, and that
+ # we never exceed our target sizes
+ self.assertLessEqual(image_x, target_x)
+ self.assertLessEqual(image_y, target_y)
+
+ if src_x < target_x and src_y < target_y:
+ self.assertEqual(src_x, image_x)
+ self.assertEqual(src_y, image_y)
+
+ def _assert_image_urls_all_default(self, modulestore_type, raw_course_image_name, expected_url=None):
+ """
+ Helper for asserting that all image_urls are defaulting to a particular value.
+
+ Returns the CourseOverview created. This function is useful when you
+ know that the thumbnail generation process is going to fail in some way
+ (e.g. unspecified source image, disabled config, runtime error) and want
+ to verify that all the image URLs are a certain expected value (either
+ the source image, or the fallback URL).
+ """
+ with self.store.default_store(modulestore_type):
+ course = CourseFactory.create(
+ default_store=modulestore_type, course_image=raw_course_image_name
+ )
+ if expected_url is None:
+ expected_url = course_image_url(course)
+
+ course_overview = CourseOverview.get_from_id(course.id)
+
+ # All the URLs that come back should be for the expected_url
+ self.assertEqual(
+ course_overview.image_urls,
+ {
+ 'raw': expected_url,
+ 'small': expected_url,
+ 'large': expected_url,
+ }
+ )
+ return course_overview
diff --git a/openedx/core/lib/courses.py b/openedx/core/lib/courses.py
index f420c1f54f..2936cc93d8 100644
--- a/openedx/core/lib/courses.py
+++ b/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)