Consolidate access to metadata, and allow some of it to be inherited between modules

common/lib/xmodule/capa_module.py

@@ -125,7 +125,7 @@ class CapaModule(XModule):
 
         # User submitted a problem, and hasn't reset. We don't want
         # more submissions.
-        if self.lcp.done and self.rerandomize == "always":
+        if self.lcp.done and self.metadata['rerandomize'] == "always":
             check_button = False
             save_button = False
 
@@ -184,15 +184,15 @@ class CapaModule(XModule):
         # TODO: Should be converted to: self.explanation=only_one(dom2.xpath('/problem/@explain'), default="closed")
         self.explain_available = only_one(dom2.xpath('/problem/@explain_available'))
 
-        display_due_date_string = only_one(dom2.xpath('/problem/@due'))
-        if len(display_due_date_string) > 0:
+        display_due_date_string = self.metadata.get('due', None)
+        if display_due_date_string is not None:
             self.display_due_date = dateutil.parser.parse(display_due_date_string)
             #log.debug("Parsed " + display_due_date_string + " to " + str(self.display_due_date))
         else:
             self.display_due_date = None
 
-        grace_period_string = only_one(dom2.xpath('/problem/@graceperiod'))
-        if len(grace_period_string) > 0 and self.display_due_date:
+        grace_period_string = self.metadata.get('graceperiod', None)
+        if grace_period_string is not None and self.display_due_date:
             self.grace_period = parse_timedelta(grace_period_string)
             self.close_date = self.display_due_date + self.grace_period
             #log.debug("Then parsed " + grace_period_string + " to closing date" + str(self.close_date))
@@ -206,12 +206,12 @@ class CapaModule(XModule):
         else:
             self.max_attempts = None
 
-        self.show_answer = only_one(dom2.xpath('/problem/@showanswer'))
+        self.show_answer = self.metadata.get('showanwser', 'closed')
 
         if self.show_answer == "":
             self.show_answer = "closed"
 
-        self.rerandomize = only_one(dom2.xpath('/problem/@rerandomize'))
+        self.rerandomize = self.metadata.get('rerandomize', 'always')
         if self.rerandomize == "" or self.rerandomize == "always" or self.rerandomize == "true":
             self.rerandomize = "always"
         elif self.rerandomize == "false" or self.rerandomize == "per_student":

common/lib/xmodule/seq_module.py

@@ -52,9 +52,9 @@ class SequenceModule(XModule):
             contents.append({
                 'content': child.get_html(),
                 'title': "\n".join(
-                    grand_child.display_name.strip()
+                    grand_child.metadata['display_name'].strip()
                     for grand_child in child.get_children()
-                    if grand_child.display_name is not None
+                    if 'metadata'  in grand_child.metadata
                 ),
                 'progress_status': Progress.to_js_status_str(progress),
                 'progress_detail': Progress.to_js_detail_str(progress),

common/lib/xmodule/x_module.py

@@ -82,11 +82,9 @@ class XModule(object):
         self.shared_state = shared_state
         self.id = self.location.url()
         self.name = self.location.name
-        self.display_name = kwargs.get('display_name', '')
         self.type = self.location.category
+        self.metadata = kwargs.get('metadata', {})
         self._loaded_children = None
-        self.graded = kwargs.get('graded', False)
-        self.format = kwargs.get('format')
 
     def get_name(self):
         name = self.__xmltree.get('name')
@@ -188,6 +186,9 @@ class XModuleDescriptor(Plugin):
     js = {}
     js_module = None
 
+    # A list of metadata that this module can inherit from its parent module
+    inheritable_metadata = ('graded', 'due', 'graceperiod', 'showanswer', 'rerandomize')
+
     @staticmethod
     def load_from_json(json_data, system, default_class=None):
         """
@@ -215,7 +216,11 @@ class XModuleDescriptor(Plugin):
         return cls(system=system, **json_data)
 
     @staticmethod
-    def load_from_xml(xml_data, system, org=None, course=None, default_class=None):
+    def load_from_xml(xml_data,
+            system,
+            org=None,
+            course=None,
+            default_class=None):
         """
         This method instantiates the correct subclass of XModuleDescriptor based
         on the contents of xml_data.
@@ -282,32 +287,48 @@ class XModuleDescriptor(Plugin):
 
         Current arguments passed in kwargs:
             location: A keystore.Location object indicating the name and ownership of this problem
-            goals: A list of strings of learning goals associated with this module
-            display_name: The name to use for displaying this module to the user
-            format: The format of this module ('Homework', 'Lab', etc)
-            graded (bool): Whether this module is should be graded or not
+            shared_state_key: The key to use for sharing StudentModules with other
+                modules of this type
+            metadata: A dictionary containing the following optional keys:
+                goals: A list of strings of learning goals associated with this module
+                display_name: The name to use for displaying this module to the user
+                format: The format of this module ('Homework', 'Lab', etc)
+                graded (bool): Whether this module is should be graded or not
+                due (string): The due date for this module
+                graceperiod (string): The amount of grace period to allow when enforcing the due date
+                showanswer (string): When to show answers for this module
+                rerandomize (string): When to generate a newly randomized instance of the module data
         """
         self.system = system
         self.definition = definition if definition is not None else {}
         self.name = Location(kwargs.get('location')).name
         self.type = Location(kwargs.get('location')).category
         self.url = Location(kwargs.get('location')).url()
-        self.display_name = kwargs.get('display_name')
-        self.format = kwargs.get('format')
-        self.graded = kwargs.get('graded', False)
+        self.metadata = kwargs.get('metadata', {})
         self.shared_state_key = kwargs.get('shared_state_key')
 
-        # For now, we represent goals as a list of strings, but this
-        # is one of the things that we are going to be iterating on heavily
-        # to find the best teaching method
-        self.goals = kwargs.get('goals', [])
-
         self._child_instances = None
 
+    def inherit_metadata(self, metadata):
+        """
+        Updates this module with metadata inherited from a containing module.
+        Only metadata specified in self.inheritable_metadata will
+        be inherited
+        """
+        # Set all inheritable metadata from kwargs that are
+        # in self.inheritable_metadata and aren't already set in metadata
+        for attr in self.inheritable_metadata:
+            if attr not in self.metadata and attr in metadata:
+                self.metadata[attr] = metadata[attr]
+
     def get_children(self):
         """Returns a list of XModuleDescriptor instances for the children of this module"""
         if self._child_instances is None:
-            self._child_instances = [self.system.load_item(child) for child in self.definition.get('children', [])]
+            self._child_instances = []
+            for child_loc in self.definition.get('children', []):
+                child = self.system.load_item(child_loc)
+                child.inherit_metadata(self.metadata)
+                self._child_instances.append(child)
 
         return self._child_instances
 
@@ -322,10 +343,14 @@ class XModuleDescriptor(Plugin):
         Returns a constructor for an XModule. This constructor takes two arguments:
         instance_state and shared_state, and returns a fully nstantiated XModule
         """
-        return partial(self.module_class, system, self.url, self.definition,
-            display_name=self.display_name,
-            format=self.format,
-            graded=self.graded)
+        return partial(
+            self.module_class,
+            system,
+            self.url,
+            self.definition,
+            metadata=self.metadata
+        )
+
 
 class DescriptorSystem(object):
     def __init__(self, load_item, resources_fs):

common/lib/xmodule/xml_module.py

@@ -29,6 +29,18 @@ class XmlDescriptor(XModuleDescriptor):
         """
         xml_object = etree.fromstring(xml_data)
 
+        metadata = {}
+        for attr in ('format', 'graceperiod', 'showanswer', 'rerandomize', 'due'):
+            from_xml = xml_object.get(attr)
+            if from_xml is not None:
+                metadata[attr] = from_xml
+
+        if xml_object.get('graded') is not None:
+            metadata['graded'] = xml_object.get('graded') == 'true'
+
+        if xml_object.get('name') is not None:
+            metadata['display_name'] = xml_object.get('name')
+
         return cls(
             system,
             cls.definition_from_xml(xml_object, system),
@@ -37,7 +49,5 @@ class XmlDescriptor(XModuleDescriptor):
                       course,
                       xml_object.tag,
                       xml_object.get('slug')],
-            display_name=xml_object.get('name'),
-            format=xml_object.get('format'),
-            graded=xml_object.get('graded') == 'true',
+            metadata=metadata,
         )

lms/djangoapps/courseware/grades.py

@@ -87,7 +87,7 @@ def grade_sheet(student, course, student_module_cache):
                     for module in yield_descendents(child):
                         yield module
 
-            graded = getattr(s, 'graded', False)
+            graded = s.metadata.get('graded', False)
             scores = []
             for module in yield_descendents(s):
                 (correct, total) = get_score(student, module, student_module_cache)
@@ -105,29 +105,27 @@ def grade_sheet(student, course, student_module_cache):
                     #We simply cannot grade a problem that is 12/0, because we might need it as a percentage
                     graded = False
 
-                scores.append(Score(correct, total, graded, module.display_name))
+                scores.append(Score(correct, total, graded, module.metadata.get('display_name')))
 
-            section_total, graded_total = graders.aggregate_scores(scores, s.display_name)
+            section_total, graded_total = graders.aggregate_scores(scores, s.metadata.get('display_name'))
             #Add the graded total to totaled_scores
-            format = getattr(s, 'format', "")
-            subtitle = getattr(s, 'subtitle', format)
+            format = s.metadata.get('format', "")
             if format and graded_total.possible > 0:
                 format_scores = totaled_scores.get(format, [])
                 format_scores.append(graded_total)
                 totaled_scores[format] = format_scores
 
             sections.append({
-                'section': s.display_name,
+                'section': s.metadata.get('display_name'),
                 'scores': scores,
                 'section_total': section_total,
                 'format': format,
-                'subtitle': subtitle,
-                'due': getattr(s, "due", ""),
+                'due': s.metadata.get("due", ""),
                 'graded': graded,
             })
 
-        chapters.append({'course': course.display_name,
-                         'chapter': c.display_name,
+        chapters.append({'course': course.metadata.get('display_name'),
+                         'chapter': c.metadata.get('display_name'),
                          'sections': sections})
 
     grader = course_settings.GRADER

lms/djangoapps/courseware/module_render.py

@@ -137,17 +137,17 @@ def toc_for_course(user, request, course_location, active_chapter, active_sectio
         sections = list()
         for section in chapter.get_display_items():
 
-            active = (chapter.display_name == active_chapter and
-                      section.display_name == active_section)
+            active = (chapter.metadata.get('display_name') == active_chapter and
+                      section.metadata.get('display_name') == active_section)
 
-            sections.append({'name': section.display_name,
-                             'format': getattr(section, 'format', ''),
-                             'due': getattr(section, 'due', ''),
+            sections.append({'name': section.metadata.get('display_name'),
+                             'format': section.metadata.get('format', ''),
+                             'due': section.metadata.get('due', ''),
                              'active': active})
 
-        chapters.append({'name': chapter.display_name,
+        chapters.append({'name': chapter.metadata.get('display_name'),
                          'sections': sections,
-                         'active': chapter.display_name == active_chapter})
+                         'active': chapter.metadata.get('display_name') == active_chapter})
     return chapters
 
 
@@ -171,7 +171,7 @@ def get_section(course, chapter, section):
 
     chapter_module = None
     for _chapter in course_module.get_children():
-        if _chapter.display_name == chapter:
+        if _chapter.metadata.get('display_name') == chapter:
             chapter_module = _chapter
             break
 
@@ -180,7 +180,7 @@ def get_section(course, chapter, section):
 
     section_module = None
     for _section in chapter_module.get_children():
-        if _section.display_name == section:
+        if _section.metadata.get('display_name') == section:
             section_module = _section
             break
 
@@ -271,9 +271,9 @@ def add_histogram(module):
     def get_html():
         module_id = module.id
         histogram = grade_histogram(module_id)
-        print histogram
         render_histogram = len(histogram) > 0
         staff_context = {'definition': json.dumps(module.definition, indent=4),
+                         'metadata': json.dumps(module.metadata, indent=4),
                          'element_id': module.location.html_id(),
                          'histogram': json.dumps(histogram),
                          'render_histogram': render_histogram,

lms/templates/profile.html

@@ -156,7 +156,7 @@ $(function() {
               
               <h3><a href="${reverse('courseware_section', args=format_url_params([chapter['course'], chapter['chapter'], section['section']])) }">
                 ${ section['section'] }</a> ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}</h3>
-              ${section['subtitle']}
+              ${section['format']}
               %if 'due' in section and section['due']!="":
                 due ${section['due']}
               %endif

lms/templates/staff_problem_info.html

 ${module_content}
 <div class="staff_info">
-${definition | h}
+definition = ${definition | h}
+metadata = ${metadata | h}
 </div>
 %if render_histogram:
 <div id="histogram_${element_id}" class="histogram" data-histogram="${histogram}"></div>