From cbfc7b201af3742c52b5527eae3132d9f6b9aaa7 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 10 Dec 2012 15:40:23 -0500 Subject: [PATCH] WIP more changes to model definitions. Next Up: actually wiring model data into the rdbms --- cms/djangoapps/contentstore/views.py | 4 +- cms/templates/edit_subsection.html | 8 +- cms/templates/new_item.html | 2 +- cms/templates/overview.html | 6 +- cms/templates/unit.html | 8 +- cms/templates/widgets/header.html | 2 +- cms/templates/widgets/navigation.html | 2 +- cms/templates/widgets/sequence-edit.html | 2 +- cms/templates/widgets/units.html | 2 +- common/djangoapps/student/views.py | 2 +- common/djangoapps/xmodule_modifiers.py | 2 +- common/lib/xmodule/setup.py | 2 +- common/lib/xmodule/xmodule/capa_module.py | 59 +++++--- common/lib/xmodule/xmodule/course_module.py | 12 +- .../lib/xmodule/xmodule/discussion_module.py | 14 +- common/lib/xmodule/xmodule/html_module.py | 11 +- .../xmodule/js/src/video/display.coffee | 1 - common/lib/xmodule/xmodule/model.py | 106 +++++++++++--- .../lib/xmodule/xmodule/modulestore/mongo.py | 3 +- common/lib/xmodule/xmodule/modulestore/xml.py | 8 +- common/lib/xmodule/xmodule/plugin.py | 64 +++++++++ common/lib/xmodule/xmodule/seq_module.py | 38 ++--- common/lib/xmodule/xmodule/vertical_module.py | 8 +- common/lib/xmodule/xmodule/video_module.py | 23 ++- common/lib/xmodule/xmodule/x_module.py | 132 +++++++----------- lms/djangoapps/courseware/grades.py | 6 +- lms/djangoapps/courseware/module_render.py | 21 ++- lms/setup.py | 18 +++ lms/templates/courseware/welcome-back.html | 4 +- lms/templates/video.html | 2 +- lms/xmodule_namespace.py | 23 +++ local-requirements.txt | 1 + 32 files changed, 376 insertions(+), 220 deletions(-) create mode 100644 common/lib/xmodule/xmodule/plugin.py create mode 100644 lms/setup.py create mode 100644 lms/xmodule_namespace.py diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 833662b218..bf706f2996 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -115,7 +115,7 @@ def index(request): return render_to_response('index.html', { 'new_course_template' : Location('i4x', 'edx', 'templates', 'course', 'Empty'), - 'courses': [(course.metadata.get('display_name'), + 'courses': [(course.title, reverse('course_index', args=[ course.location.org, course.location.course, @@ -269,7 +269,7 @@ def edit_unit(request, location): for template in templates: if template.location.category in COMPONENT_TYPES: component_templates[template.location.category].append(( - template.display_name, + template.lms.display_name, template.location.url(), )) diff --git a/cms/templates/edit_subsection.html b/cms/templates/edit_subsection.html index d3b6f73f13..53567c73e1 100644 --- a/cms/templates/edit_subsection.html +++ b/cms/templates/edit_subsection.html @@ -21,7 +21,7 @@
- +
@@ -62,11 +62,11 @@
% if subsection.start != parent_item.start and subsection.start: % if parent_start_date is None: -

The date above differs from the release date of ${parent_item.display_name}, which is unset. +

The date above differs from the release date of ${parent_item.lms.display_name}, which is unset. % else: -

The date above differs from the release date of ${parent_item.display_name} – ${parent_start_date.strftime('%m/%d/%Y')} at ${parent_start_date.strftime('%H:%M')}. +

The date above differs from the release date of ${parent_item.lms.display_name} – ${parent_start_date.strftime('%m/%d/%Y')} at ${parent_start_date.strftime('%H:%M')}. % endif -

+

% endif diff --git a/cms/templates/new_item.html b/cms/templates/new_item.html index 60da39fd2a..3c81b81c7e 100644 --- a/cms/templates/new_item.html +++ b/cms/templates/new_item.html @@ -8,7 +8,7 @@
${module_type}
% for template in module_templates: - ${template.display_name} + ${template.lms.display_name} % endfor
diff --git a/cms/templates/overview.html b/cms/templates/overview.html index 2a46908c55..fad28dbf07 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -132,9 +132,9 @@

- ${section.display_name} + ${section.lms.display_name} @@ -174,7 +174,7 @@ - ${subsection.display_name} + ${subsection.lms.display_name}

diff --git a/cms/templates/unit.html b/cms/templates/unit.html index 0599411a67..efd8123890 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -27,7 +27,7 @@
Delete Draft @@ -100,12 +100,12 @@
  1. - ${section.display_name} + ${section.lms.display_name}
    1. - ${subsection.display_name} + ${subsection.lms.display_name} ${units.enum_units(subsection, actions=False, selected=unit.location)}
      2. diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 5f41452339..76a259296d 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -8,7 +8,7 @@ % if context_course: <% ctx_loc = context_course.location %> › - ${context_course.display_name} › + ${context_course.lms.display_name} › % endif
diff --git a/cms/templates/widgets/navigation.html b/cms/templates/widgets/navigation.html index f7e79bceb3..00b147675d 100644 --- a/cms/templates/widgets/navigation.html +++ b/cms/templates/widgets/navigation.html @@ -60,7 +60,7 @@ data-type="${module.js_module_name}" data-preview-type="${module.module_class.js_module_name}"> - ${module.display_name} + ${module.lms.display_name} % endfor <%include file="module-dropdown.html"/> diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index e9d796784d..daee13fc01 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -40,7 +40,7 @@ ${child.display_name} + data-preview-type="${child.module_class.js_module_name}">${child.lms.display_name} handle %endfor diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 8e23b05bf8..f7cad23355 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -22,7 +22,7 @@ This def will enumerate through a passed in subsection and list all of the units
- ${unit.display_name} + ${unit.lms.display_name} % if actions:
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index e7562f83d0..bfde1b1419 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -231,7 +231,7 @@ def change_enrollment(request): if not has_access(user, course, 'enroll'): return {'success': False, 'error': 'enrollment in {} not allowed at this time' - .format(course.display_name)} + .format(course.lms.display_name)} org, course_num, run=course_id.split("/") statsd.increment("common.student.enrollment", diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 5c19a2f1d7..81f0205c3e 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -32,7 +32,7 @@ def wrap_xmodule(get_html, module, template, context=None): def _get_html(): context.update({ 'content': get_html(), - 'display_name' : module.metadata.get('display_name') if module.metadata is not None else None, + 'display_name': module.lms.display_name, 'class_': module.__class__.__name__, 'module_name': module.js_module_name }) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 24cddd2047..b3a8dabe1b 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -40,6 +40,6 @@ setup( "static_tab = xmodule.html_module:StaticTabDescriptor", "custom_tag_template = xmodule.raw_module:RawDescriptor", "about = xmodule.html_module:AboutDescriptor" - ] + ], } ) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 6ad6de2be6..acb4b63e1c 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -21,8 +21,6 @@ from xmodule.raw_module import RawDescriptor from xmodule.exceptions import NotFoundError from .model import Int, Scope, ModuleScope, ModelType, String, Boolean, Object, Float -Date = Timedelta = ModelType - log = logging.getLogger("mitx.courseware") #----------------------------------------------------------------------------- @@ -45,25 +43,34 @@ def only_one(lst, default="", process=lambda x: x): raise Exception('Malformed XML: expected at most one element in list.') -def parse_timedelta(time_str): - """ - time_str: A string with the following components: - day[s] (optional) - hour[s] (optional) - minute[s] (optional) - second[s] (optional) +class Timedelta(ModelType): + def from_json(self, time_str): + """ + time_str: A string with the following components: + day[s] (optional) + hour[s] (optional) + minute[s] (optional) + second[s] (optional) - Returns a datetime.timedelta parsed from the string - """ - parts = TIMEDELTA_REGEX.match(time_str) - if not parts: - return - parts = parts.groupdict() - time_params = {} - for (name, param) in parts.iteritems(): - if param: - time_params[name] = int(param) - return timedelta(**time_params) + Returns a datetime.timedelta parsed from the string + """ + parts = TIMEDELTA_REGEX.match(time_str) + if not parts: + return + parts = parts.groupdict() + time_params = {} + for (name, param) in parts.iteritems(): + if param: + time_params[name] = int(param) + return timedelta(**time_params) + + def to_json(self, value): + values = [] + for attr in ('days', 'hours', 'minutes', 'seconds'): + cur_value = getattr(value, attr, 0) + if cur_value > 0: + values.append("%d %s" % (cur_value, attr)) + return ' '.join(values) class ComplexEncoder(json.JSONEncoder): @@ -82,7 +89,7 @@ class CapaModule(XModule): attempts = Int(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.student_state) max_attempts = Int(help="Maximum number of attempts that a student is allowed", scope=Scope.settings) - due = Date(help="Date that this problem is due by", scope=Scope.settings) + due = String(help="Date that this problem is due by", scope=Scope.settings) graceperiod = Timedelta(help="Amount of time after the due date that submissions will be accepted", scope=Scope.settings) show_answer = String(help="When to show the problem answer to the student", scope=Scope.settings, default="closed") force_save_button = Boolean(help="Whether to force the save button to appear on the page", scope=Scope.settings) @@ -90,6 +97,7 @@ class CapaModule(XModule): data = String(help="XML data for the problem", scope=Scope.content) correct_map = Object(help="Dictionary with the correctness of current student answers", scope=Scope.student_state) done = Boolean(help="Whether the student has answered the problem", scope=Scope.student_state) + display_name = String(help="Display name for this module", scope=Scope.settings) js = {'coffee': [resource_string(__name__, 'js/src/capa/display.coffee'), resource_string(__name__, 'js/src/collapsible.coffee'), @@ -104,8 +112,13 @@ class CapaModule(XModule): def __init__(self, system, location, descriptor, model_data): XModule.__init__(self, system, location, descriptor, model_data) - if self.graceperiod is not None and self.due: - self.close_date = self.due + self.graceperiod + if self.due: + due_date = dateutil.parser.parse(self.due) + else: + due_date = None + + if self.graceperiod is not None and due_date: + self.close_date = due_date + self.graceperiod #log.debug("Then parsed " + grace_period_string + # " to closing date" + str(self.close_date)) else: diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 019a79e7ab..c4dc63c1b4 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -13,8 +13,7 @@ import time import copy from .model import Scope, ModelType, List, String, Object, Boolean - -Date = ModelType +from .x_module import Date log = logging.getLogger(__name__) @@ -31,6 +30,10 @@ class CourseDescriptor(SequenceDescriptor): end = Date(help="Date that this class ends", scope=Scope.settings) advertised_start = Date(help="Date that this course is advertised to start", scope=Scope.settings) grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content) + show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, scope=Scope.settings) + start = Date(help="Start time when this module is visible", scope=Scope.settings) + display_name = String(help="Display name for this module", scope=Scope.settings) + has_children = True info_sidebar_name = String(scope=Scope.settings, default='Course Handouts') @@ -342,10 +345,6 @@ class CourseDescriptor(SequenceDescriptor): def tabs(self, value): self.metadata['tabs'] = value - @property - def show_calculator(self): - return self.metadata.get("show_calculator", None) == "Yes" - @lazyproperty def grading_context(self): """ @@ -433,6 +432,7 @@ class CourseDescriptor(SequenceDescriptor): @property def start_date_text(self): + print self.advertised_start, self.start return time.strftime("%b %d, %Y", self.advertised_start or self.start) @property diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 1deceac5d0..a193604278 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -3,8 +3,7 @@ from pkg_resources import resource_string, resource_listdir from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor - -import json +from .model import String, Scope class DiscussionModule(XModule): js = {'coffee': @@ -12,18 +11,19 @@ class DiscussionModule(XModule): resource_string(__name__, 'js/src/discussion/display.coffee')] } js_module_name = "InlineDiscussion" + + data = String(help="XML definition of inline discussion", scope=Scope.content) + def get_html(self): context = { 'discussion_id': self.discussion_id, } return self.system.render_template('discussion/_discussion_module.html', context) - def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) + def __init__(self, *args, **kwargs): + XModule.__init__(self, *args, **kwargs) - if isinstance(instance_state, str): - instance_state = json.loads(instance_state) - xml_data = etree.fromstring(definition['data']) + xml_data = etree.fromstring(self.data) self.discussion_id = xml_data.attrib['id'] self.title = xml_data.attrib['for'] self.discussion_category = xml_data.attrib['discussion_category'] diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 709f86bf45..6d86fb90a8 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -15,6 +15,7 @@ from .html_checker import check_html from xmodule.modulestore import Location from xmodule.contentstore.content import XASSET_SRCREF_PREFIX, StaticContent +from .model import Scope, String log = logging.getLogger("mitx.courseware") @@ -26,15 +27,11 @@ class HtmlModule(XModule): ] } js_module_name = "HTMLModule" + + data = String(help="Html contents to display for this module", scope=Scope.content) def get_html(self): - return self.html - - def __init__(self, system, location, definition, descriptor, - instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) - self.html = self.definition['data'] + return self.data diff --git a/common/lib/xmodule/xmodule/js/src/video/display.coffee b/common/lib/xmodule/xmodule/js/src/video/display.coffee index bb87679d37..dbcab4a670 100644 --- a/common/lib/xmodule/xmodule/js/src/video/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/video/display.coffee @@ -2,7 +2,6 @@ class @Video constructor: (element) -> @el = $(element).find('.video') @id = @el.attr('id').replace(/video_/, '') - @caption_data_dir = @el.data('caption-data-dir') @caption_asset_path = @el.data('caption-asset-path') @show_captions = @el.data('show-captions') == "true" window.player = null diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py index b58ffa267a..edd94f14a9 100644 --- a/common/lib/xmodule/xmodule/model.py +++ b/common/lib/xmodule/xmodule/model.py @@ -1,4 +1,6 @@ from collections import namedtuple +from .plugin import Plugin + class ModuleScope(object): USAGE, DEFINITION, TYPE, ALL = xrange(4) @@ -15,6 +17,13 @@ Scope.student_info = Scope(student=True, module=ModuleScope.ALL) class ModelType(object): + """ + A field class that can be used as a class attribute to define what data the class will want + to refer to. + + When the class is instantiated, it will be available as an instance attribute of the same + name, by proxying through to self._model_data on the containing object. + """ sequence = 0 def __init__(self, help=None, default=None, scope=Scope.content): @@ -33,10 +42,13 @@ class ModelType(object): if instance is None: return self - return instance._model_data.get(self.name, self.default) + if self.name not in instance._model_data: + return self.default + + return self.from_json(instance._model_data[self.name]) def __set__(self, instance, value): - instance._model_data[self.name] = value + instance._model_data[self.name] = self.to_json(value) def __delete__(self, instance): del instance._model_data[self.name] @@ -47,27 +59,27 @@ class ModelType(object): def __lt__(self, other): return self._seq < other._seq + def to_json(self, value): + return value + + def from_json(self, value): + return value + Int = Float = Boolean = Object = List = String = Any = ModelType class ModelMetaclass(type): + """ + A metaclass to be used for classes that want to use ModelTypes as class attributes + to define data access. + + All class attributes that are ModelTypes will be added to the 'fields' attribute on + the instance. + + Additionally, any namespaces registered in the `xmodule.namespace` will be added to + the instance + """ def __new__(cls, name, bases, attrs): - # Find registered methods - reg_methods = {} - for value in attrs.itervalues(): - for reg_type, names in getattr(value, "_method_registrations", {}).iteritems(): - for n in names: - reg_methods[reg_type + n] = value - attrs['registered_methods'] = reg_methods - - if attrs.get('has_children', False): - attrs['children'] = ModelType(help='The children of this XModule', default=[], scope=None) - - @property - def child_map(self): - return dict((child.name, child) for child in self.children) - attrs['child_map'] = child_map - fields = [] for n, v in attrs.items(): if isinstance(v, ModelType): @@ -77,3 +89,61 @@ class ModelMetaclass(type): attrs['fields'] = fields return super(ModelMetaclass, cls).__new__(cls, name, bases, attrs) + + +class NamespacesMetaclass(type): + """ + A metaclass to be used for classes that want to include namespaced fields in their + instances. + + Any namespaces registered in the `xmodule.namespace` will be added to + the instance + """ + def __new__(cls, name, bases, attrs): + for ns_name, namespace in Namespace.load_classes(): + if issubclass(namespace, Namespace): + attrs[ns_name] = NamespaceDescriptor(namespace) + + return super(NamespacesMetaclass, cls).__new__(cls, name, bases, attrs) + + +class ParentModelMetaclass(type): + """ + A ModelMetaclass that transforms the attribute `has_children = True` + into a List field with an empty scope. + """ + def __new__(cls, name, bases, attrs): + if attrs.get('has_children', False): + attrs['children'] = List(help='The children of this XModule', default=[], scope=None) + else: + attrs['has_children'] = False + + return super(ParentModelMetaclass, cls).__new__(cls, name, bases, attrs) + + +class NamespaceDescriptor(object): + def __init__(self, namespace): + self._namespace = namespace + + def __get__(self, instance, owner): + if owner is None: + return self + return self._namespace(instance) + + +class Namespace(Plugin): + """ + A baseclass that sets up machinery for ModelType fields that proxies the contained fields + requests for _model_data to self._container._model_data. + """ + __metaclass__ = ModelMetaclass + __slots__ = ['container'] + + entry_point = 'xmodule.namespace' + + def __init__(self, container): + self._container = container + + @property + def _model_data(self): + return self._container._model_data diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index c65c031c9a..c0ba73423c 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -1,6 +1,5 @@ import pymongo import sys -import logging from bson.son import SON from fs.osfs import OSFS @@ -9,8 +8,8 @@ from path import path from importlib import import_module from xmodule.errortracker import null_error_tracker, exc_info_to_str -from xmodule.x_module import XModuleDescriptor from xmodule.mako_module import MakoDescriptorSystem +from xmodule.x_module import XModuleDescriptor from xmodule.error_module import ErrorDescriptor from . import ModuleStoreBase, Location diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 7c6887696e..8463f945a8 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -192,7 +192,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): xmlstore.modules[course_id][descriptor.location] = descriptor if hasattr(descriptor, 'children'): - for child in descriptor.children: + for child in descriptor.get_children(): parent_tracker.add_parent(child.location, descriptor.location) return descriptor @@ -318,8 +318,6 @@ class XMLModuleStore(ModuleStoreBase): # Didn't load course. Instead, save the errors elsewhere. self.errored_courses[course_dir] = errorlog - - def __unicode__(self): ''' String representation - for debugging @@ -345,8 +343,6 @@ class XMLModuleStore(ModuleStoreBase): log.warning(msg + " " + str(err)) return {} - - def load_course(self, course_dir, tracker): """ Load a course into this module store @@ -363,7 +359,7 @@ class XMLModuleStore(ModuleStoreBase): # been imported into the cms from xml course_file = StringIO(clean_out_mako_templating(course_file.read())) - course_data = etree.parse(course_file,parser=edx_xml_parser).getroot() + course_data = etree.parse(course_file, parser=edx_xml_parser).getroot() org = course_data.get('org') diff --git a/common/lib/xmodule/xmodule/plugin.py b/common/lib/xmodule/xmodule/plugin.py new file mode 100644 index 0000000000..5cf9c647aa --- /dev/null +++ b/common/lib/xmodule/xmodule/plugin.py @@ -0,0 +1,64 @@ +import pkg_resources +import logging + +log = logging.getLogger(__name__) + +class PluginNotFoundError(Exception): + pass + + +class Plugin(object): + """ + Base class for a system that uses entry_points to load plugins. + + Implementing classes are expected to have the following attributes: + + entry_point: The name of the entry point to load plugins from + """ + + _plugin_cache = None + + @classmethod + def load_class(cls, identifier, default=None): + """ + Loads a single class instance specified by identifier. If identifier + specifies more than a single class, then logs a warning and returns the + first class identified. + + If default is not None, will return default if no entry_point matching + identifier is found. Otherwise, will raise a ModuleMissingError + """ + if cls._plugin_cache is None: + cls._plugin_cache = {} + + if identifier not in cls._plugin_cache: + identifier = identifier.lower() + classes = list(pkg_resources.iter_entry_points( + cls.entry_point, name=identifier)) + + if len(classes) > 1: + log.warning("Found multiple classes for {entry_point} with " + "identifier {id}: {classes}. " + "Returning the first one.".format( + entry_point=cls.entry_point, + id=identifier, + classes=", ".join( + class_.module_name for class_ in classes))) + + if len(classes) == 0: + if default is not None: + return default + raise PluginNotFoundError(identifier) + + cls._plugin_cache[identifier] = classes[0].load() + return cls._plugin_cache[identifier] + + @classmethod + def load_classes(cls): + """ + Returns a list of containing the identifiers and their corresponding classes for all + of the available instances of this plugin + """ + return [(class_.name, class_.load()) + for class_ + in pkg_resources.iter_entry_points(cls.entry_point)] diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 155ad99480..6e80d1cf61 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -8,6 +8,7 @@ from xmodule.xml_module import XmlDescriptor from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError +from .model import Int, Scope from pkg_resources import resource_string log = logging.getLogger("mitx.common.lib.seq_module") @@ -16,6 +17,12 @@ log = logging.getLogger("mitx.common.lib.seq_module") # OBSOLETE: This obsoletes 'type' class_priority = ['video', 'problem'] +def display_name(module): + if hasattr(module, 'display_name'): + return module.display_name + + if hasattr(module, 'lms'): + return module.lms.display_name class SequenceModule(XModule): ''' Layout module which lays out content in a temporal sequence @@ -26,22 +33,18 @@ class SequenceModule(XModule): css = {'scss': [resource_string(__name__, 'css/sequence/display.scss')]} js_module_name = "Sequence" - def __init__(self, system, location, definition, descriptor, instance_state=None, - shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) - # NOTE: Position is 1-indexed. This is silly, but there are now student - # positions saved on prod, so it's not easy to fix. - self.position = 1 + has_children = True - if instance_state is not None: - state = json.loads(instance_state) - if 'position' in state: - self.position = int(state['position']) + # NOTE: Position is 1-indexed. This is silly, but there are now student + # positions saved on prod, so it's not easy to fix. + position = Int(help="Last tab viewed in this sequence", default=1, scope=Scope.student_state) + + def __init__(self, *args, **kwargs): + XModule.__init__(self, *args, **kwargs) # if position is specified in system, then use that instead - if system.get('position'): - self.position = int(system.get('position')) + if self.system.get('position'): + self.position = int(self.system.get('position')) self.rendered = False @@ -79,9 +82,9 @@ class SequenceModule(XModule): childinfo = { 'content': child.get_html(), 'title': "\n".join( - grand_child.display_name.strip() + display_name(grand_child) for grand_child in child.get_children() - if 'display_name' in grand_child.metadata + if display_name(grand_child) is not None ), 'progress_status': Progress.to_js_status_str(progress), 'progress_detail': Progress.to_js_detail_str(progress), @@ -89,7 +92,7 @@ class SequenceModule(XModule): 'id': child.id, } if childinfo['title']=='': - childinfo['title'] = child.metadata.get('display_name','') + childinfo['title'] = display_name(child) contents.append(childinfo) params = {'items': contents, @@ -116,7 +119,8 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule - stores_state = True # For remembering where in the sequence the student is + has_children = True + stores_state = True # For remembering where in the sequence the student is js = {'coffee': [resource_string(__name__, 'js/src/sequence/edit.coffee')]} js_module_name = "SequenceDescriptor" diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index 397bd3e136..a4cd779285 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -11,8 +11,10 @@ class_priority = ['video', 'problem'] class VerticalModule(XModule): ''' Layout module for laying out submodules vertically.''' - def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) + has_children = True + + def __init__(self, *args, **kwargs): + XModule.__init__(self, *args, **kwargs) self.contents = None def get_html(self): @@ -45,6 +47,8 @@ class VerticalModule(XModule): class VerticalDescriptor(SequenceDescriptor): module_class = VerticalModule + has_children = True + js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} js_module_name = "VerticalDescriptor" diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 8a9e0aadd7..34ce353afd 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -9,6 +9,7 @@ from xmodule.raw_module import RawDescriptor from xmodule.modulestore.mongo import MongoModuleStore from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent +from .model import Int, Scope, String log = logging.getLogger(__name__) @@ -27,22 +28,20 @@ class VideoModule(XModule): css = {'scss': [resource_string(__name__, 'css/video/display.scss')]} js_module_name = "Video" - def __init__(self, system, location, definition, descriptor, - instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, descriptor, - instance_state, shared_state, **kwargs) - xmltree = etree.fromstring(self.definition['data']) + data = String(help="XML data for the problem", scope=Scope.content) + position = Int(help="Current position in the video", scope=Scope.student_state) + display_name = String(help="Display name for this module", scope=Scope.settings) + + def __init__(self, *args, **kwargs): + XModule.__init__(self, *args, **kwargs) + + xmltree = etree.fromstring(self.data) self.youtube = xmltree.get('youtube') self.position = 0 self.show_captions = xmltree.get('show_captions', 'true') self.source = self._get_source(xmltree) self.track = self._get_track(xmltree) - if instance_state is not None: - state = json.loads(instance_state) - if 'position' in state: - self.position = int(float(state['position'])) - def _get_source(self, xmltree): # find the first valid source return self._get_first_external(xmltree, 'source') @@ -102,7 +101,7 @@ class VideoModule(XModule): else: # VS[compat] # cdodge: filesystem static content support. - caption_asset_path = "/static/{0}/subs/".format(self.metadata['data_dir']) + caption_asset_path = "/static/{0}/subs/".format(self.descriptor.data_dir) return self.system.render_template('video.html', { 'streams': self.video_list(), @@ -111,8 +110,6 @@ class VideoModule(XModule): 'source': self.source, 'track' : self.track, 'display_name': self.display_name, - # TODO (cpennington): This won't work when we move to data that isn't on the filesystem - 'data_dir': self.metadata['data_dir'], 'caption_asset_path': caption_asset_path, 'show_captions': self.show_captions }) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 9cf45652ca..9cad93ad5b 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1,5 +1,4 @@ import logging -import pkg_resources import yaml import os import time @@ -10,9 +9,9 @@ from collections import namedtuple from pkg_resources import resource_listdir, resource_string, resource_isdir from xmodule.modulestore import Location -from .model import ModelMetaclass, String, Scope, ModuleScope, ModelType -Date = ModelType +from .model import ModelMetaclass, ParentModelMetaclass, NamespacesMetaclass, ModelType +from .plugin import Plugin class Date(ModelType): @@ -38,6 +37,10 @@ class Date(ModelType): """ return time.strftime(self.time_format, value) + +class XModuleMetaclass(ParentModelMetaclass, NamespacesMetaclass, ModelMetaclass): + pass + log = logging.getLogger('mitx.' + __name__) @@ -45,67 +48,6 @@ def dummy_track(event_type, event): pass -class ModuleMissingError(Exception): - pass - - -class Plugin(object): - """ - Base class for a system that uses entry_points to load plugins. - - Implementing classes are expected to have the following attributes: - - entry_point: The name of the entry point to load plugins from - """ - - _plugin_cache = None - - @classmethod - def load_class(cls, identifier, default=None): - """ - Loads a single class instance specified by identifier. If identifier - specifies more than a single class, then logs a warning and returns the - first class identified. - - If default is not None, will return default if no entry_point matching - identifier is found. Otherwise, will raise a ModuleMissingError - """ - if cls._plugin_cache is None: - cls._plugin_cache = {} - - if identifier not in cls._plugin_cache: - identifier = identifier.lower() - classes = list(pkg_resources.iter_entry_points( - cls.entry_point, name=identifier)) - - if len(classes) > 1: - log.warning("Found multiple classes for {entry_point} with " - "identifier {id}: {classes}. " - "Returning the first one.".format( - entry_point=cls.entry_point, - id=identifier, - classes=", ".join( - class_.module_name for class_ in classes))) - - if len(classes) == 0: - if default is not None: - return default - raise ModuleMissingError(identifier) - - cls._plugin_cache[identifier] = classes[0].load() - return cls._plugin_cache[identifier] - - @classmethod - def load_classes(cls): - """ - Returns a list of containing the identifiers and their corresponding classes for all - of the available instances of this plugin - """ - return [(class_.name, class_.load()) - for class_ - in pkg_resources.iter_entry_points(cls.entry_point)] - - class HTMLSnippet(object): """ A base class defining an interface for an object that is able to present an @@ -179,9 +121,7 @@ class XModule(HTMLSnippet): See the HTML module for a simple example. ''' - __metaclass__ = ModelMetaclass - - display_name = String(help="Display name for this module", scope=Scope(student=False, module=ModuleScope.USAGE)) + __metaclass__ = XModuleMetaclass # The default implementation of get_icon_class returns the icon_class # attribute of the class @@ -244,9 +184,22 @@ class XModule(HTMLSnippet): self.url_name = self.location.name self.category = self.location.category self._model_data = model_data + self._loaded_children = None - if self.display_name is None: - self.display_name = self.url_name.replace('_', ' ') + def get_children(self): + ''' + Return module instances for all the children of this module. + ''' + if not self.has_children: + return [] + + if self._loaded_children is None: + children = [self.system.get_module(loc) for loc in self.children] + # get_module returns None if the current user doesn't have access + # to the location. + self._loaded_children = [c for c in children if c is not None] + + return self._loaded_children def __unicode__(self): return ''.format(self.id) @@ -257,7 +210,7 @@ class XModule(HTMLSnippet): immediately inside this module. ''' items = [] - for child in self.children(): + for child in self.get_children(): items.extend(child.displayable_items()) return items @@ -366,10 +319,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): """ entry_point = "xmodule.v1" module_class = XModule - __metaclass__ = ModelMetaclass + __metaclass__ = XModuleMetaclass - display_name = String(help="Display name for this module", scope=Scope(student=False, module=ModuleScope.USAGE)) - start = Date(help="Start time when this module is visible", scope=Scope(student=False, module=ModuleScope.USAGE)) # Attributes for inspection of the descriptor stores_state = False # Indicates whether the xmodule state should be # stored in a database (independent of shared state) @@ -430,8 +381,6 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): 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 url_name: The name to use for this module in urls and other places where a unique name is needed. format: The format of this module ('Homework', 'Lab', etc) @@ -452,12 +401,36 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): self._child_instances = None self._inherited_metadata = set() + self._child_instances = None + + def get_children(self): + """Returns a list of XModuleDescriptor instances for the children of + this module""" + if not self.has_children: + return [] + + if self._child_instances is None: + self._child_instances = [] + for child_loc in self.children: + try: + child = self.system.load_item(child_loc) + except ItemNotFoundError: + log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc)) + continue + # TODO (vshnayder): this should go away once we have + # proper inheritance support in mongo. The xml + # datastore does all inheritance on course load. + #child.inherit_metadata(self.metadata) + self._child_instances.append(child) + + return self._child_instances + def get_child_by_url_name(self, url_name): """ Return a child XModuleDescriptor with the specified url_name, if it exists, and None otherwise. """ - for c in self.children: + for c in self.get_children(): if c.url_name == url_name: return c return None @@ -472,7 +445,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): system, self.location, self, - system.xmodule_model_data(self.model_data), + system.xmodule_model_data(self._model_data), ) def has_dynamic_children(self): @@ -686,6 +659,7 @@ class ModuleSystem(object): get_module, render_template, replace_urls, + xmodule_model_data, user=None, filestore=None, debug=False, @@ -739,6 +713,7 @@ class ModuleSystem(object): self.node_path = node_path self.anonymous_student_id = anonymous_student_id self.user_is_staff = user is not None and user.is_staff + self.xmodule_model_data = xmodule_model_data def get(self, attr): ''' provide uniform access to attributes (like etree).''' @@ -748,9 +723,6 @@ class ModuleSystem(object): '''provide uniform access to attributes (like etree)''' self.__dict__[attr] = val - def xmodule_module_data(self, module_data): - return module_data - def __repr__(self): return repr(self.__dict__) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 36932f9e42..1283844ade 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -148,7 +148,7 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F format_scores = [] for section in sections: section_descriptor = section['section_descriptor'] - section_name = section_descriptor.metadata.get('display_name') + section_name = section_descriptor.display_name should_grade_section = False # If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0% @@ -276,14 +276,14 @@ def progress_summary(student, request, course, student_module_cache): # Don't include chapters that aren't displayable (e.g. due to error) for chapter_module in course_module.get_display_items(): # Skip if the chapter is hidden - hidden = chapter_module.metadata.get('hide_from_toc','false') + hidden = chapter_module._model_data.get('hide_from_toc','false') if hidden.lower() == 'true': continue sections = [] for section_module in chapter_module.get_display_items(): # Skip if the section is hidden - hidden = section_module.metadata.get('hide_from_toc','false') + hidden = section_module._model_data.get('hide_from_toc','false') if hidden.lower() == 'true': continue diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 9c40767e99..57a125adba 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -88,8 +88,7 @@ def toc_for_course(user, request, course, active_chapter, active_section): chapters = list() for chapter in course_module.get_display_items(): - hide_from_toc = chapter.metadata.get('hide_from_toc','false').lower() == 'true' - if hide_from_toc: + if chapter.lms.hide_from_toc: continue sections = list() @@ -97,18 +96,17 @@ def toc_for_course(user, request, course, active_chapter, active_section): active = (chapter.url_name == active_chapter and section.url_name == active_section) - hide_from_toc = section.metadata.get('hide_from_toc', 'false').lower() == 'true' - if not hide_from_toc: - sections.append({'display_name': section.display_name, + if not section.lms.hide_from_toc: + sections.append({'display_name': section.lms.display_name, 'url_name': section.url_name, - 'format': section.metadata.get('format', ''), - 'due': section.metadata.get('due', ''), + 'format': section.lms.format, + 'due': section.lms.due, 'active': active, - 'graded': section.metadata.get('graded', False), + 'graded': section.lms.graded, }) - chapters.append({'display_name': chapter.display_name, + chapters.append({'display_name': chapter.lms.display_name, 'url_name': chapter.url_name, 'sections': sections, 'active': chapter.url_name == active_chapter}) @@ -146,7 +144,8 @@ def get_module(user, request, location, student_module_cache, course_id, positio log.exception("Error in get_module") return None -def _get_module(user, request, location, student_module_cache, course_id, position=None, wrap_xmodule_display = True): + +def _get_module(user, request, location, student_module_cache, course_id, position=None, wrap_xmodule_display=True): """ Actually implement get_module. See docstring there for details. """ @@ -268,7 +267,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi module.get_html = replace_static_urls( _get_html, - module.metadata['data_dir'] if 'data_dir' in module.metadata else '', + getattr(module, 'data_dir', ''), course_namespace = module.location._replace(category=None, name=None)) # Allow URLs of the form '/course/' refer to the root of multicourse directory diff --git a/lms/setup.py b/lms/setup.py new file mode 100644 index 0000000000..1755b7d37e --- /dev/null +++ b/lms/setup.py @@ -0,0 +1,18 @@ +from setuptools import setup, find_packages + +setup( + name="edX LMS", + version="0.1", + install_requires=['distribute'], + requires=[ + 'xmodule', + ], + + # See http://guide.python-distribute.org/creation.html#entry-points + # for a description of entry_points + entry_points={ + 'xmodule.namespace': [ + 'lms = lms.xmodule_namespace:LmsNamespace' + ], + } +) \ No newline at end of file diff --git a/lms/templates/courseware/welcome-back.html b/lms/templates/courseware/welcome-back.html index 5d4e0fe1e3..389a54aa53 100644 --- a/lms/templates/courseware/welcome-back.html +++ b/lms/templates/courseware/welcome-back.html @@ -1,3 +1,3 @@ -

${chapter_module.display_name}

+

${chapter_module.lms.display_name}

-

You were most recently in ${prev_section.display_name}. If you're done with that, choose another section on the left.

+

You were most recently in ${prev_section.lms.display_name}. If you're done with that, choose another section on the left.

diff --git a/lms/templates/video.html b/lms/templates/video.html index 18c1bcbced..1698ae256c 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -2,7 +2,7 @@

${display_name}

% endif -
+
diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py new file mode 100644 index 0000000000..3673de73df --- /dev/null +++ b/lms/xmodule_namespace.py @@ -0,0 +1,23 @@ +from xmodule.model import Namespace, Boolean, Scope, String +from xmodule.x_module import Date + +class LmsNamespace(Namespace): + hide_from_toc = Boolean( + help="Whether to display this module in the table of contents", + default=False, + scope=Scope.settings + ) + graded = Boolean( + help="Whether this module contributes to the final course grade", + default=False, + scope=Scope.settings + ) + format = String( + help="What format this module is in (used for deciding which " + "grader to apply, and what to show in the TOC)", + scope=Scope.settings + ) + + display_name = String(help="Display name for this module", scope=Scope.settings) + start = Date(help="Start time when this module is visible", scope=Scope.settings) + due = String(help="Date that this problem is due by", scope=Scope.settings, default='') diff --git a/local-requirements.txt b/local-requirements.txt index a4d153dd36..2e951a180c 100644 --- a/local-requirements.txt +++ b/local-requirements.txt @@ -1,3 +1,4 @@ # Python libraries to install that are local to the mitx repo -e common/lib/capa -e common/lib/xmodule +-e lms