From 21ba5019893ca0a5ff079baa35635dd81a3ec1a1 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 26 Jun 2012 14:35:47 -0400 Subject: [PATCH 01/40] Revert "Make import work via mako again, to unblock others while I work on making the LMS work using XModuleDescriptors" This reverts commit 6fdf44fe8d621ead310ea9de7b7674fd6adc8779. --- .../management/commands/import.py | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 9f0cd7f21c..43c908c1bc 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -7,7 +7,6 @@ from keystore.django import keystore from raw_module import RawDescriptor from lxml import etree from fs.osfs import OSFS -from mako.lookup import TemplateLookup from path import path from x_module import XModuleDescriptor, XMLParsingSystem @@ -28,32 +27,30 @@ class Command(BaseCommand): org, course, data_dir = args data_dir = path(data_dir) + with open(data_dir / "course.xml") as course_file: - class ImportSystem(XMLParsingSystem): - def __init__(self): - self.load_item = keystore().get_item - self.fs = OSFS(data_dir) + class ImportSystem(XMLParsingSystem): + def __init__(self): + self.load_item = keystore().get_item + self.fs = OSFS(data_dir) - def process_xml(self, xml): - try: - xml_data = etree.fromstring(xml) - except: - raise CommandError("Unable to parse xml: " + xml) + def process_xml(self, xml): + try: + xml_data = etree.fromstring(xml) + except: + raise CommandError("Unable to parse xml: " + xml) - if not xml_data.get('name'): - global unnamed_modules - unnamed_modules += 1 - xml_data.set('name', '{tag}_{count}'.format(tag=xml_data.tag, count=unnamed_modules)) + if not xml_data.get('name'): + global unnamed_modules + unnamed_modules += 1 + xml_data.set('name', '{tag}_{count}'.format(tag=xml_data.tag, count=unnamed_modules)) - module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, RawDescriptor) - keystore().create_item(module.url) - if 'data' in module.definition: - keystore().update_item(module.url, module.definition['data']) - if 'children' in module.definition: - keystore().update_children(module.url, module.definition['children']) - return module + module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, RawDescriptor) + keystore().create_item(module.url) + if 'data' in module.definition: + keystore().update_item(module.url, module.definition['data']) + if 'children' in module.definition: + keystore().update_children(module.url, module.definition['children']) + return module - lookup = TemplateLookup(directories=[data_dir]) - template = lookup.get_template("course.xml") - course_string = template.render(groups=[]) - ImportSystem().process_xml(course_string) + ImportSystem().process_xml(course_file.read()) From d02abac820d7572bcaa8a5c737cc3c044dd49734 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 27 Jun 2012 14:15:46 -0400 Subject: [PATCH 02/40] Restrict the set of characters allowed in locations further --- common/lib/keystore/__init__.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py index c0fb40d33e..77f0213281 100644 --- a/common/lib/keystore/__init__.py +++ b/common/lib/keystore/__init__.py @@ -15,6 +15,8 @@ URL_RE = re.compile(""" (/(?P[^/]+))? """, re.VERBOSE) +INVALID_CHARS = re.compile(r"[^\w-]") + class Location(object): ''' @@ -26,6 +28,14 @@ class Location(object): However, they can also be represented a dictionaries (specifying each component), tuples or list (specified in order), or as strings of the url ''' + + @classmethod + def clean(cls, value): + """ + Return value, made into a form legal for locations + """ + return re.sub('_+', '_', INVALID_CHARS.sub('_', value)) + def __init__(self, location): """ Create a new location that is a clone of the specifed one. @@ -45,7 +55,7 @@ class Location(object): In both the dict and list forms, the revision is optional, and can be ommitted. - None of the components of a location may contain the '/' character + Components must be composed of alphanumeric characters, or the characters _, and - Components may be set to None, which may be interpreted by some contexts to mean wildcard selection @@ -88,7 +98,7 @@ class Location(object): raise InvalidLocationError(location) for val in self.list(): - if val is not None and '/' in val: + if val is not None and INVALID_CHARS.search(val) is not None: raise InvalidLocationError(location) def __str__(self): From 3c054306c61575c01a0dd8d9b6b20abac3ac954e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 27 Jun 2012 14:17:15 -0400 Subject: [PATCH 03/40] Add the ability to specify Keystore engines and default descriptor classes by name from settings --- cms/envs/dev.py | 10 +++++++--- common/lib/keystore/django.py | 13 +++++++------ common/lib/keystore/mongo.py | 11 +++++++---- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/cms/envs/dev.py b/cms/envs/dev.py index 16bed60729..ce775d962a 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -8,9 +8,13 @@ TEMPLATE_DEBUG = DEBUG KEYSTORE = { 'default': { - 'host': 'localhost', - 'db': 'mongo_base', - 'collection': 'key_store', + 'ENGINE': 'keystore.mongo.MongoModuleStore', + 'OPTIONS': { + 'default_class': 'xmodule.raw_module.RawDescriptor', + 'host': 'localhost', + 'db': 'mongo_base', + 'collection': 'key_store', + } } } diff --git a/common/lib/keystore/django.py b/common/lib/keystore/django.py index 98479a7f7c..89aa9d07b0 100644 --- a/common/lib/keystore/django.py +++ b/common/lib/keystore/django.py @@ -6,9 +6,9 @@ Passes settings.KEYSTORE as kwargs to MongoModuleStore from __future__ import absolute_import +from importlib import import_module + from django.conf import settings -from .mongo import MongoModuleStore -from raw_module import RawDescriptor _KEYSTORES = {} @@ -17,9 +17,10 @@ def keystore(name='default'): global _KEYSTORES if name not in _KEYSTORES: - # TODO (cpennington): Load the default class from a string - _KEYSTORES[name] = MongoModuleStore( - default_class=RawDescriptor, - **settings.KEYSTORE[name]) + class_path = settings.KEYSTORE[name]['ENGINE'] + module_path, _, class_name = class_path.rpartition('.') + class_ = getattr(import_module(module_path), class_name) + _KEYSTORES[name] = class_( + **settings.KEYSTORE[name]['OPTIONS']) return _KEYSTORES[name] diff --git a/common/lib/keystore/mongo.py b/common/lib/keystore/mongo.py index ece8b35b71..4317ce0204 100644 --- a/common/lib/keystore/mongo.py +++ b/common/lib/keystore/mongo.py @@ -1,7 +1,9 @@ import pymongo +from importlib import import_module +from xmodule.x_module import XModuleDescriptor, DescriptorSystem + from . import ModuleStore, Location from .exceptions import ItemNotFoundError, InsufficientSpecificationError -from xmodule.x_module import XModuleDescriptor, DescriptorSystem class MongoModuleStore(ModuleStore): @@ -16,7 +18,10 @@ class MongoModuleStore(ModuleStore): # Force mongo to report errors, at the expense of performance self.collection.safe = True - self.default_class = default_class + + module_path, _, class_name = default_class.rpartition('.') + class_ = getattr(import_module(module_path), class_name) + self.default_class = class_ def get_item(self, location): """ @@ -29,8 +34,6 @@ class MongoModuleStore(ModuleStore): If no object is found at that location, raises keystore.exceptions.ItemNotFoundError location: Something that can be passed to Location - default_class: An XModuleDescriptor subclass to use if no plugin matching the - location is found """ query = {} From 5b8120280e5394ed9c090af213d406ac269fad37 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 27 Jun 2012 14:20:24 -0400 Subject: [PATCH 04/40] Move the resources_fs abstraction into the primary DescriptorSystem, as it is needed for more than just XMLParsing --- .../management/commands/import.py | 35 +++++++++---------- common/lib/keystore/mongo.py | 3 +- common/lib/xmodule/translation_module.py | 2 +- common/lib/xmodule/x_module.py | 11 +++--- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 43c908c1bc..1cfdf24e2d 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -31,26 +31,25 @@ class Command(BaseCommand): class ImportSystem(XMLParsingSystem): def __init__(self): - self.load_item = keystore().get_item - self.fs = OSFS(data_dir) + def process_xml(xml): + try: + xml_data = etree.fromstring(xml) + except: + raise CommandError("Unable to parse xml: " + xml) - def process_xml(self, xml): - try: - xml_data = etree.fromstring(xml) - except: - raise CommandError("Unable to parse xml: " + xml) + if not xml_data.get('name'): + global unnamed_modules + unnamed_modules += 1 + xml_data.set('name', '{tag}_{count}'.format(tag=xml_data.tag, count=unnamed_modules)) - if not xml_data.get('name'): - global unnamed_modules - unnamed_modules += 1 - xml_data.set('name', '{tag}_{count}'.format(tag=xml_data.tag, count=unnamed_modules)) + module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, RawDescriptor) + keystore().create_item(module.url) + if 'data' in module.definition: + keystore().update_item(module.url, module.definition['data']) + if 'children' in module.definition: + keystore().update_children(module.url, module.definition['children']) + return module - module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, RawDescriptor) - keystore().create_item(module.url) - if 'data' in module.definition: - keystore().update_item(module.url, module.definition['data']) - if 'children' in module.definition: - keystore().update_children(module.url, module.definition['children']) - return module + XMLParsingSystem.__init__(self, keystore().get_item, OSFS(data_dir), process_xml) ImportSystem().process_xml(course_file.read()) diff --git a/common/lib/keystore/mongo.py b/common/lib/keystore/mongo.py index 4317ce0204..20c4ffde1a 100644 --- a/common/lib/keystore/mongo.py +++ b/common/lib/keystore/mongo.py @@ -51,8 +51,9 @@ class MongoModuleStore(ModuleStore): if item is None: raise ItemNotFoundError(location) + # TODO (cpennington): Pass a proper resources_fs to the system return XModuleDescriptor.load_from_json( - item, DescriptorSystem(self.get_item), self.default_class) + item, DescriptorSystem(self.get_item, None), self.default_class) def create_item(self, location): """ diff --git a/common/lib/xmodule/translation_module.py b/common/lib/xmodule/translation_module.py index b56fed02cd..f5c8bc2fbc 100644 --- a/common/lib/xmodule/translation_module.py +++ b/common/lib/xmodule/translation_module.py @@ -23,7 +23,7 @@ def process_includes(fn): file = next_include.get('file') if file is not None: try: - ifp = system.fs.open(file) + ifp = system.resources_fs.open(file) except Exception: log.exception('Error in problem xml include: %s' % (etree.tostring(next_include, pretty_print=True))) log.exception('Cannot find file %s in %s' % (file, dir)) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 336ccc6d0c..aad4dd94dc 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -331,20 +331,21 @@ class XModuleDescriptor(Plugin): class DescriptorSystem(object): - def __init__(self, load_item): + def __init__(self, load_item, resources_fs): """ load_item: Takes a Location and returns an XModuleDescriptor + resources_fs: A Filesystem object that contains all of the + resources needed for the course """ self.load_item = load_item + self.resources_fs = resources_fs class XMLParsingSystem(DescriptorSystem): - def __init__(self, load_item, process_xml, fs): + def __init__(self, load_item, resources_fs, process_xml): """ process_xml: Takes an xml string, and returns the the XModuleDescriptor created from that xml - fs: A Filesystem object that contains all of the xml resources needed to parse - the course """ + DescriptorSystem.__init__(self, load_item, resources_fs) self.process_xml = process_xml - self.fs = fs From de07b8b345c21905f29982d9af0890caf7210206 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 27 Jun 2012 14:25:44 -0400 Subject: [PATCH 05/40] 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 | 118 +++---- 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 | 144 ++++----- 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 | 110 ++++--- 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, 814 insertions(+), 826 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 0000000000..d5baefd787 --- /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 307b544b79..e69de29bb2 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 0000000000..dda6a58c99 --- /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: + + + + + + """ + + 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 b59bc9de56..5047b94832 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 file %s has an error:%s' % (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 0000000000..d4f2a0fa33 --- /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 b35549d971..32963600cd 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 7bb94c9b63..43a92303ad 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 30175c16a8..f95729d4ab 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 ''.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 e3a19c3d60..b60f0e4656 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_instance_state(self): + return json.dumps({'position': self.position}) - def get_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] + 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(), + }) - 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 + print json.dumps(contents, indent=4) # Split 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>'), - 'id': self.item_id, - 'position': self.position, - 'titles': titles, - 'tag': self.xmltree.tag} + params = {'items': json.dumps(contents).replace('', '<"+"/script>'), + 'element_id': "-".join(str(v) for v in self.location.list()), + 'item_id': self.id, + 'position': self.position, + 'tag': self.location.category} - 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) + 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 17d7af50db..3e3e33805f 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 51f9447c06..ae276737e6 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 the text """ - 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 b3feec8bae..d3f4cd6ad3 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 f3d615fd3d..1585944cc9 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 aad4dd94dc..3787a76752 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 + items = [] + for child in self.get_children(): + items.extend(child.displayable_items()) - 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") + return items - self.xml = xml - self.json = json - self.item_id = item_id - self.state = state - self.DEBUG = False - - self.__xmltree = etree.fromstring(xml) # PRIVATE + def displayable_items(self): + ''' + Returns list of displayable modules contained by this module. If this module + is visible, should return [self] + ''' + return [self] - 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 + 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 0000000000..34881a4d61 --- /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 95c3afed8c..70e5eeeeb6 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 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 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 00bdffb697..3c2b654682 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 a97b09ae2b..6ca67a84e7 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] - - # @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) - - # return student_module - - @classmethod - def key_for(cls, student, module_id): - return "StudentModule-student_id:{0};module_id:{1}".format(student.id, module_id) + return '/'.join([self.module_type, self.student.username, self.module_state_key, str(self.state)[:20]]) -# def clear_cache_by_student_and_module_id(sender, instance, *args, **kwargs): -# k = sender.key_for(instance.student, instance.module_id) -# cache.delete(k) - -# 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) +# TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors -#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_model(StudentModule) +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 = [] + 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] + + 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 + + for child in descriptor.get_children(): + keys.extend(self._get_module_state_keys(child, new_depth)) + + return keys + + def lookup(self, module_type, module_state_key): + ''' + 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 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 3a6fcbfb45..d05bdcefab 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') + descriptor = keystore().get_item(location) - # 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) - - 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 5cbbe18d7d..6e8eb1ab9e 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 05b05c8ec9..4d568d55a1 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 60834f9d91..8c82b2e8c1 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 decd92d136..f175ca1f53 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 17096afc70..a91314d228 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 32a90f51a5..a4a80e3407 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 ab903457dc..00221a4951 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -1,4 +1,4 @@ -
+
" return html - def __init__(self, system, xml, item_id, instance_state=None, shared_state=None): - XModule.__init__(self, system, xml, item_id, instance_state, shared_state) + 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.attempts = 0 self.max_attempts = None - dom2 = etree.fromstring(xml) + dom2 = etree.fromstring(definition['data']) self.explanation = "problems/" + only_one(dom2.xpath('/problem/@explain'), default="closed") @@ -205,7 +192,7 @@ class Module(XModule): self.display_due_date = None grace_period_string = only_one(dom2.xpath('/problem/@graceperiod')) - if len(grace_period_string) >0 and self.display_due_date: + if len(grace_period_string) > 0 and self.display_due_date: self.grace_period = parse_timedelta(grace_period_string) self.close_date = self.display_due_date + self.grace_period #log.debug("Then parsed " + grace_period_string + " to closing date" + str(self.close_date)) @@ -240,9 +227,9 @@ class Module(XModule): 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" - self.name=only_one(dom2.xpath('/problem/@name')) - self.weight=only_one(dom2.xpath('/problem/@weight')) + self.filename = "problems/" + only_one(dom2.xpath('/problem/@filename')) + ".xml" + self.name = only_one(dom2.xpath('/problem/@name')) + self.weight = only_one(dom2.xpath('/problem/@weight')) if self.rerandomize == 'never': seed = 1 elif self.rerandomize == "per_student" and hasattr(system, 'id'): @@ -250,27 +237,27 @@ class Module(XModule): else: seed = None try: - fp = self.filestore.open(self.filename) - except Exception,err: - log.exception('[courseware.capa.capa_module.Module.init] error %s: cannot open file %s' % (err,self.filename)) - if self.DEBUG: + fp = self.system.filestore.open(self.filename) + except Exception: + log.exception('cannot open file %s' % self.filename) + if self.system.DEBUG: # create a dummy problem instead of failing fp = StringIO.StringIO('Problem file %s is missing' % self.filename) fp.name = "StringIO" else: raise try: - 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) + self.lcp = LoncapaProblem(fp, self.id, instance_state, seed=seed, system=self.system) + except Exception: + msg = 'cannot create LoncapaProblem %s' % self.filename log.exception(msg) - if self.DEBUG: - msg = '

%s

' % msg.replace('<','<') - msg += '

%s

' % traceback.format_exc().replace('<','<') + if self.system.DEBUG: + msg = '

%s

' % msg.replace('<', '<') + msg += '

%s

' % traceback.format_exc().replace('<', '<') # create a dummy problem with error message instead of failing - fp = StringIO.StringIO('Problem file %s has an error:%s' % (self.filename,msg)) + fp = StringIO.StringIO('Problem file %s has an error:%s' % (self.filename, msg)) fp.name = "StringIO" - self.lcp=LoncapaProblem(fp, self.item_id, instance_state, seed = seed, system=self.system) + self.lcp = LoncapaProblem(fp, self.id, instance_state, seed=seed, system=self.system) else: raise @@ -299,8 +286,8 @@ class Module(XModule): d = handlers[dispatch](get) after = self.get_progress() d.update({ - 'progress_changed' : after != before, - 'progress_status' : Progress.to_js_status_str(after), + 'progress_changed': after != before, + 'progress_status': Progress.to_js_status_str(after), }) return json.dumps(d, cls=ComplexEncoder) @@ -313,7 +300,6 @@ class Module(XModule): return False - def answer_available(self): ''' Is the user allowed to see an answer? ''' @@ -334,7 +320,8 @@ class Module(XModule): if self.show_answer == 'always': return True - raise self.system.exception404 #TODO: Not 404 + #TODO: Not 404 + raise self.system.exception404 def get_answer(self, get): ''' @@ -348,8 +335,7 @@ class Module(XModule): raise self.system.exception404 else: answers = self.lcp.get_question_answers() - return {'answers' : answers} - + return {'answers': answers} # Figure out if we should move these to capa_problem? def get_problem(self, get): @@ -358,8 +344,8 @@ class Module(XModule): Used if we want to reconfirm we have the right thing e.g. after several AJAX calls. - ''' - return {'html' : self.get_problem_html(encapsulate=False)} + ''' + return {'html': self.get_problem_html(encapsulate=False)} @staticmethod def make_dict_of_responses(get): @@ -409,18 +395,16 @@ class Module(XModule): correct_map = self.lcp.grade_answers(answers) except StudentInputError as inst: # TODO (vshnayder): why is this line here? - self.lcp = LoncapaProblem(self.filestore.open(self.filename), + self.lcp = LoncapaProblem(self.system.filestore.open(self.filename), id=lcp_id, state=old_state, system=self.system) traceback.print_exc() return {'success': inst.message} except: # TODO: why is this line here? - self.lcp = LoncapaProblem(self.filestore.open(self.filename), + self.lcp = LoncapaProblem(self.system.filestore.open(self.filename), id=lcp_id, state=old_state, system=self.system) traceback.print_exc() - raise Exception,"error in capa_module" - # TODO: Dead code... is this a bug, or just old? - return {'success':'Unknown Error'} + raise Exception("error in capa_module") self.attempts = self.attempts + 1 self.lcp.done = True @@ -431,21 +415,18 @@ class Module(XModule): if not correct_map.is_correct(answer_id): success = 'incorrect' - event_info['correct_map'] = correct_map.get_dict() # log this in the tracker + # log this in the tracker + event_info['correct_map'] = correct_map.get_dict() event_info['success'] = success self.tracker('save_problem_check', event_info) - try: - html = self.get_problem_html(encapsulate=False) # render problem into HTML - except Exception,err: - log.error('failed to generate html') - raise + # render problem into HTML + html = self.get_problem_html(encapsulate=False) return {'success': success, 'contents': html, } - def save_problem(self, get): ''' Save the passed in answers. @@ -471,8 +452,8 @@ class Module(XModule): if self.lcp.done and self.rerandomize == "always": event_info['failure'] = 'done' self.tracker('save_problem_fail', event_info) - return {'success' : False, - 'error' : "Problem needs to be reset prior to save."} + return {'success': False, + 'error': "Problem needs to be reset prior to save."} self.lcp.student_answers = answers @@ -485,7 +466,7 @@ class Module(XModule): and causes problem to rerender itself. Returns problem html as { 'html' : html-string }. - ''' + ''' event_info = dict() event_info['old_state'] = self.lcp.get_state() event_info['filename'] = self.filename @@ -503,12 +484,21 @@ class Module(XModule): self.lcp.do_reset() if self.rerandomize == "always": # reset random number generator seed (note the self.lcp.get_state() in next line) - self.lcp.seed=None - - self.lcp = LoncapaProblem(self.filestore.open(self.filename), - self.item_id, self.lcp.get_state(), system=self.system) + self.lcp.seed = None + + self.lcp = LoncapaProblem(self.system.filestore.open(self.filename), + self.id, self.lcp.get_state(), system=self.system) event_info['new_state'] = self.lcp.get_state() self.tracker('reset_problem', event_info) - return {'html' : self.get_problem_html(encapsulate=False)} + return {'html': self.get_problem_html(encapsulate=False)} + + +class CapaDescriptor(RawDescriptor): + """ + Module implementing problems in the LON-CAPA format, + as implemented by capa.capa_problem + """ + + module_class = CapaModule diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 3e3e33805f..a9f4e1f4dc 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -19,9 +19,10 @@ setup( "section = xmodule.translation_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", "vertical = xmodule.vertical_module:VerticalDescriptor", + "problem = xmodule.capa_module:CapaDescriptor", "problemset = xmodule.seq_module:SequenceDescriptor", - "videosequence = xmodule.seq_module:SequenceDescriptor", "video = xmodule.video_module:VideoDescriptor", + "videosequence = xmodule.seq_module:SequenceDescriptor", ] } ) diff --git a/common/lib/xmodule/vertical_module.py b/common/lib/xmodule/vertical_module.py index 6153aff324..6008eb4226 100644 --- a/common/lib/xmodule/vertical_module.py +++ b/common/lib/xmodule/vertical_module.py @@ -10,6 +10,9 @@ class_priority = ['video', 'problem'] class VerticalModule(XModule): ''' Layout module for laying out submodules vertically.''' def get_html(self): + if self.contents is None: + self.contents = [child.get_html() for child in self.get_display_items()] + return self.system.render_template('vert_module.html', { 'items': self.contents }) @@ -31,7 +34,7 @@ class VerticalModule(XModule): 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()] + self.contents = None class VerticalDescriptor(SequenceDescriptor): diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index d05bdcefab..d8ebb82adb 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -60,13 +60,7 @@ class I4xSystem(object): ''' self.ajax_url = ajax_url self.track_function = track_function - 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.filestore = filestore self.get_module = get_module self.render_function = render_function self.render_template = render_template @@ -241,7 +235,7 @@ def get_module(user, request, location, student_module_cache, position=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/' + descriptor.type + '/' + descriptor.url + '/' + ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.url + '/' def _get_module(location): (module, _, _, _) = get_module(user, request, location, student_module_cache, position) @@ -330,94 +324,33 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No return context -def modx_dispatch(request, module=None, dispatch=None, id=None): +def modx_dispatch(request, dispatch=None, id=None): ''' Generic view for extensions. This is where AJAX calls go. Arguments: - request -- the django request. - - module -- the type of the module, as used in the course configuration xml. - e.g. 'problem', 'video', etc - dispatch -- the command string to pass through to the module's handle_ajax call (e.g. 'problem_reset'). If this string contains '?', only pass through the part before the first '?'. - - id -- the module id. Used to look up the student module. - e.g. filenamexformularesponse + - id -- the module id. Used to look up the XModule instance ''' # ''' (fix emacs broken parsing) - if not request.user.is_authenticated(): - return redirect('/') - - # python concats adjacent strings - error_msg = ("We're sorry, this module is temporarily unavailable. " - "Our staff is working to fix it as soon as possible") # If there are arguments, get rid of them dispatch, _, _ = dispatch.partition('?') - 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: - xp = multicourse_settings.get_course_xmlpath(coursename) - data_root = settings.DATA_DIR + xp - else: - data_root = settings.DATA_DIR + student_module_cache = StudentModuleCache(request.user, keystore().get_item(id)) + instance, instance_module, shared_module, module_type = get_module(request.user, request, id, student_module_cache) - # Grab the XML corresponding to the request from course.xml - try: - xml = content_parser.module_xml(request.user, module, 'id', id, coursename) - except: - log.exception( - "Unable to load module during ajax call. module=%s, dispatch=%s, id=%s", - module, dispatch, id) - if accepts(request, 'text/html'): - return render_to_response("module-error.html", {}) - else: - response = HttpResponse(json.dumps({'success': error_msg})) - return response - - 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) + if instance_module is None: + log.debug("Couldn't find module '%s' for user '%s'", + id, request.user) 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, 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), - ) - - try: - module_class = xmodule.get_module_class(module) - 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'): - return render_to_response("module-error.html", {}) - else: - response = HttpResponse(json.dumps({'success': error_msg})) - return response + oldgrade = instance_module.grade + old_instance_state = instance_module.state + old_shared_state = shared_module.state if shared_module is not None else None # Let the module handle the AJAX try: @@ -427,16 +360,16 @@ def modx_dispatch(request, module=None, dispatch=None, id=None): raise # Save the state back to the database - instance_state.state = instance.get_instance_state() + instance_module.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() + instance_module.grade = instance.get_score()['score'] + if instance_module.grade != oldgrade or instance_module.state != old_instance_state: + instance_module.save() - if shared_state is not None: - shared_state.state = instance.get_shared_state() - if shared_state.state != old_shared_state: - shared_state.save() + if shared_module is not None: + shared_module.state = instance.get_shared_state() + if shared_module.state != old_shared_state: + shared_module.save() # Return whatever the module wanted to return to the client/caller return HttpResponse(ajax_return) diff --git a/lms/lib/dogfood/views.py b/lms/lib/dogfood/views.py index a91314d228..ba8601cc20 100644 --- a/lms/lib/dogfood/views.py +++ b/lms/lib/dogfood/views.py @@ -174,7 +174,7 @@ def quickedit(request, id=None, qetemplate='quickedit.html',coursename=None): module = 'problem' xml = content_parser.module_xml(request.user, module, 'id', id, coursename) - ajax_url = settings.MITX_ROOT_URL + '/modx/'+module+'/'+id+'/' + ajax_url = settings.MITX_ROOT_URL + '/modx/'+id+'/' # Create the module (instance of capa_module.Module) system = I4xSystem(track_function = make_track_function(request), diff --git a/lms/static/coffee/src/courseware.coffee b/lms/static/coffee/src/courseware.coffee index de232e05e4..4e57d13194 100644 --- a/lms/static/coffee/src/courseware.coffee +++ b/lms/static/coffee/src/courseware.coffee @@ -20,8 +20,8 @@ class @Courseware id = $(this).attr('id').replace(/video_/, '') new Video id, $(this).data('streams') $('.course-content .problems-wrapper').each -> - id = $(this).attr('id').replace(/problem_/, '') - new Problem id, $(this).data('url') + id = $(this).attr('problem-id') + new Problem id, $(this).attr('id'), $(this).data('url') $('.course-content .histogram').each -> id = $(this).attr('id').replace(/histogram_/, '') new Histogram id, $(this).data('histogram') diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index f29c9eb72b..eb2c057bef 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -1,6 +1,6 @@ class @Problem - constructor: (@id, url) -> - @element = $("#problem_#{id}") + constructor: (@id, @element_id, url) -> + @element = $("##{element_id}") @render() $: (selector) -> @@ -26,13 +26,13 @@ class @Problem @element.html(content) @bind() else - $.postWithPrefix "/modx/problem/#{@id}/problem_get", (response) => + $.postWithPrefix "/modx/#{@id}/problem_get", (response) => @element.html(response.html) @bind() check: => Logger.log 'problem_check', @answers - $.postWithPrefix "/modx/problem/#{@id}/problem_check", @answers, (response) => + $.postWithPrefix "/modx/#{@id}/problem_check", @answers, (response) => switch response.success when 'incorrect', 'correct' @render(response.contents) @@ -42,14 +42,14 @@ class @Problem reset: => Logger.log 'problem_reset', @answers - $.postWithPrefix "/modx/problem/#{@id}/problem_reset", id: @id, (response) => + $.postWithPrefix "/modx/#{@id}/problem_reset", id: @id, (response) => @render(response.html) @updateProgress response show: => if !@element.hasClass 'showed' Logger.log 'problem_show', problem: @id - $.postWithPrefix "/modx/problem/#{@id}/problem_show", (response) => + $.postWithPrefix "/modx/#{@id}/problem_show", (response) => answers = response.answers $.each answers, (key, value) => if $.isArray(value) @@ -69,7 +69,7 @@ class @Problem save: => Logger.log 'problem_save', @answers - $.postWithPrefix "/modx/problem/#{@id}/problem_save", @answers, (response) => + $.postWithPrefix "/modx/#{@id}/problem_save", @answers, (response) => if response.success alert 'Saved' @updateProgress response @@ -94,4 +94,4 @@ class @Problem element.schematic.update_value() @$(".CodeMirror").each (index, element) -> element.CodeMirror.save() if element.CodeMirror.save - @answers = @$("[id^=input_#{@id}_]").serialize() + @answers = @$("[id^=input_#{@element_id}_]").serialize() diff --git a/lms/static/coffee/src/modules/sequence.coffee b/lms/static/coffee/src/modules/sequence.coffee index a4a80e3407..2c979f0853 100644 --- a/lms/static/coffee/src/modules/sequence.coffee +++ b/lms/static/coffee/src/modules/sequence.coffee @@ -88,7 +88,7 @@ class @Sequence if @position != new_position if @position != undefined @mark_visited @position - $.postWithPrefix "/modx/#{@tag}/#{@id}/goto_position", position: new_position + $.postWithPrefix "/modx/#{@id}/goto_position", position: new_position @mark_active new_position @$('#seq_content').html @elements[new_position - 1].content diff --git a/lms/templates/problem_ajax.html b/lms/templates/problem_ajax.html index 78b85df3c1..6330edfac0 100644 --- a/lms/templates/problem_ajax.html +++ b/lms/templates/problem_ajax.html @@ -1 +1 @@ -
+
diff --git a/lms/urls.py b/lms/urls.py index 313be62c51..e43c949643 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -57,7 +57,7 @@ if settings.COURSEWARE_ENABLED: url(r'^courseware/(?P[^/]*)/$', 'courseware.views.index', name="courseware_course"), url(r'^jumpto/(?P[^/]+)/$', 'courseware.views.jump_to'), url(r'^section/(?P
[^/]*)/$', 'courseware.views.render_section'), - url(r'^modx/(?P[^/]*)/(?P[^/]*)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'), + url(r'^modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'), url(r'^profile$', 'courseware.views.profile'), url(r'^profile/(?P[^/]*)/$', 'courseware.views.profile'), url(r'^change_setting$', 'student.views.change_setting'), From c140fe87662844ed6fa01f921d72afede53be4e8 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 27 Jun 2012 16:29:49 -0400 Subject: [PATCH 09/40] Get problem execution working with problems read from keystore --- common/lib/keystore/xml.py | 3 +++ common/lib/xmodule/capa_module.py | 26 ++++++++++---------- lms/static/coffee/src/modules/problem.coffee | 2 +- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py index d5baefd787..988916ed39 100644 --- a/common/lib/keystore/xml.py +++ b/common/lib/keystore/xml.py @@ -7,6 +7,9 @@ from xmodule.x_module import XModuleDescriptor, XMLParsingSystem from . import ModuleStore, Location from .exceptions import ItemNotFoundError +etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False, + remove_comments=True)) + class XMLModuleStore(ModuleStore): """ diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 0563017ff2..4e40bffb48 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -247,7 +247,7 @@ class CapaModule(XModule): else: raise try: - self.lcp = LoncapaProblem(fp, self.id, instance_state, seed=seed, system=self.system) + self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system) except Exception: msg = 'cannot create LoncapaProblem %s' % self.filename log.exception(msg) @@ -257,7 +257,7 @@ class CapaModule(XModule): # create a dummy problem with error message instead of failing fp = StringIO.StringIO('Problem file %s has an error:%s' % (self.filename, msg)) fp.name = "StringIO" - self.lcp = LoncapaProblem(fp, self.id, instance_state, seed=seed, system=self.system) + self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system) else: raise @@ -378,7 +378,7 @@ class CapaModule(XModule): # Too late. Cannot submit if self.closed(): event_info['failure'] = 'closed' - self.tracker('save_problem_check_fail', event_info) + self.system.track_function('save_problem_check_fail', event_info) # TODO (vshnayder): probably not 404? raise self.system.exception404 @@ -386,7 +386,7 @@ class CapaModule(XModule): # again. if self.lcp.done and self.rerandomize == "always": event_info['failure'] = 'unreset' - self.tracker('save_problem_check_fail', event_info) + self.system.track_function('save_problem_check_fail', event_info) raise self.system.exception404 try: @@ -415,10 +415,10 @@ class CapaModule(XModule): if not correct_map.is_correct(answer_id): success = 'incorrect' - # log this in the tracker + # log this in the track_function event_info['correct_map'] = correct_map.get_dict() event_info['success'] = success - self.tracker('save_problem_check', event_info) + self.system.track_function('save_problem_check', event_info) # render problem into HTML html = self.get_problem_html(encapsulate=False) @@ -443,7 +443,7 @@ class CapaModule(XModule): # Too late. Cannot submit if self.closed(): event_info['failure'] = 'closed' - self.tracker('save_problem_fail', event_info) + self.system.track_function('save_problem_fail', event_info) return {'success': False, 'error': "Problem is closed"} @@ -451,14 +451,14 @@ class CapaModule(XModule): # again. if self.lcp.done and self.rerandomize == "always": event_info['failure'] = 'done' - self.tracker('save_problem_fail', event_info) + self.system.track_function('save_problem_fail', event_info) return {'success': False, 'error': "Problem needs to be reset prior to save."} self.lcp.student_answers = answers # TODO: should this be save_problem_fail? Looks like success to me... - self.tracker('save_problem_fail', event_info) + self.system.track_function('save_problem_fail', event_info) return {'success': True} def reset_problem(self, get): @@ -473,12 +473,12 @@ class CapaModule(XModule): if self.closed(): event_info['failure'] = 'closed' - self.tracker('reset_problem_fail', event_info) + self.system.track_function('reset_problem_fail', event_info) return "Problem is closed" if not self.lcp.done: event_info['failure'] = 'not_done' - self.tracker('reset_problem_fail', event_info) + self.system.track_function('reset_problem_fail', event_info) return "Refresh the page and make an attempt before resetting." self.lcp.do_reset() @@ -487,10 +487,10 @@ class CapaModule(XModule): self.lcp.seed = None self.lcp = LoncapaProblem(self.system.filestore.open(self.filename), - self.id, self.lcp.get_state(), system=self.system) + self.location.html_id(), self.lcp.get_state(), system=self.system) event_info['new_state'] = self.lcp.get_state() - self.tracker('reset_problem', event_info) + self.system.track_function('reset_problem', event_info) return {'html': self.get_problem_html(encapsulate=False)} diff --git a/lms/static/coffee/src/modules/problem.coffee b/lms/static/coffee/src/modules/problem.coffee index eb2c057bef..85186a2903 100644 --- a/lms/static/coffee/src/modules/problem.coffee +++ b/lms/static/coffee/src/modules/problem.coffee @@ -94,4 +94,4 @@ class @Problem element.schematic.update_value() @$(".CodeMirror").each (index, element) -> element.CodeMirror.save() if element.CodeMirror.save - @answers = @$("[id^=input_#{@element_id}_]").serialize() + @answers = @$("[id^=input_#{@element_id.replace(/problem_/, '')}_]").serialize() From bae90ab16d86f89579b3d306beb9cb092a1dc2ab Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 08:56:18 -0400 Subject: [PATCH 10/40] Make custom tags work in the LMS --- common/lib/xmodule/setup.py | 12 +++++++++--- common/lib/xmodule/template_module.py | 21 ++++++++++----------- common/lib/xmodule/translation_module.py | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index a9f4e1f4dc..38509f182a 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -13,15 +13,21 @@ setup( # for a description of entry_points entry_points={ 'xmodule.v1': [ + "book = xmodule.translation_module:TranslateCustomTagDescriptor", "chapter = xmodule.seq_module:SequenceDescriptor", "course = xmodule.seq_module:SequenceDescriptor", + "customtag = xmodule.template_module:CustomTagDescriptor", + "discuss = xmodule.translation_module:TranslateCustomTagDescriptor", "html = xmodule.html_module:HtmlDescriptor", - "section = xmodule.translation_module:SemanticSectionDescriptor", - "sequential = xmodule.seq_module:SequenceDescriptor", - "vertical = xmodule.vertical_module:VerticalDescriptor", + "image = xmodule.translation_module:TranslateCustomTagDescriptor", "problem = xmodule.capa_module:CapaDescriptor", "problemset = xmodule.seq_module:SequenceDescriptor", + "section = xmodule.translation_module:SemanticSectionDescriptor", + "sequential = xmodule.seq_module:SequenceDescriptor", + "slides = xmodule.translation_module:TranslateCustomTagDescriptor", + "vertical = xmodule.vertical_module:VerticalDescriptor", "video = xmodule.video_module:VideoDescriptor", + "videodev = xmodule.translation_module:TranslateCustomTagDescriptor", "videosequence = xmodule.seq_module:SequenceDescriptor", ] } diff --git a/common/lib/xmodule/template_module.py b/common/lib/xmodule/template_module.py index ae276737e6..52c05616cf 100644 --- a/common/lib/xmodule/template_module.py +++ b/common/lib/xmodule/template_module.py @@ -1,14 +1,9 @@ -import json - -from x_module import XModule, XModuleDescriptor +from xmodule.x_module import XModule +from xmodule.raw_module import RawDescriptor from lxml import etree -class ModuleDescriptor(XModuleDescriptor): - pass - - -class Module(XModule): +class CustomTagModule(XModule): """ This module supports tags of the form @@ -34,9 +29,13 @@ class Module(XModule): def get_html(self): return self.html - 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) + 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']) filename = xmltree.find('impl').text params = dict(xmltree.items()) self.html = self.system.render_template(filename, params, namespace='custom_tags') + + +class CustomTagDescriptor(RawDescriptor): + module_class = CustomTagModule diff --git a/common/lib/xmodule/translation_module.py b/common/lib/xmodule/translation_module.py index f5c8bc2fbc..6c358d4eaa 100644 --- a/common/lib/xmodule/translation_module.py +++ b/common/lib/xmodule/translation_module.py @@ -65,3 +65,20 @@ class SemanticSectionDescriptor(XModuleDescriptor): else: xml_object.tag = 'sequence' return system.process_xml(etree.tostring(xml_object)) + + +class TranslateCustomTagDescriptor(XModuleDescriptor): + @classmethod + def from_xml(cls, xml_data, system, org=None, course=None): + """ + Transforms the xml_data from <$custom_tag attr="" attr=""/> to + $custom_tag + """ + + xml_object = etree.fromstring(xml_data) + tag = xml_object.tag + xml_object.tag = 'customtag' + impl = etree.SubElement(xml_object, 'impl') + impl.text = tag + + return system.process_xml(etree.tostring(xml_object)) From 7d16dbbcb468f7eeb634977841b476f33cb881ce Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 08:58:18 -0400 Subject: [PATCH 11/40] Clean up module_render.py --- lms/djangoapps/courseware/module_render.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index d8ebb82adb..de51764304 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -5,19 +5,12 @@ from lxml import etree from django.http import Http404 from django.http import HttpResponse -from django.shortcuts import redirect - -from fs.osfs import OSFS from django.conf import settings -from mitxmako.shortcuts import render_to_string, render_to_response +from mitxmako.shortcuts import render_to_string 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") @@ -83,8 +76,6 @@ class I4xSystem(object): return str(self.__dict__) - - def make_track_function(request): ''' Make a tracking function that logs what happened. From 5cd388d638fab17cbb37bb2d768002d9b4726299 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 10:35:39 -0400 Subject: [PATCH 12/40] Have the CMS use the same XMLModuleStore for import that the LMS uses for reading content --- .../management/commands/import.py | 39 ++++--------------- cms/djangoapps/contentstore/views.py | 2 +- common/lib/keystore/__init__.py | 4 +- common/lib/keystore/xml.py | 6 ++- 4 files changed, 15 insertions(+), 36 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 1cfdf24e2d..a7f95ea3c0 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -4,12 +4,8 @@ from django.core.management.base import BaseCommand, CommandError from keystore.django import keystore -from raw_module import RawDescriptor from lxml import etree -from fs.osfs import OSFS - -from path import path -from x_module import XModuleDescriptor, XMLParsingSystem +from keystore.xml import XMLModuleStore unnamed_modules = 0 @@ -26,30 +22,11 @@ class Command(BaseCommand): raise CommandError("import requires 3 arguments: ") org, course, data_dir = args - data_dir = path(data_dir) - with open(data_dir / "course.xml") as course_file: - class ImportSystem(XMLParsingSystem): - def __init__(self): - def process_xml(xml): - try: - xml_data = etree.fromstring(xml) - except: - raise CommandError("Unable to parse xml: " + xml) - - if not xml_data.get('name'): - global unnamed_modules - unnamed_modules += 1 - xml_data.set('name', '{tag}_{count}'.format(tag=xml_data.tag, count=unnamed_modules)) - - module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, RawDescriptor) - keystore().create_item(module.url) - if 'data' in module.definition: - keystore().update_item(module.url, module.definition['data']) - if 'children' in module.definition: - keystore().update_children(module.url, module.definition['children']) - return module - - XMLParsingSystem.__init__(self, keystore().get_item, OSFS(data_dir), process_xml) - - ImportSystem().process_xml(course_file.read()) + module_store = XMLModuleStore(org, course, data_dir, 'xmodule.raw_module.RawDescriptor') + for module in module_store.modules.itervalues(): + keystore().create_item(module.url) + if 'data' in module.definition: + keystore().update_item(module.url, module.definition['data']) + if 'children' in module.definition: + keystore().update_children(module.url, module.definition['children']) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 9cc7eec9b2..b85e9c05bf 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -10,7 +10,7 @@ def index(request): # TODO (cpennington): These need to be read in from the active user org = 'mit.edu' course = '6002xs12' - name = '6.002 Spring 2012' + name = '6.002_Spring_2012' course = keystore().get_item(['i4x', org, course, 'course', name]) weeks = course.get_children() return render_to_response('index.html', {'weeks': weeks}) diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py index df9b72839e..14716fbc2d 100644 --- a/common/lib/keystore/__init__.py +++ b/common/lib/keystore/__init__.py @@ -15,7 +15,7 @@ URL_RE = re.compile(""" (/(?P[^/]+))? """, re.VERBOSE) -INVALID_CHARS = re.compile(r"[^\w-]") +INVALID_CHARS = re.compile(r"[^\w.-]") class Location(object): @@ -55,7 +55,7 @@ class Location(object): In both the dict and list forms, the revision is optional, and can be ommitted. - Components must be composed of alphanumeric characters, or the characters _, and - + Components must be composed of alphanumeric characters, or the characters '_', '-', and '.' Components may be set to None, which may be interpreted by some contexts to mean wildcard selection diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py index 988916ed39..dcddb2718e 100644 --- a/common/lib/keystore/xml.py +++ b/common/lib/keystore/xml.py @@ -10,6 +10,8 @@ from .exceptions import ItemNotFoundError etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False, remove_comments=True)) +log = logging.getLogger(__name__) + class XMLModuleStore(ModuleStore): """ @@ -23,7 +25,7 @@ class XMLModuleStore(ModuleStore): class_ = getattr(import_module(module_path), class_name) self.default_class = class_ - with open(data_dir / "course.xml") as course_file: + with open(self.data_dir / "course.xml") as course_file: class ImportSystem(XMLParsingSystem): def __init__(self, keystore): self.unnamed_modules = 0 @@ -32,7 +34,7 @@ class XMLModuleStore(ModuleStore): try: xml_data = etree.fromstring(xml) except: - print xml + log.exception("Unable to parse xml:" + xml) raise if xml_data.get('name'): xml_data.set('slug', Location.clean(xml_data.get('name'))) From 23195e9d76311bea140e8f5e77f5ec2037b1fb62 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 13:08:09 -0400 Subject: [PATCH 13/40] Make user profiles work again after the switch to an XMLModuleStore. Staff user histograms are still broken --- common/lib/xmodule/abtest_module.py | 3 - common/lib/xmodule/capa_module.py | 8 +- common/lib/xmodule/video_module.py | 4 - common/lib/xmodule/x_module.py | 1 + lms/djangoapps/courseware/grades.py | 215 +++++++++------------ lms/djangoapps/courseware/models.py | 12 +- lms/djangoapps/courseware/module_render.py | 21 +- lms/djangoapps/courseware/views.py | 21 +- lms/envs/common.py | 2 +- lms/templates/video.html | 8 - 10 files changed, 133 insertions(+), 162 deletions(-) diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py index dda6a58c99..e14117eb08 100644 --- a/common/lib/xmodule/abtest_module.py +++ b/common/lib/xmodule/abtest_module.py @@ -88,8 +88,5 @@ class Module(XModule): 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 4e40bffb48..b6bfc91e80 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -229,7 +229,13 @@ class CapaModule(XModule): # TODO: Should be: self.filename=only_one(dom2.xpath('/problem/@filename')) self.filename = "problems/" + only_one(dom2.xpath('/problem/@filename')) + ".xml" self.name = only_one(dom2.xpath('/problem/@name')) - self.weight = only_one(dom2.xpath('/problem/@weight')) + + weight_string = only_one(dom2.xpath('/problem/@weight')) + if weight_string: + self.weight = float(weight_string) + else: + self.weight = 1 + if self.rerandomize == 'never': seed = 1 elif self.rerandomize == "per_student" and hasattr(system, 'id'): diff --git a/common/lib/xmodule/video_module.py b/common/lib/xmodule/video_module.py index 86f7c0c64d..4aa469db7f 100644 --- a/common/lib/xmodule/video_module.py +++ b/common/lib/xmodule/video_module.py @@ -50,7 +50,6 @@ class VideoModule(XModule): 'id': self.location.html_id(), 'position': self.position, 'name': self.name, - 'annotations': self.annotations, }) def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): @@ -65,9 +64,6 @@ class VideoModule(XModule): if 'position' in state: self.position = int(float(state['position'])) - self.annotations = [(e.get("name"), self.render_function(e)) \ - for e in xmltree] - class VideoDescriptor(RawDescriptor): module_class = VideoModule diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 838611f81f..71d069fdbe 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -83,6 +83,7 @@ class XModule(object): self.id = self.location.url() self.name = self.location.name self.display_name = kwargs.get('display_name', '') + self.type = self.location.category self._loaded_children = None def get_name(self): diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 3c2b654682..6f6ff71d35 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -3,23 +3,21 @@ Course settings module. The settings are based of django.conf. All settings in courseware.global_course_settings are first applied, and then any settings in the settings.DATA_DIR/course_settings.py are applied. A setting must be in ALL_CAPS. - + Settings are used by calling - + from courseware import course_settings -Note that courseware.course_settings is not a module -- it's an object. So +Note that courseware.course_settings is not a module -- it's an object. So importing individual settings is not possible: from courseware.course_settings import GRADER # This won't work. """ -from lxml import etree import random import imp import logging -import sys import types from django.conf import settings @@ -28,21 +26,19 @@ from courseware import global_course_settings from xmodule import graders from xmodule.graders import Score from models import StudentModule -import courseware.content_parser as content_parser -import xmodule _log = logging.getLogger("mitx.courseware") + class Settings(object): def __init__(self): # update this dict from global settings (but only for ALL_CAPS settings) for setting in dir(global_course_settings): if setting == setting.upper(): setattr(self, setting, getattr(global_course_settings, setting)) - - + data_dir = settings.DATA_DIR - + fp = None try: fp, pathname, description = imp.find_module("course_settings", [data_dir]) @@ -53,154 +49,127 @@ class Settings(object): finally: if fp: fp.close() - + for setting in dir(mod): if setting == setting.upper(): setting_value = getattr(mod, setting) setattr(self, setting, setting_value) - + # Here is where we should parse any configurations, so that we can fail early self.GRADER = graders.grader_from_conf(self.GRADER) course_settings = Settings() - - -def grade_sheet(student,coursename=None): +def grade_sheet(student, course, student_module_cache): """ This pulls a summary of all problems in the course. It returns a dictionary with two datastructures: - + - courseware_summary is a summary of all sections with problems in the course. It is organized as an array of chapters, each containing an array of sections, each containing an array of scores. This contains information for graded and ungraded problems, and is good for displaying a course summary with due dates, etc. - + - grade_summary is the output from the course grader. More information on the format is in the docstring for CourseGrader. + + Arguments: + student: A User object for the student to grade + course: An XModule containing the course to grade + student_module_cache: A StudentModuleCache initialized with all instance_modules for the student """ - dom=content_parser.course_file(student,coursename) - course = dom.xpath('//course/@name')[0] - xmlChapters = dom.xpath('//course[@name=$course]/chapter', course=course) - - responses = StudentModule.objects.filter(student=student) - response_by_id = {} - for response in responses: - response_by_id[response.module_state_key] = response - - totaled_scores = {} - chapters=[] - for c in xmlChapters: + chapters = [] + for c in course.get_children(): sections = [] - chname=c.get('name') - - - for s in dom.xpath('//course[@name=$course]/chapter[@name=$chname]/section', - course=course, chname=chname): - problems=dom.xpath('//course[@name=$course]/chapter[@name=$chname]/section[@name=$section]//problem', - course=course, chname=chname, section=s.get('name')) + for s in c.get_children(): + def yield_descendents(module): + yield module + for child in module.get_display_items(): + for module in yield_descendents(child): + yield module - graded = True if s.get('graded') == "true" else False - scores=[] - if len(problems)>0: - for p in problems: - (correct,total) = get_score(student, p, response_by_id, coursename=coursename) - - if settings.GENERATE_PROFILE_SCORES: - if total > 1: - correct = random.randrange( max(total-2, 1) , total + 1 ) - else: - correct = total - - if not total > 0: - #We simply cannot grade a problem that is 12/0, because we might need it as a percentage - graded = False - scores.append( Score(correct,total, graded, p.get("name")) ) + graded = getattr(s, 'graded', False) + scores = [] + for module in yield_descendents(s): + (correct, total) = get_score(student, module, student_module_cache) - section_total, graded_total = graders.aggregate_scores(scores, s.get("name")) - #Add the graded total to totaled_scores - format = s.get('format', "") - subtitle = s.get('subtitle', format) - if format and graded_total[1] > 0: - format_scores = totaled_scores.get(format, []) - format_scores.append( graded_total ) - totaled_scores[ format ] = format_scores + if settings.GENERATE_PROFILE_SCORES: + if total > 1: + correct = random.randrange(max(total - 2, 1), total + 1) + else: + correct = total - section_score={'section':s.get("name"), - 'scores':scores, - 'section_total' : section_total, - 'format' : format, - 'subtitle' : subtitle, - 'due' : s.get("due") or "", - 'graded' : graded, - } - sections.append(section_score) + if not total > 0: + #We simply cannot grade a problem that is 12/0, because we might need it as a percentage + graded = False + + if correct is not None and total is not None: + scores.append(Score(correct, total, graded, module.display_name)) + + section_total, graded_total = graders.aggregate_scores(scores, s.display_name) + #Add the graded total to totaled_scores + format = getattr(s, 'format', "") + subtitle = getattr(s, 'subtitle', format) + if format and graded_total[1] > 0: + format_scores = totaled_scores.get(format, []) + format_scores.append(graded_total) + totaled_scores[format] = format_scores + + sections.append({ + 'section': s.display_name, + 'scores': scores, + 'section_total': section_total, + 'format': format, + 'subtitle': subtitle, + 'due': getattr(s, "due", ""), + 'graded': graded, + }) + + chapters.append({'course': course.display_name, + 'chapter': c.display_name, + 'sections': sections}) - chapters.append({'course':course, - 'chapter' : c.get("name"), - 'sections' : sections,}) - - grader = course_settings.GRADER grade_summary = grader.grade(totaled_scores) - - return {'courseware_summary' : chapters, - 'grade_summary' : grade_summary} -def get_score(user, problem, cache, coursename=None): + return {'courseware_summary': chapters, + 'grade_summary': grade_summary} + + +def get_score(user, problem, cache): """ 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 + problem: an XModule + cache: A StudentModuleCache """ - ## HACK: assumes max score is fixed per problem - 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 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 + instance_module = cache.lookup(problem.type, problem.id) + if instance_module is None: + instance_module = StudentModule(module_type=problem.type, + module_state_key=problem.id, + student=user, + state=None, + grade=0, + max_grade=problem.max_score(), + done='i') + cache.append(instance_module) + instance_module.save() - # Grab the # correct from cache - if id in cache: - response = cache[id] - if response.grade != None: - correct = float(response.grade) + # If this problem is ungraded/ungradable, bail + if instance_module.max_grade is None: + return (None, None) - # 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 - else: - ## HACK 1: We shouldn't specifically reference capa_module - ## HACK 2: Backwards-compatibility: This should be written when a grade is saved, and removed from the system - # TODO: These are no longer correct params for I4xSystem -- figure out what this code - # does, clean it up. - # from module_render import I4xSystem - # system = I4xSystem(None, None, None, coursename=coursename) - # total=float(xmodule.capa_module.Module(system, etree.tostring(problem), "id").max_score()) - # response.max_grade = total - # response.save() - total = 1 - # For a temporary fix, we just assume a problem is worth 1 point if we haven't seen it before. This is totally incorrect - - #Now we re-weight the problem, if specified - weight = problem.get("weight", None) - if weight: - weight = float(weight) - correct = correct * weight / total - total = weight + correct = instance_module.grade if instance_module.grade is not None else 0 + total = instance_module.max_grade + + if correct is not None and total is not None: + #Now we re-weight the problem, if specified + weight = getattr(problem, 'weight', 1) + if weight != 1: + correct = correct * weight / total + total = weight return (correct, total) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 6ca67a84e7..262d177248 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -75,8 +75,16 @@ class StudentModuleCache(object): ''' 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)) + + # This works around a limitation in sqlite3 on the number of parameters + # that can be put into a single query + self.cache = [] + chunk_size = 500 + for id_chunk in [module_ids[i:i+chunk_size] for i in xrange(0, len(module_ids), chunk_size)]: + self.cache.extend(StudentModule.objects.filter( + student=user, + module_state_key__in=id_chunk) + ) else: self.cache = [] diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index de51764304..4c82eba974 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -29,7 +29,7 @@ class I4xSystem(object): and user, or other environment-specific info. ''' def __init__(self, ajax_url, track_function, render_function, - get_module, render_template, request=None, + get_module, render_template, user=None, filestore=None): ''' Create a closure around the system environment. @@ -47,7 +47,7 @@ class I4xSystem(object): and 'type'. render_template - a function that takes (template_file, context), and returns rendered html. - request - the request in progress + user - The user to base the seed off of for this request filestore - A filestore ojbect. Defaults to an instance of OSFS based at settings.DATA_DIR. ''' @@ -59,7 +59,7 @@ class I4xSystem(object): self.render_template = render_template self.exception404 = Http404 self.DEBUG = settings.DEBUG - self.id = request.user.id if request is not None else 0 + self.seed = user.id if user is not None else 0 def get(self, attr): ''' provide uniform access to attributes (like etree).''' @@ -234,13 +234,13 @@ def get_module(user, request, location, student_module_cache, position=None): system = I4xSystem(track_function=make_track_function(request), render_function=lambda xml: render_x_module( - user, request, xml, student_module_cache, position), + user, xml, student_module_cache, position), 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, + user=user, ) # pass position specified in URL to module through I4xSystem system.set('position', position) @@ -272,7 +272,7 @@ def get_module(user, request, location, student_module_cache, position=None): return (module, instance_module, shared_module, descriptor.type) -def render_x_module(user, request, module_xml, student_module_cache, position=None): +def render_x_module(user, module, student_module_cache, position=None): ''' Generic module for extensions. This renders to HTML. modules include sequential, vertical, problem, video, html @@ -282,10 +282,9 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No Arguments: - user : current django User - - request : current django HTTPrequest - - module_xml : lxml etree of xml subtree for the current 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 position within module + - module : lxml etree of xml subtree for the current 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 position within module Returns: @@ -296,7 +295,7 @@ def render_x_module(user, request, module_xml, student_module_cache, position=No return {"content": ""} (instance, _, _, module_type) = get_module( - user, request, module_xml, student_module_cache, position) + user, module_xml, student_module_cache, position) content = instance.get_html() diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 6e8eb1ab9e..56237f605a 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -20,17 +20,16 @@ from module_render import render_x_module, toc_for_course, get_module, get_secti from models import StudentModuleCache from student.models import UserProfile from multicourse import multicourse_settings +from keystore.django import keystore -import courseware.content_parser as content_parser - -import courseware.grades as grades +from courseware import grades, content_parser log = logging.getLogger("mitx.courseware") etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False, - remove_comments = True)) + remove_comments=True)) -template_imports={'urllib':urllib} +template_imports = {'urllib': urllib} @cache_control(no_cache=True, no_store=True, must_revalidate=True) def gradebook(request): @@ -49,6 +48,7 @@ def gradebook(request): return render_to_response('gradebook.html', {'students': student_info}) + @login_required @cache_control(no_cache=True, no_store=True, must_revalidate=True) def profile(request, student_id=None): @@ -60,11 +60,14 @@ def profile(request, student_id=None): else: if 'course_admin' not in content_parser.user_groups(request.user): raise Http404 - student = User.objects.get( id = int(student_id)) + student = User.objects.get(id=int(student_id)) - user_info = UserProfile.objects.get(user=student) # request.user.profile_cache # + user_info = UserProfile.objects.get(user=student) coursename = multicourse_settings.get_coursename_from_request(request) + course_location = multicourse_settings.get_course_location(coursename) + student_module_cache = StudentModuleCache(request.user, keystore().get_item(course_location)) + course, _, _, _ = get_module(request.user, request, course_location, student_module_cache) context = {'name': user_info.name, 'username': student.username, @@ -74,7 +77,7 @@ def profile(request, student_id=None): 'format_url_params': content_parser.format_url_params, 'csrf': csrf(request)['csrf_token'] } - context.update(grades.grade_sheet(student, coursename)) + context.update(grades.grade_sheet(student, course, student_module_cache)) return render_to_response('profile.html', context) @@ -127,7 +130,7 @@ def render_section(request, section): student_module_cache = StudentModuleCache(request.user, dom) try: - module = render_x_module(user, request, dom, student_module_cache) + module = render_x_module(user, dom, student_module_cache) except: log.exception("Unable to load module") context.update({ diff --git a/lms/envs/common.py b/lms/envs/common.py index 8c82b2e8c1..d1faf00f62 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -132,7 +132,7 @@ 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', + 'location': 'i4x://edx/6002xs12/course/6.002_Spring_2012', } } diff --git a/lms/templates/video.html b/lms/templates/video.html index f49b5b56c2..9f38d386a4 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -13,11 +13,3 @@
- -
    -% for t in annotations: -
  1. - ${t[1]['content']} -
    2. -% endfor -
From 35af8101d763952eeb3983883609e27e78db4638 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 13:40:37 -0400 Subject: [PATCH 14/40] Make grade graph on profile work correctly --- common/lib/xmodule/translation_module.py | 2 -- common/lib/xmodule/x_module.py | 11 ++++++++++- common/lib/xmodule/xml_module.py | 4 +++- lms/djangoapps/courseware/grades.py | 8 +++++--- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/translation_module.py b/common/lib/xmodule/translation_module.py index 6c358d4eaa..d379ced507 100644 --- a/common/lib/xmodule/translation_module.py +++ b/common/lib/xmodule/translation_module.py @@ -57,8 +57,6 @@ class SemanticSectionDescriptor(XModuleDescriptor): if len(xml_object) == 1: for (key, val) in xml_object.items(): - if key == 'format': - continue xml_object[0].set(key, val) return system.process_xml(etree.tostring(xml_object[0])) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 71d069fdbe..8ee3df38ff 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -85,6 +85,8 @@ class XModule(object): self.display_name = kwargs.get('display_name', '') self.type = self.location.category self._loaded_children = None + self.graded = kwargs.get('graded', False) + self.format = kwargs.get('format') def get_name(self): name = self.__xmltree.get('name') @@ -281,6 +283,9 @@ class XModuleDescriptor(Plugin): Current arguments passed in kwargs: location: A keystore.Location object indicating the name and ownership of this problem goals: A list of strings of learning goals associated with this module + display_name: The name to use for displaying this module to the user + format: The format of this module ('Homework', 'Lab', etc) + graded (bool): Whether this module is should be graded or not """ self.system = system self.definition = definition if definition is not None else {} @@ -288,6 +293,8 @@ class XModuleDescriptor(Plugin): self.type = Location(kwargs.get('location')).category self.url = Location(kwargs.get('location')).url() self.display_name = kwargs.get('display_name') + self.format = kwargs.get('format') + self.graded = kwargs.get('graded', False) # 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 @@ -315,7 +322,9 @@ class XModuleDescriptor(Plugin): 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) + display_name=self.display_name, + format=self.format, + graded=self.graded) 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 index 34881a4d61..d62957c3d3 100644 --- a/common/lib/xmodule/xml_module.py +++ b/common/lib/xmodule/xml_module.py @@ -37,5 +37,7 @@ class XmlDescriptor(XModuleDescriptor): course, xml_object.tag, xml_object.get('slug')], - display_name=xml_object.get('name') + display_name=xml_object.get('name'), + format=xml_object.get('format'), + graded=xml_object.get('graded') == 'true', ) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 6f6ff71d35..b5fcae86e5 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -92,6 +92,9 @@ def grade_sheet(student, course, student_module_cache): for module in yield_descendents(s): (correct, total) = get_score(student, module, student_module_cache) + if correct is None and total is None: + continue + if settings.GENERATE_PROFILE_SCORES: if total > 1: correct = random.randrange(max(total - 2, 1), total + 1) @@ -102,14 +105,13 @@ def grade_sheet(student, course, student_module_cache): #We simply cannot grade a problem that is 12/0, because we might need it as a percentage graded = False - if correct is not None and total is not None: - scores.append(Score(correct, total, graded, module.display_name)) + scores.append(Score(correct, total, graded, module.display_name)) section_total, graded_total = graders.aggregate_scores(scores, s.display_name) #Add the graded total to totaled_scores format = getattr(s, 'format', "") subtitle = getattr(s, 'subtitle', format) - if format and graded_total[1] > 0: + if format and graded_total.possible > 0: format_scores = totaled_scores.get(format, []) format_scores.append(graded_total) totaled_scores[format] = format_scores From 506c281bccb5db15e607d2f85ca06b1a58dfa9c5 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 13:55:50 -0400 Subject: [PATCH 15/40] Make gradesheet work again --- lms/djangoapps/courseware/views.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 56237f605a..e1b77c8fbe 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -39,12 +39,21 @@ def gradebook(request): coursename = multicourse_settings.get_coursename_from_request(request) student_objects = User.objects.all()[:100] - student_info = [{'username': s.username, - 'id': s.id, - 'email': s.email, - 'grade_info': grades.grade_sheet(s, coursename), - 'realname': UserProfile.objects.get(user = s).name - } for s in student_objects] + student_info = [] + + coursename = multicourse_settings.get_coursename_from_request(request) + course_location = multicourse_settings.get_course_location(coursename) + + for student in student_objects: + student_module_cache = StudentModuleCache(student, keystore().get_item(course_location)) + course, _, _, _ = get_module(request.user, request, course_location, student_module_cache) + student_info.append({ + 'username': student.username, + 'id': student.id, + 'email': student.email, + 'grade_info': grades.grade_sheet(student, course, student_module_cache), + 'realname': UserProfile.objects.get(user = student).name + }) return render_to_response('gradebook.html', {'students': student_info}) From 27b75ca76ec1c22966563118249a0be9f389e5a7 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 13:58:50 -0400 Subject: [PATCH 16/40] Use display_name in sequence title bar --- common/lib/xmodule/seq_module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index dfa46dd72b..7ef497b837 100644 --- a/common/lib/xmodule/seq_module.py +++ b/common/lib/xmodule/seq_module.py @@ -52,9 +52,9 @@ class SequenceModule(XModule): contents.append({ 'content': child.get_html(), 'title': "\n".join( - grand_child.name.strip() + grand_child.display_name.strip() for grand_child in child.get_children() - if grand_child.name is not None + if grand_child.display_name is not None ), 'progress_status': Progress.to_js_status_str(progress), 'progress_detail': Progress.to_js_detail_str(progress), From c3a432f217917de0261d690c289a4d578a292fe3 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 14:01:23 -0400 Subject: [PATCH 17/40] Make problemsets display as verticals rather than sequences --- common/lib/xmodule/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 38509f182a..93eddc5c7c 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -21,7 +21,7 @@ setup( "html = xmodule.html_module:HtmlDescriptor", "image = xmodule.translation_module:TranslateCustomTagDescriptor", "problem = xmodule.capa_module:CapaDescriptor", - "problemset = xmodule.seq_module:SequenceDescriptor", + "problemset = xmodule.vertical_module:VerticalDescriptor", "section = xmodule.translation_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", "slides = xmodule.translation_module:TranslateCustomTagDescriptor", From dcd74e6dd072009c8289fe2a7ac9152de6f6def3 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Jun 2012 16:27:46 -0400 Subject: [PATCH 18/40] Make abtests work, using the new abtest xml format --- common/lib/xmodule/abtest_module.py | 90 +++++++++++++++++++---------- common/lib/xmodule/exceptions.py | 2 + common/lib/xmodule/setup.py | 1 + common/lib/xmodule/x_module.py | 1 + 4 files changed, 62 insertions(+), 32 deletions(-) create mode 100644 common/lib/xmodule/exceptions.py diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py index e14117eb08..3bd268184a 100644 --- a/common/lib/xmodule/abtest_module.py +++ b/common/lib/xmodule/abtest_module.py @@ -2,11 +2,10 @@ import json import random from lxml import etree -from x_module import XModule, XModuleDescriptor - - -class ModuleDescriptor(XModuleDescriptor): - pass +from xmodule.x_module import XModule +from xmodule.raw_module import RawDescriptor +from xmodule.xml_module import XmlDescriptor +from xmodule.exceptions import InvalidDefinitionError def group_from_value(groups, v): @@ -25,7 +24,7 @@ def group_from_value(groups, v): return g -class Module(XModule): +class ABTestModule(XModule): """ Implements an A/B test with an aribtrary number of competing groups @@ -37,20 +36,14 @@ class Module(XModule): """ - 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) + def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) - target_groups = self.xmltree.findall('group') + target_groups = self.definition['data'].keys() 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)], + self.definition['data']['group_portions'], random.uniform(0, 1) ) else: @@ -69,24 +62,57 @@ class Module(XModule): self.group = shared_state['group'] def get_shared_state(self): + print self.group 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 + def displayable_items(self): + return [self.system.get_module(child) + for child + in self.definition['data']['group_content'][self.group]] - if group is None: - return [] - return list(group) - def get_children(self): - return [self.module_from_xml(child) for child in self._xml_children()] +class ABTestDescriptor(RawDescriptor, XmlDescriptor): + module_class = ABTestModule - def get_html(self): - return '\n'.join(child.get_html() for child in self.get_children()) + def __init__(self, system, definition=None, **kwargs): + kwargs['shared_state_key'] = definition['data']['experiment'] + RawDescriptor.__init__(self, system, definition, **kwargs) + + @classmethod + def definition_from_xml(cls, xml_object, system): + experiment = xml_object.get('experiment') + + if experiment is None: + raise InvalidDefinitionError("ABTests must specify an experiment. Not found in:\n{xml}".format(xml=etree.tostring(xml_object, pretty_print=True))) + + definition = { + 'data': { + 'experiment': experiment, + 'group_portions': [], + 'group_content': {None: []}, + }, + 'children': []} + for group in xml_object: + if group.tag == 'default': + name = None + else: + name = group.get('name') + definition['data']['group_portions'].append( + (name, float(group.get('portion', 0))) + ) + + child_content_urls = [ + system.process_xml(etree.tostring(child)).url + for child in group + ] + + definition['data']['group_content'][name] = child_content_urls + definition['children'].extend(child_content_urls) + + default_portion = 1 - sum(portion for (name, portion) in definition['data']['group_portions']) + if default_portion < 0: + raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1") + + definition['data']['group_portions'].append((None, default_portion)) + + return definition diff --git a/common/lib/xmodule/exceptions.py b/common/lib/xmodule/exceptions.py new file mode 100644 index 0000000000..9a9258d600 --- /dev/null +++ b/common/lib/xmodule/exceptions.py @@ -0,0 +1,2 @@ +class InvalidDefinitionError(Exception): + pass diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 93eddc5c7c..e45e6654c2 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -13,6 +13,7 @@ setup( # for a description of entry_points entry_points={ 'xmodule.v1': [ + "abtest = xmodule.abtest_module:ABTestDescriptor", "book = xmodule.translation_module:TranslateCustomTagDescriptor", "chapter = xmodule.seq_module:SequenceDescriptor", "course = xmodule.seq_module:SequenceDescriptor", diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 8ee3df38ff..d8559c9bb7 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -295,6 +295,7 @@ class XModuleDescriptor(Plugin): self.display_name = kwargs.get('display_name') self.format = kwargs.get('format') self.graded = kwargs.get('graded', False) + self.shared_state_key = kwargs.get('shared_state_key') # 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 From c837cf797da1f96ed2a86a84540bec494d47186e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 05:57:38 -0400 Subject: [PATCH 19/40] Remove some unused code from content_parser --- lms/djangoapps/courseware/content_parser.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/lms/djangoapps/courseware/content_parser.py b/lms/djangoapps/courseware/content_parser.py index 70e5eeeeb6..624608aaa9 100644 --- a/lms/djangoapps/courseware/content_parser.py +++ b/lms/djangoapps/courseware/content_parser.py @@ -19,12 +19,10 @@ 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. @@ -35,22 +33,11 @@ course XML file and the rest of the system. # util.memcache.fasthash (which does not depend on memcache at all) # -class ContentException(Exception): - pass - log = logging.getLogger("mitx.courseware") def format_url_params(params): return [ urllib.quote(string.replace(' ','_')) for string in params ] -def xpath_remove(tree, path): - ''' Remove all items matching path from lxml tree. Works in - place.''' - items = tree.xpath(path) - for item in items: - item.getparent().remove(item) - return tree - def id_tag(course): ''' Tag all course elements with unique IDs ''' default_ids = xmodule.get_default_ids() From d7ee03874dfe1fc5170dc311549f4c6b85d36f53 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 07:20:12 -0400 Subject: [PATCH 20/40] Make staff histograms work again --- lms/djangoapps/courseware/module_render.py | 32 +++++++++++++++++----- lms/templates/staff_problem_info.html | 5 ++-- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 4c82eba974..c5d87a52b0 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -1,17 +1,14 @@ import json import logging -from lxml import etree - +from django.conf import settings from django.http import Http404 from django.http import HttpResponse - -from django.conf import settings -from mitxmako.shortcuts import render_to_string - -from models import StudentModule, StudentModuleCache +from lxml import etree from keystore.django import keystore +from mitxmako.shortcuts import render_to_string +from models import StudentModule, StudentModuleCache log = logging.getLogger("mitx.courseware") @@ -247,6 +244,9 @@ def get_module(user, request, location, student_module_cache, position=None): module = descriptor.xmodule_constructor(system)(instance_state, shared_state) + if settings.MITX_FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF') and user.is_staff: + module = add_histogram(module) + # If StudentModule for this instance wasn't already in the database, # and this isn't a guest user, create it. if user.is_authenticated(): @@ -272,6 +272,24 @@ def get_module(user, request, location, student_module_cache, position=None): return (module, instance_module, shared_module, descriptor.type) +def add_histogram(module): + original_get_html = module.get_html + def get_html(): + module_id = module.id + print "Rendering Histogram for ", module_id + histogram = grade_histogram(module_id) + print histogram + render_histogram = len(histogram) > 0 + staff_context = {'definition': json.dumps(module.definition, indent=4), + 'element_id': module.location.html_id(), + 'histogram': json.dumps(histogram), + 'render_histogram': render_histogram, + 'module_content': original_get_html()} + return render_to_string("staff_problem_info.html", staff_context) + module.get_html = get_html + return module + + def render_x_module(user, module, student_module_cache, position=None): ''' Generic module for extensions. This renders to HTML. diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index 24450c797a..b5e07f8af4 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -1,6 +1,7 @@ +${module_content}
-${xml | h} +${definition | h}
%if render_histogram: -
+
%endif From 3fdae56a27710b36feb6c7f8176bd29a0364042b Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 07:29:49 -0400 Subject: [PATCH 21/40] Remove dead code --- lms/djangoapps/courseware/module_render.py | 51 +--------------------- lms/djangoapps/courseware/views.py | 43 +----------------- lms/urls.py | 1 - 3 files changed, 2 insertions(+), 93 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index c5d87a52b0..b331a270d4 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -25,7 +25,7 @@ class I4xSystem(object): Note that these functions can be closures over e.g. a django request and user, or other environment-specific info. ''' - def __init__(self, ajax_url, track_function, render_function, + def __init__(self, ajax_url, track_function, get_module, render_template, user=None, filestore=None): ''' @@ -38,10 +38,6 @@ class I4xSystem(object): files. Update or remove. 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 - module to html. Dictionary will contain keys 'content' - and 'type'. render_template - a function that takes (template_file, context), and returns rendered html. user - The user to base the seed off of for this request @@ -52,7 +48,6 @@ class I4xSystem(object): self.track_function = track_function self.filestore = filestore self.get_module = get_module - self.render_function = render_function self.render_template = render_template self.exception404 = Http404 self.DEBUG = settings.DEBUG @@ -230,8 +225,6 @@ def get_module(user, request, location, student_module_cache, position=None): return module system = I4xSystem(track_function=make_track_function(request), - render_function=lambda xml: render_x_module( - user, xml, student_module_cache, position), render_template=render_to_string, ajax_url=ajax_url, # TODO (cpennington): Figure out how to share info between systems @@ -290,48 +283,6 @@ def add_histogram(module): return module -def render_x_module(user, module, student_module_cache, position=None): - ''' Generic module for extensions. This renders to HTML. - - modules include sequential, vertical, problem, video, html - - Note that modules can recurse. problems, video, html, can be inside sequential or vertical. - - Arguments: - - - user : current django User - - module : lxml etree of xml subtree for the current 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 position within module - - Returns: - - - 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: - return {"content": ""} - - (instance, _, _, module_type) = get_module( - user, module_xml, student_module_cache, position) - - content = instance.get_html() - - # 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), - 'module_id': module_id, - 'histogram': json.dumps(histogram), - 'render_histogram': render_histogram} - content += render_to_string("staff_problem_info.html", staff_context) - - context = {'content': content, 'type': module_type} - return context - - def modx_dispatch(request, dispatch=None, id=None): ''' Generic view for extensions. This is where AJAX calls go. diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index e1b77c8fbe..444e830072 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -16,7 +16,7 @@ from django.views.decorators.cache import cache_control from lxml import etree -from module_render import render_x_module, toc_for_course, get_module, get_section +from module_render import toc_for_course, get_module, get_section from models import StudentModuleCache from student.models import UserProfile from multicourse import multicourse_settings @@ -115,47 +115,6 @@ def render_accordion(request, course, chapter, section): return render_to_string('accordion.html', context) -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -def render_section(request, section): - ''' TODO: Consolidate with index - ''' - user = request.user - if not settings.COURSEWARE_ENABLED: - return redirect('/') - - coursename = multicourse_settings.get_coursename_from_request(request) - - try: - dom = content_parser.section_file(user, section, coursename) - except: - log.exception("Unable to parse courseware xml") - return render_to_response('courseware-error.html', {}) - - context = { - 'csrf': csrf(request)['csrf_token'], - 'accordion': render_accordion(request, get_course(request), '', '') - } - - student_module_cache = StudentModuleCache(request.user, dom) - - try: - module = render_x_module(user, dom, student_module_cache) - except: - log.exception("Unable to load module") - context.update({ - 'init': '', - 'content': render_to_string("module-error.html", {}), - }) - return render_to_response('courseware.html', context) - - context.update({ - 'content': module['content'], - }) - - result = render_to_response('courseware.html', context) - return result - - def get_course(request, course): ''' Figure out what the correct course is. diff --git a/lms/urls.py b/lms/urls.py index e43c949643..d8d4356e5c 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -56,7 +56,6 @@ if settings.COURSEWARE_ENABLED: url(r'^courseware/(?P[^/]*)/(?P[^/]*)/$', 'courseware.views.index', name="courseware_chapter"), url(r'^courseware/(?P[^/]*)/$', 'courseware.views.index', name="courseware_course"), url(r'^jumpto/(?P[^/]+)/$', 'courseware.views.jump_to'), - url(r'^section/(?P
[^/]*)/$', 'courseware.views.render_section'), url(r'^modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'), url(r'^profile$', 'courseware.views.profile'), url(r'^profile/(?P[^/]*)/$', 'courseware.views.profile'), From 3a26b9802710ecea7dad7dc675fbe99535e877ab Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 08:44:03 -0400 Subject: [PATCH 22/40] Remove errant print line left over from debugging --- lms/djangoapps/courseware/module_render.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index b331a270d4..c2d20f1b67 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -267,9 +267,9 @@ def get_module(user, request, location, student_module_cache, position=None): def add_histogram(module): original_get_html = module.get_html + def get_html(): module_id = module.id - print "Rendering Histogram for ", module_id histogram = grade_histogram(module_id) print histogram render_histogram = len(histogram) > 0 @@ -279,6 +279,7 @@ def add_histogram(module): 'render_histogram': render_histogram, 'module_content': original_get_html()} return render_to_string("staff_problem_info.html", staff_context) + module.get_html = get_html return module From c7f95695c53e107bd83fd38c9424b7cb8bc7653a Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 08:44:16 -0400 Subject: [PATCH 23/40] Fix check_course command --- lms/djangoapps/courseware/content_parser.py | 44 ------------ .../management/commands/check_course.py | 71 ++++++++----------- 2 files changed, 31 insertions(+), 84 deletions(-) diff --git a/lms/djangoapps/courseware/content_parser.py b/lms/djangoapps/courseware/content_parser.py index 624608aaa9..d1d9cf3980 100644 --- a/lms/djangoapps/courseware/content_parser.py +++ b/lms/djangoapps/courseware/content_parser.py @@ -193,50 +193,6 @@ def replace_custom_tags(tree): replace_custom_tags_dir(tree, settings.DATA_DIR+'/custom_tags') -def course_file(user, coursename=None): - ''' Given a user, return an xml tree object for the course file. - - Handles getting the right file, and processing it depending on the - groups the user is in. Does caching of the xml strings. - ''' - - if user.is_authenticated(): - # use user.profile_cache.courseware? - filename = UserProfile.objects.get(user=user).courseware - else: - filename = 'guest_course.xml' - - # if a specific course is specified, then use multicourse to get - # the right path to the course XML directory - if coursename and settings.ENABLE_MULTICOURSE: - xp = multicourse_settings.get_course_xmlpath(coursename) - filename = xp + filename # prefix the filename with the path - - groups = user_groups(user) - options = get_options(user) - - # Try the cache... - cache_key = "{0}_processed?dev_content:{1}&groups:{2}".format( - filename, - options['dev_content'], - sorted(groups)) - - if "dev" in settings.DEFAULT_GROUPS: - tree_string = None - else: - tree_string = cache.get(cache_key) - - if tree_string: - tree = etree.XML(tree_string) - else: - tree = parse_course_file(filename, options, namespace='course') - # Cache it - tree_string = etree.tostring(tree) - cache.set(cache_key, tree_string, 60) - - return tree - - def sections_dir(coursename=None): ''' Get directory where sections information is stored. ''' diff --git a/lms/djangoapps/courseware/management/commands/check_course.py b/lms/djangoapps/courseware/management/commands/check_course.py index 8af0c5d4be..afc7e47857 100644 --- a/lms/djangoapps/courseware/management/commands/check_course.py +++ b/lms/djangoapps/courseware/management/commands/check_course.py @@ -6,50 +6,36 @@ from django.core.management.base import BaseCommand from django.conf import settings from django.contrib.auth.models import User -from courseware.content_parser import course_file -import courseware.module_render import xmodule import mitxmako.middleware as middleware middleware.MakoMiddleware() +from keystore.django import keystore +from courseware.models import StudentModuleCache +from courseware.module_render import get_module -def check_names(user, course): - ''' - Complain if any problems have non alphanumeric names. - TODO (vshnayder): there are some in 6.002x that don't. Is that actually a problem? - ''' - all_ok = True - print "Confirming all problems have alphanumeric names" - for problem in course.xpath('//problem'): - filename = problem.get('filename') - if not filename.isalnum(): - print "==============> Invalid (non-alphanumeric) filename", filename - all_ok = False - return all_ok -def check_rendering(user, course): +def check_rendering(module): '''Check that all modules render''' all_ok = True print "Confirming all modules render. Nothing should print during this step. " - for module in course.xpath('//problem|//html|//video|//vertical|//sequential|/tab'): - module_class = xmodule.modx_modules[module.tag] - # TODO: Abstract this out in render_module.py - try: - module_class(etree.tostring(module), - module.get('id'), - ajax_url='', - state=None, - track_function = lambda x,y,z:None, - render_function = lambda x: {'content':'','type':'video'}) + + def _check_module(module): + try: + module.get_html() except Exception as ex: - print "==============> Error in ", etree.tostring(module) + print "==============> Error in ", module.id print "" print ex all_ok = False + for child in module.get_children(): + _check_module(child) + _check_module(module) print "Module render check finished" return all_ok -def check_sections(user, course): + +def check_sections(course): all_ok = True sections_dir = settings.DATA_DIR + "/sections" print "Checking that all sections exist and parse properly" @@ -69,11 +55,13 @@ def check_sections(user, course): all_ok = False print "checked all sections" else: - print "Skipping check of include files -- no section includes dir ("+sections_dir+")" + print "Skipping check of include files -- no section includes dir (" + sections_dir + ")" return all_ok + class Command(BaseCommand): help = "Does basic validity tests on course.xml." + def handle(self, *args, **options): all_ok = True @@ -86,22 +74,25 @@ class Command(BaseCommand): sample_user = User.objects.all()[0] - print "Attempting to load courseware" - course = course_file(sample_user) - to_run = [check_names, - # TODO (vshnayder) : make check_rendering work (use module_render.py), - # turn it on - # check_rendering, - check_sections, - ] + # TODO (cpennington): Get coursename in a legitimate way + course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012' + student_module_cache = StudentModuleCache(sample_user, keystore().get_item(course_location)) + (course, _, _, _) = get_module(sample_user, None, course_location, student_module_cache) + + to_run = [ + #TODO (vshnayder) : make check_rendering work (use module_render.py), + # turn it on + check_rendering, + check_sections, + ] for check in to_run: - all_ok = check(sample_user, course) and all_ok + all_ok = check(course) and all_ok # TODO: print "Checking course properly annotated with preprocess.py" - + if all_ok: print 'Courseware passes all checks!' - else: + else: print "Courseware fails some checks" From 2a9eba38862dee453ec65183d4fb53af61e986e4 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 09:12:17 -0400 Subject: [PATCH 24/40] Removing content_parser --- lms/djangoapps/courseware/content_parser.py | 205 -------------------- lms/djangoapps/courseware/views.py | 48 +++-- 2 files changed, 36 insertions(+), 217 deletions(-) delete mode 100644 lms/djangoapps/courseware/content_parser.py diff --git a/lms/djangoapps/courseware/content_parser.py b/lms/djangoapps/courseware/content_parser.py deleted file mode 100644 index d1d9cf3980..0000000000 --- a/lms/djangoapps/courseware/content_parser.py +++ /dev/null @@ -1,205 +0,0 @@ -''' -courseware/content_parser.py - -This file interfaces between all courseware modules and the top-level course.xml file for a course. - -Does some caching (to be explained). - -''' - -import logging -import os -import sys -import urllib - -from lxml import etree -from util.memcache import fasthash - -from django.conf import settings - -from student.models import UserProfile -from student.models import UserTestGroup -from mitxmako.shortcuts import render_to_string -from util.cache import cache -from multicourse import multicourse_settings -import xmodule - -''' This file will eventually form an abstraction layer between the -course XML file and the rest of the system. -''' - -# ==== This section has no direct dependencies on django ==================================== -# NOTE: it does still have some indirect dependencies: -# util.memcache.fasthash (which does not depend on memcache at all) -# - -log = logging.getLogger("mitx.courseware") - -def format_url_params(params): - return [ urllib.quote(string.replace(' ','_')) for string in params ] - -def id_tag(course): - ''' Tag all course elements with unique IDs ''' - default_ids = xmodule.get_default_ids() - - # Tag elements with unique IDs - elements = course.xpath("|".join('//' + c for c in default_ids)) - for elem in elements: - if elem.get('id'): - pass - elif elem.get(default_ids[elem.tag]): - new_id = elem.get(default_ids[elem.tag]) - # Convert to alphanumeric - new_id = "".join(a for a in new_id if a.isalnum()) - - # Without this, a conflict may occur between an html or youtube id - new_id = default_ids[elem.tag] + new_id - elem.set('id', new_id) - else: - elem.set('id', "id" + fasthash(etree.tostring(elem))) - -def propogate_downward_tag(element, attribute_name, parent_attribute = None): - ''' This call is to pass down an attribute to all children. If an element - has this attribute, it will be "inherited" by all of its children. If a - child (A) already has that attribute, A will keep the same attribute and - all of A's children will inherit A's attribute. This is a recursive call.''' - - if (parent_attribute is None): - #This is the entry call. Select all elements with this attribute - all_attributed_elements = element.xpath("//*[@" + attribute_name +"]") - for attributed_element in all_attributed_elements: - attribute_value = attributed_element.get(attribute_name) - for child_element in attributed_element: - propogate_downward_tag(child_element, attribute_name, attribute_value) - else: - '''The hack below is because we would get _ContentOnlyELements from the - iterator that can't have attributes set. We can't find API for it. If we - ever have an element which subclasses BaseElement, we will not tag it''' - if not element.get(attribute_name) and type(element) == etree._Element: - element.set(attribute_name, parent_attribute) - - for child_element in element: - propogate_downward_tag(child_element, attribute_name, parent_attribute) - else: - #This element would have already been found by Xpath, so we return - #for now and trust that this element will get its turn to propogate - #to its children later. - return - - -def course_xml_process(tree): - ''' Do basic pre-processing of an XML tree. Assign IDs to all - 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") - propogate_downward_tag(tree, "graded") - propogate_downward_tag(tree, "graceperiod") - propogate_downward_tag(tree, "showanswer") - propogate_downward_tag(tree, "rerandomize") - return tree - - -def process_includes_dir(tree, dir): - """ - Process tree to replace all 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): - ''' - Process tree to replace all custom tags defined in dir. - ''' - tags = os.listdir(dir) - for tag in tags: - for element in tree.iter(tag): - element.tag = 'customtag' - impl = etree.SubElement(element, 'impl') - impl.text = tag - -def parse_course_file(filename, options, namespace): - ''' - Parse a course file with the given options, and return the resulting - xml tree object. - - Options should be a dictionary including keys - 'dev_content': bool, - 'groups' : [list, of, user, groups] - - namespace is used to in searching for the file. Could be e.g. 'course', - 'sections'. - ''' - xml = etree.XML(render_to_string(filename, options, namespace=namespace)) - return course_xml_process(xml) - - -# ==== All Django-specific code below ============================================= - -def user_groups(user): - if not user.is_authenticated(): - return [] - - # TODO: Rewrite in Django - key = 'user_group_names_{user.id}'.format(user=user) - cache_expiration = 60 * 60 # one hour - - # Kill caching on dev machines -- we switch groups a lot - group_names = cache.get(key) - - if group_names is None: - group_names = [u.name for u in UserTestGroup.objects.filter(users=user)] - cache.set(key, group_names, cache_expiration) - - return group_names - - -def get_options(user): - return {'dev_content': settings.DEV_CONTENT, - 'groups': user_groups(user)} - - -def process_includes(tree): - '''Replace 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') - - -def sections_dir(coursename=None): - ''' Get directory where sections information is stored. - ''' - # if a specific course is specified, then use multicourse to get the - # right path to the course XML directory - xp = '' - if coursename and settings.ENABLE_MULTICOURSE: - xp = multicourse_settings.get_course_xmlpath(coursename) - - return settings.DATA_DIR + xp + '/sections/' diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 444e830072..e1e1c16632 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1,8 +1,6 @@ import logging import urllib -from fs.osfs import OSFS - from django.conf import settings from django.core.context_processors import csrf from django.contrib.auth.models import User @@ -22,7 +20,9 @@ from student.models import UserProfile from multicourse import multicourse_settings from keystore.django import keystore -from courseware import grades, content_parser +from util.cache import cache +from student.models import UserTestGroup +from courseware import grades log = logging.getLogger("mitx.courseware") @@ -31,16 +31,39 @@ etree.set_default_parser(etree.XMLParser(dtd_validation=False, load_dtd=False, template_imports = {'urllib': urllib} + +def user_groups(user): + if not user.is_authenticated(): + return [] + + # TODO: Rewrite in Django + key = 'user_group_names_{user.id}'.format(user=user) + cache_expiration = 60 * 60 # one hour + + # Kill caching on dev machines -- we switch groups a lot + group_names = cache.get(key) + + if group_names is None: + group_names = [u.name for u in UserTestGroup.objects.filter(users=user)] + cache.set(key, group_names, cache_expiration) + + return group_names + + +def format_url_params(params): + return [urllib.quote(string.replace(' ', '_')) for string in params] + + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def gradebook(request): - if 'course_admin' not in content_parser.user_groups(request.user): + if 'course_admin' not in user_groups(request.user): raise Http404 coursename = multicourse_settings.get_coursename_from_request(request) student_objects = User.objects.all()[:100] student_info = [] - + coursename = multicourse_settings.get_coursename_from_request(request) course_location = multicourse_settings.get_course_location(coursename) @@ -52,7 +75,7 @@ def gradebook(request): 'id': student.id, 'email': student.email, 'grade_info': grades.grade_sheet(student, course, student_module_cache), - 'realname': UserProfile.objects.get(user = student).name + 'realname': UserProfile.objects.get(user=student).name }) return render_to_response('gradebook.html', {'students': student_info}) @@ -67,7 +90,7 @@ def profile(request, student_id=None): if student_id is None: student = request.user else: - if 'course_admin' not in content_parser.user_groups(request.user): + if 'course_admin' not in user_groups(request.user): raise Http404 student = User.objects.get(id=int(student_id)) @@ -83,7 +106,7 @@ def profile(request, student_id=None): 'location': user_info.location, 'language': user_info.language, 'email': student.email, - 'format_url_params': content_parser.format_url_params, + 'format_url_params': format_url_params, 'csrf': csrf(request)['csrf_token'] } context.update(grades.grade_sheet(student, course, student_module_cache)) @@ -110,7 +133,7 @@ def render_accordion(request, course, chapter, section): context = dict([('active_chapter', active_chapter), ('toc', toc), ('course_name', course), - ('format_url_params', content_parser.format_url_params), + ('format_url_params', format_url_params), ('csrf', csrf(request)['csrf_token'])] + template_imports.items()) return render_to_string('accordion.html', context) @@ -215,7 +238,8 @@ def jump_to(request, probname=None): # look for problem of given name pxml = xml.xpath('//problem[@filename="%s"]' % probname) - if pxml: pxml = pxml[0] + if pxml: + pxml = pxml[0] # get the parent element parent = pxml.getparent() @@ -224,7 +248,7 @@ def jump_to(request, probname=None): chapter = None section = None branch = parent - for k in range(4): # max depth of recursion + for k in range(4): # max depth of recursion if branch.tag == 'section': section = branch.get('name') if branch.tag == 'chapter': @@ -233,7 +257,7 @@ def jump_to(request, probname=None): position = None if parent.tag == 'sequential': - position = parent.index(pxml) + 1 # position in sequence + position = parent.index(pxml) + 1 # position in sequence return index(request, course=coursename, chapter=chapter, From f25478b3d45bdc03f8d19e92644131d52caab624 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 11:56:38 -0400 Subject: [PATCH 25/40] Consolidate access to metadata, and allow some of it to be inherited between modules --- common/lib/xmodule/capa_module.py | 14 ++--- common/lib/xmodule/seq_module.py | 4 +- common/lib/xmodule/x_module.py | 67 +++++++++++++++------- common/lib/xmodule/xml_module.py | 16 +++++- lms/djangoapps/courseware/grades.py | 18 +++--- lms/djangoapps/courseware/module_render.py | 20 +++---- lms/templates/profile.html | 2 +- lms/templates/staff_problem_info.html | 3 +- 8 files changed, 89 insertions(+), 55 deletions(-) diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index b6bfc91e80..9e82fbe8d4 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -125,7 +125,7 @@ class CapaModule(XModule): # User submitted a problem, and hasn't reset. We don't want # more submissions. - if self.lcp.done and self.rerandomize == "always": + if self.lcp.done and self.metadata['rerandomize'] == "always": check_button = False save_button = False @@ -184,15 +184,15 @@ class CapaModule(XModule): # TODO: Should be converted to: self.explanation=only_one(dom2.xpath('/problem/@explain'), default="closed") self.explain_available = only_one(dom2.xpath('/problem/@explain_available')) - display_due_date_string = only_one(dom2.xpath('/problem/@due')) - if len(display_due_date_string) > 0: + display_due_date_string = self.metadata.get('due', None) + if display_due_date_string is not None: self.display_due_date = dateutil.parser.parse(display_due_date_string) #log.debug("Parsed " + display_due_date_string + " to " + str(self.display_due_date)) else: self.display_due_date = None - grace_period_string = only_one(dom2.xpath('/problem/@graceperiod')) - if len(grace_period_string) > 0 and self.display_due_date: + grace_period_string = self.metadata.get('graceperiod', None) + if grace_period_string is not None and self.display_due_date: self.grace_period = parse_timedelta(grace_period_string) self.close_date = self.display_due_date + self.grace_period #log.debug("Then parsed " + grace_period_string + " to closing date" + str(self.close_date)) @@ -206,12 +206,12 @@ class CapaModule(XModule): else: self.max_attempts = None - self.show_answer = only_one(dom2.xpath('/problem/@showanswer')) + self.show_answer = self.metadata.get('showanwser', 'closed') if self.show_answer == "": self.show_answer = "closed" - self.rerandomize = only_one(dom2.xpath('/problem/@rerandomize')) + self.rerandomize = self.metadata.get('rerandomize', 'always') if self.rerandomize == "" or self.rerandomize == "always" or self.rerandomize == "true": self.rerandomize = "always" elif self.rerandomize == "false" or self.rerandomize == "per_student": diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index 7ef497b837..ffcaed2599 100644 --- a/common/lib/xmodule/seq_module.py +++ b/common/lib/xmodule/seq_module.py @@ -52,9 +52,9 @@ class SequenceModule(XModule): contents.append({ 'content': child.get_html(), 'title': "\n".join( - grand_child.display_name.strip() + grand_child.metadata['display_name'].strip() for grand_child in child.get_children() - if grand_child.display_name is not None + if 'metadata' in grand_child.metadata ), 'progress_status': Progress.to_js_status_str(progress), 'progress_detail': Progress.to_js_detail_str(progress), diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index d8559c9bb7..c027d1d774 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -82,11 +82,9 @@ class XModule(object): self.shared_state = shared_state self.id = self.location.url() self.name = self.location.name - self.display_name = kwargs.get('display_name', '') self.type = self.location.category + self.metadata = kwargs.get('metadata', {}) self._loaded_children = None - self.graded = kwargs.get('graded', False) - self.format = kwargs.get('format') def get_name(self): name = self.__xmltree.get('name') @@ -188,6 +186,9 @@ class XModuleDescriptor(Plugin): js = {} js_module = None + # A list of metadata that this module can inherit from its parent module + inheritable_metadata = ('graded', 'due', 'graceperiod', 'showanswer', 'rerandomize') + @staticmethod def load_from_json(json_data, system, default_class=None): """ @@ -215,7 +216,11 @@ class XModuleDescriptor(Plugin): return cls(system=system, **json_data) @staticmethod - def load_from_xml(xml_data, system, org=None, course=None, default_class=None): + def load_from_xml(xml_data, + system, + org=None, + course=None, + default_class=None): """ This method instantiates the correct subclass of XModuleDescriptor based on the contents of xml_data. @@ -282,32 +287,48 @@ class XModuleDescriptor(Plugin): Current arguments passed in kwargs: location: A keystore.Location object indicating the name and ownership of this problem - goals: A list of strings of learning goals associated with this module - display_name: The name to use for displaying this module to the user - format: The format of this module ('Homework', 'Lab', etc) - graded (bool): Whether this module is should be graded or not + shared_state_key: The key to use for sharing StudentModules with other + modules of this type + metadata: A dictionary containing the following optional keys: + goals: A list of strings of learning goals associated with this module + display_name: The name to use for displaying this module to the user + format: The format of this module ('Homework', 'Lab', etc) + graded (bool): Whether this module is should be graded or not + due (string): The due date for this module + graceperiod (string): The amount of grace period to allow when enforcing the due date + showanswer (string): When to show answers for this module + rerandomize (string): When to generate a newly randomized instance of the module data """ self.system = system self.definition = definition if definition is not None else {} 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') - self.format = kwargs.get('format') - self.graded = kwargs.get('graded', False) + self.metadata = kwargs.get('metadata', {}) self.shared_state_key = kwargs.get('shared_state_key') - # 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 - # to find the best teaching method - self.goals = kwargs.get('goals', []) - self._child_instances = None + def inherit_metadata(self, metadata): + """ + Updates this module with metadata inherited from a containing module. + Only metadata specified in self.inheritable_metadata will + be inherited + """ + # Set all inheritable metadata from kwargs that are + # in self.inheritable_metadata and aren't already set in metadata + for attr in self.inheritable_metadata: + if attr not in self.metadata and attr in metadata: + self.metadata[attr] = metadata[attr] + def get_children(self): """Returns a list of XModuleDescriptor instances for the children of this module""" if self._child_instances is None: - self._child_instances = [self.system.load_item(child) for child in self.definition.get('children', [])] + self._child_instances = [] + for child_loc in self.definition.get('children', []): + child = self.system.load_item(child_loc) + child.inherit_metadata(self.metadata) + self._child_instances.append(child) return self._child_instances @@ -322,10 +343,14 @@ class XModuleDescriptor(Plugin): 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, - format=self.format, - graded=self.graded) + return partial( + self.module_class, + system, + self.url, + self.definition, + metadata=self.metadata + ) + 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 index d62957c3d3..b167e52e88 100644 --- a/common/lib/xmodule/xml_module.py +++ b/common/lib/xmodule/xml_module.py @@ -29,6 +29,18 @@ class XmlDescriptor(XModuleDescriptor): """ xml_object = etree.fromstring(xml_data) + metadata = {} + for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'): + from_xml = xml_object.get(attr) + if from_xml is not None: + metadata[attr] = from_xml + + if xml_object.get('graded') is not None: + metadata['graded'] = xml_object.get('graded') == 'true' + + if xml_object.get('name') is not None: + metadata['display_name'] = xml_object.get('name') + return cls( system, cls.definition_from_xml(xml_object, system), @@ -37,7 +49,5 @@ class XmlDescriptor(XModuleDescriptor): course, xml_object.tag, xml_object.get('slug')], - display_name=xml_object.get('name'), - format=xml_object.get('format'), - graded=xml_object.get('graded') == 'true', + metadata=metadata, ) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index b5fcae86e5..deab9d47d4 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -87,7 +87,7 @@ def grade_sheet(student, course, student_module_cache): for module in yield_descendents(child): yield module - graded = getattr(s, 'graded', False) + graded = s.metadata.get('graded', False) scores = [] for module in yield_descendents(s): (correct, total) = get_score(student, module, student_module_cache) @@ -105,29 +105,27 @@ def grade_sheet(student, course, student_module_cache): #We simply cannot grade a problem that is 12/0, because we might need it as a percentage graded = False - scores.append(Score(correct, total, graded, module.display_name)) + scores.append(Score(correct, total, graded, module.metadata.get('display_name'))) - section_total, graded_total = graders.aggregate_scores(scores, s.display_name) + section_total, graded_total = graders.aggregate_scores(scores, s.metadata.get('display_name')) #Add the graded total to totaled_scores - format = getattr(s, 'format', "") - subtitle = getattr(s, 'subtitle', format) + format = s.metadata.get('format', "") if format and graded_total.possible > 0: format_scores = totaled_scores.get(format, []) format_scores.append(graded_total) totaled_scores[format] = format_scores sections.append({ - 'section': s.display_name, + 'section': s.metadata.get('display_name'), 'scores': scores, 'section_total': section_total, 'format': format, - 'subtitle': subtitle, - 'due': getattr(s, "due", ""), + 'due': s.metadata.get("due", ""), 'graded': graded, }) - chapters.append({'course': course.display_name, - 'chapter': c.display_name, + chapters.append({'course': course.metadata.get('display_name'), + 'chapter': c.metadata.get('display_name'), 'sections': sections}) grader = course_settings.GRADER diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index c2d20f1b67..ee2ccee018 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -137,17 +137,17 @@ def toc_for_course(user, request, course_location, active_chapter, active_sectio sections = list() for section in chapter.get_display_items(): - active = (chapter.display_name == active_chapter and - section.display_name == active_section) + active = (chapter.metadata.get('display_name') == active_chapter and + section.metadata.get('display_name') == active_section) - sections.append({'name': section.display_name, - 'format': getattr(section, 'format', ''), - 'due': getattr(section, 'due', ''), + sections.append({'name': section.metadata.get('display_name'), + 'format': section.metadata.get('format', ''), + 'due': section.metadata.get('due', ''), 'active': active}) - chapters.append({'name': chapter.display_name, + chapters.append({'name': chapter.metadata.get('display_name'), 'sections': sections, - 'active': chapter.display_name == active_chapter}) + 'active': chapter.metadata.get('display_name') == active_chapter}) return chapters @@ -171,7 +171,7 @@ def get_section(course, chapter, section): chapter_module = None for _chapter in course_module.get_children(): - if _chapter.display_name == chapter: + if _chapter.metadata.get('display_name') == chapter: chapter_module = _chapter break @@ -180,7 +180,7 @@ def get_section(course, chapter, section): section_module = None for _section in chapter_module.get_children(): - if _section.display_name == section: + if _section.metadata.get('display_name') == section: section_module = _section break @@ -271,9 +271,9 @@ def add_histogram(module): def get_html(): module_id = module.id histogram = grade_histogram(module_id) - print histogram render_histogram = len(histogram) > 0 staff_context = {'definition': json.dumps(module.definition, indent=4), + 'metadata': json.dumps(module.metadata, indent=4), 'element_id': module.location.html_id(), 'histogram': json.dumps(histogram), 'render_histogram': render_histogram, diff --git a/lms/templates/profile.html b/lms/templates/profile.html index e732616d5a..1ba0940eff 100644 --- a/lms/templates/profile.html +++ b/lms/templates/profile.html @@ -156,7 +156,7 @@ $(function() {

${ section['section'] } ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}

- ${section['subtitle']} + ${section['format']} %if 'due' in section and section['due']!="": due ${section['due']} %endif diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index b5e07f8af4..2cc9de9df7 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -1,6 +1,7 @@ ${module_content}
-${definition | h} +definition = ${definition | h} +metadata = ${metadata | h}
%if render_histogram:
From 87d80835c056776e0836e4ccba3aefbbb066500d Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 12:02:41 -0400 Subject: [PATCH 26/40] Initialize StudentModule with the max_score --- lms/djangoapps/courseware/module_render.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index ee2ccee018..6cf4e43cc5 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -248,7 +248,8 @@ def get_module(user, request, location, student_module_cache, position=None): student=user, module_type=descriptor.type, module_state_key=module.id, - state=module.get_instance_state()) + state=module.get_instance_state(), + max_grade=module.max_score()) instance_module.save() # Add to cache. The caller and the system context have references # to it, so the change persists past the return From 99ae0c2041c5a2b4947f870ed8a4e691b9bfe3c2 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 29 Jun 2012 13:16:04 -0400 Subject: [PATCH 27/40] Import logging where needed --- common/lib/keystore/xml.py | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py index dcddb2718e..d0f777efa9 100644 --- a/common/lib/keystore/xml.py +++ b/common/lib/keystore/xml.py @@ -1,3 +1,4 @@ +import logging from fs.osfs import OSFS from importlib import import_module from lxml import etree From 19a958a3d4b2aa556d57b9343f6114a9ce9828be Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Sun, 1 Jul 2012 13:33:32 -0400 Subject: [PATCH 28/40] Renaming keystore to modulestore for clarity --- common/lib/keystore/xml.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py index d0f777efa9..c3327b8ff3 100644 --- a/common/lib/keystore/xml.py +++ b/common/lib/keystore/xml.py @@ -28,14 +28,17 @@ class XMLModuleStore(ModuleStore): with open(self.data_dir / "course.xml") as course_file: class ImportSystem(XMLParsingSystem): - def __init__(self, keystore): + def __init__(self, modulestore): + """ + modulestore: the XMLModuleStore to store the loaded modules in + """ self.unnamed_modules = 0 def process_xml(xml): try: xml_data = etree.fromstring(xml) except: - log.exception("Unable to parse xml:" + xml) + log.exception("Unable to parse xml: {xml}".format(xml=xml)) raise if xml_data.get('name'): xml_data.set('slug', Location.clean(xml_data.get('name'))) @@ -43,11 +46,11 @@ class XMLModuleStore(ModuleStore): 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 + module = XModuleDescriptor.load_from_xml(etree.tostring(xml_data), self, org, course, modulestore.default_class) + modulestore.modules[module.url] = module return module - XMLParsingSystem.__init__(self, keystore.get_item, OSFS(data_dir), process_xml) + XMLParsingSystem.__init__(self, modulestore.get_item, OSFS(data_dir), process_xml) ImportSystem(self).process_xml(course_file.read()) From e61946dfea10757afb0242d6a1f2b51354d1e32e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Sun, 1 Jul 2012 13:52:06 -0400 Subject: [PATCH 29/40] Fix titles in sequence module display --- common/lib/xmodule/seq_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index ffcaed2599..af8563c442 100644 --- a/common/lib/xmodule/seq_module.py +++ b/common/lib/xmodule/seq_module.py @@ -54,7 +54,7 @@ class SequenceModule(XModule): 'title': "\n".join( grand_child.metadata['display_name'].strip() for grand_child in child.get_children() - if 'metadata' in grand_child.metadata + if 'display_name' in grand_child.metadata ), 'progress_status': Progress.to_js_status_str(progress), 'progress_detail': Progress.to_js_detail_str(progress), From 6686179bb604ae6f8fb6dab3867555e46a27bdbd Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Sun, 1 Jul 2012 14:15:02 -0400 Subject: [PATCH 30/40] Moving __init__ functions in xmodule to top of class for consistancy --- common/lib/xmodule/capa_module.py | 192 +++++++++++++------------- common/lib/xmodule/seq_module.py | 31 +++-- common/lib/xmodule/template_module.py | 4 +- common/lib/xmodule/vertical_module.py | 9 +- common/lib/xmodule/video_module.py | 24 ++-- common/lib/xmodule/x_module.py | 79 +++++------ 6 files changed, 171 insertions(+), 168 deletions(-) diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 9e82fbe8d4..1b186be3db 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -72,6 +72,102 @@ class CapaModule(XModule): ''' icon_class = 'problem' + 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.attempts = 0 + self.max_attempts = None + + dom2 = etree.fromstring(definition['data']) + + self.explanation = "problems/" + only_one(dom2.xpath('/problem/@explain'), + default="closed") + # TODO: Should be converted to: self.explanation=only_one(dom2.xpath('/problem/@explain'), default="closed") + self.explain_available = only_one(dom2.xpath('/problem/@explain_available')) + + display_due_date_string = self.metadata.get('due', None) + if display_due_date_string is not None: + self.display_due_date = dateutil.parser.parse(display_due_date_string) + #log.debug("Parsed " + display_due_date_string + " to " + str(self.display_due_date)) + else: + self.display_due_date = None + + grace_period_string = self.metadata.get('graceperiod', None) + if grace_period_string is not None and self.display_due_date: + self.grace_period = parse_timedelta(grace_period_string) + self.close_date = self.display_due_date + self.grace_period + #log.debug("Then parsed " + grace_period_string + " to closing date" + str(self.close_date)) + else: + self.grace_period = None + self.close_date = self.display_due_date + + self.max_attempts = only_one(dom2.xpath('/problem/@attempts')) + if len(self.max_attempts) > 0: + self.max_attempts = int(self.max_attempts) + else: + self.max_attempts = None + + self.show_answer = self.metadata.get('showanwser', 'closed') + + if self.show_answer == "": + self.show_answer = "closed" + + self.rerandomize = self.metadata.get('rerandomize', 'always') + 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) + + 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" + self.name = only_one(dom2.xpath('/problem/@name')) + + weight_string = only_one(dom2.xpath('/problem/@weight')) + if weight_string: + self.weight = float(weight_string) + else: + self.weight = 1 + + if self.rerandomize == 'never': + seed = 1 + elif self.rerandomize == "per_student" and hasattr(system, 'id'): + seed = system.id + else: + seed = None + try: + fp = self.system.filestore.open(self.filename) + except Exception: + log.exception('cannot open file %s' % self.filename) + if self.system.DEBUG: + # create a dummy problem instead of failing + fp = StringIO.StringIO('Problem file %s is missing' % self.filename) + fp.name = "StringIO" + else: + raise + try: + self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system) + except Exception: + msg = 'cannot create LoncapaProblem %s' % self.filename + log.exception(msg) + if self.system.DEBUG: + msg = '

%s

' % msg.replace('<', '<') + msg += '

%s

' % traceback.format_exc().replace('<', '<') + # create a dummy problem with error message instead of failing + fp = StringIO.StringIO('Problem file %s has an error:%s' % (self.filename, msg)) + fp.name = "StringIO" + self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system) + else: + raise + def get_instance_state(self): state = self.lcp.get_state() state['attempts'] = self.attempts @@ -171,102 +267,6 @@ class CapaModule(XModule): return 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.attempts = 0 - self.max_attempts = None - - dom2 = etree.fromstring(definition['data']) - - self.explanation = "problems/" + only_one(dom2.xpath('/problem/@explain'), - default="closed") - # TODO: Should be converted to: self.explanation=only_one(dom2.xpath('/problem/@explain'), default="closed") - self.explain_available = only_one(dom2.xpath('/problem/@explain_available')) - - display_due_date_string = self.metadata.get('due', None) - if display_due_date_string is not None: - self.display_due_date = dateutil.parser.parse(display_due_date_string) - #log.debug("Parsed " + display_due_date_string + " to " + str(self.display_due_date)) - else: - self.display_due_date = None - - grace_period_string = self.metadata.get('graceperiod', None) - if grace_period_string is not None and self.display_due_date: - self.grace_period = parse_timedelta(grace_period_string) - self.close_date = self.display_due_date + self.grace_period - #log.debug("Then parsed " + grace_period_string + " to closing date" + str(self.close_date)) - else: - self.grace_period = None - self.close_date = self.display_due_date - - self.max_attempts = only_one(dom2.xpath('/problem/@attempts')) - if len(self.max_attempts) > 0: - self.max_attempts = int(self.max_attempts) - else: - self.max_attempts = None - - self.show_answer = self.metadata.get('showanwser', 'closed') - - if self.show_answer == "": - self.show_answer = "closed" - - self.rerandomize = self.metadata.get('rerandomize', 'always') - 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) - - 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" - self.name = only_one(dom2.xpath('/problem/@name')) - - weight_string = only_one(dom2.xpath('/problem/@weight')) - if weight_string: - self.weight = float(weight_string) - else: - self.weight = 1 - - if self.rerandomize == 'never': - seed = 1 - elif self.rerandomize == "per_student" and hasattr(system, 'id'): - seed = system.id - else: - seed = None - try: - fp = self.system.filestore.open(self.filename) - except Exception: - log.exception('cannot open file %s' % self.filename) - if self.system.DEBUG: - # create a dummy problem instead of failing - fp = StringIO.StringIO('Problem file %s is missing' % self.filename) - fp.name = "StringIO" - else: - raise - try: - self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system) - except Exception: - msg = 'cannot create LoncapaProblem %s' % self.filename - log.exception(msg) - if self.system.DEBUG: - msg = '

%s

' % msg.replace('<', '<') - msg += '

%s

' % traceback.format_exc().replace('<', '<') - # create a dummy problem with error message instead of failing - fp = StringIO.StringIO('Problem file %s has an error:%s' % (self.filename, msg)) - fp.name = "StringIO" - self.lcp = LoncapaProblem(fp, self.location.html_id(), instance_state, seed=seed, system=self.system) - else: - raise - def handle_ajax(self, dispatch, get): ''' This is called by courseware.module_render, to handle an AJAX call. diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index af8563c442..b11d540143 100644 --- a/common/lib/xmodule/seq_module.py +++ b/common/lib/xmodule/seq_module.py @@ -18,6 +18,22 @@ class_priority = ['video', 'problem'] class SequenceModule(XModule): ''' Layout module which lays out content in a temporal sequence ''' + + 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 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'): + self.position = int(system.get('position')) + + self.rendered = False + def get_instance_state(self): return json.dumps({'position': self.position}) @@ -81,21 +97,6 @@ class SequenceModule(XModule): 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 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'): - self.position = int(system.get('position')) - - self.rendered = False - class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' diff --git a/common/lib/xmodule/template_module.py b/common/lib/xmodule/template_module.py index 52c05616cf..1057fc2a25 100644 --- a/common/lib/xmodule/template_module.py +++ b/common/lib/xmodule/template_module.py @@ -26,8 +26,6 @@ class CustomTagModule(XModule): Renders to:: More information given in the text """ - 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) @@ -36,6 +34,8 @@ class CustomTagModule(XModule): params = dict(xmltree.items()) self.html = self.system.render_template(filename, params, namespace='custom_tags') + def get_html(self): + return self.html class CustomTagDescriptor(RawDescriptor): module_class = CustomTagModule diff --git a/common/lib/xmodule/vertical_module.py b/common/lib/xmodule/vertical_module.py index 6008eb4226..c9ecc5ea18 100644 --- a/common/lib/xmodule/vertical_module.py +++ b/common/lib/xmodule/vertical_module.py @@ -9,6 +9,11 @@ class_priority = ['video', 'problem'] class VerticalModule(XModule): ''' Layout module for laying out submodules vertically.''' + + 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 = None + def get_html(self): if self.contents is None: self.contents = [child.get_html() for child in self.get_display_items()] @@ -32,10 +37,6 @@ class VerticalModule(XModule): 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.contents = None - class VerticalDescriptor(SequenceDescriptor): module_class = VerticalModule diff --git a/common/lib/xmodule/video_module.py b/common/lib/xmodule/video_module.py index 4aa469db7f..ed44a2d422 100644 --- a/common/lib/xmodule/video_module.py +++ b/common/lib/xmodule/video_module.py @@ -13,6 +13,18 @@ class VideoModule(XModule): video_time = 0 icon_class = 'video' + 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 instance_state is not None: + state = json.loads(instance_state) + if 'position' in state: + self.position = int(float(state['position'])) + def handle_ajax(self, dispatch, get): ''' Handle ajax calls to this video. @@ -52,18 +64,6 @@ class VideoModule(XModule): 'name': self.name, }) - 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 instance_state is not None: - state = json.loads(instance_state) - if 'position' in state: - self.position = int(float(state['position'])) - class VideoDescriptor(RawDescriptor): module_class = VideoModule diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index c027d1d774..20a52642d1 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -189,6 +189,46 @@ class XModuleDescriptor(Plugin): # A list of metadata that this module can inherit from its parent module inheritable_metadata = ('graded', 'due', 'graceperiod', 'showanswer', 'rerandomize') + def __init__(self, + system, + definition=None, + **kwargs): + """ + Construct a new XModuleDescriptor. The only required arguments are the + system, used for interaction with external resources, and the definition, + which specifies all the data needed to edit and display the problem (but none + of the associated metadata that handles recordkeeping around the problem). + + This allows for maximal flexibility to add to the interface while preserving + backwards compatibility. + + system: An XModuleSystem for interacting with external resources + definition: A dict containing `data` and `children` representing the problem definition + + Current arguments passed in kwargs: + location: A keystore.Location object indicating the name and ownership of this problem + shared_state_key: The key to use for sharing StudentModules with other + modules of this type + metadata: A dictionary containing the following optional keys: + goals: A list of strings of learning goals associated with this module + display_name: The name to use for displaying this module to the user + format: The format of this module ('Homework', 'Lab', etc) + graded (bool): Whether this module is should be graded or not + due (string): The due date for this module + graceperiod (string): The amount of grace period to allow when enforcing the due date + showanswer (string): When to show answers for this module + rerandomize (string): When to generate a newly randomized instance of the module data + """ + self.system = system + self.definition = definition if definition is not None else {} + self.name = Location(kwargs.get('location')).name + self.type = Location(kwargs.get('location')).category + self.url = Location(kwargs.get('location')).url() + self.metadata = kwargs.get('metadata', {}) + self.shared_state_key = kwargs.get('shared_state_key') + + self._child_instances = None + @staticmethod def load_from_json(json_data, system, default_class=None): """ @@ -269,45 +309,6 @@ class XModuleDescriptor(Plugin): """ return self.js_module - def __init__(self, - system, - definition=None, - **kwargs): - """ - Construct a new XModuleDescriptor. The only required arguments are the - system, used for interaction with external resources, and the definition, - which specifies all the data needed to edit and display the problem (but none - of the associated metadata that handles recordkeeping around the problem). - - This allows for maximal flexibility to add to the interface while preserving - backwards compatibility. - - system: An XModuleSystem for interacting with external resources - definition: A dict containing `data` and `children` representing the problem definition - - Current arguments passed in kwargs: - location: A keystore.Location object indicating the name and ownership of this problem - shared_state_key: The key to use for sharing StudentModules with other - modules of this type - metadata: A dictionary containing the following optional keys: - goals: A list of strings of learning goals associated with this module - display_name: The name to use for displaying this module to the user - format: The format of this module ('Homework', 'Lab', etc) - graded (bool): Whether this module is should be graded or not - due (string): The due date for this module - graceperiod (string): The amount of grace period to allow when enforcing the due date - showanswer (string): When to show answers for this module - rerandomize (string): When to generate a newly randomized instance of the module data - """ - self.system = system - self.definition = definition if definition is not None else {} - self.name = Location(kwargs.get('location')).name - self.type = Location(kwargs.get('location')).category - self.url = Location(kwargs.get('location')).url() - self.metadata = kwargs.get('metadata', {}) - self.shared_state_key = kwargs.get('shared_state_key') - - self._child_instances = None def inherit_metadata(self, metadata): """ From cff8ae462317f1c80e369418cf22314b88990a33 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 08:50:56 -0400 Subject: [PATCH 31/40] Add more documentation to XModule --- common/lib/xmodule/x_module.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 20a52642d1..af9e048385 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -56,13 +56,16 @@ class Plugin(object): class XModule(object): - ''' Implements a generic learning module. - 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 + ''' Implements a generic learning module. - See the HTML module for a simple example + Subclasses must at a minimum provide a definition for get_html in order to be displayed to users. + + See the HTML module for a simple example. ''' + + # The default implementation of get_icon_class returns the icon_class attribute of the class + # This attribute can be overridden by subclasses, and the function can also be overridden + # if the icon class depends on the data in the module icon_class = 'other' def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): @@ -72,8 +75,18 @@ class XModule(object): 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 + 'data': is JSON-like (string, dictionary, list, bool, or None, optionally nested) 'children': is a list of xmodule urls for child modules that this module depends on + instance_state: A string of serialized json that contains the state of this module for + current student accessing the system, or None if no state has been saved + shared_state: A string of serialized json that contains the state that is shared between + this module and any modules of the same type with the same shared_state_key. This + state is only shared per-student, not across different students + kwargs: Optional arguments. Subclasses should always accept kwargs and pass them + to the parent class constructor. + Current known uses of kwargs: + metadata: A dictionary containing data that specifies information that is particular + to a problem in the context of a course ''' self.system = system self.location = Location(location) @@ -121,7 +134,7 @@ class XModule(object): def get_icon_class(self): ''' - Return a class identifying this module in the context of an icon + Return a css class identifying this module in the context of an icon ''' return self.icon_class @@ -155,7 +168,7 @@ class XModule(object): def get_html(self): ''' HTML, as shown in the browser. This is the only method that must be implemented ''' - return "Unimplemented" + raise NotImplementedError("get_html must be defined for all XModules that appear on the screen. Not defined in %s" % self.__class__.__name__) def get_progress(self): ''' Return a progress.Progress object that represents how far the student has gone From 32ed18fef60fabb886caafc15005692d1ee337f7 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 08:54:13 -0400 Subject: [PATCH 32/40] Allow slightly more latitude in what is passed as the children array --- common/lib/xmodule/x_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index af9e048385..68f381a755 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -76,7 +76,7 @@ class XModule(object): location: Something Location-like that identifies this xmodule definition: A dictionary containing 'data' and 'children'. Both are optional 'data': is JSON-like (string, dictionary, list, bool, or None, optionally nested) - 'children': is a list of xmodule urls for child modules that this module depends on + 'children': is a list of Location-like values for child modules that this module depends on instance_state: A string of serialized json that contains the state of this module for current student accessing the system, or None if no state has been saved shared_state: A string of serialized json that contains the state that is shared between From 4ae711c421a8a013c0890cf1f972504ed9c4f763 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 08:54:23 -0400 Subject: [PATCH 33/40] Clarify from_json arguments --- common/lib/xmodule/x_module.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 68f381a755..35cde8b6f4 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -263,7 +263,8 @@ class XModuleDescriptor(Plugin): Creates an instance of this descriptor from the supplied json_data. This may be overridden by subclasses - json_data: Json data specifying the data, children, and metadata for the descriptor + json_data: A json object specifying the definition and any optional keyword arguments for + the XModuleDescriptor system: An XModuleSystem for interacting with external resources """ return cls(system=system, **json_data) From 7b78fa5278f2417452b873591273785dcd91ec2a Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 08:57:05 -0400 Subject: [PATCH 34/40] Make self.rerandomize a property accessor that reads from metadata --- common/lib/xmodule/capa_module.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/common/lib/xmodule/capa_module.py b/common/lib/xmodule/capa_module.py index 1b186be3db..6a95789417 100644 --- a/common/lib/xmodule/capa_module.py +++ b/common/lib/xmodule/capa_module.py @@ -112,16 +112,6 @@ class CapaModule(XModule): if self.show_answer == "": self.show_answer = "closed" - self.rerandomize = self.metadata.get('rerandomize', 'always') - 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) - if instance_state != None: instance_state = json.loads(instance_state) if instance_state != None and 'attempts' in instance_state: @@ -168,6 +158,21 @@ class CapaModule(XModule): else: raise + @property + def rerandomize(self): + """ + Property accessor that returns self.metadata['rerandomize'] in a canonical form + """ + rerandomize = self.metadata.get('rerandomize', 'always') + if rerandomize in ("", "always", "true"): + return "always" + elif rerandomize in ("false", "per_student"): + return "per_student" + elif rerandomize == "never": + return "never" + else: + raise Exception("Invalid rerandomize attribute " + rerandomize) + def get_instance_state(self): state = self.lcp.get_state() state['attempts'] = self.attempts @@ -221,7 +226,7 @@ class CapaModule(XModule): # User submitted a problem, and hasn't reset. We don't want # more submissions. - if self.lcp.done and self.metadata['rerandomize'] == "always": + if self.lcp.done and self.rerandomize == "always": check_button = False save_button = False From 7ac8fecb38582cfa0f7a73e8a223dfd08affe543 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 09:01:08 -0400 Subject: [PATCH 35/40] Rename XModule[Descriptor].type to .category to be parallel to Location --- cms/djangoapps/contentstore/views.py | 2 +- cms/templates/unit.html | 2 +- common/lib/xmodule/x_module.py | 4 ++-- lms/djangoapps/courseware/grades.py | 4 ++-- lms/djangoapps/courseware/module_render.py | 10 +++++----- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index b85e9c05bf..f7d5efe22a 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -22,7 +22,7 @@ def edit_item(request): return render_to_response('unit.html', { 'contents': item.get_html(), 'js_module': item.js_module_name(), - 'type': item.type, + 'category': item.category, 'name': item.name, }) diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 59044ab28d..34e21ca049 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -2,7 +2,7 @@

${name}

-

${type}

+

${category}

diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 35cde8b6f4..f791e7f307 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -95,7 +95,7 @@ class XModule(object): self.shared_state = shared_state self.id = self.location.url() self.name = self.location.name - self.type = self.location.category + self.category = self.location.category self.metadata = kwargs.get('metadata', {}) self._loaded_children = None @@ -235,7 +235,7 @@ class XModuleDescriptor(Plugin): self.system = system self.definition = definition if definition is not None else {} self.name = Location(kwargs.get('location')).name - self.type = Location(kwargs.get('location')).category + self.category = Location(kwargs.get('location')).category self.url = Location(kwargs.get('location')).url() self.metadata = kwargs.get('metadata', {}) self.shared_state_key = kwargs.get('shared_state_key') diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index deab9d47d4..b9b89d6cd4 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -146,9 +146,9 @@ def get_score(user, problem, cache): correct = 0.0 # If the ID is not in the cache, add the item - instance_module = cache.lookup(problem.type, problem.id) + instance_module = cache.lookup(problem.category, problem.id) if instance_module is None: - instance_module = StudentModule(module_type=problem.type, + instance_module = StudentModule(module_type=problem.category, module_state_key=problem.id, student=user, state=None, diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 6cf4e43cc5..2d47a55248 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -207,10 +207,10 @@ def get_module(user, request, location, student_module_cache, position=None): ''' descriptor = keystore().get_item(location) - instance_module = student_module_cache.lookup(descriptor.type, descriptor.url) + instance_module = student_module_cache.lookup(descriptor.category, 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) + shared_module = student_module_cache.lookup(descriptor.category, shared_state_key) else: shared_module = None @@ -246,7 +246,7 @@ def get_module(user, request, location, student_module_cache, position=None): if not instance_module: instance_module = StudentModule( student=user, - module_type=descriptor.type, + module_type=descriptor.category, module_state_key=module.id, state=module.get_instance_state(), max_grade=module.max_score()) @@ -257,13 +257,13 @@ def get_module(user, request, location, student_module_cache, position=None): if not shared_module and shared_state_key is not None: shared_module = StudentModule( student=user, - module_type=descriptor.type, + module_type=descriptor.category, 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 (module, instance_module, shared_module, descriptor.category) def add_histogram(module): From 736148f21dd133359be1e593fe8230bcc47dd15f Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 09:02:41 -0400 Subject: [PATCH 36/40] Adding clarifying comment about the contents of 'data' --- common/lib/xmodule/x_module.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index f791e7f307..0b8fbb54d4 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -75,7 +75,10 @@ class XModule(object): 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 JSON-like (string, dictionary, list, bool, or None, optionally nested) + 'data': is JSON-like (string, dictionary, list, bool, or None, optionally nested). + This defines all of the data necessary for a problem to display that is intrinsic to the problem. + It should not include any data that would vary between two courses using the same problem + (due dates, grading policy, randomization, etc.) 'children': is a list of Location-like values for child modules that this module depends on instance_state: A string of serialized json that contains the state of this module for current student accessing the system, or None if no state has been saved From 7ed9b4aa89c5a0c09104d63715a2fdbe9d500a15 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 09:08:24 -0400 Subject: [PATCH 37/40] Add hashing and equality methods to Location --- common/lib/keystore/__init__.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py index 14716fbc2d..b204e487e2 100644 --- a/common/lib/keystore/__init__.py +++ b/common/lib/keystore/__init__.py @@ -101,9 +101,6 @@ class Location(object): if val is not None and INVALID_CHARS.search(val) is not None: raise InvalidLocationError(location) - def __str__(self): - return self.url() - def url(self): """ Return a string containing the URL for this location @@ -136,6 +133,19 @@ class Location(object): 'name': self.name, 'revision': self.revision} + def __str__(self): + return self.url() + + def __repr__(self): + return 'Location(%r)' % str(self) + + def __hash__(self): + return self.url() + + def __eq__(self, other): + return (isinstance(other, Location) and + str(self) == str(other)) + class ModuleStore(object): """ From 3cf29af8fe5521fc08abf6d88862b92fac6a5bc7 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 10:17:59 -0400 Subject: [PATCH 38/40] Make location into a named tuple, and use it more as a first class entity, rather than URL for identifying content --- .../management/commands/import.py | 6 +- cms/templates/widgets/navigation.html | 6 +- cms/templates/widgets/sequence-edit.html | 2 +- common/lib/keystore/__init__.py | 92 ++++++++----------- common/lib/keystore/tests/test_location.py | 15 ++- common/lib/keystore/xml.py | 4 +- common/lib/xmodule/abtest_module.py | 2 +- common/lib/xmodule/seq_module.py | 2 +- common/lib/xmodule/x_module.py | 4 +- lms/djangoapps/courseware/models.py | 3 +- lms/djangoapps/courseware/module_render.py | 4 +- lms/djangoapps/courseware/views.py | 2 +- 12 files changed, 70 insertions(+), 72 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index a7f95ea3c0..e24111dbb7 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -25,8 +25,8 @@ class Command(BaseCommand): module_store = XMLModuleStore(org, course, data_dir, 'xmodule.raw_module.RawDescriptor') for module in module_store.modules.itervalues(): - keystore().create_item(module.url) + keystore().create_item(module.location) if 'data' in module.definition: - keystore().update_item(module.url, module.definition['data']) + keystore().update_item(module.location, module.definition['data']) if 'children' in module.definition: - keystore().update_children(module.url, module.definition['children']) + keystore().update_children(module.location, module.definition['children']) diff --git a/cms/templates/widgets/navigation.html b/cms/templates/widgets/navigation.html index 38b1cd9d94..bed0d1b4f8 100644 --- a/cms/templates/widgets/navigation.html +++ b/cms/templates/widgets/navigation.html @@ -38,7 +38,7 @@ % for week in weeks:
  • -

    ${week.name}

    +

    ${week.name}

      % if week.goals: % for goal in week.goals: @@ -52,8 +52,8 @@
        % for module in week.get_children(): -
      • - ${module.name} +
      • + ${module.name} handle
        • % endfor diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index abeec9209d..319e137638 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -37,7 +37,7 @@
          % for child in module.get_children():
        1. - ${child.name} + ${child.name} handle
          2. %endfor diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py index b204e487e2..dab758fef1 100644 --- a/common/lib/keystore/__init__.py +++ b/common/lib/keystore/__init__.py @@ -4,6 +4,7 @@ that are stored in a database an accessible using their Location as an identifie """ import re +from collections import namedtuple from .exceptions import InvalidLocationError URL_RE = re.compile(""" @@ -17,8 +18,8 @@ URL_RE = re.compile(""" INVALID_CHARS = re.compile(r"[^\w.-]") - -class Location(object): +_LocationBase = namedtuple('LocationBase', 'tag org course category name revision') +class Location(_LocationBase): ''' Encodes a location. @@ -28,6 +29,7 @@ class Location(object): However, they can also be represented a dictionaries (specifying each component), tuples or list (specified in order), or as strings of the url ''' + __slots__ = () @classmethod def clean(cls, value): @@ -36,7 +38,7 @@ class Location(object): """ return re.sub('_+', '_', INVALID_CHARS.sub('_', value)) - def __init__(self, location): + def __new__(_cls, loc_or_tag, org=None, course=None, category=None, name=None, revision=None): """ Create a new location that is a clone of the specifed one. @@ -60,47 +62,50 @@ class Location(object): Components may be set to None, which may be interpreted by some contexts to mean wildcard selection """ - self.update(location) - def update(self, location): - """ - Update this instance with data from another Location object. + if org is None and course is None and category is None and name is None and revision is None: + location = loc_or_tag + else: + location = (loc_or_tag, org, course, category, name, revision) - location: can take the same forms as specified by `__init__` - """ - self.tag = self.org = self.course = self.category = self.name = self.revision = None + def check_dict(dict_): + check_list(dict_.values()) + + def check_list(list_): + for val in list_: + if val is not None and INVALID_CHARS.search(val) is not None: + raise InvalidLocationError(location) if isinstance(location, basestring): match = URL_RE.match(location) if match is None: raise InvalidLocationError(location) else: - self.update(match.groupdict()) - elif isinstance(location, list): + groups = match.groupdict() + check_dict(groups) + return _LocationBase.__new__(_cls, **groups) + elif isinstance(location, (list, tuple)): if len(location) not in (5, 6): raise InvalidLocationError(location) - (self.tag, self.org, self.course, self.category, self.name) = location[0:5] - self.revision = location[5] if len(location) == 6 else None + if len(location) == 5: + args = tuple(location) + (None, ) + else: + args = tuple(location) + + check_list(args) + return _LocationBase.__new__(_cls, *args) elif isinstance(location, dict): - try: - self.tag = location['tag'] - self.org = location['org'] - self.course = location['course'] - self.category = location['category'] - self.name = location['name'] - except KeyError: - raise InvalidLocationError(location) - self.revision = location.get('revision') + kwargs = dict(location) + kwargs.setdefault('revision', None) + + check_dict(kwargs) + return _LocationBase.__new__(_cls, **kwargs) elif isinstance(location, Location): - self.update(location.list()) + return _LocationBase.__new__(_cls, location) else: raise InvalidLocationError(location) - for val in self.list(): - if val is not None and INVALID_CHARS.search(val) is not None: - raise InvalidLocationError(location) - def url(self): """ Return a string containing the URL for this location @@ -114,38 +119,19 @@ class Location(object): """ Return a string with a version of the location that is safe for use in html id attributes """ - return "-".join(str(v) for v in self.list() if v is not None) - - def list(self): - """ - Return a list representing this location - """ - return [self.tag, self.org, self.course, self.category, self.name, self.revision] + return "-".join(str(v) for v in self if v is not None) def dict(self): - """ - Return a dictionary representing this location - """ - return {'tag': self.tag, - 'org': self.org, - 'course': self.course, - 'category': self.category, - 'name': self.name, - 'revision': self.revision} + return self.__dict__ + + def list(self): + return list(self) def __str__(self): return self.url() def __repr__(self): - return 'Location(%r)' % str(self) - - def __hash__(self): - return self.url() - - def __eq__(self, other): - return (isinstance(other, Location) and - str(self) == str(other)) - + return "Location%r" % tuple(self) class ModuleStore(object): """ diff --git a/common/lib/keystore/tests/test_location.py b/common/lib/keystore/tests/test_location.py index f10f03c0b0..01d36d946b 100644 --- a/common/lib/keystore/tests/test_location.py +++ b/common/lib/keystore/tests/test_location.py @@ -1,4 +1,4 @@ -from nose.tools import assert_equals, assert_raises +from nose.tools import assert_equals, assert_raises, assert_not_equals from keystore import Location from keystore.exceptions import InvalidLocationError @@ -11,7 +11,6 @@ def check_string_roundtrip(url): def test_string_roundtrip(): check_string_roundtrip("tag://org/course/category/name") check_string_roundtrip("tag://org/course/category/name/revision") - check_string_roundtrip("tag://org/course/category/name with spaces/revision") def test_dict(): @@ -50,3 +49,15 @@ def test_invalid_locations(): assert_raises(InvalidLocationError, Location, ["foo", "bar"]) assert_raises(InvalidLocationError, Location, ["foo", "bar", "baz", "blat", "foo/bar"]) assert_raises(InvalidLocationError, Location, None) + assert_raises(InvalidLocationError, Location, "tag://org/course/category/name with spaces/revision") + +def test_equality(): + assert_equals( + Location('tag', 'org', 'course', 'category', 'name'), + Location('tag', 'org', 'course', 'category', 'name') + ) + + assert_not_equals( + Location('tag', 'org', 'course', 'category', 'name1'), + Location('tag', 'org', 'course', 'category', 'name') + ) diff --git a/common/lib/keystore/xml.py b/common/lib/keystore/xml.py index c3327b8ff3..e7adb56ad6 100644 --- a/common/lib/keystore/xml.py +++ b/common/lib/keystore/xml.py @@ -47,7 +47,7 @@ class XMLModuleStore(ModuleStore): 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, modulestore.default_class) - modulestore.modules[module.url] = module + modulestore.modules[module.location] = module return module XMLParsingSystem.__init__(self, modulestore.get_item, OSFS(data_dir), process_xml) @@ -68,7 +68,7 @@ class XMLModuleStore(ModuleStore): """ location = Location(location) try: - return self.modules[location.url()] + return self.modules[location] except KeyError: raise ItemNotFoundError(location) diff --git a/common/lib/xmodule/abtest_module.py b/common/lib/xmodule/abtest_module.py index 3bd268184a..beaeb4ad1c 100644 --- a/common/lib/xmodule/abtest_module.py +++ b/common/lib/xmodule/abtest_module.py @@ -102,7 +102,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor): ) child_content_urls = [ - system.process_xml(etree.tostring(child)).url + system.process_xml(etree.tostring(child)).location.url() for child in group ] diff --git a/common/lib/xmodule/seq_module.py b/common/lib/xmodule/seq_module.py index b11d540143..6d493b96ad 100644 --- a/common/lib/xmodule/seq_module.py +++ b/common/lib/xmodule/seq_module.py @@ -105,6 +105,6 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): @classmethod def definition_from_xml(cls, xml_object, system): return {'children': [ - system.process_xml(etree.tostring(child_module)).url + system.process_xml(etree.tostring(child_module)).location.url() for child_module in xml_object ]} diff --git a/common/lib/xmodule/x_module.py b/common/lib/xmodule/x_module.py index 0b8fbb54d4..191cda6b06 100644 --- a/common/lib/xmodule/x_module.py +++ b/common/lib/xmodule/x_module.py @@ -239,7 +239,7 @@ class XModuleDescriptor(Plugin): self.definition = definition if definition is not None else {} self.name = Location(kwargs.get('location')).name self.category = Location(kwargs.get('location')).category - self.url = Location(kwargs.get('location')).url() + self.location = Location(kwargs.get('location')) self.metadata = kwargs.get('metadata', {}) self.shared_state_key = kwargs.get('shared_state_key') @@ -364,7 +364,7 @@ class XModuleDescriptor(Plugin): return partial( self.module_class, system, - self.url, + self.location, self.definition, metadata=self.metadata ) diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 262d177248..f0bd8dc17e 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -85,6 +85,7 @@ class StudentModuleCache(object): student=user, module_state_key__in=id_chunk) ) + else: self.cache = [] @@ -93,7 +94,7 @@ class StudentModuleCache(object): Get a list of the state_keys needed for StudentModules required for this chunk of module xml ''' - keys = [descriptor.url] + keys = [descriptor.location.url()] shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 2d47a55248..5119cc2910 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -207,7 +207,7 @@ def get_module(user, request, location, student_module_cache, position=None): ''' descriptor = keystore().get_item(location) - instance_module = student_module_cache.lookup(descriptor.category, descriptor.url) + instance_module = student_module_cache.lookup(descriptor.category, descriptor.location.url()) shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: shared_module = student_module_cache.lookup(descriptor.category, shared_state_key) @@ -218,7 +218,7 @@ def get_module(user, request, location, student_module_cache, position=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/' + descriptor.url + '/' + ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' def _get_module(location): (module, _, _, _) = get_module(user, request, location, student_module_cache, position) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index e1e1c16632..48e9bcc795 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -209,7 +209,7 @@ def index(request, course=None, chapter=None, section=None, 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) + module, _, _, _ = get_module(request.user, request, section.location, student_module_cache) context['content'] = module.get_html() result = render_to_response('courseware.html', context) From e0e42ae8aca464ea6802fda5c84bc4feabfe3fe0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 10:18:24 -0400 Subject: [PATCH 39/40] Fix up cms after changing html and sequence descriptors to use metadata --- cms/templates/widgets/html-edit.html | 4 ++-- cms/templates/widgets/navigation.html | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/templates/widgets/html-edit.html b/cms/templates/widgets/html-edit.html index 7eec86215a..666aa1de81 100644 --- a/cms/templates/widgets/html-edit.html +++ b/cms/templates/widgets/html-edit.html @@ -33,8 +33,8 @@
    • - -
    ${module.definition['data']['text']}
    + +
    ${module.definition['data']}
    Save & Update diff --git a/cms/templates/widgets/navigation.html b/cms/templates/widgets/navigation.html index bed0d1b4f8..ea158d305a 100644 --- a/cms/templates/widgets/navigation.html +++ b/cms/templates/widgets/navigation.html @@ -40,8 +40,8 @@

    ${week.name}

      - % if week.goals: - % for goal in week.goals: + % if 'goals' in week.metadata: + % for goal in week.metadata['goals']:
    • ${goal}
      • % endfor % else: From 312dda760edeff2d68474f6cad895623f07cf6a0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 2 Jul 2012 10:25:25 -0400 Subject: [PATCH 40/40] Fixing repr for Locations --- common/lib/keystore/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/keystore/__init__.py b/common/lib/keystore/__init__.py index dab758fef1..0671e7e568 100644 --- a/common/lib/keystore/__init__.py +++ b/common/lib/keystore/__init__.py @@ -131,7 +131,8 @@ class Location(_LocationBase): return self.url() def __repr__(self): - return "Location%r" % tuple(self) + return "Location%s" % repr(tuple(self)) + class ModuleStore(object): """