Merge branch 'feature/cale/xmodule-storage-model' into feature/alex/poll-merged

Conflicts:
	common/lib/xmodule/xmodule/conditional_module.py

cms/djangoapps/contentstore/course_info_model.py

 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
-from lxml import html
+from lxml import html, etree
 import re
 from django.http import HttpResponseBadRequest
 import logging

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -109,11 +109,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
         ms = modulestore('direct')
         effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None]))
-        self.assertEqual(effort.definition['data'], '6 hours')
+        self.assertEqual(effort.data, '6 hours')
 
         # this one should be in a non-override folder
         effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None]))
-        self.assertEqual(effort.definition['data'], 'TBD')
+        self.assertEqual(effort.data, 'TBD')
 
     def test_remove_hide_progress_tab(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
@@ -123,7 +123,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
         course = ms.get_item(source_location)
-        self.assertNotIn('hide_progress_tab', course.metadata)
+        self.assertFalse(course.hide_progress_tab)
 
     def test_clone_course(self):
 
@@ -216,7 +216,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         with fs.open('grading_policy.json','r') as grading_policy:
             on_disk = loads(grading_policy.read())
             course = ms.get_item(location)
-            self.assertEqual(on_disk, course.definition['data']['grading_policy'])
+            self.assertEqual(on_disk, course.grading_policy)
 
         # remove old course
         delete_course(ms, cs, location)
@@ -407,5 +407,4 @@ class ContentStoreTest(ModuleStoreTestCase):
         self.assertIsInstance(problem, CapaDescriptor, "New problem is not a CapaDescriptor")
         context = problem.get_context()
         self.assertIn('markdown', context, "markdown is missing from context")
-        self.assertIn('markdown', problem.metadata, "markdown is missing from metadata")
         self.assertNotIn('markdown', problem.editable_metadata_fields, "Markdown slipped into the editable metadata fields")

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -251,8 +251,7 @@ class CourseGradingTest(CourseTestCase):
 
         test_grader.grace_period = {'hours': 4, 'minutes': 5, 'seconds': 0}
         altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__)
-        print test_grader.__dict__
-        print altered_grader.__dict__
+        print test_grader.grace_period, altered_grader.grace_period
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "4 hour grace period")
 
     def test_update_grader_from_json(self):

cms/djangoapps/contentstore/views.py

@@ -281,7 +281,7 @@ def edit_unit(request, location):
             component_templates[template.location.category].append((
                 template.lms.display_name,
                 template.location.url(),
-                'markdown' in template.metadata,
+                hasattr(template, 'markdown') and template.markdown != '',
                 template.location.name == 'Empty'
             ))
 
@@ -944,7 +944,7 @@ def reorder_static_tabs(request):
     for tab in course.tabs:
         if tab['type'] == 'static_tab':
             reordered_tabs.append({'type': 'static_tab',
-                'name': tab_items[static_tab_idx].metadata.get('display_name'),
+                'name': tab_items[static_tab_idx].lms.display_name,
                 'url_slug': tab_items[static_tab_idx].location.name})
             static_tab_idx += 1
         else:
@@ -953,7 +953,7 @@ def reorder_static_tabs(request):
 
     # OK, re-assemble the static tabs in the new order
     course.tabs = reordered_tabs
-    modulestore('direct').update_metadata(course.location, course.metadata)
+    modulestore('direct').update_metadata(course.location, own_metadata(course))
     return HttpResponse()
 
 

cms/djangoapps/models/settings/course_grading.py

@@ -157,10 +157,10 @@ class CourseGradingModel(object):
                 graceperiodjson = graceperiodjson['grace_period']
 
             # lms requires these to be in a fixed order
-            grace_rep = "{0[hours]:d} hours {0[minutes]:d} minutes {0[seconds]:d} seconds".format(graceperiodjson)
+            grace_timedelta = timedelta(**graceperiodjson)
 
             descriptor = get_modulestore(course_location).get_item(course_location)
-            descriptor.lms.graceperiod = grace_rep
+            descriptor.lms.graceperiod = grace_timedelta
             get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata)
 
     @staticmethod
@@ -236,7 +236,6 @@ class CourseGradingModel(object):
 
     @staticmethod
     def convert_set_grace_period(descriptor):
-<<<<<<< HEAD
         # 5 hours 59 minutes 59 seconds => converted to iso format
         rawgrace = descriptor.lms.graceperiod
         if rawgrace:
@@ -248,27 +247,19 @@ class CourseGradingModel(object):
             minutes = int(seconds / 60)
             seconds -= minutes * 60
 
-            graceperiod = {}
+            graceperiod = {'hours': 0, 'minutes': 0, 'seconds': 0}
             if hours > 0:
-                graceperiod['hours'] = str(hours)
+                graceperiod['hours'] = hours
 
             if minutes > 0:
-                graceperiod['minutes'] = str(minutes)
+                graceperiod['minutes'] = minutes
 
             if seconds > 0:
-                graceperiod['seconds'] = str(seconds)
+                graceperiod['seconds'] = seconds
 
             return graceperiod
         else:
             return None
-=======
-        # 5 hours 59 minutes 59 seconds => { hours: 5, minutes : 59, seconds : 59}
-        rawgrace = descriptor.metadata.get('graceperiod', None)
-        if rawgrace:
-            parsedgrace = {str(key): int(val) for (val, key) in re.findall('\s*(\d+)\s*(\w+)', rawgrace)}
-            return parsedgrace
-        else: return None
->>>>>>> origin/master
 
     @staticmethod
     def parse_grader(json_grader):

common/lib/xmodule/xmodule/capa_module.py

@@ -116,7 +116,7 @@ class CapaModule(XModule):
             #log.debug("Then parsed " + grace_period_string +
             #          " to closing date" + str(self.close_date))
         else:
-            self.close_date = self.due
+            self.close_date = due_date
 
         if self.seed is None:
             if self.rerandomize == 'never':
@@ -373,8 +373,11 @@ class CapaModule(XModule):
                 datetime.datetime.utcnow() > self.close_date)
 
     def closed(self):
-        ''' Is the student still allowed to submit answers? '''
-        if self.attempts == self.max_attempts:
+        ''' Is the student prevented from submitting answers? '''
+        if self.max_attempts is None:
+            return False
+
+        if self.attempts >= self.max_attempts:
             return True
         if self.is_past_due():
             return True
@@ -668,6 +671,7 @@ class CapaDescriptor(RawDescriptor):
     module_class = CapaModule
 
     weight = Float(help="How much to weight this problem by", scope=Scope.settings)
+    markdown = String(help="Markdown source of this module", scope=Scope.settings, default='')
 
     stores_state = True
     has_score = True
@@ -689,7 +693,7 @@ class CapaDescriptor(RawDescriptor):
 
     def get_context(self):
         _context = RawDescriptor.get_context(self)
-        _context.update({'markdown': self.metadata.get('markdown', '')})
+        _context.update({'markdown': self.markdown})
         return _context
 
     @property

common/lib/xmodule/xmodule/course_module.py

@@ -409,6 +409,11 @@ class CourseDescriptor(SequenceDescriptor):
     def grade_cutoffs(self, value):
         self._grading_policy['GRADE_CUTOFFS'] = value
 
+        # XBlock fields don't update after mutation
+        policy = self.grading_policy
+        policy['GRADE_CUTOFFS'] = value
+        self.grading_policy = policy
+
 
     @property
     def lowest_passing_grade(self):

common/lib/xmodule/xmodule/gst_module.py

@@ -14,6 +14,7 @@ from xmodule.xml_module import XmlDescriptor
 from xmodule.x_module import XModule
 from xmodule.stringify import stringify_children
 from pkg_resources import resource_string
+from xblock.core import String, Scope
 
 
 log = logging.getLogger(__name__)
@@ -43,6 +44,8 @@ class GraphicalSliderToolModule(XModule):
     }
     js_module_name = "GraphicalSliderTool"
 
+    render = String(scope=Scope.content)
+
     def __init__(self, system, location, definition, descriptor, instance_state=None,
                  shared_state=None, **kwargs):
         """
@@ -60,7 +63,7 @@ class GraphicalSliderToolModule(XModule):
         self.html_class = self.location.category
         self.configuration_json = self.build_configuration_json()
         params = {
-                  'gst_html': self.substitute_controls(self.definition['render']),
+                  'gst_html': self.substitute_controls(self.render),
                   'element_id': self.html_id,
                   'element_class': self.html_class,
                   'configuration_json': self.configuration_json
@@ -146,6 +149,9 @@ class GraphicalSliderToolDescriptor(MakoModuleDescriptor, XmlDescriptor):
     module_class = GraphicalSliderToolModule
     template_dir_name = 'graphical_slider_tool'
 
+    render = String(scope=Scope.content)
+    configuration = String(scope=Scope.content)
+
     @classmethod
     def definition_from_xml(cls, xml_object, system):
         """
@@ -184,7 +190,7 @@ class GraphicalSliderToolDescriptor(MakoModuleDescriptor, XmlDescriptor):
         xml_object = etree.Element('graphical_slider_tool')
 
         def add_child(k):
-            child_str = '<{tag}>{body}</{tag}>'.format(tag=k, body=self.definition[k])
+            child_str = '<{tag}>{body}</{tag}>'.format(tag=k, body=getattr(self, k))
             child_node = etree.fromstring(child_str)
             xml_object.append(child_node)
 

common/lib/xmodule/xmodule/modulestore/store_utilities.py

@@ -55,7 +55,7 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele
             )
             new_children.append(child_loc.url())
 
-      modulestore.update_children(module.location, new_children)
+        modulestore.update_children(module.location, new_children)
 
     # save metadata
     modulestore.update_metadata(module.location, module._model_data._kvs._metadata)

common/lib/xmodule/xmodule/modulestore/tests/factories.py

@@ -4,6 +4,7 @@ from uuid import uuid4
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from xmodule.timeparse import stringify_time
+from xmodule.modulestore.inheritance import own_metadata
 
 
 def XMODULE_COURSE_CREATION(class_to_create, **kwargs):
@@ -51,7 +52,7 @@ class XModuleCourseFactory(Factory):
             {"type": "progress", "name": "Progress"}]
 
         # Update the data in the mongo datastore
-        store.update_metadata(new_course.location.url(), new_course.own_metadata)
+        store.update_metadata(new_course.location.url(), own_metadata(new_course))
 
         return new_course
 
@@ -98,14 +99,11 @@ class XModuleItemFactory(Factory):
 
         new_item = store.clone_item(template, dest_location)
 
-        # TODO: This needs to be deleted when we have proper storage for static content
-        new_item.data_dir = parent.data_dir
-
         # replace the display name with an optional parameter passed in from the caller
         if display_name is not None:
             new_item.lms.display_name = display_name
 
-        store.update_metadata(new_item.location.url(), new_item.own_metadata)
+        store.update_metadata(new_item.location.url(), own_metadata(new_item))
 
         if new_item.location.category not in DETACHED_CATEGORIES:
             store.update_children(parent_location, parent.children + [new_item.location.url()])

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -32,7 +32,7 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
   policies_dir = export_fs.makeopendir('policies')
   course_run_policy_dir = policies_dir.makeopendir(course.location.name)
   with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy:
-    grading_policy.write(dumps(course.definition['data']['grading_policy']))
+    grading_policy.write(dumps(course.grading_policy))
 
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):
@@ -43,4 +43,4 @@ def export_extra_content(export_fs, modulestore, course_location, category_type,
     item_dir = export_fs.makeopendir(dirname)
     for item in items:
       with item_dir.open(item.location.name + file_suffix, 'w') as item_file:
-        item_file.write(item.definition['data'].encode('utf8'))
+        item_file.write(item.data.encode('utf8'))

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -192,6 +192,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
         course_dirs=course_dirs,
         load_error_modules=load_error_modules
     )
+    data_dir = path(data_dir)
 
     # NOTE: the XmlModuleStore does not implement get_items() which would be a preferable means
     # to enumerate the entire collection of course modules. It will be left as a TBD to implement that

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -62,37 +62,30 @@ class CapaFactory(object):
         definition = {'data': CapaFactory.sample_problem_xml, }
         location = Location(["i4x", "edX", "capa_test", "problem",
                              "SampleProblem{0}".format(CapaFactory.next_num())])
-        metadata = {}
+        model_data = {}
         if graceperiod is not None:
-            metadata['graceperiod'] = graceperiod
+            model_data['graceperiod'] = graceperiod
         if due is not None:
-            metadata['due'] = due
+            model_data['due'] = due
         if max_attempts is not None:
-            metadata['attempts'] = max_attempts
+            model_data['max_attempts'] = int(max_attempts)
         if showanswer is not None:
-            metadata['showanswer'] = showanswer
+            model_data['showanswer'] = showanswer
         if force_save_button is not None:
-            metadata['force_save_button'] = force_save_button
+            model_data['force_save_button'] = force_save_button
         if rerandomize is not None:
-            metadata['rerandomize'] = rerandomize
+            model_data['rerandomize'] = rerandomize
 
 
         descriptor = Mock(weight="1")
-        instance_state_dict = {}
         if problem_state is not None:
-            instance_state_dict = problem_state
+            model_data.update(problem_state)
         if attempts is not None:
             # converting to int here because I keep putting "0" and "1" in the tests
             # since everything else is a string.
-            instance_state_dict['attempts'] = int(attempts)
-        if len(instance_state_dict) > 0:
-            instance_state = json.dumps(instance_state_dict)
-        else:
-            instance_state = None
-
-        module = CapaModule(test_system, location,
-                            definition, descriptor,
-                                      instance_state, None, metadata=metadata)
+            model_data['attempts'] = int(attempts)
+
+        module = CapaModule(test_system, location, descriptor, model_data)
 
         return module
 

common/lib/xmodule/xmodule/tests/test_conditional.py

@@ -75,17 +75,12 @@ class ConditionalModuleTest(unittest.TestCase):
         print "Course: ", course
         print "id: ", course.id
 
-        instance_states = dict(problem=None)
-        shared_state = None
-
         def inner_get_module(descriptor):
             if isinstance(descriptor, Location):
                 location = descriptor
                 descriptor = self.modulestore.get_instance(course.id, location, depth=None)
             location = descriptor.location
-            instance_state = instance_states.get(location.category, None)
-            print "inner_get_module, location=%s, inst_state=%s" % (location, instance_state)
-            return descriptor.xmodule_constructor(test_system)(instance_state, shared_state)
+            return descriptor.xmodule(test_system)
 
         location = Location(["i4x", "edX", "cond_test", "conditional", "condone"])
 
@@ -96,7 +91,7 @@ class ConditionalModuleTest(unittest.TestCase):
 
         module = inner_get_module(location)
         print "module: ", module
-        print "module definition: ", module.definition
+        print "module.condition: ", module.condition
         print "module children: ", module.get_children()
         print "module display items (children): ", module.get_display_items()
 
@@ -110,12 +105,12 @@ class ConditionalModuleTest(unittest.TestCase):
         print "gdi=", gdi
 
         ajax = json.loads(module.handle_ajax('', ''))
-        self.assertTrue('xmodule.conditional_module' in ajax['html'])
         print "ajax: ", ajax
+        self.assertTrue('ConditionalModule' in ajax['html'])
 
         # now change state of the capa problem to make it completed
-        instance_states['problem'] = json.dumps({'attempts': 1})
+        inner_get_module(Location('i4x://edX/cond_test/problem/choiceprob')).attempts = 1
 
         ajax = json.loads(module.handle_ajax('', ''))
-        self.assertTrue('This is a secret' in ajax['html'])
         print "post-attempt ajax: ", ajax
+        self.assertTrue('This is a secret' in ajax['html'])

common/lib/xmodule/xmodule/tests/test_course_module.py

@@ -101,7 +101,7 @@ class IsNewCourseTestCase(unittest.TestCase):
         assert(descriptor.is_newish is True)
 
         descriptor = self.get_dummy_course(start='2013-02-02T12:00', is_new=False)
-        assert(descriptor.is_new is False)
+        assert(descriptor.is_newish is False)
 
         descriptor = self.get_dummy_course(start='2013-02-02T12:00', is_new=True)
         assert(descriptor.is_newish is True)

common/lib/xmodule/xmodule/tests/test_import.py

@@ -367,7 +367,7 @@ class ImportTestCase(BaseCourseTestCase):
         render_string_from_sample_gst_xml = """
         <slider var="a" style="width:400px;float:left;"/>\
 <plot style="margin-top:15px;margin-bottom:15px;"/>""".strip()
-        self.assertEqual(gst_sample.definition['render'], render_string_from_sample_gst_xml)
+        self.assertEqual(gst_sample.render, render_string_from_sample_gst_xml)
 
     def test_cohort_config(self):
         """
@@ -383,13 +383,13 @@ class ImportTestCase(BaseCourseTestCase):
         self.assertFalse(course.is_cohorted)
 
         # empty config -> False
-        course.metadata['cohort_config'] = {}
+        course.cohort_config = {}
         self.assertFalse(course.is_cohorted)
 
         # false config -> False
-        course.metadata['cohort_config'] = {'cohorted': False}
+        course.cohort_config = {'cohorted': False}
         self.assertFalse(course.is_cohorted)
 
         # and finally...
-        course.metadata['cohort_config'] = {'cohorted': True}
+        course.cohort_config = {'cohorted': True}
         self.assertTrue(course.is_cohorted)

common/lib/xmodule/xmodule/tests/test_self_assessment.py

@@ -28,8 +28,6 @@ class SelfAssessmentTest(unittest.TestCase):
     location = Location(["i4x", "edX", "sa_test", "selfassessment",
                          "SampleQuestion"])
 
-    metadata = {'attempts': '10'}
-
     descriptor = Mock()
 
     def setUp(self):
@@ -46,13 +44,14 @@ class SelfAssessmentTest(unittest.TestCase):
                 'max_score': 1,
                 'display_name': "Name",
                 'accept_file_upload': False,
-                'close_date': None
+                'close_date': None,
+                'attempts': '10'
+
                 }
 
         self.module = SelfAssessmentModule(test_system, self.location,
-                                      self.definition, self.descriptor,
-                                      static_data, 
-                                      state, metadata=self.metadata)
+                                      self.descriptor,
+                                      static_data)
 
     def test_get_html(self):
         html = self.module.get_html(test_system)

common/lib/xmodule/xmodule/x_module.py

@@ -390,9 +390,9 @@ class XModuleDescriptor(HTMLSnippet, ResourceTemplates, XBlock):
 
     def xmodule(self, system):
         """
-        Returns a constructor for an XModule. This constructor takes two
-        arguments: instance_state and shared_state, and returns a fully
-        instantiated XModule
+        Returns an XModule.
+
+        system: Module system
         """
         return self.module_class(
             system,