+
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
From c5e3380b711201f8e389d6b4653d37925a97d39d Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Tue, 11 Dec 2012 13:36:23 -0500
Subject: [PATCH 0012/1383] WIP: Save student state via StudentModule.
Inheritance doesn't work
---
common/djangoapps/xmodule_modifiers.py | 17 +--
common/lib/capa/capa/capa_problem.py | 20 ++-
common/lib/xmodule/xmodule/capa_module.py | 86 ++++++-----
common/lib/xmodule/xmodule/model.py | 9 +-
common/lib/xmodule/xmodule/modulestore/xml.py | 2 +-
lms/djangoapps/courseware/models.py | 3 +
lms/djangoapps/courseware/module_render.py | 135 ++++++++++--------
lms/templates/staff_problem_info.html | 5 +-
lms/xmodule_namespace.py | 6 +-
9 files changed, 167 insertions(+), 116 deletions(-)
diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py
index 81f0205c3e..5a13582a2d 100644
--- a/common/djangoapps/xmodule_modifiers.py
+++ b/common/djangoapps/xmodule_modifiers.py
@@ -108,36 +108,33 @@ def add_histogram(get_html, module, user):
# TODO (ichuang): Remove after fall 2012 LMS migration done
if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'):
- [filepath, filename] = module.definition.get('filename', ['', None])
+ [filepath, filename] = module.lms.filename
osfs = module.system.filestore
if filename is not None and osfs.exists(filename):
# if original, unmangled filename exists then use it (github
# doesn't like symlinks)
filepath = filename
data_dir = osfs.root_path.rsplit('/')[-1]
- giturl = module.metadata.get('giturl','https://github.com/MITx')
- edit_link = "%s/%s/tree/master/%s" % (giturl,data_dir,filepath)
+ edit_link = "%s/%s/tree/master/%s" % (module.lms.giturl, data_dir, filepath)
else:
edit_link = False
# Need to define all the variables that are about to be used
- giturl = ""
data_dir = ""
- source_file = module.metadata.get('source_file','') # source used to generate the problem XML, eg latex or word
+ source_file = module.lms.source_file # source used to generate the problem XML, eg latex or word
# useful to indicate to staff if problem has been released or not
# TODO (ichuang): use _has_access_descriptor.can_load in lms.courseware.access, instead of now>mstart comparison here
now = time.gmtime()
is_released = "unknown"
- mstart = getattr(module.descriptor,'start')
+ mstart = getattr(module.descriptor.lms,'start')
if mstart is not None:
is_released = "Yes!" if (now > mstart) else "Not yet"
- staff_context = {'definition': module.definition.get('data'),
- 'metadata': json.dumps(module.metadata, indent=4),
+ staff_context = {'fields': [(field.name, getattr(module, field.name)) for field in module.fields],
'location': module.location,
- 'xqa_key': module.metadata.get('xqa_key',''),
+ 'xqa_key': module.lms.xqa_key,
'source_file' : source_file,
- 'source_url': '%s/%s/tree/master/%s' % (giturl,data_dir,source_file),
+ 'source_url': '%s/%s/tree/master/%s' % (module.lms.giturl, data_dir, source_file),
'category': str(module.__class__.__name__),
# Template uses element_id in js function names, so can't allow dashes
'element_id': module.location.html_id().replace('-','_'),
diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py
index eb39d8a2c6..85670063c5 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, correct_map=None, done=None, seed=None, system=None):
+ def __init__(self, problem_text, id, state=None, seed=None, system=None):
'''
Initializes capa Problem.
@@ -91,8 +91,7 @@ class LoncapaProblem(object):
- problem_text (string): xml defining the problem
- id (string): identifier for this problem; often a filename (no spaces)
- - correct_map (dict): data specifying whether the student has completed the problem
- - done (bool): Whether the student has answered the problem
+ - state (dict): student state
- seed (int): random number generator seed (int)
- system (ModuleSystem): ModuleSystem instance which provides OS,
rendering, and user context
@@ -103,12 +102,19 @@ class LoncapaProblem(object):
self.do_reset()
self.problem_id = id
self.system = system
+ if self.system is None:
+ raise Exception()
self.seed = seed
- self.done = done
- self.correct_map = CorrectMap()
- if correct_map is not None:
- self.correct_map.set_dict(correct_map)
+ 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']
# TODO: Does this deplete the Linux entropy pool? Is this fast enough?
if not self.seed:
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index acb4b63e1c..7e8ad9210f 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -92,12 +92,14 @@ class CapaModule(XModule):
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)
+ force_save_button = Boolean(help="Whether to force the save button to appear on the page", scope=Scope.settings, default=False)
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)
+ correct_map = Object(help="Dictionary with the correctness of current student answers", scope=Scope.student_state, default={})
+ student_answers = Object(help="Dictionary with the current student responses", 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)
+ seed = Int(help="Random seed for this student", scope=Scope.student_state)
js = {'coffee': [resource_string(__name__, 'js/src/capa/display.coffee'),
resource_string(__name__, 'js/src/collapsible.coffee'),
@@ -124,23 +126,23 @@ class CapaModule(XModule):
else:
self.close_date = self.due
- if self.rerandomize == 'never':
- self.seed = 1
- elif self.rerandomize == "per_student" and hasattr(self.system, 'id'):
- # TODO: This line is badly broken:
- # (1) We're passing student ID to xmodule.
- # (2) There aren't bins of students. -- we only want 10 or 20 randomizations, and want to assign students
- # to these bins, and may not want cohorts. So e.g. hash(your-id, problem_id) % num_bins.
- # - analytics really needs small number of bins.
- self.seed = system.id
- else:
- self.seed = None
+ if self.seed is None:
+ if self.rerandomize == 'never':
+ self.seed = 1
+ elif self.rerandomize == "per_student" and hasattr(self.system, 'id'):
+ # TODO: This line is badly broken:
+ # (1) We're passing student ID to xmodule.
+ # (2) There aren't bins of students. -- we only want 10 or 20 randomizations, and want to assign students
+ # to these bins, and may not want cohorts. So e.g. hash(your-id, problem_id) % num_bins.
+ # - analytics really needs small number of bins.
+ self.seed = system.id
+ else:
+ self.seed = None
try:
# TODO (vshnayder): move as much as possible of this work and error
# checking to descriptor load time
- self.lcp = LoncapaProblem(self.data, self.location.html_id(),
- self.correct_map, self.done, self.seed, self.system)
+ self.lcp = self.new_lcp(self.get_state_for_lcp())
except Exception as err:
msg = 'cannot create LoncapaProblem {loc}: {err}'.format(
loc=self.location.url(), err=err)
@@ -157,9 +159,7 @@ class CapaModule(XModule):
problem_text = (''
'Problem %s has an error:%s' %
(self.location.url(), msg))
- self.lcp = LoncapaProblem(
- problem_text, self.location.html_id(),
- self.correct_map, self.done, self.seed, self.system)
+ self.lcp = self.new_lcp(self.get_state_for_lcp(), text=problem_text)
else:
# add extra info and raise
raise Exception(msg), None, sys.exc_info()[2]
@@ -169,10 +169,30 @@ class CapaModule(XModule):
elif self.rerandomize == "false":
self.rerandomize = "per_student"
- def sync_lcp_state(self):
+ def new_lcp(self, state, text=None):
+ if text is None:
+ text = self.data
+
+ return LoncapaProblem(
+ problem_text=text,
+ id=self.location.html_id(),
+ state=state,
+ system=self.system,
+ )
+
+ def get_state_for_lcp(self):
+ return {
+ 'done': self.done,
+ 'correct_map': self.correct_map,
+ 'student_answers': self.student_answers,
+ 'seed': self.seed,
+ }
+
+ def set_state_from_lcp(self):
lcp_state = self.lcp.get_state()
self.done = lcp_state['done']
self.correct_map = lcp_state['correct_map']
+ self.student_answers = lcp_state['student_answers']
self.seed = lcp_state['seed']
def get_score(self):
@@ -239,9 +259,8 @@ class CapaModule(XModule):
student_answers.pop(answer_id)
# Next, generate a fresh LoncapaProblem
- self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(),
- seed=self.seed, system=self.system)
- self.sync_lcp_state()
+ self.lcp = self.new_lcp(None)
+ self.set_state_from_lcp()
# Prepend a scary warning to the student
warning = ''\
@@ -305,7 +324,7 @@ class CapaModule(XModule):
# We may not need a "save" button if infinite number of attempts and
# non-randomized. The problem author can force it. It's a bit weird for
# randomization to control this; should perhaps be cleaned up.
- if (self.force_save_button == "false") and (self.max_attempts is None and self.rerandomize != "always"):
+ if (not self.force_save_button) and (self.max_attempts is None and self.rerandomize != "always"):
save_button = False
context = {'problem': content,
@@ -326,7 +345,7 @@ class CapaModule(XModule):
id=self.location.html_id(), ajax_url=self.system.ajax_url) + html + "
"
# now do the substitutions which are filesystem based, e.g. '/static/' prefixes
- return self.system.replace_urls(html, self.metadata['data_dir'], course_namespace=self.location)
+ return self.system.replace_urls(html, self.descriptor.data_dir, course_namespace=self.location)
def handle_ajax(self, dispatch, get):
'''
@@ -408,7 +427,7 @@ class CapaModule(XModule):
queuekey = get['queuekey']
score_msg = get['xqueue_body']
self.lcp.update_score(score_msg, queuekey)
- self.sync_lcp_state()
+ self.set_state_from_lcp()
return dict() # No AJAX return is needed
@@ -425,14 +444,18 @@ class CapaModule(XModule):
raise NotFoundError('Answer is not available')
else:
answers = self.lcp.get_question_answers()
- self.sync_lcp_state()
+ self.set_state_from_lcp()
# 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:
try:
+<<<<<<< HEAD
new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.metadata['data_dir'], course_namespace=self.location)}
+=======
+ new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.descriptor.data_dir)}
+>>>>>>> WIP: Save student state via StudentModule. Inheritance doesn't work
except TypeError:
log.debug('Unable to perform URL substitution on answers[%s]: %s' % (answer_id, answers[answer_id]))
new_answer = {answer_id: answers[answer_id]}
@@ -509,7 +532,7 @@ class CapaModule(XModule):
try:
correct_map = self.lcp.grade_answers(answers)
- self.sync_lcp_state()
+ self.set_state_from_lcp()
except StudentInputError as inst:
log.exception("StudentInputError in capa_module:problem_check")
return {'success': inst.message}
@@ -609,13 +632,8 @@ class CapaModule(XModule):
# in next line)
self.lcp.seed = None
- 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()
+ self.set_state_from_lcp()
+ self.lcp = self.new_lcp()
event_info['new_state'] = self.lcp.get_state()
self.system.track_function('reset_problem', event_info)
diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py
index edd94f14a9..9286b0337e 100644
--- a/common/lib/xmodule/xmodule/model.py
+++ b/common/lib/xmodule/xmodule/model.py
@@ -10,8 +10,8 @@ class Scope(namedtuple('ScopeBase', 'student module')):
pass
Scope.content = Scope(student=False, module=ModuleScope.DEFINITION)
+Scope.settings = Scope(student=False, module=ModuleScope.USAGE)
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)
@@ -54,7 +54,7 @@ class ModelType(object):
del instance._model_data[self.name]
def __repr__(self):
- return "<{0.__class__.__name} {0.__name__}>".format(self)
+ return "<{0.__class__.__name__} {0._name}>".format(self)
def __lt__(self, other):
return self._seq < other._seq
@@ -100,9 +100,12 @@ class NamespacesMetaclass(type):
the instance
"""
def __new__(cls, name, bases, attrs):
+ namespaces = []
for ns_name, namespace in Namespace.load_classes():
if issubclass(namespace, Namespace):
attrs[ns_name] = NamespaceDescriptor(namespace)
+ namespaces.append(ns_name)
+ attrs['namespaces'] = namespaces
return super(NamespacesMetaclass, cls).__new__(cls, name, bases, attrs)
@@ -114,7 +117,7 @@ class ParentModelMetaclass(type):
"""
def __new__(cls, name, bases, attrs):
if attrs.get('has_children', False):
- attrs['children'] = List(help='The children of this XModule', default=[], scope=None)
+ attrs['children'] = List(help='The children of this XModule', default=[], scope=Scope.settings)
else:
attrs['has_children'] = False
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 8463f945a8..349975ea77 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -175,7 +175,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
# Normally, we don't want lots of exception traces in our logs from common
# content problems. But if you're debugging the xml loading code itself,
# uncomment the next line.
- # log.exception(msg)
+ log.exception(msg)
self.error_tracker(msg)
err_msg = msg + "\n" + exc_info_to_str(sys.exc_info())
diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py
index ffc7c929de..2b7b12ac45 100644
--- a/lms/djangoapps/courseware/models.py
+++ b/lms/djangoapps/courseware/models.py
@@ -64,6 +64,9 @@ class StudentModule(models.Model):
return '/'.join([self.course_id, self.module_type,
self.student.username, self.module_state_key, str(self.state)[:20]])
+ def __repr__(self):
+ return 'StudentModule%r' % ((self.course_id, self.module_type, self.student, self.module_state_key, str(self.state)[:20]),)
+
# TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 57a125adba..f83ff35f1f 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -11,6 +11,8 @@ from django.http import Http404
from django.http import HttpResponse
from django.views.decorators.csrf import csrf_exempt
+from collections import namedtuple
+
from requests.auth import HTTPBasicAuth
from capa.xqueue_interface import XQueueInterface
@@ -26,6 +28,8 @@ from xmodule.modulestore import Location
from xmodule.modulestore.django import modulestore
from xmodule.x_module import ModuleSystem
from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
+from xmodule.runtime import DbModel, KeyValueStore
+from xmodule.model import Scope
from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule
from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -145,6 +149,70 @@ def get_module(user, request, location, student_module_cache, course_id, positio
return None
+class LmsKeyValueStore(KeyValueStore):
+ def __init__(self, course_id, user, descriptor_model_data, student_module_cache):
+ self._course_id = course_id
+ self._user = user
+ self._descriptor_model_data = descriptor_model_data
+ self._student_module_cache = student_module_cache
+
+ def _student_module(self, key):
+ student_module = self._student_module_cache.lookup(
+ self._course_id, key.module_scope_id.category, key.module_scope_id.url()
+ )
+ return student_module
+
+ def get(self, key):
+ if not key.scope.student:
+ return self._descriptor_model_data[key.field_name]
+
+ if key.scope == Scope.student_state:
+ student_module = self._student_module(key)
+
+ if student_module is None:
+ raise KeyError(key.field_name)
+
+ return json.loads(student_module.state)[key.field_name]
+
+ def set(self, key, value):
+ if not key.scope.student:
+ self._descriptor_model_data[key.field_name] = value
+
+ if key.scope == Scope.student_state:
+ student_module = self._student_module(key)
+ if student_module is None:
+ student_module = StudentModule(
+ course_id=self._course_id,
+ student=self._user,
+ module_type=key.module_scope_id.category,
+ module_state_key=key.module_scope_id,
+ state=json.dumps({})
+ )
+ self._student_module_cache.append(student_module)
+ state = json.loads(student_module.state)
+ state[key.field_name] = value
+ student_module.state = json.dumps(state)
+ student_module.save()
+
+ def delete(self, key):
+ if not key.scope.student:
+ del self._descriptor_model_data[key.field_name]
+
+ if key.scope == Scope.student_state:
+ student_module = self._student_module(key)
+
+ if student_module is None:
+ raise KeyError(key.field_name)
+
+ state = json.loads(student_module.state)
+ del state[key.field_name]
+ student_module.state = json.dumps(state)
+ student_module.save()
+
+
+LmsUsage = namedtuple('LmsUsage', 'id, def_id')
+
+
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.
@@ -162,23 +230,6 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
h.update(str(user.id))
anonymous_student_id = h.hexdigest()
- # Only check the cache if this module can possibly have state
- instance_module = None
- shared_module = None
- if user.is_authenticated():
- if descriptor.stores_state:
- instance_module = student_module_cache.lookup(
- course_id, 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(course_id,
- descriptor.category,
- shared_state_key)
-
- 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 = reverse('modx_dispatch',
kwargs=dict(course_id=course_id,
@@ -218,6 +269,14 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
return get_module(user, request, location,
student_module_cache, course_id, position)
+ def xmodule_model_data(descriptor_model_data):
+ return DbModel(
+ LmsKeyValueStore(course_id, user, descriptor_model_data, student_module_cache),
+ descriptor.module_class,
+ user.id,
+ LmsUsage(location, location)
+ )
+
# TODO (cpennington): When modules are shared between courses, the static
# prefix is going to have to be specific to the module, not the directory
# that the xml was loaded from
@@ -235,6 +294,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
replace_urls=replace_urls,
node_path=settings.NODE_PATH,
anonymous_student_id=anonymous_student_id,
+ xmodule_model_data=xmodule_model_data
)
# pass position specified in URL to module through ModuleSystem
system.set('position', position)
@@ -453,19 +513,6 @@ def modx_dispatch(request, dispatch, location, course_id):
log.debug("No module {0} for user {1}--access denied?".format(location, user))
raise Http404
- instance_module = get_instance_module(course_id, request.user, instance, student_module_cache)
- shared_module = get_shared_instance_module(course_id, request.user, instance, student_module_cache)
-
- # Don't track state for anonymous users (who don't have student modules)
- if instance_module is not None:
- oldgrade = instance_module.grade
- # The max grade shouldn't change under normal circumstances, but
- # sometimes the problem changes with the same name but a new max grade.
- # This updates the module if that happens.
- old_instance_max_grade = instance_module.max_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:
ajax_return = instance.handle_ajax(dispatch, p)
@@ -476,34 +523,6 @@ def modx_dispatch(request, dispatch, location, course_id):
log.exception("error processing ajax call")
raise
- # Save the state back to the database
- # Don't track state for anonymous users (who don't have student modules)
- if instance_module is not None:
- instance_module.state = instance.get_instance_state()
- instance_module.max_grade=instance.max_score()
- if instance.get_score():
- instance_module.grade = instance.get_score()['score']
- if (instance_module.grade != oldgrade or
- instance_module.state != old_instance_state or
- instance_module.max_grade != old_instance_max_grade):
- instance_module.save()
-
- #Bin score into range and increment stats
- score_bucket=get_score_bucket(instance_module.grade, instance_module.max_grade)
- org, course_num, run=course_id.split("/")
- statsd.increment("lms.courseware.question_answered",
- tags=["org:{0}".format(org),
- "course:{0}".format(course_num),
- "run:{0}".format(run),
- "score_bucket:{0}".format(score_bucket),
- "type:ajax"])
-
-
- 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/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html
index 21044c1f80..ad4852335f 100644
--- a/lms/templates/staff_problem_info.html
+++ b/lms/templates/staff_problem_info.html
@@ -46,8 +46,9 @@ github = ${edit_link | h}
%if source_file:
source_url = ${source_file | h}
%endif
-definition = ${definition | h}
-metadata = ${metadata | h}
+%for name, field in fields:
+${name} = ${field | h}
+%endfor
category = ${category | h}
%if render_histogram:
diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py
index 3673de73df..80b5fc6e61 100644
--- a/lms/xmodule_namespace.py
+++ b/lms/xmodule_namespace.py
@@ -1,4 +1,4 @@
-from xmodule.model import Namespace, Boolean, Scope, String
+from xmodule.model import Namespace, Boolean, Scope, String, List
from xmodule.x_module import Date
class LmsNamespace(Namespace):
@@ -21,3 +21,7 @@ class LmsNamespace(Namespace):
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='')
+ filename = List(help="DO NOT USE", scope=Scope.content, default=['', None])
+ source_file = String(help="DO NOT USE", scope=Scope.settings)
+ giturl = String(help="DO NOT USE", scope=Scope.settings, default='https://github.com/MITx')
+ xqa_key = String(help="DO NOT USE", scope=Scope.settings)
From 25754b50ff0c8d737e39b776822f89869867b9ab Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Wed, 12 Dec 2012 09:22:26 -0500
Subject: [PATCH 0013/1383] WIP. Trying to fix inheritance
---
common/lib/xmodule/xmodule/modulestore/xml.py | 27 +++++-
common/lib/xmodule/xmodule/runtime.py | 92 +++++++++++++++++++
common/lib/xmodule/xmodule/x_module.py | 5 -
3 files changed, 118 insertions(+), 6 deletions(-)
create mode 100644 common/lib/xmodule/xmodule/runtime.py
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 349975ea77..677b2e938e 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -422,7 +422,32 @@ 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)
+ def compute_inherited_metadata(descriptor):
+ """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 child in descriptor.get_children():
+ inherit_metadata(child, descriptor.metadata)
+ compute_inherited_metadata(child)
+
+ def inherit_metadata(descriptor, 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]
+
+
+ compute_inherited_metadata(course_descriptor)
# now import all pieces of course_info which is expected to be stored
# in /info or /info/
diff --git a/common/lib/xmodule/xmodule/runtime.py b/common/lib/xmodule/xmodule/runtime.py
new file mode 100644
index 0000000000..8be4bfe346
--- /dev/null
+++ b/common/lib/xmodule/xmodule/runtime.py
@@ -0,0 +1,92 @@
+from collections import MutableMapping, namedtuple
+
+from .model import ModuleScope, ModelType
+
+
+class KeyValueStore(object):
+ """The abstract interface for Key Value Stores."""
+
+ # Keys are structured to retain information about the scope of the data.
+ # Stores can use this information however they like to store and retrieve
+ # data.
+ Key = namedtuple("Key", "scope, student_id, module_scope_id, field_name")
+
+ def get(key):
+ pass
+
+ def set(key, value):
+ pass
+
+ def delete(key):
+ pass
+
+
+class DbModel(MutableMapping):
+ """A dictionary-like interface to the fields on a module."""
+
+ def __init__(self, kvs, module_cls, student_id, usage):
+ self._kvs = kvs
+ self._student_id = student_id
+ self._module_cls = module_cls
+ self._usage = usage
+
+ def __repr__(self):
+ return "<{0.__class__.__name__} {0._module_cls!r}>".format(self)
+
+ def __str__(self):
+ return str(dict(self.iteritems()))
+
+ def _getfield(self, name):
+ if (not hasattr(self._module_cls, name) or
+ not isinstance(getattr(self._module_cls, name), ModelType)):
+
+ raise KeyError(name)
+
+ return getattr(self._module_cls, name)
+
+ def _key(self, name):
+ field = self._getfield(name)
+ module = field.scope.module
+
+ if module == ModuleScope.ALL:
+ module_id = None
+ elif module == ModuleScope.USAGE:
+ module_id = self._usage.id
+ elif module == ModuleScope.DEFINITION:
+ module_id = self._usage.def_id
+ elif module == ModuleScope.TYPE:
+ module_id = self.module_type.__name__
+
+ if field.scope.student:
+ student_id = self._student_id
+ else:
+ student_id = None
+
+ key = KeyValueStore.Key(
+ scope=field.scope,
+ student_id=student_id,
+ module_scope_id=module_id,
+ field_name=name
+ )
+ return key
+
+ def __getitem__(self, name):
+ return self._kvs.get(self._key(name))
+
+ def __setitem__(self, name, value):
+ self._kvs.set(self._key(name), value)
+
+ def __delitem__(self, name):
+ self._kvs.delete(self._key(name))
+
+ def __iter__(self):
+ return iter(self.keys())
+
+ def __len__(self):
+ return len(self.keys())
+
+ def keys(self):
+ fields = [field.name for field in self._module_cls.fields]
+ for namespace_name in self._module_cls.namespaces:
+ fields.extend(field.name for field in getattr(self._module_cls, namespace_name))
+ return fields
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 9cad93ad5b..a2393aa235 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -417,15 +417,10 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
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.
From 45544396a89ebb235bd2c70582ea74caa28ef59d Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Wed, 12 Dec 2012 12:47:31 -0500
Subject: [PATCH 0014/1383] Make computed defaults work, even in namespaces
---
common/lib/xmodule/xmodule/model.py | 45 ++++++++++++++++++++++++-----
lms/xmodule_namespace.py | 6 +++-
2 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py
index 9286b0337e..89fc9d56d1 100644
--- a/common/lib/xmodule/xmodule/model.py
+++ b/common/lib/xmodule/xmodule/model.py
@@ -26,11 +26,12 @@ class ModelType(object):
"""
sequence = 0
- def __init__(self, help=None, default=None, scope=Scope.content):
+ def __init__(self, help=None, default=None, scope=Scope.content, computed_default=None):
self._seq = self.sequence
self._name = "unknown"
self.help = help
self.default = default
+ self.computed_default = computed_default
self.scope = scope
ModelType.sequence += 1
@@ -43,6 +44,9 @@ class ModelType(object):
return self
if self.name not in instance._model_data:
+ if self.default is None and self.computed_default is not None:
+ return self.computed_default(instance)
+
return self.default
return self.from_json(instance._model_data[self.name])
@@ -136,17 +140,44 @@ class NamespaceDescriptor(object):
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.
+ A baseclass that sets up machinery for ModelType fields that makes those fields be called
+ with the container as the field instance
"""
__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
+ def __getattribute__(self, name):
+ container = super(Namespace, self).__getattribute__('_container')
+ namespace_attr = getattr(type(self), name, None)
+
+ if namespace_attr is None or not isinstance(namespace_attr, ModelType):
+ return super(Namespace, self).__getattribute__(name)
+
+ return namespace_attr.__get__(container, type(container))
+
+ def __setattr__(self, name, value):
+ try:
+ container = super(Namespace, self).__getattribute__('_container')
+ except AttributeError:
+ super(Namespace, self).__setattr__(name, value)
+ return
+
+ container_class_attr = getattr(type(container), name, None)
+
+ if container_class_attr is None or not isinstance(container_class_attr, ModelType):
+ return super(Namespace, self).__setattr__(name, value)
+
+ return container_class_attr.__set__(container)
+
+ def __delattr__(self, name):
+ container = super(Namespace, self).__getattribute__('_container')
+ container_class_attr = getattr(type(container), name, None)
+
+ if container_class_attr is None or not isinstance(container_class_attr, ModelType):
+ return super(Namespace, self).__detattr__(name)
+
+ return container_class_attr.__delete__(container)
diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py
index 80b5fc6e61..7f30108b73 100644
--- a/lms/xmodule_namespace.py
+++ b/lms/xmodule_namespace.py
@@ -18,7 +18,11 @@ class LmsNamespace(Namespace):
scope=Scope.settings
)
- display_name = String(help="Display name for this module", scope=Scope.settings)
+ display_name = String(
+ help="Display name for this module",
+ scope=Scope.settings,
+ computed_default=lambda module: module.url_name.replace('_', ' ')
+ )
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='')
filename = List(help="DO NOT USE", scope=Scope.content, default=['', None])
From 57b3ceba2709537bc2669460af5c00a25fab05d8 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Wed, 12 Dec 2012 12:47:50 -0500
Subject: [PATCH 0015/1383] Add a field type that treats a string as an int
---
common/lib/xmodule/xmodule/capa_module.py | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 7e8ad9210f..1f49435ca2 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -21,6 +21,16 @@ from xmodule.raw_module import RawDescriptor
from xmodule.exceptions import NotFoundError
from .model import Int, Scope, ModuleScope, ModelType, String, Boolean, Object, Float
+
+class StringyInt(Int):
+ """
+ A model type that converts from strings to integers when reading from json
+ """
+ def from_json(self, value):
+ if isinstance(value, basestring):
+ return int(value)
+ return value
+
log = logging.getLogger("mitx.courseware")
#-----------------------------------------------------------------------------
@@ -88,7 +98,7 @@ 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)
+ max_attempts = StringyInt(help="Maximum number of attempts that a student is allowed", 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")
From 7e224f58479d4fdb17edd76b20b3ec2a0d798571 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Wed, 12 Dec 2012 12:48:49 -0500
Subject: [PATCH 0016/1383] Convert a bunch more references from metadata to
fields
---
common/lib/xmodule/xmodule/capa_module.py | 5 +++
common/lib/xmodule/xmodule/course_module.py | 41 ++++---------------
.../lib/xmodule/xmodule/discussion_module.py | 11 +++++
common/lib/xmodule/xmodule/modulestore/xml.py | 15 ++++---
lms/djangoapps/courseware/access.py | 2 +-
lms/djangoapps/courseware/grades.py | 14 +++----
lms/djangoapps/courseware/tests/tests.py | 4 +-
lms/djangoapps/django_comment_client/utils.py | 6 +--
lms/xmodule_namespace.py | 3 +-
9 files changed, 47 insertions(+), 54 deletions(-)
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 1f49435ca2..b8a85595dc 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -670,6 +670,11 @@ class CapaDescriptor(RawDescriptor):
# actually use type and points?
metadata_attributes = RawDescriptor.metadata_attributes + ('type', 'points')
+ # The capa format specifies that what we call max_attempts in the code
+ # is the attribute `attempts`. This will do that conversion
+ metadata_translations = dict(RawDescriptor.metadata_translations)
+ metadata_translations['attempts'] = 'max_attempts'
+
# VS[compat]
# TODO (cpennington): Delete this method once all fall 2012 course are being
# edited in the cms
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index c4dc63c1b4..b2a1917912 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -33,6 +33,9 @@ class CourseDescriptor(SequenceDescriptor):
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)
+ tabs = List(help="List of tabs to enable in this course", scope=Scope.settings)
+ end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings)
+ discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings, default=[])
has_children = True
info_sidebar_name = String(scope=Scope.settings, default='Course Handouts')
@@ -128,7 +131,7 @@ class CourseDescriptor(SequenceDescriptor):
if self.start is None:
msg = "Course loaded without a valid start date. id = %s" % self.id
# hack it -- start in 1970
- self.metadata['start'] = stringify_time(time.gmtime(0))
+ self.lms.start = time.gmtime(0)
log.critical(msg)
system.error_tracker(msg)
@@ -198,8 +201,6 @@ class CourseDescriptor(SequenceDescriptor):
grading_policy['GRADER'] = grader_from_conf(grading_policy['GRADER'])
self._grading_policy = grading_policy
-
-
@classmethod
def read_grading_policy(cls, paths, system):
"""Load a grading policy from the specified paths, in order, if it exists."""
@@ -221,14 +222,13 @@ class CourseDescriptor(SequenceDescriptor):
return policy_str
-
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
instance = super(CourseDescriptor, cls).from_xml(xml_data, system, org, course)
# bleh, have to parse the XML here to just pull out the url_name attribute
course_file = StringIO(xml_data)
- xml_obj = etree.parse(course_file,parser=edx_xml_parser).getroot()
+ xml_obj = etree.parse(course_file, parser=edx_xml_parser).getroot()
policy_dir = None
url_name = xml_obj.get('url_name', xml_obj.get('slug'))
@@ -241,7 +241,7 @@ class CourseDescriptor(SequenceDescriptor):
paths = [policy_dir + '/grading_policy.json'] + paths
policy = json.loads(cls.read_grading_policy(paths, system))
-
+
# 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.grading_policy = policy
@@ -250,7 +250,6 @@ class CourseDescriptor(SequenceDescriptor):
instance.set_grading_policy(policy)
return instance
-
@classmethod
def definition_from_xml(cls, xml_object, system):
@@ -334,17 +333,6 @@ class CourseDescriptor(SequenceDescriptor):
self.definition['data'].setdefault('grading_policy',{})['GRADE_CUTOFFS'] = value
- @property
- def tabs(self):
- """
- Return the tabs config, as a python object, or None if not specified.
- """
- return self.metadata.get('tabs')
-
- @tabs.setter
- def tabs(self, value):
- self.metadata['tabs'] = value
-
@lazyproperty
def grading_context(self):
"""
@@ -383,14 +371,14 @@ class CourseDescriptor(SequenceDescriptor):
for c in self.get_children():
sections = []
for s in c.get_children():
- if s.metadata.get('graded', False):
+ if s.lms.graded:
xmoduledescriptors = list(yield_descriptor_descendents(s))
xmoduledescriptors.append(s)
# The xmoduledescriptors included here are only the ones that have scores.
section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : filter(lambda child: child.has_score, xmoduledescriptors) }
- section_format = s.metadata.get('format', "")
+ section_format = s.lms.format
graded_sections[ section_format ] = graded_sections.get( section_format, [] ) + [section_description]
all_descriptors.extend(xmoduledescriptors)
@@ -447,7 +435,7 @@ class CourseDescriptor(SequenceDescriptor):
try:
blackout_periods = [(parse_time(start), parse_time(end))
for start, end
- in self.metadata.get('discussion_blackouts', [])]
+ in self.discussion_blackouts]
now = time.gmtime()
for start, end in blackout_periods:
if start <= now <= end:
@@ -457,17 +445,6 @@ class CourseDescriptor(SequenceDescriptor):
return True
-
- @property
- def end_of_course_survey_url(self):
- """
- Pull from policy. Once we have our own survey module set up, can change this to point to an automatically
- created survey for each class.
-
- Returns None if no url specified.
- """
- return self.metadata.get('end_of_course_survey_url')
-
@property
def title(self):
return self.display_name
diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py
index a193604278..19a2b4300f 100644
--- a/common/lib/xmodule/xmodule/discussion_module.py
+++ b/common/lib/xmodule/xmodule/discussion_module.py
@@ -12,6 +12,10 @@ class DiscussionModule(XModule):
}
js_module_name = "InlineDiscussion"
+ discussion_id = String(scope=Scope.settings)
+ discussion_category = String(scope=Scope.settings)
+ discussion_target = String(scope=Scope.settings)
+
data = String(help="XML definition of inline discussion", scope=Scope.content)
def get_html(self):
@@ -31,3 +35,10 @@ class DiscussionModule(XModule):
class DiscussionDescriptor(RawDescriptor):
module_class = DiscussionModule
template_dir_name = "discussion"
+
+ # The discussion XML format uses `id` and `for` attributes,
+ # but these would overload other module attributes, so we prefix them
+ # for actual use in the code
+ metadata_translations = dict(RawDescriptor.metadata_translations)
+ metadata_translations['id'] = 'discussion_id'
+ metadata_translations['for'] = 'discussion_target'
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 677b2e938e..ce6d9df093 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -430,10 +430,10 @@ class XMLModuleStore(ModuleStoreBase):
NOTE: This means that there is no such thing as lazy loading at the
moment--this accesses all the children."""
for child in descriptor.get_children():
- inherit_metadata(child, descriptor.metadata)
+ inherit_metadata(child, descriptor._model_data)
compute_inherited_metadata(child)
- def inherit_metadata(descriptor, metadata):
+ def inherit_metadata(descriptor, model_data):
"""
Updates this module with metadata inherited from a containing module.
Only metadata specified in self.inheritable_metadata will
@@ -441,11 +441,10 @@ class XMLModuleStore(ModuleStoreBase):
"""
# 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]
-
+ for attr in descriptor.inheritable_metadata:
+ if attr not in descriptor._model_data and attr in model_data:
+ descriptor._inherited_metadata.add(attr)
+ descriptor._model_data[attr] = model_data[attr]
compute_inherited_metadata(course_descriptor)
@@ -485,7 +484,7 @@ class XMLModuleStore(ModuleStoreBase):
if category == "static_tab":
for tab in course_descriptor.tabs or []:
if tab.get('url_slug') == slug:
- module.display_name = tab['name']
+ module.lms.display_name = tab['name']
module.data_dir = course_dir
self.modules[course_descriptor.id][module.location] = module
except Exception, e:
diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py
index 0bd2311021..c6342e9d13 100644
--- a/lms/djangoapps/courseware/access.py
+++ b/lms/djangoapps/courseware/access.py
@@ -155,7 +155,7 @@ def _has_access_course_desc(user, course, action):
if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'):
# if this feature is on, only allow courses that have ispublic set to be
# seen by non-staff
- if course.metadata.get('ispublic'):
+ if course.lms.ispublic:
debug("Allow: ACCESS_REQUIRE_STAFF_FOR_COURSE and ispublic")
return True
return _has_staff_access_to_descriptor(user, course)
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index 1283844ade..f6fd7bda88 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -179,12 +179,12 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F
else:
correct = total
- graded = module_descriptor.metadata.get("graded", False)
+ graded = module_descriptor.lms.graded
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, module_descriptor.metadata.get('display_name')))
+ scores.append(Score(correct, total, graded, module_descriptor.lms.display_name))
section_total, graded_total = graders.aggregate_scores(scores, section_name)
if keep_raw_scores:
@@ -288,7 +288,7 @@ def progress_summary(student, request, course, student_module_cache):
continue
# Same for sections
- graded = section_module.metadata.get('graded', False)
+ graded = section_module.lms.graded
scores = []
module_creator = lambda descriptor : section_module.system.get_module(descriptor.location)
@@ -301,20 +301,20 @@ def progress_summary(student, request, course, student_module_cache):
continue
scores.append(Score(correct, total, graded,
- module_descriptor.metadata.get('display_name')))
+ module_descriptor.lms.display_name))
scores.reverse()
section_total, graded_total = graders.aggregate_scores(
- scores, section_module.metadata.get('display_name'))
+ scores, section_module.lms.display_name)
- format = section_module.metadata.get('format', "")
+ format = section_module.lms.format
sections.append({
'display_name': section_module.display_name,
'url_name': section_module.url_name,
'scores': scores,
'section_total': section_total,
'format': format,
- 'due': section_module.metadata.get("due", ""),
+ 'due': section_module.due,
'graded': graded,
})
diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py
index 5af79d4983..7a6d498660 100644
--- a/lms/djangoapps/courseware/tests/tests.py
+++ b/lms/djangoapps/courseware/tests/tests.py
@@ -488,8 +488,8 @@ class TestViewAuth(PageLoader):
# Make courses start in the future
tomorrow = time.time() + 24*3600
- self.toy.metadata['start'] = stringify_time(time.gmtime(tomorrow))
- self.full.metadata['start'] = stringify_time(time.gmtime(tomorrow))
+ self.toy.lms.start = time.gmtime(tomorrow)
+ self.full.lms.start = time.gmtime(tomorrow)
self.assertFalse(self.toy.has_started())
self.assertFalse(self.full.has_started())
diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py
index b3a1626d22..174805a999 100644
--- a/lms/djangoapps/django_comment_client/utils.py
+++ b/lms/djangoapps/django_comment_client/utils.py
@@ -142,9 +142,9 @@ def initialize_discussion_info(course):
for location, module in all_modules.items():
if location.category == 'discussion':
- id = module.metadata['id']
- category = module.metadata['discussion_category']
- title = module.metadata['for']
+ id = module.discussion_id
+ category = module.discussion_category
+ title = module.discussion_target
sort_key = module.metadata.get('sort_key', title)
category = " / ".join([x.strip() for x in category.split("/")])
last_category = category.split("/")[-1]
diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py
index 7f30108b73..c78052ed1b 100644
--- a/lms/xmodule_namespace.py
+++ b/lms/xmodule_namespace.py
@@ -15,7 +15,7 @@ class LmsNamespace(Namespace):
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
+ scope=Scope.settings,
)
display_name = String(
@@ -29,3 +29,4 @@ class LmsNamespace(Namespace):
source_file = String(help="DO NOT USE", scope=Scope.settings)
giturl = String(help="DO NOT USE", scope=Scope.settings, default='https://github.com/MITx')
xqa_key = String(help="DO NOT USE", scope=Scope.settings)
+ ispublic = Boolean(help="Whether this course is open to the public, or only to admins", scope=Scope.settings)
From 01411ae66e9a596d19e43595a64d4192708de36d Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Thu, 13 Dec 2012 11:06:10 -0500
Subject: [PATCH 0017/1383] WIP: Trying to get tests working
---
common/djangoapps/mitxmako/shortcuts.py | 2 +-
common/djangoapps/xmodule_modifiers.py | 1 +
common/lib/capa/capa/capa_problem.py | 2 +-
common/lib/capa/capa/customrender.py | 2 +-
common/lib/capa/capa/inputtypes.py | 2 +-
common/lib/capa/capa/responsetypes.py | 2 +-
common/lib/capa/capa/xqueue_interface.py | 2 +-
common/lib/xmodule/xmodule/capa_module.py | 3 +-
common/lib/xmodule/xmodule/course_module.py | 4 +-
common/lib/xmodule/xmodule/error_module.py | 31 ++++---
common/lib/xmodule/xmodule/fields.py | 30 +++++++
common/lib/xmodule/xmodule/model.py | 18 ++--
common/lib/xmodule/xmodule/modulestore/xml.py | 3 +-
.../xmodule/modulestore/xml_importer.py | 52 ++++++-----
common/lib/xmodule/xmodule/runtime.py | 30 +++++--
.../xmodule/xmodule/self_assessment_module.py | 88 ++++++-------------
common/lib/xmodule/xmodule/tests/__init__.py | 3 +-
.../lib/xmodule/xmodule/tests/test_import.py | 42 +++++----
.../lib/xmodule/xmodule/tests/test_model.py | 8 ++
.../lib/xmodule/xmodule/tests/test_runtime.py | 0
common/lib/xmodule/xmodule/x_module.py | 29 +-----
common/lib/xmodule/xmodule/xml_module.py | 2 +
lms/djangoapps/courseware/grades.py | 16 ++--
lms/djangoapps/courseware/module_render.py | 1 +
lms/djangoapps/instructor/views.py | 2 +-
lms/lib/comment_client/utils.py | 2 +-
lms/templates/staff_problem_info.html | 19 +++-
lms/xmodule_namespace.py | 14 ++-
local-requirements.txt | 2 +-
lms/setup.py => setup.py | 4 +-
30 files changed, 219 insertions(+), 197 deletions(-)
create mode 100644 common/lib/xmodule/xmodule/fields.py
create mode 100644 common/lib/xmodule/xmodule/tests/test_model.py
create mode 100644 common/lib/xmodule/xmodule/tests/test_runtime.py
rename lms/setup.py => setup.py (85%)
diff --git a/common/djangoapps/mitxmako/shortcuts.py b/common/djangoapps/mitxmako/shortcuts.py
index 181d3befd5..8f39efdc02 100644
--- a/common/djangoapps/mitxmako/shortcuts.py
+++ b/common/djangoapps/mitxmako/shortcuts.py
@@ -14,7 +14,7 @@
import logging
-log = logging.getLogger("mitx." + __name__)
+log = logging.getLogger(__name__)
from django.template import Context
from django.http import HttpResponse
diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py
index 5a13582a2d..30098c94fc 100644
--- a/common/djangoapps/xmodule_modifiers.py
+++ b/common/djangoapps/xmodule_modifiers.py
@@ -131,6 +131,7 @@ def add_histogram(get_html, module, user):
is_released = "Yes!" if (now > mstart) else "Not yet"
staff_context = {'fields': [(field.name, getattr(module, field.name)) for field in module.fields],
+ 'lms_fields': [(field.name, getattr(module.lms, field.name)) for field in module.lms.fields],
'location': module.location,
'xqa_key': module.lms.xqa_key,
'source_file' : source_file,
diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py
index 85670063c5..d5ec6a1439 100644
--- a/common/lib/capa/capa/capa_problem.py
+++ b/common/lib/capa/capa/capa_problem.py
@@ -72,7 +72,7 @@ global_context = {'random': random,
# These should be removed from HTML output, including all subelements
html_problem_semantics = ["codeparam", "responseparam", "answer", "script", "hintgroup"]
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
#-----------------------------------------------------------------------------
# main class for this module
diff --git a/common/lib/capa/capa/customrender.py b/common/lib/capa/capa/customrender.py
index ef1044e8b1..0268d7b266 100644
--- a/common/lib/capa/capa/customrender.py
+++ b/common/lib/capa/capa/customrender.py
@@ -17,7 +17,7 @@ from lxml import etree
import xml.sax.saxutils as saxutils
from registry import TagRegistry
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
registry = TagRegistry()
diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py
index 0b2250f98d..8a15434d1d 100644
--- a/common/lib/capa/capa/inputtypes.py
+++ b/common/lib/capa/capa/inputtypes.py
@@ -44,7 +44,7 @@ import sys
from registry import TagRegistry
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
#########################################################################
diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py
index 418ee9d8ae..2074e8f648 100644
--- a/common/lib/capa/capa/responsetypes.py
+++ b/common/lib/capa/capa/responsetypes.py
@@ -33,7 +33,7 @@ from lxml import etree
from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME?
import xqueue_interface
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
#-----------------------------------------------------------------------------
diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py
index 0214488cce..a7d31db7e1 100644
--- a/common/lib/capa/capa/xqueue_interface.py
+++ b/common/lib/capa/capa/xqueue_interface.py
@@ -7,7 +7,7 @@ import logging
import requests
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
dateformat = '%Y%m%d%H%M%S'
def make_hashkey(seed):
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index b8a85595dc..36da929926 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -551,8 +551,7 @@ class CapaModule(XModule):
msg = "Error checking problem: " + str(err)
msg += '\nTraceback:\n' + traceback.format_exc()
return {'success': msg}
- log.exception("Error in capa_module problem checking")
- raise Exception("error in capa_module")
+ raise
self.attempts = self.attempts + 1
self.lcp.done = True
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index b2a1917912..b8d4032a18 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -131,9 +131,9 @@ class CourseDescriptor(SequenceDescriptor):
if self.start is None:
msg = "Course loaded without a valid start date. id = %s" % self.id
# hack it -- start in 1970
- self.lms.start = time.gmtime(0)
+ self.start = time.gmtime(0)
log.critical(msg)
- system.error_tracker(msg)
+ self.system.error_tracker(msg)
# NOTE: relies on the modulestore to call set_grading_policy() right after
# init. (Modulestore is in charge of figuring out where to load the policy from)
diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py
index 0f95bcd256..67b52d383c 100644
--- a/common/lib/xmodule/xmodule/error_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -8,6 +8,7 @@ from xmodule.x_module import XModule
from xmodule.editing_module import JSONEditingDescriptor
from xmodule.errortracker import exc_info_to_str
from xmodule.modulestore import Location
+from .model import String, Scope
log = logging.getLogger(__name__)
@@ -52,6 +53,10 @@ class ErrorDescriptor(JSONEditingDescriptor):
"""
module_class = ErrorModule
+ contents = String(scope=Scope.content)
+ error_msg = String(scope=Scope.content)
+ display_name = String(scope=Scope.settings)
+
@classmethod
def _construct(self, system, contents, error_msg, location):
@@ -66,15 +71,12 @@ class ErrorDescriptor(JSONEditingDescriptor):
name=hashlib.sha1(contents).hexdigest()
)
- definition = {
- 'data': {
- 'error_msg': str(error_msg),
- 'contents': contents,
- }
- }
-
# real metadata stays in the content, but add a display name
- model_data = {'display_name': 'Error: ' + location.name}
+ model_data = {
+ 'error_msg': str(error_msg),
+ 'contents': contents,
+ 'display_name': 'Error: ' + location.name
+ }
return ErrorDescriptor(
system,
location,
@@ -84,7 +86,7 @@ class ErrorDescriptor(JSONEditingDescriptor):
def get_context(self):
return {
'module': self,
- 'data': self.definition['data']['contents'],
+ 'data': self.contents,
}
@classmethod
@@ -100,10 +102,7 @@ class ErrorDescriptor(JSONEditingDescriptor):
def from_descriptor(cls, descriptor, error_msg='Error not available'):
return cls._construct(
descriptor.system,
- json.dumps({
- 'definition': descriptor.definition,
- 'metadata': descriptor.metadata,
- }, indent=4),
+ json.dumps(descriptor._model_data, indent=4),
error_msg,
location=descriptor.location,
)
@@ -147,14 +146,14 @@ class ErrorDescriptor(JSONEditingDescriptor):
files, etc. That would just get re-wrapped on import.
'''
try:
- xml = etree.fromstring(self.definition['data']['contents'])
+ xml = etree.fromstring(self.contents)
return etree.tostring(xml)
except etree.XMLSyntaxError:
# still not valid.
root = etree.Element('error')
- root.text = self.definition['data']['contents']
+ root.text = self.contents
err_node = etree.SubElement(root, 'error_msg')
- err_node.text = self.definition['data']['error_msg']
+ err_node.text = self.error_msg
return etree.tostring(root)
diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py
new file mode 100644
index 0000000000..715aea0c7c
--- /dev/null
+++ b/common/lib/xmodule/xmodule/fields.py
@@ -0,0 +1,30 @@
+import time
+import logging
+
+from .model import ModelType
+
+log = logging.getLogger(__name__)
+
+
+class Date(ModelType):
+ time_format = "%Y-%m-%dT%H:%M"
+
+ def from_json(self, value):
+ """
+ Parse an optional metadata key containing a time: if present, complain
+ if it doesn't parse.
+ Return None if not present or invalid.
+ """
+ try:
+ return time.strptime(value, self.time_format)
+ except ValueError as e:
+ msg = "Field {0} has bad value '{1}': '{2}'".format(
+ self._name, value, e)
+ log.warning(msg)
+ return None
+
+ def to_json(self, value):
+ """
+ Convert a time struct to a string
+ """
+ return time.strftime(self.time_format, value)
diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py
index 89fc9d56d1..93ed12f69d 100644
--- a/common/lib/xmodule/xmodule/model.py
+++ b/common/lib/xmodule/xmodule/model.py
@@ -43,14 +43,14 @@ class ModelType(object):
if instance is None:
return self
- if self.name not in instance._model_data:
+ try:
+ return self.from_json(instance._model_data[self.name])
+ except KeyError:
if self.default is None and self.computed_default is not None:
return self.computed_default(instance)
return self.default
- return self.from_json(instance._model_data[self.name])
-
def __set__(self, instance, value):
instance._model_data[self.name] = self.to_json(value)
@@ -166,18 +166,18 @@ class Namespace(Plugin):
super(Namespace, self).__setattr__(name, value)
return
- container_class_attr = getattr(type(container), name, None)
+ namespace_attr = getattr(type(self), name, None)
- if container_class_attr is None or not isinstance(container_class_attr, ModelType):
+ if namespace_attr is None or not isinstance(namespace_attr, ModelType):
return super(Namespace, self).__setattr__(name, value)
- return container_class_attr.__set__(container)
+ return namespace_attr.__set__(container, value)
def __delattr__(self, name):
container = super(Namespace, self).__getattribute__('_container')
- container_class_attr = getattr(type(container), name, None)
+ namespace_attr = getattr(type(self), name, None)
- if container_class_attr is None or not isinstance(container_class_attr, ModelType):
+ if namespace_attr is None or not isinstance(namespace_attr, ModelType):
return super(Namespace, self).__detattr__(name)
- return container_class_attr.__delete__(container)
+ return namespace_attr.__delete__(container)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index ce6d9df093..72c9093bbf 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -28,7 +28,7 @@ edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False,
etree.set_default_parser(edx_xml_parser)
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
# VS[compat]
@@ -160,7 +160,6 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
etree.tostring(xml_data), self, self.org,
self.course, xmlstore.default_class)
except Exception as err:
- print err, self.load_error_modules
if not self.load_error_modules:
raise
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 35375d7c51..f90291aaa2 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -8,6 +8,7 @@ from .xml import XMLModuleStore
from .exceptions import DuplicateItemError
from xmodule.modulestore import Location
from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX
+from xmodule.model import Scope
log = logging.getLogger(__name__)
@@ -123,7 +124,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
# Quick scan to get course Location as well as the course_data_path
for module in module_store.modules[course_id].itervalues():
if module.category == 'course':
- course_data_path = path(data_dir) / module.metadata['data_dir']
+ course_data_path = path(data_dir) / module.data_dir
course_location = module.location
if static_content_store is not None:
@@ -159,18 +160,17 @@ def import_from_xml(store, data_dir, course_dirs=None,
module.definition['children'] = new_locs
-
if module.category == 'course':
# HACK: for now we don't support progress tabs. There's a special metadata configuration setting for this.
- module.metadata['hide_progress_tab'] = True
+ module.hide_progress_tab = True
- # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which
- # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS,
- # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that -
+ # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which
+ # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS,
+ # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that -
# if there is *any* tabs - then there at least needs to be some predefined ones
if module.tabs is None or len(module.tabs) == 0:
- module.tabs = [{"type": "courseware"},
- {"type": "course_info", "name": "Course Info"},
+ module.tabs = [{"type": "courseware"},
+ {"type": "course_info", "name": "Course Info"},
{"type": "discussion", "name": "Discussion"},
{"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge
@@ -180,39 +180,43 @@ def import_from_xml(store, data_dir, course_dirs=None,
course_items.append(module)
- if 'data' in module.definition:
- module_data = module.definition['data']
-
+ if hasattr(module, 'data'):
# cdodge: now go through any link references to '/static/' and make sure we've imported
# it as a StaticContent asset
- try:
+ try:
remap_dict = {}
# use the rewrite_links as a utility means to enumerate through all links
# in the module data. We use that to load that reference into our asset store
# IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to
# do the rewrites natively in that code.
- # For example, what I'm seeing is
->
+ # For example, what I'm seeing is
->
# Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's
# no good, so we have to do this kludge
- if isinstance(module_data, str) or isinstance(module_data, unicode): # some module 'data' fields are non strings which blows up the link traversal code
- lxml_rewrite_links(module_data, lambda link: verify_content_links(module, course_data_path,
- static_content_store, link, remap_dict))
+ if isinstance(module.data, str) or isinstance(module.data, unicode): # some module 'data' fields are non strings which blows up the link traversal code
+ lxml_rewrite_links(module.data, lambda link: verify_content_links(module, course_data_path,
+ static_content_store, link, remap_dict))
for key in remap_dict.keys():
- module_data = module_data.replace(key, remap_dict[key])
+ module.data = module.data.replace(key, remap_dict[key])
- except Exception, e:
+ except Exception:
logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
- store.update_item(module.location, module_data)
+ store.update_item(module.location, module.data)
- if 'children' in module.definition:
- store.update_children(module.location, module.definition['children'])
+ if module.has_children:
+ store.update_children(module.location, module.children)
- # NOTE: It's important to use own_metadata here to avoid writing
- # inherited metadata everywhere.
- store.update_metadata(module.location, dict(module.own_metadata))
+ metadata = {}
+ for field in module.fields + module.lms.fields:
+ # Only save metadata that wasn't inherited
+ if (field.scope == Scope.settings and
+ field.name in module._inherited_metadata and
+ field.name in module._model_data):
+
+ metadata[field.name] = module._model_data[field.name]
+ store.update_metadata(module.location, metadata)
return module_store, course_items
diff --git a/common/lib/xmodule/xmodule/runtime.py b/common/lib/xmodule/xmodule/runtime.py
index 8be4bfe346..7030f04f9f 100644
--- a/common/lib/xmodule/xmodule/runtime.py
+++ b/common/lib/xmodule/xmodule/runtime.py
@@ -33,19 +33,33 @@ class DbModel(MutableMapping):
def __repr__(self):
return "<{0.__class__.__name__} {0._module_cls!r}>".format(self)
- def __str__(self):
- return str(dict(self.iteritems()))
-
def _getfield(self, name):
- if (not hasattr(self._module_cls, name) or
- not isinstance(getattr(self._module_cls, name), ModelType)):
+ # First, get the field from the class, if defined
+ module_field = getattr(self._module_cls, name, None)
+ if module_field is not None and isinstance(module_field, ModelType):
+ return module_field
- raise KeyError(name)
+ # If the class doesn't have the field, and it also
+ # doesn't have any namespaces, then the the name isn't a field
+ # so KeyError
+ if not hasattr(self._module_cls, 'namespaces'):
+ return KeyError(name)
- return getattr(self._module_cls, name)
+ # Resolve the field name in the first namespace where it's
+ # available
+ for namespace_name in self._module_cls.namespaces:
+ namespace = getattr(self._module_cls, namespace_name)
+ namespace_field = getattr(type(namespace), name, None)
+ if namespace_field is not None and isinstance(module_field, ModelType):
+ return namespace_field
+
+ # Not in the class or in any of the namespaces, so name
+ # really doesn't name a field
+ raise KeyError(name)
def _key(self, name):
field = self._getfield(name)
+ print name, field
module = field.scope.module
if module == ModuleScope.ALL:
@@ -88,5 +102,5 @@ class DbModel(MutableMapping):
def keys(self):
fields = [field.name for field in self._module_cls.fields]
for namespace_name in self._module_cls.namespaces:
- fields.extend(field.name for field in getattr(self._module_cls, namespace_name))
+ fields.extend(field.name for field in getattr(self._module_cls, namespace_name).fields)
return fields
diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py
index 2edf5467b2..2f4674cf7c 100644
--- a/common/lib/xmodule/xmodule/self_assessment_module.py
+++ b/common/lib/xmodule/xmodule/self_assessment_module.py
@@ -25,6 +25,7 @@ from .stringify import stringify_children
from .x_module import XModule
from .xml_module import XmlDescriptor
from xmodule.modulestore import Location
+from .model import List, String, Scope, Int
log = logging.getLogger("mitx.courseware")
@@ -61,67 +62,21 @@ class SelfAssessmentModule(XModule):
js = {'coffee': [resource_string(__name__, 'js/src/selfassessment/display.coffee')]}
js_module_name = "SelfAssessment"
- def __init__(self, system, location, definition, descriptor,
- instance_state=None, shared_state=None, **kwargs):
- XModule.__init__(self, system, location, definition, descriptor,
- instance_state, shared_state, **kwargs)
+ student_answers = List(scope=Scope.student_state, default=[])
+ scores = List(scope=Scope.student_state, default=[])
+ hints = List(scope=Scope.student_state, default=[])
+ state = String(scope=Scope.student_state, default=INITIAL)
- """
- Definition file should have 4 blocks -- prompt, rubric, submitmessage, hintprompt,
- and two optional attributes:
- attempts, which should be an integer that defaults to 1.
- If it's > 1, the student will be able to re-submit after they see
- the rubric.
- max_score, which should be an integer that defaults to 1.
- It defines the maximum number of points a student can get. Assumed to be integer scale
- from 0 to max_score, with an interval of 1.
+ # Used for progress / grading. Currently get credit just for
+ # completion (doesn't matter if you self-assessed correct/incorrect).
+ max_score = Int(scope=Scope.settings, default=MAX_SCORE)
- Note: all the submissions are stored.
-
- Sample file:
-
-
-
- Insert prompt text here. (arbitrary html)
-
-
- Insert grading rubric here. (arbitrary html)
-
-
- Please enter a hint below: (arbitrary html)
-
-
- Thanks for submitting! (arbitrary html)
-
-
- """
-
- # Load instance state
- if instance_state is not None:
- instance_state = json.loads(instance_state)
- else:
- instance_state = {}
-
- # Note: score responses are on scale from 0 to max_score
- self.student_answers = instance_state.get('student_answers', [])
- self.scores = instance_state.get('scores', [])
- self.hints = instance_state.get('hints', [])
-
- self.state = instance_state.get('state', 'initial')
-
- # Used for progress / grading. Currently get credit just for
- # completion (doesn't matter if you self-assessed correct/incorrect).
-
- self._max_score = int(self.metadata.get('max_score', MAX_SCORE))
-
- self.attempts = instance_state.get('attempts', 0)
-
- self.max_attempts = int(self.metadata.get('attempts', MAX_ATTEMPTS))
-
- self.rubric = definition['rubric']
- self.prompt = definition['prompt']
- self.submit_message = definition['submitmessage']
- self.hint_prompt = definition['hintprompt']
+ attempts = Int(scope=Scope.student_state, default=0), Int
+ max_attempts = Int(scope=Scope.settings, default=MAX_ATTEMPTS)
+ rubric = String(scope=Scope.content)
+ prompt = String(scope=Scope.content)
+ submit_message = String(scope=Scope.content)
+ hint_prompt = String(scope=Scope.content)
def _allow_reset(self):
"""Can the module be reset?"""
@@ -432,6 +387,21 @@ class SelfAssessmentDescriptor(XmlDescriptor, EditingDescriptor):
js = {'coffee': [resource_string(__name__, 'js/src/html/edit.coffee')]}
js_module_name = "HTMLEditingDescriptor"
+ # The capa format specifies that what we call max_attempts in the code
+ # is the attribute `attempts`. This will do that conversion
+ metadata_translations = dict(XmlDescriptor.metadata_translations)
+ metadata_translations['attempts'] = 'max_attempts'
+
+ # Used for progress / grading. Currently get credit just for
+ # completion (doesn't matter if you self-assessed correct/incorrect).
+ max_score = Int(scope=Scope.settings, default=MAX_SCORE)
+
+ max_attempts = Int(scope=Scope.settings, default=MAX_ATTEMPTS)
+ rubric = String(scope=Scope.content)
+ prompt = String(scope=Scope.content)
+ submit_message = String(scope=Scope.content)
+ hint_prompt = String(scope=Scope.content)
+
@classmethod
def definition_from_xml(cls, xml_object, system):
"""
diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
index ed64c45118..c16c6d7596 100644
--- a/common/lib/xmodule/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/__init__.py
@@ -30,7 +30,8 @@ i4xs = ModuleSystem(
debug=True,
xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 10},
node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"),
- anonymous_student_id = 'student'
+ anonymous_student_id = 'student',
+ xmodule_model_data = lambda x: x,
)
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index 77532959d7..d243ca1609 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -91,8 +91,10 @@ class ImportTestCase(unittest.TestCase):
self.assertEqual(re_import_descriptor.__class__.__name__,
'ErrorDescriptor')
- self.assertEqual(descriptor.definition['data'],
- re_import_descriptor.definition['data'])
+ self.assertEqual(descriptor.contents,
+ re_import_descriptor.contents)
+ self.assertEqual(descriptor.error_msg,
+ re_import_descriptor.error_msg)
def test_fixed_xml_tag(self):
"""Make sure a tag that's been fixed exports as the original tag type"""
@@ -126,23 +128,19 @@ class ImportTestCase(unittest.TestCase):
url_name = 'test1'
start_xml = '''
+ due="{due}" url_name="{url_name}" unicorn="purple">
Two houses, ...
- '''.format(grace=v, org=ORG, course=COURSE, url_name=url_name)
+ '''.format(due=v, org=ORG, course=COURSE, url_name=url_name)
descriptor = system.process_xml(start_xml)
- print descriptor, descriptor.metadata
- self.assertEqual(descriptor.metadata['graceperiod'], v)
- self.assertEqual(descriptor.metadata['unicorn'], 'purple')
+ print descriptor, descriptor._model_data
+ self.assertEqual(descriptor.lms.due, v)
- # Check that the child inherits graceperiod correctly
+ # Check that the child inherits due correctly
child = descriptor.get_children()[0]
- self.assertEqual(child.metadata['graceperiod'], v)
-
- # check that the child does _not_ inherit any unicorns
- self.assertTrue('unicorn' not in child.metadata)
+ self.assertEqual(child.lms.due, v)
# Now export and check things
resource_fs = MemoryFS()
@@ -169,12 +167,12 @@ class ImportTestCase(unittest.TestCase):
# did we successfully strip the url_name from the definition contents?
self.assertTrue('url_name' not in course_xml.attrib)
- # Does the chapter tag now have a graceperiod attribute?
+ # Does the chapter tag now have a due attribute?
# hardcoded path to child
with resource_fs.open('chapter/ch.xml') as f:
chapter_xml = etree.fromstring(f.read())
self.assertEqual(chapter_xml.tag, 'chapter')
- self.assertFalse('graceperiod' in chapter_xml.attrib)
+ self.assertFalse('due' in chapter_xml.attrib)
def test_is_pointer_tag(self):
"""
@@ -216,7 +214,7 @@ class ImportTestCase(unittest.TestCase):
def check_for_key(key, node):
"recursive check for presence of key"
print "Checking {0}".format(node.location.url())
- self.assertTrue(key in node.metadata)
+ self.assertTrue(key in node._model_data)
for c in node.get_children():
check_for_key(key, c)
@@ -244,15 +242,15 @@ class ImportTestCase(unittest.TestCase):
toy_ch = toy.get_children()[0]
two_toys_ch = two_toys.get_children()[0]
- self.assertEqual(toy_ch.display_name, "Overview")
- self.assertEqual(two_toys_ch.display_name, "Two Toy Overview")
+ self.assertEqual(toy_ch.lms.display_name, "Overview")
+ self.assertEqual(two_toys_ch.lms.display_name, "Two Toy Overview")
# Also check that the grading policy loaded
self.assertEqual(two_toys.grade_cutoffs['C'], 0.5999)
# Also check that keys from policy are run through the
# appropriate attribute maps -- 'graded' should be True, not 'true'
- self.assertEqual(toy.metadata['graded'], True)
+ self.assertEqual(toy.lms.graded, True)
def test_definition_loading(self):
@@ -271,8 +269,8 @@ class ImportTestCase(unittest.TestCase):
location = Location(["i4x", "edX", "toy", "video", "Welcome"])
toy_video = modulestore.get_instance(toy_id, location)
two_toy_video = modulestore.get_instance(two_toy_id, location)
- self.assertEqual(toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh8")
- self.assertEqual(two_toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh9")
+ self.assertEqual(toy_video.youtube, "1.0:p2Q6BrNhdh8")
+ self.assertEqual(two_toy_video.youtube, "1.0:p2Q6BrNhdh9")
def test_colon_in_url_name(self):
@@ -306,7 +304,7 @@ class ImportTestCase(unittest.TestCase):
cloc = course.location
loc = Location(cloc.tag, cloc.org, cloc.course, 'html', 'secret:toylab')
html = modulestore.get_instance(course_id, loc)
- self.assertEquals(html.display_name, "Toy lab")
+ self.assertEquals(html.lms.display_name, "Toy lab")
def test_url_name_mangling(self):
"""
@@ -351,4 +349,4 @@ class ImportTestCase(unittest.TestCase):
location = Location(["i4x", "edX", "sa_test", "selfassessment", "SampleQuestion"])
sa_sample = modulestore.get_instance(sa_id, location)
#10 attempts is hard coded into SampleQuestion, which is the url_name of a selfassessment xml tag
- self.assertEqual(sa_sample.metadata['attempts'], '10')
+ self.assertEqual(sa_sample.max_attempts, '10')
diff --git a/common/lib/xmodule/xmodule/tests/test_model.py b/common/lib/xmodule/xmodule/tests/test_model.py
new file mode 100644
index 0000000000..0e42df80a1
--- /dev/null
+++ b/common/lib/xmodule/xmodule/tests/test_model.py
@@ -0,0 +1,8 @@
+
+class ModelMetaclassTester(object):
+ __metaclass__ = ModelMetaclass
+
+ field_a = Int(scope=Scope.settings)
+ field_b = Int(scope=Scope.content)
+
+def test_model_metaclass():
diff --git a/common/lib/xmodule/xmodule/tests/test_runtime.py b/common/lib/xmodule/xmodule/tests/test_runtime.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index a2393aa235..f03fbe6183 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -1,7 +1,6 @@
import logging
import yaml
import os
-import time
from lxml import etree
from pprint import pprint
@@ -10,38 +9,14 @@ from pkg_resources import resource_listdir, resource_string, resource_isdir
from xmodule.modulestore import Location
-from .model import ModelMetaclass, ParentModelMetaclass, NamespacesMetaclass, ModelType
+from .model import ModelMetaclass, ParentModelMetaclass, NamespacesMetaclass
from .plugin import Plugin
-class Date(ModelType):
- time_format = "%Y-%m-%dT%H:%M"
-
- def from_json(self, value):
- """
- Parse an optional metadata key containing a time: if present, complain
- if it doesn't parse.
- Return None if not present or invalid.
- """
- try:
- return time.strptime(value, self.time_format)
- except ValueError as e:
- msg = "Field {0} has bad value '{1}': '{2}'".format(
- self._name, value, e)
- log.warning(msg)
- return None
-
- def to_json(self, value):
- """
- Convert a time struct to a string
- """
- return time.strftime(self.time_format, value)
-
-
class XModuleMetaclass(ParentModelMetaclass, NamespacesMetaclass, ModelMetaclass):
pass
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
def dummy_track(event_type, event):
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 36bea6edb2..50c4f7aa6d 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -311,11 +311,13 @@ class XmlDescriptor(XModuleDescriptor):
# Set/override any metadata specified by policy
k = policy_key(location)
if k in system.policy:
+ if k == 'video/labintro': print k, metadata, system.policy[k]
cls.apply_policy(metadata, system.policy[k])
model_data = {}
model_data.update(metadata)
model_data.update(definition)
+ if k == 'video/labintro': print model_data
return cls(
system,
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index f6fd7bda88..032378f863 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -148,7 +148,7 @@ def grade(student, request, course, student_module_cache=None, keep_raw_scores=F
format_scores = []
for section in sections:
section_descriptor = section['section_descriptor']
- section_name = section_descriptor.display_name
+ section_name = section_descriptor.lms.display_name
should_grade_section = False
# If we haven't seen a single problem in the section, we don't have to grade it at all! We can assume 0%
@@ -276,15 +276,13 @@ def progress_summary(student, request, course, student_module_cache):
# Don't include chapters that aren't displayable (e.g. due to error)
for chapter_module in course_module.get_display_items():
# Skip if the chapter is hidden
- hidden = chapter_module._model_data.get('hide_from_toc','false')
- if hidden.lower() == 'true':
+ if chapter_module.lms.hide_from_toc:
continue
sections = []
for section_module in chapter_module.get_display_items():
# Skip if the section is hidden
- hidden = section_module._model_data.get('hide_from_toc','false')
- if hidden.lower() == 'true':
+ if section_module.lms.hide_from_toc:
continue
# Same for sections
@@ -309,17 +307,17 @@ def progress_summary(student, request, course, student_module_cache):
format = section_module.lms.format
sections.append({
- 'display_name': section_module.display_name,
+ 'display_name': section_module.lms.display_name,
'url_name': section_module.url_name,
'scores': scores,
'section_total': section_total,
'format': format,
- 'due': section_module.due,
+ 'due': section_module.lms.due,
'graded': graded,
})
- chapters.append({'course': course.display_name,
- 'display_name': chapter_module.display_name,
+ chapters.append({'course': course.lms.display_name,
+ 'display_name': chapter_module.lms.display_name,
'url_name': chapter_module.url_name,
'sections': sections})
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index f83ff35f1f..be207730b3 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -92,6 +92,7 @@ def toc_for_course(user, request, course, active_chapter, active_section):
chapters = list()
for chapter in course_module.get_display_items():
+ print chapter, chapter._model_data, chapter._model_data.get('hide_from_toc'), chapter.lms.hide_from_toc
if chapter.lms.hide_from_toc:
continue
diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py
index 0b6392a7fc..0e05ad5bc5 100644
--- a/lms/djangoapps/instructor/views.py
+++ b/lms/djangoapps/instructor/views.py
@@ -76,7 +76,7 @@ def instructor_dashboard(request, course_id):
data = [['# Enrolled', CourseEnrollment.objects.filter(course_id=course_id).count()]]
data += compute_course_stats(course).items()
if request.user.is_staff:
- data.append(['metadata', escape(str(course.metadata))])
+ data.append(['metadata', escape(str(course._model_data))])
datatable['data'] = data
def return_csv(fn, datatable):
diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py
index f50797d5e0..9a7043565a 100644
--- a/lms/lib/comment_client/utils.py
+++ b/lms/lib/comment_client/utils.py
@@ -3,7 +3,7 @@ import logging
import requests
import settings
-log = logging.getLogger('mitx.' + __name__)
+log = logging.getLogger(__name__)
def strip_none(dic):
return dict([(k, v) for k, v in dic.iteritems() if v is not None])
diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html
index ad4852335f..e0c957a05c 100644
--- a/lms/templates/staff_problem_info.html
+++ b/lms/templates/staff_problem_info.html
@@ -46,9 +46,22 @@ github = ${edit_link | h}
%if source_file:
source_url = ${source_file | h}
%endif
-%for name, field in fields:
-${name} = ${field | h}
-%endfor
+
+
+ %for name, field in fields:
+
+ ${name} =
${field | h}
+
+ %endfor
+
+
+
+ %for name, field in lms_fields:
+
+ ${name} =
${field | h}
+
+ %endfor
+
category = ${category | h}
%if render_histogram:
diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py
index c78052ed1b..976bd4483f 100644
--- a/lms/xmodule_namespace.py
+++ b/lms/xmodule_namespace.py
@@ -1,8 +1,17 @@
from xmodule.model import Namespace, Boolean, Scope, String, List
-from xmodule.x_module import Date
+from xmodule.fields import Date
+
+
+class StringyBoolean(Boolean):
+ def from_json(self, value):
+ print "StringyBoolean ", value
+ if isinstance(value, basestring):
+ return value.lower() == 'true'
+ return value
+
class LmsNamespace(Namespace):
- hide_from_toc = Boolean(
+ hide_from_toc = StringyBoolean(
help="Whether to display this module in the table of contents",
default=False,
scope=Scope.settings
@@ -16,6 +25,7 @@ class LmsNamespace(Namespace):
help="What format this module is in (used for deciding which "
"grader to apply, and what to show in the TOC)",
scope=Scope.settings,
+ default='',
)
display_name = String(
diff --git a/local-requirements.txt b/local-requirements.txt
index 2e951a180c..201467d11e 100644
--- a/local-requirements.txt
+++ b/local-requirements.txt
@@ -1,4 +1,4 @@
# Python libraries to install that are local to the mitx repo
-e common/lib/capa
-e common/lib/xmodule
--e lms
+-e .
diff --git a/lms/setup.py b/setup.py
similarity index 85%
rename from lms/setup.py
rename to setup.py
index 1755b7d37e..84f242cf3d 100644
--- a/lms/setup.py
+++ b/setup.py
@@ -1,13 +1,13 @@
from setuptools import setup, find_packages
setup(
- name="edX LMS",
+ name="edX Apps",
version="0.1",
install_requires=['distribute'],
requires=[
'xmodule',
],
-
+ py_modules=['lms.xmodule_namespace'],
# See http://guide.python-distribute.org/creation.html#entry-points
# for a description of entry_points
entry_points={
From eb1ab3ea7db15fc75485d02e65bf49535215945a Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Thu, 13 Dec 2012 15:34:10 -0500
Subject: [PATCH 0018/1383] Add tests of xmodule.model and xmodule.runtime, and
fix some exposed bugs
---
common/lib/xmodule/xmodule/model.py | 2 -
common/lib/xmodule/xmodule/runtime.py | 11 +-
.../lib/xmodule/xmodule/tests/test_model.py | 154 +++++++++++++++++-
.../lib/xmodule/xmodule/tests/test_runtime.py | 67 ++++++++
4 files changed, 222 insertions(+), 12 deletions(-)
diff --git a/common/lib/xmodule/xmodule/model.py b/common/lib/xmodule/xmodule/model.py
index 93ed12f69d..380c81dbe9 100644
--- a/common/lib/xmodule/xmodule/model.py
+++ b/common/lib/xmodule/xmodule/model.py
@@ -133,8 +133,6 @@ class NamespaceDescriptor(object):
self._namespace = namespace
def __get__(self, instance, owner):
- if owner is None:
- return self
return self._namespace(instance)
diff --git a/common/lib/xmodule/xmodule/runtime.py b/common/lib/xmodule/xmodule/runtime.py
index 7030f04f9f..6f20997894 100644
--- a/common/lib/xmodule/xmodule/runtime.py
+++ b/common/lib/xmodule/xmodule/runtime.py
@@ -11,13 +11,13 @@ class KeyValueStore(object):
# data.
Key = namedtuple("Key", "scope, student_id, module_scope_id, field_name")
- def get(key):
+ def get(self, key):
pass
- def set(key, value):
+ def set(self, key, value):
pass
- def delete(key):
+ def delete(self, key):
pass
@@ -50,7 +50,7 @@ class DbModel(MutableMapping):
for namespace_name in self._module_cls.namespaces:
namespace = getattr(self._module_cls, namespace_name)
namespace_field = getattr(type(namespace), name, None)
- if namespace_field is not None and isinstance(module_field, ModelType):
+ if namespace_field is not None and isinstance(namespace_field, ModelType):
return namespace_field
# Not in the class or in any of the namespaces, so name
@@ -59,7 +59,6 @@ class DbModel(MutableMapping):
def _key(self, name):
field = self._getfield(name)
- print name, field
module = field.scope.module
if module == ModuleScope.ALL:
@@ -69,7 +68,7 @@ class DbModel(MutableMapping):
elif module == ModuleScope.DEFINITION:
module_id = self._usage.def_id
elif module == ModuleScope.TYPE:
- module_id = self.module_type.__name__
+ module_id = self._module_cls.__name__
if field.scope.student:
student_id = self._student_id
diff --git a/common/lib/xmodule/xmodule/tests/test_model.py b/common/lib/xmodule/xmodule/tests/test_model.py
index 0e42df80a1..b0ae9e5844 100644
--- a/common/lib/xmodule/xmodule/tests/test_model.py
+++ b/common/lib/xmodule/xmodule/tests/test_model.py
@@ -1,8 +1,154 @@
+from mock import patch
+from unittest import TestCase
+from nose.tools import assert_in, assert_equals, assert_raises
-class ModelMetaclassTester(object):
- __metaclass__ = ModelMetaclass
+from xmodule.model import *
- field_a = Int(scope=Scope.settings)
- field_b = Int(scope=Scope.content)
def test_model_metaclass():
+ class ModelMetaclassTester(object):
+ __metaclass__ = ModelMetaclass
+
+ field_a = Int(scope=Scope.settings)
+ field_b = Int(scope=Scope.content)
+
+ def __init__(self, model_data):
+ self._model_data = model_data
+
+ assert hasattr(ModelMetaclassTester, 'field_a')
+ assert hasattr(ModelMetaclassTester, 'field_b')
+
+ assert_in(ModelMetaclassTester.field_a, ModelMetaclassTester.fields)
+ assert_in(ModelMetaclassTester.field_b, ModelMetaclassTester.fields)
+
+
+def test_parent_metaclass():
+
+ class HasChildren(object):
+ __metaclass__ = ParentModelMetaclass
+
+ has_children = True
+
+ class WithoutChildren(object):
+ __metaclass = ParentModelMetaclass
+
+ assert hasattr(HasChildren, 'children')
+ assert not hasattr(WithoutChildren, 'children')
+
+ assert isinstance(HasChildren.children, List)
+ assert_equals(Scope.settings, HasChildren.children.scope)
+
+
+def test_field_access():
+ class FieldTester(object):
+ __metaclass__ = ModelMetaclass
+
+ field_a = Int(scope=Scope.settings)
+ field_b = Int(scope=Scope.content, default=10)
+ field_c = Int(scope=Scope.student_state, computed_default=lambda s: s.field_a + s.field_b)
+
+ def __init__(self, model_data):
+ self._model_data = model_data
+
+ field_tester = FieldTester({'field_a': 5, 'field_x': 15})
+
+ assert_equals(5, field_tester.field_a)
+ assert_equals(10, field_tester.field_b)
+ assert_equals(15, field_tester.field_c)
+ assert not hasattr(field_tester, 'field_x')
+
+ field_tester.field_a = 20
+ assert_equals(20, field_tester._model_data['field_a'])
+ assert_equals(10, field_tester.field_b)
+ assert_equals(30, field_tester.field_c)
+
+ del field_tester.field_a
+ assert_equals(None, field_tester.field_a)
+ assert hasattr(FieldTester, 'field_a')
+
+
+class TestNamespace(Namespace):
+ field_x = List(scope=Scope.content)
+ field_y = String(scope=Scope.student_state, default="default_value")
+
+
+@patch('xmodule.model.Namespace.load_classes', return_value=[('test', TestNamespace)])
+def test_namespace_metaclass(mock_load_classes):
+ class TestClass(object):
+ __metaclass__ = NamespacesMetaclass
+
+ assert hasattr(TestClass, 'test')
+ assert hasattr(TestClass.test, 'field_x')
+ assert hasattr(TestClass.test, 'field_y')
+
+ assert_in(TestNamespace.field_x, TestClass.test.fields)
+ assert_in(TestNamespace.field_y, TestClass.test.fields)
+ assert isinstance(TestClass.test, Namespace)
+
+
+@patch('xmodule.model.Namespace.load_classes', return_value=[('test', TestNamespace)])
+def test_namespace_field_access(mock_load_classes):
+ class Metaclass(ModelMetaclass, NamespacesMetaclass):
+ pass
+
+ class FieldTester(object):
+ __metaclass__ = Metaclass
+
+ field_a = Int(scope=Scope.settings)
+ field_b = Int(scope=Scope.content, default=10)
+ field_c = Int(scope=Scope.student_state, computed_default=lambda s: s.field_a + s.field_b)
+
+ def __init__(self, model_data):
+ self._model_data = model_data
+
+ field_tester = FieldTester({
+ 'field_a': 5,
+ 'field_x': [1, 2, 3],
+ })
+
+ assert_equals(5, field_tester.field_a)
+ assert_equals(10, field_tester.field_b)
+ assert_equals(15, field_tester.field_c)
+ assert_equals([1, 2, 3], field_tester.test.field_x)
+ assert_equals('default_value', field_tester.test.field_y)
+
+ field_tester.test.field_x = ['a', 'b']
+ assert_equals(['a', 'b'], field_tester._model_data['field_x'])
+
+ del field_tester.test.field_x
+ assert_equals(None, field_tester.test.field_x)
+
+ assert_raises(AttributeError, getattr, field_tester.test, 'field_z')
+ assert_raises(AttributeError, delattr, field_tester.test, 'field_z')
+
+ # Namespaces are created on the fly, so setting a new attribute on one
+ # has no long-term effect
+ field_tester.test.field_z = 'foo'
+ assert_raises(AttributeError, getattr, field_tester.test, 'field_z')
+ assert 'field_z' not in field_tester._model_data
+
+
+def test_field_serialization():
+
+ class CustomField(ModelType):
+ def from_json(self, value):
+ return value['value']
+
+ def to_json(self, value):
+ return {'value': value}
+
+ class FieldTester(object):
+ __metaclass__ = ModelMetaclass
+
+ field = CustomField()
+
+ def __init__(self, model_data):
+ self._model_data = model_data
+
+ field_tester = FieldTester({
+ 'field': {'value': 4}
+ })
+
+ assert_equals(4, field_tester.field)
+ field_tester.field = 5
+ assert_equals({'value': 5}, field_tester._model_data['field'])
diff --git a/common/lib/xmodule/xmodule/tests/test_runtime.py b/common/lib/xmodule/xmodule/tests/test_runtime.py
index e69de29bb2..1bbe3235b8 100644
--- a/common/lib/xmodule/xmodule/tests/test_runtime.py
+++ b/common/lib/xmodule/xmodule/tests/test_runtime.py
@@ -0,0 +1,67 @@
+from nose.tools import assert_equals
+from mock import patch
+
+from xmodule.model import *
+from xmodule.runtime import *
+
+class Metaclass(NamespacesMetaclass, ParentModelMetaclass, ModelMetaclass):
+ pass
+
+
+class TestNamespace(Namespace):
+ n_content = String(scope=Scope.content, default='nc')
+ n_settings = String(scope=Scope.settings, default='ns')
+ n_student_state = String(scope=Scope.student_state, default='nss')
+ n_student_preferences = String(scope=Scope.student_preferences, default='nsp')
+ n_student_info = String(scope=Scope.student_info, default='nsi')
+ n_by_type = String(scope=Scope(False, ModuleScope.TYPE), default='nbt')
+ n_for_all = String(scope=Scope(False, ModuleScope.ALL), default='nfa')
+ n_student_def = String(scope=Scope(True, ModuleScope.DEFINITION), default='nsd')
+
+
+with patch('xmodule.model.Namespace.load_classes', return_value=[('test', TestNamespace)]):
+ class TestModel(object):
+ __metaclass__ = Metaclass
+
+ content = String(scope=Scope.content, default='c')
+ settings = String(scope=Scope.settings, default='s')
+ student_state = String(scope=Scope.student_state, default='ss')
+ student_preferences = String(scope=Scope.student_preferences, default='sp')
+ student_info = String(scope=Scope.student_info, default='si')
+ by_type = String(scope=Scope(False, ModuleScope.TYPE), default='bt')
+ for_all = String(scope=Scope(False, ModuleScope.ALL), default='fa')
+ student_def = String(scope=Scope(True, ModuleScope.DEFINITION), default='sd')
+
+ def __init__(self, model_data):
+ self._model_data = model_data
+
+
+class DictKeyValueStore(KeyValueStore):
+ def __init__(self):
+ self.db = {}
+
+ def get(self, key):
+ return self.db[key]
+
+ def set(self, key, value):
+ self.db[key] = value
+
+ def delete(self, key):
+ del self.db[key]
+
+
+Usage = namedtuple('Usage', 'id, def_id')
+
+
+def test_empty():
+ tester = TestModel(DbModel(DictKeyValueStore(), TestModel, 's0', Usage('u0', 'd0')))
+
+ for collection in (tester, tester.test):
+ for field in collection.fields:
+ print "Getting %s from %r" % (field.name, collection)
+ assert_equals(field.default, getattr(collection, field.name))
+ new_value = 'new ' + field.name
+ print "Setting %s to %s on %r" % (field.name, new_value, collection)
+ setattr(collection, field.name, new_value)
+ print "Checking %s on %r" % (field.name, collection)
+ assert_equals(new_value, getattr(collection, field.name))
From b1c9b2ca0effda8ebe4e31b6a5247c7fe80eb8ab Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Thu, 13 Dec 2012 16:02:23 -0500
Subject: [PATCH 0019/1383] Add more tests of DbModel
---
.../lib/xmodule/xmodule/tests/test_runtime.py | 50 ++++++++++++++++---
1 file changed, 44 insertions(+), 6 deletions(-)
diff --git a/common/lib/xmodule/xmodule/tests/test_runtime.py b/common/lib/xmodule/xmodule/tests/test_runtime.py
index 1bbe3235b8..e71c78e4af 100644
--- a/common/lib/xmodule/xmodule/tests/test_runtime.py
+++ b/common/lib/xmodule/xmodule/tests/test_runtime.py
@@ -4,6 +4,7 @@ from mock import patch
from xmodule.model import *
from xmodule.runtime import *
+
class Metaclass(NamespacesMetaclass, ParentModelMetaclass, ModelMetaclass):
pass
@@ -53,15 +54,52 @@ class DictKeyValueStore(KeyValueStore):
Usage = namedtuple('Usage', 'id, def_id')
-def test_empty():
+def check_field(collection, field):
+ print "Getting %s from %r" % (field.name, collection)
+ assert_equals(field.default, getattr(collection, field.name))
+ new_value = 'new ' + field.name
+ print "Setting %s to %s on %r" % (field.name, new_value, collection)
+ setattr(collection, field.name, new_value)
+ print "Checking %s on %r" % (field.name, collection)
+ assert_equals(new_value, getattr(collection, field.name))
+ print "Deleting %s from %r" % (field.name, collection)
+ delattr(collection, field.name)
+ print "Back to defaults for %s in %r" % (field.name, collection)
+ assert_equals(field.default, getattr(collection, field.name))
+
+
+def test_namespace_actions():
tester = TestModel(DbModel(DictKeyValueStore(), TestModel, 's0', Usage('u0', 'd0')))
for collection in (tester, tester.test):
for field in collection.fields:
- print "Getting %s from %r" % (field.name, collection)
- assert_equals(field.default, getattr(collection, field.name))
+ yield check_field, collection, field
+
+
+def test_db_model_keys():
+ key_store = DictKeyValueStore()
+ tester = TestModel(DbModel(key_store, TestModel, 's0', Usage('u0', 'd0')))
+
+ for collection in (tester, tester.test):
+ for field in collection.fields:
new_value = 'new ' + field.name
- print "Setting %s to %s on %r" % (field.name, new_value, collection)
setattr(collection, field.name, new_value)
- print "Checking %s on %r" % (field.name, collection)
- assert_equals(new_value, getattr(collection, field.name))
+
+ print key_store.db
+ assert_equals('new content', key_store.db[KeyValueStore.Key(Scope.content, None, 'd0', 'content')])
+ assert_equals('new settings', key_store.db[KeyValueStore.Key(Scope.settings, None, 'u0', 'settings')])
+ assert_equals('new student_state', key_store.db[KeyValueStore.Key(Scope.student_state, 's0', 'u0', 'student_state')])
+ assert_equals('new student_preferences', key_store.db[KeyValueStore.Key(Scope.student_preferences, 's0', 'TestModel', 'student_preferences')])
+ assert_equals('new student_info', key_store.db[KeyValueStore.Key(Scope.student_info, 's0', None, 'student_info')])
+ assert_equals('new by_type', key_store.db[KeyValueStore.Key(Scope(False, ModuleScope.TYPE), None, 'TestModel', 'by_type')])
+ assert_equals('new for_all', key_store.db[KeyValueStore.Key(Scope(False, ModuleScope.ALL), None, None, 'for_all')])
+ assert_equals('new student_def', key_store.db[KeyValueStore.Key(Scope(True, ModuleScope.DEFINITION), 's0', 'd0', 'student_def')])
+
+ assert_equals('new n_content', key_store.db[KeyValueStore.Key(Scope.content, None, 'd0', 'n_content')])
+ assert_equals('new n_settings', key_store.db[KeyValueStore.Key(Scope.settings, None, 'u0', 'n_settings')])
+ assert_equals('new n_student_state', key_store.db[KeyValueStore.Key(Scope.student_state, 's0', 'u0', 'n_student_state')])
+ assert_equals('new n_student_preferences', key_store.db[KeyValueStore.Key(Scope.student_preferences, 's0', 'TestModel', 'n_student_preferences')])
+ assert_equals('new n_student_info', key_store.db[KeyValueStore.Key(Scope.student_info, 's0', None, 'n_student_info')])
+ assert_equals('new n_by_type', key_store.db[KeyValueStore.Key(Scope(False, ModuleScope.TYPE), None, 'TestModel', 'n_by_type')])
+ assert_equals('new n_for_all', key_store.db[KeyValueStore.Key(Scope(False, ModuleScope.ALL), None, None, 'n_for_all')])
+ assert_equals('new n_student_def', key_store.db[KeyValueStore.Key(Scope(True, ModuleScope.DEFINITION), 's0', 'd0', 'n_student_def')])
From 7977491ddf5ba8dadc3d45b8072e75b9a03d7238 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Fri, 14 Dec 2012 09:04:34 -0500
Subject: [PATCH 0020/1383] WIP: LMS Courseware appears to be working
---
common/lib/xmodule/xmodule/course_module.py | 5 ++
.../lib/xmodule/xmodule/discussion_module.py | 6 +++
lms/djangoapps/django_comment_client/utils.py | 8 ++-
lms/templates/staff_problem_info.html | 51 +++++++++----------
4 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index b8d4032a18..596344d934 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -36,6 +36,11 @@ class CourseDescriptor(SequenceDescriptor):
tabs = List(help="List of tabs to enable in this course", scope=Scope.settings)
end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings)
discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings, default=[])
+ discussion_topics = Object(
+ help="Map of topics names to ids",
+ scope=Scope.settings,
+ computed_default=lambda c: {'General': {'id': c.location.html_id()}},
+ )
has_children = True
info_sidebar_name = String(scope=Scope.settings, default='Course Handouts')
diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py
index 19a2b4300f..b373f337fb 100644
--- a/common/lib/xmodule/xmodule/discussion_module.py
+++ b/common/lib/xmodule/xmodule/discussion_module.py
@@ -15,6 +15,7 @@ class DiscussionModule(XModule):
discussion_id = String(scope=Scope.settings)
discussion_category = String(scope=Scope.settings)
discussion_target = String(scope=Scope.settings)
+ sort_key = String(scope=Scope.settings)
data = String(help="XML definition of inline discussion", scope=Scope.content)
@@ -42,3 +43,8 @@ class DiscussionDescriptor(RawDescriptor):
metadata_translations = dict(RawDescriptor.metadata_translations)
metadata_translations['id'] = 'discussion_id'
metadata_translations['for'] = 'discussion_target'
+
+ discussion_id = String(scope=Scope.settings)
+ discussion_category = String(scope=Scope.settings)
+ discussion_target = String(scope=Scope.settings)
+ sort_key = String(scope=Scope.settings)
diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py
index 174805a999..bb91dfaa28 100644
--- a/lms/djangoapps/django_comment_client/utils.py
+++ b/lms/djangoapps/django_comment_client/utils.py
@@ -145,12 +145,12 @@ def initialize_discussion_info(course):
id = module.discussion_id
category = module.discussion_category
title = module.discussion_target
- sort_key = module.metadata.get('sort_key', title)
+ sort_key = module.sort_key
category = " / ".join([x.strip() for x in category.split("/")])
last_category = category.split("/")[-1]
discussion_id_map[id] = {"location": location, "title": last_category + " / " + title}
unexpanded_category_map[category].append({"title": title, "id": id,
- "sort_key": sort_key, "start_date": module.start})
+ "sort_key": sort_key, "start_date": module.lms.start})
category_map = {"entries": defaultdict(dict), "subcategories": defaultdict(dict)}
for category_path, entries in unexpanded_category_map.items():
@@ -189,9 +189,7 @@ def initialize_discussion_info(course):
"sort_key": entry["sort_key"],
"start_date": entry["start_date"]}
- default_topics = {'General': {'id' :course.location.html_id()}}
- discussion_topics = course.metadata.get('discussion_topics', default_topics)
- for topic, entry in discussion_topics.items():
+ for topic, entry in course.discussion_topics.items():
category_map['entries'][topic] = {"id": entry["id"],
"sort_key": entry.get("sort_key", topic),
"start_date": time.gmtime()}
diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html
index e0c957a05c..4fee7c22fb 100644
--- a/lms/templates/staff_problem_info.html
+++ b/lms/templates/staff_problem_info.html
@@ -46,22 +46,18 @@ github = ${edit_link | h}
%if source_file:
source_url = ${source_file | h}
%endif
-
-
+
+ | Module Fields |
%for name, field in fields:
-
- ${name} =
${field | h}
-
+ | ${name} | ${field | h} |
%endfor
-
-
-
+
+
+ | edX Fields |
%for name, field in lms_fields:
-
- ${name} =
${field | h}
-
+ | ${name} | ${field | h} |
%endfor
-
+
category = ${category | h}
%if render_histogram:
@@ -75,18 +71,21 @@ category = ${category | h}
From 8991cdd3b5560a13680706d6d510f53d97819e74 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:12:39 -0500
Subject: [PATCH 0021/1383] Stop raising BaseExceptions
---
cms/djangoapps/contentstore/utils.py | 4 ++--
common/djangoapps/static_replace.py | 2 +-
common/lib/xmodule/xmodule/modulestore/mongo.py | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py
index da2993e463..d6dcc3811d 100644
--- a/cms/djangoapps/contentstore/utils.py
+++ b/cms/djangoapps/contentstore/utils.py
@@ -37,10 +37,10 @@ def get_course_location_for_item(location):
# make sure we found exactly one match on this above course search
found_cnt = len(courses)
if found_cnt == 0:
- raise BaseException('Could not find course at {0}'.format(course_search_location))
+ raise Exception('Could not find course at {0}'.format(course_search_location))
if found_cnt > 1:
- raise BaseException('Found more than one course at {0}. There should only be one!!! Dump = {1}'.format(course_search_location, courses))
+ raise Exception('Found more than one course at {0}. There should only be one!!! Dump = {1}'.format(course_search_location, courses))
location = courses[0].location
diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py
index e75362d784..3bc8181126 100644
--- a/common/djangoapps/static_replace.py
+++ b/common/djangoapps/static_replace.py
@@ -49,7 +49,7 @@ def replace(static_url, prefix=None, course_namespace=None):
# use the utility functions in StaticContent.py
if static_url.group('prefix') == '/static/' and not isinstance(modulestore(), XMLModuleStore):
if course_namespace is None:
- raise BaseException('You must pass in course_namespace when remapping static content urls with MongoDB stores')
+ raise Exception('You must pass in course_namespace when remapping static content urls with MongoDB stores')
url = StaticContent.convert_legacy_static_url(static_url.group('rest'), course_namespace)
else:
url = try_staticfiles_lookup(prefix + static_url.group('rest'))
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index c0ba73423c..c40bde1072 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -314,10 +314,10 @@ class MongoModuleStore(ModuleStoreBase):
# make sure we found exactly one match on this above course search
found_cnt = len(courses)
if found_cnt == 0:
- raise BaseException('Could not find course at {0}'.format(course_search_location))
+ raise Exception('Could not find course at {0}'.format(course_search_location))
if found_cnt > 1:
- raise BaseException('Found more than one course at {0}. There should only be one!!! Dump = {1}'.format(course_search_location, courses))
+ raise Exception('Found more than one course at {0}. There should only be one!!! Dump = {1}'.format(course_search_location, courses))
return courses[0]
From d11a9b1d2104551baf115f19ce7b84da89319fd9 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:14:48 -0500
Subject: [PATCH 0022/1383] Convert ABTest module to new property based format.
Doesn't account for definition_to_xml needing redefinition
---
common/lib/xmodule/xmodule/abtest_module.py | 65 +++++++++++----------
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py
index 0f655ded6c..4ee74eb29c 100644
--- a/common/lib/xmodule/xmodule/abtest_module.py
+++ b/common/lib/xmodule/xmodule/abtest_module.py
@@ -1,4 +1,3 @@
-import json
import random
import logging
from lxml import etree
@@ -7,7 +6,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
+from .model import String, Scope, Object, ModuleScope
DEFAULT = "_DEFAULT_GROUP"
@@ -37,25 +36,34 @@ class ABTestModule(XModule):
Implements an A/B test with an aribtrary number of competing groups
"""
- 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)
+ group_portions = Object(help="What proportions of students should go in each group", default={DEFAULT: 1}, scope=Scope.content)
+ group_assignments = Object(help="What group this user belongs to", scope=Scope(student=True, module=ModuleScope.TYPE), default={})
+ group_content = Object(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []})
- if shared_state is None:
+ def __init__(self, *args, **kwargs):
+ XModule.__init__(self, *args, **kwargs)
+ if self.group is None:
self.group = group_from_value(
- self.definition['data']['group_portions'].items(),
+ self.group_portions,
random.uniform(0, 1)
)
- else:
- shared_state = json.loads(shared_state)
- self.group = shared_state['group']
- def get_shared_state(self):
- return json.dumps({'group': self.group})
-
+ @property
+ def group(self):
+ return self.group_assignments.get(self.experiment)
+
+ @group.setter
+ def group(self, value):
+ self.group_assigments[self.experiment] = value
+
+ @group.deleter
+ def group(self):
+ del self.group_assignments[self.experiment]
+
def get_children_locations(self):
- return self.definition['data']['group_content'][self.group]
-
+ return self.group_content[self.group]
+
def displayable_items(self):
# Most modules return "self" as the displayable_item. We never display ourself
# (which is why we don't implement get_html). We only display our children.
@@ -70,6 +78,9 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
template_dir_name = "abtest"
experiment = String(help="Experiment that this A/B test belongs to", scope=Scope.content)
+ group_portions = Object(help="What proportions of students should go in each group", default={})
+ group_assignments = Object(help="What group this user belongs to", scope=Scope(student=True, module=ModuleScope.TYPE), default={})
+ group_content = Object(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []})
@classmethod
def definition_from_xml(cls, xml_object, system):
@@ -88,19 +99,12 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
"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': {DEFAULT: []},
- },
- 'children': []}
for group in xml_object:
if group.tag == 'default':
name = DEFAULT
else:
name = group.get('name')
- definition['data']['group_portions'][name] = float(group.get('portion', 0))
+ self.group_portions[name] = float(group.get('portion', 0))
child_content_urls = []
for child in group:
@@ -110,8 +114,8 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
log.exception("Unable to load child when parsing ABTest. Continuing...")
continue
- definition['data']['group_content'][name] = child_content_urls
- definition['children'].extend(child_content_urls)
+ self.group_content[name] = child_content_urls
+ self.children.extend(child_content_urls)
default_portion = 1 - sum(
portion for (name, portion) in definition['data']['group_portions'].items())
@@ -119,20 +123,20 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
if default_portion < 0:
raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1")
- definition['data']['group_portions'][DEFAULT] = default_portion
- definition['children'].sort()
+ self.group_portions[DEFAULT] = default_portion
+ self.children.sort()
return definition
def definition_to_xml(self, resource_fs):
xml_object = etree.Element('abtest')
- xml_object.set('experiment', self.definition['data']['experiment'])
- for name, group in self.definition['data']['group_content'].items():
+ xml_object.set('experiment', self.experiment)
+ for name, group in self.group_content.items():
if name == DEFAULT:
group_elem = etree.SubElement(xml_object, 'default')
else:
group_elem = etree.SubElement(xml_object, 'group', attrib={
- 'portion': str(self.definition['data']['group_portions'][name]),
+ 'portion': str(self.group_portions[name]),
'name': name,
})
@@ -141,7 +145,6 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
group_elem.append(etree.fromstring(child.export_to_xml(resource_fs)))
return xml_object
-
-
+
def has_dynamic_children(self):
return True
From 040abfcc6820b44e38393638a36d6d14f9cb1a4d Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:18:27 -0500
Subject: [PATCH 0023/1383] Don't call update_metadata anymore when updating
course tabs, because the updates are implicit
---
common/lib/xmodule/xmodule/modulestore/mongo.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py
index c40bde1072..d145523d16 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo.py
@@ -380,7 +380,6 @@ class MongoModuleStore(ModuleStoreBase):
tab['name'] = metadata.get('display_name')
break
course.tabs = existing_tabs
- self.update_metadata(course.location, course.metadata)
self._update_single_item(location, {'metadata': metadata})
From 8c42e0f52ed3b8c051c79504bac61c085a6ad100 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:18:53 -0500
Subject: [PATCH 0024/1383] Import course objects first, so that later updates
that try and edit the course object don't fail
---
.../xmodule/modulestore/xml_importer.py | 171 ++++++++++--------
1 file changed, 91 insertions(+), 80 deletions(-)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index f90291aaa2..1132fe77bb 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -89,6 +89,91 @@ def verify_content_links(module, base_dir, static_content_store, link, remap_dic
return link
+
+def import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None):
+ # remap module to the new namespace
+ if target_location_namespace is not None:
+ # This looks a bit wonky as we need to also change the 'name' of the imported course to be what
+ # the caller passed in
+ if module.location.category != 'course':
+ module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
+ course=target_location_namespace.course)
+ else:
+ module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
+ course=target_location_namespace.course, name=target_location_namespace.name)
+
+ # then remap children pointers since they too will be re-namespaced
+ children_locs = module.definition.get('children')
+ if children_locs is not None:
+ new_locs = []
+ for child in children_locs:
+ child_loc = Location(child)
+ new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
+ course=target_location_namespace.course)
+
+ new_locs.append(new_child_loc.url())
+
+ module.definition['children'] = new_locs
+
+ if hasattr(module, 'data'):
+ # cdodge: now go through any link references to '/static/' and make sure we've imported
+ # it as a StaticContent asset
+ try:
+ remap_dict = {}
+
+ # use the rewrite_links as a utility means to enumerate through all links
+ # in the module data. We use that to load that reference into our asset store
+ # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to
+ # do the rewrites natively in that code.
+ # For example, what I'm seeing is
->
+ # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's
+ # no good, so we have to do this kludge
+ if isinstance(module.data, str) or isinstance(module.data, unicode): # some module 'data' fields are non strings which blows up the link traversal code
+ lxml_rewrite_links(module.data, lambda link: verify_content_links(module, course_data_path,
+ static_content_store, link, remap_dict))
+
+ for key in remap_dict.keys():
+ module.data = module.data.replace(key, remap_dict[key])
+
+ except Exception:
+ logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
+
+ modulestore.update_item(module.location, module.data)
+
+ if module.has_children:
+ modulestore.update_children(module.location, module.children)
+
+ metadata = {}
+ for field in module.fields + module.lms.fields:
+ # Only save metadata that wasn't inherited
+ if (field.scope == Scope.settings and
+ field.name in module._inherited_metadata and
+ field.name in module._model_data):
+
+ metadata[field.name] = module._model_data[field.name]
+ modulestore.update_metadata(module.location, metadata)
+
+
+def import_course_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None):
+ # HACK: for now we don't support progress tabs. There's a special metadata configuration setting for this.
+ module.hide_progress_tab = True
+
+ # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which
+ # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS,
+ # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that -
+ # if there is *any* tabs - then there at least needs to be some predefined ones
+ if module.tabs is None or len(module.tabs) == 0:
+ module.tabs = [{"type": "courseware"},
+ {"type": "course_info", "name": "Course Info"},
+ {"type": "discussion", "name": "Discussion"},
+ {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge
+
+ # a bit of a hack, but typically the "course image" which is shown on marketing pages is hard coded to /images/course_image.jpg
+ # so let's make sure we import in case there are no other references to it in the modules
+ verify_content_links(module, course_data_path, static_content_store, '/static/images/course_image.jpg')
+ import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace)
+
+
def import_from_xml(store, data_dir, course_dirs=None,
default_class='xmodule.raw_module.RawDescriptor',
load_error_modules=True, static_content_store=None, target_location_namespace=None):
@@ -134,89 +219,15 @@ def import_from_xml(store, data_dir, course_dirs=None,
import_static_content(module_store.modules[course_id], course_location, course_data_path, static_content_store,
_namespace_rename, subpath='static')
+ # Import course modules first, because importing some of the children requires the course to exist
for module in module_store.modules[course_id].itervalues():
-
- # remap module to the new namespace
- if target_location_namespace is not None:
- # This looks a bit wonky as we need to also change the 'name' of the imported course to be what
- # the caller passed in
- if module.location.category != 'course':
- module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
- course=target_location_namespace.course)
- else:
- module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
- course=target_location_namespace.course, name=target_location_namespace.name)
-
- # then remap children pointers since they too will be re-namespaced
- children_locs = module.definition.get('children')
- if children_locs is not None:
- new_locs = []
- for child in children_locs:
- child_loc = Location(child)
- new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
- course=target_location_namespace.course)
-
- new_locs.append(new_child_loc.url())
-
- module.definition['children'] = new_locs
-
if module.category == 'course':
- # HACK: for now we don't support progress tabs. There's a special metadata configuration setting for this.
- module.hide_progress_tab = True
+ import_course_from_xml(store, static_content_store, course_data_path, module, target_location_namespace)
- # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which
- # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS,
- # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that -
- # if there is *any* tabs - then there at least needs to be some predefined ones
- if module.tabs is None or len(module.tabs) == 0:
- module.tabs = [{"type": "courseware"},
- {"type": "course_info", "name": "Course Info"},
- {"type": "discussion", "name": "Discussion"},
- {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge
-
- # a bit of a hack, but typically the "course image" which is shown on marketing pages is hard coded to /images/course_image.jpg
- # so let's make sure we import in case there are no other references to it in the modules
- verify_content_links(module, course_data_path, static_content_store, '/static/images/course_image.jpg')
-
- course_items.append(module)
-
- if hasattr(module, 'data'):
- # cdodge: now go through any link references to '/static/' and make sure we've imported
- # it as a StaticContent asset
- try:
- remap_dict = {}
-
- # use the rewrite_links as a utility means to enumerate through all links
- # in the module data. We use that to load that reference into our asset store
- # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to
- # do the rewrites natively in that code.
- # For example, what I'm seeing is
->
- # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's
- # no good, so we have to do this kludge
- if isinstance(module.data, str) or isinstance(module.data, unicode): # some module 'data' fields are non strings which blows up the link traversal code
- lxml_rewrite_links(module.data, lambda link: verify_content_links(module, course_data_path,
- static_content_store, link, remap_dict))
-
- for key in remap_dict.keys():
- module.data = module.data.replace(key, remap_dict[key])
-
- except Exception:
- logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
-
- store.update_item(module.location, module.data)
-
- if module.has_children:
- store.update_children(module.location, module.children)
-
- metadata = {}
- for field in module.fields + module.lms.fields:
- # Only save metadata that wasn't inherited
- if (field.scope == Scope.settings and
- field.name in module._inherited_metadata and
- field.name in module._model_data):
-
- metadata[field.name] = module._model_data[field.name]
- store.update_metadata(module.location, metadata)
+ # Import the rest of the modules
+ for module in module_store.modules[course_id].itervalues():
+ if module.category != 'course':
+ import_module_from_xml(store, static_content_store, course_data_path, module, target_location_namespace)
return module_store, course_items
From 5cb31c0e325054c294999c2fc920f7d57a97b8a9 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:19:18 -0500
Subject: [PATCH 0025/1383] Make load_from_json on descriptors pass the right
sort of arguments
---
common/lib/xmodule/xmodule/x_module.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index f03fbe6183..a38b8e5549 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -456,7 +456,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
system: A DescriptorSystem for interacting with external resources
"""
- return cls(system=system, **json_data)
+ return cls(system=system, location=json_data['location'], model_data=json_data)
# ================================= XML PARSING ============================
@staticmethod
From 3adb1e71090adcdf1b64872198d4d2c4dd903602 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:19:38 -0500
Subject: [PATCH 0026/1383] Make grading not require get_instance_module
---
lms/djangoapps/courseware/grades.py | 39 ++++++++++++++++-------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index 032378f863..b81147f905 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -9,7 +9,7 @@ from django.conf import settings
from django.contrib.auth.models import User
from models import StudentModuleCache
-from module_render import get_module, get_instance_module
+from module_render import get_module
from xmodule import graders
from xmodule.capa_module import CapaModule
from xmodule.course_module import CourseDescriptor
@@ -338,6 +338,9 @@ def get_score(course_id, user, problem_descriptor, module_creator, student_modul
Can return None if user doesn't have access, or if something else went wrong.
cache: A StudentModuleCache
"""
+ if not user.is_authenticated():
+ return (None, None)
+
if not (problem_descriptor.stores_state and problem_descriptor.has_score):
# These are not problems, and do not have a score
return (None, None)
@@ -347,29 +350,29 @@ def get_score(course_id, user, problem_descriptor, module_creator, student_modul
instance_module = student_module_cache.lookup(
course_id, problem_descriptor.category, problem_descriptor.location.url())
- if not instance_module:
+ if instance_module:
+ if instance_module.max_grade is None:
+ return (None, None)
+
+ correct = instance_module.grade if instance_module.grade is not None else 0
+ total = instance_module.max_grade
+ else:
# If the problem was not in the cache, we need to instantiate the problem.
# Otherwise, the max score (cached in instance_module) won't be available
problem = module_creator(problem_descriptor)
if problem is None:
return (None, None)
- instance_module = get_instance_module(course_id, user, problem, student_module_cache)
- # If this problem is ungraded/ungradable, bail
- if not instance_module or instance_module.max_grade is None:
- return (None, None)
+ correct = 0
+ total = problem.max_score()
- 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_descriptor, 'weight', None)
- if weight is not None:
- if total == 0:
- log.exception("Cannot reweight a problem with zero total points. Problem: " + str(instance_module))
- return (correct, total)
- correct = correct * weight / total
- total = weight
+ #Now we re-weight the problem, if specified
+ weight = getattr(problem_descriptor, 'weight', None)
+ if weight is not None:
+ if total == 0:
+ log.exception("Cannot reweight a problem with zero total points. Problem: " + str(instance_module))
+ return (correct, total)
+ correct = correct * weight / total
+ total = weight
return (correct, total)
From 306dbcff9c708f89572148f0b1d661d2193da68c Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:19:53 -0500
Subject: [PATCH 0027/1383] Rationalize StudentModule unicode and repr strings
---
lms/djangoapps/courseware/models.py | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py
index 2b7b12ac45..9c318427ec 100644
--- a/lms/djangoapps/courseware/models.py
+++ b/lms/djangoapps/courseware/models.py
@@ -60,13 +60,17 @@ class StudentModule(models.Model):
created = models.DateTimeField(auto_now_add=True, db_index=True)
modified = models.DateTimeField(auto_now=True, db_index=True)
- def __unicode__(self):
- return '/'.join([self.course_id, self.module_type,
- self.student.username, self.module_state_key, str(self.state)[:20]])
-
def __repr__(self):
- return 'StudentModule%r' % ((self.course_id, self.module_type, self.student, self.module_state_key, str(self.state)[:20]),)
+ return 'StudentModule<%r>' % ({
+ 'course_id': self.course_id,
+ 'module_type': self.module_type,
+ 'student': self.student.username,
+ 'module_state_key': self.module_state_key,
+ 'state': str(self.state)[:20],
+ },)
+ def __unicode__(self):
+ return unicode(repr(self))
# TODO (cpennington): Remove these once the LMS switches to using XModuleDescriptors
From 13cab34a7d8c384bc92fda9101a90d36ed260f2e Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:20:33 -0500
Subject: [PATCH 0028/1383] Always use the url form of the location when making
StudentModules
---
lms/djangoapps/courseware/module_render.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index be207730b3..c9e46c7ce0 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -186,7 +186,7 @@ class LmsKeyValueStore(KeyValueStore):
course_id=self._course_id,
student=self._user,
module_type=key.module_scope_id.category,
- module_state_key=key.module_scope_id,
+ module_state_key=key.module_scope_id.url(),
state=json.dumps({})
)
self._student_module_cache.append(student_module)
From 64848a32ee53f23adb27237e9aff7663df6e35e7 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:20:55 -0500
Subject: [PATCH 0029/1383] Delete get_instance_module and
get_shared_instance_module, as they are obsolete with the new properties
---
lms/djangoapps/courseware/module_render.py | 59 ----------------------
lms/djangoapps/courseware/views.py | 2 +-
2 files changed, 1 insertion(+), 60 deletions(-)
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index c9e46c7ce0..ec9c6b54eb 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -341,65 +341,6 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
return module
-# TODO (vshnayder): Rename this? It's very confusing.
-def get_instance_module(course_id, user, module, student_module_cache):
- """
- Returns the StudentModule specific to this module for this student,
- or None if this is an anonymous user
- """
- if user.is_authenticated():
- if not module.descriptor.stores_state:
- log.exception("Attempted to get the instance_module for a module "
- + str(module.id) + " which does not store state.")
- return None
-
- instance_module = student_module_cache.lookup(
- course_id, module.category, module.location.url())
-
- if not instance_module:
- instance_module = StudentModule(
- course_id=course_id,
- student=user,
- module_type=module.category,
- module_state_key=module.id,
- state=module.get_instance_state(),
- max_grade=module.max_score())
- instance_module.save()
- student_module_cache.append(instance_module)
-
- return instance_module
- else:
- return None
-
-def get_shared_instance_module(course_id, user, module, student_module_cache):
- """
- Return shared_module is a StudentModule specific to all modules with the same
- 'shared_state_key' attribute, or None if the module does not elect to
- share state
- """
- if user.is_authenticated():
- # To get the shared_state_key, we need to descriptor
- descriptor = modulestore().get_instance(course_id, module.location)
-
- shared_state_key = getattr(module, 'shared_state_key', None)
- if shared_state_key is not None:
- shared_module = student_module_cache.lookup(module.category,
- shared_state_key)
- if not shared_module:
- shared_module = StudentModule(
- course_id=course_id,
- student=user,
- module_type=descriptor.category,
- module_state_key=shared_state_key,
- state=module.get_shared_state())
- shared_module.save()
- student_module_cache.append(shared_module)
- else:
- shared_module = None
-
- return shared_module
- else:
- return None
@csrf_exempt
def xqueue_callback(request, course_id, userid, id, dispatch):
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index fffbd2fc18..0c6e2245b9 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -24,7 +24,7 @@ from courseware.access import has_access
from courseware.courses import (get_course_with_access, get_courses_by_university)
import courseware.tabs as tabs
from courseware.models import StudentModuleCache
-from module_render import toc_for_course, get_module, get_instance_module
+from module_render import toc_for_course, get_module
from student.models import UserProfile
from multicourse import multicourse_settings
From fbd9499c5130d0bdf512cb812ec1c5be01f2b0ee Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:21:24 -0500
Subject: [PATCH 0030/1383] Make debug message use the available request.user
object
---
lms/djangoapps/courseware/module_render.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index ec9c6b54eb..0aac05568c 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -452,7 +452,7 @@ def modx_dispatch(request, dispatch, location, course_id):
if instance is None:
# Either permissions just changed, or someone is trying to be clever
# and load something they shouldn't have access to.
- log.debug("No module {0} for user {1}--access denied?".format(location, user))
+ log.debug("No module {0} for user {1}--access denied?".format(location, request.user))
raise Http404
# Let the module handle the AJAX
From 2879853eee15fe8e600034b57af3f1b9d77a3ff9 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Mon, 17 Dec 2012 13:23:21 -0500
Subject: [PATCH 0031/1383] Pep8 fixes
---
lms/djangoapps/courseware/tests/tests.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py
index 7a6d498660..6b2988d1db 100644
--- a/lms/djangoapps/courseware/tests/tests.py
+++ b/lms/djangoapps/courseware/tests/tests.py
@@ -742,11 +742,11 @@ class TestCourseGrader(PageLoader):
"""
problem_location = "i4x://edX/graded/problem/{0}".format(problem_url_name)
- modx_url = reverse('modx_dispatch',
+ modx_url = reverse('modx_dispatch',
kwargs={
- 'course_id' : self.graded_course.id,
- 'location' : problem_location,
- 'dispatch' : 'problem_check', }
+ 'course_id': self.graded_course.id,
+ 'location': problem_location,
+ 'dispatch': 'problem_check', }
)
resp = self.client.post(modx_url, {
From 8693d288c86d4813d731ec258cbbbb7d6cd7864c Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Tue, 18 Dec 2012 09:32:36 -0500
Subject: [PATCH 0032/1383] Fix errors from running unit tests (some tests
still fail)
---
common/lib/xmodule/xmodule/abtest_module.py | 23 +++++---
common/lib/xmodule/xmodule/course_module.py | 4 +-
common/lib/xmodule/xmodule/html_module.py | 4 +-
common/lib/xmodule/xmodule/raw_module.py | 2 +-
.../xmodule/xmodule/self_assessment_module.py | 2 +-
common/lib/xmodule/xmodule/seq_module.py | 2 +-
common/lib/xmodule/xmodule/x_module.py | 54 ++++++++++++++++---
common/lib/xmodule/xmodule/xml_module.py | 9 ++--
lms/djangoapps/courseware/access.py | 4 +-
lms/djangoapps/courseware/grades.py | 2 +
lms/djangoapps/courseware/module_render.py | 2 +
11 files changed, 78 insertions(+), 30 deletions(-)
diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py
index 4ee74eb29c..9c639e8f30 100644
--- a/common/lib/xmodule/xmodule/abtest_module.py
+++ b/common/lib/xmodule/xmodule/abtest_module.py
@@ -79,7 +79,6 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
experiment = String(help="Experiment that this A/B test belongs to", scope=Scope.content)
group_portions = Object(help="What proportions of students should go in each group", default={})
- group_assignments = Object(help="What group this user belongs to", scope=Scope(student=True, module=ModuleScope.TYPE), default={})
group_content = Object(help="What content to display to each group", scope=Scope.content, default={DEFAULT: []})
@classmethod
@@ -99,12 +98,16 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
"ABTests must specify an experiment. Not found in:\n{xml}"
.format(xml=etree.tostring(xml_object, pretty_print=True)))
+ group_portions = {}
+ group_content = {}
+ children = []
+
for group in xml_object:
if group.tag == 'default':
name = DEFAULT
else:
name = group.get('name')
- self.group_portions[name] = float(group.get('portion', 0))
+ group_portions[name] = float(group.get('portion', 0))
child_content_urls = []
for child in group:
@@ -114,19 +117,23 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor):
log.exception("Unable to load child when parsing ABTest. Continuing...")
continue
- self.group_content[name] = child_content_urls
- self.children.extend(child_content_urls)
+ group_content[name] = child_content_urls
+ children.extend(child_content_urls)
default_portion = 1 - sum(
- portion for (name, portion) in definition['data']['group_portions'].items())
+ portion for (name, portion) in group_portions.items()
+ )
if default_portion < 0:
raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1")
- self.group_portions[DEFAULT] = default_portion
- self.children.sort()
+ group_portions[DEFAULT] = default_portion
+ children.sort()
- return definition
+ return {
+ 'group_portions': group_portions,
+ 'group_content': group_content,
+ }, children
def definition_to_xml(self, resource_fs):
xml_object = etree.Element('abtest')
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 596344d934..510857303a 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -270,12 +270,12 @@ class CourseDescriptor(SequenceDescriptor):
wiki_slug = wiki_tag.attrib.get("slug", default=None)
xml_object.remove(wiki_tag)
- definition = super(CourseDescriptor, cls).definition_from_xml(xml_object, system)
+ definition, children = super(CourseDescriptor, cls).definition_from_xml(xml_object, system)
definition.setdefault('data', {})['textbooks'] = textbooks
definition['data']['wiki_slug'] = wiki_slug
- return definition
+ return definition, children
def has_ended(self):
"""
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 6d86fb90a8..562eeeb361 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -87,7 +87,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
if filename is None:
definition_xml = copy.deepcopy(xml_object)
cls.clean_metadata_from_xml(definition_xml)
- return {'data': stringify_children(definition_xml)}
+ return {'data': stringify_children(definition_xml)}, []
else:
# html is special. cls.filename_extension is 'xml', but
# if 'filename' is in the definition, that means to load
@@ -131,7 +131,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
# for Fall 2012 LMS migration: keep filename (and unmangled filename)
definition['filename'] = [ filepath, filename ]
- return definition
+ return definition, []
except (ResourceNotFoundError) as err:
msg = 'Unable to load file contents at path {0}: {1} '.format(
diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py
index 5ff16098ac..bdbc049712 100644
--- a/common/lib/xmodule/xmodule/raw_module.py
+++ b/common/lib/xmodule/xmodule/raw_module.py
@@ -13,7 +13,7 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor):
"""
@classmethod
def definition_from_xml(cls, xml_object, system):
- return {'data': etree.tostring(xml_object, pretty_print=True)}
+ return {'data': etree.tostring(xml_object, pretty_print=True)}, []
def definition_to_xml(self, resource_fs):
try:
diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py
index 2f4674cf7c..ca6eae9913 100644
--- a/common/lib/xmodule/xmodule/self_assessment_module.py
+++ b/common/lib/xmodule/xmodule/self_assessment_module.py
@@ -428,7 +428,7 @@ class SelfAssessmentDescriptor(XmlDescriptor, EditingDescriptor):
'prompt': parse('prompt'),
'submitmessage': parse('submitmessage'),
'hintprompt': parse('hintprompt'),
- }
+ }, []
def definition_to_xml(self, resource_fs):
diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py
index 6e80d1cf61..a136473653 100644
--- a/common/lib/xmodule/xmodule/seq_module.py
+++ b/common/lib/xmodule/xmodule/seq_module.py
@@ -136,7 +136,7 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor):
if system.error_tracker is not None:
system.error_tracker("ERROR: " + str(e))
continue
- return {'children': children}
+ return {}, children
def definition_to_xml(self, resource_fs):
xml_object = etree.Element('sequential')
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index a38b8e5549..6c499aa611 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -165,11 +165,8 @@ class XModule(HTMLSnippet):
'''
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]
+ children = [self.system.get_module(loc) for loc in self.get_children_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]
@@ -179,6 +176,24 @@ class XModule(HTMLSnippet):
def __unicode__(self):
return ''.format(self.id)
+ 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.
+ '''
+ if not self.has_children:
+ return []
+
+ return self.children
+
def get_display_items(self):
'''
Returns a list of descendent module instances that will display
@@ -437,7 +452,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
on the contents of json_data.
json_data must contain a 'location' element, and must be suitable to be
- passed into the subclasses `from_json` method.
+ passed into the subclasses `from_json` method as model_data
"""
class_ = XModuleDescriptor.load_class(
json_data['location']['category'],
@@ -451,12 +466,35 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
Creates an instance of this descriptor from the supplied json_data.
This may be overridden by subclasses
- json_data: A json object specifying the definition and any optional
- keyword arguments for the XModuleDescriptor
+ json_data: A json object with the keys 'definition' and 'metadata',
+ definition: A json object with the keys 'data' and 'children'
+ data: A json value
+ children: A list of edX Location urls
+ metadata: A json object with any keys
+
+ This json_data is transformed to model_data using the following rules:
+ 1) The model data contains all of the fields from metadata
+ 2) The model data contains the 'children' array
+ 3) If 'definition.data' is a json object, model data contains all of its fields
+ Otherwise, it contains the single field 'data'
+ 4) Any value later in this list overrides a value earlier in this list
system: A DescriptorSystem for interacting with external resources
"""
- return cls(system=system, location=json_data['location'], model_data=json_data)
+ model_data = {}
+ model_data.update(json_data.get('metadata', {}))
+
+ definition = json_data.get('definition', {})
+ if 'children' in definition:
+ model_data['children'] = definition['children']
+
+ if 'data' in definition:
+ if isinstance(definition['data'], dict):
+ model_data.update(definition['data'])
+ else:
+ model_data['data'] = definition['data']
+
+ return cls(system=system, location=json_data['location'], model_data=model_data)
# ================================= XML PARSING ============================
@staticmethod
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 50c4f7aa6d..754d9b523e 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -220,7 +220,7 @@ class XmlDescriptor(XModuleDescriptor):
definition_metadata = get_metadata_from_xml(definition_xml)
cls.clean_metadata_from_xml(definition_xml)
- definition = cls.definition_from_xml(definition_xml, system)
+ definition, children = cls.definition_from_xml(definition_xml, system)
if definition_metadata:
definition['definition_metadata'] = definition_metadata
@@ -228,7 +228,7 @@ class XmlDescriptor(XModuleDescriptor):
# for Fall 2012 LMS migration: keep filename (and unmangled filename)
definition['filename'] = [ filepath, filename ]
- return definition
+ return definition, children
@classmethod
def load_metadata(cls, xml_object):
@@ -289,7 +289,7 @@ class XmlDescriptor(XModuleDescriptor):
else:
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, children = 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):
@@ -311,13 +311,12 @@ class XmlDescriptor(XModuleDescriptor):
# Set/override any metadata specified by policy
k = policy_key(location)
if k in system.policy:
- if k == 'video/labintro': print k, metadata, system.policy[k]
cls.apply_policy(metadata, system.policy[k])
model_data = {}
model_data.update(metadata)
model_data.update(definition)
- if k == 'video/labintro': print model_data
+ model_data['children'] = children
return cls(
system,
diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py
index c6342e9d13..4318bf81bf 100644
--- a/lms/djangoapps/courseware/access.py
+++ b/lms/djangoapps/courseware/access.py
@@ -226,9 +226,9 @@ def _has_access_descriptor(user, descriptor, action, course_context=None):
return True
# Check start date
- if descriptor.start is not None:
+ if descriptor.lms.start is not None:
now = time.gmtime()
- if now > descriptor.start:
+ if now > descriptor.lms.start:
# after start date, everyone can see it
debug("Allow: now > start date")
return True
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index b81147f905..1d894d3707 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -36,6 +36,8 @@ def yield_dynamic_descriptor_descendents(descriptor, module_creator):
def get_dynamic_descriptor_children(descriptor):
if descriptor.has_dynamic_children():
module = module_creator(descriptor)
+ if module is None:
+ print "FOO", descriptor
child_locations = module.get_children_locations()
return [descriptor.system.load_item(child_location) for child_location in child_locations ]
else:
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index 0aac05568c..3fd1abec55 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -313,9 +313,11 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
import_system = descriptor.system
if has_access(user, location, 'staff', course_id):
err_descriptor = ErrorDescriptor.from_xml(str(descriptor), import_system,
+ org=descriptor.location.org, course=descriptor.location.course,
error_msg=exc_info_to_str(sys.exc_info()))
else:
err_descriptor = NonStaffErrorDescriptor.from_xml(str(descriptor), import_system,
+ org=descriptor.location.org, course=descriptor.location.course,
error_msg=exc_info_to_str(sys.exc_info()))
# Make an error module
From d61c91c139e58d45723862d15c5d5459bee46b11 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Tue, 18 Dec 2012 09:54:25 -0500
Subject: [PATCH 0033/1383] Fix errors around error descriptors and custom tag
modules
---
common/lib/xmodule/xmodule/error_module.py | 8 ++++++--
common/lib/xmodule/xmodule/raw_module.py | 8 ++++++--
common/lib/xmodule/xmodule/template_module.py | 9 ++-------
lms/djangoapps/courseware/views.py | 11 +++--------
4 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py
index 67b52d383c..37e98b5b77 100644
--- a/common/lib/xmodule/xmodule/error_module.py
+++ b/common/lib/xmodule/xmodule/error_module.py
@@ -22,6 +22,10 @@ log = logging.getLogger(__name__)
class ErrorModule(XModule):
+
+ contents = String(scope=Scope.content)
+ error_msg = String(scope=Scope.content)
+
def get_html(self):
'''Show an error to staff.
TODO (vshnayder): proper style, divs, etc.
@@ -29,8 +33,8 @@ class ErrorModule(XModule):
# staff get to see all the details
return self.system.render_template('module-error.html', {
'staff_access': True,
- 'data': self.definition['data']['contents'],
- 'error': self.definition['data']['error_msg'],
+ 'data': self.contents,
+ 'error': self.error_msg,
})
diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py
index bdbc049712..5e50bdf6a0 100644
--- a/common/lib/xmodule/xmodule/raw_module.py
+++ b/common/lib/xmodule/xmodule/raw_module.py
@@ -3,25 +3,29 @@ from xmodule.editing_module import XMLEditingDescriptor
from xmodule.xml_module import XmlDescriptor
import logging
import sys
+from .model import String, Scope
log = logging.getLogger(__name__)
+
class RawDescriptor(XmlDescriptor, XMLEditingDescriptor):
"""
Module that provides a raw editing view of its data and children. It
requires that the definition xml is valid.
"""
+ data = String(help="XML data for the module", scope=Scope.content)
+
@classmethod
def definition_from_xml(cls, xml_object, system):
return {'data': etree.tostring(xml_object, pretty_print=True)}, []
def definition_to_xml(self, resource_fs):
try:
- return etree.fromstring(self.definition['data'])
+ return etree.fromstring(self.data)
except etree.XMLSyntaxError as err:
# Can't recover here, so just add some info and
# re-raise
- lines = self.definition['data'].split('\n')
+ lines = self.data.split('\n')
line, offset = err.position
msg = ("Unable to create xml for problem {loc}. "
"Context: '{context}'".format(
diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py
index f14254c011..988aaaf7b7 100644
--- a/common/lib/xmodule/xmodule/template_module.py
+++ b/common/lib/xmodule/xmodule/template_module.py
@@ -27,11 +27,6 @@ class CustomTagModule(XModule):
More information given in the text
"""
- 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 get_html(self):
return self.descriptor.rendered_html
@@ -62,14 +57,14 @@ class CustomTagDescriptor(RawDescriptor):
template_loc = self.location._replace(category='custom_tag_template', name=template_name)
template_module = modulestore().get_instance(system.course_id, template_loc)
- template_module_data = template_module.definition['data']
+ template_module_data = template_module.data
template = Template(template_module_data)
return template.render(**params)
@property
def rendered_html(self):
- return self.render_template(self.system, self.definition['data'])
+ return self.render_template(self.system, self.data)
def export_to_file(self):
"""
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index 0c6e2245b9..7ed0685704 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -143,10 +143,9 @@ def redirect_to_course_position(course_module, first_time):
'chapter': chapter.url_name,
'section': section.url_name}))
-def save_child_position(seq_module, child_name, instance_module):
+def save_child_position(seq_module, child_name):
"""
child_name: url_name of the child
- instance_module: the StudentModule object for the seq_module
"""
for i, c in enumerate(seq_module.get_display_items()):
if c.url_name == child_name:
@@ -155,8 +154,6 @@ def save_child_position(seq_module, child_name, instance_module):
# Only save if position changed
if position != seq_module.position:
seq_module.position = position
- instance_module.state = seq_module.get_instance_state()
- instance_module.save()
@login_required
@ensure_csrf_cookie
@@ -222,8 +219,7 @@ def index(request, course_id, chapter=None, section=None,
chapter_descriptor = course.get_child_by_url_name(chapter)
if chapter_descriptor is not None:
- instance_module = get_instance_module(course_id, request.user, course_module, student_module_cache)
- save_child_position(course_module, chapter, instance_module)
+ save_child_position(course_module, chapter)
else:
raise Http404
@@ -250,8 +246,7 @@ def index(request, course_id, chapter=None, section=None,
raise Http404
# Save where we are in the chapter
- instance_module = get_instance_module(course_id, request.user, chapter_module, student_module_cache)
- save_child_position(chapter_module, section, instance_module)
+ save_child_position(chapter_module, section)
context['content'] = section_module.get_html()
From 81e065bdb6224d67368581fd5bce3b35f154e228 Mon Sep 17 00:00:00 2001
From: Calen Pennington
Date: Tue, 18 Dec 2012 10:14:42 -0500
Subject: [PATCH 0034/1383] Fix more errors in tests
---
common/lib/xmodule/xmodule/capa_module.py | 16 +++++++++++-
common/lib/xmodule/xmodule/course_module.py | 28 ++-------------------
common/lib/xmodule/xmodule/html_module.py | 2 ++
3 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 36da929926..4728c713af 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -355,7 +355,11 @@ class CapaModule(XModule):
id=self.location.html_id(), ajax_url=self.system.ajax_url) + html + " "
# now do the substitutions which are filesystem based, e.g. '/static/' prefixes
- return self.system.replace_urls(html, self.descriptor.data_dir, course_namespace=self.location)
+ return self.system.replace_urls(
+ html,
+ getattr(self.descriptor, 'data_dir', ''),
+ course_namespace=self.location
+ )
def handle_ajax(self, dispatch, get):
'''
@@ -461,11 +465,21 @@ class CapaModule(XModule):
new_answers = dict()
for answer_id in answers:
try:
+<<<<<<< HEAD
<<<<<<< HEAD
new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.metadata['data_dir'], course_namespace=self.location)}
=======
new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.descriptor.data_dir)}
>>>>>>> WIP: Save student state via StudentModule. Inheritance doesn't work
+=======
+ new_answer = {
+ answer_id: self.system.replace_urls(
+ answers[answer_id],
+ getattr(self, 'data_dir', ''),
+ course_namespace=self.location
+ )
+ }
+>>>>>>> Fix more errors in tests
except TypeError:
log.debug('Unable to perform URL substitution on answers[%s]: %s' % (answer_id, answers[answer_id]))
new_answer = {answer_id: answers[answer_id]}
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 510857303a..c11024e406 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -290,30 +290,6 @@ class CourseDescriptor(SequenceDescriptor):
def has_started(self):
return time.gmtime() > self.start
- @property
- def end(self):
- return self._try_parse_time("end")
- @end.setter
- def end(self, value):
- if isinstance(value, time.struct_time):
- self.metadata['end'] = stringify_time(value)
- @property
- def enrollment_start(self):
- return self._try_parse_time("enrollment_start")
-
- @enrollment_start.setter
- def enrollment_start(self, value):
- if isinstance(value, time.struct_time):
- self.metadata['enrollment_start'] = stringify_time(value)
- @property
- def enrollment_end(self):
- return self._try_parse_time("enrollment_end")
-
- @enrollment_end.setter
- def enrollment_end(self, value):
- if isinstance(value, time.struct_time):
- self.metadata['enrollment_end'] = stringify_time(value)
-
@property
def grader(self):
return self._grading_policy['GRADER']
@@ -326,7 +302,7 @@ class CourseDescriptor(SequenceDescriptor):
def raw_grader(self, value):
# NOTE WELL: this change will not update the processed graders. If we need that, this needs to call grader_from_conf
self._grading_policy['RAW_GRADER'] = value
- self.definition['data'].setdefault('grading_policy',{})['GRADER'] = value
+ self.grading_policy['GRADER'] = value
@property
def grade_cutoffs(self):
@@ -335,7 +311,7 @@ class CourseDescriptor(SequenceDescriptor):
@grade_cutoffs.setter
def grade_cutoffs(self, value):
self._grading_policy['GRADE_CUTOFFS'] = value
- self.definition['data'].setdefault('grading_policy',{})['GRADE_CUTOFFS'] = value
+ self.grading_policy['GRADE_CUTOFFS'] = value
@lazyproperty
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 562eeeb361..6ec1061451 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -43,6 +43,8 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor):
module_class = HtmlModule
filename_extension = "xml"
template_dir_name = "html"
+
+ data = String(help="Html contents to display for this module", scope=Scope.content)
js = {'coffee': [resource_string(__name__, 'js/src/html/edit.coffee')]}
js_module_name = "HTMLEditingDescriptor"
From 7679fda17229d1f6f367ea88d19ed3d735fe46f2 Mon Sep 17 00:00:00 2001
From: Calen Pennington