From de07b8b345c21905f29982d9af0890caf7210206 Mon Sep 17 00:00:00 2001 From: Calen Pennington <calen.pennington@gmail.com> Date: Wed, 27 Jun 2012 14:25:44 -0400 Subject: [PATCH] Begin using a Keystore for XML parsing. Still broken: sequence icons, custom tags, problems, video js --- common/lib/keystore/xml.py | 87 ++++++ common/lib/xmodule/__init__.py | 62 ---- common/lib/xmodule/abtest_module.py | 95 ++++++ common/lib/xmodule/capa_module.py | 39 +-- common/lib/xmodule/hidden_module.py | 10 + common/lib/xmodule/html_module.py | 73 +---- common/lib/xmodule/raw_module.py | 28 +- common/lib/xmodule/schematic_module.py | 13 - common/lib/xmodule/seq_module.py | 122 +++----- common/lib/xmodule/setup.py | 17 +- common/lib/xmodule/template_module.py | 11 +- common/lib/xmodule/vertical_module.py | 31 +- common/lib/xmodule/video_module.py | 38 +-- common/lib/xmodule/x_module.py | 150 ++++----- common/lib/xmodule/xml_module.py | 41 +++ lms/djangoapps/courseware/content_parser.py | 171 +++-------- lms/djangoapps/courseware/grades.py | 48 +-- lms/djangoapps/courseware/models.py | 102 ++++--- lms/djangoapps/courseware/module_render.py | 288 +++++++++++------- lms/djangoapps/courseware/views.py | 121 ++------ .../multicourse/multicourse_settings.py | 42 ++- lms/envs/common.py | 15 + lms/envs/dev.py | 2 +- lms/lib/dogfood/views.py | 22 +- lms/static/coffee/src/modules/sequence.coffee | 4 +- lms/templates/seq_module.html | 4 +- lms/templates/vert_module.html | 6 +- 27 files changed, 815 insertions(+), 827 deletions(-) create mode 100644 common/lib/keystore/xml.py create mode 100644 common/lib/xmodule/abtest_module.py create mode 100644 common/lib/xmodule/hidden_module.py create mode 100644 common/lib/xmodule/xml_module.py diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py new file mode 100644 index 00000000000..d5baefd7877 --- /dev/null +++ b/common/lib/keystore/xml.py @@ -0,0 +1,87 @@ +from fs.osfs import OSFS +from importlib import import_module +from lxml import etree +from path import path +from xmodule.x_module import XModuleDescriptor, XMLParsingSystem + +from . import ModuleStore, Location +from .exceptions import ItemNotFoundError + + +class XMLModuleStore(ModuleStore): + """ + An XML backed ModuleStore + """ + def __init__(self, org, course, data_dir, default_class=None): + self.data_dir = path(data_dir) + self.modules = {} + + module_path, _, class_name = default_class.rpartition('.') + class_ = getattr(import_module(module_path), class_name) + self.default_class = class_ + + with open(data_dir / "course.xml") as course_file: + class ImportSystem(XMLParsingSystem): + def __init__(self, keystore): + self.unnamed_modules = 0 + + def process_xml(xml): + try: + xml_data = etree.fromstring(xml) + except: + print xml + raise + if xml_data.get('name'): + xml_data.set('slug', Location.clean(xml_data.get('name'))) + else: + self.unnamed_modules += 1 + xml_data.set('slug', '{tag}_{count}'.format(tag=xml_data.tag, count=self.unnamed_modules)) + + module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, keystore.default_class) + keystore.modules[module.url] = module + return module + + XMLParsingSystem.__init__(self, keystore.get_item, OSFS(data_dir), process_xml) + + ImportSystem(self).process_xml(course_file.read()) + + def get_item(self, location): + """ + Returns an XModuleDescriptor instance for the item at location. + If location.revision is None, returns the most item with the most + recent revision + + If any segment of the location is None except revision, raises + keystore.exceptions.InsufficientSpecificationError + If no object is found at that location, raises keystore.exceptions.ItemNotFoundError + + location: Something that can be passed to Location + """ + location = Location(location) + try: + return self.modules[location.url()] + except KeyError: + raise ItemNotFoundError(location) + + def create_item(self, location): + raise NotImplementedError("XMLModuleStores are read-only") + + def update_item(self, location, data): + """ + Set the data in the item specified by the location to + data + + location: Something that can be passed to Location + data: A nested dictionary of problem data + """ + raise NotImplementedError("XMLModuleStores are read-only") + + def update_children(self, location, children): + """ + Set the children for the item specified by the location to + data + + location: Something that can be passed to Location + children: A list of child item identifiers + """ + raise NotImplementedError("XMLModuleStores are read-only") diff --git a/common/lib/xmodule/__init__.py b/common/lib/xmodule/__init__.py index 307b544b794..e69de29bb2d 100644 --- a/common/lib/xmodule/__init__.py +++ b/common/lib/xmodule/__init__.py @@ -1,62 +0,0 @@ -import capa_module -import html_module -import schematic_module -import seq_module -import template_module -import vertical_module -import video_module - -# Import all files in modules directory, excluding backups (# and . in name) -# and __init__ -# -# Stick them in a list -# modx_module_list = [] - -# for f in os.listdir(os.path.dirname(__file__)): -# if f!='__init__.py' and \ -# f[-3:] == ".py" and \ -# "." not in f[:-3] \ -# and '#' not in f: -# mod_path = 'courseware.modules.'+f[:-3] -# mod = __import__(mod_path, fromlist = "courseware.modules") -# if 'Module' in mod.__dict__: -# modx_module_list.append(mod) - -#print modx_module_list -modx_module_list = [capa_module, html_module, schematic_module, seq_module, template_module, vertical_module, video_module] -#print modx_module_list - -modx_modules = {} - -# Convert list to a dictionary for lookup by tag -def update_modules(): - global modx_modules - modx_modules = dict() - for module in modx_module_list: - for tag in module.Module.get_xml_tags(): - modx_modules[tag] = module.Module - -update_modules() - -def get_module_class(tag): - ''' Given an XML tag (e.g. 'video'), return - the associated module (e.g. video_module.Module). - ''' - if tag not in modx_modules: - update_modules() - return modx_modules[tag] - -def get_module_id(tag): - ''' Given an XML tag (e.g. 'video'), return - the default ID for that module (e.g. 'youtube_id') - ''' - return modx_modules[tag].id_attribute - -def get_valid_tags(): - return modx_modules.keys() - -def get_default_ids(): - tags = get_valid_tags() - ids = map(get_module_id, tags) - return dict(zip(tags, ids)) - diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py new file mode 100644 index 00000000000..dda6a58c994 --- /dev/null +++ b/common/lib/xmodule/abtest_module.py @@ -0,0 +1,95 @@ +import json +import random +from lxml import etree + +from x_module import XModule, XModuleDescriptor + + +class ModuleDescriptor(XModuleDescriptor): + pass + + +def group_from_value(groups, v): + ''' Given group: (('a',0.3),('b',0.4),('c',0.3)) And random value + in [0,1], return the associated group (in the above case, return + 'a' if v<0.3, 'b' if 0.3<=v<0.7, and 'c' if v>0.7 +''' + sum = 0 + for (g, p) in groups: + sum = sum + p + if sum > v: + return g + + # Round off errors might cause us to run to the end of the list + # If the do, return the last element + return g + + +class Module(XModule): + """ + Implements an A/B test with an aribtrary number of competing groups + + Format: + <abtest> + <group name="a" portion=".1"><contenta/></group> + <group name="b" portion=".2"><contentb/></group> + <default><contentdefault/></default> + </abtest> + """ + + def __init__(self, system, xml, item_id, instance_state=None, shared_state=None): + XModule.__init__(self, system, xml, item_id, instance_state, shared_state) + self.xmltree = etree.fromstring(xml) + + target_groups = self.xmltree.findall('group') + if shared_state is None: + target_values = [ + (elem.get('name'), float(elem.get('portion'))) + for elem in target_groups + ] + default_value = 1 - sum(val for (_, val) in target_values) + + self.group = group_from_value( + target_values + [(None, default_value)], + random.uniform(0, 1) + ) + else: + shared_state = json.loads(shared_state) + + # TODO (cpennington): Remove this once we aren't passing in + # groups from django groups + if 'groups' in shared_state: + self.group = None + target_names = [elem.get('name') for elem in target_groups] + for group in shared_state['groups']: + if group in target_names: + self.group = group + break + else: + self.group = shared_state['group'] + + def get_shared_state(self): + return json.dumps({'group': self.group}) + + def _xml_children(self): + group = None + if self.group is None: + group = self.xmltree.find('default') + else: + for candidate_group in self.xmltree.find('group'): + if self.group == candidate_group.get('name'): + group = candidate_group + break + + if group is None: + return [] + return list(group) + + def get_children(self): + return [self.module_from_xml(child) for child in self._xml_children()] + + def rendered_children(self): + return [self.render_function(child) for child in self._xml_children()] + + def get_html(self): + return '\n'.join(child.get_html() for child in self.get_children()) diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index b59bc9de56d..5047b948328 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -81,14 +81,7 @@ class Module(XModule): reset. ''' - id_attribute = "filename" - - @classmethod - def get_xml_tags(c): - return ["problem"] - - - def get_state(self): + def get_instance_state(self): state = self.lcp.get_state() state['attempts'] = self.attempts return json.dumps(state) @@ -191,8 +184,8 @@ class Module(XModule): return html - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) + def __init__(self, system, xml, item_id, instance_state=None, shared_state=None): + XModule.__init__(self, system, xml, item_id, instance_state, shared_state) self.attempts = 0 self.max_attempts = None @@ -232,19 +225,19 @@ class Module(XModule): self.show_answer = "closed" self.rerandomize = only_one(dom2.xpath('/problem/@rerandomize')) - if self.rerandomize == "" or self.rerandomize=="always" or self.rerandomize=="true": - self.rerandomize="always" - elif self.rerandomize=="false" or self.rerandomize=="per_student": - self.rerandomize="per_student" - elif self.rerandomize=="never": - self.rerandomize="never" + if self.rerandomize == "" or self.rerandomize == "always" or self.rerandomize == "true": + self.rerandomize = "always" + elif self.rerandomize == "false" or self.rerandomize == "per_student": + self.rerandomize = "per_student" + elif self.rerandomize == "never": + self.rerandomize = "never" else: - raise Exception("Invalid rerandomize attribute "+self.rerandomize) + raise Exception("Invalid rerandomize attribute " + self.rerandomize) - if state!=None: - state=json.loads(state) - if state!=None and 'attempts' in state: - self.attempts=state['attempts'] + if instance_state != None: + instance_state = json.loads(instance_state) + if instance_state != None and 'attempts' in instance_state: + self.attempts = instance_state['attempts'] # TODO: Should be: self.filename=only_one(dom2.xpath('/problem/@filename')) self.filename= "problems/"+only_one(dom2.xpath('/problem/@filename'))+".xml" @@ -267,7 +260,7 @@ class Module(XModule): else: raise try: - self.lcp=LoncapaProblem(fp, self.item_id, state, seed = seed, system=self.system) + self.lcp=LoncapaProblem(fp, self.item_id, instance_state, seed = seed, system=self.system) except Exception,err: msg = '[courseware.capa.capa_module.Module.init] error %s: cannot create LoncapaProblem %s' % (err,self.filename) log.exception(msg) @@ -277,7 +270,7 @@ class Module(XModule): # create a dummy problem with error message instead of failing fp = StringIO.StringIO('<problem><text><font color="red" size="+2">Problem file %s has an error:</font>%s</text></problem>' % (self.filename,msg)) fp.name = "StringIO" - self.lcp=LoncapaProblem(fp, self.item_id, state, seed = seed, system=self.system) + self.lcp=LoncapaProblem(fp, self.item_id, instance_state, seed = seed, system=self.system) else: raise diff --git a/common/lib/xmodule/hidden_module.py b/common/lib/xmodule/hidden_module.py new file mode 100644 index 00000000000..d4f2a0fa331 --- /dev/null +++ b/common/lib/xmodule/hidden_module.py @@ -0,0 +1,10 @@ +from xmodule.x_module import XModule +from xmodule.raw_module import RawDescriptor + + +class HiddenModule(XModule): + pass + + +class HiddenDescriptor(RawDescriptor): + module_class = HiddenModule diff --git a/common/lib/xmodule/html_module.py b/common/lib/xmodule/html_module.py index b35549d9718..32963600cdf 100644 --- a/common/lib/xmodule/html_module.py +++ b/common/lib/xmodule/html_module.py @@ -1,75 +1,34 @@ import json import logging -from x_module import XModule -from mako_module import MakoModuleDescriptor +from xmodule.x_module import XModule +from xmodule.mako_module import MakoModuleDescriptor +from xmodule.xml_module import XmlDescriptor from lxml import etree from pkg_resources import resource_string log = logging.getLogger("mitx.courseware") -#----------------------------------------------------------------------------- -class HtmlModuleDescriptor(MakoModuleDescriptor): +class HtmlModule(XModule): + def get_html(self): + return self.html + + def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + self.html = self.definition['data']['text'] + + +class HtmlDescriptor(MakoModuleDescriptor, XmlDescriptor): """ Module for putting raw html in a course """ mako_template = "widgets/html-edit.html" + module_class = HtmlModule js = {'coffee': [resource_string(__name__, 'js/module/html.coffee')]} js_module = 'HTML' @classmethod - def from_xml(cls, xml_data, system, org=None, course=None): - """ - Creates an instance of this descriptor from the supplied xml_data. - This may be overridden by subclasses - - xml_data: A string of xml that will be translated into data and children for - this module - system: An XModuleSystem for interacting with external resources - org and course are optional strings that will be used in the generated modules - url identifiers - """ - xml_object = etree.fromstring(xml_data) - return cls( - system, - definition={'data': {'text': xml_data}}, - location=['i4x', - org, - course, - xml_object.tag, - xml_object.get('name')] - ) - -class Module(XModule): - id_attribute = 'filename' - - def get_state(self): - return json.dumps({ }) - - @classmethod - def get_xml_tags(c): - return ["html"] - - def get_html(self): - if self.filename==None: - xmltree=etree.fromstring(self.xml) - textlist=[xmltree.text]+[etree.tostring(i) for i in xmltree]+[xmltree.tail] - textlist=[i for i in textlist if type(i)==str] - return "".join(textlist) - try: - filename="html/"+self.filename - return self.filestore.open(filename).read() - except: # For backwards compatibility. TODO: Remove - if self.DEBUG: - log.info('[courseware.modules.html_module] filename=%s' % self.filename) - return self.system.render_template(self.filename, {'id': self.item_id}, namespace='course') - - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) - xmltree=etree.fromstring(xml) - self.filename = None - filename_l=xmltree.xpath("/html/@filename") - if len(filename_l)>0: - self.filename=str(filename_l[0]) + def definition_from_xml(cls, xml_object, system): + return {'data': {'text': etree.tostring(xml_object)}} diff --git a/common/lib/xmodule/raw_module.py b/common/lib/xmodule/raw_module.py index 7bb94c9b633..43a92303ad1 100644 --- a/common/lib/xmodule/raw_module.py +++ b/common/lib/xmodule/raw_module.py @@ -1,8 +1,9 @@ from pkg_resources import resource_string -from mako_module import MakoModuleDescriptor from lxml import etree +from xmodule.mako_module import MakoModuleDescriptor +from xmodule.xml_module import XmlDescriptor -class RawDescriptor(MakoModuleDescriptor): +class RawDescriptor(MakoModuleDescriptor, XmlDescriptor): """ Module that provides a raw editing view of it's data and children """ @@ -18,24 +19,5 @@ class RawDescriptor(MakoModuleDescriptor): } @classmethod - def from_xml(cls, xml_data, system, org=None, course=None): - """ - Creates an instance of this descriptor from the supplied xml_data. - This may be overridden by subclasses - - xml_data: A string of xml that will be translated into data and children for - this module - system: An XModuleSystem for interacting with external resources - org and course are optional strings that will be used in the generated modules - url identifiers - """ - xml_object = etree.fromstring(xml_data) - return cls( - system, - definition={'data': xml_data}, - location=['i4x', - org, - course, - xml_object.tag, - xml_object.get('name')] - ) + def definition_from_xml(cls, xml_object, system): + return {'data': etree.tostring(xml_object)} diff --git a/common/lib/xmodule/schematic_module.py b/common/lib/xmodule/schematic_module.py index 30175c16a84..f95729d4abe 100644 --- a/common/lib/xmodule/schematic_module.py +++ b/common/lib/xmodule/schematic_module.py @@ -6,18 +6,5 @@ class ModuleDescriptor(XModuleDescriptor): pass class Module(XModule): - id_attribute = 'id' - - def get_state(self): - return json.dumps({ }) - - @classmethod - def get_xml_tags(c): - return ["schematic"] - def get_html(self): return '<input type="hidden" class="schematic" name="{item_id}" height="480" width="640">'.format(item_id=self.item_id) - - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) - diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index e3a19c3d60b..b60f0e46567 100644 --- a/common/lib/xmodule/seq_module.py +++ b/common/lib/xmodule/seq_module.py @@ -3,8 +3,9 @@ import logging from lxml import etree -from x_module import XModule -from mako_module import MakoModuleDescriptor +from xmodule.mako_module import MakoModuleDescriptor +from xmodule.xml_module import XmlDescriptor +from xmodule.x_module import XModule from xmodule.progress import Progress log = logging.getLogger("mitx.common.lib.seq_module") @@ -13,32 +14,17 @@ log = logging.getLogger("mitx.common.lib.seq_module") # OBSOLETE: This obsoletes 'type' class_priority = ['video', 'problem'] -class Module(XModule): + +class SequenceModule(XModule): ''' Layout module which lays out content in a temporal sequence ''' - id_attribute = 'id' - - def get_state(self): - return json.dumps({ 'position':self.position }) + def get_instance_state(self): + return json.dumps({'position': self.position}) - @classmethod - def get_xml_tags(c): - obsolete_tags = ["sequential", 'tab'] - modern_tags = ["videosequence"] - return obsolete_tags + modern_tags - def get_html(self): self.render() return self.content - def get_init_js(self): - self.render() - return self.init_js - - def get_destroy_js(self): - self.render() - return self.destroy_js - def get_progress(self): ''' Return the total progress, adding total done and total available. (assumes that each submodule uses the same "units" for progress.) @@ -60,53 +46,51 @@ class Module(XModule): if self.rendered: return ## Returns a set of all types of all sub-children - child_classes = [set([i.tag for i in e.iter()]) for e in self.xmltree] - - titles = ["\n".join([i.get("name").strip() for i in e.iter() if i.get("name") is not None]) \ - for e in self.xmltree] - - children = self.get_children() - progresses = [child.get_progress() for child in children] - - self.contents = self.rendered_children() - - for contents, title, progress in zip(self.contents, titles, progresses): - contents['title'] = title - contents['progress_status'] = Progress.to_js_status_str(progress) - contents['progress_detail'] = Progress.to_js_detail_str(progress) - - for (content, element_class) in zip(self.contents, child_classes): - new_class = 'other' - for c in class_priority: - if c in element_class: - new_class = c - content['type'] = new_class + contents = [] + for child in self.get_display_items(): + progress = child.get_progress() + contents.append({ + 'content': child.get_html(), + 'title': "\n".join( + grand_child.name.strip() + for grand_child in child.get_children() + if grand_child.name is not None + ), + 'progress_status': Progress.to_js_status_str(progress), + 'progress_detail': Progress.to_js_detail_str(progress), + 'type': child.get_icon_class(), + }) + + print json.dumps(contents, indent=4) # Split </script> tags -- browsers handle this as end # of script, even if it occurs mid-string. Do this after json.dumps()ing # so that we can be sure of the quotations being used - params={'items': json.dumps(self.contents).replace('</script>', '<"+"/script>'), - 'id': self.item_id, - 'position': self.position, - 'titles': titles, - 'tag': self.xmltree.tag} - - if self.xmltree.tag in ['sequential', 'videosequence']: - self.content = self.system.render_template('seq_module.html', params) - if self.xmltree.tag == 'tab': - self.content = self.system.render_template('tab_module.html', params) - log.debug("rendered content: %s", content) + params = {'items': json.dumps(contents).replace('</script>', '<"+"/script>'), + 'element_id': "-".join(str(v) for v in self.location.list()), + 'item_id': self.id, + 'position': self.position, + 'tag': self.location.category} + + self.content = self.system.render_template('seq_module.html', params) self.rendered = True - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) - self.xmltree = etree.fromstring(xml) + def get_icon_class(self): + child_classes = set(child.get_icon_class() for child in self.get_children()) + new_class = 'other' + for c in class_priority: + if c in child_classes: + new_class = c + return new_class + def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) self.position = 1 - if state is not None: - state = json.loads(state) - if 'position' in state: self.position = int(state['position']) + if instance_state is not None: + state = json.loads(instance_state) + if 'position' in state: + self.position = int(state['position']) # if position is specified in system, then use that instead if system.get('position'): @@ -115,23 +99,13 @@ class Module(XModule): self.rendered = False -class SequenceDescriptor(MakoModuleDescriptor): +class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' + module_class = SequenceModule @classmethod - def from_xml(cls, xml_data, system, org=None, course=None): - xml_object = etree.fromstring(xml_data) - - children = [ + def definition_from_xml(cls, xml_object, system): + return {'children': [ system.process_xml(etree.tostring(child_module)).url for child_module in xml_object - ] - - return cls( - system, {'children': children}, - location=['i4x', - org, - course, - xml_object.tag, - xml_object.get('name')] - ) + ]} diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 17d7af50db8..3e3e33805f7 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -13,14 +13,15 @@ setup( # for a description of entry_points entry_points={ 'xmodule.v1': [ - "chapter = seq_module:SequenceDescriptor", - "course = seq_module:SequenceDescriptor", - "html = html_module:HtmlModuleDescriptor", - "section = translation_module:SemanticSectionDescriptor", - "sequential = seq_module:SequenceDescriptor", - "vertical = seq_module:SequenceDescriptor", - "problemset = seq_module:SequenceDescriptor", - "videosequence = seq_module:SequenceDescriptor", + "chapter = xmodule.seq_module:SequenceDescriptor", + "course = xmodule.seq_module:SequenceDescriptor", + "html = xmodule.html_module:HtmlDescriptor", + "section = xmodule.translation_module:SemanticSectionDescriptor", + "sequential = xmodule.seq_module:SequenceDescriptor", + "vertical = xmodule.vertical_module:VerticalDescriptor", + "problemset = xmodule.seq_module:SequenceDescriptor", + "videosequence = xmodule.seq_module:SequenceDescriptor", + "video = xmodule.video_module:VideoDescriptor", ] } ) diff --git a/common/lib/xmodule/template_module.py b/common/lib/xmodule/template_module.py index 51f9447c063..ae276737e67 100644 --- a/common/lib/xmodule/template_module.py +++ b/common/lib/xmodule/template_module.py @@ -31,18 +31,11 @@ class Module(XModule): Renders to:: More information given in <a href="/book/234">the text</a> """ - def get_state(self): - return json.dumps({}) - - @classmethod - def get_xml_tags(c): - return ['customtag'] - def get_html(self): return self.html - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) + def __init__(self, system, xml, item_id, instance_state=None, shared_state=None): + XModule.__init__(self, system, xml, item_id, instance_state, shared_state) xmltree = etree.fromstring(xml) filename = xmltree.find('impl').text params = dict(xmltree.items()) diff --git a/common/lib/xmodule/vertical_module.py b/common/lib/xmodule/vertical_module.py index b3feec8baec..d3f4cd6ad37 100644 --- a/common/lib/xmodule/vertical_module.py +++ b/common/lib/xmodule/vertical_module.py @@ -1,23 +1,10 @@ -import json - -from x_module import XModule, XModuleDescriptor +from xmodule.x_module import XModule +from xmodule.seq_module import SequenceDescriptor from xmodule.progress import Progress -from lxml import etree -class ModuleDescriptor(XModuleDescriptor): - pass -class Module(XModule): +class VerticalModule(XModule): ''' Layout module for laying out submodules vertically.''' - id_attribute = 'id' - - def get_state(self): - return json.dumps({ }) - - @classmethod - def get_xml_tags(c): - return ["vertical", "problemset"] - def get_html(self): return self.system.render_template('vert_module.html', { 'items': self.contents @@ -30,8 +17,10 @@ class Module(XModule): progress = reduce(Progress.add_counts, progresses) return progress - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) - xmltree=etree.fromstring(xml) - self.contents=[(e.get("name"),self.render_function(e)) \ - for e in xmltree] + def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + self.contents = [child.get_html() for child in self.get_display_items()] + + +class VerticalDescriptor(SequenceDescriptor): + module_class = VerticalModule diff --git a/common/lib/xmodule/video_module.py b/common/lib/xmodule/video_module.py index f3d615fd3d9..1585944cc95 100644 --- a/common/lib/xmodule/video_module.py +++ b/common/lib/xmodule/video_module.py @@ -3,16 +3,13 @@ import logging from lxml import etree -from x_module import XModule, XModuleDescriptor -from progress import Progress +from xmodule.x_module import XModule +from xmodule.raw_module import RawDescriptor -log = logging.getLogger("mitx.courseware.modules") +log = logging.getLogger(__name__) -class ModuleDescriptor(XModuleDescriptor): - pass -class Module(XModule): - id_attribute = 'youtube' +class VideoModule(XModule): video_time = 0 def handle_ajax(self, dispatch, get): @@ -39,14 +36,9 @@ class Module(XModule): ''' return None - def get_state(self): + def get_instance_state(self): log.debug(u"STATE POSITION {0}".format(self.position)) - return json.dumps({ 'position': self.position }) - - @classmethod - def get_xml_tags(c): - '''Tags in the courseware file guaranteed to correspond to the module''' - return ["video"] + return json.dumps({'position': self.position}) def video_list(self): return self.youtube @@ -54,27 +46,27 @@ class Module(XModule): def get_html(self): return self.system.render_template('video.html', { 'streams': self.video_list(), - 'id': self.item_id, + 'id': self.id, 'position': self.position, 'name': self.name, 'annotations': self.annotations, }) - def __init__(self, system, xml, item_id, state=None): - XModule.__init__(self, system, xml, item_id, state) - xmltree = etree.fromstring(xml) + def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + xmltree = etree.fromstring(self.definition['data']) self.youtube = xmltree.get('youtube') self.name = xmltree.get('name') self.position = 0 - if state is not None: - state = json.loads(state) + if instance_state is not None: + state = json.loads(instance_state) if 'position' in state: self.position = int(float(state['position'])) - self.annotations=[(e.get("name"),self.render_function(e)) \ + self.annotations = [(e.get("name"), self.render_function(e)) \ for e in xmltree] -class VideoSegmentDescriptor(XModuleDescriptor): - pass +class VideoDescriptor(RawDescriptor): + module_class = VideoModule diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index aad4dd94dce..3787a76752a 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -3,6 +3,7 @@ import pkg_resources import logging from keystore import Location +from functools import partial log = logging.getLogger('mitx.' + __name__) @@ -56,85 +57,87 @@ class Plugin(object): class XModule(object): ''' Implements a generic learning module. - Initialized on access with __init__, first time with state=None, and - then with state + Initialized on access with __init__, first time with instance_state=None, and + shared_state=None. In later instantiations, instance_state will not be None, + but shared_state may be See the HTML module for a simple example ''' - id_attribute='id' # An attribute guaranteed to be unique + def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + ''' + Construct a new xmodule - @classmethod - def get_xml_tags(c): - ''' Tags in the courseware file guaranteed to correspond to the module ''' - return [] - - @classmethod - def get_usage_tags(c): - ''' We should convert to a real module system - For now, this tells us whether we use this as an xmodule, a CAPA response type - or a CAPA input type ''' - return ['xmodule'] + system: An I4xSystem allowing access to external resources + location: Something Location-like that identifies this xmodule + definition: A dictionary containing 'data' and 'children'. Both are optional + 'data': is a json object specifying the behavior of this xmodule + 'children': is a list of xmodule urls for child modules that this module depends on + ''' + self.system = system + self.location = Location(location) + self.definition = definition + self.instance_state = instance_state + self.shared_state = shared_state + self.id = self.location.url() + self.name = self.location.name + self.display_name = kwargs.get('display_name', '') + self._loaded_children = None def get_name(self): name = self.__xmltree.get('name') - if name: + if name: return name - else: + else: raise "We should iterate through children and find a default name" def get_children(self): ''' Return module instances for all the children of this module. ''' - children = [self.module_from_xml(e) for e in self.__xmltree] - return children + if self._loaded_children is None: + self._loaded_children = [self.system.get_module(child) for child in self.definition.get('children', [])] + return self._loaded_children - def rendered_children(self): + def get_display_items(self): ''' - Render all children. - This really ought to return a list of xmodules, instead of dictionaries + Returns a list of descendent module instances that will display immediately + inside this module ''' - children = [self.render_function(e) for e in self.__xmltree] - return children - - def __init__(self, system = None, xml = None, item_id = None, - json = None, track_url=None, state=None): - ''' In most cases, you must pass state or xml''' - if not item_id: - raise ValueError("Missing Index") - if not xml and not json: - raise ValueError("xml or json required") - if not system: - raise ValueError("System context required") - - self.xml = xml - self.json = json - self.item_id = item_id - self.state = state - self.DEBUG = False - - self.__xmltree = etree.fromstring(xml) # PRIVATE - - if system: - ## These are temporary; we really should go - ## through self.system. - self.ajax_url = system.ajax_url - self.tracker = system.track_function - self.filestore = system.filestore - self.render_function = system.render_function - self.module_from_xml = system.module_from_xml - self.DEBUG = system.DEBUG - self.system = system + items = [] + for child in self.get_children(): + items.extend(child.displayable_items()) + + return items + + def displayable_items(self): + ''' + Returns list of displayable modules contained by this module. If this module + is visible, should return [self] + ''' + return [self] + + def get_icon_class(self): + ''' + Return a class identifying this module in the context of an icon + ''' + return 'other' ### Functions used in the LMS - def get_state(self): - ''' State of the object, as stored in the database + def get_instance_state(self): + ''' State of the object, as stored in the database ''' - return "" + return '{}' + + def get_shared_state(self): + ''' + Get state that should be shared with other instances + using the same 'shared_state_key' attribute. + ''' + return '{}' def get_score(self): - ''' Score the student received on the problem. + ''' Score the student received on the problem. ''' return None @@ -281,6 +284,7 @@ class XModuleDescriptor(Plugin): self.name = Location(kwargs.get('location')).name self.type = Location(kwargs.get('location')).category self.url = Location(kwargs.get('location')).url() + self.display_name = kwargs.get('display_name') # For now, we represent goals as a list of strings, but this # is one of the things that we are going to be iterating on heavily @@ -302,33 +306,13 @@ class XModuleDescriptor(Plugin): """ raise NotImplementedError("get_html() must be provided by specific modules") - def get_xml(self): - ''' For conversions between JSON and legacy XML representations. - ''' - if self.xml: - return self.xml - else: - raise NotImplementedError("JSON->XML Translation not implemented") - - def get_json(self): - ''' For conversions between JSON and legacy XML representations. - ''' - if self.json: - raise NotImplementedError - return self.json # TODO: Return context as well -- files, etc. - else: - raise NotImplementedError("XML->JSON Translation not implemented") - - #def handle_cms_json(self): - # raise NotImplementedError - - #def render(self, size): - # ''' Size: [thumbnail, small, full] - # Small ==> what we drag around - # Full ==> what we edit - # ''' - # raise NotImplementedError - + def xmodule_constructor(self, system): + """ + Returns a constructor for an XModule. This constructor takes two arguments: + instance_state and shared_state, and returns a fully nstantiated XModule + """ + return partial(self.module_class, system, self.url, self.definition, + display_name=self.display_name) class DescriptorSystem(object): def __init__(self, load_item, resources_fs): diff --git a/common/lib/xmodule/xml_module.py b/common/lib/xmodule/xml_module.py new file mode 100644 index 00000000000..34881a4d613 --- /dev/null +++ b/common/lib/xmodule/xml_module.py @@ -0,0 +1,41 @@ +from xmodule.x_module import XModuleDescriptor +from lxml import etree + + +class XmlDescriptor(XModuleDescriptor): + """ + Mixin class for standardized parsing of from xml + """ + + @classmethod + def definition_from_xml(cls, xml_object, system): + """ + Return the definition to be passed to the newly created descriptor + during from_xml + """ + raise NotImplementedError("%s does not implement definition_from_xml" % cls.__class__.__name__) + + @classmethod + def from_xml(cls, xml_data, system, org=None, course=None): + """ + Creates an instance of this descriptor from the supplied xml_data. + This may be overridden by subclasses + + xml_data: A string of xml that will be translated into data and children for + this module + system: An XModuleSystem for interacting with external resources + org and course are optional strings that will be used in the generated modules + url identifiers + """ + xml_object = etree.fromstring(xml_data) + + return cls( + system, + cls.definition_from_xml(xml_object, system), + location=['i4x', + org, + course, + xml_object.tag, + xml_object.get('slug')], + display_name=xml_object.get('name') + ) diff --git a/lms/djangoapps/courseware/content_parser.py b/lms/djangoapps/courseware/content_parser.py index 95c3afed8c1..70e5eeeeb62 100644 --- a/lms/djangoapps/courseware/content_parser.py +++ b/lms/djangoapps/courseware/content_parser.py @@ -19,10 +19,12 @@ from django.conf import settings from student.models import UserProfile from student.models import UserTestGroup +from courseware.models import StudentModuleCache from mitxmako.shortcuts import render_to_string from util.cache import cache from multicourse import multicourse_settings import xmodule +from keystore.django import keystore ''' This file will eventually form an abstraction layer between the course XML file and the rest of the system. @@ -103,6 +105,7 @@ def course_xml_process(tree): items without. Propagate due dates, grace periods, etc. to child items. ''' + process_includes(tree) replace_custom_tags(tree) id_tag(tree) propogate_downward_tag(tree, "due") @@ -113,45 +116,32 @@ def course_xml_process(tree): return tree -def toc_from_xml(dom, active_chapter, active_section): - ''' - Create a table of contents from the course xml. - - Return format: - [ {'name': name, 'sections': SECTIONS, 'active': bool}, ... ] - - where SECTIONS is a list - [ {'name': name, 'format': format, 'due': due, 'active' : bool}, ...] - - active is set for the section and chapter corresponding to the passed - parameters. Everything else comes from the xml, or defaults to "". - - chapters with name 'hidden' are skipped. - ''' - name = dom.xpath('//course/@name')[0] - - chapters = dom.xpath('//course[@name=$name]/chapter', name=name) - ch = list() - for c in chapters: - if c.get('name') == 'hidden': - continue - sections = list() - for s in dom.xpath('//course[@name=$name]/chapter[@name=$chname]/section', - name=name, chname=c.get('name')): - - format = s.get("subtitle") if s.get("subtitle") else s.get("format") or "" - active = (c.get("name") == active_chapter and - s.get("name") == active_section) - - sections.append({'name': s.get("name") or "", - 'format': format, - 'due': s.get("due") or "", - 'active': active}) - - ch.append({'name': c.get("name"), - 'sections': sections, - 'active': c.get("name") == active_chapter}) - return ch +def process_includes_dir(tree, dir): + """ + Process tree to replace all <include file=""> tags + with the contents of the file specified, relative to dir + """ + includes = tree.findall('.//include') + for inc in includes: + file = inc.get('file') + if file is not None: + try: + ifp = open(os.path.join(dir, file)) + except Exception: + log.exception('Error in problem xml include: %s' % (etree.tostring(inc, pretty_print=True))) + log.exception('Cannot find file %s in %s' % (file, dir)) + raise + try: + # read in and convert to XML + incxml = etree.XML(ifp.read()) + except Exception: + log.exception('Error in problem xml include: %s' % (etree.tostring(inc, pretty_print=True))) + log.exception('Cannot parse XML in %s' % (file)) + raise + # insert new XML into tree in place of inlcude + parent = inc.getparent() + parent.insert(parent.index(inc), incxml) + parent.remove(inc) def replace_custom_tags_dir(tree, dir): @@ -181,78 +171,6 @@ def parse_course_file(filename, options, namespace): return course_xml_process(xml) -def get_section(section, options, dirname): - ''' - Given the name of a section, an options dict containing keys - 'dev_content' and 'groups', and a directory to look in, - returns the xml tree for the section, or None if there's no - such section. - ''' - filename = section + ".xml" - - if filename not in os.listdir(dirname): - log.error(filename + " not in " + str(os.listdir(dirname))) - return None - - tree = parse_course_file(filename, options, namespace='sections') - return tree - - -def get_module(tree, module, id_tag, module_id, sections_dirname, options): - ''' - Given the xml tree of the course, get the xml string for a module - with the specified module type, id_tag, module_id. Looks in - sections_dirname for sections. - - id_tag -- use id_tag if the place the module stores its id is not 'id' - ''' - # Sanitize input - if not module.isalnum(): - raise Exception("Module is not alphanumeric") - - if not module_id.isalnum(): - raise Exception("Module ID is not alphanumeric") - - # Generate search - xpath_search='//{module}[(@{id_tag} = "{id}") or (@id = "{id}")]'.format( - module=module, - id_tag=id_tag, - id=module_id) - - - result_set = tree.xpath(xpath_search) - if len(result_set) < 1: - # Not found in main tree. Let's look in the section files. - section_list = (s[:-4] for s in os.listdir(sections_dirname) if s.endswith('.xml')) - for section in section_list: - try: - s = get_section(section, options, sections_dirname) - except etree.XMLSyntaxError: - ex = sys.exc_info() - raise ContentException("Malformed XML in " + section + - "(" + str(ex[1].msg) + ")") - result_set = s.xpath(xpath_search) - if len(result_set) != 0: - break - - if len(result_set) > 1: - log.error("WARNING: Potentially malformed course file", module, module_id) - - if len(result_set)==0: - log.error('[content_parser.get_module] cannot find %s in course.xml tree', - xpath_search) - log.error('tree = %s' % etree.tostring(tree, pretty_print=True)) - return None - - # log.debug('[courseware.content_parser.module_xml] found %s' % result_set) - - return etree.tostring(result_set[0]) - - - - - - # ==== All Django-specific code below ============================================= def user_groups(user): @@ -278,6 +196,11 @@ def get_options(user): 'groups': user_groups(user)} +def process_includes(tree): + '''Replace <include> tags with the contents from the course directory''' + process_includes_dir(tree, settings.DATA_DIR) + + def replace_custom_tags(tree): '''Replace custom tags defined in our custom_tags dir''' replace_custom_tags_dir(tree, settings.DATA_DIR+'/custom_tags') @@ -337,29 +260,3 @@ def sections_dir(coursename=None): xp = multicourse_settings.get_course_xmlpath(coursename) return settings.DATA_DIR + xp + '/sections/' - - - -def section_file(user, section, coursename=None): - ''' - Given a user and the name of a section, return that section. - This is done specific to each course. - - Returns the xml tree for the section, or None if there's no such section. - ''' - dirname = sections_dir(coursename) - - - return get_section(section, options, dirname) - - -def module_xml(user, module, id_tag, module_id, coursename=None): - ''' Get XML for a module based on module and module_id. Assumes - module occurs once in courseware XML file or hidden section. - ''' - tree = course_file(user, coursename) - sdirname = sections_dir(coursename) - options = get_options(user) - - return get_module(tree, module, id_tag, module_id, sdirname, options) - diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 00bdffb6977..3c2b6546829 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -81,12 +81,12 @@ def grade_sheet(student,coursename=None): course = dom.xpath('//course/@name')[0] xmlChapters = dom.xpath('//course[@name=$course]/chapter', course=course) - responses=StudentModule.objects.filter(student=student) + responses = StudentModule.objects.filter(student=student) response_by_id = {} for response in responses: - response_by_id[response.module_id] = response - - + response_by_id[response.module_state_key] = response + + totaled_scores = {} chapters=[] for c in xmlChapters: @@ -147,27 +147,39 @@ def grade_sheet(student,coursename=None): 'grade_summary' : grade_summary} def get_score(user, problem, cache, coursename=None): + """ + Return the score for a user on a problem + + user: a Student object + problem: the xml for the problem + cache: a dictionary mapping module_state_key tuples to instantiated StudentModules + module_state_key is either the problem_id, or a key used by the problem + to share state across instances + """ ## HACK: assumes max score is fixed per problem - id = problem.get('id') + module_type = problem.tag + module_class = xmodule.get_module_class(module_type) + module_id = problem.get('id') + module_state_key = problem.get(module_class.state_key, module_id) correct = 0.0 - + # If the ID is not in the cache, add the item - if id not in cache: - module = StudentModule(module_type = 'problem', # TODO: Move into StudentModule.__init__? - module_id = id, - student = user, - state = None, - grade = 0, - max_grade = None, - done = 'i') - cache[id] = module + if module_state_key not in cache: + module = StudentModule(module_type='problem', # TODO: Move into StudentModule.__init__? + module_state_key=id, + student=user, + state=None, + grade=0, + max_grade=None, + done='i') + cache[module_id] = module # Grab the # correct from cache if id in cache: response = cache[id] - if response.grade!=None: - correct=float(response.grade) - + if response.grade != None: + correct = float(response.grade) + # Grab max grade from cache, or if it doesn't exist, compute and save to DB if id in cache and response.max_grade is not None: total = response.max_grade diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index a97b09ae2be..6ca67a84e75 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -13,7 +13,6 @@ ASSUMPTIONS: modules have unique IDs, even across different module_types """ from django.db import models -from django.db.models.signals import post_save, post_delete #from django.core.cache import cache from django.contrib.auth.models import User @@ -21,72 +20,97 @@ from django.contrib.auth.models import User #CACHE_TIMEOUT = 60 * 60 * 4 # Set the cache timeout to be four hours + class StudentModule(models.Model): # For a homework problem, contains a JSON # object consisting of state - MODULE_TYPES = (('problem','problem'), - ('video','video'), - ('html','html'), + MODULE_TYPES = (('problem', 'problem'), + ('video', 'video'), + ('html', 'html'), ) ## These three are the key for the object module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True) - module_id = models.CharField(max_length=255, db_index=True) # Filename for homeworks, etc. + + # Key used to share state. By default, this is the module_id, + # but for abtests and the like, this can be set to a shared value + # for many instances of the module. + # Filename for homeworks, etc. + module_state_key = models.CharField(max_length=255, db_index=True, db_column='module_id') student = models.ForeignKey(User, db_index=True) + class Meta: - unique_together = (('student', 'module_id'),) + unique_together = (('student', 'module_state_key'),) ## Internal state of the object state = models.TextField(null=True, blank=True) - ## Grade, and are we done? + ## Grade, and are we done? grade = models.FloatField(null=True, blank=True, db_index=True) max_grade = models.FloatField(null=True, blank=True) - DONE_TYPES = (('na','NOT_APPLICABLE'), - ('f','FINISHED'), - ('i','INCOMPLETE'), + DONE_TYPES = (('na', 'NOT_APPLICABLE'), + ('f', 'FINISHED'), + ('i', 'INCOMPLETE'), ) done = models.CharField(max_length=8, choices=DONE_TYPES, default='na', db_index=True) - # DONE_TYPES = (('done','DONE'), # Finished - # ('incomplete','NOTDONE'), # Not finished - # ('na','NA')) # Not applicable (e.g. vertical) - # done = models.CharField(max_length=16, choices=DONE_TYPES) - created = models.DateTimeField(auto_now_add=True, db_index=True) modified = models.DateTimeField(auto_now=True, db_index=True) def __unicode__(self): - return self.module_type+'/'+self.student.username+"/"+self.module_id+'/'+str(self.state)[:20] + return '/'.join([self.module_type, self.student.username, self.module_state_key, str(self.state)[:20]]) + + +# TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors + + - # @classmethod - # def get_with_caching(cls, student, module_id): - # k = cls.key_for(student, module_id) - # student_module = cache.get(k) - # if student_module is None: - # student_module = StudentModule.objects.filter(student=student, - # module_id=module_id)[0] - # # It's possible it really doesn't exist... - # if student_module is not None: - # cache.set(k, student_module, CACHE_TIMEOUT) +class StudentModuleCache(object): + """ + A cache of StudentModules for a specific student + """ + def __init__(self, user, descriptor, depth=None): + ''' + Find any StudentModule objects that are needed by any child modules of the + supplied descriptor. Avoids making multiple queries to the database + ''' + if user.is_authenticated(): + module_ids = self._get_module_state_keys(descriptor, depth) + self.cache = list(StudentModule.objects.filter(student=user, + module_state_key__in=module_ids)) + else: + self.cache = [] - # return student_module + def _get_module_state_keys(self, descriptor, depth): + ''' + Get a list of the state_keys needed for StudentModules + required for this chunk of module xml + ''' + keys = [descriptor.url] - @classmethod - def key_for(cls, student, module_id): - return "StudentModule-student_id:{0};module_id:{1}".format(student.id, module_id) + shared_state_key = getattr(descriptor, 'shared_state_key', None) + if shared_state_key is not None: + keys.append(shared_state_key) + if depth is None or depth > 0: + new_depth = depth - 1 if depth is not None else depth -# def clear_cache_by_student_and_module_id(sender, instance, *args, **kwargs): -# k = sender.key_for(instance.student, instance.module_id) -# cache.delete(k) + for child in descriptor.get_children(): + keys.extend(self._get_module_state_keys(child, new_depth)) -# def update_cache_by_student_and_module_id(sender, instance, *args, **kwargs): -# k = sender.key_for(instance.student, instance.module_id) -# cache.set(k, instance, CACHE_TIMEOUT) + return keys + def lookup(self, module_type, module_state_key): + ''' + Look for a student module with the given type and id in the cache. -#post_save.connect(update_cache_by_student_and_module_id, sender=StudentModule, weak=False) -#post_delete.connect(clear_cache_by_student_and_module_id, sender=StudentModule, weak=False) + cache -- list of student modules -#cache_model(StudentModule) + returns first found object, or None + ''' + for o in self.cache: + if o.module_type == module_type and o.module_state_key == module_state_key: + return o + return None + def append(self, student_module): + self.cache.append(student_module) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 3a6fcbfb45c..d05bdcefab8 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -12,19 +12,21 @@ from fs.osfs import OSFS from django.conf import settings from mitxmako.shortcuts import render_to_string, render_to_response -from models import StudentModule +from models import StudentModule, StudentModuleCache from multicourse import multicourse_settings from util.views import accepts import courseware.content_parser as content_parser import xmodule +from keystore.django import keystore log = logging.getLogger("mitx.courseware") + class I4xSystem(object): ''' - This is an abstraction such that x_modules can function independent - of the courseware (e.g. import into other types of courseware, LMS, + This is an abstraction such that x_modules can function independent + of the courseware (e.g. import into other types of courseware, LMS, or if we want to have a sandbox server for user-contributed content) I4xSystem objects are passed to x_modules to provide access to system @@ -34,7 +36,7 @@ class I4xSystem(object): and user, or other environment-specific info. ''' def __init__(self, ajax_url, track_function, render_function, - module_from_xml, render_template, request=None, + get_module, render_template, request=None, filestore=None): ''' Create a closure around the system environment. @@ -44,7 +46,7 @@ class I4xSystem(object): or otherwise tracking the event. TODO: Not used, and has inconsistent args in different files. Update or remove. - module_from_xml - function that takes (module_xml) and returns a corresponding + get_module - function that takes (location) and returns a corresponding module instance object. render_function - function that takes (module_xml) and renders it, returning a dictionary with a context for rendering the @@ -58,14 +60,14 @@ class I4xSystem(object): ''' self.ajax_url = ajax_url self.track_function = track_function - if not filestore: + if not filestore: self.filestore = OSFS(settings.DATA_DIR) else: self.filestore = filestore if settings.DEBUG: log.info("[courseware.module_render.I4xSystem] filestore path = %s", filestore) - self.module_from_xml = module_from_xml + self.get_module = get_module self.render_function = render_function self.render_template = render_template self.exception404 = Http404 @@ -75,8 +77,8 @@ class I4xSystem(object): def get(self, attr): ''' provide uniform access to attributes (like etree).''' return self.__dict__.get(attr) - - def set(self,attr,val): + + def set(self, attr, val): '''provide uniform access to attributes (like etree)''' self.__dict__[attr] = val @@ -86,21 +88,11 @@ class I4xSystem(object): def __str__(self): return str(self.__dict__) -def smod_cache_lookup(cache, module_type, module_id): - ''' - Look for a student module with the given type and id in the cache. - cache -- list of student modules - returns first found object, or None - ''' - for o in cache: - if o.module_type == module_type and o.module_id == module_id: - return o - return None def make_track_function(request): - ''' + ''' Make a tracking function that logs what happened. For use in I4xSystem. ''' @@ -110,8 +102,9 @@ def make_track_function(request): return track.views.server_track(request, event_type, event, page='x_module') return f + def grade_histogram(module_id): - ''' Print out a histogram of grades on a given problem. + ''' Print out a histogram of grades on a given problem. Part of staff member debug info. ''' from django.db import connection @@ -137,13 +130,87 @@ def make_module_from_xml_fn(user, request, student_module_cache, position): def module_from_xml(xml): '''Modules need a way to convert xml to instance objects. Pass the rest of the context through.''' - (instance, sm, module_type) = get_module( + (instance, _, _, _) = get_module( user, request, xml, student_module_cache, position) return instance return module_from_xml -def get_module(user, request, module_xml, student_module_cache, position=None): +def toc_for_course(user, request, course_location, active_chapter, active_section): + ''' + Create a table of contents from the module store + + Return format: + [ {'name': name, 'sections': SECTIONS, 'active': bool}, ... ] + + where SECTIONS is a list + [ {'name': name, 'format': format, 'due': due, 'active' : bool}, ...] + + active is set for the section and chapter corresponding to the passed + parameters. Everything else comes from the xml, or defaults to "". + + chapters with name 'hidden' are skipped. + ''' + + student_module_cache = StudentModuleCache(user, keystore().get_item(course_location), depth=2) + (course, _, _, _) = get_module(user, request, course_location, student_module_cache) + + chapters = list() + for chapter in course.get_display_items(): + sections = list() + for section in chapter.get_display_items(): + + active = (chapter.display_name == active_chapter and + section.display_name == active_section) + + sections.append({'name': section.display_name, + 'format': getattr(section, 'format', ''), + 'due': getattr(section, 'due', ''), + 'active': active}) + + chapters.append({'name': chapter.display_name, + 'sections': sections, + 'active': chapter.display_name == active_chapter}) + return chapters + + +def get_section(course, chapter, section): + """ + Returns the xmodule descriptor for the name course > chapter > section, + or None if this doesn't specify a valid section + + course: Course url + chapter: Chapter name + section: Section name + """ + try: + course_module = keystore().get_item(course) + except: + log.exception("Unable to load course_module") + return None + + if course_module is None: + return + + chapter_module = None + for _chapter in course_module.get_children(): + if _chapter.display_name == chapter: + chapter_module = _chapter + break + + if chapter_module is None: + return + + section_module = None + for _section in chapter_module.get_children(): + if _section.display_name == section: + section_module = _section + break + + return section_module + + +def get_module(user, request, location, student_module_cache, position=None): ''' Get an instance of the xmodule class corresponding to module_xml, setting the state based on an existing StudentModule, or creating one if none exists. @@ -152,65 +219,73 @@ def get_module(user, request, module_xml, student_module_cache, position=None): - user : current django User - request : current django HTTPrequest - module_xml : lxml etree of xml subtree for the requested module - - student_module_cache : list of StudentModule objects, one of which may - match this module type and id - - position : extra information from URL for user-specified + - student_module_cache : a StudentModuleCache + - position : extra information from URL for user-specified position within module Returns: - - a tuple (xmodule instance, student module, module type). + - a tuple (xmodule instance, instance_module, shared_module, module type). + instance_module is a StudentModule specific to this module for this student + shared_module is a StudentModule specific to all modules with the same 'shared_state_key' attribute, or None if the module doesn't elect to share state ''' - module_type = module_xml.tag - module_class = xmodule.get_module_class(module_type) - module_id = module_xml.get('id') - - # Grab xmodule state from StudentModule cache - smod = smod_cache_lookup(student_module_cache, module_type, module_id) - state = smod.state if smod else None - - # get coursename if present in request - coursename = multicourse_settings.get_coursename_from_request(request) + descriptor = keystore().get_item(location) - if coursename and settings.ENABLE_MULTICOURSE: - # path to XML for the course - xp = multicourse_settings.get_course_xmlpath(coursename) - data_root = settings.DATA_DIR + xp + instance_module = student_module_cache.lookup(descriptor.type, descriptor.url) + shared_state_key = getattr(descriptor, 'shared_state_key', None) + if shared_state_key is not None: + shared_module = student_module_cache.lookup(descriptor.type, shared_state_key) else: - data_root = settings.DATA_DIR + shared_module = None + + instance_state = instance_module.state if instance_module is not None else None + shared_state = shared_module.state if shared_module is not None else None # Setup system context for module instance - ajax_url = settings.MITX_ROOT_URL + '/modx/' + module_type + '/' + module_id + '/' + ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.type + '/' + descriptor.url + '/' - module_from_xml = make_module_from_xml_fn( - user, request, student_module_cache, position) - - system = I4xSystem(track_function = make_track_function(request), - render_function = lambda xml: render_x_module( + def _get_module(location): + (module, _, _, _) = get_module(user, request, location, student_module_cache, position) + return module + + system = I4xSystem(track_function=make_track_function(request), + render_function=lambda xml: render_x_module( user, request, xml, student_module_cache, position), - render_template = render_to_string, - ajax_url = ajax_url, - request = request, - filestore = OSFS(data_root), - module_from_xml = module_from_xml, + render_template=render_to_string, + ajax_url=ajax_url, + request=request, + # TODO (cpennington): Figure out how to share info between systems + filestore=descriptor.system.resources_fs, + get_module=_get_module, ) # pass position specified in URL to module through I4xSystem - system.set('position', position) - instance = module_class(system, - etree.tostring(module_xml), - module_id, - state=state) + system.set('position', position) + + module = descriptor.xmodule_constructor(system)(instance_state, shared_state) # If StudentModule for this instance wasn't already in the database, # and this isn't a guest user, create it. - if not smod and user.is_authenticated(): - smod = StudentModule(student=user, module_type = module_type, - module_id=module_id, state=instance.get_state()) - smod.save() - # Add to cache. The caller and the system context have references - # to it, so the change persists past the return - student_module_cache.append(smod) + if user.is_authenticated(): + if not instance_module: + instance_module = StudentModule( + student=user, + module_type=descriptor.type, + module_state_key=module.id, + state=module.get_instance_state()) + instance_module.save() + # Add to cache. The caller and the system context have references + # to it, so the change persists past the return + student_module_cache.append(instance_module) + if not shared_module and shared_state_key is not None: + shared_module = StudentModule( + student=user, + module_type=descriptor.type, + module_state_key=shared_state_key, + state=module.get_shared_state()) + shared_module.save() + student_module_cache.append(shared_module) + + return (module, instance_module, shared_module, descriptor.type) - return (instance, smod, module_type) def render_x_module(user, request, module_xml, student_module_cache, position=None): ''' Generic module for extensions. This renders to HTML. @@ -232,20 +307,20 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No - dict which is context for HTML rendering of the specified module. Will have key 'content', and will have 'type' key if passed a valid module. ''' - if module_xml is None : + if module_xml is None: return {"content": ""} - (instance, smod, module_type) = get_module( + (instance, _, _, module_type) = get_module( user, request, module_xml, student_module_cache, position) content = instance.get_html() - # special extra information about each problem, only for users who are staff + # special extra information about each problem, only for users who are staff if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: module_id = module_xml.get('id') histogram = grade_histogram(module_id) render_histogram = len(histogram) > 0 - staff_context = {'xml': etree.tostring(module_xml), + staff_context = {'xml': etree.tostring(module_xml), 'module_id': module_id, 'histogram': json.dumps(histogram), 'render_histogram': render_histogram} @@ -254,6 +329,7 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No context = {'content': content, 'type': module_type} return context + def modx_dispatch(request, module=None, dispatch=None, id=None): ''' Generic view for extensions. This is where AJAX calls go. @@ -276,24 +352,10 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): error_msg = ("We're sorry, this module is temporarily unavailable. " "Our staff is working to fix it as soon as possible") - - # Grab the student information for the module from the database - s = StudentModule.objects.filter(student=request.user, - module_id=id) - - if s is None or len(s) == 0: - log.debug("Couldn't find module '%s' for user '%s' and id '%s'", - module, request.user, id) - raise Http404 - s = s[0] - - oldgrade = s.grade - oldstate = s.state - # If there are arguments, get rid of them dispatch, _, _ = dispatch.partition('?') - ajax_url = '{root}/modx/{module}/{id}'.format(root = settings.MITX_ROOT_URL, + ajax_url = '{root}/modx/{module}/{id}'.format(root=settings.MITX_ROOT_URL, module=module, id=id) coursename = multicourse_settings.get_coursename_from_request(request) if coursename and settings.ENABLE_MULTICOURSE: @@ -315,26 +377,40 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): response = HttpResponse(json.dumps({'success': error_msg})) return response - # TODO: This doesn't have a cache of child student modules. Just - # passing the current one. If ajax calls end up needing children, - # this won't work (but fixing it may cause performance issues...) - # Figure out :) + module_xml = etree.fromstring(xml) + student_module_cache = StudentModuleCache(request.user, module_xml) + (instance, instance_state, shared_state, module_type) = get_module( + request.user, request, module_xml, + student_module_cache, None) + + if instance_state is None: + log.debug("Couldn't find module '%s' for user '%s' and id '%s'", + module, request.user, id) + raise Http404 + + oldgrade = instance_state.grade + old_instance_state = instance_state.state + old_shared_state = shared_state.state if shared_state is not None else None + module_from_xml = make_module_from_xml_fn( - request.user, request, [s], None) + request.user, request, student_module_cache, None) # Create the module - system = I4xSystem(track_function = make_track_function(request), - render_function = None, - module_from_xml = module_from_xml, - render_template = render_to_string, - ajax_url = ajax_url, - request = request, - filestore = OSFS(data_root), + system = I4xSystem(track_function=make_track_function(request), + render_function=None, + module_from_xml=module_from_xml, + render_template=render_to_string, + ajax_url=ajax_url, + request=request, + filestore=OSFS(data_root), ) try: module_class = xmodule.get_module_class(module) - instance = module_class(system, xml, id, state=oldstate) + instance = module_class( + system, xml, id, + instance_state=old_instance_state, + shared_state=old_shared_state) except: log.exception("Unable to load module instance during ajax call") if accepts(request, 'text/html'): @@ -351,10 +427,16 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): raise # Save the state back to the database - s.state = instance.get_state() - if instance.get_score(): - s.grade = instance.get_score()['score'] - if s.grade != oldgrade or s.state != oldstate: - s.save() + instance_state.state = instance.get_instance_state() + if instance.get_score(): + instance_state.grade = instance.get_score()['score'] + if instance_state.grade != oldgrade or instance_state.state != old_instance_state: + instance_state.save() + + if shared_state is not None: + shared_state.state = instance.get_shared_state() + if shared_state.state != old_shared_state: + shared_state.save() + # Return whatever the module wanted to return to the client/caller return HttpResponse(ajax_return) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 5cbbe18d7d1..6e8eb1ab9ef 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -16,11 +16,10 @@ from django.views.decorators.cache import cache_control from lxml import etree -from module_render import render_x_module, make_track_function, I4xSystem -from models import StudentModule +from module_render import render_x_module, toc_for_course, get_module, get_section +from models import StudentModuleCache from student.models import UserProfile from multicourse import multicourse_settings -import xmodule import courseware.content_parser as content_parser @@ -87,23 +86,20 @@ def render_accordion(request, course, chapter, section): If chapter and section are '' or None, renders a default accordion. Returns (initialization_javascript, content)''' - if not course: - course = "6.002 Spring 2012" - toc = content_parser.toc_from_xml( - content_parser.course_file(request.user, course), chapter, section) + course_location = multicourse_settings.get_course_location(course) + toc = toc_for_course(request.user, request, course_location, chapter, section) active_chapter = 1 for i in range(len(toc)): if toc[i]['active']: active_chapter = i - context=dict([('active_chapter', active_chapter), - ('toc', toc), - ('course_name', course), - ('format_url_params', content_parser.format_url_params), - ('csrf', csrf(request)['csrf_token'])] + - template_imports.items()) + context = dict([('active_chapter', active_chapter), + ('toc', toc), + ('course_name', course), + ('format_url_params', content_parser.format_url_params), + ('csrf', csrf(request)['csrf_token'])] + template_imports.items()) return render_to_string('accordion.html', context) @@ -125,16 +121,10 @@ def render_section(request, section): context = { 'csrf': csrf(request)['csrf_token'], - 'accordion': render_accordion(request, '', '', '') + 'accordion': render_accordion(request, get_course(request), '', '') } - module_ids = dom.xpath("//@id") - - if user.is_authenticated(): - student_module_cache = list(StudentModule.objects.filter(student=user, - module_id__in=module_ids)) - else: - student_module_cache = [] + student_module_cache = StudentModuleCache(request.user, dom) try: module = render_x_module(user, request, dom, student_module_cache) @@ -147,13 +137,13 @@ def render_section(request, section): return render_to_response('courseware.html', context) context.update({ - 'init': module.get('init_js', ''), 'content': module['content'], }) result = render_to_response('courseware.html', context) return result + def get_course(request, course): ''' Figure out what the correct course is. @@ -161,7 +151,7 @@ def get_course(request, course): TODO: Can this go away once multicourse becomes standard? ''' - if course==None: + if course == None: if not settings.ENABLE_MULTICOURSE: course = "6.002 Spring 2012" elif 'coursename' in request.session: @@ -170,35 +160,6 @@ def get_course(request, course): course = settings.COURSE_DEFAULT return course -def get_module_xml(user, course, chapter, section): - ''' Look up the module xml for the given course/chapter/section path. - - Takes the user to look up the course file. - - Returns None if there was a problem, or the lxml etree for the module. - ''' - try: - # this is the course.xml etree - dom = content_parser.course_file(user, course) - except: - log.exception("Unable to parse courseware xml") - return None - - # this is the module's parent's etree - path = "//course[@name=$course]/chapter[@name=$chapter]//section[@name=$section]" - dom_module = dom.xpath(path, course=course, chapter=chapter, section=section) - - module_wrapper = dom_module[0] if len(dom_module) > 0 else None - if module_wrapper is None: - module = None - elif module_wrapper.get("src"): - module = content_parser.section_file( - user=user, section=module_wrapper.get("src"), coursename=course) - else: - # Copy the element out of the module's etree - module = etree.XML(etree.tostring(module_wrapper[0])) - return module - @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @@ -228,55 +189,6 @@ def index(request, course=None, chapter=None, section=None, ''' return s.replace('_', ' ') if s is not None else None - def get_submodule_ids(module_xml): - ''' - Get a list with ids of the modules within this module. - ''' - return module_xml.xpath("//@id") - - def preload_student_modules(module_xml): - ''' - Find any StudentModule objects for this user that match - one of the given module_ids. Used as a cache to avoid having - each rendered module hit the db separately. - - Returns the list, or None on error. - ''' - if request.user.is_authenticated(): - module_ids = get_submodule_ids(module_xml) - return list(StudentModule.objects.filter(student=request.user, - module_id__in=module_ids)) - else: - return [] - - def get_module_context(): - ''' - Look up the module object and render it. If all goes well, returns - {'init': module-init-js, 'content': module-rendered-content} - - If there's an error, returns - {'content': module-error message} - ''' - user = request.user - - module_xml = get_module_xml(user, course, chapter, section) - if module_xml is None: - log.exception("couldn't get module_xml: course/chapter/section: '%s/%s/%s'", - course, chapter, section) - return {'content' : render_to_string("module-error.html", {})} - - student_module_cache = preload_student_modules(module_xml) - - try: - module_context = render_x_module(user, request, module_xml, - student_module_cache, position) - except: - log.exception("Unable to load module") - return {'content' : render_to_string("module-error.html", {})} - - return {'init': module_context.get('init_js', ''), - 'content': module_context['content']} - if not settings.COURSEWARE_ENABLED: return redirect('/') @@ -300,11 +212,16 @@ def index(request, course=None, chapter=None, section=None, look_for_module = chapter is not None and section is not None if look_for_module: - context.update(get_module_context()) + course_location = multicourse_settings.get_course_location(course) + section = get_section(course_location, chapter, section) + student_module_cache = StudentModuleCache(request.user, section) + module, _, _, _ = get_module(request.user, request, section.url, student_module_cache) + context['content'] = module.get_html() result = render_to_response('courseware.html', context) return result + def jump_to(request, probname=None): ''' Jump to viewing a specific problem. The problem is specified by a diff --git a/lms/djangoapps/multicourse/multicourse_settings.py b/lms/djangoapps/multicourse/multicourse_settings.py index 05b05c8ec97..4d568d55a17 100644 --- a/lms/djangoapps/multicourse/multicourse_settings.py +++ b/lms/djangoapps/multicourse/multicourse_settings.py @@ -31,11 +31,13 @@ if hasattr(settings,'COURSE_SETTINGS'): # in the future, this could be repla elif hasattr(settings,'COURSE_NAME'): # backward compatibility COURSE_SETTINGS = {settings.COURSE_NAME: {'number': settings.COURSE_NUMBER, 'title': settings.COURSE_TITLE, + 'location': settings.COURSE_LOCATION, }, } else: # default to 6.002_Spring_2012 COURSE_SETTINGS = {'6.002_Spring_2012': {'number': '6.002x', 'title': 'Circuits and Electronics', + 'location': 'i4x://edx/6002xs12/course/6.002 Spring 2012', }, } @@ -51,31 +53,47 @@ def get_coursename_from_request(request): def get_course_settings(coursename): if not coursename: - if hasattr(settings,'COURSE_DEFAULT'): + if hasattr(settings, 'COURSE_DEFAULT'): coursename = settings.COURSE_DEFAULT else: coursename = '6.002_Spring_2012' - if coursename in COURSE_SETTINGS: return COURSE_SETTINGS[coursename] - coursename = coursename.replace(' ','_') - if coursename in COURSE_SETTINGS: return COURSE_SETTINGS[coursename] + if coursename in COURSE_SETTINGS: + return COURSE_SETTINGS[coursename] + coursename = coursename.replace(' ', '_') + if coursename in COURSE_SETTINGS: + return COURSE_SETTINGS[coursename] return None + def is_valid_course(coursename): return get_course_settings(coursename) != None -def get_course_property(coursename,property): + +def get_course_property(coursename, property): cs = get_course_settings(coursename) - if not cs: return '' # raise exception instead? - if property in cs: return cs[property] - return '' # default + + # raise exception instead? + if not cs: + return '' + + if property in cs: + return cs[property] + + # default + return '' + def get_course_xmlpath(coursename): - return get_course_property(coursename,'xmlpath') + return get_course_property(coursename, 'xmlpath') + def get_course_title(coursename): - return get_course_property(coursename,'title') + return get_course_property(coursename, 'title') + def get_course_number(coursename): - return get_course_property(coursename,'number') - + return get_course_property(coursename, 'number') + +def get_course_location(coursename): + return get_course_property(coursename, 'location') diff --git a/lms/envs/common.py b/lms/envs/common.py index 60834f9d913..8c82b2e8c19 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -132,10 +132,25 @@ COURSE_DEFAULT = '6.002_Spring_2012' COURSE_SETTINGS = {'6.002_Spring_2012': {'number' : '6.002x', 'title' : 'Circuits and Electronics', 'xmlpath': '6002x/', + 'location': 'i4x://edx/6002xs12/course/6_002_Spring_2012', } } +############################### XModule Store ################################## +KEYSTORE = { + 'default': { + 'ENGINE': 'keystore.xml.XMLModuleStore', + 'OPTIONS': { + 'org': 'edx', + 'course': '6002xs12', + 'data_dir': DATA_DIR, + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + } + } +} + + ############################### DJANGO BUILT-INS ############################### # Change DEBUG/TEMPLATE_DEBUG in your environment settings files, not here DEBUG = False diff --git a/lms/envs/dev.py b/lms/envs/dev.py index decd92d136d..f175ca1f533 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -11,7 +11,7 @@ from .common import * from .logsettings import get_logger_config DEBUG = True -TEMPLATE_DEBUG = True +TEMPLATE_DEBUG = False LOGGING = get_logger_config(ENV_ROOT / "log", logging_env="dev", diff --git a/lms/lib/dogfood/views.py b/lms/lib/dogfood/views.py index 17096afc703..a91314d228b 100644 --- a/lms/lib/dogfood/views.py +++ b/lms/lib/dogfood/views.py @@ -184,29 +184,29 @@ def quickedit(request, id=None, qetemplate='quickedit.html',coursename=None): filestore = OSFS(settings.DATA_DIR + xp), #role = 'staff' if request.user.is_staff else 'student', # TODO: generalize this ) - instance=xmodule.get_module_class(module)(system, - xml, + instance = xmodule.get_module_class(module)(system, + xml, id, state=None) log.info('ajax_url = ' + instance.ajax_url) # create empty student state for this problem, if not previously existing - s = StudentModule.objects.filter(student=request.user, - module_id=id) + s = StudentModule.objects.filter(student=request.user, + module_state_key=id) if len(s) == 0 or s is None: - smod=StudentModule(student=request.user, - module_type = 'problem', - module_id=id, - state=instance.get_state()) + smod = StudentModule(student=request.user, + module_type='problem', + module_state_key=id, + state=instance.get_instance_state()) smod.save() lcp = instance.lcp pxml = lcp.tree - pxmls = etree.tostring(pxml,pretty_print=True) + pxmls = etree.tostring(pxml, pretty_print=True) return instance, pxmls - instance, pxmls = get_lcp(coursename,id) + instance, pxmls = get_lcp(coursename, id) # if there was a POST, then process it msg = '' @@ -246,8 +246,6 @@ def quickedit(request, id=None, qetemplate='quickedit.html',coursename=None): # get the rendered problem HTML phtml = instance.get_html() # phtml = instance.get_problem_html() - # init_js = instance.get_init_js() - # destory_js = instance.get_destroy_js() context = {'id':id, 'msg' : msg, diff --git a/lms/static/coffee/src/modules/sequence.coffee b/lms/static/coffee/src/modules/sequence.coffee index 32a90f51a52..a4a80e34078 100644 --- a/lms/static/coffee/src/modules/sequence.coffee +++ b/lms/static/coffee/src/modules/sequence.coffee @@ -1,6 +1,6 @@ class @Sequence - constructor: (@id, @elements, @tag, position) -> - @element = $("#sequence_#{@id}") + constructor: (@id, @element_id, @elements, @tag, position) -> + @element = $("#sequence_#{@element_id}") @buildNavigation() @initProgress() @bind() diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html index ab903457dcd..00221a49513 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -1,4 +1,4 @@ -<div id="sequence_${id}" class="sequence"> +<div id="sequence_${element_id}" class="sequence"> <nav aria-label="Section Navigation" class="sequence-nav"> <ol id="sequence-list"> </ol> @@ -22,7 +22,7 @@ <%block name="js_extra"> <script type="text/javascript"> $(function(){ - new Sequence('${id}', ${items}, '${tag}', ${position}); + new Sequence('${item_id}', '${element_id}', ${items}, '${tag}', ${position}); }); </script> </%block> diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index c68f4ae60e7..baa432fc933 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -1,7 +1,7 @@ <ol class="vert-mod"> -% for t in items: - <li id="vert-${items.index(t)}"> - ${t[1]['content']} +% for idx, item in enumerate(items): + <li id="vert-${idx}"> + ${item} </li> % endfor </ol> -- GitLab