update_item and other refactorings continued

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)
 

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -54,7 +54,7 @@ class ChecklistTestCase(CourseTestCase):
         # 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)

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -124,7 +124,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
                                            '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 @@ class ContentStoreTest(ModuleStoreTestCase):
         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 @@ class ContentStoreTest(ModuleStoreTestCase):
         # 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 @@ class MetadataSaveTestCase(ModuleStoreTestCase):
             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))

cms/djangoapps/contentstore/tests/test_course_updates.py

@@ -123,7 +123,7 @@ class CourseUpdateTest(CourseTestCase):
         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>'

cms/djangoapps/contentstore/tests/test_crud.py

@@ -172,7 +172,7 @@ class TemplateTests(unittest.TestCase):
         )
         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)

cms/djangoapps/contentstore/tests/test_orphan.py

@@ -34,7 +34,7 @@ class TestOrphan(CourseTestCase):
             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):
         """

cms/djangoapps/contentstore/tests/test_textbooks.py

@@ -59,7 +59,7 @@ class TextbookIndexTestCase(CourseTestCase):
         ]
         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 @@ class TextbookDetailTestCase(CourseTestCase):
         # 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):

cms/djangoapps/contentstore/tests/test_transcripts.py

@@ -67,7 +67,7 @@ class Basetranscripts(CourseTestCase):
         # 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 @@ class TestUploadtranscripts(Basetranscripts):
                 <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 @@ class TestUploadtranscripts(Basetranscripts):
         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 @@ class TestUploadtranscripts(Basetranscripts):
 
     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 @@ class TestDownloadtranscripts(Basetranscripts):
 
     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 @@ class TestDownloadtranscripts(Basetranscripts):
                 <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 @@ class TestDownloadtranscripts(Basetranscripts):
                 <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 @@ class TestDownloadtranscripts(Basetranscripts):
                 <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 @@ class TestDownloadtranscripts(Basetranscripts):
                 <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 @@ class TestDownloadtranscripts(Basetranscripts):
                 <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 @@ class TestChecktranscripts(Basetranscripts):
                 <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 @@ class TestChecktranscripts(Basetranscripts):
 
     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 @@ class TestChecktranscripts(Basetranscripts):
                 <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],

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(

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:

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

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

cms/djangoapps/contentstore/views/tabs.py

@@ -23,7 +23,7 @@ from django.utils.translation import ugettext as _
 __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 tabs_handler(request, tag=None, package_id=None, branch=None, version_guid=N
 
                 # 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**')
 

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.

cms/djangoapps/models/settings/course_details.py

@@ -73,7 +73,7 @@ class CourseDetails(object):
         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 @@ class CourseDetails(object):
             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 @@ class CourseDetails(object):
             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

cms/djangoapps/models/settings/course_grading.py

@@ -52,7 +52,7 @@ class CourseGradingModel(object):
                     }
 
     @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 @@ class CourseGradingModel(object):
         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 @@ class CourseGradingModel(object):
         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 @@ class CourseGradingModel(object):
         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 @@ class CourseGradingModel(object):
             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 @@ class CourseGradingModel(object):
             # 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 @@ class CourseGradingModel(object):
 
         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 @@ class CourseGradingModel(object):
         }
 
     @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 @@ class CourseGradingModel(object):
             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

cms/djangoapps/models/settings/course_metadata.py

@@ -47,7 +47,7 @@ class CourseMetadata(object):
         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 @@ class CourseMetadata(object):
                 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)

common/djangoapps/external_auth/tests/test_shib.py

@@ -330,7 +330,7 @@ class ShibSPTest(ModuleStoreTestCase):
         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 @@ class ShibSPTest(ModuleStoreTestCase):
         # 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 @@ class ShibSPTest(ModuleStoreTestCase):
 
         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

common/djangoapps/student/tests/test_login.py

@@ -202,7 +202,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
         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',

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
 

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -27,6 +27,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         """
         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 @@ class MixedModuleStore(ModuleStoreWriteBase):
     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 @@ class MixedModuleStore(ModuleStoreWriteBase):
         """
         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(

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

@@ -716,7 +716,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
             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 @@ class MongoModuleStore(ModuleStoreWriteBase):
             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.

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

@@ -174,7 +174,7 @@ class DraftModuleStore(MongoModuleStore):
 
         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 @@ class DraftModuleStore(MongoModuleStore):
                         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):

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -57,8 +57,9 @@ from pytz import UTC
 
 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 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         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).

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

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

@@ -205,7 +205,7 @@ class ModuleStoreTestCase(TestCase):
         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
 

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

@@ -151,7 +151,7 @@ class ItemFactory(XModuleFactory):
         # 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 @@ class ItemFactory(XModuleFactory):
 
         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)

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

@@ -201,7 +201,7 @@ class TestMixedModuleStore(object):
     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):

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

@@ -71,7 +71,7 @@ class TestPublish(unittest.TestCase):
                 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 @@ class TestPublish(unittest.TestCase):
         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,

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

@@ -102,7 +102,7 @@ class TestMigration(unittest.TestCase):
         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 @@ class TestMigration(unittest.TestCase):
         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 @@ class TestMigration(unittest.TestCase):
         # 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 @@ class TestMigration(unittest.TestCase):
                     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

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

@@ -748,7 +748,7 @@ class TestItemCrud(SplitModuleTest):
 
         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 @@ class TestItemCrud(SplitModuleTest):
         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 @@ class TestItemCrud(SplitModuleTest):
         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 @@ class TestItemCrud(SplitModuleTest):
         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 @@ class TestItemCrud(SplitModuleTest):
 
         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 @@ class TestItemCrud(SplitModuleTest):
         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)

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

@@ -4,14 +4,13 @@ import mimetypes
 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_course_draft(
 
                             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,

lms/djangoapps/courseware/tests/test_submitting_problems.py

@@ -231,7 +231,7 @@ class TestCourseGrader(TestSubmittingProblems):
 
         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):

lms/djangoapps/courseware/views.py

 import logging
 import urllib
 
-from functools import partial
 from collections import defaultdict
 
 from django.conf import settings
@@ -10,7 +9,7 @@ from django.core.exceptions import PermissionDenied
 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)

lms/djangoapps/instructor_task/tests/test_base.py

@@ -210,7 +210,7 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         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`."""

lms/djangoapps/instructor_task/tests/test_integration.py

@@ -292,7 +292,7 @@ class TestRescoringTask(TestIntegrationTask):
                 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