diff --git a/common/djangoapps/mitxmako/shortcuts.py b/common/djangoapps/mitxmako/shortcuts.py index 181d3befd578a65942c37f1b98d5936838c539f8..8f39efdc02b3da789e3a5ecaeb7ecb6595ad110a 100644 --- a/common/djangoapps/mitxmako/shortcuts.py +++ b/common/djangoapps/mitxmako/shortcuts.py @@ -14,7 +14,7 @@ import logging -log = logging.getLogger("mitx." + __name__) +log = logging.getLogger(__name__) from django.template import Context from django.http import HttpResponse diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 5a13582a2dfcafd85926cd7f11a1341f2e646748..30098c94fc8052a36f7bb148951139064116a702 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -131,6 +131,7 @@ def add_histogram(get_html, module, user): is_released = "<font color='red'>Yes!</font>" if (now > mstart) else "<font color='green'>Not yet</font>" staff_context = {'fields': [(field.name, getattr(module, field.name)) for field in module.fields], + 'lms_fields': [(field.name, getattr(module.lms, field.name)) for field in module.lms.fields], 'location': module.location, 'xqa_key': module.lms.xqa_key, 'source_file' : source_file, diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 85670063c58a9bf5cf03fd10812860b2d9953bf2..d5ec6a1439e09c0b65aac0e0e5f8746118f7993e 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -72,7 +72,7 @@ global_context = {'random': random, # These should be removed from HTML output, including all subelements html_problem_semantics = ["codeparam", "responseparam", "answer", "script", "hintgroup"] -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) #----------------------------------------------------------------------------- # main class for this module diff --git a/common/lib/capa/capa/customrender.py b/common/lib/capa/capa/customrender.py index ef1044e8b1f85b0b98af1213892a17470647d98f..0268d7b266ab6ba41c547f85dc9392d63d63d362 100644 --- a/common/lib/capa/capa/customrender.py +++ b/common/lib/capa/capa/customrender.py @@ -17,7 +17,7 @@ from lxml import etree import xml.sax.saxutils as saxutils from registry import TagRegistry -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) registry = TagRegistry() diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 0b2250f98d10cccf0712d8b168bef4c2bde97a99..8a15434d1d56d5cebab32e27878215d120b25258 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -44,7 +44,7 @@ import sys from registry import TagRegistry -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) ######################################################################### diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 418ee9d8aeb019256754fb8bee7071ab3c67faaf..2074e8f6484b7cf71bdb8b6043672c2291a7ea56 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -33,7 +33,7 @@ from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? import xqueue_interface -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) #----------------------------------------------------------------------------- diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 0214488cce0d06ede6bb78d8cd908e1b3d94640a..a7d31db7e1af2793854b466e03939eabd00ba8d4 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -7,7 +7,7 @@ import logging import requests -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) dateformat = '%Y%m%d%H%M%S' def make_hashkey(seed): diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index b8a85595dc33bd1c959be8feabd3ad8c536932a0..36da9299267674bd01608a89c20102be5f8b0f82 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -551,8 +551,7 @@ class CapaModule(XModule): msg = "Error checking problem: " + str(err) msg += '\nTraceback:\n' + traceback.format_exc() return {'success': msg} - log.exception("Error in capa_module problem checking") - raise Exception("error in capa_module") + raise self.attempts = self.attempts + 1 self.lcp.done = True diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index b2a191791292d792e88b2e01cb8799ba2724865b..b8d4032a18b80369ee835fa2bdf9e09e3027a5a0 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -131,9 +131,9 @@ class CourseDescriptor(SequenceDescriptor): if self.start is None: msg = "Course loaded without a valid start date. id = %s" % self.id # hack it -- start in 1970 - self.lms.start = time.gmtime(0) + self.start = time.gmtime(0) log.critical(msg) - system.error_tracker(msg) + self.system.error_tracker(msg) # NOTE: relies on the modulestore to call set_grading_policy() right after # init. (Modulestore is in charge of figuring out where to load the policy from) diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 0f95bcd2566142faeaeaa24343ef484efc0df725..67b52d383cc62601ae83e94c9d8caf3c3687ee17 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -8,6 +8,7 @@ from xmodule.x_module import XModule from xmodule.editing_module import JSONEditingDescriptor from xmodule.errortracker import exc_info_to_str from xmodule.modulestore import Location +from .model import String, Scope log = logging.getLogger(__name__) @@ -52,6 +53,10 @@ class ErrorDescriptor(JSONEditingDescriptor): """ module_class = ErrorModule + contents = String(scope=Scope.content) + error_msg = String(scope=Scope.content) + display_name = String(scope=Scope.settings) + @classmethod def _construct(self, system, contents, error_msg, location): @@ -66,15 +71,12 @@ class ErrorDescriptor(JSONEditingDescriptor): name=hashlib.sha1(contents).hexdigest() ) - definition = { - 'data': { - 'error_msg': str(error_msg), - 'contents': contents, - } - } - # real metadata stays in the content, but add a display name - model_data = {'display_name': 'Error: ' + location.name} + model_data = { + 'error_msg': str(error_msg), + 'contents': contents, + 'display_name': 'Error: ' + location.name + } return ErrorDescriptor( system, location, @@ -84,7 +86,7 @@ class ErrorDescriptor(JSONEditingDescriptor): def get_context(self): return { 'module': self, - 'data': self.definition['data']['contents'], + 'data': self.contents, } @classmethod @@ -100,10 +102,7 @@ class ErrorDescriptor(JSONEditingDescriptor): def from_descriptor(cls, descriptor, error_msg='Error not available'): return cls._construct( descriptor.system, - json.dumps({ - 'definition': descriptor.definition, - 'metadata': descriptor.metadata, - }, indent=4), + json.dumps(descriptor._model_data, indent=4), error_msg, location=descriptor.location, ) @@ -147,14 +146,14 @@ class ErrorDescriptor(JSONEditingDescriptor): files, etc. That would just get re-wrapped on import. ''' try: - xml = etree.fromstring(self.definition['data']['contents']) + xml = etree.fromstring(self.contents) return etree.tostring(xml) except etree.XMLSyntaxError: # still not valid. root = etree.Element('error') - root.text = self.definition['data']['contents'] + root.text = self.contents err_node = etree.SubElement(root, 'error_msg') - err_node.text = self.definition['data']['error_msg'] + err_node.text = self.error_msg return etree.tostring(root) diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py new file mode 100644 index 0000000000000000000000000000000000000000..715aea0c7cb9d8dcaaf46f252fd9457b3bb9c3e9 --- /dev/null +++ b/common/lib/xmodule/xmodule/fields.py @@ -0,0 +1,30 @@ +import time +import logging + +from .model import ModelType + +log = logging.getLogger(__name__) + + +class Date(ModelType): + time_format = "%Y-%m-%dT%H:%M" + + def from_json(self, value): + """ + Parse an optional metadata key containing a time: if present, complain + if it doesn't parse. + Return None if not present or invalid. + """ + try: + return time.strptime(value, self.time_format) + except ValueError as e: + msg = "Field {0} has bad value '{1}': '{2}'".format( + self._name, value, e) + log.warning(msg) + return None + + def to_json(self, value): + """ + Convert a time struct to a string + """ + return time.strftime(self.time_format, value) diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py index 89fc9d56d13540f429e40b3e8a62a6be713c5f93..93ed12f69d1281eb9dc7a646ba6e6346be8b28ee 100644 --- a/common/lib/xmodule/xmodule/model.py +++ b/common/lib/xmodule/xmodule/model.py @@ -43,14 +43,14 @@ class ModelType(object): if instance is None: return self - if self.name not in instance._model_data: + try: + return self.from_json(instance._model_data[self.name]) + except KeyError: if self.default is None and self.computed_default is not None: return self.computed_default(instance) return self.default - return self.from_json(instance._model_data[self.name]) - def __set__(self, instance, value): instance._model_data[self.name] = self.to_json(value) @@ -166,18 +166,18 @@ class Namespace(Plugin): super(Namespace, self).__setattr__(name, value) return - container_class_attr = getattr(type(container), name, None) + namespace_attr = getattr(type(self), name, None) - if container_class_attr is None or not isinstance(container_class_attr, ModelType): + if namespace_attr is None or not isinstance(namespace_attr, ModelType): return super(Namespace, self).__setattr__(name, value) - return container_class_attr.__set__(container) + return namespace_attr.__set__(container, value) def __delattr__(self, name): container = super(Namespace, self).__getattribute__('_container') - container_class_attr = getattr(type(container), name, None) + namespace_attr = getattr(type(self), name, None) - if container_class_attr is None or not isinstance(container_class_attr, ModelType): + if namespace_attr is None or not isinstance(namespace_attr, ModelType): return super(Namespace, self).__detattr__(name) - return container_class_attr.__delete__(container) + return namespace_attr.__delete__(container) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index ce6d9df093a9396af234ae642771b2789b86fcda..72c9093bbfd05163a55b75859870d2983da6a0d3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -28,7 +28,7 @@ edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, etree.set_default_parser(edx_xml_parser) -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) # VS[compat] @@ -160,7 +160,6 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): etree.tostring(xml_data), self, self.org, self.course, xmlstore.default_class) except Exception as err: - print err, self.load_error_modules if not self.load_error_modules: raise diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 35375d7c51e66b9a20a46ef18eb53e46be9f886f..f90291aaa2143333ee7854effa3eb79c123f7317 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -8,6 +8,7 @@ from .xml import XMLModuleStore from .exceptions import DuplicateItemError from xmodule.modulestore import Location from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX +from xmodule.model import Scope log = logging.getLogger(__name__) @@ -123,7 +124,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # Quick scan to get course Location as well as the course_data_path for module in module_store.modules[course_id].itervalues(): if module.category == 'course': - course_data_path = path(data_dir) / module.metadata['data_dir'] + course_data_path = path(data_dir) / module.data_dir course_location = module.location if static_content_store is not None: @@ -159,18 +160,17 @@ def import_from_xml(store, data_dir, course_dirs=None, module.definition['children'] = new_locs - 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 + module.hide_progress_tab = True - # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which - # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS, - # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that - + # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which + # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS, + # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that - # if there is *any* tabs - then there at least needs to be some predefined ones if module.tabs is None or len(module.tabs) == 0: - module.tabs = [{"type": "courseware"}, - {"type": "course_info", "name": "Course Info"}, + module.tabs = [{"type": "courseware"}, + {"type": "course_info", "name": "Course Info"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge @@ -180,39 +180,43 @@ def import_from_xml(store, data_dir, course_dirs=None, course_items.append(module) - if 'data' in module.definition: - module_data = module.definition['data'] - + if hasattr(module, 'data'): # cdodge: now go through any link references to '/static/' and make sure we've imported # it as a StaticContent asset - try: + 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'> + # 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, course_data_path, - static_content_store, link, remap_dict)) + 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, course_data_path, + static_content_store, link, remap_dict)) for key in remap_dict.keys(): - module_data = module_data.replace(key, remap_dict[key]) + module.data = module.data.replace(key, remap_dict[key]) - except Exception, e: + except Exception: logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location)) - store.update_item(module.location, module_data) + store.update_item(module.location, module.data) - if 'children' in module.definition: - store.update_children(module.location, module.definition['children']) + if module.has_children: + store.update_children(module.location, module.children) - # NOTE: It's important to use own_metadata here to avoid writing - # inherited metadata everywhere. - store.update_metadata(module.location, dict(module.own_metadata)) + metadata = {} + for field in module.fields + module.lms.fields: + # Only save metadata that wasn't inherited + if (field.scope == Scope.settings and + field.name in module._inherited_metadata and + field.name in module._model_data): + + metadata[field.name] = module._model_data[field.name] + store.update_metadata(module.location, metadata) return module_store, course_items diff --git a/common/lib/xmodule/xmodule/runtime.py b/common/lib/xmodule/xmodule/runtime.py index 8be4bfe34686ef6082913804b7a9d0dc2f803ee6..7030f04f9fff756398113a8ac80bc8eedcfee017 100644 --- a/common/lib/xmodule/xmodule/runtime.py +++ b/common/lib/xmodule/xmodule/runtime.py @@ -33,19 +33,33 @@ class DbModel(MutableMapping): def __repr__(self): return "<{0.__class__.__name__} {0._module_cls!r}>".format(self) - def __str__(self): - return str(dict(self.iteritems())) - def _getfield(self, name): - if (not hasattr(self._module_cls, name) or - not isinstance(getattr(self._module_cls, name), ModelType)): - - raise KeyError(name) + # First, get the field from the class, if defined + module_field = getattr(self._module_cls, name, None) + if module_field is not None and isinstance(module_field, ModelType): + return module_field + + # If the class doesn't have the field, and it also + # doesn't have any namespaces, then the the name isn't a field + # so KeyError + if not hasattr(self._module_cls, 'namespaces'): + return KeyError(name) + + # Resolve the field name in the first namespace where it's + # available + for namespace_name in self._module_cls.namespaces: + namespace = getattr(self._module_cls, namespace_name) + namespace_field = getattr(type(namespace), name, None) + if namespace_field is not None and isinstance(module_field, ModelType): + return namespace_field - return getattr(self._module_cls, name) + # Not in the class or in any of the namespaces, so name + # really doesn't name a field + raise KeyError(name) def _key(self, name): field = self._getfield(name) + print name, field module = field.scope.module if module == ModuleScope.ALL: @@ -88,5 +102,5 @@ class DbModel(MutableMapping): def keys(self): fields = [field.name for field in self._module_cls.fields] for namespace_name in self._module_cls.namespaces: - fields.extend(field.name for field in getattr(self._module_cls, namespace_name)) + fields.extend(field.name for field in getattr(self._module_cls, namespace_name).fields) return fields diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index 2edf5467b27104ea5a3b7b30bdb567abe93d1b03..2f4674cf7c71b7e77d61deee6ddab4755343644a 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -25,6 +25,7 @@ from .stringify import stringify_children from .x_module import XModule from .xml_module import XmlDescriptor from xmodule.modulestore import Location +from .model import List, String, Scope, Int log = logging.getLogger("mitx.courseware") @@ -61,67 +62,21 @@ class SelfAssessmentModule(XModule): js = {'coffee': [resource_string(__name__, 'js/src/selfassessment/display.coffee')]} js_module_name = "SelfAssessment" - def __init__(self, system, location, definition, descriptor, - instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) + student_answers = List(scope=Scope.student_state, default=[]) + scores = List(scope=Scope.student_state, default=[]) + hints = List(scope=Scope.student_state, default=[]) + state = String(scope=Scope.student_state, default=INITIAL) - """ - Definition file should have 4 blocks -- prompt, rubric, submitmessage, hintprompt, - and two optional attributes: - attempts, which should be an integer that defaults to 1. - If it's > 1, the student will be able to re-submit after they see - the rubric. - max_score, which should be an integer that defaults to 1. - It defines the maximum number of points a student can get. Assumed to be integer scale - from 0 to max_score, with an interval of 1. - - Note: all the submissions are stored. - - Sample file: - - <selfassessment attempts="1" max_score="1"> - <prompt> - Insert prompt text here. (arbitrary html) - </prompt> - <rubric> - Insert grading rubric here. (arbitrary html) - </rubric> - <hintprompt> - Please enter a hint below: (arbitrary html) - </hintprompt> - <submitmessage> - Thanks for submitting! (arbitrary html) - </submitmessage> - </selfassessment> - """ - - # Load instance state - if instance_state is not None: - instance_state = json.loads(instance_state) - else: - instance_state = {} - - # Note: score responses are on scale from 0 to max_score - self.student_answers = instance_state.get('student_answers', []) - self.scores = instance_state.get('scores', []) - self.hints = instance_state.get('hints', []) - - self.state = instance_state.get('state', 'initial') - - # Used for progress / grading. Currently get credit just for - # completion (doesn't matter if you self-assessed correct/incorrect). + # Used for progress / grading. Currently get credit just for + # completion (doesn't matter if you self-assessed correct/incorrect). + max_score = Int(scope=Scope.settings, default=MAX_SCORE) - self._max_score = int(self.metadata.get('max_score', MAX_SCORE)) - - self.attempts = instance_state.get('attempts', 0) - - self.max_attempts = int(self.metadata.get('attempts', MAX_ATTEMPTS)) - - self.rubric = definition['rubric'] - self.prompt = definition['prompt'] - self.submit_message = definition['submitmessage'] - self.hint_prompt = definition['hintprompt'] + attempts = Int(scope=Scope.student_state, default=0), Int + max_attempts = Int(scope=Scope.settings, default=MAX_ATTEMPTS) + rubric = String(scope=Scope.content) + prompt = String(scope=Scope.content) + submit_message = String(scope=Scope.content) + hint_prompt = String(scope=Scope.content) def _allow_reset(self): """Can the module be reset?""" @@ -432,6 +387,21 @@ class SelfAssessmentDescriptor(XmlDescriptor, EditingDescriptor): js = {'coffee': [resource_string(__name__, 'js/src/html/edit.coffee')]} js_module_name = "HTMLEditingDescriptor" + # The capa format specifies that what we call max_attempts in the code + # is the attribute `attempts`. This will do that conversion + metadata_translations = dict(XmlDescriptor.metadata_translations) + metadata_translations['attempts'] = 'max_attempts' + + # Used for progress / grading. Currently get credit just for + # completion (doesn't matter if you self-assessed correct/incorrect). + max_score = Int(scope=Scope.settings, default=MAX_SCORE) + + max_attempts = Int(scope=Scope.settings, default=MAX_ATTEMPTS) + rubric = String(scope=Scope.content) + prompt = String(scope=Scope.content) + submit_message = String(scope=Scope.content) + hint_prompt = String(scope=Scope.content) + @classmethod def definition_from_xml(cls, xml_object, system): """ diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index ed64c451183415528e3de490949201687b81a1d2..c16c6d75963220bbd1fd7cdf2f8c64af67475934 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -30,7 +30,8 @@ i4xs = ModuleSystem( debug=True, xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 10}, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"), - anonymous_student_id = 'student' + anonymous_student_id = 'student', + xmodule_model_data = lambda x: x, ) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 77532959d7917fccc1a25dddd55d396bfd56f89b..d243ca1609998185b65a20eb2ea6d8d7087e3a5b 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -91,8 +91,10 @@ class ImportTestCase(unittest.TestCase): self.assertEqual(re_import_descriptor.__class__.__name__, 'ErrorDescriptor') - self.assertEqual(descriptor.definition['data'], - re_import_descriptor.definition['data']) + self.assertEqual(descriptor.contents, + re_import_descriptor.contents) + self.assertEqual(descriptor.error_msg, + re_import_descriptor.error_msg) def test_fixed_xml_tag(self): """Make sure a tag that's been fixed exports as the original tag type""" @@ -126,23 +128,19 @@ class ImportTestCase(unittest.TestCase): url_name = 'test1' start_xml = ''' <course org="{org}" course="{course}" - graceperiod="{grace}" url_name="{url_name}" unicorn="purple"> + due="{due}" url_name="{url_name}" unicorn="purple"> <chapter url="hi" url_name="ch" display_name="CH"> <html url_name="h" display_name="H">Two houses, ...</html> </chapter> - </course>'''.format(grace=v, org=ORG, course=COURSE, url_name=url_name) + </course>'''.format(due=v, org=ORG, course=COURSE, url_name=url_name) descriptor = system.process_xml(start_xml) - print descriptor, descriptor.metadata - self.assertEqual(descriptor.metadata['graceperiod'], v) - self.assertEqual(descriptor.metadata['unicorn'], 'purple') + print descriptor, descriptor._model_data + self.assertEqual(descriptor.lms.due, v) - # Check that the child inherits graceperiod correctly + # Check that the child inherits due correctly child = descriptor.get_children()[0] - self.assertEqual(child.metadata['graceperiod'], v) - - # check that the child does _not_ inherit any unicorns - self.assertTrue('unicorn' not in child.metadata) + self.assertEqual(child.lms.due, v) # Now export and check things resource_fs = MemoryFS() @@ -169,12 +167,12 @@ class ImportTestCase(unittest.TestCase): # did we successfully strip the url_name from the definition contents? self.assertTrue('url_name' not in course_xml.attrib) - # Does the chapter tag now have a graceperiod attribute? + # Does the chapter tag now have a due attribute? # hardcoded path to child with resource_fs.open('chapter/ch.xml') as f: chapter_xml = etree.fromstring(f.read()) self.assertEqual(chapter_xml.tag, 'chapter') - self.assertFalse('graceperiod' in chapter_xml.attrib) + self.assertFalse('due' in chapter_xml.attrib) def test_is_pointer_tag(self): """ @@ -216,7 +214,7 @@ class ImportTestCase(unittest.TestCase): def check_for_key(key, node): "recursive check for presence of key" print "Checking {0}".format(node.location.url()) - self.assertTrue(key in node.metadata) + self.assertTrue(key in node._model_data) for c in node.get_children(): check_for_key(key, c) @@ -244,15 +242,15 @@ class ImportTestCase(unittest.TestCase): toy_ch = toy.get_children()[0] two_toys_ch = two_toys.get_children()[0] - self.assertEqual(toy_ch.display_name, "Overview") - self.assertEqual(two_toys_ch.display_name, "Two Toy Overview") + self.assertEqual(toy_ch.lms.display_name, "Overview") + self.assertEqual(two_toys_ch.lms.display_name, "Two Toy Overview") # Also check that the grading policy loaded self.assertEqual(two_toys.grade_cutoffs['C'], 0.5999) # Also check that keys from policy are run through the # appropriate attribute maps -- 'graded' should be True, not 'true' - self.assertEqual(toy.metadata['graded'], True) + self.assertEqual(toy.lms.graded, True) def test_definition_loading(self): @@ -271,8 +269,8 @@ class ImportTestCase(unittest.TestCase): location = Location(["i4x", "edX", "toy", "video", "Welcome"]) toy_video = modulestore.get_instance(toy_id, location) two_toy_video = modulestore.get_instance(two_toy_id, location) - self.assertEqual(toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh8") - self.assertEqual(two_toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh9") + self.assertEqual(toy_video.youtube, "1.0:p2Q6BrNhdh8") + self.assertEqual(two_toy_video.youtube, "1.0:p2Q6BrNhdh9") def test_colon_in_url_name(self): @@ -306,7 +304,7 @@ class ImportTestCase(unittest.TestCase): cloc = course.location loc = Location(cloc.tag, cloc.org, cloc.course, 'html', 'secret:toylab') html = modulestore.get_instance(course_id, loc) - self.assertEquals(html.display_name, "Toy lab") + self.assertEquals(html.lms.display_name, "Toy lab") def test_url_name_mangling(self): """ @@ -351,4 +349,4 @@ class ImportTestCase(unittest.TestCase): location = Location(["i4x", "edX", "sa_test", "selfassessment", "SampleQuestion"]) sa_sample = modulestore.get_instance(sa_id, location) #10 attempts is hard coded into SampleQuestion, which is the url_name of a selfassessment xml tag - self.assertEqual(sa_sample.metadata['attempts'], '10') + self.assertEqual(sa_sample.max_attempts, '10') diff --git a/common/lib/xmodule/xmodule/tests/test_model.py b/common/lib/xmodule/xmodule/tests/test_model.py new file mode 100644 index 0000000000000000000000000000000000000000..0e42df80a1f8ce61e74d7feb533523f5b81217f0 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_model.py @@ -0,0 +1,8 @@ + +class ModelMetaclassTester(object): + __metaclass__ = ModelMetaclass + + field_a = Int(scope=Scope.settings) + field_b = Int(scope=Scope.content) + +def test_model_metaclass(): diff --git a/common/lib/xmodule/xmodule/tests/test_runtime.py b/common/lib/xmodule/xmodule/tests/test_runtime.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index a2393aa235c6d89fe53c77b8354fad2440b6a9b2..f03fbe61834ffcb13afeb97b4a80db388afaa60e 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1,7 +1,6 @@ import logging import yaml import os -import time from lxml import etree from pprint import pprint @@ -10,38 +9,14 @@ from pkg_resources import resource_listdir, resource_string, resource_isdir from xmodule.modulestore import Location -from .model import ModelMetaclass, ParentModelMetaclass, NamespacesMetaclass, ModelType +from .model import ModelMetaclass, ParentModelMetaclass, NamespacesMetaclass from .plugin import Plugin -class Date(ModelType): - time_format = "%Y-%m-%dT%H:%M" - - def from_json(self, value): - """ - Parse an optional metadata key containing a time: if present, complain - if it doesn't parse. - Return None if not present or invalid. - """ - try: - return time.strptime(value, self.time_format) - except ValueError as e: - msg = "Field {0} has bad value '{1}': '{2}'".format( - self._name, value, e) - log.warning(msg) - return None - - def to_json(self, value): - """ - Convert a time struct to a string - """ - return time.strftime(self.time_format, value) - - class XModuleMetaclass(ParentModelMetaclass, NamespacesMetaclass, ModelMetaclass): pass -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) def dummy_track(event_type, event): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 36bea6edb2a6d924db09fec13dff4510ce19a5e0..50c4f7aa6d226bbd9b472a29878b80af4d5b960a 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -311,11 +311,13 @@ class XmlDescriptor(XModuleDescriptor): # Set/override any metadata specified by policy k = policy_key(location) if k in system.policy: + if k == 'video/labintro': print k, metadata, system.policy[k] cls.apply_policy(metadata, system.policy[k]) model_data = {} model_data.update(metadata) model_data.update(definition) + if k == 'video/labintro': print model_data return cls( system, diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index f6fd7bda8852341c3f7135b1b3168dd561971cc0..032378f863690c6b10d0d78dfc79b16c3d3ba9cb 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -148,7 +148,7 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F format_scores = [] for section in sections: section_descriptor = section['section_descriptor'] - section_name = section_descriptor.display_name + section_name = section_descriptor.lms.display_name should_grade_section = False # If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0% @@ -276,15 +276,13 @@ def progress_summary(student, request, course, student_module_cache): # Don't include chapters that aren't displayable (e.g. due to error) for chapter_module in course_module.get_display_items(): # Skip if the chapter is hidden - hidden = chapter_module._model_data.get('hide_from_toc','false') - if hidden.lower() == 'true': + if chapter_module.lms.hide_from_toc: continue sections = [] for section_module in chapter_module.get_display_items(): # Skip if the section is hidden - hidden = section_module._model_data.get('hide_from_toc','false') - if hidden.lower() == 'true': + if section_module.lms.hide_from_toc: continue # Same for sections @@ -309,17 +307,17 @@ def progress_summary(student, request, course, student_module_cache): format = section_module.lms.format sections.append({ - 'display_name': section_module.display_name, + 'display_name': section_module.lms.display_name, 'url_name': section_module.url_name, 'scores': scores, 'section_total': section_total, 'format': format, - 'due': section_module.due, + 'due': section_module.lms.due, 'graded': graded, }) - chapters.append({'course': course.display_name, - 'display_name': chapter_module.display_name, + chapters.append({'course': course.lms.display_name, + 'display_name': chapter_module.lms.display_name, 'url_name': chapter_module.url_name, 'sections': sections}) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index f83ff35f1f068e307ea0401af8d5bb122ca002c5..be207730b3f5ad3af71cf5680ca096375ba6fa26 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -92,6 +92,7 @@ def toc_for_course(user, request, course, active_chapter, active_section): chapters = list() for chapter in course_module.get_display_items(): + print chapter, chapter._model_data, chapter._model_data.get('hide_from_toc'), chapter.lms.hide_from_toc if chapter.lms.hide_from_toc: continue diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 0b6392a7fcd2df97be3b19c2db41c512ff13e0f7..0e05ad5bc59f88e96f59cb72eed7aebaa13462a4 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -76,7 +76,7 @@ def instructor_dashboard(request, course_id): data = [['# Enrolled', CourseEnrollment.objects.filter(course_id=course_id).count()]] data += compute_course_stats(course).items() if request.user.is_staff: - data.append(['metadata', escape(str(course.metadata))]) + data.append(['metadata', escape(str(course._model_data))]) datatable['data'] = data def return_csv(fn, datatable): diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py index f50797d5e0d65934ed0f58bed5e7ffc6ed237df7..9a7043565a56c57931415b3f712d9fc3b73d630a 100644 --- a/lms/lib/comment_client/utils.py +++ b/lms/lib/comment_client/utils.py @@ -3,7 +3,7 @@ import logging import requests import settings -log = logging.getLogger('mitx.' + __name__) +log = logging.getLogger(__name__) def strip_none(dic): return dict([(k, v) for k, v in dic.iteritems() if v is not None]) diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index ad4852335f11b15bd43964b93d3ada556a0f7a28..e0c957a05cf6e8f8bc212b83d670efa3a8ba1069 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -46,9 +46,22 @@ github = <a href="${edit_link}">${edit_link | h}</a> %if source_file: source_url = <a href="${source_url}">${source_file | h}</a> %endif -%for name, field in fields: -${name} = <pre style="display:inline-block">${field | h}</pre> -%endfor +<div> + <header><h3>Module Fields</h3></header> + %for name, field in fields: + <div> + ${name} = <pre style="display:inline-block">${field | h}</pre> + </div> + %endfor +</div> +<div> + <header><h3>edX Fields</h3></header> + %for name, field in lms_fields: + <div> + ${name} = <pre style="display:inline-block">${field | h}</pre> + </div> + %endfor +</div> category = ${category | h} </div> %if render_histogram: diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py index c78052ed1b94fd3e962c36d51d642964a993c8ee..976bd4483f2e2012be7ca730082f89aad538e01e 100644 --- a/lms/xmodule_namespace.py +++ b/lms/xmodule_namespace.py @@ -1,8 +1,17 @@ from xmodule.model import Namespace, Boolean, Scope, String, List -from xmodule.x_module import Date +from xmodule.fields import Date + + +class StringyBoolean(Boolean): + def from_json(self, value): + print "StringyBoolean ", value + if isinstance(value, basestring): + return value.lower() == 'true' + return value + class LmsNamespace(Namespace): - hide_from_toc = Boolean( + hide_from_toc = StringyBoolean( help="Whether to display this module in the table of contents", default=False, scope=Scope.settings @@ -16,6 +25,7 @@ class LmsNamespace(Namespace): help="What format this module is in (used for deciding which " "grader to apply, and what to show in the TOC)", scope=Scope.settings, + default='', ) display_name = String( diff --git a/local-requirements.txt b/local-requirements.txt index 2e951a180c97d341a0efd3ab9d69ab19b8dc391d..201467d11e4f24076857279e3905439b9ddd4dee 100644 --- a/local-requirements.txt +++ b/local-requirements.txt @@ -1,4 +1,4 @@ # Python libraries to install that are local to the mitx repo -e common/lib/capa -e common/lib/xmodule --e lms +-e . diff --git a/lms/setup.py b/setup.py similarity index 85% rename from lms/setup.py rename to setup.py index 1755b7d37e90acda18af7a4eec16ef41e3179ac9..84f242cf3dac3dab3d938ddad62ae75f3cdccfcd 100644 --- a/lms/setup.py +++ b/setup.py @@ -1,13 +1,13 @@ from setuptools import setup, find_packages setup( - name="edX LMS", + name="edX Apps", version="0.1", install_requires=['distribute'], requires=[ 'xmodule', ], - + py_modules=['lms.xmodule_namespace'], # See http://guide.python-distribute.org/creation.html#entry-points # for a description of entry_points entry_points={