Convert a bunch more references from metadata to fields

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

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

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'

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:

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)

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,
             })
 

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())

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]

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)