From 1c2958e35d6d4e36b15cca0d23b7dfb50a8fdc1b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 9 Aug 2013 14:21:03 -0400 Subject: [PATCH] address PR feedback --- .../xmodule/modulestore/store_utilities.py | 29 +++++++++---------- .../xmodule/modulestore/xml_importer.py | 7 ++--- .../test/data/toy/html/nonportable_link.html | 1 + .../test/data/toy/html/nonportable_link.xml | 2 +- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index 8cc3a14696..32da6e918b 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -20,7 +20,7 @@ def _prefix_only_url_replace_regex(prefix): (?P{prefix}) # the prefix (?P.*?) # everything else in the url (?P=quote) # the first matching closing quote - """.format(prefix=prefix) + """.format(prefix=re.escape(prefix)) def _prefix_and_category_url_replace_regex(prefix): @@ -37,7 +37,7 @@ def _prefix_and_category_url_replace_regex(prefix): (?P[^/]+)/ (?P.*?) # everything else in the url (?P=quote) # the first matching closing quote - """.format(prefix=prefix) + """.format(prefix=re.escape(prefix)) def rewrite_nonportable_content_links(source_course_id, dest_course_id, text): @@ -54,18 +54,19 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text): def portable_asset_link_subtitution(match): quote = match.group('quote') rest = match.group('rest') - return "".join([quote, '/static/'+rest, quote]) + return quote + '/static/' + rest + quote def portable_jump_to_link_substitution(match): quote = match.group('quote') rest = match.group('rest') - return "".join([quote, '/jump_to_id/'+rest, quote]) + return quote + '/jump_to_id/' + rest + quote def generic_courseware_link_substitution(match): quote = match.group('quote') rest = match.group('rest') dest_generic_courseware_lik_base = '/courses/{org}/{course}/{run}/'.format( - org=dest_org, course=dest_course, run=dest_run) + org=dest_org, course=dest_course, run=dest_run + ) return "".join([quote, dest_generic_courseware_lik_base+rest, quote]) @@ -77,18 +78,15 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text): c4x_link_base = '{0}/'.format(StaticContent.get_base_url_path_for_course_assets(course_location)) text = re.sub(_prefix_only_url_replace_regex(c4x_link_base), portable_asset_link_subtitution, text) except Exception, e: - logging.warning("Error going regex subtituion (0) on text = {1}.\n\nError msg = {2}".format( - c4x_link_base, text, str(e))) - pass + logging.warning("Error going regex subtituion %r on text = %r.\n\nError msg = %s", c4x_link_base, text, str(e)) try: jump_to_link_base = '/courses/{org}/{course}/{run}/jump_to/i4x://{org}/{course}/'.format( - org=org, course=course, run=run) + org=org, course=course, run=run + ) text = re.sub(_prefix_and_category_url_replace_regex(jump_to_link_base), portable_jump_to_link_substitution, text) except Exception, e: - logging.warning("Error going regex subtituion (0) on text = {1}.\n\nError msg = {2}".format( - jump_to_link_base, text, str(e))) - pass + logging.warning("Error going regex subtituion %r on text = %r.\n\nError msg = %s", jump_to_link_base, text, str(e)) # Also, there commonly is a set of link URL's used in the format: # /courses/// which will be broken if migrated to a different course_id @@ -99,12 +97,11 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text): if source_course_id != dest_course_id: try: generic_courseware_link_base = '/courses/{org}/{course}/{run}/'.format( - org=org, course=course, run=run) + org=org, course=course, run=run + ) text = re.sub(_prefix_only_url_replace_regex(generic_courseware_link_base), portable_asset_link_subtitution, text) except Exception, e: - logging.warning("Error going regex subtituion (0) on text = {1}.\n\nError msg = {2}".format( - generic_courseware_link_base, text, str(e))) - pass + logging.warning("Error going regex subtituion %r on text = %r.\n\nError msg = %s", generic_courseware_link_base, text, str(e)) return text diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 856a30435c..82e6b75f0a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -128,8 +128,8 @@ def import_from_xml(store, data_dir, course_dirs=None, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge - import_module(module, store, course_data_path, static_content_store, course_location, - target_location_namespace if target_location_namespace else course_location) + import_module(module, store, course_data_path, static_content_store, course_location, + target_location_namespace or course_location) course_items.append(module) @@ -161,7 +161,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # now import any 'draft' items if draft_store is not None: import_course_draft(xml_module_store, store, draft_store, course_data_path, - static_content_store, course_location, target_location_namespace if target_location_namespace is not None + static_content_store, course_location, target_location_namespace if target_location_namespace else course_location) finally: @@ -543,4 +543,3 @@ def import_course_from_xml(modulestore, static_content_store, course_data_path, {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace, verbose=verbose) - diff --git a/common/test/data/toy/html/nonportable_link.html b/common/test/data/toy/html/nonportable_link.html index 7b41de986b..95da68c654 100644 --- a/common/test/data/toy/html/nonportable_link.html +++ b/common/test/data/toy/html/nonportable_link.html @@ -1 +1,2 @@ link + diff --git a/common/test/data/toy/html/nonportable_link.xml b/common/test/data/toy/html/nonportable_link.xml index 61f1870d7a..1afca159bb 100644 --- a/common/test/data/toy/html/nonportable_link.xml +++ b/common/test/data/toy/html/nonportable_link.xml @@ -1 +1 @@ - \ No newline at end of file +