diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 0663f9fdea..8ed44a1f5e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -1,11 +1,14 @@ from pprint import pprint # pylint: disable=E0611 from nose.tools import assert_equals, assert_raises, \ - assert_not_equals, assert_false + assert_not_equals, assert_false, assert_true from itertools import ifilter # pylint: enable=E0611 +from path import path import pymongo import logging +import shutil +from tempfile import mkdtemp from uuid import uuid4 from xblock.fields import Scope @@ -16,6 +19,7 @@ from xmodule.tests import DATA_DIR from xmodule.modulestore import Location, MONGO_MODULESTORE_TYPE from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore from xmodule.modulestore.draft import DraftModuleStore +from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint from xmodule.contentstore.mongo import MongoContentStore @@ -310,6 +314,50 @@ class TestMongoModuleStore(object): assert_equals(len(course_locations), 1) assert_in(Location('i4x', 'edX', 'simple', 'course', '2012_Fall'), course_locations) + def test_export_course_image(self): + """ + Test to make sure that we have a course image in the contentstore, + then export it to ensure it gets copied to both file locations. + """ + location = Location('c4x', 'edX', 'simple', 'asset', 'images_course_image.jpg') + course_location = Location('i4x', 'edX', 'simple', 'course', '2012_Fall') + + # This will raise if the course image is missing + self.content_store.find(location) + + root_dir = path(mkdtemp()) + export_to_xml(self.store, self.content_store, course_location, root_dir, 'test_export') + assert_true(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) + assert_true(path(root_dir / 'test_export/static/images_course_image.jpg').isfile()) + shutil.rmtree(root_dir) + + def test_export_course_image_nondefault(self): + """ + Make sure that if a non-default image path is specified that we + don't export it to the static default location + """ + course = self.get_course_by_id('edX/toy/2012_Fall') + assert_true(course.course_image, 'just_a_test.jpg') + + root_dir = path(mkdtemp()) + export_to_xml(self.store, self.content_store, course.location, root_dir, 'test_export') + assert_true(path(root_dir / 'test_export/static/just_a_test.jpg').isfile()) + assert_false(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) + shutil.rmtree(root_dir) + + def test_course_without_image(self): + """ + Make sure we elegantly passover our code when there isn't a static + image + """ + course = self.get_course_by_id('edX/simple_with_draft/2012_Fall') + root_dir = path(mkdtemp()) + export_to_xml(self.store, self.content_store, course.location, root_dir, 'test_export') + assert_false(path(root_dir / 'test_export/static/images/course_image.jpg').isfile()) + assert_false(path(root_dir / 'test_export/static/images_course_image.jpg').isfile()) + shutil.rmtree(root_dir) + + class TestMongoKeyValueStore(object): """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index b442e2ef96..00c759385a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -5,6 +5,8 @@ Methods for exporting course data to XML import logging import lxml.etree from xblock.fields import Scope +from xmodule.contentstore.content import StaticContent +from xmodule.exceptions import NotFoundError from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata from fs.osfs import OSFS @@ -79,6 +81,26 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d root_dir + '/' + course_dir + '/policies/assets.json', ) + # If we are using the default course image, export it to the + # legacy location to support backwards compatibility. + if course.course_image == course.fields['course_image'].default: + try: + course_image = contentstore.find( + StaticContent.compute_location( + course.location.org, + course.location.course, + course.course_image + ), + ) + except NotFoundError: + pass + else: + output_dir = root_dir + '/' + course_dir + '/static/images/' + if not os.path.isdir(output_dir): + os.makedirs(output_dir) + with OSFS(output_dir).open('course_image.jpg', 'wb') as course_image_file: + course_image_file.write(course_image.data) + # export the static tabs export_extra_content(export_fs, modulestore, course_id, course_location, 'static_tab', 'tabs', '.html') diff --git a/common/test/data/simple/static/images_course_image.jpg b/common/test/data/simple/static/images_course_image.jpg new file mode 100644 index 0000000000..6bb7f377a0 Binary files /dev/null and b/common/test/data/simple/static/images_course_image.jpg differ diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index ce8a2399b5..4a98ef3cc9 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -1,4 +1,4 @@ - + diff --git a/common/test/data/toy/static/just_a_test.jpg b/common/test/data/toy/static/just_a_test.jpg new file mode 100644 index 0000000000..6bb7f377a0 Binary files /dev/null and b/common/test/data/toy/static/just_a_test.jpg differ diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index d5cfc47977..51192088fe 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -106,7 +106,14 @@ def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" if course.static_asset_path or modulestore().get_modulestore_type(course.location.course_id) == XML_MODULESTORE_TYPE: - return '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) + "/images/course_image.jpg" + # If we are a static course with the course_image attribute + # set different than the default, return that path so that + # courses can use custom course image paths, otherwise just + # return the default static path. + if hasattr(course, 'course_image') and course.course_image != course.fields['course_image'].default: + return '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) + '/' + course.course_image + else: + return '/static/' + (course.static_asset_path or getattr(course, 'data_dir', '')) + "/images/course_image.jpg" else: loc = StaticContent.compute_location(course.location.org, course.location.course, course.course_image) _path = StaticContent.get_url_path_from_location(loc) diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 32eb3c242f..ba258e6017 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -124,6 +124,29 @@ class MongoCourseImageTestCase(ModuleStoreTestCase): ) ) + def test_static_asset_path_default(self): + """ + Test that without course_image being set, but static_asset_path + being set that we get the right course_image url. + """ + course = CourseFactory.create(static_asset_path="foo") + self.assertEquals( + course_image_url(course), + '/static/foo/images/course_image.jpg' + ) + + def test_static_asset_path_set(self): + """ + Test that without course_image being set, but static_asset_path + being set that we get the right course_image url. + """ + course = CourseFactory.create(course_image=u'things_stuff.jpg', + static_asset_path="foo") + self.assertEquals( + course_image_url(course), + '/static/foo/things_stuff.jpg' + ) + class XmlCourseImageTestCase(XModuleXmlImportTest): """Tests for course image URLs when using an xml modulestore.""" @@ -134,14 +157,12 @@ class XmlCourseImageTestCase(XModuleXmlImportTest): self.assertEquals(course_image_url(course), '/static/xml_test_course/images/course_image.jpg') def test_non_ascii_image_name(self): - # XML Course images are always stored at /images/course_image.jpg course = self.process_xml(xml.CourseFactory.build(course_image=u'before_\N{SNOWMAN}_after.jpg')) - self.assertEquals(course_image_url(course), '/static/xml_test_course/images/course_image.jpg') + self.assertEquals(course_image_url(course), u'/static/xml_test_course/before_\N{SNOWMAN}_after.jpg') def test_spaces_in_image_name(self): - # XML Course images are always stored at /images/course_image.jpg course = self.process_xml(xml.CourseFactory.build(course_image=u'before after.jpg')) - self.assertEquals(course_image_url(course), '/static/xml_test_course/images/course_image.jpg') + self.assertEquals(course_image_url(course), u'/static/xml_test_course/before after.jpg') class CoursesRenderTest(ModuleStoreTestCase):