From c601b75675e804f2eca668fb25c9a713716ee1ee Mon Sep 17 00:00:00 2001
From: Don Mitchell <dmitchell@edx.org>
Date: Thu, 16 Jan 2014 15:41:26 -0500
Subject: [PATCH 1/4] MixedModulestore wraps most getters, update_item,
 delete_item with code to translate between addressing schemes based on app
 and persistence layer addressing scheme specification.

STUD-1206
---
 CHANGELOG.rst                                 |   6 +
 .../contentstore/course_info_model.py         |   4 +-
 .../contentstore/tests/test_checklists.py     |   2 +-
 .../contentstore/tests/test_contentstore.py   |  84 +----
 .../contentstore/tests/test_course_updates.py |   2 +-
 .../contentstore/tests/test_orphan.py         |   3 +-
 .../contentstore/tests/test_textbooks.py      |   7 +-
 .../contentstore/tests/test_transcripts.py    |  57 ++--
 .../contentstore/transcripts_utils.py         |   2 +-
 cms/djangoapps/contentstore/utils.py          |  10 -
 .../contentstore/views/checklist.py           |   5 +-
 cms/djangoapps/contentstore/views/course.py   |  20 +-
 cms/djangoapps/contentstore/views/item.py     |  43 ++-
 cms/djangoapps/contentstore/views/tabs.py     |   8 +-
 .../models/settings/course_details.py         |  39 ++-
 .../models/settings/course_grading.py         |  28 +-
 .../models/settings/course_metadata.py        |   3 +-
 .../external_auth/tests/test_shib.py          |  16 +-
 common/djangoapps/student/tests/test_login.py |   4 +-
 .../xmodule/xmodule/modulestore/__init__.py   |  47 +--
 .../xmodule/modulestore/loc_mapper_store.py   |  10 +-
 .../xmodule/xmodule/modulestore/locator.py    |  13 +
 .../lib/xmodule/xmodule/modulestore/mixed.py  | 310 +++++++++++++++---
 .../xmodule/xmodule/modulestore/mongo/base.py | 141 ++++----
 .../xmodule/modulestore/mongo/draft.py        |  65 +---
 .../xmodule/modulestore/split_mongo/split.py  |  35 +-
 .../xmodule/modulestore/store_utilities.py    |  16 +-
 .../xmodule/modulestore/tests/django_utils.py |   8 +-
 .../xmodule/modulestore/tests/factories.py    |   8 +-
 .../modulestore/tests/test_location_mapper.py |  12 +-
 .../modulestore/tests/test_locators.py        |  18 +
 .../tests/test_mixed_modulestore.py           |  13 +-
 .../xmodule/modulestore/tests/test_orphan.py  |   2 +-
 .../xmodule/modulestore/tests/test_publish.py |   6 +-
 .../modulestore/tests/test_split_migrator.py  |  10 +-
 common/lib/xmodule/xmodule/modulestore/xml.py |  32 +-
 .../xmodule/modulestore/xml_importer.py       |  41 +--
 .../tests/test_submitting_problems.py         |   4 +-
 .../tests/test_view_authentication.py         |  40 ++-
 lms/djangoapps/courseware/views.py            |   4 +-
 .../instructor_task/tests/test_base.py        |   4 +-
 .../instructor_task/tests/test_integration.py |   6 +-
 lms/envs/acceptance.py                        |   1 +
 lms/envs/bok_choy.auth.json                   |   1 +
 lms/envs/cms/mixed_dev.py                     |   1 +
 45 files changed, 632 insertions(+), 559 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1e464d063a1c..ea4753a51f35 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -49,6 +49,12 @@ Blades: Fix comparison of float numbers. BLD-434.
 
 Blades: Allow regexp strings as the correct answer to a string response question. BLD-475.
 
+Common: MixedModulestore is now the only approved access to the persistence layer
+  - takes a new parameter 'reference_type' which can be 'Location' or 'Locator'. Mixed
+  then tries to ensure that every reference in any xblock gets converted to that type on
+  retrieval. Because we're moving to Locators, the default is Locator; so, you should change
+  all existing configurations to 'Location' (unless you're using split)
+
 Common: Add feature flags to allow developer use of pure XBlocks
   - ALLOW_ALL_ADVANCED_COMPONENTS disables the hard-coded list of advanced
     components in Studio, and allows any xblock to be added as an
diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py
index 314793db72ac..0e2551cac81f 100644
--- a/cms/djangoapps/contentstore/course_info_model.py
+++ b/cms/djangoapps/contentstore/course_info_model.py
@@ -102,7 +102,7 @@ def update_course_updates(location, update, passed_id=None):
 
     # update db record
     course_updates.data = html.tostring(course_html_parsed)
-    modulestore('direct').update_item(location, course_updates.data)
+    modulestore('direct').update_item(course_updates, 'course_info_model')
 
     return {
         "id": idx,
@@ -158,7 +158,7 @@ def delete_course_update(location, update, passed_id):
         # update db record
         course_updates.data = html.tostring(course_html_parsed)
         store = modulestore('direct')
-        store.update_item(location, course_updates.data)
+        store.update_item(course_updates, 'course_info_model')
 
     return get_course_updates(location, None)
 
diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py
index 864fe23c8333..14164680fc7a 100644
--- a/cms/djangoapps/contentstore/tests/test_checklists.py
+++ b/cms/djangoapps/contentstore/tests/test_checklists.py
@@ -54,7 +54,7 @@ def test_get_checklists(self):
         # 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))
+        modulestore.update_item(self.course, self.user.pk)
         self.assertEqual(self.get_persisted_checklists(), None)
         response = self.client.get(self.checklists_url)
         self.assertEqual(payload, response.content)
diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index d28a3ab3de40..c7f4b9339c70 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -47,8 +47,6 @@
 
 from django_comment_common.utils import are_permissions_roles_seeded
 from xmodule.exceptions import InvalidVersionError
-import datetime
-from pytz import UTC
 from uuid import uuid4
 from pymongo import MongoClient
 from student.models import CourseEnrollment
@@ -126,11 +124,7 @@ 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))
+        store.update_item(course, self.user.username)
 
         # just pick one vertical
         descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
@@ -269,7 +263,7 @@ def test_draft_metadata(self):
         self.assertIn('graceperiod', own_metadata(html_module))
         self.assertEqual(html_module.graceperiod, new_graceperiod)
 
-        draft_store.update_metadata(html_module.location, own_metadata(html_module))
+        draft_store.update_item(html_module, self.user.username)
 
         # read back to make sure it reads as 'own-metadata'
         html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None))
@@ -385,8 +379,7 @@ def test_create_static_tab_and_rename(self):
         self.assertEqual(course.tabs, expected_tabs)
 
         item.display_name = 'Updated'
-        item.save()
-        module_store.update_metadata(item.location, own_metadata(item))
+        module_store.update_item(item, self.user.username)
 
         course = module_store.get_item(course_location)
 
@@ -834,9 +827,9 @@ def test_portable_link_rewrites_during_clone_course(self):
         html_module = module_store.get_instance(source_location.course_id, html_module_location)
 
         self.assertIsInstance(html_module.data, basestring)
-        new_data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format(
+        new_data = html_module.data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format(
             source_location.org, source_location.course))
-        module_store.update_item(html_module_location, new_data)
+        module_store.update_item(html_module, None)
 
         html_module = module_store.get_instance(source_location.course_id, html_module_location)
         self.assertEqual(new_data, html_module.data)
@@ -858,22 +851,18 @@ def test_illegal_draft_crud_ops(self):
         draft_store = modulestore('draft')
         direct_store = modulestore('direct')
 
-        CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
+        course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
 
         location = Location('i4x://MITx/999/chapter/neuvo')
         # Ensure draft mongo store does not allow us to create chapters either directly or via convert to draft
         self.assertRaises(InvalidVersionError, draft_store.create_and_save_xmodule, location)
         direct_store.create_and_save_xmodule(location)
         self.assertRaises(InvalidVersionError, draft_store.convert_to_draft, location)
+        chapter = draft_store.get_instance(course.id, location)
+        chapter.data = 'chapter data'
 
-        self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data')
-
-        # taking advantage of update_children and other functions never checking that the ids are valid
-        self.assertRaises(InvalidVersionError, draft_store.update_children, location,
-                          ['i4x://MITx/999/problem/doesntexist'])
-
-        self.assertRaises(InvalidVersionError, draft_store.update_metadata, location,
-                          {'due': datetime.datetime.now(UTC)})
+        with self.assertRaises(InvalidVersionError):
+            draft_store.update_item(chapter, 'user')
 
         self.assertRaises(InvalidVersionError, draft_store.unpublish, location)
 
@@ -992,8 +981,8 @@ def test_export_course(self, mock_get):
         sequential = module_store.get_item(Location(['i4x', 'edX', 'toy',
                                            'sequential', 'vertical_sequential', None]))
         private_location_no_draft = private_vertical.location.replace(revision=None)
-        module_store.update_children(sequential.location, sequential.children +
-                                     [private_location_no_draft.url()])
+        sequential.children.append(private_location_no_draft.url())
+        module_store.update_item(sequential, 'user')
 
         # read back the sequential, to make sure we have a pointer to
         sequential = module_store.get_item(Location(['i4x', 'edX', 'toy',
@@ -1285,31 +1274,6 @@ def test_prefetch_children(self):
         self.assertFalse(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])
                          in course.system.module_data)
 
-    def test_export_course_with_unknown_metadata(self):
-        module_store = modulestore('direct')
-        content_store = contentstore()
-
-        import_from_xml(module_store, 'common/test/data/', ['toy'])
-        location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
-
-        root_dir = path(mkdtemp_clean())
-
-        course = module_store.get_item(location)
-
-        metadata = own_metadata(course)
-        # 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)
-
-        # export out to a tempdir
-        export_to_xml(module_store, content_store, location, root_dir, 'test_export')
-
     def test_export_course_without_content_store(self):
         module_store = modulestore('direct')
         content_store = contentstore()
@@ -1319,16 +1283,7 @@ def test_export_course_without_content_store(self):
         import_from_xml(module_store, 'common/test/data/', ['toy'])
         location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
 
-        # Add a sequence
-
         stub_location = Location(['i4x', 'edX', 'toy', 'sequential', 'vertical_sequential'])
-        sequential = module_store.get_item(stub_location)
-        module_store.update_children(sequential.location, sequential.children)
-
-        # Get course and export it without a content_store
-
-        course = module_store.get_item(location)
-        course.save()
 
         root_dir = path(mkdtemp_clean())
 
@@ -1343,7 +1298,7 @@ def test_export_course_without_content_store(self):
             module_store, root_dir, ['test_export_no_content_store'],
             draft_store=None,
             static_content_store=None,
-            target_location_namespace=course.location
+            target_location_namespace=location
         )
 
         # Verify reimported course
@@ -1810,7 +1765,8 @@ def test_metadata_inheritance(self):
         # crate a new module and add it as a child to a vertical
         module_store.create_and_save_xmodule(new_component_location)
         parent = verticals[0]
-        module_store.update_children(parent.location, parent.children + [new_component_location.url()])
+        parent.children.append(new_component_location.url())
+        module_store.update_item(parent, 'user')
 
         # flush the cache
         module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
@@ -1827,8 +1783,7 @@ def test_metadata_inheritance(self):
         # now let's define an override at the leaf node level
         #
         new_module.graceperiod = timedelta(1)
-        new_module.save()
-        module_store.update_metadata(new_module.location, own_metadata(new_module))
+        module_store.update_item(new_module, self.user.username)
 
         # flush the cache and refetch
         module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
@@ -1942,10 +1897,7 @@ def test_metadata_not_persistence(self):
             delattr(self.video_descriptor, field_name)
 
         self.assertNotIn('html5_sources', own_metadata(self.video_descriptor))
-        get_modulestore(location).update_metadata(
-            location,
-            own_metadata(self.video_descriptor)
-        )
+        get_modulestore(location).update_item(self.video_descriptor, 'testuser')
         module = get_modulestore(location).get_item(location)
 
         self.assertNotIn('html5_sources', own_metadata(module))
@@ -2001,7 +1953,7 @@ def _course_factory_create_course():
     Creates a course via the CourseFactory and returns the locator for it.
     """
     course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
-    return loc_mapper().translate_location(course.location.course_id, course.location, True, True)
+    return loc_mapper().translate_location(course.id, course.location, False, True)
 
 
 def _get_course_id(test_course_data):
diff --git a/cms/djangoapps/contentstore/tests/test_course_updates.py b/cms/djangoapps/contentstore/tests/test_course_updates.py
index 5ee5f1289bb7..1a112f87e14e 100644
--- a/cms/djangoapps/contentstore/tests/test_course_updates.py
+++ b/cms/djangoapps/contentstore/tests/test_course_updates.py
@@ -123,7 +123,7 @@ def test_no_ol_course_update(self):
         modulestore('direct').create_and_save_xmodule(location)
         course_updates = modulestore('direct').get_item(location)
         course_updates.data = 'bad news'
-        modulestore('direct').update_item(location, course_updates.data)
+        modulestore('direct').update_item(course_updates, 'test_course_updates')
 
         init_content = '<iframe width="560" height="315" src="http://www.youtube.com/embed/RocY-Jd93XU" frameborder="0">'
         content = init_content + '</iframe>'
diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py
index e7babd25fd49..3585d81e1569 100644
--- a/cms/djangoapps/contentstore/tests/test_orphan.py
+++ b/cms/djangoapps/contentstore/tests/test_orphan.py
@@ -4,7 +4,6 @@
 import json
 from contentstore.tests.utils import CourseTestCase
 from xmodule.modulestore.django import editable_modulestore, loc_mapper
-from django.core.urlresolvers import reverse
 from student.models import CourseEnrollment
 
 class TestOrphan(CourseTestCase):
@@ -35,7 +34,7 @@ def _create_item(self, category, name, data, metadata, parent_category, parent_n
             parent_location = self.course.location.replace(category=parent_category, name=parent_name)
             parent = editable_modulestore('direct').get_item(parent_location)
             parent.children.append(location.url())
-            editable_modulestore('direct').update_children(parent_location, parent.children)
+            editable_modulestore('direct').update_item(parent, self.user.pk)
 
     def test_mongo_orphan(self):
         """
diff --git a/cms/djangoapps/contentstore/tests/test_textbooks.py b/cms/djangoapps/contentstore/tests/test_textbooks.py
index d1fe60635440..aab4ccc39989 100644
--- a/cms/djangoapps/contentstore/tests/test_textbooks.py
+++ b/cms/djangoapps/contentstore/tests/test_textbooks.py
@@ -58,11 +58,8 @@ 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))
+        store.update_item(self.course, self.user.pk)
 
         resp = self.client.get(
             self.url,
@@ -200,7 +197,7 @@ def setUp(self):
         # 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.store.update_item(self.course, self.user.pk)
         self.url_nonexist = self.course_locator.url_reverse("textbooks", "20")
 
     def test_get_1(self):
diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/tests/test_transcripts.py
index f4c7f773adf0..57e30ed9db1e 100644
--- a/cms/djangoapps/contentstore/tests/test_transcripts.py
+++ b/cms/djangoapps/contentstore/tests/test_transcripts.py
@@ -63,13 +63,13 @@ def setUp(self):
         self.item_locator, self.item_location = self._get_locator(resp)
         self.assertEqual(resp.status_code, 200)
 
+        self.item = modulestore().get_item(self.item_location)
         # hI10vDNYz4M - valid Youtube ID with transcripts.
         # JMD_ifUUfsU, AKqURZnYqpk, DYpADpL7jAY - valid Youtube IDs without transcripts.
-        data = '<video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY" />'
-        modulestore().update_item(self.item_location, data)
+        self.item.data = '<video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY" />'
+        modulestore().update_item(self.item, 'test_transcripts')
 
         self.item = modulestore().get_item(self.item_location)
-
         # Remove all transcripts for current module.
         self.clear_subs_content()
 
@@ -130,14 +130,14 @@ def setUp(self):
         self.bad_name_srt_file.seek(0)
 
     def test_success_video_module_source_subs_uploading(self):
-        data = textwrap.dedent("""
+        self.item.data = textwrap.dedent("""
             <video youtube="">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """)
-        modulestore().update_item(self.item_location, data)
+        modulestore().update_item(self.item, 'test_transcripts')
 
         link = reverse('upload_transcripts')
         filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
@@ -212,8 +212,9 @@ def test_fail_for_non_video_module(self):
         }
         resp = self.client.ajax_post('/xblock', data)
         item_locator, item_location = self._get_locator(resp)
-        data = '<non_video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M" />'
-        modulestore().update_item(item_location, data)
+        item = modulestore().get_item(item_location)
+        item.data = '<non_video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M" />'
+        modulestore().update_item(item, 'test_transcripts')
 
         # non_video module: testing
 
@@ -232,8 +233,8 @@ def test_fail_for_non_video_module(self):
         self.assertEqual(json.loads(resp.content).get('status'), 'Transcripts are supported only for "video" modules.')
 
     def test_fail_bad_xml(self):
-        data = '<<<video youtube="0.75:JMD_ifUUfsU,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY" />'
-        modulestore().update_item(self.item_location, data)
+        self.item.data = '<<<video youtube="0.75:JMD_ifUUfsU,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY" />'
+        modulestore().update_item(self.item, 'test_transcripts')
 
         link = reverse('upload_transcripts')
         filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
@@ -344,8 +345,8 @@ def remove_subs_from_store(self, subs_id):
             pass
 
     def test_success_download_youtube(self):
-        data = '<video youtube="1:JMD_ifUUfsU" />'
-        modulestore().update_item(self.item_location, data)
+        self.item.data = '<video youtube="1:JMD_ifUUfsU" />'
+        modulestore().update_item(self.item, 'test_transcripts')
 
         subs = {
             'start': [100, 200, 240],
@@ -365,14 +366,14 @@ def test_success_download_youtube(self):
 
     def test_success_download_nonyoutube(self):
         subs_id = str(uuid4())
-        data = textwrap.dedent("""
+        self.item.data = textwrap.dedent("""
             <video youtube="" sub="{}">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """.format(subs_id))
-        modulestore().update_item(self.item_location, data)
+        modulestore().update_item(self.item, 'test_transcripts')
 
         subs = {
             'start': [100, 200, 240],
@@ -424,14 +425,15 @@ def test_fail_for_non_video_module(self):
         resp = self.client.ajax_post('/xblock', data)
         item_locator, item_location = self._get_locator(resp)
         subs_id = str(uuid4())
-        data = textwrap.dedent("""
+        item = modulestore().get_item(item_location)
+        item.data = textwrap.dedent("""
             <videoalpha youtube="" sub="{}">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </videoalpha>
         """.format(subs_id))
-        modulestore().update_item(item_location, data)
+        modulestore().update_item(item, 'test_transcripts')
 
         subs = {
             'start': [100, 200, 240],
@@ -449,28 +451,28 @@ def test_fail_for_non_video_module(self):
         self.assertEqual(resp.status_code, 404)
 
     def test_fail_nonyoutube_subs_dont_exist(self):
-        data = textwrap.dedent("""
+        self.item.data = textwrap.dedent("""
             <video youtube="" sub="UNDEFINED">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """)
-        modulestore().update_item(self.item_location, data)
+        modulestore().update_item(self.item, 'test_transcripts')
 
         link = reverse('download_transcripts')
         resp = self.client.get(link, {'locator': self.item_locator})
         self.assertEqual(resp.status_code, 404)
 
     def test_empty_youtube_attr_and_sub_attr(self):
-        data = textwrap.dedent("""
+        self.item.data = textwrap.dedent("""
             <video youtube="">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """)
-        modulestore().update_item(self.item_location, data)
+        modulestore().update_item(self.item, 'test_transcripts')
 
         link = reverse('download_transcripts')
         resp = self.client.get(link, {'locator': self.item_locator})
@@ -479,14 +481,14 @@ def test_empty_youtube_attr_and_sub_attr(self):
 
     def test_fail_bad_sjson_subs(self):
         subs_id = str(uuid4())
-        data = textwrap.dedent("""
+        self.item.data = textwrap.dedent("""
             <video youtube="" sub="{}">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """.format(subs_id))
-        modulestore().update_item(self.item_location, data)
+        modulestore().update_item(self.item, 'test_transcripts')
 
         subs = {
             'start': [100, 200, 240],
@@ -532,14 +534,14 @@ def remove_subs_from_store(self, subs_id):
 
     def test_success_download_nonyoutube(self):
         subs_id = str(uuid4())
-        data = textwrap.dedent("""
+        self.item.data = textwrap.dedent("""
             <video youtube="" sub="{}">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """.format(subs_id))
-        modulestore().update_item(self.item_location, data)
+        modulestore().update_item(self.item, 'test_transcripts')
 
         subs = {
             'start': [100, 200, 240],
@@ -582,8 +584,8 @@ def test_success_download_nonyoutube(self):
         transcripts_utils.remove_subs_from_store(subs_id, self.item)
 
     def test_check_youtube(self):
-        data = '<video youtube="1:JMD_ifUUfsU" />'
-        modulestore().update_item(self.item_location, data)
+        self.item.data = '<video youtube="1:JMD_ifUUfsU" />'
+        modulestore().update_item(self.item, 'test_transcripts')
 
         subs = {
             'start': [100, 200, 240],
@@ -674,14 +676,15 @@ def test_fail_for_non_video_module(self):
         resp = self.client.ajax_post('/xblock', data)
         item_locator, item_location = self._get_locator(resp)
         subs_id = str(uuid4())
-        data = textwrap.dedent("""
+        item = modulestore().get_item(item_location)
+        item.data = textwrap.dedent("""
             <not_video youtube="" sub="{}">
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.webm"/>
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </videoalpha>
         """.format(subs_id))
-        modulestore().update_item(item_location, data)
+        modulestore().update_item(item, 'test_transcript')
 
         subs = {
             'start': [100, 200, 240],
diff --git a/cms/djangoapps/contentstore/transcripts_utils.py b/cms/djangoapps/contentstore/transcripts_utils.py
index 79024bc4faef..03aa46fcd54d 100644
--- a/cms/djangoapps/contentstore/transcripts_utils.py
+++ b/cms/djangoapps/contentstore/transcripts_utils.py
@@ -286,7 +286,7 @@ def save_module(item):
     """
     item.save()
     store = get_modulestore(Location(item.id))
-    store.update_metadata(item.id, own_metadata(item))
+    store.update_item(item, 'save_module')
 
 
 def copy_or_rename_transcript(new_name, old_name, item, delete_old=False):
diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py
index fbe04186c43c..29874583c455 100644
--- a/cms/djangoapps/contentstore/utils.py
+++ b/cms/djangoapps/contentstore/utils.py
@@ -222,16 +222,6 @@ def compute_unit_state(unit):
         return UnitState.public
 
 
-def update_item(location, value):
-    """
-    If value is None, delete the db entry. Otherwise, update it using the correct modulestore.
-    """
-    if value is None:
-        get_modulestore(location).delete_item(location)
-    else:
-        get_modulestore(location).update_item(location, value)
-
-
 def add_extra_panel_tab(tab_type, course):
     """
     Used to add the panel tab to a course if it does not exist.
diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py
index 123ff08f9a0a..2068d3ace6b6 100644
--- a/cms/djangoapps/contentstore/views/checklist.py
+++ b/cms/djangoapps/contentstore/views/checklist.py
@@ -51,8 +51,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
         # from the template.
         if not course_module.checklists:
             course_module.checklists = CourseDescriptor.checklists.default
-            course_module.save()
-            modulestore.update_metadata(old_location, own_metadata(course_module))
+            modulestore.update_item(course_module, request.user.username)
 
         expanded_checklists = expand_all_action_urls(course_module)
         if json_request:
@@ -81,7 +80,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
             # not default
             course_module.checklists = course_module.checklists
             course_module.save()
-            modulestore.update_metadata(old_location, own_metadata(course_module))
+            modulestore.update_item(course_module, request.user.username)
             expanded_checklist = expand_checklist_action_url(course_module, persisted_checklist)
             return JsonResponse(expanded_checklist)
         else:
diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py
index e930e33c1a5e..f06ed195016c 100644
--- a/cms/djangoapps/contentstore/views/course.py
+++ b/cms/djangoapps/contentstore/views/course.py
@@ -163,8 +163,7 @@ def course_listing(request):
     """
     List all courses available to the logged in user
     """
-    # there's an index on category which will be used if none of its antecedents are set
-    courses = modulestore('direct').get_items(Location(None, None, None, 'course', None))
+    courses = modulestore('direct').get_courses()
 
     # filter out courses that we don't have access too
     def course_filter(course):
@@ -743,10 +742,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers
         if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs):
             course.tabs.append({"type": "pdf_textbooks"})
         course.pdf_textbooks = textbooks
-        store.update_metadata(
-            course.location,
-            own_metadata(course)
-        )
+        store.update_item(course, request.user.username)
         return JsonResponse(course.pdf_textbooks)
     elif request.method == 'POST':
         # create a new textbook for the course
@@ -764,7 +760,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers
             tabs = course.tabs
             tabs.append({"type": "pdf_textbooks"})
             course.tabs = tabs
-        store.update_metadata(course.location, own_metadata(course))
+        store.update_item(course, request.user.username)
         resp = JsonResponse(textbook, status=201)
         resp["Location"] = locator.url_reverse('textbooks', textbook["id"])
         return resp
@@ -815,10 +811,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
             course.pdf_textbooks = new_textbooks
         else:
             course.pdf_textbooks.append(new_textbook)
-        store.update_metadata(
-            course.location,
-            own_metadata(course)
-        )
+        store.update_item(course, request.user.username)
         return JsonResponse(new_textbook, status=201)
     elif request.method == 'DELETE':
         if not textbook:
@@ -827,10 +820,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
         new_textbooks = course.pdf_textbooks[0:i]
         new_textbooks.extend(course.pdf_textbooks[i + 1:])
         course.pdf_textbooks = new_textbooks
-        store.update_metadata(
-            course.location,
-            own_metadata(course)
-        )
+        store.update_item(course, request.user.username)
         return JsonResponse()
 
 
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index 8d80d9e0bba5..de8177974988 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/cms/djangoapps/contentstore/views/item.py
@@ -232,7 +232,8 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
             modulestore().convert_to_draft(item_location)
 
     if data:
-        store.update_item(item_location, data)
+        # TODO Allow any scope.content fields not just "data" (exactly like the get below this)
+        existing_item.data = data
     else:
         data = existing_item.get_explicitly_set_fields_by_scope(Scope.content)
 
@@ -242,9 +243,9 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
             for child_locator
             in children
         ]
-        store.update_children(item_location, children_ids)
+        existing_item.children = children_ids
 
-    # cdodge: also commit any metadata which might have been passed along
+    # also commit any metadata which might have been passed along
     if nullout is not None or metadata is not None:
         # 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 use the original (existing_item) and
@@ -269,15 +270,12 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
                         return JsonResponse({"error": "Invalid data"}, 400)
                     field.write_to(existing_item, 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))
-
         if existing_item.category == 'video':
             manage_video_subtitles_save(existing_item, existing_item)
 
+    # commit to datastore
+    store.update_item(existing_item, request.user)
+
     result = {
         'id': unicode(usage_loc),
         'data': data,
@@ -339,7 +337,8 @@ def _create_item(request):
 
     # TODO replace w/ nicer accessor
     if not 'detached' in parent.runtime.load_block_type(category)._class_tags:
-        get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()])
+        parent.children.append(dest_location.url())
+        get_modulestore(parent.location).update_item(parent, request.user)
 
     course_location = loc_mapper().translate_locator_to_location(parent_locator, get_course=True)
     locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True)
@@ -373,13 +372,14 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
         system=source_item.runtime,
     )
 
+    dest_module = get_modulestore(category).get_item(dest_location)
     # Children are not automatically copied over (and not all xblocks have a 'children' attribute).
     # Because DAGs are not fully supported, we need to actually duplicate each child as well.
     if source_item.has_children:
-        copied_children = []
+        dest_module.children = []
         for child in source_item.children:
-            copied_children.append(_duplicate_item(dest_location, Location(child)).url())
-        get_modulestore(dest_location).update_children(dest_location, copied_children)
+            dest_module.children.append(_duplicate_item(dest_location, Location(child)).url())
+        get_modulestore(dest_location).update_item(dest_module, 'user')
 
     if not 'detached' in source_item.runtime.load_block_type(category)._class_tags:
         parent = get_modulestore(parent_location).get_item(parent_location)
@@ -390,7 +390,7 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
             parent.children.insert(source_index + 1, dest_location.url())
         else:
             parent.children.append(dest_location.url())
-        get_modulestore(parent_location).update_children(parent_location, parent.children)
+        get_modulestore(parent_location).update_item(parent, 'user')
 
     return dest_location
 
@@ -406,22 +406,19 @@ def _delete_item_at_location(item_location, delete_children=False, delete_all_ve
     item = store.get_item(item_location)
 
     if delete_children:
-        _xmodule_recurse(item, lambda i: store.delete_item(i.location, delete_all_versions))
+        _xmodule_recurse(item, lambda i: store.delete_item(i.location, delete_all_versions=delete_all_versions))
     else:
-        store.delete_item(item.location, delete_all_versions)
+        store.delete_item(item.location, delete_all_versions=delete_all_versions)
 
     # cdodge: we need to remove our parent's pointer to us so that it is no longer dangling
     if delete_all_versions:
         parent_locs = modulestore('direct').get_parent_locations(item_location, None)
 
+        item_url = item_location.url()
         for parent_loc in parent_locs:
             parent = modulestore('direct').get_item(parent_loc)
-            item_url = item_location.url()
-            if item_url in parent.children:
-                children = parent.children
-                children.remove(item_url)
-                parent.children = children
-                modulestore('direct').update_children(parent.location, parent.children)
+            parent.children.remove(item_url)
+            modulestore('direct').update_item(parent, 'user')
 
     return JsonResponse()
 
@@ -452,7 +449,7 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid
         if request.user.is_staff:
             items = modulestore().get_orphans(old_location, 'draft')
             for item in items:
-                modulestore('draft').delete_item(item, True)
+                modulestore('draft').delete_item(item, delete_all_versions=True)
             return JsonResponse({'deleted': items})
         else:
             raise PermissionDenied()
diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py
index 4a34886ebc9f..f0c99ededb23 100644
--- a/cms/djangoapps/contentstore/views/tabs.py
+++ b/cms/djangoapps/contentstore/views/tabs.py
@@ -47,7 +47,7 @@ def initialize_course_tabs(course):
         {"type": "progress", "name": _("Progress")},
     ]
 
-    modulestore('direct').update_metadata(course.location.url(), own_metadata(course))
+    modulestore('direct').update_item(course, 'system')
 
 @expect_json
 @login_required
@@ -123,7 +123,7 @@ def get_location_for_tab(tab):
 
                 # OK, re-assemble the static tabs in the new order
                 course_item.tabs = reordered_tabs
-                modulestore('direct').update_metadata(course_item.location, own_metadata(course_item))
+                modulestore('direct').update_item(course_item, request.user.username)
                 return JsonResponse()
             else:
                 raise NotImplementedError('Creating or changing tab content is not supported.')
@@ -179,7 +179,7 @@ def primitive_delete(course, num):
     # Note for future implementations: if you delete a static_tab, then Chris Dodge
     # points out that there's other stuff to delete beyond this element.
     # This code happens to not delete static_tab so it doesn't come up.
-    modulestore('direct').update_metadata(course.location, own_metadata(course))
+    modulestore('direct').update_item(course, 'primitive delete')
 
 
 def primitive_insert(course, num, tab_type, name):
@@ -188,5 +188,5 @@ def primitive_insert(course, num, tab_type, name):
     new_tab = {u'type': unicode(tab_type), u'name': unicode(name)}
     tabs = course.tabs
     tabs.insert(num, new_tab)
-    modulestore('direct').update_metadata(course.location, own_metadata(course))
+    modulestore('direct').update_item(course, 'primitive insert')
 
diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py
index dd8582ba7648..dc842780ce80 100644
--- a/cms/djangoapps/models/settings/course_details.py
+++ b/cms/djangoapps/models/settings/course_details.py
@@ -6,10 +6,8 @@
 
 from xmodule.modulestore import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError
-from xmodule.modulestore.inheritance import own_metadata
 from contentstore.utils import get_modulestore, course_image_url
 from models.settings import course_grading
-from contentstore.utils import update_item
 from xmodule.fields import Date
 from xmodule.modulestore.django import loc_mapper
 
@@ -74,6 +72,24 @@ def fetch(cls, course_locator):
 
         return course
 
+    @classmethod
+    def update_about_item(cls, course_old_location, about_key, data, course):
+        """
+        Update the about item with the new data blob. If data is None, then
+        delete the about item.
+        """
+        temploc = Location(course_old_location).replace(category='about', name=about_key)
+        store = get_modulestore(temploc)
+        if data is None:
+            store.delete_item(temploc)
+        else:
+            try:
+                about_item = store.get_item(temploc)
+            except ItemNotFoundError:
+                about_item = store.create_xmodule(temploc, system=course.runtime)
+            about_item.data = data
+            store.update_item(about_item, 'update_about_item')
+
     @classmethod
     def update_from_json(cls, course_locator, jsondict):
         """
@@ -130,26 +146,15 @@ def update_from_json(cls, course_locator, jsondict):
             dirty = True
 
         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_old_location).update_metadata(course_old_location, own_metadata(descriptor))
+            get_modulestore(course_old_location).update_item(descriptor, 'course_details')
 
         # NOTE: below auto writes to the db w/o verifying that any of the fields actually changed
         # to make faster, could compare against db or could have client send over a list of which fields changed.
-        temploc = Location(course_old_location).replace(category='about', name='syllabus')
-        update_item(temploc, jsondict['syllabus'])
+        for about_type in ['syllabus', 'overview', 'effort']:
+            cls.update_about_item(course_old_location, about_type, jsondict[about_type], descriptor)
 
-        temploc = temploc.replace(name='overview')
-        update_item(temploc, jsondict['overview'])
-
-        temploc = temploc.replace(name='effort')
-        update_item(temploc, jsondict['effort'])
-
-        temploc = temploc.replace(name='video')
         recomposed_video_tag = CourseDetails.recompose_video_tag(jsondict['intro_video'])
-        update_item(temploc, recomposed_video_tag)
+        cls.update_about_item(course_old_location, 'video', recomposed_video_tag, descriptor)
 
         # Could just return jsondict w/o doing any db reads, but I put the reads in as a means to confirm
         # it persisted correctly
diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py
index 519323298bf4..5123b52f8ab5 100644
--- a/cms/djangoapps/models/settings/course_grading.py
+++ b/cms/djangoapps/models/settings/course_grading.py
@@ -65,9 +65,7 @@ def update_from_json(course_locator, jsondict):
         descriptor.raw_grader = graders_parsed
         descriptor.grade_cutoffs = jsondict['grade_cutoffs']
 
-        get_modulestore(course_old_location).update_item(
-            course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
-        )
+        get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
 
         CourseGradingModel.update_grace_period_from_json(course_locator, jsondict['grace_period'])
 
@@ -91,9 +89,7 @@ def update_grader_from_json(course_location, grader):
         else:
             descriptor.raw_grader.append(grader)
 
-        get_modulestore(course_old_location).update_item(
-            course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
-        )
+        get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
 
         return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index])
 
@@ -107,9 +103,7 @@ def update_cutoffs_from_json(course_location, cutoffs):
         descriptor = get_modulestore(course_old_location).get_item(course_old_location)
         descriptor.grade_cutoffs = cutoffs
 
-        get_modulestore(course_old_location).update_item(
-            course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
-        )
+        get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
 
         return cutoffs
 
@@ -132,9 +126,7 @@ def update_grace_period_from_json(course_location, graceperiodjson):
             grace_timedelta = timedelta(**graceperiodjson)
             descriptor.graceperiod = grace_timedelta
 
-            get_modulestore(course_old_location).update_metadata(
-                course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings)
-            )
+            get_modulestore(course_old_location).update_item(descriptor, 'update_grace_period')
 
     @staticmethod
     def delete_grader(course_location, index):
@@ -150,9 +142,7 @@ def delete_grader(course_location, index):
             # force propagation to definition
             descriptor.raw_grader = descriptor.raw_grader
 
-        get_modulestore(course_old_location).update_item(
-            course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content)
-        )
+        get_modulestore(course_old_location).update_item(descriptor, 'delete_grader')
 
     @staticmethod
     def delete_grace_period(course_location):
@@ -164,9 +154,7 @@ def delete_grace_period(course_location):
 
         del descriptor.graceperiod
 
-        get_modulestore(course_old_location).update_metadata(
-            course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings)
-        )
+        get_modulestore(course_old_location).update_item(descriptor, 'delete_grace_period')
 
     @staticmethod
     def get_section_grader_type(location):
@@ -186,9 +174,7 @@ def update_section_grader_type(descriptor, grader_type):
             del descriptor.format
             del descriptor.graded
 
-        get_modulestore(descriptor.location).update_metadata(
-            descriptor.location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings)
-        )
+        get_modulestore(descriptor.location).update_item(descriptor, 'update_grader')
         return {'graderType': grader_type}
 
     @staticmethod
diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py
index 3c561b6047ff..88aa3c04e931 100644
--- a/cms/djangoapps/models/settings/course_metadata.py
+++ b/cms/djangoapps/models/settings/course_metadata.py
@@ -1,7 +1,6 @@
 from xblock.fields import Scope
 
 from contentstore.utils import get_modulestore
-from xmodule.modulestore.inheritance import own_metadata
 from cms.lib.xblock.mixin import CmsBlockMixin
 
 
@@ -78,6 +77,6 @@ def update_from_json(cls, descriptor, jsondict, filter_tabs=True):
                 setattr(descriptor, key, value)
 
         if dirty:
-            get_modulestore(descriptor.location).update_metadata(descriptor.location, own_metadata(descriptor))
+            get_modulestore(descriptor.location).update_item(descriptor, 'update_settings')
 
         return cls.fetch(descriptor)
diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py
index 0ef912464b6e..6397685c115a 100644
--- a/common/djangoapps/external_auth/tests/test_shib.py
+++ b/common/djangoapps/external_auth/tests/test_shib.py
@@ -330,9 +330,7 @@ def test_course_specific_login_and_reg(self):
         for domain in ["", "shib:https://idp.stanford.edu/"]:
             # set domains
             course.enrollment_domain = domain
-            metadata = own_metadata(course)
-            metadata['enrollment_domain'] = domain
-            self.store.update_metadata(course.location.url(), metadata)
+            self.store.update_item(course, 'test_shib')
 
             # setting location to test that GET params get passed through
             login_request = self.request_factory.get('/course_specific_login/MITx/999/Robot_Super_Course' +
@@ -401,15 +399,11 @@ def test_enrollment_limit_by_domain(self):
         # create 2 course, one with limited enrollment one without
         shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
         shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        metadata = own_metadata(shib_course)
-        metadata['enrollment_domain'] = shib_course.enrollment_domain
-        self.store.update_metadata(shib_course.location.url(), metadata)
+        self.store.update_item(shib_course, 'test_enroll_limit')
 
         open_enroll_course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course')
         open_enroll_course.enrollment_domain = ''
-        metadata = own_metadata(open_enroll_course)
-        metadata['enrollment_domain'] = open_enroll_course.enrollment_domain
-        self.store.update_metadata(open_enroll_course.location.url(), metadata)
+        self.store.update_item(open_enroll_course, 'test_enroll_limit')
 
         # create 3 kinds of students, external_auth matching shib_course, external_auth not matching, no external auth
         shib_student = UserFactory.create()
@@ -475,9 +469,7 @@ def test_shib_login_enrollment(self):
 
         course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
         course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        metadata = own_metadata(course)
-        metadata['enrollment_domain'] = course.enrollment_domain
-        self.store.update_metadata(course.location.url(), metadata)
+        self.store.update_item(course, 'test_shib')
 
         # use django test client for sessions and url processing
         # no enrollment before trying
diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py
index 9be1e7de04b4..b50d80d1ad9c 100644
--- a/common/djangoapps/student/tests/test_login.py
+++ b/common/djangoapps/student/tests/test_login.py
@@ -202,9 +202,7 @@ def setUp(self):
         self.course = CourseFactory.create(org='Stanford', number='456', display_name='NO SHIB')
         self.shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
         self.shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        metadata = own_metadata(self.shib_course)
-        metadata['enrollment_domain'] = self.shib_course.enrollment_domain
-        self.store.update_metadata(self.shib_course.location.url(), metadata)
+        self.store.update_item(self.shib_course, 'test_shib')
         self.user_w_map = UserFactory.create(email='withmap@stanford.edu')
         self.extauth = ExternalAuthMap(external_id='withmap@stanford.edu',
                                        external_email='withmap@stanford.edu',
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 5d241731a7e4..f764ce6d9693 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -339,7 +339,7 @@ def get_item_errors(self, location):
         pass
 
     @abstractmethod
-    def get_items(self, location, course_id=None, depth=0):
+    def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         """
         Returns a list of XModuleDescriptor instances for the items
         that match location. Any element of location that is None is treated
@@ -378,6 +378,15 @@ def get_parent_locations(self, location, course_id):
         '''
         pass
 
+    @abstractmethod
+    def get_orphans(self, course_location, branch):
+        """
+        Get all of the xblocks in the given course which have no parents and are not of types which are
+        usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
+        use children to point to their dependents.
+        """
+        pass
+
     @abstractmethod
     def get_errored_courses(self):
         """
@@ -404,44 +413,16 @@ class ModuleStoreWrite(ModuleStoreRead):
     __metaclass__ = ABCMeta
 
     @abstractmethod
-    def update_item(self, location, data, allow_not_found=False):
+    def update_item(self, xblock, user, allow_not_found=False):
         """
-        Set the data in the item specified by the location to
-        data
-
-        location: Something that can be passed to Location
-        data: A nested dictionary of problem data
+        Update the given xblock's persisted repr
         """
         pass
 
     @abstractmethod
-    def update_children(self, location, children):
-        """
-        Set the children for the item specified by the location to
-        children
-
-        location: Something that can be passed to Location
-        children: A list of child item identifiers
+    def delete_item(self, location, user_id=None, delete_all_versions=False, delete_children=False, force=False):
         """
-        pass
-
-    @abstractmethod
-    def update_metadata(self, location, metadata):
-        """
-        Set the metadata for the item specified by the location to
-        metadata
-
-        location: Something that can be passed to Location
-        metadata: A nested dictionary of module metadata
-        """
-        pass
-
-    @abstractmethod
-    def delete_item(self, location):
-        """
-        Delete an item from this modulestore
-
-        location: Something that can be passed to Location
+        Delete an item from persistence
         """
         pass
 
diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
index 37853d07896a..9ada7fe9705a 100644
--- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
+++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
@@ -163,8 +163,14 @@ def translate_location(self, old_style_course_id, location, published=True, add_
             else:
                 raise ItemNotFoundError(location)
         elif isinstance(block_id, dict):
-            # name is not unique, look through for the right category
-            if location.category in block_id:
+            # jump_to_id uses a None category.
+            if location.category is None:
+                if len(block_id) == 1:
+                    # unique match (most common case)
+                    block_id = block_id.values()[0]
+                else:
+                    raise InvalidLocationError()
+            elif location.category in block_id:
                 block_id = block_id[location.category]
             elif add_entry_if_missing:
                 block_id = self._add_to_block_map(location, location_id, entry['block_map'])
diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py
index 2b58797532bf..01c7d4155c09 100644
--- a/common/lib/xmodule/xmodule/modulestore/locator.py
+++ b/common/lib/xmodule/xmodule/modulestore/locator.py
@@ -488,6 +488,19 @@ def init_block_ref_from_package_id(self, package_id):
             raise ValueError('Could not parse "%s" as a package_id' % package_id)
         self._set_value(parse, 'block', self.set_block_id)
 
+    @classmethod
+    def make_relative(cls, course_locator, block_id):
+        """
+        Return a new instance which has the given block_id in the given course
+        :param course_locator: may be a BlockUsageLocator in the same snapshot
+        """
+        return BlockUsageLocator(
+            package_id=course_locator.package_id,
+            version_guid=course_locator.version_guid,
+            branch=course_locator.branch,
+            block_id=block_id
+        )
+
     def __unicode__(self):
         """
         Return a string representing this location.
diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py
index 46da67cb6550..009c8c95f7a4 100644
--- a/common/lib/xmodule/xmodule/modulestore/mixed.py
+++ b/common/lib/xmodule/xmodule/modulestore/mixed.py
@@ -3,29 +3,42 @@
 
 In this way, courses can be served up both - say - XMLModuleStore or MongoModuleStore
 
-IMPORTANT: This modulestore only supports READONLY applications, e.g. LMS
 """
 
 from . import ModuleStoreWriteBase
-from xmodule.modulestore.django import create_modulestore_instance
+from xmodule.modulestore.django import create_modulestore_instance, loc_mapper
 import logging
+from xmodule.modulestore import Location
+from xblock.fields import Reference, ReferenceList, String
+from xmodule.modulestore.locator import CourseLocator, Locator, BlockUsageLocator
+from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemNotFoundError
+from xmodule.modulestore.parsers import ALLOWED_ID_CHARS
+import re
 
 log = logging.getLogger(__name__)
 
 
 class MixedModuleStore(ModuleStoreWriteBase):
     """
-    ModuleStore that can be backed by either XML or Mongo
+    ModuleStore knows how to route requests to the right persistence ms and how to convert any
+    references in the xblocks to the type required by the app and the persistence layer.
     """
-    def __init__(self, mappings, stores, **kwargs):
+    def __init__(self, mappings, stores, reference_type=None, **kwargs):
         """
         Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a
         collection of other modulestore configuration informations
+        :param reference_type: either Location or Locator to indicate what type of reference this app
+        uses.
         """
         super(MixedModuleStore, self).__init__(**kwargs)
 
         self.modulestores = {}
         self.mappings = mappings
+        # temporary code for transition period
+        if reference_type is None:
+            log.warn("reference_type not specified in MixedModuleStore settings.",
+                "Will default temporarily to the to-be-deprecated Location.")
+        self.use_locations = (reference_type != 'Locator')
         if 'default' not in stores:
             raise Exception('Missing a default modulestore in the MixedModuleStore __init__ method.')
 
@@ -45,60 +58,201 @@ def _get_modulestore_for_courseid(self, course_id):
         mapping = self.mappings.get(course_id, 'default')
         return self.modulestores[mapping]
 
-    def has_item(self, course_id, location):
-        return self._get_modulestore_for_courseid(course_id).has_item(course_id, location)
-
-    def get_item(self, location, depth=0):
+    def _incoming_reference_adaptor(self, store, course_id, reference):
         """
-        This method is explicitly not implemented as we need a course_id to disambiguate
-        We should be able to fix this when the data-model rearchitecting is done
+        Convert the reference to the type the persistence layer wants
         """
-        raise NotImplementedError
-
-    def get_instance(self, course_id, location, depth=0):
-        return self._get_modulestore_for_courseid(course_id).get_instance(course_id, location, depth)
+        if issubclass(store.reference_type, Location if self.use_locations else Locator):
+            return reference
+        stringify = isinstance(reference, basestring)
+        if store.reference_type == Location:
+            if stringify:
+                reference = BlockUsageLocator(url=reference)
+            location = loc_mapper().translate_locator_to_location(reference)
+            return location.url() if stringify else location
+        if stringify:
+            reference = Location(reference)
+        locator = loc_mapper().translate_location(course_id, reference, reference.revision == 'draft', True)
+        return unicode(locator) if stringify else locator
 
-    def get_items(self, location, course_id=None, depth=0):
+    def _outgoing_reference_adaptor(self, store, course_id, reference):
         """
-        Returns a list of XModuleDescriptor instances for the items
-        that match location. Any element of location that is None is treated
-        as a wildcard that matches any value
+        Convert the reference to the type the application wants
+        """
+        if issubclass(store.reference_type, Location if self.use_locations else Locator):
+            return reference
+        stringify = isinstance(reference, basestring)
+        if store.reference_type == Location:
+            if stringify:
+                reference = Location(reference)
+            locator = loc_mapper().translate_location(
+                course_id, reference, reference.revision == 'draft', True
+            )
+            return unicode(locator) if stringify else locator
+        else:
+            if stringify:
+                reference = BlockUsageLocator(url=reference)
+            location = loc_mapper().translate_locator_to_location(reference)
+            return location.url() if stringify else location
 
-        location: Something that can be passed to Location
+    def _incoming_xblock_adaptor(self, store, course_id, xblock):
+        """
+        Change all reference fields in this xblock to the type expected by the persistence layer
+        """
+        string_converter = self._get_string_converter(
+            course_id, store.reference_type, xblock.location
+        )
+        for field in xblock.fields.itervalues():
+            if field.is_set_on(xblock):
+                if isinstance(field, Reference):
+                    field.write_to(
+                        xblock,
+                        self._incoming_reference_adaptor(store, course_id, field.read_from(xblock)))
+                elif isinstance(field, ReferenceList):
+                    field.write_to(
+                        xblock,
+                        [self._incoming_reference_adaptor(store, course_id, ref)
+                         for ref in field.read_from(xblock)]
+                    )
+                elif string_converter is not None and isinstance(field, String):
+                    # replace links within the string
+                    string_converter(field, xblock)
+        return xblock
 
-        depth: An argument that some module stores may use to prefetch
-            descendents of the queried modules for more efficient results later
-            in the request. The depth is counted in the number of calls to
-            get_children() to cache. None indicates to cache all descendents
+    def _outgoing_xblock_adaptor(self, store, course_id, xblock):
+        """
+        Change all reference fields in this xblock to the type expected by the persistence layer
         """
-        if not course_id:
-            raise Exception("Must pass in a course_id when calling get_items() with MixedModuleStore")
+        string_converter = self._get_string_converter(
+            course_id, xblock.location.__class__, xblock.location
+        )
+        for field in xblock.fields.itervalues():
+            if field.is_set_on(xblock):
+                if isinstance(field, Reference):
+                    field.write_to(
+                        xblock,
+                        self._outgoing_reference_adaptor(store, course_id, field.read_from(xblock)))
+                elif isinstance(field, ReferenceList):
+                    field.write_to(
+                        xblock,
+                        [self._outgoing_reference_adaptor(store, course_id, ref)
+                         for ref in field.read_from(xblock)]
+                    )
+                elif string_converter is not None and isinstance(field, String):
+                    # replace links within the string
+                    string_converter(field, xblock)
+        return xblock
 
-        return self._get_modulestore_for_courseid(course_id).get_items(location, course_id, depth)
+    CONVERT_RE = re.compile(r"/jump_to_id/({}+)".format(ALLOWED_ID_CHARS))
 
-    def update_item(self, location, data, allow_not_found=False):
+    def _get_string_converter(self, course_id, reference_type, from_base_addr):
         """
-        MixedModuleStore is for read-only (aka LMS)
+        Return a closure which finds and replaces all embedded links in a string field
+        with the correct rewritten link for the target type
         """
-        raise NotImplementedError
+        if self.use_locations and reference_type == Location:
+            return None
+        if not self.use_locations and issubclass(reference_type, Locator):
+            return None
+        if isinstance(from_base_addr, Location):
+            def mapper(found_id):
+                """
+                Convert the found id to BlockUsageLocator block_id
+                """
+                location = from_base_addr.replace(category=None, name=found_id)
+                # NOTE without category, it cannot create a new mapping if there's not one already
+                return loc_mapper().translate_location(course_id, location).block_id
+        else:
+            def mapper(found_id):
+                """
+                Convert the found id to Location block_id
+                """
+                locator = BlockUsageLocator.make_relative(from_base_addr, found_id)
+                return loc_mapper().translate_locator_to_location(locator).name
 
-    def update_children(self, location, children):
+        def converter(field, xblock):
+            """
+            Find all of the ids in the block and replace them w/ their mapped values
+            """
+            value = field.read_from(xblock)
+            self.CONVERT_RE.sub(mapper, value)
+            field.write_to(xblock, value)
+        return converter
+
+    def has_item(self, course_id, reference):
         """
-        MixedModuleStore is for read-only (aka LMS)
+        Does the course include the xblock who's id is reference?
+        :param course_id: a course_id or package_id (slashed or dotted)
+        :param reference: a Location or BlockUsageLocator
         """
-        raise NotImplementedError
+        store = self._get_modulestore_for_courseid(course_id)
+        decoded_ref = self._incoming_reference_adaptor(store, course_id, reference)
+        return store.has_item(course_id, decoded_ref)
 
-    def update_metadata(self, location, metadata):
+    def get_item(self, location, depth=0):
         """
-        MixedModuleStore is for read-only (aka LMS)
+        This method is explicitly not implemented as we need a course_id to disambiguate
+        We should be able to fix this when the data-model rearchitecting is done
         """
         raise NotImplementedError
 
-    def delete_item(self, location):
+    def get_instance(self, course_id, location, depth=0):
+        store = self._get_modulestore_for_courseid(course_id)
+        decoded_ref = self._incoming_reference_adaptor(store, course_id, location)
+        xblock = store.get_instance(course_id, decoded_ref, depth)
+        return self._outgoing_xblock_adaptor(store, course_id, xblock)
+
+    def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         """
-        MixedModuleStore is for read-only (aka LMS)
+        Returns a list of XModuleDescriptor instances for the items
+        that match location. Any element of location that is None is treated
+        as a wildcard that matches any value. NOTE: don't use this to look for courses
+        as the course_id is required. Use get_courses.
+
+        location: either a Location possibly w/ None as wildcards for category or name or
+        a Locator with at least a package_id and branch but possibly no block_id.
+
+        depth: An argument that some module stores may use to prefetch
+            descendents of the queried modules for more efficient results later
+            in the request. The depth is counted in the number of calls to
+            get_children() to cache. None indicates to cache all descendents
         """
-        raise NotImplementedError
+        if not (course_id or hasattr(location, 'package_id')):
+            raise Exception("Must pass in a course_id when calling get_items()")
+
+        store = self._get_modulestore_for_courseid(course_id or getattr(location, 'package_id'))
+        # translate won't work w/ missing fields so work around it
+        if store.reference_type == Location:
+            if not self.use_locations:
+                if getattr(location, 'block_id', False):
+                    location = self._incoming_reference_adaptor(store, course_id, location)
+                else:
+                    # get the course's location
+                    location = loc_mapper().translate_locator_to_location(location, get_course=True)
+                    # now remove the unknowns
+                    location = location.replace(
+                        category=qualifiers.get('category', None),
+                        name=None
+                    )
+        else:
+            if self.use_locations:
+                if not isinstance(location, Location):
+                    location = Location(location)
+                try:
+                    location.ensure_fully_specified()
+                    location = loc_mapper().translate_location(
+                        course_id, location, location.revision == 'published', True
+                    )
+                except InsufficientSpecificationError:
+                    # construct the Locator by hand
+                    if location.category is not None and qualifiers.get('category', False):
+                        qualifiers['category'] = location.category
+                    location = loc_mapper().translate_location_to_course_locator(
+                        course_id, location, location.revision == 'published'
+                    )
+        xblocks = store.get_items(location, course_id, depth, qualifiers)
+        xblocks = [self._outgoing_xblock_adaptor(store, course_id, xblock) for xblock in xblocks]
+        return xblocks
 
     def get_courses(self):
         '''
@@ -126,15 +280,49 @@ def get_courses(self):
 
     def get_course(self, course_id):
         """
-        returns the course module associated with the course_id
+        returns the course module associated with the course_id. If no such course exists,
+        it returns None
+        :param course_id: must be either a string course_id or a CourseLocator
         """
-        return self._get_modulestore_for_courseid(course_id).get_course(course_id)
+        store = self._get_modulestore_for_courseid(
+            course_id.package_id if hasattr(course_id, 'package_id') else course_id)
+        try:
+            # translate won't work w/ missing fields so work around it
+            if store.reference_type == Location:
+                # takes the course_id: figure out if this is old or new style
+                if not self.use_locations:
+                    if isinstance(course_id, basestring):
+                        course_id = CourseLocator(package_id=course_id, branch='published')
+                    course_location = loc_mapper().translate_locator_to_location(course_id, get_course=True)
+                    course_id = course_location.course_id
+                xblock = store.get_course(course_id)
+            else:
+                # takes a courseLocator
+                if isinstance(course_id, CourseLocator):
+                    location = course_id
+                    course_id = None  # not an old style course_id; so, don't use it further
+                elif '/' in course_id:
+                    location = loc_mapper().translate_location_to_course_locator(course_id, None, True)
+                else:
+                    location = CourseLocator(package_id=course_id, branch='published')
+                    course_id = None  # not an old style course_id; so, don't use it further
+                xblock = store.get_course(location)
+        except ItemNotFoundError:
+            return None
+        if xblock is not None:
+            return self._outgoing_xblock_adaptor(store, course_id, xblock)
+        else:
+            return None
 
     def get_parent_locations(self, location, course_id):
         """
-        returns the parent locations for a given lcoation and course_id
+        returns the parent locations for a given location and course_id
         """
-        return self._get_modulestore_for_courseid(course_id).get_parent_locations(location, course_id)
+        store = self._get_modulestore_for_courseid(course_id)
+        decoded_ref = self._incoming_reference_adaptor(store, course_id, location)
+        parents = store.get_parent_locations(decoded_ref, course_id)
+        return [self._outgoing_reference_adaptor(store, course_id, reference)
+                for reference in parents]
 
     def get_modulestore_type(self, course_id):
         """
@@ -146,6 +334,17 @@ def get_modulestore_type(self, course_id):
         """
         return self._get_modulestore_for_courseid(course_id).get_modulestore_type(course_id)
 
+    def get_orphans(self, course_location, branch):
+        """
+        Get all of the xblocks in the given course which have no parents and are not of types which are
+        usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
+        use children to point to their dependents.
+        """
+        course_id = getattr(course_location, 'course_id', getattr(course_location, 'package_id', None))
+        store = self._get_modulestore_for_courseid(course_id)
+        decoded_ref = self._incoming_reference_adaptor(store, course_id, course_location)
+        return store.get_orphans(decoded_ref, branch)
+
     def get_errored_courses(self):
         """
         Return a dictionary of course_dir -> [(msg, exception_str)], for each
@@ -155,3 +354,32 @@ def get_errored_courses(self):
         for store in self.modulestores.values():
             errs.update(store.get_errored_courses())
         return errs
+
+    def update_item(self, xblock, user_id, allow_not_found=False):
+        """
+        Update the xblock persisted to be the same as the given for all types of fields
+        (content, children, and metadata) attribute the change to the given user.
+        """
+        if self.use_locations:
+            raise NotImplementedError
+
+        locator = xblock.location
+        course_id = locator.package_id
+        store = self._get_modulestore_for_courseid(course_id)
+
+        # if an xblock, convert its contents to correct addr scheme
+        xblock = self._incoming_xblock_adaptor(store, course_id, xblock)
+        xblock = store.update_item(xblock, user_id)
+
+        return self._outgoing_xblock_adaptor(store, course_id, xblock)
+
+    def delete_item(self, location, **kwargs):
+        """
+        Delete the given item from persistence.
+        """
+        if self.use_locations:
+            raise NotImplementedError
+
+        store = self._get_modulestore_for_courseid(location.package_id)
+        decoded_ref = self._incoming_reference_adaptor(store, location.package_id, location)
+        return store.delete_item(decoded_ref, **kwargs)
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 9870a1d3d811..7c77956aed30 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -38,10 +38,6 @@
 
 log = logging.getLogger(__name__)
 
-# TODO (cpennington): This code currently operates under the assumption that
-# there is only one revision for each item. Once we start versioning inside the CMS,
-# that assumption will have to change
-
 
 def get_course_id_no_run(location):
     '''
@@ -224,8 +220,10 @@ def namedtuple_to_son(namedtuple, prefix=''):
     Converts a namedtuple into a SON object with the same key order
     """
     son = SON()
+    # pylint: disable=protected-access
     for idx, field_name in enumerate(namedtuple._fields):
         son[prefix + field_name] = namedtuple[idx]
+    # pylint: enable=protected-access
     return son
 
 
@@ -299,9 +297,11 @@ def do_connection(
 
         # Force mongo to maintain an index over _id.* that is in the same order
         # that is used when querying by a location
+        # pylint: disable=no-member, protected_access
         self.collection.ensure_index(
             zip(('_id.' + field for field in Location._fields), repeat(1)),
         )
+        # pylint: enable=no-member, protected_access
 
         if default_class is not None:
             module_path, _, class_name = default_class.rpartition('.')
@@ -313,6 +313,7 @@ def do_connection(
         self.error_tracker = error_tracker
         self.render_template = render_template
         self.ignore_write_events_on_courses = []
+        self.reference_type = Location
 
     def compute_metadata_inheritance_tree(self, location):
         '''
@@ -363,11 +364,6 @@ def _compute_inherited_metadata(url):
             """
             Helper method for computing inherited metadata for a specific location url
             """
-            # check for presence of metadata key. Note that a given module may not yet be fully formed.
-            # example: update_item -> update_children -> update_metadata sequence on new item create
-            # if we get called here without update_metadata called first then 'metadata' hasn't been set
-            # as we're not fully transactional at the DB layer. Same comment applies to below key name
-            # check
             my_metadata = results_by_url[url].get('metadata', {})
 
             # go through all the children and recurse, but only if we have
@@ -443,6 +439,9 @@ def _clean_item_data(self, item):
         del item['_id']
 
     def _query_children_for_cache_children(self, items):
+        """
+        Generate a pymongo in query for finding the items and return the payloads
+        """
         # first get non-draft in a round-trip
         query = {
             '_id': {'$in': [namedtuple_to_son(Location(item)) for item in items]}
@@ -531,8 +530,7 @@ def get_courses(self):
         '''
         Returns a list of course descriptors.
         '''
-        # TODO (vshnayder): Why do I have to specify i4x here?
-        course_filter = Location("i4x", category="course")
+        course_filter = Location(category="course")
         return [
             course
             for course
@@ -556,6 +554,16 @@ def _find_one(self, location):
             raise ItemNotFoundError(location)
         return item
 
+    def get_course(self, course_id):
+        """
+        Get the course with the given courseid (org/course/run)
+        """
+        id_components = course_id.split('/')
+        try:
+            return self.get_item(Location('i4x', id_components[0], id_components[1], 'course', id_components[2]))
+        except ItemNotFoundError:
+            return None
+
     def has_item(self, course_id, location):
         """
         Returns True if location exists in this ModuleStore.
@@ -599,7 +607,7 @@ def get_instance(self, course_id, location, depth=0):
         """
         return self.get_item(location, depth=depth)
 
-    def get_items(self, location, course_id=None, depth=0):
+    def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         items = self.collection.find(
             location_to_query(location),
             sort=[('revision', pymongo.ASCENDING)],
@@ -664,13 +672,13 @@ def save_xmodule(self, xmodule):
         # Save any changes to the xmodule to the MongoKeyValueStore
         xmodule.save()
         self.collection.save({
-                '_id': xmodule.location.dict(),
-                'metadata': own_metadata(xmodule),
-                'definition': {
-                    'data': xmodule.get_explicitly_set_fields_by_scope(Scope.content),
-                    'children': xmodule.children if xmodule.has_children else []
-                }
-            })
+            '_id': namedtuple_to_son(xmodule.location),
+            'metadata': own_metadata(xmodule),
+            'definition': {
+                'data': xmodule.get_explicitly_set_fields_by_scope(Scope.content),
+                'children': xmodule.children if xmodule.has_children else []
+            }
+        })
         # recompute (and update) the metadata inheritance tree which is cached
         self.refresh_cached_metadata_inheritance_tree(xmodule.location)
         self.fire_updated_modulestore_signal(get_course_id_no_run(xmodule.location), xmodule.location)
@@ -708,7 +716,7 @@ def create_and_save_xmodule(self, location, definition_data=None, metadata=None,
             course.tabs = existing_tabs
             # Save any changes to the course to the MongoKeyValueStore
             course.save()
-            self.update_metadata(course.location, course.get_explicitly_set_fields_by_scope(Scope.settings))
+            self.update_item(course, 'create_and_save_xmodule')
 
     def fire_updated_modulestore_signal(self, course_id, location):
         """
@@ -754,7 +762,7 @@ def _update_single_item(self, location, update):
         # See http://www.mongodb.org/display/DOCS/Updating for
         # atomic update syntax
         result = self.collection.update(
-            {'_id': Location(location).dict()},
+            {'_id': namedtuple_to_son(Location(location))},
             {'$set': update},
             multi=False,
             upsert=True,
@@ -765,73 +773,56 @@ def _update_single_item(self, location, update):
         if result['n'] == 0:
             raise ItemNotFoundError(location)
 
-    def update_item(self, location, data, allow_not_found=False):
+    def update_item(self, xblock, user, allow_not_found=False):
         """
-        Set the data in the item specified by the location to
-        data
+        Update the persisted version of xblock to reflect its current values.
 
         location: Something that can be passed to Location
         data: A nested dictionary of problem data
         """
         try:
-            self._update_single_item(location, {'definition.data': data})
+            definition_data = xblock.get_explicitly_set_fields_by_scope()
+            if len(definition_data) == 1 and 'data' in definition_data:
+                definition_data = definition_data['data']
+            payload = {
+                'definition.data': definition_data,
+                'metadata': own_metadata(xblock),
+            }
+            if xblock.has_children:
+                # convert all to urls
+                xblock.children = [child.url() if isinstance(child, Location) else child
+                                   for child in xblock.children]
+                payload.update({'definition.children': xblock.children})
+            self._update_single_item(xblock.location, payload)
+            if xblock.category == 'static_tab':
+                course = self._get_course_for_item(xblock.location)
+                existing_tabs = course.tabs or []
+                for tab in existing_tabs:
+                    if tab.get('url_slug') == xblock.location.name:
+                        if xblock.fields['display_name'].is_set_on(xblock):
+                            tab['name'] = xblock.display_name
+                        break
+                course.tabs = existing_tabs
+                # Save the updates to the course to the MongoKeyValueStore
+                self.update_item(course, user)
+
+            # recompute (and update) the metadata inheritance tree which is cached
+            # was conditional on children or metadata having changed before dhm made one update to rule them all
+            self.refresh_cached_metadata_inheritance_tree(xblock.location)
+            # fire signal that we've written to DB
+            self.fire_updated_modulestore_signal(get_course_id_no_run(xblock.location), xblock.location)
         except ItemNotFoundError:
             if not allow_not_found:
                 raise
 
-    def update_children(self, location, children):
-        """
-        Set the children for the item specified by the location to
-        children
-
-        location: Something that can be passed to Location
-        children: A list of child item identifiers
-        """
-        # Normalize the children to urls
-        children = [Location(child).url() for child in children]
-
-        self._update_single_item(location, {'definition.children': children})
-        # recompute (and update) the metadata inheritance tree which is cached
-        self.refresh_cached_metadata_inheritance_tree(Location(location))
-        # fire signal that we've written to DB
-        self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
-
-    def update_metadata(self, location, metadata):
-        """
-        Set the metadata for the item specified by the location to
-        metadata
-
-        location: Something that can be passed to Location
-        metadata: A nested dictionary of module metadata
-        """
-        # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
-        # if we add one then we need to also add it to the policy information (i.e. metadata)
-        # we should remove this once we can break this reference from the course to static tabs
-        loc = Location(location)
-        if loc.category == 'static_tab':
-            course = self._get_course_for_item(loc)
-            existing_tabs = course.tabs or []
-            for tab in existing_tabs:
-                if tab.get('url_slug') == loc.name:
-                    tab['name'] = metadata.get('display_name', tab.get('name'))
-                    break
-            course.tabs = existing_tabs
-            # Save the updates to the course to the MongoKeyValueStore
-            course.save()
-            self.update_metadata(course.location, own_metadata(course))
-
-        self._update_single_item(location, {'metadata': metadata})
-        # recompute (and update) the metadata inheritance tree which is cached
-        self.refresh_cached_metadata_inheritance_tree(loc)
-        self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location))
-
-    def delete_item(self, location, delete_all_versions=False):
+    # pylint: disable=unused-argument
+    def delete_item(self, location, **kwargs):
         """
         Delete an item from this modulestore
 
         location: Something that can be passed to Location
-        delete_all_versions: is here because the DraftMongoModuleStore needs it and we need to keep the interface the same. It is unused.
         """
+        # pylint: enable=unused-argument
         # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
         # if we add one then we need to also add it to the policy information (i.e. metadata)
         # we should remove this once we can break this reference from the course to static tabs
@@ -842,7 +833,7 @@ def delete_item(self, location, delete_all_versions=False):
             course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != location.name]
             # Save the updates to the course to the MongoKeyValueStore
             course.save()
-            self.update_metadata(course.location, own_metadata(course))
+            self.update_item(course, 'delete_item')
 
         # Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False")
         # from overriding our default value set in the init method.
@@ -889,7 +880,7 @@ def get_orphans(self, course_location, _branch):
         item_locs -= all_reachable
         return list(item_locs)
 
-    def _create_new_field_data(self, category, location, definition_data, metadata):
+    def _create_new_field_data(self, _category, _location, definition_data, metadata):
         """
         To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs
         """
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index f7a025015552..abb59e498347 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -11,11 +11,9 @@
 from xmodule.exceptions import InvalidVersionError
 from xmodule.modulestore import Location
 from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError
-from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.mongo.base import location_to_query, namedtuple_to_son, get_course_id_no_run, MongoModuleStore
 import pymongo
 from pytz import UTC
-from xblock.fields import Scope
 
 DRAFT = 'draft'
 # Things w/ these categories should never be marked as version='draft'
@@ -109,7 +107,7 @@ def create_xmodule(self, location, definition_data=None, metadata=None, system=N
         return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system)
 
 
-    def get_items(self, location, course_id=None, depth=0):
+    def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         """
         Returns a list of XModuleDescriptor instances for the items
         that match location. Any element of location that is None is treated
@@ -146,7 +144,9 @@ def convert_to_draft(self, source_location):
         draft_location = as_draft(source_location)
         if draft_location.category in DIRECT_ONLY_CATEGORIES:
             raise InvalidVersionError(source_location)
-        original['_id'] = draft_location.dict()
+        if not original:
+            raise ItemNotFoundError
+        original['_id'] = namedtuple_to_son(draft_location)
         try:
             self.collection.insert(original)
         except pymongo.errors.DuplicateKeyError:
@@ -157,60 +157,27 @@ def convert_to_draft(self, source_location):
 
         return self._load_items([original])[0]
 
-    def update_item(self, location, data, allow_not_found=False):
+    def update_item(self, xblock, user, allow_not_found=False):
         """
-        Set the data in the item specified by the location to
-        data
+        Save the current values to persisted version of the xblock
 
         location: Something that can be passed to Location
         data: A nested dictionary of problem data
         """
-        draft_loc = as_draft(location)
+        draft_loc = as_draft(xblock.location)
         try:
-            draft_item = self.get_item(location)
-            if not getattr(draft_item, 'is_draft', False):
-                self.convert_to_draft(location)
+            if not self.has_item(None, draft_loc):
+                self.convert_to_draft(xblock.location)
         except ItemNotFoundError, e:
             if not allow_not_found:
                 raise e
 
-        return super(DraftModuleStore, self).update_item(draft_loc, data)
-
-    def update_children(self, location, children):
-        """
-        Set the children for the item specified by the location to
-        children
-
-        location: Something that can be passed to Location
-        children: A list of child item identifiers
-        """
-        draft_loc = as_draft(location)
-        draft_item = self.get_item(location)
-        if not getattr(draft_item, 'is_draft', False):
-            self.convert_to_draft(location)
-
-        return super(DraftModuleStore, self).update_children(draft_loc, children)
-
-    def update_metadata(self, location, metadata):
-        """
-        Set the metadata for the item specified by the location to
-        metadata
-
-        location: Something that can be passed to Location
-        metadata: A nested dictionary of module metadata
-        """
-        draft_loc = as_draft(location)
-        draft_item = self.get_item(location)
-
-        if not getattr(draft_item, 'is_draft', False):
-            self.convert_to_draft(location)
-
-        if 'is_draft' in metadata:
-            del metadata['is_draft']
-
-        return super(DraftModuleStore, self).update_metadata(draft_loc, metadata)
+        xblock.location = draft_loc
+        super(DraftModuleStore, self).update_item(xblock, user)
+        # don't allow locations to truly represent thenmselves as draft outside of this file
+        xblock.location = as_published(xblock.location)
 
-    def delete_item(self, location, delete_all_versions=False):
+    def delete_item(self, location, delete_all_versions=False, **kwargs):
         """
         Delete an item from this modulestore
 
@@ -243,7 +210,6 @@ def publish(self, location, published_by_id):
 
         draft.published_date = datetime.now(UTC)
         draft.published_by = published_by_id
-        super(DraftModuleStore, self).update_item(location, draft.get_explicitly_set_fields_by_scope(Scope.content))
         if draft.has_children:
             if original_published is not None:
                 # see if children were deleted. 2 reasons for children lists to differ:
@@ -254,8 +220,7 @@ def publish(self, location, published_by_id):
                         rents = [Location(mom) for mom in self.get_parent_locations(child, None)]
                         if (len(rents) == 1 and rents[0] == Location(location)):  # the 1 is this original_published
                             self.delete_item(child, True)
-            super(DraftModuleStore, self).update_children(location, draft.children)
-        super(DraftModuleStore, self).update_metadata(location, own_metadata(draft))
+        super(DraftModuleStore, self).update_item(draft, None)
         self.delete_item(location)
 
     def unpublish(self, location):
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 279002e47327..ea89f7b4b77e 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -57,7 +57,8 @@
 
 from xmodule.errortracker import null_error_tracker
 from xmodule.x_module import prefer_xmodules
-from xmodule.modulestore.locator import BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId
+from xmodule.modulestore.locator import BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId, \
+    Locator
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError
 from xmodule.modulestore import inheritance, ModuleStoreWriteBase, Location, SPLIT_MONGO_MODULESTORE_TYPE
 
@@ -109,6 +110,7 @@ def __init__(self, doc_store_config, fs_root, render_template,
 
         super(SplitMongoModuleStore, self).__init__(**kwargs)
         self.loc_mapper = loc_mapper
+        self.reference_type = Locator
 
         self.db_connection = MongoConnection(**doc_store_config)
         self.db = self.db_connection.database
@@ -1115,14 +1117,6 @@ def _compare_settings(self, settings, original_fields):
                 if key not in new_keys or original_fields[key] != settings[key]:
                     return True
 
-    def update_children(self, location, children):
-        '''Deprecated, use update_item.'''
-        raise NotImplementedError('use update_item')
-
-    def update_metadata(self, location, metadata):
-        '''Deprecated, use update_item.'''
-        raise NotImplementedError('use update_item')
-
     def xblock_publish(self, user_id, source_course, destination_course, subtree_list, blacklist):
         """
         Publishes each xblock in subtree_list and those blocks descendants excluding blacklist
@@ -1211,7 +1205,8 @@ def update_course_index(self, updated_index_entry):
         """
         self.db_connection.update_course_index(updated_index_entry)
 
-    def delete_item(self, usage_locator, user_id, delete_children=False, force=False):
+    # TODO impl delete_all_versions
+    def delete_item(self, usage_locator, user_id, delete_all_versions=False, delete_children=False, force=False):
         """
         Delete the block or tree rooted at block (if delete_children) and any references w/in the course to the block
         from a new version of the course structure.
@@ -1361,6 +1356,16 @@ def definition_locator(self, definition):
         else:
             return DefinitionLocator(definition['_id'])
 
+    def get_modulestore_type(self, course_id):
+        """
+        Returns an enumeration-like type reflecting the type of this modulestore
+        The return can be one of:
+        "xml" (for XML based courses),
+        "mongo" for old-style MongoDB backed courses,
+        "split" for new-style split MongoDB backed courses.
+        """
+        return SPLIT_MONGO_MODULESTORE_TYPE
+
     def internal_clean_children(self, course_locator):
         """
         Only intended for rather low level methods to use. Goes through the children attrs of
@@ -1509,16 +1514,6 @@ def _filter_special_fields(self, fields):
             del fields['category']
         return fields
 
-    def get_modulestore_type(self, course_id):
-        """
-        Returns an enumeration-like type reflecting the type of this modulestore
-        The return can be one of:
-        "xml" (for XML based courses),
-        "mongo" for old-style MongoDB backed courses,
-        "split" for new-style split MongoDB backed courses.
-        """
-        return SPLIT_MONGO_MODULESTORE_TYPE
-
     def _new_structure(self, user_id, root_block_id,
                        root_category=None, block_fields=None, definition_id=None):
         """
diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
index 9408dc14fd91..43e1b4ec0fd6 100644
--- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py
+++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
@@ -1,7 +1,6 @@
 import re
 from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore import Location
-from xmodule.modulestore.inheritance import own_metadata
 
 import logging
 
@@ -125,13 +124,9 @@ def _clone_modules(modulestore, modules, source_location, dest_location):
 
         print "Cloning module {0} to {1}....".format(original_loc, module.location)
 
-        # NOTE: usage of the the internal module.xblock_kvs._data does not include any 'default' values for the fields
-        data = module.xblock_kvs._data
-        if isinstance(data, basestring):
-            data = rewrite_nonportable_content_links(
-                source_location.course_id, dest_location.course_id, data)
-
-        modulestore.update_item(module.location, data)
+        if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring):
+            module.data = rewrite_nonportable_content_links(
+                source_location.course_id, dest_location.course_id, module.data)
 
         # repoint children
         if module.has_children:
@@ -145,10 +140,9 @@ def _clone_modules(modulestore, modules, source_location, dest_location):
                 )
                 new_children.append(child_loc.url())
 
-            modulestore.update_children(module.location, new_children)
+            module.children = new_children
 
-        # save metadata
-        modulestore.update_metadata(module.location, own_metadata(module))
+        modulestore.update_item(module, '_clone_modules')
 
 
 def clone_course(modulestore, contentstore, source_location, dest_location, delete_original=False):
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
index 88bc6c87d9fb..8211d71e2da5 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
@@ -33,6 +33,7 @@ def mixed_store_config(data_dir, mappings):
             'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
             'OPTIONS': {
                 'mappings': mappings,
+                'reference_type': 'Location',
                 'stores': {
                     'default': mongo_config['default'],
                     'xml': xml_config['default']
@@ -196,18 +197,15 @@ class FooTest(ModuleStoreTestCase):
     """
 
     @staticmethod
-    def update_course(course, data):
+    def update_course(course):
         """
         Updates the version of course in the modulestore
-        with the metadata in 'data' and returns the updated version.
 
         'course' is an instance of CourseDescriptor for which we want
         to update metadata.
-
-        'data' is a dictionary with an entry for each CourseField we want to update.
         """
         store = editable_modulestore('direct')
-        store.update_metadata(course.location, data)
+        store.update_item(course, 'testuser')
         updated_course = store.get_instance(course.id, course.location)
         return updated_course
 
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index 70b580b69d49..793792dd8d5a 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -1,9 +1,6 @@
-import datetime
-
 from factory import Factory, lazy_attribute_sequence, lazy_attribute
 from factory.containers import CyclicDefinitionError
 from uuid import uuid4
-from pytz import UTC
 
 from xmodule.modulestore import Location
 from xmodule.x_module import prefer_xmodules
@@ -127,7 +124,6 @@ def _create(cls, target_class, **kwargs):
         # passed in via **kwargs. However, some of those aren't actual field values,
         # so pop those off for use separately
 
-        DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info']
         # catch any old style users before they get into trouble
         assert 'template' not in kwargs
         parent_location = Location(kwargs.pop('parent_location', None))
@@ -165,8 +161,8 @@ def _create(cls, target_class, **kwargs):
 
         store.save_xmodule(module)
 
-        if location.category not in DETACHED_CATEGORIES:
+        if 'detached' not in module._class_tags:
             parent.children.append(location.url())
-            store.update_children(parent_location, parent.children)
+            store.update_item(parent, 'factory')
 
         return store.get_item(location)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
index a0e869d9fe39..0848e5c707f3 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
@@ -7,7 +7,7 @@
 import uuid
 from xmodule.modulestore import Location
 from xmodule.modulestore.locator import BlockUsageLocator
-from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 from xmodule.modulestore.loc_mapper_store import LocMapperStore
 from mock import Mock
 
@@ -114,7 +114,7 @@ def test_translate_location_read_only(self):
 
         new_style_package_id = '{}.geek_dept.{}.baz_run'.format(org, course)
         block_map = {
-            'abc123': {'problem': 'problem2'},
+            'abc123': {'problem': 'problem2', 'vertical': 'vertical2'},
             'def456': {'problem': 'problem4'},
             'ghi789': {'problem': 'problem7'},
         }
@@ -136,6 +136,14 @@ def test_translate_location_read_only(self):
                 Location('i4x', org, course, 'problem', '1def23'),
                 add_entry_if_missing=False
             )
+        test_no_cat_locn = test_problem_locn.replace(category=None)
+        with self.assertRaises(InvalidLocationError):
+            loc_mapper().translate_location(
+                old_style_course_id, test_no_cat_locn, False, False
+            )
+        test_no_cat_locn = test_no_cat_locn.replace(name='def456')
+        # only one course matches
+        self.translate_n_check(test_no_cat_locn, old_style_course_id, new_style_package_id, 'problem4', 'published')
 
         # add a distractor course (note that abc123 has a different translation in this one)
         distractor_block_map = {
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
index 5922f13afcf1..5ec34ae801bb 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
@@ -259,6 +259,24 @@ def test_colon_name(self):
         testobj = BlockUsageLocator(package_id=package_id, branch=branch, block_id=block_id)
         self.check_block_locn_fields(testobj, 'Cannot handle colon', package_id=package_id, branch=branch, block=block_id)
 
+    def test_relative(self):
+        """
+        It seems we used to use colons in names; so, ensure they're acceptable.
+        """
+        package_id = 'mit.eecs-1'
+        branch = 'foo'
+        baseobj = CourseLocator(package_id=package_id, branch=branch)
+        block_id = 'problem:with-colon~2'
+        testobj = BlockUsageLocator.make_relative(baseobj, block_id)
+        self.check_block_locn_fields(
+            testobj, 'Cannot make relative to course', package_id=package_id, branch=branch, block=block_id
+        )
+        block_id = 'completely_different'
+        testobj = BlockUsageLocator.make_relative(testobj, block_id)
+        self.check_block_locn_fields(
+            testobj, 'Cannot make relative to block usage', package_id=package_id, branch=branch, block=block_id
+        )
+
     def test_repr(self):
         testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published/' + BLOCK_PREFIX + 'HW3'
         testobj = BlockUsageLocator(package_id=testurn)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
index 834e55398c07..339b662db553 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -37,6 +37,7 @@
         XML_COURSEID2: 'xml',
         IMPORT_COURSEID: 'default'
     },
+    'reference_type': 'Location',
     'stores': {
         'xml': {
             'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
@@ -182,6 +183,7 @@ def test_get_instance(self):
             )
 
     def test_get_items(self):
+        # NOTE: use get_course if you just want the course. get_items only allows wildcarding of category and name
         modules = self.store.get_items(Location('i4x', None, None, 'course', None), IMPORT_COURSEID)
         assert_equals(len(modules), 1)
         assert_equals(modules[0].location.course, self.import_course)
@@ -190,22 +192,15 @@ def test_get_items(self):
         assert_equals(len(modules), 1)
         assert_equals(modules[0].location.course, 'toy')
 
-        modules = self.store.get_items(Location('i4x', None, None, 'course', None), XML_COURSEID2)
+        modules = self.store.get_items(Location('i4x', 'edX', 'simple', 'course', None), XML_COURSEID2)
         assert_equals(len(modules), 1)
         assert_equals(modules[0].location.course, 'simple')
 
     def test_update_item(self):
+        # FIXME update
         with assert_raises(NotImplementedError):
             self.store.update_item(self.fake_location, None)
 
-    def test_update_children(self):
-        with assert_raises(NotImplementedError):
-            self.store.update_children(self.fake_location, None)
-
-    def test_update_metadata(self):
-        with assert_raises(NotImplementedError):
-            self.store.update_metadata(self.fake_location, None)
-
     def test_delete_item(self):
         with assert_raises(NotImplementedError):
             self.store.delete_item(self.fake_location)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py
index 16c70ea634f6..bd199cc02b00 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py
@@ -82,7 +82,7 @@ def _create_item(self, category, name, data, metadata, parent_category, parent_n
             parent_location = Location('i4x', 'test_org', 'test_course', parent_category, parent_name)
             parent = self.old_mongo.get_item(parent_location)
             parent.children.append(location.url())
-            self.old_mongo.update_children(parent_location, parent.children)
+            self.old_mongo.update_item(parent, self.userid)
             # create pointer for split
             course_or_parent_locator = BlockUsageLocator(
                 package_id=self.split_package_id,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
index 867351370c2f..40a7d1a88f9d 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
@@ -71,7 +71,7 @@ def _create_item(self, category, name, data, metadata, parent_category, parent_n
                 mongo = self.old_mongo
             else:
                 mongo = self.draft_mongo
-            mongo.update_children(parent_location, parent.children)
+            mongo.update_item(parent, '_create_item')
 
     def _create_course(self):
         """
@@ -174,8 +174,8 @@ def test_publish_draft_delete(self):
         draft_vert.children.remove(other_child_loc.url())
         other_vert = self.draft_mongo.get_item(self.course_location.replace(category='vertical', name='Vert2'), 0)
         other_vert.children.append(other_child_loc.url())
-        self.draft_mongo.update_children(draft_vert.location, draft_vert.children)
-        self.draft_mongo.update_children(other_vert.location, other_vert.children)
+        self.draft_mongo.update_item(draft_vert, 'test_publish_delete')
+        self.draft_mongo.update_item(other_vert, 'test_publish_delete')
         # publish
         self._xmodule_recurse(
             draft_vert,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
index 0341d43436bb..1e662c197b99 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
@@ -102,7 +102,7 @@ def create_source_course(self):
         location = location.replace(category='chapter', name=uuid.uuid4().hex)
         chapter2 = self._create_and_get_item(self.old_mongo, location, {}, {'display_name': 'Chapter 2'}, runtime)
         course_root.children.append(chapter2.location.url())
-        self.old_mongo.update_children(course_root.location, course_root.children)
+        self.old_mongo.update_item(course_root, 'test_split_migrate')
         # vertical in live only
         location = location.replace(category='vertical', name=uuid.uuid4().hex)
         live_vert = self._create_and_get_item(self.old_mongo, location, {}, {'display_name': 'Live vertical'}, runtime)
@@ -140,7 +140,7 @@ def create_source_course(self):
         self.create_random_units(self.old_mongo, live_vert)
 
         # update the chapter
-        self.old_mongo.update_children(chapter1.location, chapter1.children)
+        self.old_mongo.update_item(chapter1, 'test_split_migrator')
 
         # now the other one w/ the conditional
         # first create some show children
@@ -169,7 +169,7 @@ def create_source_course(self):
         # add direct children
         self.create_random_units(self.old_mongo, conditional)
         chapter2.children.append(conditional.location.url())
-        self.old_mongo.update_children(chapter2.location, chapter2.children)
+        self.old_mongo.update_item(chapter2, 'test_split_migrator')
 
         # and the ancillary docs (not children)
         location = location.replace(category='static_tab', name=uuid.uuid4().hex)
@@ -207,9 +207,9 @@ def create_random_units(self, store, parent, cc_store=None, cc_parent=None):
                     cc_store, location, data, {'display_name': str(uuid.uuid4())}, parent.runtime
                 )
                 cc_parent.children.append(element.location.url())
-        store.update_children(parent.location, parent.children)
+        store.update_item(parent, 'test_split_migrator')
         if cc_store is not None:
-            cc_store.update_children(cc_parent.location, cc_parent.children)
+            cc_store.update_item(cc_parent, 'test_split_migrator')
 
     def compare_courses(self, presplit, published):
         # descend via children to do comparison
diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py
index 0cdaf7f7e328..d78a4dcf1e11 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml.py
@@ -377,6 +377,7 @@ class to use if none is specified in entry_points
             self.default_class = class_
 
         self.parent_trackers = defaultdict(ParentTracker)
+        self.reference_type = Location
 
         # All field data will be stored in an inheriting field data.
         self.field_data = inheriting_field_data(kvs=DictKeyValueStore())
@@ -640,7 +641,7 @@ def get_item(self, location, depth=0):
         raise NotImplementedError("XMLModuleStores can't guarantee that definitions"
                                   " are unique. Use get_instance.")
 
-    def get_items(self, location, course_id=None, depth=0):
+    def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         items = []
 
         def _add_get_items(self, location, modules):
@@ -672,33 +673,22 @@ def get_errored_courses(self):
         """
         return dict((k, self.errored_courses[k].errors) for k in self.errored_courses)
 
-    def update_item(self, location, data):
+    def get_orphans(self, course_location, _branch):
         """
-        Set the data in the item specified by the location to
-        data
-
-        location: Something that can be passed to Location
-        data: A nested dictionary of problem data
+        Get all of the xblocks in the given course which have no parents and are not of types which are
+        usually orphaned. NOTE: may include xblocks which still have references via xblocks which don't
+        use children to point to their dependents.
         """
-        raise NotImplementedError("XMLModuleStores are read-only")
+        # here just to quell the abstractmethod. someone could write the impl if needed
+        raise NotImplementedError
 
-    def update_children(self, location, children):
+    def update_item(self, xblock, user, **kwargs):
         """
-        Set the children for the item specified by the location to
+        Set the data in the item specified by the location to
         data
 
         location: Something that can be passed to Location
-        children: A list of child item identifiers
-        """
-        raise NotImplementedError("XMLModuleStores are read-only")
-
-    def update_metadata(self, location, metadata):
-        """
-        Set the metadata for the item specified by the location to
-        metadata
-
-        location: Something that can be passed to Location
-        metadata: A nested dictionary of module metadata
+        data: A nested dictionary of problem data
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 47dedecc6bff..5815a459bc2a 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -312,43 +312,13 @@ def import_module(
 
     logging.debug('processing import of module {}...'.format(module.location.url()))
 
-    content = {}
-    for field in module.fields.values():
-        if field.scope != Scope.content:
-            continue
-        try:
-            content[field.name] = module._field_data.get(module, field.name)
-        except KeyError:
-            # Ignore any missing keys in _field_data
-            pass
-
-    module_data = {}
-    if 'data' in content:
-        module_data = content['data']
-    else:
-        module_data = content
-
-    if isinstance(module_data, basestring) and do_import_static:
+    if 'data' in module.fields and do_import_static:
         # we want to convert all 'non-portable' links in the module_data
         # (if it is a string) to portable strings (e.g. /static/)
-        module_data = rewrite_nonportable_content_links(
+        module.data = rewrite_nonportable_content_links(
             source_course_location.course_id,
-            dest_course_location.course_id, module_data
-        )
-
-    if allow_not_found:
-        store.update_item(
-            module.location, module_data, allow_not_found=allow_not_found
+            dest_course_location.course_id, module.data
         )
-    else:
-        store.update_item(module.location, module_data)
-
-    if hasattr(module, 'children') and module.children != []:
-        store.update_children(module.location, module.children)
-
-    # NOTE: It's important to use own_metadata here to avoid writing
-    # inherited metadata everywhere.
-
     # remove any export/import only xml_attributes
     # which are used to wire together draft imports
     if 'parent_sequential_url' in getattr(module, 'xml_attributes', []):
@@ -356,9 +326,8 @@ def import_module(
 
     if 'index_in_children_list' in getattr(module, 'xml_attributes', []):
         del module.xml_attributes['index_in_children_list']
-    module.save()
 
-    store.update_metadata(module.location, dict(own_metadata(module)))
+    store.update_item(module, None, allow_not_found=allow_not_found)
 
 
 def import_course_draft(
@@ -490,7 +459,7 @@ def _import_module(module):
 
                             if non_draft_location.url() not in sequential.children:
                                 sequential.children.insert(index, non_draft_location.url())
-                                store.update_children(sequential.location, sequential.children)
+                                store.update_item(sequential, 'importer')
 
                         import_module(
                             module, draft_store, course_data_path,
diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py
index 6221211e44f7..662aec3910cd 100644
--- a/lms/djangoapps/courseware/tests/test_submitting_problems.py
+++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py
@@ -229,9 +229,9 @@ def add_grading_policy(self, grading_policy):
         Add a grading policy to the course.
         """
 
-        course_data = {'grading_policy': grading_policy}
+        self.course.grading_policy = grading_policy
         store = editable_modulestore('direct')
-        store.update_item(self.course.location, course_data)
+        store.update_item(self.course, 'add_grading_policy')
         self.refresh_course()
 
     def get_grade_summary(self):
diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py
index acc39c500107..e7fadb006fec 100644
--- a/lms/djangoapps/courseware/tests/test_view_authentication.py
+++ b/lms/djangoapps/courseware/tests/test_view_authentication.py
@@ -25,6 +25,7 @@
     OrgStaffFactory,
     OrgInstructorFactory,
 )
+from xmodule.modulestore.django import modulestore
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -84,7 +85,7 @@ def _check_staff(self, course):
         urls.extend([
             reverse('book', kwargs={'course_id': course.id,
                                     'book_index': index})
-            for index, book in enumerate(course.textbooks)
+            for index, _book in enumerate(course.textbooks)
         ])
         for url in urls:
             check_for_get_code(self, 200, url)
@@ -112,6 +113,7 @@ def setUp(self):
         self.course = CourseFactory.create(number='999', display_name='Robot_Super_Course')
         self.overview_chapter = ItemFactory.create(display_name='Overview')
         self.courseware_chapter = ItemFactory.create(display_name='courseware')
+        self.course = modulestore().get_course(self.course.id)
 
         self.test_course = CourseFactory.create(number='666', display_name='Robot_Sub_Course')
         self.other_org_course = CourseFactory.create(org='Other_Org_Course')
@@ -126,6 +128,7 @@ def setUp(self):
             parent_location=self.overview_chapter.location,
             display_name='Welcome'
         )
+        self.test_course = modulestore().get_course(self.test_course.id)
 
         self.global_staff_user = GlobalStaffFactory()
         self.unenrolled_user = UserFactory(last_name="Unenrolled")
@@ -264,10 +267,10 @@ def test_dark_launch_enrolled_student(self):
         # Make courses start in the future
         now = datetime.datetime.now(pytz.UTC)
         tomorrow = now + datetime.timedelta(days=1)
-        course_data = {'start': tomorrow}
-        test_course_data = {'start': tomorrow}
-        self.course = self.update_course(self.course, course_data)
-        self.test_course = self.update_course(self.test_course, test_course_data)
+        self.course.start = tomorrow
+        self.test_course.start = tomorrow
+        self.course = self.update_course(self.course)
+        self.test_course = self.update_course(self.test_course)
 
         self.assertFalse(self.course.has_started())
         self.assertFalse(self.test_course.has_started())
@@ -289,10 +292,10 @@ def test_dark_launch_instructor(self):
         """
         now = datetime.datetime.now(pytz.UTC)
         tomorrow = now + datetime.timedelta(days=1)
-        course_data = {'start': tomorrow}
-        test_course_data = {'start': tomorrow}
-        self.course = self.update_course(self.course, course_data)
-        self.test_course = self.update_course(self.test_course, test_course_data)
+        self.course.start = tomorrow
+        self.test_course.start = tomorrow
+        self.course = self.update_course(self.course)
+        self.test_course = self.update_course(self.test_course)
 
         self.login(self.instructor_user)
         # Enroll in the classes---can't see courseware otherwise.
@@ -312,10 +315,10 @@ def test_dark_launch_global_staff(self):
         """
         now = datetime.datetime.now(pytz.UTC)
         tomorrow = now + datetime.timedelta(days=1)
-        course_data = {'start': tomorrow}
-        test_course_data = {'start': tomorrow}
-        self.course = self.update_course(self.course, course_data)
-        self.test_course = self.update_course(self.test_course, test_course_data)
+        self.course.start = tomorrow
+        self.test_course.start = tomorrow
+        self.course = self.update_course(self.course)
+        self.test_course = self.update_course(self.test_course)
 
         self.login(self.global_staff_user)
         self.enroll(self.course, True)
@@ -336,13 +339,14 @@ def test_enrollment_period(self):
         nextday = tomorrow + datetime.timedelta(days=1)
         yesterday = now - datetime.timedelta(days=1)
 
-        course_data = {'enrollment_start': tomorrow, 'enrollment_end': nextday}
-        test_course_data = {'enrollment_start': yesterday, 'enrollment_end': tomorrow}
-
         # self.course's enrollment period hasn't started
-        self.course = self.update_course(self.course, course_data)
+        self.course.enrollment_start = tomorrow
+        self.course.enrollment_end = nextday
         # test_course course's has
-        self.test_course = self.update_course(self.test_course, test_course_data)
+        self.test_course.enrollment_start = yesterday
+        self.test_course.enrollment_end = tomorrow
+        self.course = self.update_course(self.course)
+        self.test_course = self.update_course(self.test_course)
 
         # First, try with an enrolled student
         self.login(self.unenrolled_user)
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index a47ebac0178c..d9b1bbf82552 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -234,13 +234,13 @@ def index(request, course_id, chapter=None, section=None,
      - HTTPresponse
     """
     user = User.objects.prefetch_related("groups").get(id=request.user.id)
-    request.user = user	# keep just one instance of User
+    request.user = user  # keep just one instance of User
     course = get_course_with_access(user, course_id, 'load', depth=2)
     staff_access = has_access(user, course, 'staff')
     registered = registered_for_course(course, user)
     if not registered:
         # TODO (vshnayder): do course instructors need to be registered to see course?
-        log.debug(u'User {0} tried to view course {1} but is not enrolled'.format(user, course.location.url()))
+        log.debug('User %s tried to view course %s but is not enrolled', user, course.location.url())
         return redirect(reverse('about_course', args=[course.id]))
 
     masq = setup_masquerade(request, staff_access)
diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py
index 6ee8d3dc3d42..b6f9b9df8d2d 100644
--- a/lms/djangoapps/instructor_task/tests/test_base.py
+++ b/lms/djangoapps/instructor_task/tests/test_base.py
@@ -208,7 +208,9 @@ def redefine_option_problem(self, problem_url_name):
                         'num_responses': 2}
         problem_xml = factory.build_xml(**factory_args)
         location = InstructorTaskTestCase.problem_location(problem_url_name)
-        self.module_store.update_item(location, problem_xml)
+        item = self.module_store.get_instance(self.course.id, location)
+        item.data = problem_xml
+        self.module_store.update_item(item, 'redefine_option_problem')
 
     def get_student_module(self, username, descriptor):
         """Get StudentModule object for test course, given the `username` and the problem's `descriptor`."""
diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py
index e6cdb9218306..056a89b20208 100644
--- a/lms/djangoapps/instructor_task/tests/test_integration.py
+++ b/lms/djangoapps/instructor_task/tests/test_integration.py
@@ -288,7 +288,11 @@ def check_func(expect, answer_given):
             """ % ('!=' if redefine else '=='))
         problem_xml = factory.build_xml(script=script, cfn="check_func", expect="42", num_responses=1)
         if redefine:
-            self.module_store.update_item(InstructorTaskModuleTestCase.problem_location(problem_url_name), problem_xml)
+            descriptor = self.module_store.get_instance(
+                self.course.id, InstructorTaskModuleTestCase.problem_location(problem_url_name)
+            )
+            descriptor.data = problem_xml
+            self.module_store.update_item(descriptor, 'define_randomized_custom_response_problem')
         else:
             # Use "per-student" rerandomization so that check-problem can be called more than once.
             # Using "always" means we cannot check a problem twice, but we want to call once to get the
diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py
index b1d49b523766..a63aba0a2cdb 100644
--- a/lms/envs/acceptance.py
+++ b/lms/envs/acceptance.py
@@ -44,6 +44,7 @@ def seed():
         'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
         'OPTIONS': {
             'mappings': {},
+            'reference_type': 'Location',
             'stores': {
                 'default': {
                     'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
diff --git a/lms/envs/bok_choy.auth.json b/lms/envs/bok_choy.auth.json
index 931963c6d085..8f9e757490ec 100644
--- a/lms/envs/bok_choy.auth.json
+++ b/lms/envs/bok_choy.auth.json
@@ -51,6 +51,7 @@
             "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
             "OPTIONS": {
                 "mappings": {},
+                "reference_type": "Location",
                 "stores": {
                     "default": {
                         "DOC_STORE_CONFIG": {
diff --git a/lms/envs/cms/mixed_dev.py b/lms/envs/cms/mixed_dev.py
index 0454c2540905..35c5e0b82c15 100644
--- a/lms/envs/cms/mixed_dev.py
+++ b/lms/envs/cms/mixed_dev.py
@@ -12,6 +12,7 @@
     'default': {
         'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
         'OPTIONS': {
+            'reference_type': 'Location',
             'mappings': {
                 'MITx/2.01x/2013_Spring': 'xml'
             },

From 53f7c50731dd6cf9f664adbffd466ce4b182ec07 Mon Sep 17 00:00:00 2001
From: Don Mitchell <dmitchell@edx.org>
Date: Fri, 31 Jan 2014 13:38:14 -0500
Subject: [PATCH 2/4] Added test for defaulting use_locations

---
 .../lib/xmodule/xmodule/modulestore/mixed.py  |  2 +-
 .../tests/test_mixed_modulestore.py           | 24 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py
index 009c8c95f7a4..7d8627160b5a 100644
--- a/common/lib/xmodule/xmodule/modulestore/mixed.py
+++ b/common/lib/xmodule/xmodule/modulestore/mixed.py
@@ -36,7 +36,7 @@ def __init__(self, mappings, stores, reference_type=None, **kwargs):
         self.mappings = mappings
         # temporary code for transition period
         if reference_type is None:
-            log.warn("reference_type not specified in MixedModuleStore settings.",
+            log.warn("reference_type not specified in MixedModuleStore settings. %s",
                 "Will default temporarily to the to-be-deprecated Location.")
         self.use_locations = (reference_type != 'Locator')
         if 'default' not in stores:
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
index 339b662db553..1f66d0438280 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -13,6 +13,8 @@
 # Mixed modulestore depends on django, so we'll manually configure some django settings
 # before importing the module
 from django.conf import settings
+import unittest
+import copy
 if not settings.configured:
     settings.configure()
 
@@ -245,3 +247,25 @@ def test_get_parent_locations(self):
         assert_equals(Location(parents[0]).org, 'edX')
         assert_equals(Location(parents[0]).course, 'toy')
         assert_equals(Location(parents[0]).name, '2012_Fall')
+
+class TestMixedMSInit(unittest.TestCase):
+    """
+    Test initializing w/o a reference_type
+    """
+    def setUp(self):
+        unittest.TestCase.setUp(self)
+        options = copy.copy(OPTIONS)
+        del options['reference_type']
+        self.connection = pymongo.MongoClient(
+            host=HOST,
+            port=PORT,
+            tz_aware=True,
+        )
+        self.store = MixedModuleStore(**options)
+
+    def test_use_locations(self):
+        """
+        Test that use_locations defaulted correctly
+        """
+        self.assertTrue(self.store.use_locations)
+

From b8ea7f3c432f6a87eafb1b6d8be9b6962316934c Mon Sep 17 00:00:00 2001
From: Don Mitchell <dmitchell@edx.org>
Date: Fri, 31 Jan 2014 14:55:12 -0500
Subject: [PATCH 3/4] update_item and other refactorings continued

---
 .../contentstore/course_info_model.py         |  8 +--
 .../contentstore/tests/test_checklists.py     |  2 +-
 .../contentstore/tests/test_contentstore.py   | 18 ++---
 .../tests/test_course_settings.py             | 70 +++++++++++--------
 .../contentstore/tests/test_course_updates.py |  2 +-
 .../contentstore/tests/test_crud.py           |  2 +-
 .../contentstore/tests/test_orphan.py         |  2 +-
 .../contentstore/tests/test_textbooks.py      |  4 +-
 .../contentstore/tests/test_transcripts.py    | 26 +++----
 .../contentstore/transcripts_utils.py         | 12 ++--
 .../contentstore/views/checklist.py           |  4 +-
 cms/djangoapps/contentstore/views/course.py   | 25 +++----
 cms/djangoapps/contentstore/views/item.py     | 25 +++----
 cms/djangoapps/contentstore/views/tabs.py     | 12 ++--
 .../contentstore/views/transcripts_ajax.py    | 16 ++---
 .../models/settings/course_details.py         | 12 ++--
 .../models/settings/course_grading.py         | 30 ++++----
 .../models/settings/course_metadata.py        |  4 +-
 .../external_auth/tests/test_shib.py          |  8 +--
 common/djangoapps/student/tests/test_login.py |  2 +-
 .../xmodule/xmodule/modulestore/__init__.py   | 27 ++++++-
 .../lib/xmodule/xmodule/modulestore/mixed.py  |  3 +
 .../xmodule/xmodule/modulestore/mongo/base.py |  4 +-
 .../xmodule/modulestore/mongo/draft.py        |  4 +-
 .../xmodule/modulestore/split_mongo/split.py  |  7 +-
 .../xmodule/modulestore/store_utilities.py    |  2 +-
 .../xmodule/modulestore/tests/django_utils.py |  2 +-
 .../xmodule/modulestore/tests/factories.py    |  4 +-
 .../tests/test_mixed_modulestore.py           |  2 +-
 .../xmodule/modulestore/tests/test_publish.py |  6 +-
 .../modulestore/tests/test_split_migrator.py  | 10 +--
 .../tests/test_split_modulestore.py           | 12 ++--
 .../xmodule/modulestore/xml_importer.py       | 11 ++-
 .../tests/test_submitting_problems.py         |  2 +-
 lms/djangoapps/courseware/views.py            |  5 +-
 .../instructor_task/tests/test_base.py        |  2 +-
 .../instructor_task/tests/test_integration.py |  2 +-
 37 files changed, 213 insertions(+), 176 deletions(-)

diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py
index 0e2551cac81f..9dfd00a0da96 100644
--- a/cms/djangoapps/contentstore/course_info_model.py
+++ b/cms/djangoapps/contentstore/course_info_model.py
@@ -56,7 +56,7 @@ def get_course_updates(location, provided_id):
     return course_upd_collection
 
 
-def update_course_updates(location, update, passed_id=None):
+def update_course_updates(location, update, passed_id=None, user=None):
     """
     Either add or update the given course update. It will add it if the passed_id is absent or None. It will update it if
     it has an passed_id which has a valid value. Until updates have distinct values, the passed_id is the location url + an index
@@ -102,7 +102,7 @@ def update_course_updates(location, update, passed_id=None):
 
     # update db record
     course_updates.data = html.tostring(course_html_parsed)
-    modulestore('direct').update_item(course_updates, 'course_info_model')
+    modulestore('direct').update_item(course_updates, user.id if user else None)
 
     return {
         "id": idx,
@@ -125,7 +125,7 @@ def _course_info_content(html_parsed):
 
 
 # pylint: disable=unused-argument
-def delete_course_update(location, update, passed_id):
+def delete_course_update(location, update, passed_id, user):
     """
     Delete the given course_info update from the db.
     Returns the resulting course_updates b/c their ids change.
@@ -158,7 +158,7 @@ def delete_course_update(location, update, passed_id):
         # update db record
         course_updates.data = html.tostring(course_html_parsed)
         store = modulestore('direct')
-        store.update_item(course_updates, 'course_info_model')
+        store.update_item(course_updates, user.id)
 
     return get_course_updates(location, None)
 
diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py
index 14164680fc7a..90dd40a8b7bf 100644
--- a/cms/djangoapps/contentstore/tests/test_checklists.py
+++ b/cms/djangoapps/contentstore/tests/test_checklists.py
@@ -54,7 +54,7 @@ def test_get_checklists(self):
         # Save the changed `checklists` to the underlying KeyValueStore before updating the modulestore
         self.course.save()
         modulestore = get_modulestore(self.course.location)
-        modulestore.update_item(self.course, self.user.pk)
+        modulestore.update_item(self.course, self.user.id)
         self.assertEqual(self.get_persisted_checklists(), None)
         response = self.client.get(self.checklists_url)
         self.assertEqual(payload, response.content)
diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index c7f4b9339c70..b904a65afc61 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -124,7 +124,7 @@ def check_components_on_page(self, component_types, expected_types):
 
         course.advanced_modules = component_types
 
-        store.update_item(course, self.user.username)
+        store.update_item(course, self.user.id)
 
         # just pick one vertical
         descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
@@ -263,7 +263,7 @@ def test_draft_metadata(self):
         self.assertIn('graceperiod', own_metadata(html_module))
         self.assertEqual(html_module.graceperiod, new_graceperiod)
 
-        draft_store.update_item(html_module, self.user.username)
+        draft_store.update_item(html_module, self.user.id)
 
         # read back to make sure it reads as 'own-metadata'
         html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None))
@@ -379,7 +379,7 @@ def test_create_static_tab_and_rename(self):
         self.assertEqual(course.tabs, expected_tabs)
 
         item.display_name = 'Updated'
-        module_store.update_item(item, self.user.username)
+        module_store.update_item(item, self.user.id)
 
         course = module_store.get_item(course_location)
 
@@ -829,7 +829,7 @@ def test_portable_link_rewrites_during_clone_course(self):
         self.assertIsInstance(html_module.data, basestring)
         new_data = html_module.data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format(
             source_location.org, source_location.course))
-        module_store.update_item(html_module, None)
+        module_store.update_item(html_module, self.user.id)
 
         html_module = module_store.get_instance(source_location.course_id, html_module_location)
         self.assertEqual(new_data, html_module.data)
@@ -862,7 +862,7 @@ def test_illegal_draft_crud_ops(self):
         chapter.data = 'chapter data'
 
         with self.assertRaises(InvalidVersionError):
-            draft_store.update_item(chapter, 'user')
+            draft_store.update_item(chapter, self.user.id)
 
         self.assertRaises(InvalidVersionError, draft_store.unpublish, location)
 
@@ -982,7 +982,7 @@ def test_export_course(self, mock_get):
                                            'sequential', 'vertical_sequential', None]))
         private_location_no_draft = private_vertical.location.replace(revision=None)
         sequential.children.append(private_location_no_draft.url())
-        module_store.update_item(sequential, 'user')
+        module_store.update_item(sequential, self.user.id)
 
         # read back the sequential, to make sure we have a pointer to
         sequential = module_store.get_item(Location(['i4x', 'edX', 'toy',
@@ -1766,7 +1766,7 @@ def test_metadata_inheritance(self):
         module_store.create_and_save_xmodule(new_component_location)
         parent = verticals[0]
         parent.children.append(new_component_location.url())
-        module_store.update_item(parent, 'user')
+        module_store.update_item(parent, self.user.id)
 
         # flush the cache
         module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
@@ -1783,7 +1783,7 @@ def test_metadata_inheritance(self):
         # now let's define an override at the leaf node level
         #
         new_module.graceperiod = timedelta(1)
-        module_store.update_item(new_module, self.user.username)
+        module_store.update_item(new_module, self.user.id)
 
         # flush the cache and refetch
         module_store.refresh_cached_metadata_inheritance_tree(new_component_location)
@@ -1897,7 +1897,7 @@ def test_metadata_not_persistence(self):
             delattr(self.video_descriptor, field_name)
 
         self.assertNotIn('html5_sources', own_metadata(self.video_descriptor))
-        get_modulestore(location).update_item(self.video_descriptor, 'testuser')
+        get_modulestore(location).update_item(self.video_descriptor, '**replace_user**')
         module = get_modulestore(location).get_item(location)
 
         self.assertNotIn('html5_sources', own_metadata(module))
diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py
index bfe244820d9b..e6cae27619d7 100644
--- a/cms/djangoapps/contentstore/tests/test_course_settings.py
+++ b/cms/djangoapps/contentstore/tests/test_course_settings.py
@@ -73,32 +73,32 @@ def test_update_and_fetch(self):
         jsondetails.syllabus = "<a href='foo'>bar</a>"
         # encode - decode to convert date fields and other data which changes form
         self.assertEqual(
-            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).syllabus,
+            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).syllabus,
             jsondetails.syllabus, "After set syllabus"
         )
         jsondetails.overview = "Overview"
         self.assertEqual(
-            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).overview,
+            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).overview,
             jsondetails.overview, "After set overview"
         )
         jsondetails.intro_video = "intro_video"
         self.assertEqual(
-            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).intro_video,
+            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).intro_video,
             jsondetails.intro_video, "After set intro_video"
         )
         jsondetails.effort = "effort"
         self.assertEqual(
-            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).effort,
+            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).effort,
             jsondetails.effort, "After set effort"
         )
         jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC())
         self.assertEqual(
-            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).start_date,
+            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).start_date,
             jsondetails.start_date
         )
         jsondetails.course_image_name = "an_image.jpg"
         self.assertEqual(
-            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).course_image_name,
+            CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).course_image_name,
             jsondetails.course_image_name
         )
 
@@ -241,67 +241,72 @@ def test_fetch_grader(self):
 
     def test_update_from_json(self):
         test_grader = CourseGradingModel.fetch(self.course_locator)
-        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__)
+        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user)
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Noop update")
 
         test_grader.graders[0]['weight'] = test_grader.graders[0].get('weight') * 2
-        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__)
+        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user)
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Weight[0] * 2")
 
         test_grader.grade_cutoffs['D'] = 0.3
-        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__)
+        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user)
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "cutoff add D")
 
         test_grader.grace_period = {'hours': 4, 'minutes': 5, 'seconds': 0}
-        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__)
+        altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user)
         self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "4 hour grace period")
 
     def test_update_grader_from_json(self):
         test_grader = CourseGradingModel.fetch(self.course_locator)
-        altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1])
+        altered_grader = CourseGradingModel.update_grader_from_json(
+            self.course_locator, test_grader.graders[1], self.user)
         self.assertDictEqual(test_grader.graders[1], altered_grader, "Noop update")
 
         test_grader.graders[1]['min_count'] = test_grader.graders[1].get('min_count') + 2
-        altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1])
+        altered_grader = CourseGradingModel.update_grader_from_json(
+            self.course_locator, test_grader.graders[1], self.user)
         self.assertDictEqual(test_grader.graders[1], altered_grader, "min_count[1] + 2")
 
         test_grader.graders[1]['drop_count'] = test_grader.graders[1].get('drop_count') + 1
-        altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1])
+        altered_grader = CourseGradingModel.update_grader_from_json(
+            self.course_locator, test_grader.graders[1], self.user)
         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_locator)
-        CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs)
+        CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs, self.user)
         # 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_locator)
         self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "Noop update")
 
         test_grader.grade_cutoffs['D'] = 0.3
-        CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs)
+        CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs, self.user)
         altered_grader = CourseGradingModel.fetch(self.course_locator)
         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(self.course_locator, test_grader.grade_cutoffs)
+        CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs, self.user)
         altered_grader = CourseGradingModel.fetch(self.course_locator)
         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_locator)
-        CourseGradingModel.update_grace_period_from_json(self.course_locator, test_grader.grace_period)
+        CourseGradingModel.update_grace_period_from_json(
+            self.course_locator, test_grader.grace_period, self.user)
         # update_grace_period_from_json doesn't return anything, so query the db for its contents.
         altered_grader = CourseGradingModel.fetch(self.course_locator)
         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(self.course_locator, test_grader.grace_period)
+        CourseGradingModel.update_grace_period_from_json(
+            self.course_locator, test_grader.grace_period, self.user)
         altered_grader = CourseGradingModel.fetch(self.course_locator)
         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(self.course_locator)
+        CourseGradingModel.delete_grace_period(self.course_locator, self.user)
         # update_grace_period_from_json doesn't return anything, so query the db for its contents.
         altered_grader = CourseGradingModel.fetch(self.course_locator)
         # Once deleted, the grace period should simply be None
@@ -317,7 +322,7 @@ def test_update_section_grader_type(self):
         self.assertEqual(False, descriptor.graded)
 
         # Change the default grader type to Homework, which should also mark the section as graded
-        CourseGradingModel.update_section_grader_type(self.course, 'Homework')
+        CourseGradingModel.update_section_grader_type(self.course, 'Homework', self.user)
         descriptor = get_modulestore(self.course.location).get_item(self.course.location)
         section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator)
 
@@ -326,7 +331,7 @@ def test_update_section_grader_type(self):
         self.assertEqual(True, descriptor.graded)
 
         # Change the grader type back to notgraded, which should also unmark the section as graded
-        CourseGradingModel.update_section_grader_type(self.course, 'notgraded')
+        CourseGradingModel.update_section_grader_type(self.course, 'notgraded', self.user)
         descriptor = get_modulestore(self.course.location).get_item(self.course.location)
         section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator)
 
@@ -439,19 +444,25 @@ def test_fetch_initial_fields(self):
         self.assertIn('xqa_key', test_model, 'xqa_key field ')
 
     def test_update_from_json(self):
-        test_model = CourseMetadata.update_from_json(self.course, {
-            "advertised_start": "start A",
-            "days_early_for_beta": 2
-        })
+        test_model = CourseMetadata.update_from_json(
+            self.course, {
+                "advertised_start": "start A",
+                "days_early_for_beta": 2
+            },
+            user=self.user
+         )
         self.update_check(test_model)
         # try fresh fetch to ensure persistence
         fresh = modulestore().get_item(self.course_location)
         test_model = CourseMetadata.fetch(fresh)
         self.update_check(test_model)
         # now change some of the existing metadata
-        test_model = CourseMetadata.update_from_json(fresh, {
-            "advertised_start": "start B",
-            "display_name": "jolly roger"}
+        test_model = CourseMetadata.update_from_json(
+            fresh, {
+                "advertised_start": "start B",
+                "display_name": "jolly roger"
+            },
+            user=self.user
         )
         self.assertIn('display_name', test_model, 'Missing editable metadata field')
         self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value")
@@ -470,7 +481,8 @@ def test_delete_key(self):
         test_model = CourseMetadata.update_from_json(
             self.fullcourse, {
                 "unsetKeys": ['showanswer', 'xqa_key']
-            }
+            },
+            user=self.user
         )
         # ensure no harm
         self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in')
diff --git a/cms/djangoapps/contentstore/tests/test_course_updates.py b/cms/djangoapps/contentstore/tests/test_course_updates.py
index 1a112f87e14e..38b24d9f53e7 100644
--- a/cms/djangoapps/contentstore/tests/test_course_updates.py
+++ b/cms/djangoapps/contentstore/tests/test_course_updates.py
@@ -123,7 +123,7 @@ def test_no_ol_course_update(self):
         modulestore('direct').create_and_save_xmodule(location)
         course_updates = modulestore('direct').get_item(location)
         course_updates.data = 'bad news'
-        modulestore('direct').update_item(course_updates, 'test_course_updates')
+        modulestore('direct').update_item(course_updates, self.user.id)
 
         init_content = '<iframe width="560" height="315" src="http://www.youtube.com/embed/RocY-Jd93XU" frameborder="0">'
         content = init_content + '</iframe>'
diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py
index 6094c7cf3b97..290b90e4bc2a 100644
--- a/cms/djangoapps/contentstore/tests/test_crud.py
+++ b/cms/djangoapps/contentstore/tests/test_crud.py
@@ -172,7 +172,7 @@ def test_block_generations(self):
         )
         first_problem.max_attempts = 3
         first_problem.save()  # decache the above into the kvs
-        updated_problem = modulestore('split').update_item(first_problem, 'testbot')
+        updated_problem = modulestore('split').update_item(first_problem, '**replace_user**')
         self.assertIsNotNone(updated_problem.previous_version)
         self.assertEqual(updated_problem.previous_version, first_problem.update_version)
         self.assertNotEqual(updated_problem.update_version, first_problem.update_version)
diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py
index 3585d81e1569..0bdcb69bbc3f 100644
--- a/cms/djangoapps/contentstore/tests/test_orphan.py
+++ b/cms/djangoapps/contentstore/tests/test_orphan.py
@@ -34,7 +34,7 @@ def _create_item(self, category, name, data, metadata, parent_category, parent_n
             parent_location = self.course.location.replace(category=parent_category, name=parent_name)
             parent = editable_modulestore('direct').get_item(parent_location)
             parent.children.append(location.url())
-            editable_modulestore('direct').update_item(parent, self.user.pk)
+            editable_modulestore('direct').update_item(parent, self.user.id)
 
     def test_mongo_orphan(self):
         """
diff --git a/cms/djangoapps/contentstore/tests/test_textbooks.py b/cms/djangoapps/contentstore/tests/test_textbooks.py
index aab4ccc39989..7296ab851a07 100644
--- a/cms/djangoapps/contentstore/tests/test_textbooks.py
+++ b/cms/djangoapps/contentstore/tests/test_textbooks.py
@@ -59,7 +59,7 @@ def test_view_index_xhr_content(self):
         ]
         self.course.pdf_textbooks = content
         store = get_modulestore(self.course.location)
-        store.update_item(self.course, self.user.pk)
+        store.update_item(self.course, self.user.id)
 
         resp = self.client.get(
             self.url,
@@ -197,7 +197,7 @@ def setUp(self):
         # MongoKeyValueStore before we update the mongo datastore.
         self.course.save()
         self.store = get_modulestore(self.course.location)
-        self.store.update_item(self.course, self.user.pk)
+        self.store.update_item(self.course, self.user.id)
         self.url_nonexist = self.course_locator.url_reverse("textbooks", "20")
 
     def test_get_1(self):
diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/tests/test_transcripts.py
index 57e30ed9db1e..138b60b51322 100644
--- a/cms/djangoapps/contentstore/tests/test_transcripts.py
+++ b/cms/djangoapps/contentstore/tests/test_transcripts.py
@@ -67,7 +67,7 @@ def setUp(self):
         # hI10vDNYz4M - valid Youtube ID with transcripts.
         # JMD_ifUUfsU, AKqURZnYqpk, DYpADpL7jAY - valid Youtube IDs without transcripts.
         self.item.data = '<video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY" />'
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         self.item = modulestore().get_item(self.item_location)
         # Remove all transcripts for current module.
@@ -137,7 +137,7 @@ def test_success_video_module_source_subs_uploading(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """)
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         link = reverse('upload_transcripts')
         filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
@@ -214,7 +214,7 @@ def test_fail_for_non_video_module(self):
         item_locator, item_location = self._get_locator(resp)
         item = modulestore().get_item(item_location)
         item.data = '<non_video youtube="0.75:JMD_ifUUfsU,1.0:hI10vDNYz4M" />'
-        modulestore().update_item(item, 'test_transcripts')
+        modulestore().update_item(item, self.user.id)
 
         # non_video module: testing
 
@@ -234,7 +234,7 @@ def test_fail_for_non_video_module(self):
 
     def test_fail_bad_xml(self):
         self.item.data = '<<<video youtube="0.75:JMD_ifUUfsU,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY" />'
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         link = reverse('upload_transcripts')
         filename = os.path.splitext(os.path.basename(self.good_srt_file.name))[0]
@@ -346,7 +346,7 @@ def remove_subs_from_store(self, subs_id):
 
     def test_success_download_youtube(self):
         self.item.data = '<video youtube="1:JMD_ifUUfsU" />'
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
@@ -373,7 +373,7 @@ def test_success_download_nonyoutube(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """.format(subs_id))
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
@@ -433,7 +433,7 @@ def test_fail_for_non_video_module(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </videoalpha>
         """.format(subs_id))
-        modulestore().update_item(item, 'test_transcripts')
+        modulestore().update_item(item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
@@ -458,7 +458,7 @@ def test_fail_nonyoutube_subs_dont_exist(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """)
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         link = reverse('download_transcripts')
         resp = self.client.get(link, {'locator': self.item_locator})
@@ -472,7 +472,7 @@ def test_empty_youtube_attr_and_sub_attr(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """)
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         link = reverse('download_transcripts')
         resp = self.client.get(link, {'locator': self.item_locator})
@@ -488,7 +488,7 @@ def test_fail_bad_sjson_subs(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """.format(subs_id))
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
@@ -541,7 +541,7 @@ def test_success_download_nonyoutube(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </video>
         """.format(subs_id))
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
@@ -585,7 +585,7 @@ def test_success_download_nonyoutube(self):
 
     def test_check_youtube(self):
         self.item.data = '<video youtube="1:JMD_ifUUfsU" />'
-        modulestore().update_item(self.item, 'test_transcripts')
+        modulestore().update_item(self.item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
@@ -684,7 +684,7 @@ def test_fail_for_non_video_module(self):
                 <source src="http://www.quirksmode.org/html5/videos/big_buck_bunny.ogv"/>
             </videoalpha>
         """.format(subs_id))
-        modulestore().update_item(item, 'test_transcript')
+        modulestore().update_item(item, self.user.id)
 
         subs = {
             'start': [100, 200, 240],
diff --git a/cms/djangoapps/contentstore/transcripts_utils.py b/cms/djangoapps/contentstore/transcripts_utils.py
index 03aa46fcd54d..48c7d3d690f2 100644
--- a/cms/djangoapps/contentstore/transcripts_utils.py
+++ b/cms/djangoapps/contentstore/transcripts_utils.py
@@ -280,16 +280,16 @@ def generate_srt_from_sjson(sjson_subs, speed):
     return output
 
 
-def save_module(item):
+def save_module(item, user):
     """
     Proceed with additional save operations.
     """
     item.save()
     store = get_modulestore(Location(item.id))
-    store.update_item(item, 'save_module')
+    store.update_item(item, user.id if user else None)
 
 
-def copy_or_rename_transcript(new_name, old_name, item, delete_old=False):
+def copy_or_rename_transcript(new_name, old_name, item, delete_old=False, user=None):
     """
     Renames `old_name` transcript file in storage to `new_name`.
 
@@ -303,12 +303,12 @@ def copy_or_rename_transcript(new_name, old_name, item, delete_old=False):
     transcripts = contentstore().find(content_location).data
     save_subs_to_store(json.loads(transcripts), new_name, item)
     item.sub = new_name
-    save_module(item)
+    save_module(item, user)
     if delete_old:
         remove_subs_from_store(old_name, item)
 
 
-def manage_video_subtitles_save(old_item, new_item):
+def manage_video_subtitles_save(old_item, new_item, user):
     """
     Does some specific things, that can be done only on save.
 
@@ -340,7 +340,7 @@ def manage_video_subtitles_save(old_item, new_item):
         # copy_or_rename_transcript changes item.sub of module
         try:
             # updates item.sub with `video_id`, if it is successful.
-            copy_or_rename_transcript(video_id, sub_name, new_item)
+            copy_or_rename_transcript(video_id, sub_name, new_item, user=user)
         except NotFoundError:
             # subtitles file `sub_name` is not presented in the system. Nothing to copy or rename.
             log.debug(
diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py
index 2068d3ace6b6..aafecc2b0e58 100644
--- a/cms/djangoapps/contentstore/views/checklist.py
+++ b/cms/djangoapps/contentstore/views/checklist.py
@@ -51,7 +51,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
         # from the template.
         if not course_module.checklists:
             course_module.checklists = CourseDescriptor.checklists.default
-            modulestore.update_item(course_module, request.user.username)
+            modulestore.update_item(course_module, request.user.id)
 
         expanded_checklists = expand_all_action_urls(course_module)
         if json_request:
@@ -80,7 +80,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_
             # not default
             course_module.checklists = course_module.checklists
             course_module.save()
-            modulestore.update_item(course_module, request.user.username)
+            modulestore.update_item(course_module, request.user.id)
             expanded_checklist = expand_checklist_action_url(course_module, persisted_checklist)
             return JsonResponse(expanded_checklist)
         else:
diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py
index f06ed195016c..eb4c65bfdaa2 100644
--- a/cms/djangoapps/contentstore/views/course.py
+++ b/cms/djangoapps/contentstore/views/course.py
@@ -330,7 +330,7 @@ def create_new_course(request):
         definition_data=overview_template.get('data')
     )
 
-    initialize_course_tabs(new_course)
+    initialize_course_tabs(new_course, request.user)
 
     new_location = loc_mapper().translate_location(new_course.location.course_id, new_course.location, False, True)
     # can't use auth.add_users here b/c it requires request.user to already have Instructor perms in this course
@@ -416,7 +416,7 @@ def course_info_update_handler(request, tag=None, package_id=None, branch=None,
         return JsonResponse(get_course_updates(updates_location, provided_id))
     elif request.method == 'DELETE':
         try:
-            return JsonResponse(delete_course_update(updates_location, request.json, provided_id))
+            return JsonResponse(delete_course_update(updates_location, request.json, provided_id, request.user))
         except:
             return HttpResponseBadRequest(
                 "Failed to delete",
@@ -425,7 +425,7 @@ def course_info_update_handler(request, tag=None, package_id=None, branch=None,
     # can be either and sometimes django is rewriting one to the other:
     elif request.method in ('POST', 'PUT'):
         try:
-            return JsonResponse(update_course_updates(updates_location, request.json, provided_id))
+            return JsonResponse(update_course_updates(updates_location, request.json, provided_id, request.user))
         except:
             return HttpResponseBadRequest(
                 "Failed to save",
@@ -478,7 +478,7 @@ def settings_handler(request, tag=None, package_id=None, branch=None, version_gu
             )
         else:  # post or put, doesn't matter.
             return JsonResponse(
-                CourseDetails.update_from_json(locator, request.json),
+                CourseDetails.update_from_json(locator, request.json, request.user),
                 encoder=CourseSettingsEncoder
             )
 
@@ -525,15 +525,15 @@ def grading_handler(request, tag=None, package_id=None, branch=None, version_gui
             # None implies update the whole model (cutoffs, graceperiod, and graders) not a specific grader
             if grader_index is None:
                 return JsonResponse(
-                    CourseGradingModel.update_from_json(locator, request.json),
+                    CourseGradingModel.update_from_json(locator, request.json, request.user),
                     encoder=CourseSettingsEncoder
                 )
             else:
                 return JsonResponse(
-                    CourseGradingModel.update_grader_from_json(locator, request.json)
+                    CourseGradingModel.update_grader_from_json(locator, request.json, request.user)
                 )
         elif request.method == "DELETE" and grader_index is not None:
-            CourseGradingModel.delete_grader(locator, grader_index)
+            CourseGradingModel.delete_grader(locator, grader_index, request.user)
             return JsonResponse()
 
 
@@ -624,7 +624,8 @@ def advanced_settings_handler(request, package_id=None, branch=None, version_gui
                 return JsonResponse(CourseMetadata.update_from_json(
                     course_module,
                     request.json,
-                    filter_tabs=filter_tabs
+                    filter_tabs=filter_tabs,
+                    user=request.user
                 ))
             except (TypeError, ValueError) as err:
                 return HttpResponseBadRequest(
@@ -742,7 +743,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers
         if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs):
             course.tabs.append({"type": "pdf_textbooks"})
         course.pdf_textbooks = textbooks
-        store.update_item(course, request.user.username)
+        store.update_item(course, request.user.id)
         return JsonResponse(course.pdf_textbooks)
     elif request.method == 'POST':
         # create a new textbook for the course
@@ -760,7 +761,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers
             tabs = course.tabs
             tabs.append({"type": "pdf_textbooks"})
             course.tabs = tabs
-        store.update_item(course, request.user.username)
+        store.update_item(course, request.user.id)
         resp = JsonResponse(textbook, status=201)
         resp["Location"] = locator.url_reverse('textbooks', textbook["id"])
         return resp
@@ -811,7 +812,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
             course.pdf_textbooks = new_textbooks
         else:
             course.pdf_textbooks.append(new_textbook)
-        store.update_item(course, request.user.username)
+        store.update_item(course, request.user.id)
         return JsonResponse(new_textbook, status=201)
     elif request.method == 'DELETE':
         if not textbook:
@@ -820,7 +821,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non
         new_textbooks = course.pdf_textbooks[0:i]
         new_textbooks.extend(course.pdf_textbooks[i + 1:])
         course.pdf_textbooks = new_textbooks
-        store.update_item(course, request.user.username)
+        store.update_item(course, request.user.id)
         return JsonResponse()
 
 
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index de8177974988..1271c0884a80 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/cms/djangoapps/contentstore/views/item.py
@@ -159,7 +159,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
             delete_children = str_to_bool(request.REQUEST.get('recurse', 'False'))
             delete_all_versions = str_to_bool(request.REQUEST.get('all_versions', 'False'))
 
-            return _delete_item_at_location(old_location, delete_children, delete_all_versions)
+            return _delete_item_at_location(old_location, delete_children, delete_all_versions, request.user)
         else:  # Since we have a package_id, we are updating an existing xblock.
             return _save_item(
                 request,
@@ -184,7 +184,8 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
             dest_location = _duplicate_item(
                 parent_location,
                 duplicate_source_location,
-                request.json.get('display_name')
+                request.json.get('display_name'),
+                request.user
             )
             course_location = loc_mapper().translate_locator_to_location(BlockUsageLocator(parent_locator), get_course=True)
             dest_locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True)
@@ -271,10 +272,10 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
                     field.write_to(existing_item, value)
 
         if existing_item.category == 'video':
-            manage_video_subtitles_save(existing_item, existing_item)
+            manage_video_subtitles_save(existing_item, existing_item, request.user)
 
     # commit to datastore
-    store.update_item(existing_item, request.user)
+    store.update_item(existing_item, request.user.id)
 
     result = {
         'id': unicode(usage_loc),
@@ -283,7 +284,7 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta
     }
 
     if grader_type is not None:
-        result.update(CourseGradingModel.update_section_grader_type(existing_item, grader_type))
+        result.update(CourseGradingModel.update_section_grader_type(existing_item, grader_type, request.user))
 
     # Make public after updating the xblock, in case the caller asked
     # for both an update and a publish.
@@ -338,14 +339,14 @@ def _create_item(request):
     # TODO replace w/ nicer accessor
     if not 'detached' in parent.runtime.load_block_type(category)._class_tags:
         parent.children.append(dest_location.url())
-        get_modulestore(parent.location).update_item(parent, request.user)
+        get_modulestore(parent.location).update_item(parent, request.user.id)
 
     course_location = loc_mapper().translate_locator_to_location(parent_locator, get_course=True)
     locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True)
     return JsonResponse({"locator": unicode(locator)})
 
 
-def _duplicate_item(parent_location, duplicate_source_location, display_name=None):
+def _duplicate_item(parent_location, duplicate_source_location, display_name=None, user=None):
     """
     Duplicate an existing xblock as a child of the supplied parent_location.
     """
@@ -378,8 +379,8 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
     if source_item.has_children:
         dest_module.children = []
         for child in source_item.children:
-            dest_module.children.append(_duplicate_item(dest_location, Location(child)).url())
-        get_modulestore(dest_location).update_item(dest_module, 'user')
+            dest_module.children.append(_duplicate_item(dest_location, Location(child), user=user).url())
+        get_modulestore(dest_location).update_item(dest_module, user.id if user else None)
 
     if not 'detached' in source_item.runtime.load_block_type(category)._class_tags:
         parent = get_modulestore(parent_location).get_item(parent_location)
@@ -390,12 +391,12 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
             parent.children.insert(source_index + 1, dest_location.url())
         else:
             parent.children.append(dest_location.url())
-        get_modulestore(parent_location).update_item(parent, 'user')
+        get_modulestore(parent_location).update_item(parent, user.id if user else None)
 
     return dest_location
 
 
-def _delete_item_at_location(item_location, delete_children=False, delete_all_versions=False):
+def _delete_item_at_location(item_location, delete_children=False, delete_all_versions=False, user=None):
     """
     Deletes the item at with the given Location.
 
@@ -418,7 +419,7 @@ def _delete_item_at_location(item_location, delete_children=False, delete_all_ve
         for parent_loc in parent_locs:
             parent = modulestore('direct').get_item(parent_loc)
             parent.children.remove(item_url)
-            modulestore('direct').update_item(parent, 'user')
+            modulestore('direct').update_item(parent, user.id if user else None)
 
     return JsonResponse()
 
diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py
index f0c99ededb23..f3291ae92818 100644
--- a/cms/djangoapps/contentstore/views/tabs.py
+++ b/cms/djangoapps/contentstore/views/tabs.py
@@ -23,7 +23,7 @@
 __all__ = ['tabs_handler']
 
 
-def initialize_course_tabs(course):
+def initialize_course_tabs(course, user):
     """
     set up the default tabs
     I've added this because when we add static tabs, the LMS either expects a None for the tabs list or
@@ -47,7 +47,7 @@ def initialize_course_tabs(course):
         {"type": "progress", "name": _("Progress")},
     ]
 
-    modulestore('direct').update_item(course, 'system')
+    modulestore('direct').update_item(course, user.id)
 
 @expect_json
 @login_required
@@ -123,14 +123,14 @@ def get_location_for_tab(tab):
 
                 # OK, re-assemble the static tabs in the new order
                 course_item.tabs = reordered_tabs
-                modulestore('direct').update_item(course_item, request.user.username)
+                modulestore('direct').update_item(course_item, request.user.id)
                 return JsonResponse()
             else:
                 raise NotImplementedError('Creating or changing tab content is not supported.')
     elif request.method == 'GET':  # assume html
         # see tabs have been uninitialized (e.g. supporting courses created before tab support in studio)
         if course_item.tabs is None or len(course_item.tabs) == 0:
-            initialize_course_tabs(course_item)
+            initialize_course_tabs(course_item, request.user)
 
         # first get all static tabs from the tabs list
         # we do this because this is also the order in which items are displayed in the LMS
@@ -179,7 +179,7 @@ def primitive_delete(course, num):
     # Note for future implementations: if you delete a static_tab, then Chris Dodge
     # points out that there's other stuff to delete beyond this element.
     # This code happens to not delete static_tab so it doesn't come up.
-    modulestore('direct').update_item(course, 'primitive delete')
+    modulestore('direct').update_item(course, '**replace_user**')
 
 
 def primitive_insert(course, num, tab_type, name):
@@ -188,5 +188,5 @@ def primitive_insert(course, num, tab_type, name):
     new_tab = {u'type': unicode(tab_type), u'name': unicode(name)}
     tabs = course.tabs
     tabs.insert(num, new_tab)
-    modulestore('direct').update_item(course, 'primitive insert')
+    modulestore('direct').update_item(course, '**replace_user**')
 
diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py
index 03bc2067d1b9..b2ff9f8c4c77 100644
--- a/cms/djangoapps/contentstore/views/transcripts_ajax.py
+++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py
@@ -126,17 +126,17 @@ def upload_transcripts(request):
             video_name = video_dict['video']
             # We are creating transcripts for every video source,
             # for the case that in future, some of video sources can be deleted.
-            statuses[video_name] = copy_or_rename_transcript(video_name, sub_attr, item)
+            statuses[video_name] = copy_or_rename_transcript(video_name, sub_attr, item, user=request.user)
             try:
                 # updates item.sub with `video_name` if it is successful.
-                copy_or_rename_transcript(video_name, sub_attr, item)
+                copy_or_rename_transcript(video_name, sub_attr, item, user=request.user)
                 selected_name = video_name  # name to write to item.sub field, chosen at random.
             except NotFoundError:
                 # subtitles file `sub_attr` is not presented in the system. Nothing to copy or rename.
                 return error_response(response, "Can't find transcripts in storage for {}".format(sub_attr))
 
         item.sub = selected_name  # write one of  new subtitles names to item.sub attribute.
-        save_module(item)
+        save_module(item, request.user)
         response['subs'] = item.sub
         response['status'] = 'Success'
     else:
@@ -389,7 +389,7 @@ def choose_transcripts(request):
 
     if item.sub != html5_id:  # update sub value
         item.sub = html5_id
-        save_module(item)
+        save_module(item, request.user)
     response = {'status': 'Success',  'subs': item.sub}
     return JsonResponse(response)
 
@@ -420,7 +420,7 @@ def replace_transcripts(request):
         return error_response(response, e.message)
 
     item.sub = youtube_id
-    save_module(item)
+    save_module(item, request.user)
     response = {'status': 'Success',  'subs': item.sub}
     return JsonResponse(response)
 
@@ -483,7 +483,7 @@ def rename_transcripts(request):
     for new_name in videos['html5'].keys():  # copy subtitles for every HTML5 source
         try:
             # updates item.sub with new_name if it is successful.
-            copy_or_rename_transcript(new_name, old_name, item)
+            copy_or_rename_transcript(new_name, old_name, item, user=request.user)
         except NotFoundError:
             # subtitles file `item.sub` is not presented in the system. Nothing to copy or rename.
             error_response(response, "Can't find transcripts in storage for {}".format(old_name))
@@ -519,10 +519,10 @@ def save_transcripts(request):
         for metadata_key, value in metadata.items():
             setattr(item, metadata_key, value)
 
-        save_module(item)  # item becomes updated with new values
+        save_module(item, request.user)  # item becomes updated with new values
 
         if new_sub:
-            manage_video_subtitles_save(None, item)
+            manage_video_subtitles_save(None, item, request.user)
         else:
             # If `new_sub` is empty, it means that user explicitly does not want to use
             # transcripts for current video ids and we remove all transcripts from storage.
diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py
index dc842780ce80..eeb82c54a513 100644
--- a/cms/djangoapps/models/settings/course_details.py
+++ b/cms/djangoapps/models/settings/course_details.py
@@ -73,7 +73,7 @@ def fetch(cls, course_locator):
         return course
 
     @classmethod
-    def update_about_item(cls, course_old_location, about_key, data, course):
+    def update_about_item(cls, course_old_location, about_key, data, course, user):
         """
         Update the about item with the new data blob. If data is None, then
         delete the about item.
@@ -88,10 +88,10 @@ def update_about_item(cls, course_old_location, about_key, data, course):
             except ItemNotFoundError:
                 about_item = store.create_xmodule(temploc, system=course.runtime)
             about_item.data = data
-            store.update_item(about_item, 'update_about_item')
+            store.update_item(about_item, user.id)
 
     @classmethod
-    def update_from_json(cls, course_locator, jsondict):
+    def update_from_json(cls, course_locator, jsondict, user):
         """
         Decode the json into CourseDetails and save any changed attrs to the db
         """
@@ -146,15 +146,15 @@ def update_from_json(cls, course_locator, jsondict):
             dirty = True
 
         if dirty:
-            get_modulestore(course_old_location).update_item(descriptor, 'course_details')
+            get_modulestore(course_old_location).update_item(descriptor, user.id)
 
         # NOTE: below auto writes to the db w/o verifying that any of the fields actually changed
         # to make faster, could compare against db or could have client send over a list of which fields changed.
         for about_type in ['syllabus', 'overview', 'effort']:
-            cls.update_about_item(course_old_location, about_type, jsondict[about_type], descriptor)
+            cls.update_about_item(course_old_location, about_type, jsondict[about_type], descriptor, user)
 
         recomposed_video_tag = CourseDetails.recompose_video_tag(jsondict['intro_video'])
-        cls.update_about_item(course_old_location, 'video', recomposed_video_tag, descriptor)
+        cls.update_about_item(course_old_location, 'video', recomposed_video_tag, descriptor, user)
 
         # Could just return jsondict w/o doing any db reads, but I put the reads in as a means to confirm
         # it persisted correctly
diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py
index 5123b52f8ab5..45a1ebcc3d5f 100644
--- a/cms/djangoapps/models/settings/course_grading.py
+++ b/cms/djangoapps/models/settings/course_grading.py
@@ -52,7 +52,7 @@ def fetch_grader(course_location, index):
                     }
 
     @staticmethod
-    def update_from_json(course_locator, jsondict):
+    def update_from_json(course_locator, jsondict, user):
         """
         Decode the json into CourseGradingModel and save any changes. Returns the modified model.
         Probably not the usual path for updates as it's too coarse grained.
@@ -65,14 +65,14 @@ def update_from_json(course_locator, jsondict):
         descriptor.raw_grader = graders_parsed
         descriptor.grade_cutoffs = jsondict['grade_cutoffs']
 
-        get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
+        get_modulestore(course_old_location).update_item(descriptor, user.id)
 
-        CourseGradingModel.update_grace_period_from_json(course_locator, jsondict['grace_period'])
+        CourseGradingModel.update_grace_period_from_json(course_locator, jsondict['grace_period'], user)
 
         return CourseGradingModel.fetch(course_locator)
 
     @staticmethod
-    def update_grader_from_json(course_location, grader):
+    def update_grader_from_json(course_location, grader, user):
         """
         Create or update the grader of the given type (string key) for the given course. Returns the modified
         grader which is a full model on the client but not on the server (just a dict)
@@ -89,12 +89,12 @@ def update_grader_from_json(course_location, grader):
         else:
             descriptor.raw_grader.append(grader)
 
-        get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
+        get_modulestore(course_old_location).update_item(descriptor, user.id)
 
         return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index])
 
     @staticmethod
-    def update_cutoffs_from_json(course_location, cutoffs):
+    def update_cutoffs_from_json(course_location, cutoffs, user):
         """
         Create or update the grade cutoffs for the given course. Returns sent in cutoffs (ie., no extra
         db fetch).
@@ -103,12 +103,12 @@ def update_cutoffs_from_json(course_location, cutoffs):
         descriptor = get_modulestore(course_old_location).get_item(course_old_location)
         descriptor.grade_cutoffs = cutoffs
 
-        get_modulestore(course_old_location).update_item(descriptor, 'course_grading')
+        get_modulestore(course_old_location).update_item(descriptor, user.id)
 
         return cutoffs
 
     @staticmethod
-    def update_grace_period_from_json(course_location, graceperiodjson):
+    def update_grace_period_from_json(course_location, graceperiodjson, user):
         """
         Update the course's default grace period. Incoming dict is {hours: h, minutes: m} possibly as a
         grace_period entry in an enclosing dict. It is also safe to call this method with a value of
@@ -126,10 +126,10 @@ def update_grace_period_from_json(course_location, graceperiodjson):
             grace_timedelta = timedelta(**graceperiodjson)
             descriptor.graceperiod = grace_timedelta
 
-            get_modulestore(course_old_location).update_item(descriptor, 'update_grace_period')
+            get_modulestore(course_old_location).update_item(descriptor, user.id)
 
     @staticmethod
-    def delete_grader(course_location, index):
+    def delete_grader(course_location, index, user):
         """
         Delete the grader of the given type from the given course.
         """
@@ -142,10 +142,10 @@ def delete_grader(course_location, index):
             # force propagation to definition
             descriptor.raw_grader = descriptor.raw_grader
 
-        get_modulestore(course_old_location).update_item(descriptor, 'delete_grader')
+        get_modulestore(course_old_location).update_item(descriptor, user.id)
 
     @staticmethod
-    def delete_grace_period(course_location):
+    def delete_grace_period(course_location, user):
         """
         Delete the course's grace period.
         """
@@ -154,7 +154,7 @@ def delete_grace_period(course_location):
 
         del descriptor.graceperiod
 
-        get_modulestore(course_old_location).update_item(descriptor, 'delete_grace_period')
+        get_modulestore(course_old_location).update_item(descriptor, user.id)
 
     @staticmethod
     def get_section_grader_type(location):
@@ -166,7 +166,7 @@ def get_section_grader_type(location):
         }
 
     @staticmethod
-    def update_section_grader_type(descriptor, grader_type):
+    def update_section_grader_type(descriptor, grader_type, user):
         if grader_type is not None and grader_type != u'notgraded':
             descriptor.format = grader_type
             descriptor.graded = True
@@ -174,7 +174,7 @@ def update_section_grader_type(descriptor, grader_type):
             del descriptor.format
             del descriptor.graded
 
-        get_modulestore(descriptor.location).update_item(descriptor, 'update_grader')
+        get_modulestore(descriptor.location).update_item(descriptor, user.id)
         return {'graderType': grader_type}
 
     @staticmethod
diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py
index 88aa3c04e931..5b40aa5a115d 100644
--- a/cms/djangoapps/models/settings/course_metadata.py
+++ b/cms/djangoapps/models/settings/course_metadata.py
@@ -47,7 +47,7 @@ def fetch(cls, descriptor):
         return result
 
     @classmethod
-    def update_from_json(cls, descriptor, jsondict, filter_tabs=True):
+    def update_from_json(cls, descriptor, jsondict, filter_tabs=True, user=None):
         """
         Decode the json into CourseMetadata and save any changed attrs to the db.
 
@@ -77,6 +77,6 @@ def update_from_json(cls, descriptor, jsondict, filter_tabs=True):
                 setattr(descriptor, key, value)
 
         if dirty:
-            get_modulestore(descriptor.location).update_item(descriptor, 'update_settings')
+            get_modulestore(descriptor.location).update_item(descriptor, user.id if user else None)
 
         return cls.fetch(descriptor)
diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py
index 6397685c115a..01ef206cc9d1 100644
--- a/common/djangoapps/external_auth/tests/test_shib.py
+++ b/common/djangoapps/external_auth/tests/test_shib.py
@@ -330,7 +330,7 @@ def test_course_specific_login_and_reg(self):
         for domain in ["", "shib:https://idp.stanford.edu/"]:
             # set domains
             course.enrollment_domain = domain
-            self.store.update_item(course, 'test_shib')
+            self.store.update_item(course, '**replace_user**')
 
             # setting location to test that GET params get passed through
             login_request = self.request_factory.get('/course_specific_login/MITx/999/Robot_Super_Course' +
@@ -399,11 +399,11 @@ def test_enrollment_limit_by_domain(self):
         # create 2 course, one with limited enrollment one without
         shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
         shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        self.store.update_item(shib_course, 'test_enroll_limit')
+        self.store.update_item(shib_course, '**replace_user**')
 
         open_enroll_course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course')
         open_enroll_course.enrollment_domain = ''
-        self.store.update_item(open_enroll_course, 'test_enroll_limit')
+        self.store.update_item(open_enroll_course, '**replace_user**')
 
         # create 3 kinds of students, external_auth matching shib_course, external_auth not matching, no external auth
         shib_student = UserFactory.create()
@@ -469,7 +469,7 @@ def test_shib_login_enrollment(self):
 
         course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
         course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        self.store.update_item(course, 'test_shib')
+        self.store.update_item(course, '**replace_user**')
 
         # use django test client for sessions and url processing
         # no enrollment before trying
diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py
index b50d80d1ad9c..e6ea9e27091f 100644
--- a/common/djangoapps/student/tests/test_login.py
+++ b/common/djangoapps/student/tests/test_login.py
@@ -202,7 +202,7 @@ def setUp(self):
         self.course = CourseFactory.create(org='Stanford', number='456', display_name='NO SHIB')
         self.shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
         self.shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        self.store.update_item(self.shib_course, 'test_shib')
+        self.store.update_item(self.shib_course, '**replace_user**')
         self.user_w_map = UserFactory.create(email='withmap@stanford.edu')
         self.extauth = ExternalAuthMap(external_id='withmap@stanford.edu',
                                        external_email='withmap@stanford.edu',
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index f764ce6d9693..99a3e3bb3be2 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -413,16 +413,37 @@ class ModuleStoreWrite(ModuleStoreRead):
     __metaclass__ = ABCMeta
 
     @abstractmethod
-    def update_item(self, xblock, user, allow_not_found=False):
+    def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
         """
-        Update the given xblock's persisted repr
+        Update the given xblock's persisted repr. Pass the user's unique id which the persistent store
+        should save with the update if it has that ability.
+
+        :param allow_not_found: whether this method should raise an exception of the given xblock
+        has not been persisted before.
+
+        For version tracking and conflict detecting persistence stores
+
+        :raises VersionConflictError: if package_id and version_guid given and the current
+        version head != version_guid and force is not True.
+        :param force: fork the structure and don't update the course draftVersion if the above
+
         """
         pass
 
     @abstractmethod
     def delete_item(self, location, user_id=None, delete_all_versions=False, delete_children=False, force=False):
         """
-        Delete an item from persistence
+        Delete an item from persistence. Pass the user's unique id which the persistent store
+        should save with the update if it has that ability.
+
+        :param delete_all_versions: removes both the draft and published version of this item from
+        the course if using draft and old mongo. Split may or may not implement this.
+
+        For version tracking and conflict detecting persistence stores
+
+        :raises VersionConflictError: if package_id and version_guid given and the current
+        version head != version_guid and force is not True.
+        :param force: fork the structure and don't update the course draftVersion if the above
         """
         pass
 
diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py
index 7d8627160b5a..83b57108de8a 100644
--- a/common/lib/xmodule/xmodule/modulestore/mixed.py
+++ b/common/lib/xmodule/xmodule/modulestore/mixed.py
@@ -27,6 +27,7 @@ def __init__(self, mappings, stores, reference_type=None, **kwargs):
         """
         Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a
         collection of other modulestore configuration informations
+
         :param reference_type: either Location or Locator to indicate what type of reference this app
         uses.
         """
@@ -182,6 +183,7 @@ def converter(field, xblock):
     def has_item(self, course_id, reference):
         """
         Does the course include the xblock who's id is reference?
+
         :param course_id: a course_id or package_id (slashed or dotted)
         :param reference: a Location or BlockUsageLocator
         """
@@ -282,6 +284,7 @@ def get_course(self, course_id):
         """
         returns the course module associated with the course_id. If no such course exists,
         it returns None
+
         :param course_id: must be either a string course_id or a CourseLocator
         """
         store = self._get_modulestore_for_courseid(
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 7c77956aed30..357e45ca230f 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -716,7 +716,7 @@ def create_and_save_xmodule(self, location, definition_data=None, metadata=None,
             course.tabs = existing_tabs
             # Save any changes to the course to the MongoKeyValueStore
             course.save()
-            self.update_item(course, 'create_and_save_xmodule')
+            self.update_item(course, '**replace_user**')
 
     def fire_updated_modulestore_signal(self, course_id, location):
         """
@@ -833,7 +833,7 @@ def delete_item(self, location, **kwargs):
             course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != location.name]
             # Save the updates to the course to the MongoKeyValueStore
             course.save()
-            self.update_item(course, 'delete_item')
+            self.update_item(course, '**replace_user**')
 
         # Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False")
         # from overriding our default value set in the init method.
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index abb59e498347..d09a5afeba49 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -174,7 +174,7 @@ def update_item(self, xblock, user, allow_not_found=False):
 
         xblock.location = draft_loc
         super(DraftModuleStore, self).update_item(xblock, user)
-        # don't allow locations to truly represent thenmselves as draft outside of this file
+        # don't allow locations to truly represent themselves as draft outside of this file
         xblock.location = as_published(xblock.location)
 
     def delete_item(self, location, delete_all_versions=False, **kwargs):
@@ -220,7 +220,7 @@ def publish(self, location, published_by_id):
                         rents = [Location(mom) for mom in self.get_parent_locations(child, None)]
                         if (len(rents) == 1 and rents[0] == Location(location)):  # the 1 is this original_published
                             self.delete_item(child, True)
-        super(DraftModuleStore, self).update_item(draft, None)
+        super(DraftModuleStore, self).update_item(draft, '**replace_user**')
         self.delete_item(location)
 
     def unpublish(self, location):
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index ea89f7b4b77e..5d52a9bcc4df 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -57,8 +57,9 @@
 
 from xmodule.errortracker import null_error_tracker
 from xmodule.x_module import prefer_xmodules
-from xmodule.modulestore.locator import BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId, \
-    Locator
+from xmodule.modulestore.locator import (
+    BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId, Locator
+)
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError
 from xmodule.modulestore import inheritance, ModuleStoreWriteBase, Location, SPLIT_MONGO_MODULESTORE_TYPE
 
@@ -938,7 +939,7 @@ def create_course(
         self.db_connection.insert_course_index(index_entry)
         return self.get_course(CourseLocator(package_id=new_id, branch=master_branch))
 
-    def update_item(self, descriptor, user_id, force=False):
+    def update_item(self, descriptor, user_id, allow_not_found=False, force=False):
         """
         Save the descriptor's fields. it doesn't descend the course dag to save the children.
         Return the new descriptor (updated location).
diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
index 43e1b4ec0fd6..4f892264ecc1 100644
--- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py
+++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
@@ -142,7 +142,7 @@ def _clone_modules(modulestore, modules, source_location, dest_location):
 
             module.children = new_children
 
-        modulestore.update_item(module, '_clone_modules')
+        modulestore.update_item(module, '**replace_user**')
 
 
 def clone_course(modulestore, contentstore, source_location, dest_location, delete_original=False):
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
index 8211d71e2da5..2f1bdf6460a0 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
@@ -205,7 +205,7 @@ def update_course(course):
         to update metadata.
         """
         store = editable_modulestore('direct')
-        store.update_item(course, 'testuser')
+        store.update_item(course, '**replace_user**')
         updated_course = store.get_instance(course.id, course.location)
         return updated_course
 
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index 793792dd8d5a..4788c8a1d188 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -151,7 +151,7 @@ def _create(cls, target_class, **kwargs):
         # replace the display name with an optional parameter passed in from the caller
         if display_name is not None:
             metadata['display_name'] = display_name
-        module = store.create_and_save_xmodule(location, metadata=metadata, definition_data=data)
+        store.create_and_save_xmodule(location, metadata=metadata, definition_data=data)
 
         module = store.get_item(location)
 
@@ -163,6 +163,6 @@ def _create(cls, target_class, **kwargs):
 
         if 'detached' not in module._class_tags:
             parent.children.append(location.url())
-            store.update_item(parent, 'factory')
+            store.update_item(parent, '**replace_user**')
 
         return store.get_item(location)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
index 1f66d0438280..0061f3ec242a 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -201,7 +201,7 @@ def test_get_items(self):
     def test_update_item(self):
         # FIXME update
         with assert_raises(NotImplementedError):
-            self.store.update_item(self.fake_location, None)
+            self.store.update_item(self.fake_location, '**replace_user**')
 
     def test_delete_item(self):
         with assert_raises(NotImplementedError):
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
index 40a7d1a88f9d..9124eeffec84 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py
@@ -71,7 +71,7 @@ def _create_item(self, category, name, data, metadata, parent_category, parent_n
                 mongo = self.old_mongo
             else:
                 mongo = self.draft_mongo
-            mongo.update_item(parent, '_create_item')
+            mongo.update_item(parent, '**replace_user**')
 
     def _create_course(self):
         """
@@ -174,8 +174,8 @@ def test_publish_draft_delete(self):
         draft_vert.children.remove(other_child_loc.url())
         other_vert = self.draft_mongo.get_item(self.course_location.replace(category='vertical', name='Vert2'), 0)
         other_vert.children.append(other_child_loc.url())
-        self.draft_mongo.update_item(draft_vert, 'test_publish_delete')
-        self.draft_mongo.update_item(other_vert, 'test_publish_delete')
+        self.draft_mongo.update_item(draft_vert, '**replace_user**')
+        self.draft_mongo.update_item(other_vert, '**replace_user**')
         # publish
         self._xmodule_recurse(
             draft_vert,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
index 1e662c197b99..f57c347e112f 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
@@ -102,7 +102,7 @@ def create_source_course(self):
         location = location.replace(category='chapter', name=uuid.uuid4().hex)
         chapter2 = self._create_and_get_item(self.old_mongo, location, {}, {'display_name': 'Chapter 2'}, runtime)
         course_root.children.append(chapter2.location.url())
-        self.old_mongo.update_item(course_root, 'test_split_migrate')
+        self.old_mongo.update_item(course_root, '**replace_user**')
         # vertical in live only
         location = location.replace(category='vertical', name=uuid.uuid4().hex)
         live_vert = self._create_and_get_item(self.old_mongo, location, {}, {'display_name': 'Live vertical'}, runtime)
@@ -140,7 +140,7 @@ def create_source_course(self):
         self.create_random_units(self.old_mongo, live_vert)
 
         # update the chapter
-        self.old_mongo.update_item(chapter1, 'test_split_migrator')
+        self.old_mongo.update_item(chapter1, '**replace_user**')
 
         # now the other one w/ the conditional
         # first create some show children
@@ -169,7 +169,7 @@ def create_source_course(self):
         # add direct children
         self.create_random_units(self.old_mongo, conditional)
         chapter2.children.append(conditional.location.url())
-        self.old_mongo.update_item(chapter2, 'test_split_migrator')
+        self.old_mongo.update_item(chapter2, '**replace_user**')
 
         # and the ancillary docs (not children)
         location = location.replace(category='static_tab', name=uuid.uuid4().hex)
@@ -207,9 +207,9 @@ def create_random_units(self, store, parent, cc_store=None, cc_parent=None):
                     cc_store, location, data, {'display_name': str(uuid.uuid4())}, parent.runtime
                 )
                 cc_parent.children.append(element.location.url())
-        store.update_item(parent, 'test_split_migrator')
+        store.update_item(parent, '**replace_user**')
         if cc_store is not None:
-            cc_store.update_item(cc_parent, 'test_split_migrator')
+            cc_store.update_item(cc_parent, '**replace_user**')
 
     def compare_courses(self, presplit, published):
         # descend via children to do comparison
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
index aa3db34f72d4..653b2a2f7063 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
@@ -748,7 +748,7 @@ def test_update_metadata(self):
 
         problem.max_attempts = 4
         problem.save()  # decache above setting into the kvs
-        updated_problem = modulestore().update_item(problem, 'changeMaven')
+        updated_problem = modulestore().update_item(problem, '**replace_user**')
         # check that course version changed and course's previous is the other one
         self.assertEqual(updated_problem.definition_locator.definition_id, pre_def_id)
         self.assertNotEqual(updated_problem.location.version_guid, pre_version_guid)
@@ -767,7 +767,7 @@ def test_update_metadata(self):
         history_info = modulestore().get_course_history_info(current_course.location)
         self.assertEqual(history_info['previous_version'], pre_version_guid)
         self.assertEqual(str(history_info['original_version']), self.GUID_D3)
-        self.assertEqual(history_info['edited_by'], "changeMaven")
+        self.assertEqual(history_info['edited_by'], "**replace_user**")
         self.assertGreaterEqual(history_info['edited_on'], premod_time)
         self.assertLessEqual(history_info['edited_on'], datetime.datetime.now(UTC))
 
@@ -784,7 +784,7 @@ def test_update_children(self):
         self.assertGreater(len(block.children), 0, "meaningless test")
         moved_child = block.children.pop()
         block.save()  # decache model changes
-        updated_problem = modulestore().update_item(block, 'childchanger')
+        updated_problem = modulestore().update_item(block, '**replace_user**')
         # check that course version changed and course's previous is the other one
         self.assertEqual(updated_problem.definition_locator.definition_id, pre_def_id)
         self.assertNotEqual(updated_problem.location.version_guid, pre_version_guid)
@@ -794,7 +794,7 @@ def test_update_children(self):
         other_block = modulestore().get_item(locator)
         other_block.children.append(moved_child)
         other_block.save()  # decache model changes
-        other_updated = modulestore().update_item(other_block, 'childchanger')
+        other_updated = modulestore().update_item(other_block, '**replace_user**')
         self.assertIn(moved_child, other_updated.children)
 
     def test_update_definition(self):
@@ -808,7 +808,7 @@ def test_update_definition(self):
 
         block.grading_policy['GRADER'][0]['min_count'] = 13
         block.save()  # decache model changes
-        updated_block = modulestore().update_item(block, 'definition_changer')
+        updated_block = modulestore().update_item(block, '**replace_user**')
 
         self.assertNotEqual(updated_block.definition_locator.definition_id, pre_def_id)
         self.assertNotEqual(updated_block.location.version_guid, pre_version_guid)
@@ -846,7 +846,7 @@ def test_update_manifold(self):
         block.advertised_start = "Soon"
 
         block.save()  # decache model changes
-        updated_block = modulestore().update_item(block, "test_update_manifold")
+        updated_block = modulestore().update_item(block, "**replace_user**")
         self.assertNotEqual(updated_block.definition_locator.definition_id, pre_def_id)
         self.assertNotEqual(updated_block.location.version_guid, pre_version_guid)
         self.assertEqual(updated_block.grading_policy['GRADER'][0]['min_count'], 13)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 5815a459bc2a..d4b21920dfed 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -4,14 +4,13 @@
 from path import path
 import json
 
-from xblock.fields import Scope
-
 from .xml import XMLModuleStore, ImportSystem, ParentTracker
 from xmodule.modulestore import Location
 from xmodule.contentstore.content import StaticContent
 from .inheritance import own_metadata
 from xmodule.errortracker import make_error_tracker
 from .store_utilities import rewrite_nonportable_content_links
+import xblock
 
 log = logging.getLogger(__name__)
 
@@ -312,7 +311,7 @@ def import_module(
 
     logging.debug('processing import of module {}...'.format(module.location.url()))
 
-    if 'data' in module.fields and do_import_static:
+    if do_import_static and 'data' in module.fields and isinstance(module.fields['data'], xblock.fields.String):
         # we want to convert all 'non-portable' links in the module_data
         # (if it is a string) to portable strings (e.g. /static/)
         module.data = rewrite_nonportable_content_links(
@@ -327,7 +326,7 @@ def import_module(
     if 'index_in_children_list' in getattr(module, 'xml_attributes', []):
         del module.xml_attributes['index_in_children_list']
 
-    store.update_item(module, None, allow_not_found=allow_not_found)
+    store.update_item(module, '**replace_user**', allow_not_found=allow_not_found)
 
 
 def import_course_draft(
@@ -372,7 +371,7 @@ def import_course_draft(
     # First it is necessary to order the draft items by their desired index in the child list
     # (order os.walk returns them in is not guaranteed).
     drafts = dict()
-    for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"):
+    for dirname, _dirnames, filenames in os.walk(draft_dir + "/vertical"):
         for filename in filenames:
             module_path = os.path.join(dirname, filename)
             with open(module_path, 'r') as f:
@@ -459,7 +458,7 @@ def _import_module(module):
 
                             if non_draft_location.url() not in sequential.children:
                                 sequential.children.insert(index, non_draft_location.url())
-                                store.update_item(sequential, 'importer')
+                                store.update_item(sequential, '**replace_user**')
 
                         import_module(
                             module, draft_store, course_data_path,
diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py
index 662aec3910cd..e81b4b8a2db0 100644
--- a/lms/djangoapps/courseware/tests/test_submitting_problems.py
+++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py
@@ -231,7 +231,7 @@ def add_grading_policy(self, grading_policy):
 
         self.course.grading_policy = grading_policy
         store = editable_modulestore('direct')
-        store.update_item(self.course, 'add_grading_policy')
+        store.update_item(self.course, '**replace_user**')
         self.refresh_course()
 
     def get_grade_summary(self):
diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py
index d9b1bbf82552..18a8287eee74 100644
--- a/lms/djangoapps/courseware/views.py
+++ b/lms/djangoapps/courseware/views.py
@@ -1,7 +1,6 @@
 import logging
 import urllib
 
-from functools import partial
 from collections import defaultdict
 
 from django.conf import settings
@@ -10,7 +9,7 @@
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
 from django.contrib.auth.decorators import login_required
-from django.http import Http404, HttpResponse, HttpResponseRedirect
+from django.http import Http404, HttpResponse
 from django.shortcuts import redirect
 from edxmako.shortcuts import render_to_response, render_to_string
 from django_future.csrf import ensure_csrf_cookie
@@ -240,7 +239,7 @@ def index(request, course_id, chapter=None, section=None,
     registered = registered_for_course(course, user)
     if not registered:
         # TODO (vshnayder): do course instructors need to be registered to see course?
-        log.debug('User %s tried to view course %s but is not enrolled', user, course.location.url())
+        log.debug(u'User %s tried to view course %s but is not enrolled', user, course.location.url())
         return redirect(reverse('about_course', args=[course.id]))
 
     masq = setup_masquerade(request, staff_access)
diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py
index b6f9b9df8d2d..ea5eeb169075 100644
--- a/lms/djangoapps/instructor_task/tests/test_base.py
+++ b/lms/djangoapps/instructor_task/tests/test_base.py
@@ -210,7 +210,7 @@ def redefine_option_problem(self, problem_url_name):
         location = InstructorTaskTestCase.problem_location(problem_url_name)
         item = self.module_store.get_instance(self.course.id, location)
         item.data = problem_xml
-        self.module_store.update_item(item, 'redefine_option_problem')
+        self.module_store.update_item(item, '**replace_user**')
 
     def get_student_module(self, username, descriptor):
         """Get StudentModule object for test course, given the `username` and the problem's `descriptor`."""
diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py
index 056a89b20208..bd2ab2516063 100644
--- a/lms/djangoapps/instructor_task/tests/test_integration.py
+++ b/lms/djangoapps/instructor_task/tests/test_integration.py
@@ -292,7 +292,7 @@ def check_func(expect, answer_given):
                 self.course.id, InstructorTaskModuleTestCase.problem_location(problem_url_name)
             )
             descriptor.data = problem_xml
-            self.module_store.update_item(descriptor, 'define_randomized_custom_response_problem')
+            self.module_store.update_item(descriptor, '**replace_user**')
         else:
             # Use "per-student" rerandomization so that check-problem can be called more than once.
             # Using "always" means we cannot check a problem twice, but we want to call once to get the

From 1de9d558447fc8e711f1c180ce76a6fd83877c05 Mon Sep 17 00:00:00 2001
From: Don Mitchell <dmitchell@edx.org>
Date: Wed, 5 Feb 2014 16:34:24 -0500
Subject: [PATCH 4/4] Review-driven changes (to-be-squashed)

---
 .../tests/test_course_settings.py             | 29 +++---
 cms/djangoapps/contentstore/views/course.py   |  2 +-
 cms/djangoapps/contentstore/views/item.py     |  9 +-
 .../xmodule/xmodule/modulestore/__init__.py   | 17 ++--
 .../lib/xmodule/xmodule/modulestore/mixed.py  | 97 ++++++++++---------
 .../xmodule/xmodule/modulestore/mongo/base.py | 19 ++--
 .../xmodule/modulestore/mongo/draft.py        |  4 +-
 .../xmodule/modulestore/split_mongo/split.py  |  2 +-
 .../xmodule/modulestore/store_utilities.py    |  3 +-
 .../modulestore/tests/test_locators.py        |  2 +-
 .../tests/test_view_authentication.py         |  2 +-
 11 files changed, 92 insertions(+), 94 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py
index e6cae27619d7..9b33654b70f8 100644
--- a/cms/djangoapps/contentstore/tests/test_course_settings.py
+++ b/cms/djangoapps/contentstore/tests/test_course_settings.py
@@ -120,8 +120,8 @@ def test_marketing_site_fetch(self):
 
             self.assertContains(response, "Introducing Your Course")
             self.assertContains(response, "Course Image")
-            self.assertNotContains(response,"Course Overview")
-            self.assertNotContains(response,"Course Introduction Video")
+            self.assertNotContains(response, "Course Overview")
+            self.assertNotContains(response, "Course Introduction Video")
             self.assertNotContains(response, "Requirements")
 
     def test_regular_site_fetch(self):
@@ -141,8 +141,8 @@ def test_regular_site_fetch(self):
 
             self.assertContains(response, "Introducing Your Course")
             self.assertContains(response, "Course Image")
-            self.assertContains(response,"Course Overview")
-            self.assertContains(response,"Course Introduction Video")
+            self.assertContains(response, "Course Overview")
+            self.assertContains(response, "Course Introduction Video")
             self.assertContains(response, "Requirements")
 
 
@@ -259,7 +259,8 @@ def test_update_from_json(self):
     def test_update_grader_from_json(self):
         test_grader = CourseGradingModel.fetch(self.course_locator)
         altered_grader = CourseGradingModel.update_grader_from_json(
-            self.course_locator, test_grader.graders[1], self.user)
+            self.course_locator, test_grader.graders[1], self.user
+        )
         self.assertDictEqual(test_grader.graders[1], altered_grader, "Noop update")
 
         test_grader.graders[1]['min_count'] = test_grader.graders[1].get('min_count') + 2
@@ -293,7 +294,8 @@ def test_update_cutoffs_from_json(self):
     def test_delete_grace_period(self):
         test_grader = CourseGradingModel.fetch(self.course_locator)
         CourseGradingModel.update_grace_period_from_json(
-            self.course_locator, test_grader.grace_period, self.user)
+            self.course_locator, test_grader.grace_period, self.user
+        )
         # update_grace_period_from_json doesn't return anything, so query the db for its contents.
         altered_grader = CourseGradingModel.fetch(self.course_locator)
         self.assertEqual(test_grader.grace_period, altered_grader.grace_period, "Noop update")
@@ -445,9 +447,10 @@ def test_fetch_initial_fields(self):
 
     def test_update_from_json(self):
         test_model = CourseMetadata.update_from_json(
-            self.course, {
+            self.course,
+            {
                 "advertised_start": "start A",
-                "days_early_for_beta": 2
+                "days_early_for_beta": 2,
             },
             user=self.user
          )
@@ -458,9 +461,10 @@ def test_update_from_json(self):
         self.update_check(test_model)
         # now change some of the existing metadata
         test_model = CourseMetadata.update_from_json(
-            fresh, {
+            fresh,
+            {
                 "advertised_start": "start B",
-                "display_name": "jolly roger"
+                "display_name": "jolly roger",
             },
             user=self.user
         )
@@ -479,9 +483,8 @@ def update_check(self, test_model):
 
     def test_delete_key(self):
         test_model = CourseMetadata.update_from_json(
-            self.fullcourse, {
-                "unsetKeys": ['showanswer', 'xqa_key']
-            },
+            self.fullcourse,
+            {"unsetKeys": ['showanswer', 'xqa_key']},
             user=self.user
         )
         # ensure no harm
diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py
index eb4c65bfdaa2..e539edf999c0 100644
--- a/cms/djangoapps/contentstore/views/course.py
+++ b/cms/djangoapps/contentstore/views/course.py
@@ -625,7 +625,7 @@ def advanced_settings_handler(request, package_id=None, branch=None, version_gui
                     course_module,
                     request.json,
                     filter_tabs=filter_tabs,
-                    user=request.user
+                    user=request.user,
                 ))
             except (TypeError, ValueError) as err:
                 return HttpResponseBadRequest(
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index 1271c0884a80..372ba7596943 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/cms/djangoapps/contentstore/views/item.py
@@ -9,7 +9,6 @@
 from static_replace import replace_static_urls
 from xmodule_modifiers import wrap_xblock
 
-from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.contrib.auth.decorators import login_required
 from django.http import HttpResponseBadRequest, HttpResponse
@@ -18,9 +17,8 @@
 
 from xblock.fields import Scope
 from xblock.fragment import Fragment
-from xblock.core import XBlock
 
-import xmodule.x_module
+import xmodule
 from xmodule.modulestore.django import modulestore, loc_mapper
 from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 from xmodule.modulestore.inheritance import own_metadata
@@ -185,7 +183,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
                 parent_location,
                 duplicate_source_location,
                 request.json.get('display_name'),
-                request.user
+                request.user,
             )
             course_location = loc_mapper().translate_locator_to_location(BlockUsageLocator(parent_locator), get_course=True)
             dest_locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True)
@@ -379,7 +377,8 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non
     if source_item.has_children:
         dest_module.children = []
         for child in source_item.children:
-            dest_module.children.append(_duplicate_item(dest_location, Location(child), user=user).url())
+            dupe = _duplicate_item(dest_location, Location(child), user=user)
+            dest_module.children.append(dupe.url())
         get_modulestore(dest_location).update_item(dest_module, user.id if user else None)
 
     if not 'detached' in source_item.runtime.load_block_type(category)._class_tags:
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 99a3e3bb3be2..517295af0646 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -418,15 +418,13 @@ def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
         Update the given xblock's persisted repr. Pass the user's unique id which the persistent store
         should save with the update if it has that ability.
 
-        :param allow_not_found: whether this method should raise an exception of the given xblock
+        :param allow_not_found: whether this method should raise an exception if the given xblock
         has not been persisted before.
-
-        For version tracking and conflict detecting persistence stores
+        :param force: fork the structure and don't update the course draftVersion if there's a version
+        conflict (only applicable to version tracking and conflict detecting persistence stores)
 
         :raises VersionConflictError: if package_id and version_guid given and the current
-        version head != version_guid and force is not True.
-        :param force: fork the structure and don't update the course draftVersion if the above
-
+        version head != version_guid and force is not True. (only applicable to version tracking stores)
         """
         pass
 
@@ -438,12 +436,11 @@ def delete_item(self, location, user_id=None, delete_all_versions=False, delete_
 
         :param delete_all_versions: removes both the draft and published version of this item from
         the course if using draft and old mongo. Split may or may not implement this.
-
-        For version tracking and conflict detecting persistence stores
+        :param force: fork the structure and don't update the course draftVersion if there's a version
+        conflict (only applicable to version tracking and conflict detecting persistence stores)
 
         :raises VersionConflictError: if package_id and version_guid given and the current
-        version head != version_guid and force is not True.
-        :param force: fork the structure and don't update the course draftVersion if the above
+        version head != version_guid and force is not True. (only applicable to version tracking stores)
         """
         pass
 
diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py
index 83b57108de8a..e8fa955e16b5 100644
--- a/common/lib/xmodule/xmodule/modulestore/mixed.py
+++ b/common/lib/xmodule/xmodule/modulestore/mixed.py
@@ -59,67 +59,81 @@ def _get_modulestore_for_courseid(self, course_id):
         mapping = self.mappings.get(course_id, 'default')
         return self.modulestores[mapping]
 
-    def _incoming_reference_adaptor(self, store, course_id, reference):
+    def _locator_to_location(self, reference):
         """
-        Convert the reference to the type the persistence layer wants
+        Convert the referenced locator to a location casting to and from a string as necessary
+        """
+        stringify = isinstance(reference, basestring)
+        if stringify:
+            reference = BlockUsageLocator(url=reference)
+        location = loc_mapper().translate_locator_to_location(reference)
+        return location.url() if stringify else location
+
+    def _location_to_locator(self, course_id, reference):
+        """
+        Convert the referenced location to a locator casting to and from a string as necessary
         """
-        if issubclass(store.reference_type, Location if self.use_locations else Locator):
-            return reference
         stringify = isinstance(reference, basestring)
-        if store.reference_type == Location:
-            if stringify:
-                reference = BlockUsageLocator(url=reference)
-            location = loc_mapper().translate_locator_to_location(reference)
-            return location.url() if stringify else location
         if stringify:
             reference = Location(reference)
         locator = loc_mapper().translate_location(course_id, reference, reference.revision == 'draft', True)
         return unicode(locator) if stringify else locator
 
+    def _incoming_reference_adaptor(self, store, course_id, reference):
+        """
+        Convert the reference to the type the persistence layer wants
+        """
+        if issubclass(store.reference_type, Location if self.use_locations else Locator):
+            return reference
+        if store.reference_type == Location:
+            return self._locator_to_location(reference)
+        return self._location_to_locator(course_id, reference)
+
     def _outgoing_reference_adaptor(self, store, course_id, reference):
         """
         Convert the reference to the type the application wants
         """
         if issubclass(store.reference_type, Location if self.use_locations else Locator):
             return reference
-        stringify = isinstance(reference, basestring)
         if store.reference_type == Location:
-            if stringify:
-                reference = Location(reference)
-            locator = loc_mapper().translate_location(
-                course_id, reference, reference.revision == 'draft', True
-            )
-            return unicode(locator) if stringify else locator
-        else:
-            if stringify:
-                reference = BlockUsageLocator(url=reference)
-            location = loc_mapper().translate_locator_to_location(reference)
-            return location.url() if stringify else location
+            return self._location_to_locator(course_id, reference)
+        return self._locator_to_location(reference)
 
-    def _incoming_xblock_adaptor(self, store, course_id, xblock):
+    def _xblock_adaptor_iterator(self, adaptor, string_converter, store, course_id, xblock):
         """
-        Change all reference fields in this xblock to the type expected by the persistence layer
+        Change all reference fields in this xblock to the type expected by the receiving layer
         """
-        string_converter = self._get_string_converter(
-            course_id, store.reference_type, xblock.location
-        )
         for field in xblock.fields.itervalues():
             if field.is_set_on(xblock):
                 if isinstance(field, Reference):
                     field.write_to(
                         xblock,
-                        self._incoming_reference_adaptor(store, course_id, field.read_from(xblock)))
+                        adaptor(store, course_id, field.read_from(xblock))
+                    )
                 elif isinstance(field, ReferenceList):
                     field.write_to(
                         xblock,
-                        [self._incoming_reference_adaptor(store, course_id, ref)
-                         for ref in field.read_from(xblock)]
+                        [
+                            adaptor(store, course_id, ref)
+                            for ref in field.read_from(xblock)
+                        ]
                     )
-                elif string_converter is not None and isinstance(field, String):
+                elif isinstance(field, String):
                     # replace links within the string
                     string_converter(field, xblock)
         return xblock
 
+    def _incoming_xblock_adaptor(self, store, course_id, xblock):
+        """
+        Change all reference fields in this xblock to the type expected by the persistence layer
+        """
+        string_converter = self._get_string_converter(
+            course_id, store.reference_type, xblock.location
+        )
+        return self._xblock_adaptor_iterator(
+            self._incoming_reference_adaptor, string_converter, store, course_id, xblock
+        )
+
     def _outgoing_xblock_adaptor(self, store, course_id, xblock):
         """
         Change all reference fields in this xblock to the type expected by the persistence layer
@@ -127,22 +141,9 @@ def _outgoing_xblock_adaptor(self, store, course_id, xblock):
         string_converter = self._get_string_converter(
             course_id, xblock.location.__class__, xblock.location
         )
-        for field in xblock.fields.itervalues():
-            if field.is_set_on(xblock):
-                if isinstance(field, Reference):
-                    field.write_to(
-                        xblock,
-                        self._outgoing_reference_adaptor(store, course_id, field.read_from(xblock)))
-                elif isinstance(field, ReferenceList):
-                    field.write_to(
-                        xblock,
-                        [self._outgoing_reference_adaptor(store, course_id, ref)
-                         for ref in field.read_from(xblock)]
-                    )
-                elif string_converter is not None and isinstance(field, String):
-                    # replace links within the string
-                    string_converter(field, xblock)
-        return xblock
+        return self._xblock_adaptor_iterator(
+            self._outgoing_reference_adaptor, string_converter, store, course_id, xblock
+        )
 
     CONVERT_RE = re.compile(r"/jump_to_id/({}+)".format(ALLOWED_ID_CHARS))
 
@@ -152,9 +153,9 @@ def _get_string_converter(self, course_id, reference_type, from_base_addr):
         with the correct rewritten link for the target type
         """
         if self.use_locations and reference_type == Location:
-            return None
+            return lambda field, xblock: None
         if not self.use_locations and issubclass(reference_type, Locator):
-            return None
+            return lambda field, xblock: None
         if isinstance(from_base_addr, Location):
             def mapper(found_id):
                 """
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 357e45ca230f..1b26b9f46ed9 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -223,7 +223,6 @@ def namedtuple_to_son(namedtuple, prefix=''):
     # pylint: disable=protected-access
     for idx, field_name in enumerate(namedtuple._fields):
         son[prefix + field_name] = namedtuple[idx]
-    # pylint: enable=protected-access
     return son
 
 
@@ -256,6 +255,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
     """
     A Mongodb backed ModuleStore
     """
+    reference_type = Location
 
     # TODO (cpennington): Enable non-filesystem filestores
     # pylint: disable=C0103
@@ -313,7 +313,6 @@ def do_connection(
         self.error_tracker = error_tracker
         self.render_template = render_template
         self.ignore_write_events_on_courses = []
-        self.reference_type = Location
 
     def compute_metadata_inheritance_tree(self, location):
         '''
@@ -794,17 +793,17 @@ def update_item(self, xblock, user, allow_not_found=False):
                                    for child in xblock.children]
                 payload.update({'definition.children': xblock.children})
             self._update_single_item(xblock.location, payload)
+            # for static tabs, their containing course also records their display name
             if xblock.category == 'static_tab':
                 course = self._get_course_for_item(xblock.location)
-                existing_tabs = course.tabs or []
-                for tab in existing_tabs:
+                # find the course's reference to this tab and update the name.
+                for tab in course.tabs:
                     if tab.get('url_slug') == xblock.location.name:
-                        if xblock.fields['display_name'].is_set_on(xblock):
+                        # only update if changed
+                        if tab['name'] != xblock.display_name:
                             tab['name'] = xblock.display_name
-                        break
-                course.tabs = existing_tabs
-                # Save the updates to the course to the MongoKeyValueStore
-                self.update_item(course, user)
+                            self.update_item(course, user)
+                            break
 
             # recompute (and update) the metadata inheritance tree which is cached
             # was conditional on children or metadata having changed before dhm made one update to rule them all
@@ -831,8 +830,6 @@ def delete_item(self, location, **kwargs):
             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]
-            # Save the updates to the course to the MongoKeyValueStore
-            course.save()
             self.update_item(course, '**replace_user**')
 
         # Must include this to avoid the django debug toolbar (which defines the deprecated "safe=False")
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index d09a5afeba49..df31badf80bf 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -168,9 +168,9 @@ def update_item(self, xblock, user, allow_not_found=False):
         try:
             if not self.has_item(None, draft_loc):
                 self.convert_to_draft(xblock.location)
-        except ItemNotFoundError, e:
+        except ItemNotFoundError:
             if not allow_not_found:
-                raise e
+                raise
 
         xblock.location = draft_loc
         super(DraftModuleStore, self).update_item(xblock, user)
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 5d52a9bcc4df..6f0cba331854 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -100,6 +100,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
     A Mongodb backed ModuleStore supporting versions, inheritance,
     and sharing.
     """
+    reference_type = Locator
     def __init__(self, doc_store_config, fs_root, render_template,
                  default_class=None,
                  error_tracker=null_error_tracker,
@@ -111,7 +112,6 @@ def __init__(self, doc_store_config, fs_root, render_template,
 
         super(SplitMongoModuleStore, self).__init__(**kwargs)
         self.loc_mapper = loc_mapper
-        self.reference_type = Locator
 
         self.db_connection = MongoConnection(**doc_store_config)
         self.db = self.db_connection.database
diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
index 4f892264ecc1..bcb51908a7a8 100644
--- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py
+++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py
@@ -126,7 +126,8 @@ def _clone_modules(modulestore, modules, source_location, dest_location):
 
         if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring):
             module.data = rewrite_nonportable_content_links(
-                source_location.course_id, dest_location.course_id, module.data)
+                source_location.course_id, dest_location.course_id, module.data
+            )
 
         # repoint children
         if module.has_children:
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
index 5ec34ae801bb..047ee65dbbd5 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py
@@ -261,7 +261,7 @@ def test_colon_name(self):
 
     def test_relative(self):
         """
-        It seems we used to use colons in names; so, ensure they're acceptable.
+        Test making a relative usage locator.
         """
         package_id = 'mit.eecs-1'
         branch = 'foo'
diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py
index e7fadb006fec..6d92a1a1c2b5 100644
--- a/lms/djangoapps/courseware/tests/test_view_authentication.py
+++ b/lms/djangoapps/courseware/tests/test_view_authentication.py
@@ -85,7 +85,7 @@ def _check_staff(self, course):
         urls.extend([
             reverse('book', kwargs={'course_id': course.id,
                                     'book_index': index})
-            for index, _book in enumerate(course.textbooks)
+            for index in xrange(len(course.textbooks))
         ])
         for url in urls:
             check_for_get_code(self, 200, url)