diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 41ac02c75d..181c81b852 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -1,14 +1,14 @@ """ Tests for utils. """ -from contentstore import utils -import mock import collections import copy +import mock from django.test import TestCase -from xmodule.modulestore.tests.factories import CourseFactory from django.test.utils import override_settings +from contentstore import utils from xmodule.modulestore import Location +from xmodule.modulestore.tests.factories import CourseFactory class LMSLinksTestCase(TestCase): @@ -166,3 +166,22 @@ class CourseImageTestCase(TestCase): course = CourseFactory.create(org='edX', course='999') url = utils.course_image_url(course) self.assertEquals(url, '/c4x/edX/999/asset/{0}'.format(course.course_image)) + + def test_non_ascii_image_name(self): + # Verify that non-ascii image names are cleaned + course = CourseFactory.create(course_image=u'before_\N{SNOWMAN}_after.jpg') + self.assertEquals( + utils.course_image_url(course), + '/c4x/{org}/{course}/asset/before___after.jpg'.format(org=course.location.org, course=course.location.course) + ) + + def test_spaces_in_image_name(self): + # Verify that image names with spaces in them are cleaned + course = CourseFactory.create(course_image=u'before after.jpg') + self.assertEquals( + utils.course_image_url(course), + '/c4x/{org}/{course}/asset/before_after.jpg'.format( + org=course.location.org, + course=course.location.course + ) + ) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1aeb447331..f5b3196ccb 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -191,7 +191,7 @@ def get_lms_link_for_about_page(location): def course_image_url(course): """Returns the image url for the course.""" - loc = course.location._replace(tag='c4x', category='asset', name=course.course_image) + loc = StaticContent.compute_location(course.location.org, course.location.course, course.course_image) path = StaticContent.get_url_path_from_location(loc) return path diff --git a/common/lib/xmodule/xmodule/tests/xml/__init__.py b/common/lib/xmodule/xmodule/tests/xml/__init__.py index 9131459d08..416dfa8de2 100644 --- a/common/lib/xmodule/xmodule/tests/xml/__init__.py +++ b/common/lib/xmodule/xmodule/tests/xml/__init__.py @@ -3,6 +3,7 @@ Xml parsing tests for XModules """ import pprint from mock import Mock +from unittest import TestCase from xmodule.x_module import XMLParsingSystem, policy_key from xmodule.mako_module import MakoDescriptorSystem @@ -54,7 +55,7 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable return self._descriptors[Location(location).url()] -class XModuleXmlImportTest(object): +class XModuleXmlImportTest(TestCase): """Base class for tests that use basic XML parsing""" def process_xml(self, xml_import_data): """Use the `xml_import_data` to import an :class:`XBlock` from XML.""" diff --git a/common/lib/xmodule/xmodule/tests/xml/factories.py b/common/lib/xmodule/xmodule/tests/xml/factories.py index 29857f2571..a66fdba2e1 100644 --- a/common/lib/xmodule/xmodule/tests/xml/factories.py +++ b/common/lib/xmodule/xmodule/tests/xml/factories.py @@ -119,6 +119,10 @@ class XmlImportFactory(Factory): class CourseFactory(XmlImportFactory): """Factory for nodes""" tag = 'course' + org = 'edX' + course = 'xml_test_course' + name = '101' + static_asset_path = 'xml_test_course' class SequenceFactory(XmlImportFactory): diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index fd8adf998b..119e2e33d6 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -106,7 +106,7 @@ def course_image_url(course): 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" else: - loc = course.location.replace(tag='c4x', category='asset', name=course.course_image) + loc = StaticContent.compute_location(course.location.org, course.location.course, course.course_image) _path = StaticContent.get_url_path_from_location(loc) return _path diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 8dcab808a3..0bf7f26a3b 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -4,12 +4,15 @@ Tests for course access """ import mock -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from django.http import Http404 from django.test.utils import override_settings -from courseware.courses import get_course_by_id, get_course, get_cms_course_link from xmodule.modulestore.django import get_default_store_name_for_current_request +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.tests.xml import factories as xml +from xmodule.tests.xml import XModuleXmlImportTest + +from courseware.courses import get_course_by_id, get_course, get_cms_course_link, course_image_url from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE @@ -79,3 +82,56 @@ class CoursesTest(ModuleStoreTestCase): ) def test_default_modulestore_published_mapping(self): self.assertEqual(get_default_store_name_for_current_request(), 'default') + + +@override_settings( + MODULESTORE=TEST_DATA_MONGO_MODULESTORE, CMS_BASE=CMS_BASE_TEST +) +class MongoCourseImageTestCase(ModuleStoreTestCase): + """Tests for course image URLs when using a mongo modulestore.""" + + def test_get_image_url(self): + """Test image URL formatting.""" + course = CourseFactory.create(org='edX', course='999') + self.assertEquals(course_image_url(course), '/c4x/edX/999/asset/{0}'.format(course.course_image)) + + def test_non_ascii_image_name(self): + # Verify that non-ascii image names are cleaned + course = CourseFactory.create(course_image=u'before_\N{SNOWMAN}_after.jpg') + self.assertEquals( + course_image_url(course), + '/c4x/{org}/{course}/asset/before___after.jpg'.format( + org=course.location.org, + course=course.location.course + ) + ) + + def test_spaces_in_image_name(self): + # Verify that image names with spaces in them are cleaned + course = CourseFactory.create(course_image=u'before after.jpg') + self.assertEquals( + course_image_url(course), + '/c4x/{org}/{course}/asset/before_after.jpg'.format( + org=course.location.org, + course=course.location.course + ) + ) + + +class XmlCourseImageTestCase(XModuleXmlImportTest): + """Tests for course image URLs when using an xml modulestore.""" + + def test_get_image_url(self): + """Test image URL formatting.""" + course = self.process_xml(xml.CourseFactory.build()) + 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') + + 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')