From dc983cb9a5d5920903c2d1fd2c6be2269443d495 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 22 Mar 2013 11:15:14 -0400 Subject: [PATCH 1/3] add checking for metadata that we can't support editing for in Studio. This is now an error and will have to be addressed by course authors --- .../contentstore/tests/test_contentstore.py | 6 ++++- .../xmodule/modulestore/xml_importer.py | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 615ffb6ed0..34c4b761b7 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -25,7 +25,7 @@ from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml -from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint from xmodule.modulestore.inheritance import own_metadata from xmodule.capa_module import CapaDescriptor @@ -115,6 +115,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # check that there's actually content in the 'question' field self.assertGreater(len(items[0].question),0) + def test_xlint_fails(self): + err_cnt = perform_xlint('common/test/data', ['full']) + self.assertGreater(err_cnt, 0) + def test_delete(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 6a4ce5131b..bf1c8be612 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -356,6 +356,24 @@ def remap_namespace(module, target_location_namespace): return module +def validate_no_non_editable_metadata(module_store, course_id, category): + ''' + Assert that there is no metadata within a particular category that we can't support editing + ''' + err_cnt = 0 + for module_loc in module_store.modules[course_id]: + module = module_store.modules[course_id][module_loc] + if module.location.category == category: + my_metadata = dict(own_metadata(module)) + for key in my_metadata.keys(): + if key != 'xml_attributes' and key != 'display_name': + err_cnt = err_cnt + 1 + print 'ERROR: found metadata on {0}. Metadata: {1} = {2}'.format( + module.location.url(), key, my_metadata[key]) + + return err_cnt + + def validate_category_hierarchy(module_store, course_id, parent_category, expected_child_category): err_cnt = 0 @@ -440,6 +458,8 @@ def perform_xlint(data_dir, course_dirs, err_cnt += validate_category_hierarchy(module_store, course_id, "chapter", "sequential") # constrain that sequentials only have 'verticals' err_cnt += validate_category_hierarchy(module_store, course_id, "sequential", "vertical") + # don't allow metadata on verticals, since we can't edit them in studio + err_cnt += validate_no_non_editable_metadata(module_store, course_id, "vertical") # check for a presence of a course marketing video location_elements = course_id.split('/') @@ -456,3 +476,5 @@ def perform_xlint(data_dir, course_dirs, print "This course can be imported, but some errors may occur during the run of the course. It is recommend that you fix your courseware before importing" else: print "This course can be imported successfully." + + return err_cnt From c55d54b071aff268442defb3d06efa1ca6a90794 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 13:03:34 -0400 Subject: [PATCH 2/3] also, we don't support metadata on chapters --- common/lib/xmodule/xmodule/modulestore/xml_importer.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index bf1c8be612..a800a90493 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -460,6 +460,9 @@ def perform_xlint(data_dir, course_dirs, err_cnt += validate_category_hierarchy(module_store, course_id, "sequential", "vertical") # don't allow metadata on verticals, since we can't edit them in studio err_cnt += validate_no_non_editable_metadata(module_store, course_id, "vertical") + # don't allow metadata on chapters, since we can't edit them in studio + err_cnt += validate_no_non_editable_metadata(module_store, course_id, "chapter") + # check for a presence of a course marketing video location_elements = course_id.split('/') From 3ce01882bb50ca06c6270bc05fa92bb8a67dca44 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 29 Mar 2013 13:59:59 -0400 Subject: [PATCH 3/3] add an 'allowed' list of metadata (e.g. display_name, etc.) and also restrict metadata on sequentials --- .../xmodule/xmodule/modulestore/xml_importer.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index a800a90493..023e7bc9e0 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -356,20 +356,22 @@ def remap_namespace(module, target_location_namespace): return module -def validate_no_non_editable_metadata(module_store, course_id, category): +def validate_no_non_editable_metadata(module_store, course_id, category, allowed=[]): ''' Assert that there is no metadata within a particular category that we can't support editing + However we always allow display_name and 'xml_attribtues' ''' + allowed = allowed + ['xml_attributes', 'display_name'] + err_cnt = 0 for module_loc in module_store.modules[course_id]: module = module_store.modules[course_id][module_loc] if module.location.category == category: my_metadata = dict(own_metadata(module)) for key in my_metadata.keys(): - if key != 'xml_attributes' and key != 'display_name': + if key not in allowed: err_cnt = err_cnt + 1 - print 'ERROR: found metadata on {0}. Metadata: {1} = {2}'.format( - module.location.url(), key, my_metadata[key]) + print ': found metadata on {0}. Studio will not support editing this piece of metadata, so it is not allowed. Metadata: {1} = {2}'. format(module.location.url(), key, my_metadata[key]) return err_cnt @@ -461,8 +463,10 @@ def perform_xlint(data_dir, course_dirs, # don't allow metadata on verticals, since we can't edit them in studio err_cnt += validate_no_non_editable_metadata(module_store, course_id, "vertical") # don't allow metadata on chapters, since we can't edit them in studio - err_cnt += validate_no_non_editable_metadata(module_store, course_id, "chapter") - + err_cnt += validate_no_non_editable_metadata(module_store, course_id, "chapter",['start']) + # don't allow metadata on sequences that we can't edit + err_cnt += validate_no_non_editable_metadata(module_store, course_id, "sequential", + ['due','format','start','graded']) # check for a presence of a course marketing video location_elements = course_id.split('/')