Merge pull request #3464 from mitocw/cg/course_image_fixes

Additional logic to handle more course_image URL edge cases

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

 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,56 @@ 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())
+        try:
+            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())
+        finally:
+            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())
+        try:
+            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())
+        finally:
+            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())
+        try:
+            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())
+        finally:
+            shutil.rmtree(root_dir)
+
+
 
 class TestMongoKeyValueStore(object):
     """

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')
 

common/test/data/toy/course/2012_Fall.xml

-<course>
+<course course_image="just_a_test.jpg">
     <textbook title="Textbook" book_url="https://s3.amazonaws.com/edx-textbooks/guttag_computation_v3/"/>
   <chapter url_name="Overview">
     <videosequence url_name="Toy_Videos">

lms/djangoapps/courseware/courses.py

@@ -106,11 +106,19 @@ 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.
+        url = '/static/' + (course.static_asset_path or getattr(course, 'data_dir', ''))
+        if hasattr(course, 'course_image') and course.course_image != course.fields['course_image'].default:
+            url += '/' + course.course_image
+        else:
+            url += '/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)
-        return _path
+        url = StaticContent.get_url_path_from_location(loc)
+    return url
 
 
 def find_file(filesystem, dirs, filename):

lms/djangoapps/courseware/tests/test_courses.py

@@ -124,6 +124,29 @@ class MongoCourseImageTestCase(ModuleStoreTestCase):
             )
         )
 
+    def test_static_asset_path_course_image_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_course_image_set(self):
+        """
+        Test that with course_image and static_asset_path both
+        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):