diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 84e1bb474b..01a132a9b5 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -17,6 +17,10 @@ captions.
CMS: Allow editors to delete uploaded files/assets
+XModules: `XModuleDescriptor.__init__` and `XModule.__init__` dropped the
+`location` parameter (and added it as a field), and renamed `system` to `runtime`,
+to accord more closely to `XBlock.__init__`
+
LMS: Some errors handling Non-ASCII data in XML courses have been fixed.
LMS: Add page-load tracking using segment-io (if SEGMENT_IO_LMS_KEY and
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index ca10d4d2a0..fee80a34ff 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -117,6 +117,8 @@ class CapaModule(CapaFields, XModule):
'''
An XModule implementing LonCapa format problems, implemented by way of
capa.capa_problem.LoncapaProblem
+
+ CapaModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__
'''
icon_class = 'problem'
@@ -131,8 +133,9 @@ class CapaModule(CapaFields, XModule):
js_module_name = "Problem"
css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]}
- def __init__(self, system, location, descriptor, model_data):
- XModule.__init__(self, system, location, descriptor, model_data)
+ def __init__(self, *args, **kwargs):
+ """ Accepts the same arguments as xmodule.x_module:XModule.__init__ """
+ XModule.__init__(self, *args, **kwargs)
due_date = self.due
diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py
index dc753a64b8..ac95567946 100644
--- a/common/lib/xmodule/xmodule/combined_open_ended_module.py
+++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py
@@ -116,6 +116,8 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
incorporates multiple children (tasks):
openendedmodule
selfassessmentmodule
+
+ CombinedOpenEndedModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__
"""
STATE_VERSION = 1
@@ -139,8 +141,7 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]}
- def __init__(self, system, location, descriptor, model_data):
- XModule.__init__(self, system, location, descriptor, model_data)
+ def __init__(self, *args, **kwargs):
"""
Definition file should have one or many task blocks, a rubric block, and a prompt block:
@@ -175,9 +176,9 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
"""
+ XModule.__init__(self, *args, **kwargs)
- self.system = system
- self.system.set('location', location)
+ self.system.set('location', self.location)
if self.task_states is None:
self.task_states = []
@@ -189,13 +190,11 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
attributes = self.student_attributes + self.settings_attributes
- static_data = {
- 'rewrite_content_links': self.rewrite_content_links,
- }
+ static_data = {}
instance_state = {k: getattr(self, k) for k in attributes}
self.child_descriptor = version_tuple.descriptor(self.system)
self.child_definition = version_tuple.descriptor.definition_from_xml(etree.fromstring(self.data), self.system)
- self.child_module = version_tuple.module(self.system, location, self.child_definition, self.child_descriptor,
+ self.child_module = version_tuple.module(self.system, self.location, self.child_definition, self.child_descriptor,
instance_state=instance_state, static_data=static_data,
attributes=attributes)
self.save_instance_data()
diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py
index e998ceb58e..a37081c447 100644
--- a/common/lib/xmodule/xmodule/error_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -94,11 +94,11 @@ class ErrorDescriptor(ErrorFields, JSONEditingDescriptor):
model_data = {
'error_msg': str(error_msg),
'contents': contents,
- 'display_name': 'Error: ' + location.name
+ 'display_name': 'Error: ' + location.name,
+ 'location': location,
}
return cls(
system,
- location,
model_data,
)
diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py
index 8abb1d7777..526fc1a0eb 100644
--- a/common/lib/xmodule/xmodule/mako_module.py
+++ b/common/lib/xmodule/xmodule/mako_module.py
@@ -18,14 +18,16 @@ class MakoModuleDescriptor(XModuleDescriptor):
Expects the descriptor to have the `mako_template` attribute set
with the name of the template to render, and it will pass
the descriptor as the `module` parameter to that template
+
+ MakoModuleDescriptor.__init__ takes the same arguments as xmodule.x_module:XModuleDescriptor.__init__
"""
- def __init__(self, system, location, model_data):
- if getattr(system, 'render_template', None) is None:
- raise TypeError('{system} must have a render_template function'
+ def __init__(self, *args, **kwargs):
+ super(MakoModuleDescriptor, self).__init__(*args, **kwargs)
+ if getattr(self.runtime, 'render_template', None) is None:
+ raise TypeError('{runtime} must have a render_template function'
' in order to use a MakoDescriptor'.format(
- system=system))
- super(MakoModuleDescriptor, self).__init__(system, location, model_data)
+ runtime=self.runtime))
def get_context(self):
"""
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index 87a995cd06..422abbdd73 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -37,15 +37,23 @@ def get_course_id_no_run(location):
return "/".join([location.org, location.course])
+class InvalidWriteError(Exception):
+ """
+ Raised to indicate that writing to a particular key
+ in the KeyValueStore is disabled
+ """
+
+
class MongoKeyValueStore(KeyValueStore):
"""
A KeyValueStore that maps keyed data access to one of the 3 data areas
known to the MongoModuleStore (data, children, and metadata)
"""
- def __init__(self, data, children, metadata):
+ def __init__(self, data, children, metadata, location):
self._data = data
self._children = children
self._metadata = metadata
+ self._location = location
def get(self, key):
if key.scope == Scope.children:
@@ -55,7 +63,9 @@ class MongoKeyValueStore(KeyValueStore):
elif key.scope == Scope.settings:
return self._metadata[key.field_name]
elif key.scope == Scope.content:
- if key.field_name == 'data' and not isinstance(self._data, dict):
+ if key.field_name == 'location':
+ return self._location
+ elif key.field_name == 'data' and not isinstance(self._data, dict):
return self._data
else:
return self._data[key.field_name]
@@ -68,7 +78,9 @@ class MongoKeyValueStore(KeyValueStore):
elif key.scope == Scope.settings:
self._metadata[key.field_name] = value
elif key.scope == Scope.content:
- if key.field_name == 'data' and not isinstance(self._data, dict):
+ if key.field_name == 'location':
+ self._location = value
+ elif key.field_name == 'data' and not isinstance(self._data, dict):
self._data = value
else:
self._data[key.field_name] = value
@@ -82,7 +94,9 @@ class MongoKeyValueStore(KeyValueStore):
if key.field_name in self._metadata:
del self._metadata[key.field_name]
elif key.scope == Scope.content:
- if key.field_name == 'data' and not isinstance(self._data, dict):
+ if key.field_name == 'location':
+ self._location = Location(None)
+ elif key.field_name == 'data' and not isinstance(self._data, dict):
self._data = None
else:
del self._data[key.field_name]
@@ -95,7 +109,9 @@ class MongoKeyValueStore(KeyValueStore):
elif key.scope == Scope.settings:
return key.field_name in self._metadata
elif key.scope == Scope.content:
- if key.field_name == 'data' and not isinstance(self._data, dict):
+ if key.field_name == 'location':
+ return True
+ elif key.field_name == 'data' and not isinstance(self._data, dict):
return True
else:
return key.field_name in self._data
@@ -171,10 +187,11 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
definition.get('data', {}),
definition.get('children', []),
metadata,
+ location,
)
model_data = DbModel(kvs, class_, None, MongoUsage(self.course_id, location))
- module = class_(self, location, model_data)
+ module = class_(self, model_data)
if self.cached_metadata is not None:
# parent container pointers don't differentiate between draft and non-draft
# so when we do the lookup, we should do so with a non-draft location
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
index 8a479fbf9e..07e6124537 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
@@ -1,11 +1,14 @@
import pymongo
from mock import Mock
-from nose.tools import assert_equals, assert_raises, assert_not_equals, with_setup, assert_false
+from nose.tools import assert_equals, assert_raises, assert_not_equals, assert_false
from pprint import pprint
+from xblock.core import Scope
+from xblock.runtime import KeyValueStore, InvalidScopeError
+
from xmodule.modulestore import Location
-from xmodule.modulestore.mongo import MongoModuleStore
+from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore
from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.templates import update_templates
@@ -114,3 +117,75 @@ class TestMongoModuleStore(object):
course.location.org == 'edx' and course.location.course == 'templates',
'{0} is a template course'.format(course)
)
+
+class TestMongoKeyValueStore(object):
+
+ def setUp(self):
+ self.data = {'foo': 'foo_value'}
+ self.location = Location('i4x://org/course/category/name@version')
+ self.children = ['i4x://org/course/child/a', 'i4x://org/course/child/b']
+ self.metadata = {'meta': 'meta_val'}
+ self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location)
+
+ def _check_read(self, key, expected_value):
+ assert_equals(expected_value, self.kvs.get(key))
+ assert self.kvs.has(key)
+
+ def test_read(self):
+ assert_equals(self.data['foo'], self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'foo')))
+ assert_equals(self.location, self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'location')))
+ assert_equals(self.children, self.kvs.get(KeyValueStore.Key(Scope.children, None, None, 'children')))
+ assert_equals(self.metadata['meta'], self.kvs.get(KeyValueStore.Key(Scope.settings, None, None, 'meta')))
+ assert_equals(None, self.kvs.get(KeyValueStore.Key(Scope.parent, None, None, 'parent')))
+
+ def test_read_invalid_scope(self):
+ for scope in (Scope.preferences, Scope.user_info, Scope.user_state):
+ key = KeyValueStore.Key(scope, None, None, 'foo')
+ with assert_raises(InvalidScopeError):
+ self.kvs.get(key)
+ assert_false(self.kvs.has(key))
+
+ def test_read_non_dict_data(self):
+ self.kvs._data = 'xml_data'
+ assert_equals('xml_data', self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'data')))
+
+ def _check_write(self, key, value):
+ self.kvs.set(key, value)
+ assert_equals(value, self.kvs.get(key))
+
+ def test_write(self):
+ yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'foo'), 'new_data')
+ yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'location'), Location('i4x://org/course/category/name@new_version'))
+ yield (self._check_write, KeyValueStore.Key(Scope.children, None, None, 'children'), [])
+ yield (self._check_write, KeyValueStore.Key(Scope.settings, None, None, 'meta'), 'new_settings')
+
+ def test_write_non_dict_data(self):
+ self.kvs._data = 'xml_data'
+ self._check_write(KeyValueStore.Key(Scope.content, None, None, 'data'), 'new_data')
+
+ def test_write_invalid_scope(self):
+ for scope in (Scope.preferences, Scope.user_info, Scope.user_state, Scope.parent):
+ with assert_raises(InvalidScopeError):
+ self.kvs.set(KeyValueStore.Key(scope, None, None, 'foo'), 'new_value')
+
+ def _check_delete_default(self, key, default_value):
+ self.kvs.delete(key)
+ assert_equals(default_value, self.kvs.get(key))
+ assert self.kvs.has(key)
+
+ def _check_delete_key_error(self, key):
+ self.kvs.delete(key)
+ with assert_raises(KeyError):
+ self.kvs.get(key)
+ assert_false(self.kvs.has(key))
+
+ def test_delete(self):
+ yield (self._check_delete_key_error, KeyValueStore.Key(Scope.content, None, None, 'foo'))
+ yield (self._check_delete_default, KeyValueStore.Key(Scope.content, None, None, 'location'), Location(None))
+ yield (self._check_delete_default, KeyValueStore.Key(Scope.children, None, None, 'children'), [])
+ yield (self._check_delete_key_error, KeyValueStore.Key(Scope.settings, None, None, 'meta'))
+
+ def test_delete_invalid_scope(self):
+ for scope in (Scope.preferences, Scope.user_info, Scope.user_state, Scope.parent):
+ with assert_raises(InvalidScopeError):
+ self.kvs.delete(KeyValueStore.Key(scope, None, None, 'foo'))
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 6ab6843216..a704fc2ae8 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -463,7 +463,7 @@ class XMLModuleStore(ModuleStoreBase):
# 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, loc, {'data': html})
+ module = HtmlDescriptor(system, {'data': html, 'location': loc})
# VS[compat]:
# Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them)
# from the course policy
diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py
index b48c70c8d5..45e73442d0 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py
@@ -117,7 +117,6 @@ class CombinedOpenEndedV1Module():
self.instance_state = instance_state
self.display_name = instance_state.get('display_name', "Open Ended")
- self.rewrite_content_links = static_data.get('rewrite_content_links', "")
#We need to set the location here so the child modules can use it
system.set('location', location)
@@ -354,17 +353,7 @@ class CombinedOpenEndedV1Module():
Output: Child task HTML
"""
self.update_task_states()
- html = self.current_task.get_html(self.system)
- return_html = html
- try:
- #Without try except block, get this error:
- # File "/home/vik/mitx_all/mitx/common/lib/xmodule/xmodule/x_module.py", line 263, in rewrite_content_links
- # if link.startswith(XASSET_SRCREF_PREFIX):
- # Placing try except so that if the error is fixed, this code will start working again.
- return_html = rewrite_links(html, self.rewrite_content_links)
- except Exception:
- pass
- return return_html
+ return self.current_task.get_html(self.system)
def get_current_attributes(self, task_number):
"""
diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py
index 979bc95cb0..a13fef8e40 100644
--- a/common/lib/xmodule/xmodule/peer_grading_module.py
+++ b/common/lib/xmodule/xmodule/peer_grading_module.py
@@ -62,6 +62,9 @@ class PeerGradingFields(object):
class PeerGradingModule(PeerGradingFields, XModule):
+ """
+ PeerGradingModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__
+ """
_VERSION = 1
js = {'coffee': [resource_string(__name__, 'js/src/peergrading/peer_grading.coffee'),
@@ -73,12 +76,11 @@ class PeerGradingModule(PeerGradingFields, XModule):
css = {'scss': [resource_string(__name__, 'css/combinedopenended/display.scss')]}
- def __init__(self, system, location, descriptor, model_data):
- XModule.__init__(self, system, location, descriptor, model_data)
+ def __init__(self, *args, **kwargs):
+ super(PeerGradingModule, self).__init__(*args, **kwargs)
- # We need to set the location here so the child modules can use it
- system.set('location', location)
- self.system = system
+ #We need to set the location here so the child modules can use it
+ self.runtime.set('location', self.location)
if (self.system.open_ended_grading_interface):
self.peer_gs = PeerGradingService(self.system.open_ended_grading_interface, self.system)
else:
diff --git a/common/lib/xmodule/xmodule/tests/test_annotatable_module.py b/common/lib/xmodule/xmodule/tests/test_annotatable_module.py
index 43eae8e43e..268f743421 100644
--- a/common/lib/xmodule/xmodule/tests/test_annotatable_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_annotatable_module.py
@@ -29,10 +29,10 @@ class AnnotatableModuleTestCase(unittest.TestCase):
'''
descriptor = Mock()
- module_data = {'data': sample_xml}
+ module_data = {'data': sample_xml, 'location': location}
def setUp(self):
- self.annotatable = AnnotatableModule(test_system(), self.location, self.descriptor, self.module_data)
+ self.annotatable = AnnotatableModule(test_system(), self.descriptor, self.module_data)
def test_annotation_data_attr(self):
el = etree.fromstring('