From 8ba41635572002b45a5725b42d53e1bdda3096c2 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 7 Dec 2012 12:48:57 -0500 Subject: [PATCH] WIP. Data loads, but not all of it --- cms/djangoapps/contentstore/views.py | 2 +- common/lib/capa/capa/capa_problem.py | 18 +- common/lib/xmodule/xmodule/abtest_module.py | 33 +-- common/lib/xmodule/xmodule/capa_module.py | 154 +++++------- common/lib/xmodule/xmodule/course_module.py | 82 ++++--- common/lib/xmodule/xmodule/error_module.py | 7 +- common/lib/xmodule/xmodule/mako_module.py | 9 +- common/lib/xmodule/xmodule/model.py | 79 +++++++ common/lib/xmodule/xmodule/modulestore/xml.py | 25 +- common/lib/xmodule/xmodule/template_module.py | 4 - common/lib/xmodule/xmodule/x_module.py | 222 +++++------------- common/lib/xmodule/xmodule/xml_module.py | 17 +- jenkins/base.sh | 12 + lms/djangoapps/courseware/courses.py | 8 +- lms/djangoapps/courseware/module_render.py | 4 +- rakefile | 2 +- 16 files changed, 296 insertions(+), 382 deletions(-) create mode 100644 common/lib/xmodule/xmodule/model.py create mode 100644 jenkins/base.sh diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 8f10eadc4b..833662b218 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -499,7 +499,7 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ """ system = preview_module_system(request, preview_id, descriptor) try: - module = descriptor.xmodule_constructor(system)(instance_state, shared_state) + module = descriptor.xmodule(system) except: module = ErrorDescriptor.from_descriptor( descriptor, diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 451891d067..eb39d8a2c6 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -83,7 +83,7 @@ class LoncapaProblem(object): Main class for capa Problems. ''' - def __init__(self, problem_text, id, state=None, seed=None, system=None): + def __init__(self, problem_text, id, correct_map=None, done=None, seed=None, system=None): ''' Initializes capa Problem. @@ -91,7 +91,8 @@ class LoncapaProblem(object): - problem_text (string): xml defining the problem - id (string): identifier for this problem; often a filename (no spaces) - - state (dict): student state + - correct_map (dict): data specifying whether the student has completed the problem + - done (bool): Whether the student has answered the problem - seed (int): random number generator seed (int) - system (ModuleSystem): ModuleSystem instance which provides OS, rendering, and user context @@ -103,16 +104,11 @@ class LoncapaProblem(object): self.problem_id = id self.system = system self.seed = seed + self.done = done + self.correct_map = CorrectMap() - if state: - if 'seed' in state: - self.seed = state['seed'] - if 'student_answers' in state: - self.student_answers = state['student_answers'] - if 'correct_map' in state: - self.correct_map.set_dict(state['correct_map']) - if 'done' in state: - self.done = state['done'] + if correct_map is not None: + self.correct_map.set_dict(correct_map) # TODO: Does this deplete the Linux entropy pool? Is this fast enough? if not self.seed: diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 3091b6ec02..0f655ded6c 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -7,6 +7,7 @@ from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.xml_module import XmlDescriptor from xmodule.exceptions import InvalidDefinitionError +from .model import String, Scope DEFAULT = "_DEFAULT_GROUP" @@ -68,37 +69,7 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor): template_dir_name = "abtest" - def __init__(self, system, definition=None, **kwargs): - """ - definition is a dictionary with the following layout: - {'data': { - 'experiment': 'the name of the experiment', - 'group_portions': { - 'group_a': 0.1, - 'group_b': 0.2 - }, - 'group_contents': { - 'group_a': [ - 'url://for/content/module/1', - 'url://for/content/module/2', - ], - 'group_b': [ - 'url://for/content/module/3', - ], - DEFAULT: [ - 'url://for/default/content/1' - ] - } - }, - 'children': [ - 'url://for/content/module/1', - 'url://for/content/module/2', - 'url://for/content/module/3', - 'url://for/default/content/1', - ]} - """ - kwargs['shared_state_key'] = definition['data']['experiment'] - RawDescriptor.__init__(self, system, definition, **kwargs) + experiment = String(help="Experiment that this A/B test belongs to", scope=Scope.content) @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 9922b1b8a0..6ad6de2be6 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -19,6 +19,9 @@ from progress import Progress from xmodule.x_module import XModule 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") @@ -77,6 +80,17 @@ class CapaModule(XModule): ''' icon_class = 'problem' + 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) + 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) + rerandomize = String(help="When to rerandomize the problem", default="always") + 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) + js = {'coffee': [resource_string(__name__, 'js/src/capa/display.coffee'), resource_string(__name__, 'js/src/collapsible.coffee'), resource_string(__name__, 'js/src/javascript_loader.coffee'), @@ -87,51 +101,15 @@ class CapaModule(XModule): js_module_name = "Problem" css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]} - 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, system, location, descriptor, model_data): + XModule.__init__(self, system, location, descriptor, model_data) - self.attempts = 0 - self.max_attempts = None - - dom2 = etree.fromstring(definition['data']) - - 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 + if self.graceperiod is not None and self.due: + self.close_date = self.due + self.graceperiod #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 = self.metadata.get('attempts', None) - if self.max_attempts is not None: - self.max_attempts = int(self.max_attempts) - - self.show_answer = self.metadata.get('showanswer', 'closed') - - self.force_save_button = self.metadata.get('force_save_button', 'false') - - if self.show_answer == "": - self.show_answer = "closed" - - if instance_state is not None: - instance_state = json.loads(instance_state) - if instance_state is not None and 'attempts' in instance_state: - self.attempts = instance_state['attempts'] - - self.name = only_one(dom2.xpath('/problem/@name')) + self.close_date = self.due if self.rerandomize == 'never': self.seed = 1 @@ -148,8 +126,8 @@ class CapaModule(XModule): try: # TODO (vshnayder): move as much as possible of this work and error # checking to descriptor load time - self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), - instance_state, seed=self.seed, system=self.system) + self.lcp = LoncapaProblem(self.data, self.location.html_id(), + self.correct_map, self.done, self.seed, self.system) except Exception as err: msg = 'cannot create LoncapaProblem {loc}: {err}'.format( loc=self.location.url(), err=err) @@ -168,33 +146,21 @@ class CapaModule(XModule): (self.location.url(), msg)) self.lcp = LoncapaProblem( problem_text, self.location.html_id(), - instance_state, seed=self.seed, system=self.system) + self.correct_map, self.done, self.seed, self.system) else: # add extra info and raise raise Exception(msg), None, sys.exc_info()[2] - @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" - elif rerandomize == "onreset": - return "onreset" - else: - raise Exception("Invalid rerandomize attribute " + rerandomize) + if self.rerandomize in ("", "true"): + self.rerandomize = "always" + elif self.rerandomize == "false": + self.rerandomize = "per_student" - def get_instance_state(self): - state = self.lcp.get_state() - state['attempts'] = self.attempts - return json.dumps(state) + def sync_lcp_state(self): + lcp_state = self.lcp.get_state() + self.done = lcp_state['done'] + self.correct_map = lcp_state['correct_map'] + self.seed = lcp_state['seed'] def get_score(self): return self.lcp.get_score() @@ -211,7 +177,7 @@ class CapaModule(XModule): if total > 0: try: return Progress(score, total) - except Exception as err: + except Exception: log.exception("Got bad progress") return None return None @@ -261,8 +227,8 @@ class CapaModule(XModule): # Next, generate a fresh LoncapaProblem self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), - state=None, # Tabula rasa seed=self.seed, system=self.system) + self.sync_lcp_state() # Prepend a scary warning to the student warning = '
'\ @@ -280,8 +246,8 @@ class CapaModule(XModule): html = warning try: html += self.lcp.get_html() - except Exception, err: # Couldn't do it. Give up - log.exception(err) + except Exception: # Couldn't do it. Give up + log.exception("Unable to generate html from LoncapaProblem") raise content = {'name': self.display_name, @@ -311,7 +277,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.done and self.rerandomize == "always": check_button = False save_button = False @@ -320,7 +286,7 @@ class CapaModule(XModule): reset_button = False # User hasn't submitted an answer yet -- we don't want resets - if not self.lcp.done: + if not self.done: reset_button = False # We may not need a "save" button if infinite number of attempts and @@ -406,7 +372,7 @@ class CapaModule(XModule): return self.attempts > 0 if self.show_answer == 'answered': - return self.lcp.done + return self.done if self.show_answer == 'closed': return self.closed() @@ -429,6 +395,7 @@ class CapaModule(XModule): queuekey = get['queuekey'] score_msg = get['xqueue_body'] self.lcp.update_score(score_msg, queuekey) + self.sync_lcp_state() return dict() # No AJAX return is needed @@ -445,8 +412,9 @@ class CapaModule(XModule): raise NotFoundError('Answer is not available') else: answers = self.lcp.get_question_answers() + self.sync_lcp_state() - # answers (eg ) may have embedded images + # answers (eg ) may have embedded images # but be careful, some problems are using non-string answer dicts new_answers = dict() for answer_id in answers: @@ -512,7 +480,7 @@ class CapaModule(XModule): raise NotFoundError('Problem is closed') # Problem submitted. Student should reset before checking again - if self.lcp.done and self.rerandomize == "always": + if self.done and self.rerandomize == "always": event_info['failure'] = 'unreset' self.system.track_function('save_problem_check_fail', event_info) raise NotFoundError('Problem must be reset before it can be checked again') @@ -522,14 +490,13 @@ class CapaModule(XModule): current_time = datetime.datetime.now() prev_submit_time = self.lcp.get_recentmost_queuetime() waittime_between_requests = self.system.xqueue['waittime'] - if (current_time-prev_submit_time).total_seconds() < waittime_between_requests: + if (current_time - prev_submit_time).total_seconds() < waittime_between_requests: msg = 'You must wait at least %d seconds between submissions' % waittime_between_requests - return {'success': msg, 'html': ''} # Prompts a modal dialog in ajax callback + return {'success': msg, 'html': ''} # Prompts a modal dialog in ajax callback try: - old_state = self.lcp.get_state() - lcp_id = self.lcp.problem_id correct_map = self.lcp.grade_answers(answers) + self.sync_lcp_state() except StudentInputError as inst: log.exception("StudentInputError in capa_module:problem_check") return {'success': inst.message} @@ -554,11 +521,11 @@ class CapaModule(XModule): # 'success' will always be incorrect event_info['correct_map'] = correct_map.get_dict() event_info['success'] = success - event_info['attempts'] = self.attempts + event_info['attempts'] = self.attempts self.system.track_function('save_problem_check', event_info) - if hasattr(self.system,'psychometrics_handler'): # update PsychometricsData using callback - self.system.psychometrics_handler(self.get_instance_state()) + if hasattr(self.system, 'psychometrics_handler'): # update PsychometricsData using callback + self.system.psychometrics_handler(self.get_instance_state()) # render problem into HTML html = self.get_problem_html(encapsulate=False) @@ -589,7 +556,7 @@ class CapaModule(XModule): # Problem submitted. Student should reset before saving # again. - if self.lcp.done and self.rerandomize == "always": + if self.done and self.rerandomize == "always": event_info['failure'] = 'done' self.system.track_function('save_problem_fail', event_info) return {'success': False, @@ -617,7 +584,7 @@ class CapaModule(XModule): return {'success': False, 'error': "Problem is closed"} - if not self.lcp.done: + if not self.done: event_info['failure'] = 'not_done' self.system.track_function('reset_problem_fail', event_info) return {'success': False, @@ -629,9 +596,13 @@ class CapaModule(XModule): # in next line) self.lcp.seed = None - self.lcp = LoncapaProblem(self.definition['data'], - self.location.html_id(), self.lcp.get_state(), - system=self.system) + self.lcp = LoncapaProblem(self.data, + self.location.html_id(), + self.lcp.correct_map, + self.lcp.done, + self.lcp.seed, + self.system) + self.sync_lcp_state() event_info['new_state'] = self.lcp.get_state() self.system.track_function('reset_problem', event_info) @@ -647,6 +618,8 @@ class CapaDescriptor(RawDescriptor): module_class = CapaModule + weight = Float(help="How much to weight this problem by", scope=Scope.settings) + stores_state = True has_score = True template_dir_name = 'problem' @@ -665,12 +638,3 @@ class CapaDescriptor(RawDescriptor): 'problems/' + path[8:], path[8:], ] - - def __init__(self, *args, **kwargs): - super(CapaDescriptor, self).__init__(*args, **kwargs) - - weight_string = self.metadata.get('weight', None) - if weight_string: - self.weight = float(weight_string) - else: - self.weight = None diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 474cec0a45..019a79e7ab 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -12,6 +12,9 @@ import requests import time import copy +from .model import Scope, ModelType, List, String, Object, Boolean + +Date = ModelType log = logging.getLogger(__name__) @@ -21,6 +24,39 @@ edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, class CourseDescriptor(SequenceDescriptor): module_class = SequenceModule + textbooks = List(help="List of pairs of (title, url) for textbooks used in this course", default=[], scope=Scope.content) + wiki_slug = String(help="Slug that points to the wiki for this course", scope=Scope.content) + enrollment_start = Date(help="Date that enrollment for this class is opened", scope=Scope.settings) + enrollment_end = Date(help="Date that enrollment for this class is closed", scope=Scope.settings) + 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) + + info_sidebar_name = String(scope=Scope.settings, default='Course Handouts') + + # An extra property is used rather than the wiki_slug/number because + # there are courses that change the number for different runs. This allows + # courses to share the same css_class across runs even if they have + # different numbers. + # + # TODO get rid of this as soon as possible or potentially build in a robust + # way to add in course-specific styling. There needs to be a discussion + # about the right way to do this, but arjun will address this ASAP. Also + # note that the courseware template needs to change when this is removed. + css_class = String(help="DO NOT USE THIS", scope=Scope.settings) + + # TODO: This is a quick kludge to allow CS50 (and other courses) to + # specify their own discussion forums as external links by specifying a + # "discussion_link" in their policy JSON file. This should later get + # folded in with Syllabus, Course Info, and additional Custom tabs in a + # more sensible framework later. + discussion_link = String(help="DO NOT USE THIS", scope=Scope.settings) + + # TODO: same as above, intended to let internal CS50 hide the progress tab + # until we get grade integration set up. + # Explicit comparison to True because we always want to return a bool. + hide_progress_tab = Boolean(help="DO NOT USE THIS", scope=Scope.settings) + template_dir_name = 'course' class Textbook: @@ -69,10 +105,11 @@ class CourseDescriptor(SequenceDescriptor): return table_of_contents - def __init__(self, system, definition=None, **kwargs): - super(CourseDescriptor, self).__init__(system, definition, **kwargs) + def __init__(self, *args, **kwargs): + super(CourseDescriptor, self).__init__(*args, **kwargs) + self.textbooks = [] - for title, book_url in self.definition['data']['textbooks']: + for title, book_url in self.textbooks: try: self.textbooks.append(self.Textbook(title, book_url)) except: @@ -81,7 +118,8 @@ class CourseDescriptor(SequenceDescriptor): log.exception("Couldn't load textbook ({0}, {1})".format(title, book_url)) continue - self.wiki_slug = self.definition['data']['wiki_slug'] or self.location.course + if self.wiki_slug is None: + self.wiki_slug = self.location.course msg = None if self.start is None: @@ -98,7 +136,7 @@ class CourseDescriptor(SequenceDescriptor): # disable the syllabus content for courses that do not provide a syllabus self.syllabus_present = self.system.resources_fs.exists(path('syllabus')) - self.set_grading_policy(self.definition['data'].get('grading_policy', None)) + self.set_grading_policy(self.grading_policy) def defaut_grading_policy(self): """ @@ -203,7 +241,7 @@ class CourseDescriptor(SequenceDescriptor): # cdodge: import the grading policy information that is on disk and put into the # descriptor 'definition' bucket as a dictionary so that it is persisted in the DB - instance.definition['data']['grading_policy'] = policy + instance.grading_policy = policy # now set the current instance. set_grading_policy() will apply some inheritance rules instance.set_grading_policy(policy) @@ -395,38 +433,14 @@ class CourseDescriptor(SequenceDescriptor): @property def start_date_text(self): - displayed_start = self._try_parse_time('advertised_start') or self.start - return time.strftime("%b %d, %Y", displayed_start) + return time.strftime("%b %d, %Y", self.advertised_start or self.start) @property def end_date_text(self): return time.strftime("%b %d, %Y", self.end) - # An extra property is used rather than the wiki_slug/number because - # there are courses that change the number for different runs. This allows - # courses to share the same css_class across runs even if they have - # different numbers. - # - # TODO get rid of this as soon as possible or potentially build in a robust - # way to add in course-specific styling. There needs to be a discussion - # about the right way to do this, but arjun will address this ASAP. Also - # note that the courseware template needs to change when this is removed. - @property - def css_class(self): - return self.metadata.get('css_class', '') - @property - def info_sidebar_name(self): - return self.metadata.get('info_sidebar_name', 'Course Handouts') - @property - def discussion_link(self): - """TODO: This is a quick kludge to allow CS50 (and other courses) to - specify their own discussion forums as external links by specifying a - "discussion_link" in their policy JSON file. This should later get - folded in with Syllabus, Course Info, and additional Custom tabs in a - more sensible framework later.""" - return self.metadata.get('discussion_link', None) @property def forum_posts_allowed(self): @@ -443,12 +457,6 @@ class CourseDescriptor(SequenceDescriptor): return True - @property - def hide_progress_tab(self): - """TODO: same as above, intended to let internal CS50 hide the progress tab - until we get grade integration set up.""" - # Explicit comparison to True because we always want to return a bool. - return self.metadata.get('hide_progress_tab') == True @property def end_of_course_survey_url(self): diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 65fceb77c7..0f95bcd256 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -74,12 +74,11 @@ class ErrorDescriptor(JSONEditingDescriptor): } # real metadata stays in the content, but add a display name - metadata = {'display_name': 'Error: ' + location.name} + model_data = {'display_name': 'Error: ' + location.name} return ErrorDescriptor( system, - definition, - location=location, - metadata=metadata + location, + model_data, ) def get_context(self): diff --git a/common/lib/xmodule/xmodule/mako_module.py b/common/lib/xmodule/xmodule/mako_module.py index f5f2fae23b..bdf3cb4749 100644 --- a/common/lib/xmodule/xmodule/mako_module.py +++ b/common/lib/xmodule/xmodule/mako_module.py @@ -21,20 +21,19 @@ class MakoModuleDescriptor(XModuleDescriptor): the descriptor as the `module` parameter to that template """ - def __init__(self, system, definition=None, **kwargs): + def __init__(self, system, location, model_data): if getattr(system, 'render_template', None) is None: raise TypeError('{system} must have a render_template function' ' in order to use a MakoDescriptor'.format( system=system)) - super(MakoModuleDescriptor, self).__init__(system, definition, **kwargs) + super(MakoModuleDescriptor, self).__init__(system, location, model_data) def get_context(self): """ Return the context to render the mako template with """ return {'module': self, - 'metadata': self.metadata, - 'editable_metadata_fields' : self.editable_metadata_fields + 'editable_metadata_fields': self.editable_fields } def get_html(self): @@ -44,6 +43,6 @@ class MakoModuleDescriptor(XModuleDescriptor): # cdodge: encapsulate a means to expose "editable" metadata fields (i.e. not internal system metadata) @property def editable_metadata_fields(self): - subset = [name for name in self.metadata.keys() if name not in self.system_metadata_fields] + subset = [field.name for field in self.fields if field.name not in self.system_metadata_fields] return subset diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py new file mode 100644 index 0000000000..b58ffa267a --- /dev/null +++ b/common/lib/xmodule/xmodule/model.py @@ -0,0 +1,79 @@ +from collections import namedtuple + +class ModuleScope(object): + USAGE, DEFINITION, TYPE, ALL = xrange(4) + + +class Scope(namedtuple('ScopeBase', 'student module')): + pass + +Scope.content = Scope(student=False, module=ModuleScope.DEFINITION) +Scope.student_state = Scope(student=True, module=ModuleScope.USAGE) +Scope.settings = Scope(student=True, module=ModuleScope.USAGE) +Scope.student_preferences = Scope(student=True, module=ModuleScope.TYPE) +Scope.student_info = Scope(student=True, module=ModuleScope.ALL) + + +class ModelType(object): + sequence = 0 + + def __init__(self, help=None, default=None, scope=Scope.content): + self._seq = self.sequence + self._name = "unknown" + self.help = help + self.default = default + self.scope = scope + ModelType.sequence += 1 + + @property + def name(self): + return self._name + + def __get__(self, instance, owner): + if instance is None: + return self + + return instance._model_data.get(self.name, self.default) + + def __set__(self, instance, value): + instance._model_data[self.name] = value + + def __delete__(self, instance): + del instance._model_data[self.name] + + def __repr__(self): + return "<{0.__class__.__name} {0.__name__}>".format(self) + + def __lt__(self, other): + return self._seq < other._seq + +Int = Float = Boolean = Object = List = String = Any = ModelType + + +class ModelMetaclass(type): + 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): + v._name = n + fields.append(v) + fields.sort() + attrs['fields'] = fields + + return super(ModelMetaclass, cls).__new__(cls, name, bases, attrs) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index e3ad1fb0dd..7c6887696e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -187,12 +187,13 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): err_msg ) - descriptor.metadata['data_dir'] = course_dir + setattr(descriptor, 'data_dir', course_dir) xmlstore.modules[course_id][descriptor.location] = descriptor - for child in descriptor.get_children(): - parent_tracker.add_parent(child.location, descriptor.location) + if hasattr(descriptor, 'children'): + for child in descriptor.children: + parent_tracker.add_parent(child.location, descriptor.location) return descriptor render_template = lambda: '' @@ -425,14 +426,14 @@ class XMLModuleStore(ModuleStoreBase): # breaks metadata inheritance via get_children(). Instead # (actually, in addition to, for now), we do a final inheritance pass # after we have the course descriptor. - XModuleDescriptor.compute_inherited_metadata(course_descriptor) + #XModuleDescriptor.compute_inherited_metadata(course_descriptor) # now import all pieces of course_info which is expected to be stored # in /info or /info/ self.load_extra_content(system, course_descriptor, 'course_info', self.data_dir / course_dir / 'info', course_dir, url_name) # now import all static tabs which are expected to be stored in - # in /tabs or /tabs/ + # in /tabs or /tabs/ self.load_extra_content(system, course_descriptor, 'static_tab', self.data_dir / course_dir / 'tabs', course_dir, url_name) self.load_extra_content(system, course_descriptor, 'custom_tag_template', self.data_dir / course_dir / 'custom_tags', course_dir, url_name) @@ -444,30 +445,30 @@ class XMLModuleStore(ModuleStoreBase): def load_extra_content(self, system, course_descriptor, category, base_dir, course_dir, url_name): if url_name: - path = base_dir / url_name + path = base_dir / url_name if not os.path.exists(path): path = base_dir - for filepath in glob.glob(path/ '*'): + for filepath in glob.glob(path / '*'): with open(filepath) as f: try: html = f.read().decode('utf-8') # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix slug = os.path.splitext(os.path.basename(filepath))[0] loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) - module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc}) + module = HtmlDescriptor(system, loc, {'data': html}) # VS[compat]: # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them) # from the course policy if category == "static_tab": for tab in course_descriptor.tabs or []: if tab.get('url_slug') == slug: - module.metadata['display_name'] = tab['name'] - module.metadata['data_dir'] = course_dir - self.modules[course_descriptor.id][module.location] = module + module.display_name = tab['name'] + module.data_dir = course_dir + self.modules[course_descriptor.id][module.location] = module except Exception, e: - logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) + logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) system.error_tracker("ERROR: " + str(e)) def get_instance(self, course_id, location, depth=0): diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 07ef2c6511..f14254c011 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -67,10 +67,6 @@ class CustomTagDescriptor(RawDescriptor): return template.render(**params) - def __init__(self, system, definition, **kwargs): - '''Render and save the template for this descriptor instance''' - super(CustomTagDescriptor, self).__init__(system, definition, **kwargs) - @property def rendered_html(self): return self.render_template(self.system, self.definition['data']) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 690f78fd53..9cf45652ca 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -2,19 +2,41 @@ import logging import pkg_resources import yaml import os +import time -from functools import partial from lxml import etree from pprint import pprint from collections import namedtuple from pkg_resources import resource_listdir, resource_string, resource_isdir from xmodule.modulestore import Location -from xmodule.timeparse import parse_time, stringify_time +from .model import ModelMetaclass, String, Scope, ModuleScope, ModelType -from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX -from xmodule.modulestore.exceptions import ItemNotFoundError -import time +Date = ModelType + + +class Date(ModelType): + time_format = "%Y-%m-%dT%H:%M" + + def from_json(self, value): + """ + Parse an optional metadata key containing a time: if present, complain + if it doesn't parse. + Return None if not present or invalid. + """ + try: + return time.strptime(value, self.time_format) + except ValueError as e: + msg = "Field {0} has bad value '{1}': '{2}'".format( + self._name, value, e) + log.warning(msg) + return None + + def to_json(self, value): + """ + Convert a time struct to a string + """ + return time.strftime(self.time_format, value) log = logging.getLogger('mitx.' + __name__) @@ -157,6 +179,10 @@ 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)) + # The default implementation of get_icon_class returns the icon_class # attribute of the class # @@ -165,8 +191,7 @@ class XModule(HTMLSnippet): # in the module icon_class = 'other' - def __init__(self, system, location, definition, descriptor, - instance_state=None, shared_state=None, **kwargs): + def __init__(self, system, location, descriptor, model_data): ''' Construct a new xmodule @@ -214,63 +239,25 @@ class XModule(HTMLSnippet): ''' self.system = system self.location = Location(location) - self.definition = definition self.descriptor = descriptor - self.instance_state = instance_state - self.shared_state = shared_state self.id = self.location.url() self.url_name = self.location.name self.category = self.location.category - self.metadata = kwargs.get('metadata', {}) - self._loaded_children = None + self._model_data = model_data - @property - def display_name(self): - ''' - Return a display name for the module: use display_name if defined in - metadata, otherwise convert the url name. - ''' - return self.metadata.get('display_name', - self.url_name.replace('_', ' ')) + if self.display_name is None: + self.display_name = self.url_name.replace('_', ' ') def __unicode__(self): return ''.format(self.id) - def get_children(self): - ''' - Return module instances for all the children of this module. - ''' - if self._loaded_children is None: - child_locations = self.get_children_locations() - children = [self.system.get_module(loc) for loc in child_locations] - # 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 get_children_locations(self): - ''' - Returns the locations of each of child modules. - - Overriding this changes the behavior of get_children and - anything that uses get_children, such as get_display_items. - - This method will not instantiate the modules of the children - unless absolutely necessary, so it is cheaper to call than get_children - - These children will be the same children returned by the - descriptor unless descriptor.has_dynamic_children() is true. - ''' - return self.definition.get('children', []) - def get_display_items(self): ''' Returns a list of descendent module instances that will display immediately inside this module. ''' items = [] - for child in self.get_children(): + for child in self.children(): items.extend(child.displayable_items()) return items @@ -290,18 +277,6 @@ class XModule(HTMLSnippet): ### Functions used in the LMS - def get_instance_state(self): - ''' State of the object, as stored in the database - ''' - 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. ''' @@ -391,7 +366,10 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): """ entry_point = "xmodule.v1" module_class = XModule + __metaclass__ = ModelMetaclass + 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) @@ -424,8 +402,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): # ============================= STRUCTURAL MANIPULATION =================== def __init__(self, system, - definition=None, - **kwargs): + location, + model_data): """ Construct a new XModuleDescriptor. The only required arguments are the system, used for interaction with external resources, and the @@ -467,116 +445,36 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): instance of the module data """ self.system = system - self.metadata = kwargs.get('metadata', {}) - self.definition = definition if definition is not None else {} - self.location = Location(kwargs.get('location')) + self.location = Location(location) self.url_name = self.location.name self.category = self.location.category - self.shared_state_key = kwargs.get('shared_state_key') + self._model_data = model_data self._child_instances = None self._inherited_metadata = set() - @property - def display_name(self): - ''' - Return a display name for the module: use display_name if defined in - metadata, otherwise convert the url name. - ''' - return self.metadata.get('display_name', - self.url_name.replace('_', ' ')) - - @property - def start(self): - """ - If self.metadata contains start, return it. Else return None. - """ - if 'start' not in self.metadata: - return None - return self._try_parse_time('start') - - @start.setter - def start(self, value): - if isinstance(value, time.struct_time): - self.metadata['start'] = stringify_time(value) - - @property - def own_metadata(self): - """ - Return the metadata that is not inherited, but was defined on this module. - """ - return dict((k, v) for k, v in self.metadata.items() - if k not in self._inherited_metadata) - - @staticmethod - def compute_inherited_metadata(node): - """Given a descriptor, traverse all of its descendants and do metadata - inheritance. Should be called on a CourseDescriptor after importing a - course. - - NOTE: This means that there is no such thing as lazy loading at the - moment--this accesses all the children.""" - for c in node.get_children(): - c.inherit_metadata(node.metadata) - XModuleDescriptor.compute_inherited_metadata(c) - - 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._inherited_metadata.add(attr) - 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 = [] - for child_loc in self.definition.get('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.get_children(): + for c in self.children: if c.url_name == url_name: return c return None - def xmodule_constructor(self, system): + def xmodule(self, system): """ Returns a constructor for an XModule. This constructor takes two arguments: instance_state and shared_state, and returns a fully instantiated XModule """ - return partial( - self.module_class, + return self.module_class( system, self.location, - self.definition, self, - metadata=self.metadata + system.xmodule_model_data(self.model_data), ) - def has_dynamic_children(self): """ Returns True if this descriptor has dynamic children for a given @@ -701,31 +599,14 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): return eq def __repr__(self): - return ("{class_}({system!r}, {definition!r}, location={location!r}," - " metadata={metadata!r})".format( + return ("{class_}({system!r}, location={location!r}," + " model_data={model_data!r})".format( class_=self.__class__.__name__, system=self.system, - definition=self.definition, location=self.location, - metadata=self.metadata + model_data=self._model_data, )) - # ================================ Internal helpers ======================= - - def _try_parse_time(self, key): - """ - Parse an optional metadata key containing a time: if present, complain - if it doesn't parse. - Return None if not present or invalid. - """ - if key in self.metadata: - try: - return parse_time(self.metadata[key]) - except ValueError as e: - msg = "Descriptor {0} loaded with a bad metadata key '{1}': '{2}'".format( - self.location.url(), self.metadata[key], e) - log.warning(msg) - return None class DescriptorSystem(object): @@ -867,6 +748,9 @@ 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/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 38fcaddd20..36bea6edb2 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -287,9 +287,9 @@ class XmlDescriptor(XModuleDescriptor): filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name)) definition_xml = cls.load_file(filepath, system.resources_fs, location) else: - definition_xml = xml_object # this is just a pointer, not the real definition content + definition_xml = xml_object # this is just a pointer, not the real definition content - definition = cls.load_definition(definition_xml, system, location) # note this removes metadata + definition = cls.load_definition(definition_xml, system, location) # note this removes metadata # VS[compat] -- make Ike's github preview links work in both old and # new file layouts if is_pointer_tag(xml_object): @@ -299,13 +299,13 @@ class XmlDescriptor(XModuleDescriptor): metadata = cls.load_metadata(definition_xml) # move definition metadata into dict - dmdata = definition.get('definition_metadata','') + dmdata = definition.get('definition_metadata', '') if dmdata: metadata['definition_metadata_raw'] = dmdata try: metadata.update(json.loads(dmdata)) except Exception as err: - log.debug('Error %s in loading metadata %s' % (err,dmdata)) + log.debug('Error %s in loading metadata %s' % (err, dmdata)) metadata['definition_metadata_err'] = str(err) # Set/override any metadata specified by policy @@ -313,11 +313,14 @@ class XmlDescriptor(XModuleDescriptor): if k in system.policy: cls.apply_policy(metadata, system.policy[k]) + model_data = {} + model_data.update(metadata) + model_data.update(definition) + return cls( system, - definition, - location=location, - metadata=metadata, + location, + model_data, ) @classmethod diff --git a/jenkins/base.sh b/jenkins/base.sh new file mode 100644 index 0000000000..c7175e6e52 --- /dev/null +++ b/jenkins/base.sh @@ -0,0 +1,12 @@ + +function github_status { + gcli status create mitx mitx $GIT_COMMIT \ + --params=$1 \ + target_url:$BUILD_URL \ + description:"Build #$BUILD_NUMBER is running" \ + -f csv +} + +function github_mark_failed_on_exit { + trap '[ $? == "0" ] || github_status state:failed' EXIT +} \ No newline at end of file diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 643382b485..5312a228a4 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -85,7 +85,7 @@ def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" if isinstance(modulestore(), XMLModuleStore): - path = course.metadata['data_dir'] + "/images/course_image.jpg" + path = course.data_dir + "/images/course_image.jpg" return try_staticfiles_lookup(path) else: loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') @@ -162,7 +162,9 @@ def get_course_about_section(course, section_key): key=section_key, url=course.location.url())) return None elif section_key == "title": - return course.metadata.get('display_name', course.url_name) + if course.display_name is None: + return course.url_name + return course.display_name elif section_key == "university": return course.location.org elif section_key == "number": @@ -220,7 +222,7 @@ def get_course_syllabus_section(course, section_key): filepath = find_file(fs, dirs, section_key + ".html") with fs.open(filepath) as htmlFile: return replace_urls(htmlFile.read().decode('utf-8'), - course.metadata['data_dir'], course_namespace=course.location) + course.data_dir, course_namespace=course.location) except ResourceNotFoundError: log.exception("Missing syllabus section {key} in course {url}".format( key=section_key, url=course.location.url())) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 9343301fb7..9c40767e99 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -245,7 +245,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi make_psychometrics_data_update_handler(instance_module)) try: - module = descriptor.xmodule_constructor(system)(instance_state, shared_state) + module = descriptor.xmodule(system) except: log.exception("Error creating module from descriptor {0}".format(descriptor)) @@ -259,7 +259,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi error_msg=exc_info_to_str(sys.exc_info())) # Make an error module - return err_descriptor.xmodule_constructor(system)(None, None) + return err_descriptor.xmodule(system) _get_html = module.get_html diff --git a/rakefile b/rakefile index ca20de9a39..adf16cc462 100644 --- a/rakefile +++ b/rakefile @@ -40,7 +40,7 @@ end def django_admin(system, env, command, *args) django_admin = ENV['DJANGO_ADMIN_PATH'] || select_executable('django-admin.py', 'django-admin') - return "#{django_admin} #{command} --settings=#{system}.envs.#{env} --pythonpath=. #{args.join(' ')}" + return "#{django_admin} #{command} --traceback --settings=#{system}.envs.#{env} --pythonpath=. #{args.join(' ')}" end def django_for_jasmine(system, django_reload)