Fix bug related to creation of thumbnails for non-image files.
https://edx.lighthouseapp.com/projects/102637-studio/tickets/139-uploads-of-non-images-dont-have-reasonable-icon#ticket-139-2
This commit is contained in:
@@ -760,14 +760,16 @@ def upload_asset(request, org, course, coursename):
|
||||
content_loc = StaticContent.compute_location(org, course, filename)
|
||||
content = StaticContent(content_loc, filename, mime_type, filedata)
|
||||
|
||||
# first let's save a thumbnail so we can get back a thumbnail location
|
||||
thumbnail_content = contentstore().generate_thumbnail(content)
|
||||
# first let's see if a thumbnail can be created
|
||||
(thumbnail_content, thumbnail_location) = contentstore().generate_thumbnail(content)
|
||||
|
||||
# delete cached thumbnail even if one couldn't be created this time (else the old thumbnail will continue to show)
|
||||
del_cached_content(thumbnail_location)
|
||||
# now store thumbnail location only if we could create it
|
||||
if thumbnail_content is not None:
|
||||
content.thumbnail_location = thumbnail_content.location
|
||||
del_cached_content(thumbnail_content.location)
|
||||
content.thumbnail_location = thumbnail_location
|
||||
|
||||
#then commit the content
|
||||
#then commit the content
|
||||
contentstore().save(content)
|
||||
del_cached_content(content.location)
|
||||
|
||||
@@ -777,7 +779,7 @@ def upload_asset(request, org, course, coursename):
|
||||
response_payload = {'displayname' : content.name,
|
||||
'uploadDate' : get_date_display(readback.last_modified_at),
|
||||
'url' : StaticContent.get_url_path_from_location(content.location),
|
||||
'thumb_url' : StaticContent.get_url_path_from_location(thumbnail_content.location) if thumbnail_content is not None else None,
|
||||
'thumb_url' : StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None,
|
||||
'msg' : 'Upload completed'
|
||||
}
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ class StaticContent(object):
|
||||
self.content_type = content_type
|
||||
self.data = data
|
||||
self.last_modified_at = last_modified_at
|
||||
self.thumbnail_location = Location(thumbnail_location)
|
||||
self.thumbnail_location = Location(thumbnail_location) if thumbnail_location is not None else None
|
||||
# optional information about where this file was imported from. This is needed to support import/export
|
||||
# cycles
|
||||
self.import_path = import_path
|
||||
@@ -113,6 +113,12 @@ class ContentStore(object):
|
||||
|
||||
def generate_thumbnail(self, content):
|
||||
thumbnail_content = None
|
||||
# use a naming convention to associate originals with the thumbnail
|
||||
thumbnail_name = StaticContent.generate_thumbnail_name(content.location.name)
|
||||
|
||||
thumbnail_file_location = StaticContent.compute_location(content.location.org, content.location.course,
|
||||
thumbnail_name, is_thumbnail = True)
|
||||
|
||||
# if we're uploading an image, then let's generate a thumbnail so that we can
|
||||
# serve it up when needed without having to rescale on the fly
|
||||
if content.content_type is not None and content.content_type.split('/')[0] == 'image':
|
||||
@@ -131,13 +137,8 @@ class ContentStore(object):
|
||||
thumbnail_file = StringIO.StringIO()
|
||||
im.save(thumbnail_file, 'JPEG')
|
||||
thumbnail_file.seek(0)
|
||||
|
||||
# use a naming convention to associate originals with the thumbnail
|
||||
thumbnail_name = StaticContent.generate_thumbnail_name(content.location.name)
|
||||
|
||||
# then just store this thumbnail as any other piece of content
|
||||
thumbnail_file_location = StaticContent.compute_location(content.location.org, content.location.course,
|
||||
thumbnail_name, is_thumbnail = True)
|
||||
# store this thumbnail as any other piece of content
|
||||
thumbnail_content = StaticContent(thumbnail_file_location, thumbnail_name,
|
||||
'image/jpeg', thumbnail_file)
|
||||
|
||||
@@ -147,7 +148,7 @@ class ContentStore(object):
|
||||
# log and continue as thumbnails are generally considered as optional
|
||||
logging.exception("Failed to generate thumbnail for {0}. Exception: {1}".format(content.location, str(e)))
|
||||
|
||||
return thumbnail_content
|
||||
return thumbnail_content, thumbnail_file_location
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -35,10 +35,10 @@ def import_static_content(modules, course_loc, course_data_path, static_content_
|
||||
content = StaticContent(content_loc, filename, mime_type, data, import_path = fullname_with_subpath)
|
||||
|
||||
# first let's save a thumbnail so we can get back a thumbnail location
|
||||
thumbnail_content = static_content_store.generate_thumbnail(content)
|
||||
(thumbnail_content, thumbnail_location) = static_content_store.generate_thumbnail(content)
|
||||
|
||||
if thumbnail_content is not None:
|
||||
content.thumbnail_location = thumbnail_content.location
|
||||
content.thumbnail_location = thumbnail_location
|
||||
|
||||
#then commit the content
|
||||
static_content_store.save(content)
|
||||
@@ -69,10 +69,10 @@ def verify_content_links(module, base_dir, static_content_store, link, remap_dic
|
||||
content = StaticContent(content_loc, filename, mime_type, data, import_path = path)
|
||||
|
||||
# first let's save a thumbnail so we can get back a thumbnail location
|
||||
thumbnail_content = static_content_store.generate_thumbnail(content)
|
||||
(thumbnail_content, thumbnail_location) = static_content_store.generate_thumbnail(content)
|
||||
|
||||
if thumbnail_content is not None:
|
||||
content.thumbnail_location = thumbnail_content.location
|
||||
content.thumbnail_location = thumbnail_location
|
||||
|
||||
#then commit the content
|
||||
static_content_store.save(content)
|
||||
|
||||
25
common/lib/xmodule/xmodule/tests/test_content.py
Normal file
25
common/lib/xmodule/xmodule/tests/test_content.py
Normal file
@@ -0,0 +1,25 @@
|
||||
import unittest
|
||||
from xmodule.contentstore.content import StaticContent
|
||||
from xmodule.contentstore.content import ContentStore
|
||||
from xmodule.modulestore import Location
|
||||
|
||||
class Content:
|
||||
def __init__(self, location, content_type):
|
||||
self.location = location
|
||||
self.content_type = content_type
|
||||
|
||||
class ContentTest(unittest.TestCase):
|
||||
def test_thumbnail_none(self):
|
||||
# We had a bug where a thumbnail location of None was getting transformed into a Location tuple, with
|
||||
# all elements being None. It is important that the location be just None for rendering.
|
||||
content = StaticContent('loc', 'name', 'content_type', 'data', None, None, None)
|
||||
self.assertIsNone(content.thumbnail_location)
|
||||
|
||||
content = StaticContent('loc', 'name', 'content_type', 'data')
|
||||
self.assertIsNone(content.thumbnail_location)
|
||||
def test_generate_thumbnail_nonimage(self):
|
||||
contentStore = ContentStore()
|
||||
content = Content(Location(u'c4x', u'mitX', u'800', u'asset', u'monsters.jpg'), None)
|
||||
(thumbnail_content, thumbnail_file_location) = contentStore.generate_thumbnail(content)
|
||||
self.assertIsNone(thumbnail_content)
|
||||
self.assertEqual(Location(u'c4x', u'mitX', u'800', u'thumbnail', u'monsters.jpg'), thumbnail_file_location)
|
||||
Reference in New Issue
Block a user