From f4822c23dee665b676b2219bd01f6fef4afabef0 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Tue, 30 Oct 2012 11:52:31 -0400
Subject: [PATCH] lots of tweeks to better support importing of existing
courseware
---
cms/djangoapps/contentstore/views.py | 9 +-
common/djangoapps/mitxmako/shortcuts.py | 2 +-
common/djangoapps/static_replace.py | 24 +++-
common/djangoapps/xmodule_modifiers.py | 4 +-
common/lib/xmodule/setup.py | 2 +
.../xmodule/xmodule/contentstore/content.py | 7 ++
common/lib/xmodule/xmodule/course_module.py | 3 +-
.../lib/xmodule/xmodule/modulestore/mongo.py | 2 +
common/lib/xmodule/xmodule/modulestore/xml.py | 52 ++++-----
.../xmodule/modulestore/xml_importer.py | 103 ++++++++++++++----
common/lib/xmodule/xmodule/template_module.py | 20 +++-
lms/djangoapps/courseware/courses.py | 26 +++--
lms/djangoapps/courseware/module_render.py | 3 +-
lms/djangoapps/courseware/tests/tests.py | 11 +-
lms/djangoapps/courseware/views.py | 2 +-
15 files changed, 186 insertions(+), 84 deletions(-)
diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index be4a157de44..4d1cfed5533 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -287,9 +287,13 @@ def edit_unit(request, location):
# TODO (cpennington): If we share units between courses,
# this will need to change to check permissions correctly so as
# to pick the correct parent subsection
+
+ logging.debug('looking for parent of {0}'.format(location))
+
containing_subsection_locs = modulestore().get_parent_locations(location)
containing_subsection = modulestore().get_item(containing_subsection_locs[0])
+ logging.debug('looking for parent of {0}'.format(containing_subsection.location))
containing_section_locs = modulestore().get_parent_locations(containing_subsection.location)
containing_section = modulestore().get_item(containing_section_locs[0])
@@ -997,7 +1001,8 @@ def import_course(request, org, course, name):
data_root = path(settings.GITHUB_REPO_ROOT)
- course_dir = data_root / "{0}-{1}-{2}".format(org, course, name)
+ course_subdir = "{0}-{1}-{2}".format(org, course, name)
+ course_dir = data_root / course_subdir
if not course_dir.isdir():
os.mkdir(course_dir)
@@ -1033,7 +1038,7 @@ def import_course(request, org, course, name):
shutil.move(r/fname, course_dir)
module_store, course_items = import_from_xml(modulestore('direct'), settings.GITHUB_REPO_ROOT,
- [course_dir], load_error_modules=False, static_content_store=contentstore(), target_location_namespace = Location(location))
+ [course_subdir], load_error_modules=False, static_content_store=contentstore(), target_location_namespace = Location(location))
# we can blow this away when we're done importing.
shutil.rmtree(course_dir)
diff --git a/common/djangoapps/mitxmako/shortcuts.py b/common/djangoapps/mitxmako/shortcuts.py
index ba22f2db207..181d3befd57 100644
--- a/common/djangoapps/mitxmako/shortcuts.py
+++ b/common/djangoapps/mitxmako/shortcuts.py
@@ -42,7 +42,7 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'):
context_dictionary.update(context)
# fetch and render template
template = middleware.lookup[namespace].get_template(template_name)
- return template.render(**context_dictionary)
+ return template.render_unicode(**context_dictionary)
def render_to_response(template_name, dictionary, context_instance=None, namespace='main', **kwargs):
diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py
index 58e2c8da159..ee63e3cf93e 100644
--- a/common/djangoapps/static_replace.py
+++ b/common/djangoapps/static_replace.py
@@ -5,6 +5,10 @@ from staticfiles.storage import staticfiles_storage
from staticfiles import finders
from django.conf import settings
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.xml import XMLModuleStore
+from xmodule.contentstore.content import StaticContent
+
log = logging.getLogger(__name__)
def try_staticfiles_lookup(path):
@@ -22,7 +26,7 @@ def try_staticfiles_lookup(path):
return url
-def replace(static_url, prefix=None):
+def replace(static_url, prefix=None, course_namespace=None):
if prefix is None:
prefix = ''
else:
@@ -41,13 +45,23 @@ def replace(static_url, prefix=None):
return static_url.group(0)
else:
# don't error if file can't be found
- url = try_staticfiles_lookup(prefix + static_url.group('rest'))
- return "".join([quote, url, quote])
+ # cdodge: to support the change over to Mongo backed content stores, lets
+ # use the utility functions in StaticContent.py
+ if static_url.group('prefix') == '/static/' and not isinstance(modulestore(), XMLModuleStore):
+ if course_namespace is None:
+ raise BaseException('You must pass in course_namespace when remapping static content urls with MongoDB stores')
+ url = StaticContent.convert_legacy_static_url(static_url.group('rest'), course_namespace)
+ else:
+ url = try_staticfiles_lookup(prefix + static_url.group('rest'))
+
+ new_link = "".join([quote, url, quote])
+ return new_link
+
-def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/'):
+def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', course_namespace=None):
def replace_url(static_url):
- return replace(static_url, staticfiles_prefix)
+ return replace(static_url, staticfiles_prefix, course_namespace = course_namespace)
return re.sub(r"""
(?x) # flags=re.VERBOSE
diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py
index 9473dfe26be..cda3d013cd0 100644
--- a/common/djangoapps/xmodule_modifiers.py
+++ b/common/djangoapps/xmodule_modifiers.py
@@ -50,7 +50,7 @@ def replace_course_urls(get_html, course_id):
return replace_urls(get_html(), staticfiles_prefix='/courses/'+course_id, replace_prefix='/course/')
return _get_html
-def replace_static_urls(get_html, prefix):
+def replace_static_urls(get_html, prefix, course_namespace=None):
"""
Updates the supplied module with a new get_html function that wraps
the old get_html function and substitutes urls of the form /static/...
@@ -59,7 +59,7 @@ def replace_static_urls(get_html, prefix):
@wraps(get_html)
def _get_html():
- return replace_urls(get_html(), staticfiles_prefix=prefix)
+ return replace_urls(get_html(), staticfiles_prefix=prefix, course_namespace = course_namespace)
return _get_html
diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py
index e9e78b57628..ff5be4d6dfe 100644
--- a/common/lib/xmodule/setup.py
+++ b/common/lib/xmodule/setup.py
@@ -37,6 +37,8 @@ setup(
"discussion = xmodule.discussion_module:DiscussionDescriptor",
"course_info = xmodule.html_module:HtmlDescriptor",
"static_tab = xmodule.html_module:HtmlDescriptor",
+ "custom_tag_template = xmodule.raw_module:RawDescriptor",
+ "about = xmodule.html_module:HtmlDescriptor"
]
}
)
diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py
index 6dcf70fbe1f..a7a76fa2421 100644
--- a/common/lib/xmodule/xmodule/contentstore/content.py
+++ b/common/lib/xmodule/xmodule/contentstore/content.py
@@ -62,6 +62,13 @@ class StaticContent(object):
@staticmethod
def get_id_from_path(path):
return get_id_from_location(get_location_from_path(path))
+
+ @staticmethod
+ def convert_legacy_static_url(path, course_namespace):
+ loc = StaticContent.compute_location(course_namespace.org, course_namespace.course, path)
+ return StaticContent.get_url_path_from_location(loc)
+
+
class ContentStore(object):
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index ae3c01f6393..94ab02cf398 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -202,7 +202,7 @@ class CourseDescriptor(SequenceDescriptor):
# Try to load grading policy
paths = ['grading_policy.json']
if policy_dir:
- paths = [policy_dir + 'grading_policy.json'] + paths
+ paths = [policy_dir + '/grading_policy.json'] + paths
policy = json.loads(cls.read_grading_policy(paths, system))
@@ -394,3 +394,4 @@ class CourseDescriptor(SequenceDescriptor):
return self.location.org
+
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index 164bfd3590f..deef3af3bfa 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -1,5 +1,6 @@
import pymongo
import sys
+import logging
from bson.son import SON
from fs.osfs import OSFS
@@ -49,6 +50,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
self.load_item, resources_fs, error_tracker, render_template)
self.modulestore = modulestore
self.module_data = module_data
+
self.default_class = default_class
def load_item(self, location):
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index ddb437afe28..6794703998e 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -427,42 +427,38 @@ class XMLModuleStore(ModuleStoreBase):
# now import all pieces of course_info which is expected to be stored
# in <content_dir>/info or <content_dir>/info/<url_name>
- if url_name:
- info_path = self.data_dir / course_dir / 'info' / url_name
-
- if not os.path.exists(info_path):
- info_path = self.data_dir / course_dir / 'info'
-
- # we have a fixed number of .html info files that we expect there
- for info_filename in ['handouts', 'guest_handouts', 'updates', 'guest_updates']:
- filepath = info_path / info_filename + '.html'
- if os.path.exists(filepath):
- with open(filepath) as info_file:
- html = info_file.read().decode('utf-8')
- loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, 'course_info', info_filename)
- info_module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc})
- self.modules[course_id][info_module.location] = info_module
+ self.load_extra_content(system, course_descriptor, 'course_info', self.data_dir / course_dir / 'info', course_dir, url_name)
# now import all static tabs which are expected to be stored in
- # in <content_dir>/tabs or <content_dir>/tabs/<url_name>
- if url_name:
- tabs_path = self.data_dir / course_dir / 'tabs' / url_name
+ # in <content_dir>/tabs or <content_dir>/tabs/<url_name>
+ self.load_extra_content(system, course_descriptor, 'static_tab', self.data_dir / course_dir / 'tabs', course_dir, url_name)
- if not os.path.exists(tabs_path):
- tabs_path = self.data_dir / course_dir / 'tabs'
+ self.load_extra_content(system, course_descriptor, 'custom_tag_template', self.data_dir / course_dir / 'custom_tags', course_dir, url_name)
- for tab_filepath in glob.glob(tabs_path / '*.html'):
- with open(tab_filepath) as tab_file:
- html = tab_file.read().decode('utf-8')
- # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix
- slug = os.path.splitext(os.path.basename(tab_filepath))[0]
- loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, 'static_tab', slug)
- tab_module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc})
- self.modules[course_id][tab_module.location] = tab_module
+ self.load_extra_content(system, course_descriptor, 'about', self.data_dir / course_dir / 'about', course_dir, url_name)
log.debug('========> Done with course import from {0}'.format(course_dir))
return course_descriptor
+ def load_extra_content(self, system, course_descriptor, category, base_dir, course_dir, url_name):
+ if url_name:
+ path = base_dir / url_name
+
+ if not os.path.exists(path):
+ path = base_dir
+
+ for filepath in glob.glob(path/ '*'):
+ with open(filepath) as f:
+ try:
+ html = f.read().decode('utf-8')
+ # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix
+ slug = os.path.splitext(os.path.basename(filepath))[0]
+ loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug)
+ module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc})
+ module.metadata['data_dir'] = course_dir
+ self.modules[course_descriptor.id][module.location] = module
+ except Exception, e:
+ logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e)))
def get_instance(self, course_id, location, depth=0):
"""
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index bced371001a..6a7d44489bf 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -1,6 +1,7 @@
import logging
import os
import mimetypes
+from lxml.html import rewrite_links as lxml_rewrite_links
from .xml import XMLModuleStore
from .exceptions import DuplicateItemError
@@ -9,7 +10,7 @@ from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX
log = logging.getLogger(__name__)
-def import_static_content(modules, data_dir, static_content_store):
+def import_static_content(modules, data_dir, static_content_store, target_location_namespace):
remap_dict = {}
@@ -31,15 +32,13 @@ def import_static_content(modules, data_dir, static_content_store):
# now import all static assets
static_dir = '{0}/static/'.format(course_data_dir)
- logging.debug("Importing static assets in {0}".format(static_dir))
-
for dirname, dirnames, filenames in os.walk(static_dir):
for filename in filenames:
try:
content_path = os.path.join(dirname, filename)
fullname_with_subpath = content_path.replace(static_dir, '') # strip away leading path from the name
- content_loc = StaticContent.compute_location(course_loc.org, course_loc.course, fullname_with_subpath)
+ content_loc = StaticContent.compute_location(target_location_namespace.org, target_location_namespace.course, fullname_with_subpath)
mime_type = mimetypes.guess_type(filename)[0]
f = open(content_path, 'rb')
@@ -59,12 +58,50 @@ def import_static_content(modules, data_dir, static_content_store):
#store the remapping information which will be needed to subsitute in the module data
remap_dict[fullname_with_subpath] = content_loc.name
-
except:
- raise
+ raise
return remap_dict
+def verify_content_links(module, base_dir, static_content_store, link, remap_dict = None):
+ if link.startswith('/static/'):
+ # yes, then parse out the name
+ path = link[len('/static/'):]
+
+ static_pathname = base_dir / path
+
+ if os.path.exists(static_pathname):
+ try:
+ content_loc = StaticContent.compute_location(module.location.org, module.location.course, path)
+ filename = os.path.basename(path)
+ mime_type = mimetypes.guess_type(filename)[0]
+
+ f = open(static_pathname, 'rb')
+ data = f.read()
+ f.close()
+
+ content = StaticContent(content_loc, filename, mime_type, data)
+
+ # first let's save a thumbnail so we can get back a thumbnail location
+ thumbnail_content = static_content_store.generate_thumbnail(content)
+
+ if thumbnail_content is not None:
+ content.thumbnail_location = thumbnail_content.location
+
+ #then commit the content
+ static_content_store.save(content)
+
+ new_link = StaticContent.get_url_path_from_location(content_loc)
+
+ if remap_dict is not None:
+ remap_dict[link] = new_link
+
+ return new_link
+ except Exception, e:
+ logging.exception('Skipping failed content load from {0}. Exception: {1}'.format(path, e))
+
+ return link
+
def import_from_xml(store, data_dir, course_dirs=None,
default_class='xmodule.raw_module.RawDescriptor',
load_error_modules=True, static_content_store=None, target_location_namespace = None):
@@ -94,15 +131,20 @@ def import_from_xml(store, data_dir, course_dirs=None,
# method on XmlModuleStore.
course_items = []
for course_id in module_store.modules.keys():
- remap_dict = {}
+
+ course_data_dir = None
+ for module in module_store.modules[course_id].itervalues():
+ if module.category == 'course':
+ course_data_dir = module.metadata['data_dir']
+
if static_content_store is not None:
- remap_dict = import_static_content(module_store.modules[course_id], data_dir, static_content_store)
+ import_static_content(module_store.modules[course_id], data_dir, static_content_store,
+ target_location_namespace if target_location_namespace is not None else module_store.modules[course_id].location)
for module in module_store.modules[course_id].itervalues():
# remap module to the new namespace
if target_location_namespace is not None:
-
# This looks a bit wonky as we need to also change the 'name' of the imported course to be what
# the caller passed in
if module.location.category != 'course':
@@ -120,8 +162,8 @@ def import_from_xml(store, data_dir, course_dirs=None,
child_loc = Location(child)
new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
course=target_location_namespace.course)
-
- new_locs.append(new_child_loc)
+
+ new_locs.append(new_child_loc.url())
module.definition['children'] = new_locs
@@ -129,22 +171,37 @@ def import_from_xml(store, data_dir, course_dirs=None,
if module.category == 'course':
# HACK: for now we don't support progress tabs. There's a special metadata configuration setting for this.
module.metadata['hide_progress_tab'] = True
+
+ # a bit of a hack, but typically the "course image" which is shown on marketing pages is hard coded to /images/course_image.jpg
+ # so let's make sure we import in case there are no other references to it in the modules
+ verify_content_links(module, data_dir / course_data_dir, static_content_store, '/static/images/course_image.jpg')
+
course_items.append(module)
if 'data' in module.definition:
module_data = module.definition['data']
- # cdodge: update any references to the static content paths
- # This is a bit brute force - simple search/replace - but it's unlikely that such references to '/static/....'
- # would occur naturally (in the wild)
- # @TODO, sorry a bit of technical debt here. There are some helper methods in xmodule_modifiers.py and static_replace.py which could
- # better do the url replace on the html rendering side rather than on the ingest side
- try:
- if '/static/' in module_data:
- for subkey in remap_dict.keys():
- module_data = module_data.replace('/static/' + subkey, 'xasset:' + remap_dict[subkey])
- except:
- pass # part of the techincal debt is that module_data might not be a string (e.g. ABTest)
+ # cdodge: now go through any link references to '/static/' and make sure we've imported
+ # it as a StaticContent asset
+ try:
+ remap_dict = {}
+
+ # use the rewrite_links as a utility means to enumerate through all links
+ # in the module data. We use that to load that reference into our asset store
+ # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to
+ # do the rewrites natively in that code.
+ # For example, what I'm seeing is <img src='foo.jpg' /> -> <img src='bar.jpg'>
+ # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's
+ # no good, so we have to do this kludge
+ if isinstance(module_data, str) or isinstance(module_data, unicode): # some module 'data' fields are non strings which blows up the link traversal code
+ lxml_rewrite_links(module_data, lambda link: verify_content_links(module, data_dir / course_data_dir,
+ static_content_store, link, remap_dict))
+
+ for key in remap_dict.keys():
+ module_data = module_data.replace(key, remap_dict[key])
+
+ except Exception, e:
+ logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
store.update_item(module.location, module_data)
@@ -156,4 +213,6 @@ def import_from_xml(store, data_dir, course_dirs=None,
# inherited metadata everywhere.
store.update_metadata(module.location, dict(module.own_metadata))
+
+
return module_store, course_items
diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py
index 4df1c4ee3a7..cca2cb0ca8b 100644
--- a/common/lib/xmodule/xmodule/template_module.py
+++ b/common/lib/xmodule/xmodule/template_module.py
@@ -2,6 +2,7 @@ from xmodule.x_module import XModule
from xmodule.raw_module import RawDescriptor
from lxml import etree
from mako.template import Template
+from xmodule.modulestore.django import modulestore
class CustomTagModule(XModule):
@@ -40,8 +41,7 @@ class CustomTagDescriptor(RawDescriptor):
module_class = CustomTagModule
template_dir_name = 'customtag'
- @staticmethod
- def render_template(system, xml_data):
+ def render_template(self, system, xml_data):
'''Render the template, given the definition xml_data'''
xmltree = etree.fromstring(xml_data)
if 'impl' in xmltree.attrib:
@@ -57,15 +57,23 @@ class CustomTagDescriptor(RawDescriptor):
.format(location))
params = dict(xmltree.items())
- with system.resources_fs.open('custom_tags/{name}'
- .format(name=template_name)) as template:
- return Template(template.read()).render(**params)
+
+ # cdodge: look up the template as a module
+ template_loc = self.location._replace(category='custom_tag_template', name=template_name)
+
+ template_module = modulestore().get_item(template_loc)
+ template_module_data = template_module.definition['data']
+ template = Template(template_module_data)
+ return template.render(**params)
def __init__(self, system, definition, **kwargs):
'''Render and save the template for this descriptor instance'''
super(CustomTagDescriptor, self).__init__(system, definition, **kwargs)
- self.rendered_html = self.render_template(system, definition['data'])
+
+ @property
+ def rendered_html(self):
+ return self.render_template(self.system, self.definition['data'])
def export_to_file(self):
"""
diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py
index 0103e9de004..c4dc6fa77b9 100644
--- a/lms/djangoapps/courseware/courses.py
+++ b/lms/djangoapps/courseware/courses.py
@@ -14,6 +14,7 @@ from module_render import get_module
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore import Location
from xmodule.modulestore.django import modulestore
+from xmodule.contentstore.content import StaticContent
from xmodule.modulestore.xml import XMLModuleStore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.x_module import XModule
@@ -68,8 +69,13 @@ def get_opt_course_with_access(user, course_id, action):
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"""
- path = course.metadata['data_dir'] + "/images/course_image.jpg"
- return try_staticfiles_lookup(path)
+ if isinstance(modulestore(), XMLModuleStore):
+ path = course.metadata['data_dir'] + "/images/course_image.jpg"
+ return try_staticfiles_lookup(path)
+ else:
+ loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg')
+ path = StaticContent.get_url_path_from_location(loc)
+ return path
def find_file(fs, dirs, filename):
"""
@@ -123,14 +129,12 @@ def get_course_about_section(course, section_key):
'effort', 'end_date', 'prerequisites', 'ocw_links']:
try:
- fs = course.system.resources_fs
- # first look for a run-specific version
- dirs = [path("about") / course.url_name, path("about")]
- filepath = find_file(fs, dirs, section_key + ".html")
- with fs.open(filepath) as htmlFile:
- return replace_urls(htmlFile.read().decode('utf-8'),
- course.metadata['data_dir'])
- except ResourceNotFoundError:
+ loc = course.location._replace(category='about', name=section_key)
+ item = modulestore().get_instance(course.id, loc)
+
+ return item.definition['data']
+
+ except ItemNotFoundError:
log.warning("Missing about section {key} in course {url}".format(
key=section_key, url=course.location.url()))
return None
@@ -160,8 +164,6 @@ def get_course_info_section(request, cache, course, section_key):
loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key)
course_module = get_module(request.user, request, loc, cache, course.id)
- logging.debug('course_module = {0}'.format(course_module))
-
html = ''
if course_module is not None:
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 3c9958b36d9..d9f87d77b6c 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -256,7 +256,8 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
module.get_html = replace_static_urls(
wrap_xmodule(module.get_html, module, 'xmodule_display.html'),
- module.metadata['data_dir'] if 'data_dir' in module.metadata else '')
+ module.metadata['data_dir'] if 'data_dir' in module.metadata else '',
+ course_namespace = module.location._replace(category=None, name=None))
# Allow URLs of the form '/course/' refer to the root of multicourse directory
# hierarchy of this course
diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py
index 3de344b1566..8bdcf59815e 100644
--- a/lms/djangoapps/courseware/tests/tests.py
+++ b/lms/djangoapps/courseware/tests/tests.py
@@ -247,6 +247,7 @@ class PageLoader(ActivateLoginTestCase):
all_ok = True
for descriptor in modstore.get_items(
Location(None, None, None, None, None)):
+
n += 1
print "Checking ", descriptor.location.url()
#print descriptor.__class__, descriptor.location
@@ -256,9 +257,13 @@ class PageLoader(ActivateLoginTestCase):
msg = str(resp.status_code)
if resp.status_code != 302:
- msg = "ERROR " + msg
- all_ok = False
- num_bad += 1
+ # cdodge: we're adding new module category as part of the Studio work
+ # such as 'course_info', etc, which are not supposed to be jump_to'able
+ # so a successful return value here should be a 404
+ if descriptor.location.category not in ['about', 'static_tab', 'course_info', 'custom_tag_template'] or resp.status_code != 404:
+ msg = "ERROR " + msg
+ all_ok = False
+ num_bad += 1
print msg
self.assertTrue(all_ok) # fail fast
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index fc834768e7e..bacc41fdf49 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -421,7 +421,7 @@ def course_about(request, course_id):
settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'))
return render_to_response('portal/course_about.html',
- {'course': course,
+ { 'course': course,
'registered': registered,
'course_target': course_target,
'show_courseware_link' : show_courseware_link})
--
GitLab