diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 9d4744e2c9..bb55fa7ee8 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -31,8 +31,8 @@ class ABTestModule(XModule): Implements an A/B test with an aribtrary number of competing groups """ - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + 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) if shared_state is None: diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index ba49f98257..eb083e97db 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -11,13 +11,13 @@ from datetime import timedelta from lxml import etree from pkg_resources import resource_string -from xmodule.x_module import XModule -from xmodule.raw_module import RawDescriptor -from xmodule.exceptions import NotFoundError -from progress import Progress from capa.capa_problem import LoncapaProblem from capa.responsetypes import StudentInputError from capa.util import convert_files_to_filenames +from progress import Progress +from xmodule.x_module import XModule +from xmodule.raw_module import RawDescriptor +from xmodule.exceptions import NotFoundError log = logging.getLogger("mitx.courseware") @@ -80,9 +80,9 @@ class CapaModule(XModule): js_module_name = "Problem" css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]} - def __init__(self, system, location, definition, instance_state=None, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.attempts = 0 @@ -134,12 +134,14 @@ class CapaModule(XModule): if self.rerandomize == 'never': seed = 1 - elif self.rerandomize == "per_student" and hasattr(system, 'id'): + elif self.rerandomize == "per_student" and hasattr(self.system, 'id'): seed = system.id else: seed = None try: + # TODO (vshnayder): move as much as possible of this work and error + # checking to descriptor load time self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), instance_state, seed=seed, system=self.system) except Exception as err: @@ -426,7 +428,7 @@ class CapaModule(XModule): event_info = dict() event_info['state'] = self.lcp.get_state() event_info['problem_id'] = self.location.url() - + answers = self.make_dict_of_responses(get) event_info['answers'] = convert_files_to_filenames(answers) @@ -563,6 +565,9 @@ class CapaDescriptor(RawDescriptor): module_class = CapaModule + stores_state = True + has_score = True + # Capa modules have some additional metadata: # TODO (vshnayder): do problems have any other metadata? Do they # actually use type and points? diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index ce1b93aa44..7ec9f54cd2 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -99,10 +99,10 @@ class CourseDescriptor(SequenceDescriptor): sections = [] for s in c.get_children(): if s.metadata.get('graded', False): - # TODO: Only include modules that have a score here - xmoduledescriptors = [child for child in yield_descriptor_descendents(s)] - - section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : xmoduledescriptors} + xmoduledescriptors = list(yield_descriptor_descendents(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', "") graded_sections[ section_format ] = graded_sections.get( section_format, [] ) + [section_description] diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index d8837d876f..5e7c1f7e3f 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -18,9 +18,9 @@ class HtmlModule(XModule): def get_html(self): return self.html - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.html = self.definition['data'] diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index e742a58471..2986c948d3 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -25,9 +25,9 @@ class SequenceModule(XModule): css = {'scss': [resource_string(__name__, 'css/sequence/display.scss')]} js_module_name = "Sequence" - def __init__(self, system, location, definition, instance_state=None, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.position = 1 @@ -107,6 +107,8 @@ class SequenceModule(XModule): class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule + + stores_state = True # For remembering where in the sequence the student is @classmethod def definition_from_xml(cls, xml_object, system): diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 260ad009f9..a13d0fa095 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -26,12 +26,26 @@ class CustomTagModule(XModule): More information given in the text """ - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) + self.html = definition['html'] - xmltree = etree.fromstring(self.definition['data']) + def get_html(self): + return self.html + + +class CustomTagDescriptor(RawDescriptor): + """ Descriptor for custom tags. Loads the template when created.""" + module_class = CustomTagModule + + @classmethod + def definition_from_xml(cls, xml_object, system): + definition = RawDescriptor.definition_from_xml(xml_object, system) + + # Render the template and save it. + xmltree = etree.fromstring(definition['data']) if 'impl' in xmltree.attrib: template_name = xmltree.attrib['impl'] else: @@ -45,13 +59,8 @@ class CustomTagModule(XModule): .format(location)) params = dict(xmltree.items()) - with self.system.filestore.open( - 'custom_tags/{name}'.format(name=template_name)) as template: - self.html = Template(template.read()).render(**params) + with system.resources_fs.open('custom_tags/{name}' + .format(name=template_name)) as template: + definition['html'] = Template(template.read()).render(**params) - def get_html(self): - return self.html - - -class CustomTagDescriptor(RawDescriptor): - module_class = CustomTagModule + return definition diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 1115d85c7f..db454aa483 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -708,6 +708,6 @@ class ModuleProgressTest(unittest.TestCase): ''' def test_xmodule_default(self): '''Make sure default get_progress exists, returns None''' - xm = x_module.XModule(i4xs, 'a://b/c/d/e', {}) + xm = x_module.XModule(i4xs, 'a://b/c/d/e', None, {}) p = xm.get_progress() self.assertEqual(p, None) diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index 884fd0a056..52e5825e43 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -10,8 +10,8 @@ class_priority = ['video', 'problem'] class VerticalModule(XModule): ''' Layout module for laying out submodules vertically.''' - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.contents = None def get_html(self): diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index fb68ba982b..7d11dea283 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -23,9 +23,9 @@ class VideoModule(XModule): css = {'scss': [resource_string(__name__, 'css/video/display.scss')]} js_module_name = "Video" - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) xmltree = etree.fromstring(self.definition['data']) self.youtube = xmltree.get('youtube') @@ -80,3 +80,5 @@ class VideoModule(XModule): class VideoDescriptor(RawDescriptor): module_class = VideoModule + + stores_state = True diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index f8f7499911..06b9fa0c26 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -143,7 +143,7 @@ class XModule(HTMLSnippet): # in the module icon_class = 'other' - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): ''' Construct a new xmodule @@ -189,6 +189,7 @@ class XModule(HTMLSnippet): self.system = system self.location = Location(location) self.definition = definition + self.descriptor = descriptor self.instance_state = instance_state self.shared_state = shared_state self.id = self.location.url() @@ -303,6 +304,13 @@ class XModuleDescriptor(Plugin, HTMLSnippet): """ entry_point = "xmodule.v1" module_class = XModule + + # Attributes for inpsection of the descriptor + stores_state = False # Indicates whether the xmodule state should be + # stored in a database (independent of shared state) + has_score = False # This indicates whether the xmodule is a problem-type. + # It should respond to max_score() and grade(). It can be graded or ungraded + # (like a practice problem). # A list of metadata that this module can inherit from its parent module inheritable_metadata = ( @@ -427,6 +435,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet): system, self.location, self.definition, + self, metadata=self.metadata ) diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 925b8429c6..b0220670eb 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -12,10 +12,12 @@ from models import StudentModule log = logging.getLogger("mitx.courseware") def yield_module_descendents(module): - for child in module.get_display_items(): - yield child - for module in yield_module_descendents(child): - yield module + stack = module.get_display_items() + + while len(stack) > 0: + next_module = stack.pop() + stack.extend( next_module.get_display_items() ) + yield next_module def grade(student, request, course, student_module_cache=None): """ @@ -89,7 +91,7 @@ def grade(student, request, course, student_module_cache=None): if graded_total.possible > 0: format_scores.append(graded_total) else: - log.exception("Unable to grade a section with a total possible score of zero. " + str(section_descriptor.id)) + log.exception("Unable to grade a section with a total possible score of zero. " + str(section_descriptor.location)) totaled_scores[section_format] = format_scores @@ -183,6 +185,10 @@ def get_score(user, problem, student_module_cache): problem: an XModule cache: A StudentModuleCache """ + if not (problem.descriptor.stores_state and problem.descriptor.has_score): + # These are not problems, and do not have a score + return (None, None) + correct = 0.0 # If the ID is not in the cache, add the item diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 631a750643..aa9f946733 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -69,10 +69,10 @@ class StudentModuleCache(object): """ def __init__(self, user, descriptors): ''' - Find any StudentModule objects that are needed by any child modules of the - supplied descriptor, or caches only the StudentModule objects specifically - for every descriptor in descriptors. Avoids making multiple queries to the - database. + Find any StudentModule objects that are needed by any descriptor + in descriptors. Avoids making multiple queries to the database. + Note: Only modules that have store_state = True or have shared + state will have a StudentModule. Arguments user: The user for which to fetch maching StudentModules @@ -134,7 +134,8 @@ class StudentModuleCache(object): ''' keys = [] for descriptor in descriptors: - keys.append(descriptor.location.url()) + if descriptor.stores_state: + keys.append(descriptor.location.url()) shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 5da8cdd869..852e3d8a77 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -127,19 +127,18 @@ def get_module(user, request, location, student_module_cache, position=None): descriptor = modulestore().get_item(location) #TODO Only check the cache if this module can possibly have state + instance_module = None + shared_module = None if user.is_authenticated(): - instance_module = student_module_cache.lookup(descriptor.category, - descriptor.location.url()) + if descriptor.stores_state: + instance_module = student_module_cache.lookup(descriptor.category, + descriptor.location.url()) shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: shared_module = student_module_cache.lookup(descriptor.category, shared_state_key) - else: - shared_module = None - else: - instance_module = None - shared_module = None + instance_state = instance_module.state if instance_module is not None else None @@ -206,6 +205,11 @@ def get_instance_module(user, module, student_module_cache): 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(module.category, module.location.url()) diff --git a/lms/templates/gradebook.html b/lms/templates/gradebook.html index e5721969e9..bdc1516339 100644 --- a/lms/templates/gradebook.html +++ b/lms/templates/gradebook.html @@ -11,11 +11,11 @@ <%static:css group='course'/> %block> @@ -54,7 +54,7 @@ data_class = "grade_" + letter_grade %> -