Add XBlock bulk saves to LMS/CMS

CHANGELOG.rst

@@ -9,6 +9,8 @@ Common: Added *experimental* support for jsinput type.
 
 Common: Added setting to specify Celery Broker vhost
 
+Common: Utilize new XBlock bulk save API in LMS and CMS.
+
 Studio: Add table for tracking course creator permissions (not yet used).
 Update rake django-admin[syncdb] and rake django-admin[migrate] so they
 run for both LMS and CMS.

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -46,6 +46,8 @@ def test_get_checklists(self):
         # Now delete the checklists from the course and verify they get repopulated (for courses
         # created before checklists were introduced).
         self.course.checklists = None
+        # Save the changed `checklists` to the underlying KeyValueStore before updating the modulestore
+        self.course.save()
         modulestore = get_modulestore(self.course.location)
         modulestore.update_metadata(self.course.location, own_metadata(self.course))
         self.assertEqual(self.get_persisted_checklists(), None)

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -87,6 +87,8 @@ def setUp(self):
         self.user.is_active = True
         # Staff has access to view all courses
         self.user.is_staff = True
+
+        # Save the data that we've just changed to the db.
         self.user.save()
 
         self.client = Client()
@@ -117,6 +119,10 @@ def check_components_on_page(self, component_types, expected_types):
 
         course.advanced_modules = component_types
 
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        course.save()
+
         store.update_metadata(course.location, own_metadata(course))
 
         # just pick one vertical
@@ -239,6 +245,9 @@ def test_draft_metadata(self):
 
         self.assertNotIn('graceperiod', own_metadata(html_module))
         html_module.lms.graceperiod = new_graceperiod
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        html_module.save()
         self.assertIn('graceperiod', own_metadata(html_module))
         self.assertEqual(html_module.lms.graceperiod, new_graceperiod)
 
@@ -883,6 +892,9 @@ def test_export_course_with_unknown_metadata(self):
         # add a bool piece of unknown metadata so we can verify we don't throw an exception
         metadata['new_metadata'] = True
 
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        course.save()
         module_store.update_metadata(location, metadata)
 
         print 'Exporting to tempdir = {0}'.format(root_dir)
@@ -1299,6 +1311,7 @@ def test_metadata_inheritance(self):
         # now let's define an override at the leaf node level
         #
         new_module.lms.graceperiod = timedelta(1)
+        new_module.save()
         module_store.update_metadata(new_module.location, own_metadata(new_module))
 
         # flush the cache and refetch

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -290,6 +290,71 @@ def test_update_grader_from_json(self):
         altered_grader = CourseGradingModel.update_grader_from_json(test_grader.course_location, test_grader.graders[1])
         self.assertDictEqual(test_grader.graders[1], altered_grader, "drop_count[1] + 2")
 
+    def test_update_cutoffs_from_json(self):
+        test_grader = CourseGradingModel.fetch(self.course.location)
+        CourseGradingModel.update_cutoffs_from_json(test_grader.course_location, test_grader.grade_cutoffs)
+        # Unlike other tests, need to actually perform a db fetch for this test since update_cutoffs_from_json
+        #  simply returns the cutoffs you send into it, rather than returning the db contents.
+        altered_grader = CourseGradingModel.fetch(self.course.location)
+        self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "Noop update")
+
+        test_grader.grade_cutoffs['D'] = 0.3
+        CourseGradingModel.update_cutoffs_from_json(test_grader.course_location, test_grader.grade_cutoffs)
+        altered_grader = CourseGradingModel.fetch(self.course.location)
+        self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "cutoff add D")
+
+        test_grader.grade_cutoffs['Pass'] = 0.75
+        CourseGradingModel.update_cutoffs_from_json(test_grader.course_location, test_grader.grade_cutoffs)
+        altered_grader = CourseGradingModel.fetch(self.course.location)
+        self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "cutoff change 'Pass'")
+
+    def test_delete_grace_period(self):
+        test_grader = CourseGradingModel.fetch(self.course.location)
+        CourseGradingModel.update_grace_period_from_json(test_grader.course_location, test_grader.grace_period)
+        # update_grace_period_from_json doesn't return anything, so query the db for its contents.
+        altered_grader = CourseGradingModel.fetch(self.course.location)
+        self.assertEqual(test_grader.grace_period, altered_grader.grace_period, "Noop update")
+
+        test_grader.grace_period = {'hours': 15, 'minutes': 5, 'seconds': 30}
+        CourseGradingModel.update_grace_period_from_json(test_grader.course_location, test_grader.grace_period)
+        altered_grader = CourseGradingModel.fetch(self.course.location)
+        self.assertDictEqual(test_grader.grace_period, altered_grader.grace_period, "Adding in a grace period")
+
+        test_grader.grace_period = {'hours': 1, 'minutes': 10, 'seconds': 0}
+        # Now delete the grace period
+        CourseGradingModel.delete_grace_period(test_grader.course_location)
+        # update_grace_period_from_json doesn't return anything, so query the db for its contents.
+        altered_grader = CourseGradingModel.fetch(self.course.location)
+        # Once deleted, the grace period should simply be None
+        self.assertEqual(None, altered_grader.grace_period, "Delete grace period")
+
+    def test_update_section_grader_type(self):
+        # Get the descriptor and the section_grader_type and assert they are the default values
+        descriptor = get_modulestore(self.course.location).get_item(self.course.location)
+        section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)
+
+        self.assertEqual('Not Graded', section_grader_type['graderType'])
+        self.assertEqual(None, descriptor.lms.format)
+        self.assertEqual(False, descriptor.lms.graded)
+
+        # Change the default grader type to Homework, which should also mark the section as graded
+        CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Homework'})
+        descriptor = get_modulestore(self.course.location).get_item(self.course.location)
+        section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)
+
+        self.assertEqual('Homework', section_grader_type['graderType'])
+        self.assertEqual('Homework', descriptor.lms.format)
+        self.assertEqual(True, descriptor.lms.graded)
+
+        # Change the grader type back to Not Graded, which should also unmark the section as graded
+        CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Not Graded'})
+        descriptor = get_modulestore(self.course.location).get_item(self.course.location)
+        section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location)
+
+        self.assertEqual('Not Graded', section_grader_type['graderType'])
+        self.assertEqual(None, descriptor.lms.format)
+        self.assertEqual(False, descriptor.lms.graded)
+
 
 class CourseMetadataEditingTest(CourseTestCase):
     """

cms/djangoapps/contentstore/tests/test_textbooks.py

@@ -62,6 +62,9 @@ def test_view_index_xhr_content(self):
             }
         ]
         self.course.pdf_textbooks = content
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        self.course.save()
         store = get_modulestore(self.course.location)
         store.update_metadata(self.course.location, own_metadata(self.course))
 
@@ -220,6 +223,9 @@ def setUp(self):
             'tid': 2,
         })
         self.course.pdf_textbooks = [self.textbook1, self.textbook2]
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        self.course.save()
         self.store = get_modulestore(self.course.location)
         self.store.update_metadata(self.course.location, own_metadata(self.course))
         self.url_nonexist = reverse('textbook_by_id', kwargs={

cms/djangoapps/contentstore/views/course.py

@@ -1,10 +1,9 @@
 """
 Views related to operations on course objects
 """
-#pylint: disable=W0402
 import json
 import random
-import string
+import string  # pylint: disable=W0402
 
 from django.contrib.auth.decorators import login_required
 from django_future.csrf import ensure_csrf_cookie
@@ -496,6 +495,9 @@ def textbook_index(request, org, course, name):
             if not any(tab['type'] == 'pdf_textbooks' for tab in course_module.tabs):
                 course_module.tabs.append({"type": "pdf_textbooks"})
             course_module.pdf_textbooks = textbooks
+            # Save the data that we've just changed to the underlying
+            # MongoKeyValueStore before we update the mongo datastore.
+            course_module.save()
             store.update_metadata(course_module.location, own_metadata(course_module))
             return JsonResponse(course_module.pdf_textbooks)
     else:
@@ -542,6 +544,9 @@ def create_textbook(request, org, course, name):
         tabs = course_module.tabs
         tabs.append({"type": "pdf_textbooks"})
         course_module.tabs = tabs
+    # Save the data that we've just changed to the underlying
+    # MongoKeyValueStore before we update the mongo datastore.
+    course_module.save()
     store.update_metadata(course_module.location, own_metadata(course_module))
     resp = JsonResponse(textbook, status=201)
     resp["Location"] = reverse("textbook_by_id", kwargs={
@@ -585,18 +590,22 @@ def textbook_by_id(request, org, course, name, tid):
             i = course_module.pdf_textbooks.index(textbook)
             new_textbooks = course_module.pdf_textbooks[0:i]
             new_textbooks.append(new_textbook)
-            new_textbooks.extend(course_module.pdf_textbooks[i+1:])
+            new_textbooks.extend(course_module.pdf_textbooks[i + 1:])
             course_module.pdf_textbooks = new_textbooks
         else:
             course_module.pdf_textbooks.append(new_textbook)
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        course_module.save()
         store.update_metadata(course_module.location, own_metadata(course_module))
         return JsonResponse(new_textbook, status=201)
     elif request.method == 'DELETE':
         if not textbook:
             return JsonResponse(status=404)
         i = course_module.pdf_textbooks.index(textbook)
         new_textbooks = course_module.pdf_textbooks[0:i]
-        new_textbooks.extend(course_module.pdf_textbooks[i+1:])
+        new_textbooks.extend(course_module.pdf_textbooks[i + 1:])
         course_module.pdf_textbooks = new_textbooks
+        course_module.save()
         store.update_metadata(course_module.location, own_metadata(course_module))
         return JsonResponse()

cms/djangoapps/contentstore/views/item.py

@@ -70,6 +70,9 @@ def save_item(request):
                 delattr(existing_item, metadata_key)
             else:
                 setattr(existing_item, metadata_key, value)
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        existing_item.save()
         # commit to datastore
         store.update_metadata(item_location, own_metadata(existing_item))
 

cms/djangoapps/contentstore/views/preview.py

@@ -7,7 +7,7 @@
 from django.contrib.auth.decorators import login_required
 from mitxmako.shortcuts import render_to_response
 
-from xmodule_modifiers import replace_static_urls, wrap_xmodule
+from xmodule_modifiers import replace_static_urls, wrap_xmodule, save_module  # pylint: disable=F0401
 from xmodule.error_module import ErrorDescriptor
 from xmodule.errortracker import exc_info_to_str
 from xmodule.exceptions import NotFoundError, ProcessingError
@@ -47,6 +47,8 @@ def preview_dispatch(request, preview_id, location, dispatch=None):
     # Let the module handle the AJAX
     try:
         ajax_return = instance.handle_ajax(dispatch, request.POST)
+        # Save any module data that has changed to the underlying KeyValueStore
+        instance.save()
 
     except NotFoundError:
         log.exception("Module indicating to user that request doesn't exist")
@@ -166,6 +168,11 @@ def load_preview_module(request, preview_id, descriptor):
         course_namespace=Location([module.location.tag, module.location.org, module.location.course, None, None])
     )
 
+    module.get_html = save_module(
+        module.get_html,
+        module
+    )
+
     return module
 
 

cms/djangoapps/contentstore/views/tabs.py

@@ -76,6 +76,9 @@ def reorder_static_tabs(request):
 
     # OK, re-assemble the static tabs in the new order
     course.tabs = reordered_tabs
+    # Save the data that we've just changed to the underlying
+    # MongoKeyValueStore before we update the mongo datastore.
+    course.save()
     modulestore('direct').update_metadata(course.location, own_metadata(course))
     return HttpResponse()
 

cms/djangoapps/models/settings/course_details.py

@@ -122,6 +122,10 @@ def update_from_json(cls, jsondict):
             descriptor.enrollment_end = converted
 
         if dirty:
+            # Save the data that we've just changed to the underlying
+            # MongoKeyValueStore before we update the mongo datastore.
+            descriptor.save()
+
             get_modulestore(course_location).update_metadata(course_location, own_metadata(descriptor))
 
         # NOTE: below auto writes to the db w/o verifying that any of the fields actually changed

cms/djangoapps/models/settings/course_grading.py

@@ -7,6 +7,9 @@ class CourseGradingModel(object):
     """
     Basically a DAO and Model combo for CRUD operations pertaining to grading policy.
     """
+    # Within this class, allow access to protected members of client classes.
+    # This comes up when accessing kvs data and caches during kvs saves and modulestore writes.
+    # pylint: disable=W0212
     def __init__(self, course_descriptor):
         self.course_location = course_descriptor.location
         self.graders = [CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader)]  # weights transformed to ints [0..100]
@@ -83,13 +86,16 @@ def update_from_json(jsondict):
         """
         course_location = Location(jsondict['course_location'])
         descriptor = get_modulestore(course_location).get_item(course_location)
-
         graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']]
 
         descriptor.raw_grader = graders_parsed
         descriptor.grade_cutoffs = jsondict['grade_cutoffs']
 
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        descriptor.save()
         get_modulestore(course_location).update_item(course_location, descriptor.xblock_kvs._data)
+
         CourseGradingModel.update_grace_period_from_json(course_location, jsondict['grace_period'])
 
         return CourseGradingModel.fetch(course_location)
@@ -116,6 +122,9 @@ def update_grader_from_json(course_location, grader):
         else:
             descriptor.raw_grader.append(grader)
 
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        descriptor.save()
         get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
 
         return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index])
@@ -131,6 +140,10 @@ def update_cutoffs_from_json(course_location, cutoffs):
 
         descriptor = get_modulestore(course_location).get_item(course_location)
         descriptor.grade_cutoffs = cutoffs
+
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        descriptor.save()
         get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
 
         return cutoffs
@@ -156,6 +169,10 @@ def update_grace_period_from_json(course_location, graceperiodjson):
 
             descriptor = get_modulestore(course_location).get_item(course_location)
             descriptor.lms.graceperiod = grace_timedelta
+
+            # Save the data that we've just changed to the underlying
+            # MongoKeyValueStore before we update the mongo datastore.
+            descriptor.save()
             get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata)
 
     @staticmethod
@@ -172,22 +189,11 @@ def delete_grader(course_location, index):
             del descriptor.raw_grader[index]
             # force propagation to definition
             descriptor.raw_grader = descriptor.raw_grader
-            get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
 
-    # NOTE cannot delete cutoffs. May be useful to reset
-    @staticmethod
-    def delete_cutoffs(course_location, cutoffs):
-        """
-        Resets the cutoffs to the defaults
-        """
-        if not isinstance(course_location, Location):
-            course_location = Location(course_location)
-
-        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._model_data._kvs._data)
-
-        return descriptor.grade_cutoffs
+            # Save the data that we've just changed to the underlying
+            # MongoKeyValueStore before we update the mongo datastore.
+            descriptor.save()
+            get_modulestore(course_location).update_item(course_location, descriptor._model_data._kvs._data)
 
     @staticmethod
     def delete_grace_period(course_location):
@@ -199,6 +205,10 @@ def delete_grace_period(course_location):
 
         descriptor = get_modulestore(course_location).get_item(course_location)
         del descriptor.lms.graceperiod
+
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        descriptor.save()
         get_modulestore(course_location).update_metadata(course_location, descriptor._model_data._kvs._metadata)
 
     @staticmethod
@@ -225,6 +235,9 @@ def update_section_grader_type(location, jsondict):
             del descriptor.lms.format
             del descriptor.lms.graded
 
+        # Save the data that we've just changed to the underlying
+        # MongoKeyValueStore before we update the mongo datastore.
+        descriptor.save()
         get_modulestore(location).update_metadata(location, descriptor._model_data._kvs._metadata)
 
     @staticmethod