Fixing tests

cms/djangoapps/contentstore/module_info_model.py

@@ -8,77 +8,78 @@ import re
 from django.http import HttpResponseBadRequest, Http404
 
 def get_module_info(store, location, parent_location = None, rewrite_static_links = False):
-  try:
-    if location.revision is None:
-        module = store.get_item(location)
-    else:
-        module = store.get_item(location)
-  except ItemNotFoundError:
-    raise Http404
+    try:
+        if location.revision is None:
+            module = store.get_item(location)
+        else:
+            module = store.get_item(location)
+    except ItemNotFoundError:
+        raise Http404
 
-  data = module.definition['data']
-  if rewrite_static_links:
-    data = replace_urls(module.definition['data'], course_namespace = Location([module.location.tag, module.location.org, module.location.course, None, None]))
+    data = module.data
+    if rewrite_static_links:
+        data = replace_urls(module.data, course_namespace = Location([module.location.tag, module.location.org, module.location.course, None, None]))
 
-  return {
+    return {
         'id': module.location.url(),
         'data': data,
-        'metadata': module.metadata
+        # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata
+        'metadata': module._model_data._kvs._metadata
     }
 
 def set_module_info(store, location, post_data):
-  module = None
-  isNew = False
-  try:
-    if location.revision is None:
-        module = store.get_item(location)
-    else:
-        module = store.get_item(location)
-  except:
-    pass
+    module = None
+    isNew = False
+    try:
+        if location.revision is None:
+            module = store.get_item(location)
+        else:
+            module = store.get_item(location)
+    except:
+        pass
 
-  if module is None:
-    # new module at this location
-    # presume that we have an 'Empty' template
-    template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty'])
-    module = store.clone_item(template_location, location)
-    isNew = True
+    if module is None:
+        # new module at this location
+        # presume that we have an 'Empty' template
+        template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty'])
+        module = store.clone_item(template_location, location)
+        isNew = True
 
-  if post_data.get('data') is not None:
-      data = post_data['data']
-      store.update_item(location, data)
-     
-  # cdodge: note calling request.POST.get('children') will return None if children is an empty array
-  # so it lead to a bug whereby the last component to be deleted in the UI was not actually
-  # deleting the children object from the children collection
-  if 'children' in post_data and post_data['children'] is not None:
-      children = post_data['children']
-      store.update_children(location, children)
+    if post_data.get('data') is not None:
+        data = post_data['data']
+        store.update_item(location, data)
 
-  # cdodge: also commit any metadata which might have been passed along in the
-  # POST from the client, if it is there
-  # NOTE, that the postback is not the complete metadata, as there's system metadata which is
-  # not presented to the end-user for editing. So let's fetch the original and
-  # 'apply' the submitted metadata, so we don't end up deleting system metadata
-  if post_data.get('metadata') is not None:
-      posted_metadata = post_data['metadata']
+    # cdodge: note calling request.POST.get('children') will return None if children is an empty array
+    # so it lead to a bug whereby the last component to be deleted in the UI was not actually
+    # deleting the children object from the children collection
+    if 'children' in post_data and post_data['children'] is not None:
+        children = post_data['children']
+        store.update_children(location, children)
 
-      # update existing metadata with submitted metadata (which can be partial)
-      # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it'
-      for metadata_key in posted_metadata.keys():
+    # cdodge: also commit any metadata which might have been passed along in the
+    # POST from the client, if it is there
+    # NOTE, that the postback is not the complete metadata, as there's system metadata which is
+    # not presented to the end-user for editing. So let's fetch the original and
+    # 'apply' the submitted metadata, so we don't end up deleting system metadata
+    if post_data.get('metadata') is not None:
+        posted_metadata = post_data['metadata']
 
-          # let's strip out any metadata fields from the postback which have been identified as system metadata
-          # and therefore should not be user-editable, so we should accept them back from the client
-          if metadata_key in module.system_metadata_fields:
-              del posted_metadata[metadata_key]
-          elif posted_metadata[metadata_key] is None:
-              # remove both from passed in collection as well as the collection read in from the modulestore
-              if metadata_key in module.metadata:
-                  del module.metadata[metadata_key]
-              del posted_metadata[metadata_key]
+        # update existing metadata with submitted metadata (which can be partial)
+        # IMPORTANT NOTE: if the client passed pack 'null' (None) for a piece of metadata that means 'remove it'
+        for metadata_key in posted_metadata.keys():
 
-      # overlay the new metadata over the modulestore sourced collection to support partial updates
-      module.metadata.update(posted_metadata)
+            # let's strip out any metadata fields from the postback which have been identified as system metadata
+            # and therefore should not be user-editable, so we should accept them back from the client
+            if metadata_key in module.system_metadata_fields:
+                del posted_metadata[metadata_key]
+            elif posted_metadata[metadata_key] is None:
+                # remove both from passed in collection as well as the collection read in from the modulestore
+                if metadata_key in module._model_data:
+                    del module._model_data[metadata_key]
+                del posted_metadata[metadata_key]
+            else:
+                module._model_data[metadata_key] = value
 
-      # commit to datastore
-      store.update_metadata(location, module.metadata)
+        # commit to datastore
+        # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata
+        store.update_metadata(item_location, module._model_data._kvs._metadata)

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -255,8 +255,10 @@ class CourseGradingTest(CourseTestCase):
         altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__)
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "cutoff add D")
         
-        test_grader.grace_period = {'hours' :  '4'}
+        test_grader.grace_period = {'hours': '4'}
         altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__)
+        print test_grader.__dict__
+        print altered_grader.__dict__
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "4 hour grace period")
         
     def test_update_grader_from_json(self):

cms/djangoapps/contentstore/views.py

@@ -410,8 +410,6 @@ def preview_dispatch(request, preview_id, location, dispatch=None):
         log.exception("error processing ajax call")
         raise
 
-    print request.session.items()
-
     return HttpResponse(ajax_return)
 
 
@@ -634,8 +632,6 @@ def save_item(request):
             if metadata_key in existing_item.system_metadata_fields:
                 del posted_metadata[metadata_key]
             elif posted_metadata[metadata_key] is None:
-                print "DELETING", metadata_key, value
-                print metadata_key in existing_item._model_data
                 # remove both from passed in collection as well as the collection read in from the modulestore
                 if metadata_key in existing_item._model_data:
                     del existing_item._model_data[metadata_key]
@@ -645,7 +641,6 @@ def save_item(request):
 
         # commit to datastore
         # TODO (cpennington): This really shouldn't have to do this much reaching in to get the metadata
-        print existing_item._model_data._kvs._metadata
         store.update_metadata(item_location, existing_item._model_data._kvs._metadata)
 
     return HttpResponse()
@@ -1231,7 +1226,7 @@ def initialize_course_tabs(course):
         {"type": "wiki", "name": "Wiki"},
         {"type": "progress", "name": "Progress"}]
 
-    modulestore('direct').update_metadata(course.location.url(), own_metadata(new_course))
+    modulestore('direct').update_metadata(course.location.url(), own_metadata(course))
 
 @ensure_csrf_cookie
 @login_required

cms/djangoapps/models/settings/course_grading.py

@@ -2,6 +2,7 @@ from xmodule.modulestore import Location
 from contentstore.utils import get_modulestore
 import re
 from util import converters
+from datetime import timedelta
 
 
 class CourseGradingModel:
@@ -91,7 +92,7 @@ class CourseGradingModel:
         descriptor.raw_grader = graders_parsed
         descriptor.grade_cutoffs = jsondict['grade_cutoffs']
         
-        get_modulestore(course_location).update_item(course_location, descriptor.definition['data'])
+        get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
         CourseGradingModel.update_grace_period_from_json(course_location, jsondict['grace_period'])
         
         return CourseGradingModel.fetch(course_location)
@@ -119,7 +120,7 @@ class CourseGradingModel:
         else:
             descriptor.raw_grader.append(grader)
             
-        get_modulestore(course_location).update_item(course_location, descriptor.definition['data'])
+        get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
         
         return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index])
         
@@ -155,11 +156,17 @@ class CourseGradingModel:
             if 'grace_period' in graceperiodjson:
                 graceperiodjson = graceperiodjson['grace_period']
 
-            grace_rep = " ".join(["%s %s" % (value, key) for (key, value) in graceperiodjson.iteritems()])
+            timedelta_kwargs = dict(
+                (key, float(val))
+                for key, val
+                in graceperiodjson.items()
+                if key in ('days', 'seconds', 'minutes', 'hours')
+            )
+            grace_rep = timedelta(**timedelta_kwargs)
 
             descriptor = get_modulestore(course_location).get_item(course_location)
-            descriptor.metadata['graceperiod'] = grace_rep
-            get_modulestore(course_location).update_metadata(course_location, descriptor.metadata)
+            descriptor.lms.graceperiod = grace_rep
+            get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata)
         
     @staticmethod
     def delete_grader(course_location, index):
@@ -170,12 +177,12 @@ class CourseGradingModel:
             course_location = Location(course_location)
         
         descriptor = get_modulestore(course_location).get_item(course_location)
-        index = int(index)        
+        index = int(index)
         if index < len(descriptor.raw_grader):
             del descriptor.raw_grader[index]
             # force propagation to definition
             descriptor.raw_grader = descriptor.raw_grader
-            get_modulestore(course_location).update_item(course_location, descriptor.definition['data'])
+            get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
         
     # NOTE cannot delete cutoffs. May be useful to reset 
     @staticmethod
@@ -188,7 +195,7 @@ class CourseGradingModel:
         
         descriptor = get_modulestore(course_location).get_item(course_location)
         descriptor.grade_cutoffs = descriptor.defaut_grading_policy['GRADE_CUTOFFS']
-        get_modulestore(course_location).update_item(course_location, descriptor.definition['data'])
+        get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
         
         return descriptor.grade_cutoffs
         
@@ -202,7 +209,7 @@ class CourseGradingModel:
             
         descriptor = get_modulestore(course_location).get_item(course_location)
         if 'graceperiod' in descriptor.metadata: del descriptor.metadata['graceperiod']
-        get_modulestore(course_location).update_metadata(course_location, descriptor.metadata)
+        get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._data)
         
     @staticmethod
     def get_section_grader_type(location):
@@ -240,14 +247,22 @@ class CourseGradingModel:
             hours_from_days = rawgrace.days*24
             seconds = rawgrace.seconds
             hours_from_seconds = int(seconds / 3600)
+            hours = hours_from_days + hours_from_seconds
             seconds -= hours_from_seconds * 3600
             minutes = int(seconds / 60)
             seconds -= minutes * 60
-            return {
-                'hours': hours_from_days + hours_from_seconds,
-                'minutes': minutes,
-                'seconds': seconds,
-            }
+
+            graceperiod = {}
+            if hours > 0:
+                graceperiod['hours'] = str(hours)
+
+            if minutes > 0:
+                graceperiod['minutes'] = str(minutes)
+
+            if seconds > 0:
+                graceperiod['seconds'] = str(seconds)
+
+            return graceperiod
         else:
             return None
 

common/lib/xmodule/xmodule/capa_module.py

@@ -421,13 +421,6 @@ class CapaModule(XModule):
         new_answers = dict()
         for answer_id in answers:
             try:
-<<<<<<< HEAD
-<<<<<<< HEAD
-                new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.metadata['data_dir'], course_namespace=self.location)}
-=======
-                new_answer = {answer_id: self.system.replace_urls(answers[answer_id], self.descriptor.data_dir)}
->>>>>>> WIP: Save student state via StudentModule. Inheritance doesn't work
-=======
                 new_answer = {
                     answer_id: self.system.replace_urls(
                         answers[answer_id],
@@ -435,7 +428,6 @@ class CapaModule(XModule):
                         course_namespace=self.location
                     )
                 }
->>>>>>> Fix more errors in tests
             except TypeError:
                 log.debug('Unable to perform URL substitution on answers[%s]: %s' % (answer_id, answers[answer_id]))
                 new_answer = {answer_id: answers[answer_id]}

common/lib/xmodule/xmodule/course_module.py

@@ -13,7 +13,7 @@ import time
 import copy
 
 from .model import Scope, ModelType, List, String, Object, Boolean
-from .x_module import Date
+from .fields import Date
 
 log = logging.getLogger(__name__)
 
@@ -99,7 +99,7 @@ class CourseDescriptor(SequenceDescriptor):
     start = Date(help="Start time when this module is visible", scope=Scope.settings)
     end = Date(help="Date that this class ends", scope=Scope.settings)
     advertised_start = Date(help="Date that this course is advertised to start", scope=Scope.settings)
-    grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content)
+    grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content, default={})
     show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, 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)

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -20,6 +20,7 @@ from . import ModuleStoreBase, Location
 from .draft import DraftModuleStore
 from .exceptions import (ItemNotFoundError,
                          DuplicateItemError)
+from .inheritance import own_metadata
 
 
 log = logging.getLogger(__name__)
@@ -40,7 +41,6 @@ class MongoKeyValueStore(KeyValueStore):
         self._metadata = metadata
 
     def get(self, key):
-        print "GET", key
         if key.field_name == 'children':
             return self._children
         elif key.scope == Scope.settings:
@@ -54,7 +54,6 @@ class MongoKeyValueStore(KeyValueStore):
             raise InvalidScopeError(key.scope)
 
     def set(self, key, value):
-        print "SET", key, value
         if key.field_name == 'children':
             self._children = value
         elif key.scope == Scope.settings:
@@ -68,7 +67,6 @@ class MongoKeyValueStore(KeyValueStore):
             raise InvalidScopeError(key.scope)
 
     def delete(self, key):
-        print "DELETE", key
         if key.field_name == 'children':
             self._children = []
         elif key.scope == Scope.settings:
@@ -457,6 +455,7 @@ class MongoModuleStore(ModuleStoreBase):
                     tab['name'] = metadata.get('display_name')
                     break
             course.tabs = existing_tabs
+            self.update_metadata(course.location, own_metadata(course))
 
         self._update_single_item(location, {'metadata': metadata})
 
@@ -474,7 +473,7 @@ class MongoModuleStore(ModuleStoreBase):
             course = self.get_course_for_item(item.location)
             existing_tabs = course.tabs or []
             course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != location.name]
-            self.update_metadata(course.location, course.metadata)
+            self.update_metadata(course.location, own_metadata(course))
 
         self.collection.remove({'_id': Location(location).dict()})
 

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

@@ -40,22 +40,24 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele
 
     print "Cloning module {0} to {1}....".format(original_loc, module.location)
 
-    if 'data' in module.definition:
-      modulestore.update_item(module.location, module.definition['data'])
+    modulestore.update_item(module.location, module._model_data._kvs._data)
 
     # repoint children
-    if 'children' in module.definition:
+    if module.has_children:
       new_children = []
-      for child_loc_url in module.definition['children']:
+      for child_loc_url in module.children:
         child_loc = Location(child_loc_url)
-        child_loc = child_loc._replace(tag = dest_location.tag, org = dest_location.org, 
-        course = dest_location.course)
-        new_children = new_children + [child_loc.url()]
+        child_loc = child_loc._replace(
+          tag = dest_location.tag,
+          org = dest_location.org,
+          course = dest_location.course
+        )
+        new_children.append(child_loc.url())
 
       modulestore.update_children(module.location, new_children)
 
     # save metadata
-    modulestore.update_metadata(module.location, module.metadata)
+    modulestore.update_metadata(module.location, module._model_data._kvs._metadata)
 
   # now iterate through all of the assets and clone them
   # first the thumbnails

common/lib/xmodule/xmodule/runtime.py

@@ -116,5 +116,4 @@ class DbModel(MutableMapping):
         fields = [field.name for field in self._module_cls.fields]
         for namespace_name in self._module_cls.namespaces:
             fields.extend(field.name for field in getattr(self._module_cls, namespace_name).fields)
-        print fields
         return fields

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

@@ -271,8 +271,8 @@ class ImportTestCase(unittest.TestCase):
         location = Location(["i4x", "edX", "toy", "video", "Welcome"])
         toy_video = modulestore.get_instance(toy_id, location)
         two_toy_video =  modulestore.get_instance(two_toy_id, location)
-        self.assertEqual(toy_video.youtube, "1.0:p2Q6BrNhdh8")
-        self.assertEqual(two_toy_video.youtube, "1.0:p2Q6BrNhdh9")
+        self.assertEqual(etree.fromstring(toy_video.data).get('youtube'), "1.0:p2Q6BrNhdh8")
+        self.assertEqual(etree.fromstring(two_toy_video.data).get('youtube'), "1.0:p2Q6BrNhdh9")
 
 
     def test_colon_in_url_name(self):