Fix errors from running unit tests (some tests still fail)

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

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):
         """

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(

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:

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

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

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 '<x_module(id={0})>'.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

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,

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

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:

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