From 5a7bcd7bb3f5b3ad5269b173397b1c56692575fb Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 20 Aug 2013 22:43:53 -0400 Subject: [PATCH 1/5] always serialize out HTML content to a separate .html file --- .../contentstore/tests/test_contentstore.py | 25 +++++++++++++++++++ common/lib/xmodule/xmodule/html_module.py | 7 +----- common/test/data/toy/course/2012_Fall.xml | 1 + 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 96b0b84e36..282753fcf9 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1057,6 +1057,31 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # It should now contain empty data self.assertEquals(imported_word_cloud.data, '') + def test_html_export_roundtrip(self): + """ + Test that a course which has HTML that has style formatting is preserved in export/import + """ + module_store = modulestore('direct') + content_store = contentstore() + + import_from_xml(module_store, 'common/test/data/', ['toy']) + + location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') + + # Export the course + root_dir = path(mkdtemp_clean()) + export_to_xml(module_store, content_store, location, root_dir, 'test_roundtrip') + + # Reimport and get the video back + import_from_xml(module_store, root_dir) + + # get the sample HTML with styling information + html_module = module_store.get_instance( + 'edX/toy/2012_Fall', + Location(['i4x', 'edX', 'toy', 'html', 'with_styling']) + ) + self.assertIn('

', html_module.data) + def test_course_handouts_rewrites(self): module_store = modulestore('direct') diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 567f5c7eef..726cac77f8 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -164,14 +164,9 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): # TODO (vshnayder): make export put things in the right places. def definition_to_xml(self, resource_fs): - '''If the contents are valid xml, write them to filename.xml. Otherwise, - write just to filename.xml, and the html + ''' Write to filename.xml, and the html string to filename.html. ''' - try: - return etree.fromstring(self.data) - except etree.XMLSyntaxError: - pass # Not proper format. Write html to file, return an empty tag pathname = name_to_pathname(self.url_name) diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index ec75ef0b9d..ebbc2bb75e 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -8,6 +8,7 @@ +

Red text here

\ No newline at end of file diff --git a/common/test/data/toy/html/with_styling.xml b/common/test/data/toy/html/with_styling.xml new file mode 100644 index 0000000000..1ee6ca5c24 --- /dev/null +++ b/common/test/data/toy/html/with_styling.xml @@ -0,0 +1 @@ + \ No newline at end of file From b8a8b202a46c20a68b7a556557729fdf737fa8ef Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 20 Aug 2013 22:53:03 -0400 Subject: [PATCH 3/5] update comments and some code violation drive by fixes --- common/lib/xmodule/xmodule/html_module.py | 31 +++++++++++++++-------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 726cac77f8..7a68c42ac9 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -33,11 +33,13 @@ class HtmlFields(object): class HtmlModule(HtmlFields, XModule): - js = {'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee'), - resource_string(__name__, 'js/src/collapsible.coffee'), - resource_string(__name__, 'js/src/html/display.coffee') - ] - } + js = { + 'coffee': [ + resource_string(__name__, 'js/src/javascript_loader.coffee'), + resource_string(__name__, 'js/src/collapsible.coffee'), + resource_string(__name__, 'js/src/html/display.coffee') + ] + } js_module_name = "HTMLModule" css = {'scss': [resource_string(__name__, 'css/html/display.scss')]} @@ -118,8 +120,10 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): # from .html # 'filename' in html pointers is a relative path # (not same as 'html/blah.html' when the pointer is in a directory itself) - pointer_path = "{category}/{url_path}".format(category='html', - url_path=name_to_pathname(location.name)) + pointer_path = "{category}/{url_path}".format( + category='html', + url_path=name_to_pathname(location.name) + ) base = path(pointer_path).dirname() # log.debug("base = {0}, base.dirname={1}, filename={2}".format(base, base.dirname(), filename)) filepath = "{base}/{name}.html".format(base=base, name=filename) @@ -168,10 +172,12 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): string to filename.html. ''' - # Not proper format. Write html to file, return an empty tag + # Write html to file, return an empty tag pathname = name_to_pathname(self.url_name) - filepath = u'{category}/{pathname}.html'.format(category=self.category, - pathname=pathname) + filepath = u'{category}/{pathname}.html'.format( + category=self.category, + pathname=pathname + ) resource_fs.makedir(os.path.dirname(filepath), recursive=True, allow_recreate=True) with resource_fs.open(filepath, 'w') as filestream: @@ -185,6 +191,7 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor): elt.set("filename", relname) return elt + class AboutFields(object): display_name = String( help="Display name for this module", @@ -197,12 +204,14 @@ class AboutFields(object): scope=Scope.content ) + class AboutModule(AboutFields, HtmlModule): """ Overriding defaults but otherwise treated as HtmlModule. """ pass + class AboutDescriptor(AboutFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located @@ -211,6 +220,7 @@ class AboutDescriptor(AboutFields, HtmlDescriptor): template_dir_name = "about" module_class = AboutModule + class StaticTabFields(object): """ The overrides for Static Tabs @@ -236,6 +246,7 @@ class StaticTabModule(StaticTabFields, HtmlModule): """ pass + class StaticTabDescriptor(StaticTabFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located From d523167bddd852a69f28891d8a7ef3ba75473356 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 20 Aug 2013 23:16:39 -0400 Subject: [PATCH 4/5] add another piece of test data. just an img tag. --- cms/djangoapps/contentstore/tests/test_contentstore.py | 7 +++++++ common/test/data/toy/course/2012_Fall.xml | 1 + common/test/data/toy/html/just_img.html | 1 + common/test/data/toy/html/just_img.xml | 1 + 4 files changed, 10 insertions(+) create mode 100644 common/test/data/toy/html/just_img.html create mode 100644 common/test/data/toy/html/just_img.xml diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 282753fcf9..93f353015d 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1082,6 +1082,13 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): ) self.assertIn('

', html_module.data) + # get the sample HTML with just a simple tag information + html_module = module_store.get_instance( + 'edX/toy/2012_Fall', + Location(['i4x', 'edX', 'toy', 'html', 'just_img']) + ) + self.assertIn('', html_module.data) + def test_course_handouts_rewrites(self): module_store = modulestore('direct') diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index ebbc2bb75e..ce8a2399b5 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -9,6 +9,7 @@ +