Merge pull request #4339 from edx/nimisha/fix-user-ids

Nimisha/fix user ids and LMS tests branch setting

cms/djangoapps/contentstore/management/commands/clone_course.py

@@ -7,6 +7,7 @@ from student.roles import CourseInstructorRole, CourseStaffRole
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from xmodule.modulestore import ModuleStoreEnum
 
 
 #
@@ -38,7 +39,7 @@ class Command(BaseCommand):
         print("Cloning course {0} to {1}".format(source_course_id, dest_course_id))
 
         with mstore.bulk_write_operations(dest_course_id):
-            if mstore.clone_course(source_course_id, dest_course_id, None):
+            if mstore.clone_course(source_course_id, dest_course_id, ModuleStoreEnum.UserID.mgmt_command):
                 print("copying User permissions...")
                 # purposely avoids auth.add_user b/c it doesn't have a caller to authorize
                 CourseInstructorRole(dest_course_id).add_users(

cms/djangoapps/contentstore/management/commands/delete_course.py

@@ -7,7 +7,7 @@ from contentstore.utils import delete_course_and_groups
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
-
+from xmodule.modulestore import ModuleStoreEnum
 
 class Command(BaseCommand):
     help = '''Delete a MongoDB backed course'''
@@ -28,6 +28,6 @@ class Command(BaseCommand):
         if commit:
             print('Actually going to delete the course from DB....')
 
-        if query_yes_no("Deleting course {0}. Confirm?".format(course_key), default="no"):
-            if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
-                delete_course_and_groups(course_key, commit)
+            if query_yes_no("Deleting course {0}. Confirm?".format(course_key), default="no"):
+                if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
+                    delete_course_and_groups(course_key, ModuleStoreEnum.UserID.mgmt_command)

cms/djangoapps/contentstore/management/commands/import.py

@@ -6,6 +6,7 @@ from django.core.management.base import BaseCommand, CommandError, make_option
 from django_comment_common.utils import (seed_permissions_roles,
                                          are_permissions_roles_seeded)
 from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
 
@@ -40,7 +41,7 @@ class Command(BaseCommand):
         mstore = modulestore()
 
         _, course_items = import_from_xml(
-            mstore, "**replace_user**", data_dir, course_dirs, load_error_modules=False,
+            mstore, ModuleStoreEnum.UserID.mgmt_command, data_dir, course_dirs, load_error_modules=False,
             static_content_store=contentstore(), verbose=True,
             do_import_static=do_import_static,
             create_new_course_if_not_present=True,

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -30,7 +30,6 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.inheritance import own_metadata
 from opaque_keys.edx.keys import UsageKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
-from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint
@@ -632,7 +631,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
         self.store.convert_to_draft(vertical.location, self.user.id)
 
         # delete the course
-        delete_course(self.store, content_store, course_id, commit=True)
+        self.store.delete_course(course_id, self.user.id)
 
         # assert that there's absolutely no non-draft modules in the course
         # this should also include all draft items
@@ -697,7 +696,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
             self.assertEqual(on_disk['course/2012_Fall'], own_metadata(course))
 
         # remove old course
-        delete_course(self.store, content_store, course_id, commit=True)
+        self.store.delete_course(course_id, self.user.id)
 
         # reimport over old course
         self.check_import(root_dir, content_store, course_id)
@@ -909,7 +908,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
 
         # Delete the course from module store and reimport it
 
-        delete_course(self.store, content_store, course_id, commit=True)
+        self.store.delete_course(course_id, self.user.id)
 
         import_from_xml(
             self.store, self.user.id, root_dir, ['test_export_no_content_store'],
@@ -997,7 +996,7 @@ class ContentStoreTest(ContentStoreTestCase):
         test_course_data = self.assert_created_course(number_suffix=uuid4().hex)
         course_id = _get_course_id(test_course_data)
         self.assertTrue(are_permissions_roles_seeded(course_id))
-        delete_course_and_groups(course_id, commit=True)
+        delete_course_and_groups(course_id, self.user.id)
         # should raise an exception for checking permissions on deleted course
         with self.assertRaises(ItemNotFoundError):
             are_permissions_roles_seeded(course_id)
@@ -1009,7 +1008,7 @@ class ContentStoreTest(ContentStoreTestCase):
 
         # unseed the forums for the first course
         course_id = _get_course_id(test_course_data)
-        delete_course_and_groups(course_id, commit=True)
+        delete_course_and_groups(course_id, self.user.id)
         # should raise an exception for checking permissions on deleted course
         with self.assertRaises(ItemNotFoundError):
             are_permissions_roles_seeded(course_id)
@@ -1029,7 +1028,7 @@ class ContentStoreTest(ContentStoreTestCase):
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id))
         self.assertTrue(self.user.roles.filter(name="Student", course_id=course_id))  # pylint: disable=no-member
 
-        delete_course_and_groups(course_id, commit=True)
+        delete_course_and_groups(course_id, self.user.id)
         # check that user's enrollment for this course is not deleted
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id))
         # check that user has form role "Student" for this course even after deleting it
@@ -1051,7 +1050,7 @@ class ContentStoreTest(ContentStoreTestCase):
         self.assertTrue(len(instructor_role.users_with_role()) > 0)
 
         # Now delete course and check that user not in instructor groups of this course
-        delete_course_and_groups(course_id, commit=True)
+        delete_course_and_groups(course_id, self.user.id)
 
         # Update our cached user since its roles have changed
         self.user = User.objects.get_by_natural_key(self.user.natural_key()[0])

cms/djangoapps/contentstore/tests/test_course_listing.py

@@ -145,7 +145,7 @@ class TestCourseListing(ModuleStoreTestCase):
         self.assertEqual(courses_list, courses_list_by_groups)
 
         # now delete this course and re-add user to instructor group of this course
-        delete_course_and_groups(course_key, commit=True)
+        delete_course_and_groups(course_key, self.user.id)
 
         CourseInstructorRole(course_key).add_users(self.user)
 
@@ -237,7 +237,7 @@ class TestCourseListing(ModuleStoreTestCase):
         self.assertEqual(len(courses_list_by_groups), 2)
 
         # now delete first course (course_location_caps) and check that it is no longer accessible
-        delete_course_and_groups(course_location_caps, commit=True)
+        delete_course_and_groups(course_location_caps, self.user.id)
 
         # test that get courses through iterating all courses now returns one course
         courses_list = _accessible_courses_list(self.request)
@@ -269,7 +269,7 @@ class TestCourseListing(ModuleStoreTestCase):
 
         course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun')
         self._create_course_with_access_groups(course_location, self.user)
-        store.delete_course(course_location)
+        store.delete_course(course_location, self.user.id)
 
         course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun')
         course = self._create_course_with_access_groups(course_location, self.user)

cms/djangoapps/contentstore/tests/test_crud.py

@@ -162,7 +162,7 @@ class TemplateTests(unittest.TestCase):
         self.assertIsInstance(self.split_store.get_course(id_locator), CourseDescriptor)
         # and by guid
         self.assertIsInstance(self.split_store.get_item(guid_locator), CourseDescriptor)
-        self.split_store.delete_course(id_locator)
+        self.split_store.delete_course(id_locator, ModuleStoreEnum.UserID.test)
         # test can no longer retrieve by id
         self.assertRaises(ItemNotFoundError, self.split_store.get_course, id_locator)
         # but can by guid
@@ -187,11 +187,11 @@ class TemplateTests(unittest.TestCase):
         )
         first_problem.max_attempts = 3
         first_problem.save()  # decache the above into the kvs
-        updated_problem = self.split_store.update_item(first_problem, '**replace_user**')
+        updated_problem = self.split_store.update_item(first_problem, ModuleStoreEnum.UserID.test)
         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)
-        updated_loc = self.split_store.delete_item(updated_problem.location, '**replace_user**', 'testbot')
+        updated_loc = self.split_store.delete_item(updated_problem.location, ModuleStoreEnum.UserID.test, 'testbot')
 
         second_problem = persistent_factories.ItemFactory.create(
             display_name='problem 2',

cms/djangoapps/contentstore/tests/test_export_git.py

@@ -65,7 +65,7 @@ class TestExportGit(CourseTestCase):
         Test failed course export response.
         """
         self.course_module.giturl = 'foobar'
-        modulestore().update_item(self.course_module, '**replace_user**')
+        modulestore().update_item(self.course_module, self.user.id)
 
         response = self.client.get('{}?action=push'.format(self.test_url))
         self.assertIn('Export Failed:', response.content)
@@ -75,7 +75,7 @@ class TestExportGit(CourseTestCase):
         Regression test for making sure errors are properly stringified
         """
         self.course_module.giturl = 'foobar'
-        modulestore().update_item(self.course_module, '**replace_user**')
+        modulestore().update_item(self.course_module, self.user.id)
 
         response = self.client.get('{}?action=push'.format(self.test_url))
         self.assertNotIn('django.utils.functional.__proxy__', response.content)
@@ -98,7 +98,7 @@ class TestExportGit(CourseTestCase):
 
         self.populate_course()
         self.course_module.giturl = 'file://{}'.format(bare_repo_dir)
-        modulestore().update_item(self.course_module, '**replace_user**')
+        modulestore().update_item(self.course_module, self.user.id)
 
         response = self.client.get('{}?action=push'.format(self.test_url))
         self.assertIn('Export Succeeded', response.content)

cms/djangoapps/contentstore/tests/test_import.py

@@ -33,25 +33,10 @@ class ContentStoreImportTest(ModuleStoreTestCase):
     NOTE: refactor using CourseFactory so they do not.
     """
     def setUp(self):
-
-        uname = 'testuser'
-        email = 'test+courses@edx.org'
-        password = 'foo'
-
-        # Create the use so we can log them in.
-        self.user = User.objects.create_user(uname, email, password)
-
-        # Note that we do not actually need to do anything
-        # for registration if we directly mark them active.
-        self.user.is_active = True
-        # Staff has access to view all courses
-        self.user.is_staff = True
-
-        # Save the data that we've just changed to the db.
-        self.user.save()
-
+        password = super(ContentStoreImportTest, self).setUp()
+        
         self.client = Client()
-        self.client.login(username=uname, password=password)
+        self.client.login(username=self.user.username, password=password)
 
     def tearDown(self):
         contentstore().drop_database()
@@ -66,7 +51,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         module_store = modulestore()
         import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['test_import_course'],
             static_content_store=content_store,
@@ -86,7 +71,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         module_store, __, course = self.load_test_import_course()
         __, course_items = import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data',
             ['test_import_course_2'],
             target_course_id=course.id,
@@ -102,7 +87,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         course_id = SlashSeparatedCourseKey(u'Юникода', u'unicode_course', u'échantillon')
         import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['2014_Uni'],
             target_course_id=course_id
@@ -148,7 +133,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         content_store = contentstore()
 
         module_store = modulestore()
-        import_from_xml(module_store, '**replace_user**', 'common/test/data/', ['toy'], static_content_store=content_store, do_import_static=False, verbose=True)
+        import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], static_content_store=content_store, do_import_static=False, verbose=True)
 
         course = module_store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall'))
 
@@ -159,7 +144,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
 
     def test_no_static_link_rewrites_on_import(self):
         module_store = modulestore()
-        _, courses = import_from_xml(module_store, '**replace_user**', 'common/test/data/', ['toy'], do_import_static=False, verbose=True)
+        _, courses = import_from_xml(module_store, self.user.id, 'common/test/data/', ['toy'], do_import_static=False, verbose=True)
         course_key = courses[0].id
 
         handouts = module_store.get_item(course_key.make_usage_key('course_info', 'handouts'))
@@ -178,7 +163,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         target_course_id = SlashSeparatedCourseKey('testX', 'conditional_copy', 'copy_run')
         import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['conditional'],
             target_course_id=target_course_id
@@ -208,7 +193,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         target_course_id = SlashSeparatedCourseKey('testX', 'peergrading_copy', 'copy_run')
         import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['open_ended'],
             target_course_id=target_course_id
@@ -227,7 +212,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         target_course_id = SlashSeparatedCourseKey('testX', 'split_test_copy', 'copy_run')
         import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['split_test_module'],
             target_course_id=target_course_id

cms/djangoapps/contentstore/tests/test_import_draft_order.py

@@ -10,7 +10,7 @@ class DraftReorderTestCase(ModuleStoreTestCase):
 
     def test_order(self):
         store = modulestore()
-        _, course_items = import_from_xml(store, '**replace_user**', 'common/test/data/', ['import_draft_order'])
+        _, course_items = import_from_xml(store, self.user.id, 'common/test/data/', ['import_draft_order'])
         course_key = course_items[0].id
         sequential = store.get_item(course_key.make_usage_key('sequential', '0f4f7649b10141b0bdc9922dcf94515a'))
         verticals = sequential.children

cms/djangoapps/contentstore/tests/test_import_pure_xblock.py

@@ -28,9 +28,6 @@ class StubXBlock(XBlock):
 
 class XBlockImportTest(ModuleStoreTestCase):
 
-    def setUp(self):
-        self.store = modulestore()
-
     @XBlock.register_temp_plugin(StubXBlock)
     def test_import_public(self):
         self._assert_import(
@@ -62,7 +59,7 @@ class XBlockImportTest(ModuleStoreTestCase):
 
         """
         _, courses = import_from_xml(
-            self.store, '**replace_user**', 'common/test/data', [course_dir]
+            self.store, self.user.id, 'common/test/data', [course_dir]
         )
 
         xblock_location = courses[0].id.make_usage_key('stubxblock', 'xblock_test')

cms/djangoapps/contentstore/tests/test_permissions.py

@@ -24,23 +24,10 @@ class TestCourseAccess(ModuleStoreTestCase):
 
         Create a pool of users w/o granting them any permissions
         """
-        super(TestCourseAccess, self).setUp()
-        uname = 'testuser'
-        email = 'test+courses@edx.org'
-        password = 'foo'
-
-        # Create the use so we can log them in.
-        self.user = User.objects.create_user(uname, email, password)
-
-        # Note that we do not actually need to do anything
-        # for registration if we directly mark them active.
-        self.user.is_active = True
-        # Staff has access to view all courses
-        self.user.is_staff = True
-        self.user.save()
+        user_password = super(TestCourseAccess, self).setUp()
 
         self.client = AjaxEnabledTestClient()
-        self.client.login(username=uname, password=password)
+        self.client.login(username=self.user.username, password=user_password)
 
         # create a course via the view handler which has a different strategy for permissions than the factory
         self.course_key = SlashSeparatedCourseKey('myu', 'mydept.mycourse', 'myrun')

cms/djangoapps/contentstore/tests/test_users_default_role.py

@@ -62,7 +62,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase):
         # check that user has his default "Student" forum role for this course
         self.assertTrue(self.user.roles.filter(name="Student", course_id=self.course_key))  # pylint: disable=no-member
 
-        delete_course_and_groups(self.course_key, commit=True)
+        delete_course_and_groups(self.course_key, self.user.id)
 
         # check that user's enrollment for this course is not deleted
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
@@ -80,7 +80,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase):
         self.assertTrue(self.user.roles.filter(name="Student", course_id=self.course_key))  # pylint: disable=no-member
 
         # delete this course and recreate this course with same user
-        delete_course_and_groups(self.course_key, commit=True)
+        delete_course_and_groups(self.course_key, self.user.id)
         resp = self._create_course_with_given_location(self.course_key)
         self.assertEqual(resp.status_code, 200)
 
@@ -98,7 +98,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase):
         # check that user has enrollment and his default "Student" forum role for this course
         self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
         # delete this course and recreate this course with same user
-        delete_course_and_groups(self.course_key, commit=True)
+        delete_course_and_groups(self.course_key, self.user.id)
 
         # now create same course with different name case ('uppercase')
         new_course_key = self.course_key.replace(course=self.course_key.course.upper())

cms/djangoapps/contentstore/tests/test_utils.py

@@ -9,6 +9,7 @@ from django.test import TestCase
 from django.test.utils import override_settings
 
 from contentstore import utils
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.factories import CourseFactory
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
 
@@ -193,7 +194,7 @@ class XBlockVisibilityTestCase(TestCase):
     """Tests for xblock visibility for students."""
 
     def setUp(self):
-        self.dummy_user = 123
+        self.dummy_user = ModuleStoreEnum.UserID.test
         self.past = datetime(1970, 1, 1)
         self.future = datetime.now(UTC) + timedelta(days=1)
 

cms/djangoapps/contentstore/tests/utils.py

@@ -6,8 +6,8 @@ Utilities for contentstore tests
 import json
 import re
 
-from django.contrib.auth.models import User
 from django.test.client import Client
+from django.contrib.auth.models import User
 
 from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.content import StaticContent
@@ -68,53 +68,32 @@ class AjaxEnabledTestClient(Client):
 class CourseTestCase(ModuleStoreTestCase):
     def setUp(self):
         """
-        These tests need a user in the DB so that the django Test Client
-        can log them in.
+        These tests need a user in the DB so that the django Test Client can log them in.
+        The test user is created in the ModuleStoreTestCase setUp method.
         They inherit from the ModuleStoreTestCase class so that the mongodb collection
         will be cleared out before each test case execution and deleted
         afterwards.
         """
-        uname = 'testuser'
-        email = 'test+courses@edx.org'
-        password = 'foo'
-
-        # Create the user so we can log them in.
-        self.user = User.objects.create_user(uname, email, password)
-
-        # Note that we do not actually need to do anything
-        # for registration if we directly mark them active.
-        self.user.is_active = True
-        # Staff has access to view all courses
-        self.user.is_staff = True
-        self.user.save()
+        user_password = super(CourseTestCase, self).setUp()
 
         self.client = AjaxEnabledTestClient()
-        self.client.login(username=uname, password=password)
+        self.client.login(username=self.user.username, password=user_password)
 
         self.course = CourseFactory.create(
             org='MITx',
             number='999',
             display_name='Robot Super Course',
         )
-        self.store = modulestore()
 
     def create_non_staff_authed_user_client(self, authenticate=True):
         """
         Create a non-staff user, log them in (if authenticate=True), and return the client, user to use for testing.
         """
-        uname = 'teststudent'
-        password = 'foo'
-        nonstaff = User.objects.create_user(uname, 'test+student@edx.org', password)
-
-        # Note that we do not actually need to do anything
-        # for registration if we directly mark them active.
-        nonstaff.is_active = True
-        nonstaff.is_staff = False
-        nonstaff.save()
+        nonstaff, password = self.create_non_staff_user()
 
         client = Client()
         if authenticate:
-            client.login(username=uname, password=password)
+            client.login(username=nonstaff.username, password=password)
         return client, nonstaff
 
     def populate_course(self):

cms/djangoapps/contentstore/utils.py

@@ -11,12 +11,10 @@ from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
 
 from xmodule.contentstore.content import StaticContent
-from xmodule.contentstore.django import contentstore
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
-from xmodule.modulestore.store_utilities import delete_course
 from student.roles import CourseInstructorRole, CourseStaffRole
 
 
@@ -28,27 +26,25 @@ NOTES_PANEL = {"name": _("My Notes"), "type": "notes"}
 EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]])
 
 
-def delete_course_and_groups(course_id, commit=False):
+def delete_course_and_groups(course_id, user_id):
     """
     This deletes the courseware associated with a course_id as well as cleaning update_item
     the various user table stuff (groups, permissions, etc.)
     """
     module_store = modulestore()
-    content_store = contentstore()
 
     with module_store.bulk_write_operations(course_id):
-        if delete_course(module_store, content_store, course_id, commit):
-
-            print 'removing User permissions from course....'
-            # in the django layer, we need to remove all the user permissions groups associated with this course
-            if commit:
-                try:
-                    staff_role = CourseStaffRole(course_id)
-                    staff_role.remove_users(*staff_role.users_with_role())
-                    instructor_role = CourseInstructorRole(course_id)
-                    instructor_role.remove_users(*instructor_role.users_with_role())
-                except Exception as err:
-                    log.error("Error in deleting course groups for {0}: {1}".format(course_id, err))
+        module_store.delete_course(course_id, user_id)
+
+        print 'removing User permissions from course....'
+        # in the django layer, we need to remove all the user permissions groups associated with this course
+        try:
+            staff_role = CourseStaffRole(course_id)
+            staff_role.remove_users(*staff_role.users_with_role())
+            instructor_role = CourseInstructorRole(course_id)
+            instructor_role.remove_users(*instructor_role.users_with_role())
+        except Exception as err:
+            log.error("Error in deleting course groups for {0}: {1}".format(course_id, err))
 
 
 def get_lms_link_for_item(location, preview=False):

cms/djangoapps/contentstore/views/component.py

@@ -449,8 +449,8 @@ def component_handler(request, usage_key_string, handler, suffix=''):
         log.info("XBlock %s attempted to access missing handler %r", descriptor, handler, exc_info=True)
         raise Http404
 
-    # unintentional update to handle any side effects of handle call; so, request user didn't author
-    # the change
-    modulestore().update_item(descriptor, None)
+    # unintentional update to handle any side effects of handle call
+    # could potentially be updating actual course data or simply caching its values
+    modulestore().update_item(descriptor, request.user.id)
 
     return webob_to_django_response(resp)

cms/djangoapps/contentstore/views/course.py

@@ -337,6 +337,7 @@ def create_new_course(request):
         new_course = modulestore().create_course(
             course_key.org,
             course_key.offering,
+            request.user.id,
             fields=fields,
         )
 

cms/djangoapps/contentstore/views/item.py

@@ -140,8 +140,8 @@ def xblock_handler(request, usage_key_string):
             dest_usage_key = _duplicate_item(
                 parent_usage_key,
                 duplicate_source_usage_key,
-                request.json.get('display_name'),
                 request.user,
+                request.json.get('display_name'),
             )
 
             return JsonResponse({"locator": unicode(dest_usage_key), "courseKey": unicode(dest_usage_key.course_key)})
@@ -193,8 +193,7 @@ def xblock_view_handler(request, usage_key_string, view_name):
                 log.debug("unable to render studio_view for %r", xblock, exc_info=True)
                 fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)}))
 
-            # change not authored by requestor but by xblocks.
-            store.update_item(xblock, None)
+            store.update_item(xblock, request.user.id)
         elif view_name in (unit_views + container_views):
             is_container_view = (view_name in container_views)
 
@@ -432,7 +431,7 @@ def _create_item(request):
     return JsonResponse({"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)})
 
 
-def _duplicate_item(parent_usage_key, duplicate_source_usage_key, display_name=None, user=None):
+def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_name=None):
     """
     Duplicate an existing xblock as a child of the supplied parent_usage_key.
     """
@@ -454,6 +453,8 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, display_name=N
 
     dest_module = store.create_and_save_xmodule(
         dest_usage_key,
+
+
         user.id,
         definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content),
         metadata=duplicate_metadata,
@@ -467,7 +468,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, display_name=N
         for child in source_item.children:
             dupe = _duplicate_item(dest_module.location, child, user=user)
             dest_module.children.append(dupe)
-        store.update_item(dest_module, user.id if user else None)
+        store.update_item(dest_module, user.id)
 
     if not 'detached' in source_item.runtime.load_block_type(category)._class_tags:
         parent = store.get_item(parent_usage_key)
@@ -478,7 +479,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, display_name=N
             parent.children.insert(source_index + 1, dest_module.location)
         else:
             parent.children.append(dest_module.location)
-        store.update_item(parent, user.id if user else None)
+        store.update_item(parent, user.id)
 
     return dest_module.location
 

cms/djangoapps/contentstore/views/tabs.py

@@ -12,6 +12,7 @@ from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.http import require_http_methods
 from edxmako.shortcuts import render_to_response
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.tabs import CourseTabList, StaticTab, CourseTab, InvalidTabsException
 from opaque_keys.edx.keys import CourseKey, UsageKey
 
@@ -192,7 +193,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().update_item(course, '**replace_user**')
+    modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command)
 
 
 def primitive_insert(course, num, tab_type, name):
@@ -201,5 +202,5 @@ def primitive_insert(course, num, tab_type, name):
     new_tab = CourseTab.from_json({u'type': unicode(tab_type), u'name': unicode(name)})
     tabs = course.tabs
     tabs.insert(num, new_tab)
-    modulestore().update_item(course, '**replace_user**')
+    modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command)
 

cms/djangoapps/contentstore/views/tests/test_assets.py

@@ -51,7 +51,7 @@ class BasicAssetsTestCase(AssetsTestCase):
         module_store = modulestore()
         _, course_items = import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['toy'],
             static_content_store=contentstore(),
@@ -195,7 +195,7 @@ class LockAssetTestCase(AssetsTestCase):
         module_store = modulestore()
         _, course_items = import_from_xml(
             module_store,
-            '**replace_user**',
+            self.user.id,
             'common/test/data/',
             ['toy'],
             static_content_store=contentstore(),

cms/djangoapps/contentstore/views/tests/test_container_page.py

@@ -20,16 +20,11 @@ class ContainerPageTestCase(StudioPageTestCase):
 
     def setUp(self):
         super(ContainerPageTestCase, self).setUp()
-        self.vertical = ItemFactory.create(parent_location=self.sequential.location,
-                                           category='vertical', display_name='Unit')
-        self.html = ItemFactory.create(parent_location=self.vertical.location,
-                                        category="html", display_name="HTML")
-        self.child_container = ItemFactory.create(parent_location=self.vertical.location,
-                                                  category='split_test', display_name='Split Test')
-        self.child_vertical = ItemFactory.create(parent_location=self.child_container.location,
-                                                 category='vertical', display_name='Child Vertical')
-        self.video = ItemFactory.create(parent_location=self.child_vertical.location,
-                                        category="video", display_name="My Video")
+        self.vertical = self._create_item(self.sequential.location, 'vertical', 'Unit')
+        self.html = self._create_item(self.vertical.location, "html", "HTML")
+        self.child_container = self._create_item(self.vertical.location, 'split_test', 'Split Test')
+        self.child_vertical = self._create_item(self.child_container.location, 'vertical', 'Child Vertical')
+        self.video = self._create_item(self.child_vertical.location, "video", "My Video")
         self.store = modulestore()
 
     def test_container_html(self):
@@ -51,14 +46,8 @@ class ContainerPageTestCase(StudioPageTestCase):
         Create the scenario of an xblock with children (non-vertical) on the container page.
         This should create a container page that is a child of another container page.
         """
-        draft_container = ItemFactory.create(
-            parent_location=self.child_container.location,
-            category="wrapper", display_name="Wrapper"
-        )
-        ItemFactory.create(
-            parent_location=draft_container.location,
-            category="html", display_name="Child HTML"
-        )
+        draft_container = self._create_item(self.child_container.location, "wrapper", "Wrapper")
+        self._create_item(draft_container.location, "html", "Child HTML")
 
         def test_container_html(xblock):
             self._test_html_content(
@@ -135,9 +124,8 @@ class ContainerPageTestCase(StudioPageTestCase):
         """
         Verify that a public container rendered as a child of the container page returns the expected HTML.
         """
-        empty_child_container = ItemFactory.create(parent_location=self.vertical.location,
-                                                   category='split_test', display_name='Split Test')
-        published_empty_child_container = self.store.publish(empty_child_container.location, '**replace_user**')
+        empty_child_container = self._create_item(self.vertical.location, 'split_test', 'Split Test')
+        published_empty_child_container = self.store.publish(empty_child_container.location, self.user.id)
         self.validate_preview_html(published_empty_child_container, self.reorderable_child_view,
                                    can_reorder=False, can_edit=False, can_add=False)
 
@@ -145,7 +133,18 @@ class ContainerPageTestCase(StudioPageTestCase):
         """
         Verify that a draft container rendered as a child of the container page returns the expected HTML.
         """
-        empty_child_container = ItemFactory.create(parent_location=self.vertical.location,
-                                                   category='split_test', display_name='Split Test')
+        empty_child_container = self._create_item(self.vertical.location, 'split_test', 'Split Test')
         self.validate_preview_html(empty_child_container, self.reorderable_child_view,
                                    can_reorder=True, can_edit=True, can_add=False)
+
+    def _create_item(self, parent_location, category, display_name, **kwargs):
+        """
+        creates an item in the module store, without publishing it.
+        """
+        return ItemFactory.create(
+            parent_location=parent_location,
+            category=category,
+            display_name=display_name,
+            publish_item=False,
+            **kwargs
+        )

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -25,10 +25,9 @@ from contentstore.tests.utils import CourseTestCase
 from student.tests.factories import UserFactory
 from xmodule.capa_module import CapaDescriptor
 from xmodule.modulestore import PublishState
-from xmodule.modulestore.django import modulestore
 from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW
 from xblock.exceptions import NoSuchHandlerError
-from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.keys import UsageKey, CourseKey
 from opaque_keys.edx.locations import Location
 from xmodule.partitions.partitions import Group, UserPartition
 
@@ -40,7 +39,6 @@ class ItemTest(CourseTestCase):
 
         self.course_key = self.course.id
         self.usage_key = self.course.location
-        self.store = modulestore()
 
     def get_item_from_modulestore(self, usage_key, verify_is_draft=False):
         """

cms/djangoapps/contentstore/views/tests/test_unit_page.py

@@ -40,7 +40,7 @@ class UnitPageTestCase(StudioPageTestCase):
         """
         Verify that a public xblock's preview returns the expected HTML.
         """
-        published_video = self.store.publish(self.video.location, '**replace_user**')
+        published_video = self.store.publish(self.video.location, self.user.id)
         self.validate_preview_html(self.video, STUDENT_VIEW,
                                    can_edit=True, can_reorder=True, can_add=False)
 
@@ -60,7 +60,7 @@ class UnitPageTestCase(StudioPageTestCase):
                                              category='split_test', display_name='Split Test')
         ItemFactory.create(parent_location=child_container.location,
                            category='html', display_name='grandchild')
-        published_child_container = self.store.publish(child_container.location, '**replace_user**')
+        published_child_container = self.store.publish(child_container.location, self.user.id)
         self.validate_preview_html(published_child_container, STUDENT_VIEW,
                                    can_reorder=True, can_edit=True, can_add=False)
 

cms/djangoapps/models/settings/course_metadata.py

@@ -53,7 +53,7 @@ class CourseMetadata(object):
         return result
 
     @classmethod
-    def update_from_json(cls, descriptor, jsondict, filter_tabs=True, user=None):
+    def update_from_json(cls, descriptor, jsondict, user, filter_tabs=True):
         """
         Decode the json into CourseMetadata and save any changed attrs to the db.
 
@@ -84,6 +84,6 @@ class CourseMetadata(object):
             setattr(descriptor, key, value)
 
         if len(key_values) > 0:
-            modulestore().update_item(descriptor, user.id if user else None)
+            modulestore().update_item(descriptor, user.id)
 
         return cls.fetch(descriptor)

common/djangoapps/contentserver/tests/test.py

@@ -5,7 +5,6 @@ import copy
 import logging
 from uuid import uuid4
 
-from django.contrib.auth.models import User
 from django.conf import settings
 from django.test.client import Client
 from django.test.utils import override_settings
@@ -34,12 +33,16 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         """
         Create user and login.
         """
+        self.staff_pwd = super(ContentStoreToyCourseTest, self).setUp()
+        self.staff_usr = self.user
+        self.non_staff_usr, self.non_staff_pwd = self.create_non_staff_user()
+
         self.client = Client()
         self.contentstore = contentstore()
 
         self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
 
-        import_from_xml(modulestore(), '**replace_user**', 'common/test/data/', ['toy'],
+        import_from_xml(modulestore(), self.user.id, 'common/test/data/', ['toy'],
                 static_content_store=self.contentstore, verbose=True)
 
         # A locked asset
@@ -52,24 +55,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         self.contentstore.set_attr(self.locked_asset, 'locked', True)
 
-        # Create user
-        self.usr = 'testuser'
-        self.pwd = 'foo'
-        email = 'test+courses@edx.org'
-        self.user = User.objects.create_user(self.usr, email, self.pwd)
-        self.user.is_active = True
-        self.user.save()
-
-        # Create staff user
-        self.staff_usr = 'stafftestuser'
-        self.staff_pwd = 'foo'
-        staff_email = 'stafftest+courses@edx.org'
-        self.staff_user = User.objects.create_user(self.staff_usr, staff_email,
-                self.staff_pwd)
-        self.staff_user.is_active = True
-        self.staff_user.is_staff = True
-        self.staff_user.save()
-
     def tearDown(self):
 
         contentstore().drop_database()
@@ -97,7 +82,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         Test that locked assets behave appropriately in case user is logged in
         in but not registered for the course.
         """
-        self.client.login(username=self.usr, password=self.pwd)
+        self.client.login(username=self.non_staff_usr, password=self.non_staff_pwd)
         resp = self.client.get(self.url_locked)
         self.assertEqual(resp.status_code, 403) # pylint: disable=E1103
 
@@ -106,10 +91,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         Test that locked assets behave appropriately in case user is logged in
         and registered for the course.
         """
-        CourseEnrollment.enroll(self.user, self.course_key)
-        self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key))
+        CourseEnrollment.enroll(self.non_staff_usr, self.course_key)
+        self.assertTrue(CourseEnrollment.is_enrolled(self.non_staff_usr, self.course_key))
 
-        self.client.login(username=self.usr, password=self.pwd)
+        self.client.login(username=self.non_staff_usr, password=self.non_staff_pwd)
         resp = self.client.get(self.url_locked)
         self.assertEqual(resp.status_code, 200) # pylint: disable=E1103
 

common/djangoapps/external_auth/tests/test_shib.py

@@ -17,8 +17,8 @@ from django.utils.importlib import import_module
 
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config
-from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore import ModuleStoreEnum
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
 from external_auth.models import ExternalAuthMap
@@ -80,7 +80,8 @@ class ShibSPTest(ModuleStoreTestCase):
     request_factory = RequestFactory()
 
     def setUp(self):
-        self.store = modulestore()
+        super(ShibSPTest, self).setUp(create_user=False)
+        self.test_user_id = ModuleStoreEnum.UserID.test
 
     @unittest.skipUnless(settings.FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
     def test_exception_shib_login(self):
@@ -377,13 +378,21 @@ class ShibSPTest(ModuleStoreTestCase):
         """
         Tests that the correct course specific login and registration urls work for shib
         """
-        course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course')
+        course = CourseFactory.create(
+            org='MITx',
+            number='999',
+            display_name='Robot Super Course',
+            user_id=self.test_user_id,
+        )
 
         # Test for cases where course is found
         for domain in ["", "shib:https://idp.stanford.edu/"]:
             # set domains
-            course.enrollment_domain = domain
-            self.store.update_item(course, '**replace_user**')
+
+            # temporarily set the branch to draft-preferred so we can update the course
+            with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id):
+                course.enrollment_domain = domain
+                self.store.update_item(course, self.test_user_id)
 
             # setting location to test that GET params get passed through
             login_request = self.request_factory.get('/course_specific_login/MITx/999/Robot_Super_Course' +
@@ -450,13 +459,21 @@ 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, '**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, '**replace_user**')
+        shib_course = CourseFactory.create(
+            org='Stanford',
+            number='123',
+            display_name='Shib Only',
+            enrollment_domain='shib:https://idp.stanford.edu/',
+            user_id=self.test_user_id,
+        )
+
+        open_enroll_course = CourseFactory.create(
+            org='MITx',
+            number='999',
+            display_name='Robot Super Course',
+            enrollment_domain='',
+            user_id=self.test_user_id,
+        )
 
         # create 3 kinds of students, external_auth matching shib_course, external_auth not matching, no external auth
         shib_student = UserFactory.create()
@@ -520,9 +537,13 @@ class ShibSPTest(ModuleStoreTestCase):
         student.save()
         extauth.save()
 
-        course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
-        course.enrollment_domain = 'shib:https://idp.stanford.edu/'
-        self.store.update_item(course, '**replace_user**')
+        course = CourseFactory.create(
+            org='Stanford',
+            number='123',
+            display_name='Shib Only',
+            enrollment_domain='shib:https://idp.stanford.edu/',
+            user_id=self.test_user_id,
+        )
 
         # use django test client for sessions and url processing
         # no enrollment before trying

common/djangoapps/student/tests/test_course_listing.py

@@ -97,7 +97,7 @@ class TestCourseListing(ModuleStoreTestCase):
 
         course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun')
         self._create_course_with_access_groups(course_location)
-        mongo_store.delete_course(course_location)
+        mongo_store.delete_course(course_location, ModuleStoreEnum.UserID.test)
 
         course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun')
         course = self._create_course_with_access_groups(course_location)

common/djangoapps/student/tests/test_login.py

@@ -17,7 +17,6 @@ from student.views import _parse_course_id_from_string, _get_course_enrollment_d
 
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config
-from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.django import modulestore
 
 from external_auth.models import ExternalAuthMap
@@ -346,11 +345,20 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
     Tests how login_user() interacts with ExternalAuth, in particular Shib
     """
     def setUp(self):
-        self.store = modulestore()
-        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, '**replace_user**')
+        super(ExternalAuthShibTest, self).setUp()
+        self.course = CourseFactory.create(
+            org='Stanford',
+            number='456',
+            display_name='NO SHIB',
+            user_id=self.user.id,
+        )
+        self.shib_course = CourseFactory.create(
+            org='Stanford',
+            number='123',
+            display_name='Shib Only',
+            enrollment_domain='shib:https://idp.stanford.edu/',
+            user_id=self.user.id,
+        )
         self.user_w_map = UserFactory.create(email='withmap@stanford.edu')
         self.extauth = ExternalAuthMap(external_id='withmap@stanford.edu',
                                        external_email='withmap@stanford.edu',

common/djangoapps/util/testing.py

@@ -30,8 +30,8 @@ class UrlResetMixin(object):
         # Resolve a URL so that the new urlconf gets loaded
         resolve('/')
 
-    def setUp(self):
+    def setUp(self, **kwargs):
         """Reset django default urlconf before tests and after tests"""
-        super(UrlResetMixin, self).setUp()
+        super(UrlResetMixin, self).setUp(**kwargs)
         self._reset_urls()
         self.addCleanup(self._reset_urls)

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -72,6 +72,21 @@ class ModuleStoreEnum(object):
         draft = 'draft-branch'
         published = 'published-branch'
 
+    class UserID(object):
+        """
+        Values for user ID defaults
+        """
+        # Note: we use negative values here to (try to) not collide
+        # with user identifiers provided by actual user services.
+
+        # user ID to use for all management commands
+        mgmt_command = -1
+
+        # user ID to use for primitive commands
+        primitive_command = -2
+
+        # user ID to use for tests that do not have a django user available
+        test = -3
 
 class PublishState(object):
     """
@@ -270,6 +285,18 @@ class ModuleStoreRead(object):
         """
         pass
 
+    @abstractmethod
+    def compute_publish_state(self, xblock):
+        """
+        Returns whether this xblock is draft, public, or private.
+
+        Returns:
+            PublishState.draft - content is in the process of being edited, but still has a previous
+                version deployed to LMS
+            PublishState.public - content is locked and deployed to LMS
+            PublishState.private - content is editable and not deployed to LMS
+        """
+        pass
 
 class ModuleStoreWrite(ModuleStoreRead):
     """
@@ -280,7 +307,7 @@ class ModuleStoreWrite(ModuleStoreRead):
     __metaclass__ = ABCMeta
 
     @abstractmethod
-    def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
+    def update_item(self, xblock, user_id, 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.
@@ -296,7 +323,7 @@ class ModuleStoreWrite(ModuleStoreRead):
         pass
 
     @abstractmethod
-    def delete_item(self, location, user_id=None, **kwargs):
+    def delete_item(self, location, user_id, **kwargs):
         """
         Delete an item and its subtree from persistence. Remove the item from any parents (Note, does not
         affect parents from other branches or logical branches; thus, in old mongo, deleting something
@@ -315,7 +342,7 @@ class ModuleStoreWrite(ModuleStoreRead):
         pass
 
     @abstractmethod
-    def create_course(self, org, offering, user_id=None, fields=None, **kwargs):
+    def create_course(self, org, offering, user_id, fields=None, **kwargs):
         """
         Creates and returns the course.
 
@@ -348,7 +375,7 @@ class ModuleStoreWrite(ModuleStoreRead):
         pass
 
     @abstractmethod
-    def delete_course(self, course_key, user_id=None):
+    def delete_course(self, course_key, user_id):
         """
         Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions
         depending on the persistence layer and how tightly bound the xblocks are to the course.
@@ -441,6 +468,12 @@ class ModuleStoreReadBase(ModuleStoreRead):
                 None
             )
 
+    def compute_publish_state(self, xblock):
+        """
+        Returns PublishState.public since this is a read-only store.
+        """
+        return PublishState.public
+
     def heartbeat(self):
         """
         Is this modulestore ready?
@@ -499,7 +532,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
             result[field.scope][field_name] = value
         return result
 
-    def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
+    def update_item(self, xblock, user_id, 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.
@@ -514,7 +547,7 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
         """
         raise NotImplementedError
 
-    def delete_item(self, location, user_id=None, force=False):
+    def delete_item(self, location, user_id, force=False):
         """
         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.
@@ -553,6 +586,15 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
         super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id)
         return dest_course_id
 
+    def delete_course(self, course_key, user_id):
+        """
+        This base method just deletes the assets. The lower level impls must do the actual deleting of
+        content.
+        """
+        # delete the assets
+        self.contentstore.delete_all_course_assets(course_key)
+        super(ModuleStoreWriteBase, self).delete_course(course_key, user_id)
+
     @contextmanager
     def bulk_write_operations(self, course_id):
         """

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

@@ -229,16 +229,13 @@ class MixedModuleStore(ModuleStoreWriteBase):
         store = self._get_modulestore_for_courseid(course_id)
         return store.has_course(course_id, ignore_case)
 
-    def delete_course(self, course_key, user_id=None):
+    def delete_course(self, course_key, user_id):
         """
         See xmodule.modulestore.__init__.ModuleStoreWrite.delete_course
         """
         assert(isinstance(course_key, CourseKey))
         store = self._get_modulestore_for_courseid(course_key)
-        if hasattr(store, 'delete_course'):
-            return store.delete_course(course_key, user_id)
-        else:
-            raise NotImplementedError(u"Cannot delete a course on store {}".format(store))
+        return store.delete_course(course_key, user_id)
 
     def get_parent_location(self, location, **kwargs):
         """
@@ -276,7 +273,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
             errs.update(store.get_errored_courses())
         return errs
 
-    def create_course(self, org, offering, user_id=None, fields=None, **kwargs):
+    def create_course(self, org, offering, user_id, fields=None, **kwargs):
         """
         Creates and returns the course.
 
@@ -290,10 +287,6 @@ class MixedModuleStore(ModuleStoreWriteBase):
         Returns: a CourseDescriptor
         """
         store = self._get_modulestore_for_courseid(None)
-
-        if not hasattr(store, 'create_course'):
-            raise NotImplementedError(u"Cannot create a course on store {}".format(store))
-
         return store.create_course(org, offering, user_id, fields, **kwargs)
 
     def clone_course(self, source_course_id, dest_course_id, user_id):
@@ -326,7 +319,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
                 source_course_id, user_id, dest_course_id.org, dest_course_id.offering
             )
 
-    def create_item(self, course_or_parent_loc, category, user_id=None, **kwargs):
+    def create_item(self, course_or_parent_loc, category, user_id, **kwargs):
         """
         Create and return the item. If parent_loc is a specific location v a course id,
         it installs the new item as a child of the parent (if the parent_loc is a specific
@@ -353,7 +346,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
             if parent_loc is not None and not 'detached' in xblock._class_tags:
                 parent = store.get_item(parent_loc)
                 parent.children.append(location)
-                store.update_item(parent)
+                store.update_item(parent, user_id)
         elif isinstance(store, SplitMongoModuleStore):
             if not isinstance(course_or_parent_loc, (CourseLocator, BlockUsageLocator)):
                 raise ValueError(u"Cannot create a child of {} in split. Wrong repr.".format(course_or_parent_loc))
@@ -378,7 +371,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         store = self._verify_modulestore_support(xblock.location, 'update_item')
         return store.update_item(xblock, user_id, allow_not_found)
 
-    def delete_item(self, location, user_id=None, **kwargs):
+    def delete_item(self, location, user_id, **kwargs):
         """
         Delete the given item from persistence. kwargs allow modulestore specific parameters.
         """
@@ -456,13 +449,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         """
         course_id = xblock.scope_ids.usage_id.course_key
         store = self._get_modulestore_for_courseid(course_id)
-        if hasattr(store, 'compute_publish_state'):
-            return store.compute_publish_state(xblock)
-        elif hasattr(store, 'publish'):
-            raise NotImplementedError(u"Cannot compute_publish_state on store {}".format(store))
-        else:
-            # read-only store; so, everything's public
-            return PublishState.public
+        return store.compute_publish_state(xblock)
 
     def publish(self, location, user_id):
         """

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

@@ -841,7 +841,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         modules = self._load_items(course_id, list(items))
         return modules
 
-    def create_course(self, org, offering, user_id=None, fields=None, **kwargs):
+    def create_course(self, org, offering, user_id, fields=None, **kwargs):
         """
         Creates and returns the course.
 
@@ -892,15 +892,6 @@ class MongoModuleStore(ModuleStoreWriteBase):
 
         return course
 
-    def delete_course(self, course_key, user_id=None):
-        """
-        The impl removes all of the db records for the course.
-        :param course_key:
-        :param user_id:
-        """
-        course_query = self._course_key_to_son(course_key)
-        self.collection.remove(course_query, multi=True)
-
     def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}):
         """
         Create the new xmodule but don't save it. Returns the new module.
@@ -992,7 +983,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
             self._update_single_item(parent, update)
             self._update_ancestors(parent, update)
 
-    def update_item(self, xblock, user_id=None, allow_not_found=False, force=False, isPublish=False,
+    def update_item(self, xblock, user_id, allow_not_found=False, force=False, isPublish=False,
                     is_publish_root=True):
         """
         Update the persisted version of xblock to reflect its current values.

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

@@ -144,6 +144,18 @@ class DraftModuleStore(MongoModuleStore):
             del key['_id.revision']
             return self.collection.find(key).count() > 0
 
+    def delete_course(self, course_key, user_id):
+        """
+        :param course_key: which course to delete
+        :param user_id: id of the user deleting the course
+        """
+        # delete the assets
+        super(DraftModuleStore, self).delete_course(course_key, user_id)
+
+        # delete all of the db records for the course
+        course_query = self._course_key_to_son(course_key)
+        self.collection.remove(course_query, multi=True)
+
     def clone_course(self, source_course_id, dest_course_id, user_id):
         """
         Only called if cloning within this store or if env doesn't set up mixed.
@@ -419,7 +431,7 @@ class DraftModuleStore(MongoModuleStore):
         # get_item will wrap_draft so don't call it here (otherwise, it would override the is_draft attribute)
         return self.get_item(location)
 
-    def update_item(self, xblock, user_id=None, allow_not_found=False, force=False, isPublish=False):
+    def update_item(self, xblock, user_id, allow_not_found=False, force=False, isPublish=False):
         """
         See superclass doc.
         In addition to the superclass's behavior, this method converts the unit to draft if it's not

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

@@ -1380,7 +1380,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
 
         return result
 
-    def delete_course(self, course_key, user_id=None):
+    def delete_course(self, course_key, user_id):
         """
         Remove the given course from the course index.
 
@@ -1395,6 +1395,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         log.info(u"deleting course from split-mongo: %s", course_key)
         self.db_connection.delete_course_index(index)
 
+        # We do NOT call the super class here since we need to keep the assets
+        # in case the course is later restored.
+        # super(SplitMongoModuleStore, self).delete_course(course_key, user_id)
+
     def revert_to_published(self, location, user_id=None):
         """
         Reverts an item to its last published version (recursively traversing all of its descendants).
@@ -1407,13 +1411,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         """
         raise NotImplementedError()
 
-    def get_errored_courses(self):
-        """
-        This function doesn't make sense for the mongo modulestore, as structures
-        are loaded on demand, rather than up front
-        """
-        return {}
-
     def inherit_settings(self, block_map, block_json, inheriting_settings=None):
         """
         Updates block_json with any inheritable setting set by an ancestor and recurses to children.

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

@@ -85,25 +85,3 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text):
             logging.warning("Error producing regex substitution %r for text = %r.\n\nError msg = %s", source_course_id, text, str(exc))
 
     return text
-
-
-def delete_course(modulestore, contentstore, course_key, commit=False):
-    """
-    This method will actually do the work to delete all content in a course in a MongoDB backed
-    courseware store. BE VERY CAREFUL, this is not reversable.
-    """
-
-    # check to see if the source course is actually there
-    if not modulestore.has_course(course_key):
-        raise Exception("Cannot find a course at {0}. Aborting".format(course_key))
-
-    if commit:
-        print "Deleting assets and thumbnails {}".format(course_key)
-        contentstore.delete_all_course_assets(course_key)
-
-    # finally delete the course
-    print "Deleting {0}...".format(course_key)
-    if commit:
-        modulestore.delete_course(course_key, '**replace-user**')
-
-    return True

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

@@ -4,8 +4,10 @@ Modulestore configuration for test cases.
 
 from uuid import uuid4
 from django.test import TestCase
+from django.contrib.auth.models import User
 from xmodule.modulestore.django import (
-    modulestore, clear_existing_modulestores, loc_mapper)
+    modulestore, clear_existing_modulestores, loc_mapper
+)
 from xmodule.modulestore import ModuleStoreEnum
 
 
@@ -126,18 +128,62 @@ class ModuleStoreTestCase(TestCase):
           `clear_existing_modulestores()` directly in
           your `setUp()` method.
     """
+    def setUp(self, **kwargs):
+        """
+        Creates a test User if `create_user` is True.
+        Returns the password for the test User.
 
-    @staticmethod
-    def update_course(course):
+        Args:
+            create_user - specifies whether or not to create a test User.  Default is True.
+        """
+        super(ModuleStoreTestCase, self).setUp()
+
+        self.store = modulestore()
+
+        uname = 'testuser'
+        email = 'test+courses@edx.org'
+        password = 'foo'
+
+        if kwargs.pop('create_user', True):
+            # Create the user so we can log them in.
+            self.user = User.objects.create_user(uname, email, password)
+
+            # Note that we do not actually need to do anything
+            # for registration if we directly mark them active.
+            self.user.is_active = True
+
+            # Staff has access to view all courses
+            self.user.is_staff = True
+            self.user.save()
+
+        return password
+
+    def create_non_staff_user(self):
+        """
+        Creates a non-staff test user.
+        Returns the non-staff test user and its password.
+        """
+        uname = 'teststudent'
+        password = 'foo'
+        nonstaff_user = User.objects.create_user(uname, 'test+student@edx.org', password)
+
+        # Note that we do not actually need to do anything
+        # for registration if we directly mark them active.
+        nonstaff_user.is_active = True
+        nonstaff_user.is_staff = False
+        nonstaff_user.save()
+        return nonstaff_user, password
+
+    def update_course(self, course, user_id):
         """
         Updates the version of course in the modulestore
 
         'course' is an instance of CourseDescriptor for which we want
         to update metadata.
         """
-        store = modulestore()
-        store.update_item(course, '**replace_user**')
-        updated_course = store.get_course(course.id)
+        with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id):
+            self.store.update_item(course, user_id)
+        updated_course = self.store.get_course(course.id)
         return updated_course
 
     @staticmethod

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

@@ -2,7 +2,7 @@ from factory import Factory, lazy_attribute_sequence, lazy_attribute
 from factory.containers import CyclicDefinitionError
 from uuid import uuid4
 
-from xmodule.modulestore import prefer_xmodules
+from xmodule.modulestore import prefer_xmodules, ModuleStoreEnum
 from opaque_keys.edx.locations import Location
 from xblock.core import XBlock
 from xmodule.tabs import StaticTab
@@ -52,21 +52,23 @@ class CourseFactory(XModuleFactory):
         store = kwargs.pop('modulestore')
         name = kwargs.get('name', kwargs.get('run', Location.clean(kwargs.get('display_name'))))
         run = kwargs.get('run', name)
+        user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test)
 
         location = Location(org, number, run, 'course', name)
 
-        # Write the data to the mongo datastore
-        new_course = store.create_xmodule(location, metadata=kwargs.get('metadata', None))
+        with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
+            # Write the data to the mongo datastore
+            new_course = store.create_xmodule(location, metadata=kwargs.get('metadata', None))
 
-        # The rest of kwargs become attributes on the course:
-        for k, v in kwargs.iteritems():
-            setattr(new_course, k, v)
+            # The rest of kwargs become attributes on the course:
+            for k, v in kwargs.iteritems():
+                setattr(new_course, k, v)
 
-        # Save the attributes we just set
-        new_course.save()
-        # Update the data in the mongo datastore
-        store.update_item(new_course, '**replace_user**')
-        return new_course
+            # Save the attributes we just set
+            new_course.save()
+            # Update the data in the mongo datastore
+            store.update_item(new_course, user_id)
+            return new_course
 
 
 class ItemFactory(XModuleFactory):
@@ -128,6 +130,8 @@ class ItemFactory(XModuleFactory):
 
         :boilerplate: (optional) the boilerplate for overriding field values
 
+        :publish_item: (optional) whether or not to publish the item (default is True)
+
         :target_class: is ignored
         """
 
@@ -143,7 +147,8 @@ class ItemFactory(XModuleFactory):
         display_name = kwargs.pop('display_name', None)
         metadata = kwargs.pop('metadata', {})
         location = kwargs.pop('location')
-        user_id = kwargs.pop('user_id', 999)
+        user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test)
+        publish_item = kwargs.pop('publish_item', True)
 
         assert isinstance(location, Location)
         assert location != parent_location
@@ -153,47 +158,55 @@ class ItemFactory(XModuleFactory):
         # This code was based off that in cms/djangoapps/contentstore/views.py
         parent = kwargs.pop('parent', None) or store.get_item(parent_location)
 
-        if 'boilerplate' in kwargs:
-            template_id = kwargs.pop('boilerplate')
-            clz = XBlock.load_class(category, select=prefer_xmodules)
-            template = clz.get_template(template_id)
-            assert template is not None
-            metadata.update(template.get('metadata', {}))
-            if not isinstance(data, basestring):
-                data.update(template.get('data'))
-
-        # replace the display name with an optional parameter passed in from the caller
-        if display_name is not None:
-            metadata['display_name'] = display_name
-        runtime = parent.runtime if parent else None
-        store.create_and_save_xmodule(location, user_id, metadata=metadata, definition_data=data, runtime=runtime)
-
-        module = store.get_item(location)
-
-        for attr, val in kwargs.items():
-            setattr(module, attr, val)
-        # Save the attributes we just set
-        module.save()
-
-        store.update_item(module, '**replace_user**')
-
-        if 'detached' not in module._class_tags:
-            parent.children.append(location)
-            store.update_item(parent, '**replace_user**')
-
-        # 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
-        if category == 'static_tab':
-            course = store.get_course(location.course_key)
-            course.tabs.append(
-                StaticTab(
-                    name=display_name,
-                    url_slug=location.name,
+        with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
+
+            if 'boilerplate' in kwargs:
+                template_id = kwargs.pop('boilerplate')
+                clz = XBlock.load_class(category, select=prefer_xmodules)
+                template = clz.get_template(template_id)
+                assert template is not None
+                metadata.update(template.get('metadata', {}))
+                if not isinstance(data, basestring):
+                    data.update(template.get('data'))
+
+            # replace the display name with an optional parameter passed in from the caller
+            if display_name is not None:
+                metadata['display_name'] = display_name
+            runtime = parent.runtime if parent else None
+            store.create_and_save_xmodule(location, user_id, metadata=metadata, definition_data=data, runtime=runtime)
+
+            module = store.get_item(location)
+
+            for attr, val in kwargs.items():
+                setattr(module, attr, val)
+            # Save the attributes we just set
+            module.save()
+
+            store.update_item(module, user_id)
+
+            # 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
+            if category == 'static_tab':
+                course = store.get_course(location.course_key)
+                course.tabs.append(
+                    StaticTab(
+                        name=display_name,
+                        url_slug=location.name,
+                    )
                 )
-            )
-            store.update_item(course, '**replace_user**')
-
+                store.update_item(course, user_id)
+
+            # parent and publish the item, so it can be accessed
+            if 'detached' not in module._class_tags:
+                parent.children.append(location)
+                store.update_item(parent, user_id)
+                if publish_item:
+                    store.publish(parent.location, user_id)
+            elif publish_item:
+                store.publish(location, user_id)
+
+        # return the published item
         return store.get_item(location)
 
 

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

@@ -33,7 +33,7 @@ class PersistentCourseFactory(SplitFactory):
 
     # pylint: disable=W0613
     @classmethod
-    def _create(cls, target_class, offering='999', org='testX', user_id='test_user',
+    def _create(cls, target_class, offering='999', org='testX', user_id=ModuleStoreEnum.UserID.test,
                 master_branch=ModuleStoreEnum.BranchName.draft, **kwargs):
 
         modulestore = kwargs.pop('modulestore')
@@ -59,7 +59,7 @@ class ItemFactory(SplitFactory):
     # pylint: disable=W0613
     @classmethod
     def _create(cls, target_class, parent_location, category='chapter',
-                user_id='test_user', block_id=None, definition_locator=None, force=False,
+                user_id=ModuleStoreEnum.UserID.test, block_id=None, definition_locator=None, force=False,
                 continue_version=False, **kwargs):
         """
         passes *kwargs* as the new item's field values:

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

@@ -116,7 +116,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         self.writable_chapter_location = self.store = self.fake_location = self.xml_chapter_location = None
         self.course_locations = []
 
-        self.user_id = 0
+        self.user_id = ModuleStoreEnum.UserID.test
 
     # pylint: disable=invalid-name
     def _create_course(self, default, course_key):
@@ -127,12 +127,12 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
             offering = course_key.offering.replace('/', '.')
         else:
             offering = course_key.offering
-        course = self.store.create_course(course_key.org, offering)
+        course = self.store.create_course(course_key.org, offering, self.user_id)
         category = self.writable_chapter_location.category
         block_id = self.writable_chapter_location.name
         chapter = self.store.create_item(
             # don't use course_location as it may not be the repr
-            course.location, category, location=self.writable_chapter_location, block_id=block_id
+            course.location, category, self.user_id, location=self.writable_chapter_location, block_id=block_id
         )
         if isinstance(course.id, CourseLocator):
             self.course_locations[self.MONGO_COURSEID] = course.location.version_agnostic()
@@ -184,7 +184,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         ]
 
         def create_sub_tree(parent, block_info):
-            block = self.store.create_item(parent.location, category=block_info.category, block_id=block_info.display_name)
+            block = self.store.create_item(parent.location, block_info.category, self.user_id, block_id=block_info.display_name)
             for tree in block_info.sub_tree:
                 create_sub_tree(block, tree)
             # reload the block to update its children field
@@ -306,13 +306,13 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         self.assertFalse(course.show_calculator, "Default changed making test meaningless")
         course.show_calculator = True
         with self.assertRaises(NotImplementedError):  # ensure it doesn't allow writing
-            self.store.update_item(course, None)
+            self.store.update_item(course, self.user_id)
         # now do it for a r/w db
         course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key)
         # if following raised, then the test is really a noop, change it
         self.assertFalse(course.show_calculator, "Default changed making test meaningless")
         course.show_calculator = True
-        self.store.update_item(course, None)
+        self.store.update_item(course, self.user_id)
         course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key)
         self.assertTrue(course.show_calculator)
 
@@ -324,8 +324,8 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         self.initdb(default_ms)
         # r/o try deleting the course (is here to ensure it can't be deleted)
         with self.assertRaises(NotImplementedError):
-            self.store.delete_item(self.xml_chapter_location, 13)
-        self.store.delete_item(self.writable_chapter_location, 9)
+            self.store.delete_item(self.xml_chapter_location, self.user_id)
+        self.store.delete_item(self.writable_chapter_location, self.user_id)
         # verify it's gone
         with self.assertRaises(ItemNotFoundError):
             self.store.get_item(self.writable_chapter_location)
@@ -367,7 +367,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         xml_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml)
         # the important thing is not which exception it raises but that it raises an exception
         with self.assertRaises(AttributeError):
-            xml_store.create_course("org", "course/run", 999)
+            xml_store.create_course("org", "course/run", self.user_id)
 
     @ddt.data('draft', 'split')
     def test_get_course(self, default_ms):
@@ -547,7 +547,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         self.initdb(default_ms)
         # create an orphan
         course_id = self.course_locations[self.MONGO_COURSEID].course_key
-        orphan = self.store.create_item(course_id, 'problem', block_id='orphan')
+        orphan = self.store.create_item(course_id, 'problem', self.user_id, block_id='orphan')
         found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key)
         if default_ms == 'split':
             self.assertEqual(found_orphans, [orphan.location.version_agnostic()])
@@ -561,7 +561,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
         new location for the child
         """
         self.initdb(default_ms)
-        self.store.create_item(self.course_locations[self.MONGO_COURSEID], 'problem', block_id='orphan')
+        self.store.create_item(self.course_locations[self.MONGO_COURSEID], 'problem', self.user_id, block_id='orphan')
         orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key)
         self.assertEqual(len(orphans), 0, "unexpected orphans: {}".format(orphans))
 

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

@@ -5,6 +5,7 @@ Tests for split_migrator
 import uuid
 import random
 import mock
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.mongo.base import MongoRevisionKey
 from xmodule.modulestore.loc_mapper_store import LocMapperStore
 from xmodule.modulestore.split_migrator import SplitMigrator
@@ -63,7 +64,7 @@ class TestMigration(SplitWMongoCourseBoostrapper):
         self.create_random_units(False, both_vert_loc)
         draft_both = self.draft_mongo.get_item(both_vert_loc)
         draft_both.display_name = 'Both vertical renamed'
-        self.draft_mongo.update_item(draft_both)
+        self.draft_mongo.update_item(draft_both, ModuleStoreEnum.UserID.test)
         self.create_random_units(True, both_vert_loc)
         # vertical in draft only (x2)
         draft_vert_loc = self.old_course_key.make_usage_key('vertical', uuid.uuid4().hex)

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

@@ -10,7 +10,6 @@ from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
 from xmodule.modulestore.mongo import MongoModuleStore, DraftMongoModuleStore
 from xmodule.modulestore.mongo.draft import DIRECT_ONLY_CATEGORIES
 from xmodule.modulestore import ModuleStoreEnum
-from mock import Mock
 
 
 class SplitWMongoCourseBoostrapper(unittest.TestCase):
@@ -138,6 +137,6 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase):
             self.split_mongo.create_course(
                 self.split_course_key.org, self.split_course_key.offering, self.userid, fields=fields, root_block_id='runid'
             )
-        old_course = self.old_mongo.create_course(self.split_course_key.org, 'test_course/runid', fields=fields)
+        old_course = self.old_mongo.create_course(self.split_course_key.org, 'test_course/runid', self.userid, fields=fields)
         self.old_course_key = old_course.id
         self.runtime = old_course.runtime

common/lib/xmodule/xmodule/split_test_module.py

@@ -559,7 +559,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
                 changed = True
 
         if changed:
-            # request does not have a user attribute, so pass None for user.
+            # TODO user.id - to be fixed by Publishing team
             self.system.modulestore.update_item(self, None)
         return Response()
 

common/lib/xmodule/xmodule/video_module/video_module.py

@@ -283,7 +283,7 @@ class VideoDescriptor(VideoFields, VideoStudioViewHandlers, TabsEditingDescripto
         Save module with updated metadata to database."
         """
         self.save()
-        self.runtime.modulestore.update_item(self, user.id if user else None)
+        self.runtime.modulestore.update_item(self, user.id)
 
     @property
     def editable_metadata_fields(self):

lms/djangoapps/branding/tests.py

@@ -28,12 +28,13 @@ class AnonymousIndexPageTest(ModuleStoreTestCase):
     Tests that anonymous users can access the '/' page,  Need courses with start date
     """
     def setUp(self):
-        self.store = modulestore()
+        super(AnonymousIndexPageTest, self).setUp()
         self.factory = RequestFactory()
-        self.course = CourseFactory.create()
-        self.course.days_early_for_beta = 5
-        self.course.enrollment_start = datetime.datetime.now(UTC) + datetime.timedelta(days=3)
-        self.store.update_item(self.course, '**replace_user**')
+        self.course = CourseFactory.create(
+            days_early_for_beta=5,
+            enrollment_start=datetime.datetime.now(UTC)+datetime.timedelta(days=3),
+            user_id=self.user.id,
+        )
 
     @override_settings(FEATURES=FEATURES_WITH_STARTDATE)
     def test_none_user_index_access_with_startdate_fails(self):

lms/djangoapps/courseware/features/annotatable.py

@@ -3,7 +3,7 @@ import textwrap
 from lettuce import world, steps
 from nose.tools import assert_in, assert_equals, assert_true
 
-from common import i_am_registered_for_the_course, visit_scenario_item, publish
+from common import i_am_registered_for_the_course, visit_scenario_item
 
 DATA_TEMPLATE = textwrap.dedent("""\
     <annotatable>
@@ -78,9 +78,6 @@ class AnnotatableSteps(object):
             display_name="Test Annotation Module",
             data=DATA_TEMPLATE.format("\n".join(ANNOTATION_TEMPLATE.format(i) for i in xrange(count)))
         )
-
-        publish(world.scenario_dict['ANNOTATION_VERTICAL'].location)
-
         self.annotations_count = count
 
     def view_component(self, step):
@@ -125,7 +122,6 @@ class AnnotatableSteps(object):
                     )
                 )
             )
-        publish(world.scenario_dict['ANNOTATION_VERTICAL'].location)
 
     def click_reply(self, step, problem):
         r"""I click "Reply to annotation" on passage (?P<problem>\d+)$"""

lms/djangoapps/courseware/features/common.py

@@ -134,10 +134,6 @@ def section_location(course_num):
     return world.scenario_dict['SECTION'].location.replace(course=course_num)
 
 
-def publish(location):
-    modulestore().publish(location, '**replace_user**')
-
-
 def visit_scenario_item(item_key):
     """
     Go to the courseware page containing the item stored in `world.scenario_dict`

lms/djangoapps/courseware/features/conditional.py

@@ -4,7 +4,7 @@
 from lettuce import world, steps
 from nose.tools import assert_in, assert_true  # pylint: disable=no-name-in-module
 
-from common import i_am_registered_for_the_course, visit_scenario_item, publish
+from common import i_am_registered_for_the_course, visit_scenario_item
 from problems_setup import add_problem_to_course, answer_problem
 
 @steps
@@ -67,9 +67,6 @@ class ConditionalSteps(object):
             data='<html><div class="hidden-contents">Hidden Contents</p></html>'
         )
 
-        publish(world.scenario_dict['VERTICAL'].location)
-
-
     def setup_problem_attempts(self, step, not_attempted=None):
         r'that the conditioned problem has (?P<not_attempted>not )?been attempted$'
         visit_scenario_item('CONDITION_SOURCE')

lms/djangoapps/courseware/features/gst.py

@@ -2,7 +2,7 @@
 from lettuce import world, steps
 from nose.tools import assert_in, assert_equals, assert_true
 
-from common import i_am_registered_for_the_course, visit_scenario_item, publish
+from common import i_am_registered_for_the_course, visit_scenario_item
 from problems_setup import add_problem_to_course, answer_problem
 
 

lms/djangoapps/courseware/management/commands/tests/test_dump_course.py

@@ -18,6 +18,7 @@ from courseware.tests.modulestore_config import TEST_DATA_XML_MODULESTORE
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 from courseware.tests.modulestore_config import TEST_DATA_MONGO_MODULESTORE
 
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -56,7 +57,7 @@ class CommandsTestBase(TestCase):
         courses = store.get_courses()
         # NOTE: if xml store owns these, it won't import them into mongo
         if SlashSeparatedCourseKey.from_deprecated_string(TEST_COURSE_ID) not in [c.id for c in courses]:
-            import_from_xml(store, "**replace_user**", DATA_DIR, ['toy', 'simple'])
+            import_from_xml(store, ModuleStoreEnum.UserID.mgmt_command, DATA_DIR, ['toy', 'simple'])
 
         return [course.id for course in store.get_courses()]
 

lms/djangoapps/courseware/tests/__init__.py

@@ -132,6 +132,7 @@ class BaseTestXmodule(ModuleStoreTestCase):
         self.assertTrue(all(self.login_statuses))
 
     def setUp(self):
+        super(BaseTestXmodule, self).setUp()
         self.setup_course()
         self.initialize_module(metadata=self.METADATA, data=self.DATA)
 

lms/djangoapps/courseware/tests/helpers.py

@@ -67,7 +67,7 @@ class LoginEnrollmentTestCase(TestCase):
         self.email = 'foo@test.com'
         self.password = 'bar'
         self.username = 'test'
-        self.create_account(self.username,
+        self.user = self.create_account(self.username,
                             self.email, self.password)
         self.activate_user(self.email)
         self.login(self.email, self.password)
@@ -107,7 +107,9 @@ class LoginEnrollmentTestCase(TestCase):
         data = json.loads(resp.content)
         self.assertEqual(data['success'], True)
         # Check both that the user is created, and inactive
-        self.assertFalse(User.objects.get(email=email).is_active)
+        user = User.objects.get(email=email)
+        self.assertFalse(user.is_active)
+        return user
 
     def activate_user(self, email):
         """

lms/djangoapps/courseware/tests/test_lti_integration.py

@@ -10,7 +10,8 @@ from django.test.utils import override_settings
 from django.core.urlresolvers import reverse
 from django.conf import settings
 
-from xmodule.modulestore.mongo.base import MongoRevisionKey
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.x_module import STUDENT_VIEW
@@ -160,7 +161,8 @@ class TestLTIModuleListing(ModuleStoreTestCase):
             parent_location=self.section2.location,
             display_name="lti draft",
             category="lti",
-            location=self.course.id.make_usage_key('lti', 'lti_published').replace(revision=MongoRevisionKey.draft),
+            location=self.course.id.make_usage_key('lti', 'lti_published'),
+            publish_item=False,
         )
 
     def expected_handler_url(self, handler):

lms/djangoapps/courseware/tests/test_submitting_problems.py

@@ -19,8 +19,6 @@ from django.test.utils import override_settings
 from courseware import grades
 from courseware.models import StudentModule
 
-from xmodule.modulestore.django import modulestore
-
 #import factories and parent testcase modules
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -46,7 +44,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
     def setUp(self):
 
-        super(TestSubmittingProblems, self).setUp()
+        super(TestSubmittingProblems, self).setUp(create_user=False)
         # Create course
         self.course = CourseFactory.create(display_name=self.COURSE_NAME, number=self.COURSE_SLUG)
         assert self.course, "Couldn't load course %r" % self.COURSE_NAME
@@ -64,7 +62,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase):
         """
         Re-fetch the course from the database so that the object being dealt with has everything added to it.
         """
-        self.course = modulestore().get_course(self.course.id)
+        self.course = self.store.get_course(self.course.id)
 
     def problem_location(self, problem_url_name):
         """
@@ -230,8 +228,7 @@ class TestCourseGrader(TestSubmittingProblems):
         """
 
         self.course.grading_policy = grading_policy
-        store = modulestore()
-        store.update_item(self.course, '**replace_user**')
+        self.update_course(self.course, self.student_user.id)
         self.refresh_course()
 
     def get_grade_summary(self):

lms/djangoapps/courseware/tests/test_video_handlers.py

@@ -12,6 +12,7 @@ from webob import Request
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import contentstore
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.x_module import STUDENT_VIEW
 from . import BaseTestXmodule
 from .test_video_xml import SOURCE_XML
@@ -411,7 +412,8 @@ class TestTranscriptTranslationGetDispatch(TestVideo):
         self.course.static_asset_path = 'dummy/static'
         self.course.save()
         store = modulestore()
-        store.update_item(self.course, 'OEoXaMPEzfM')
+        with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
+            store.update_item(self.course, self.user.id)
 
         # Test youtube style en
         request = Request.blank('/translation/en?videoId=12345')

lms/djangoapps/courseware/tests/test_view_authentication.py

@@ -110,6 +110,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         return super(TestViewAuth, self).login(user.email, 'test')
 
     def setUp(self):
+        super(TestViewAuth, self).setUp()
 
         self.course = CourseFactory.create(number='999', display_name='Robot_Super_Course')
         self.courseware_chapter = ItemFactory.create(display_name='courseware')
@@ -296,8 +297,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         tomorrow = now + datetime.timedelta(days=1)
         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.course = self.update_course(self.course, self.user.id)
+        self.test_course = self.update_course(self.test_course, self.user.id)
 
         self.assertFalse(self.course.has_started())
         self.assertFalse(self.test_course.has_started())
@@ -321,8 +322,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         tomorrow = now + datetime.timedelta(days=1)
         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.course = self.update_course(self.course, self.user.id)
+        self.test_course = self.update_course(self.test_course, self.user.id)
 
         self.login(self.instructor_user)
         # Enroll in the classes---can't see courseware otherwise.
@@ -345,8 +346,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
         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.course = self.update_course(self.course, self.user.id)
+        self.test_course = self.update_course(self.test_course, self.user.id)
 
         self.login(self.global_staff_user)
         self.enroll(self.course, True)
@@ -373,8 +374,8 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
         # test_course course's has
         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)
+        self.course = self.update_course(self.course, self.user.id)
+        self.test_course = self.update_course(self.test_course, self.user.id)
 
         # First, try with an enrolled student
         self.login(self.unenrolled_user)

lms/djangoapps/courseware/tests/tests.py

@@ -142,9 +142,8 @@ class TestMongoCoursesLoad(ModuleStoreTestCase, PageLoaderTestCase):
         super(TestMongoCoursesLoad, self).setUp()
         self.setup_user()
 
-        # Import the toy course into a Mongo-backed modulestore
-        self.store = modulestore()
-        import_from_xml(self.store, "**replace_user**", TEST_DATA_DIR, ['toy'])
+        # Import the toy course
+        import_from_xml(self.store, self.user.id, TEST_DATA_DIR, ['toy'])
 
     @mock.patch('xmodule.course_module.requests.get')
     def test_toy_textbooks_loads(self, mock_get):

lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py

@@ -15,10 +15,9 @@ from django.core.management.base import CommandError
 from django.test.utils import override_settings
 
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
-from xmodule.contentstore.django import contentstore
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
-from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 import dashboard.git_import as git_import
 from dashboard.git_import import GitImportError
@@ -161,9 +160,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
                             self.TEST_BRANCH)
 
         # Delete to test branching back to master
-        delete_course(def_ms, contentstore(),
-                      self.TEST_BRANCH_COURSE,
-                      True)
+        def_ms.delete_course(self.TEST_BRANCH_COURSE, ModuleStoreEnum.UserID.test)
         self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
         git_import.add_repo(self.TEST_REPO,
                             repo_dir / 'edx4edx_lite',

lms/djangoapps/dashboard/sysadmin.py

@@ -38,10 +38,8 @@ from external_auth.models import ExternalAuthMap
 from external_auth.views import generate_password
 from student.models import CourseEnrollment, UserProfile, Registration
 import track.views
-from xmodule.contentstore.django import contentstore
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.xml import XMLModuleStore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
@@ -591,9 +589,7 @@ class Courses(SysadminDashboardView):
 
             elif course_found and not is_xml_course:
                 # delete course that is stored with mongodb backend
-                content_store = contentstore()
-                commit = True
-                delete_course(self.def_ms, content_store, course.id, commit)
+                self.def_ms.delete_course(course.id, request.user.id)
                 # don't delete user permission groups, though
                 self.msg += \
                     u"<font color='red'>{0} {1} = {2} ({3})</font>".format(

lms/djangoapps/dashboard/tests/test_sysadmin.py

@@ -54,7 +54,7 @@ class SysadminBaseTestCase(ModuleStoreTestCase):
 
     def setUp(self):
         """Setup test case by adding primary user."""
-        super(SysadminBaseTestCase, self).setUp()
+        super(SysadminBaseTestCase, self).setUp(create_user=False)
         self.user = UserFactory.create(username='test_user',
                                        email='test_user+sysadmin@edx.org',
                                        password='foo')
@@ -129,6 +129,9 @@ class TestSysadmin(SysadminBaseTestCase):
             response = self.client.get(reverse(view))
             self.assertEqual(response.status_code, 302)
 
+        self.user.is_staff = False
+        self.user.save()
+
         logged_in = self.client.login(username=self.user.username,
                                       password='foo')
         self.assertTrue(logged_in)

lms/djangoapps/django_comment_client/base/tests.py

@@ -41,7 +41,7 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
         # Patching the ENABLE_DISCUSSION_SERVICE value affects the contents of urls.py,
         # so we need to call super.setUp() which reloads urls.py (because
         # of the UrlResetMixin)
-        super(ViewsTestCase, self).setUp()
+        super(ViewsTestCase, self).setUp(create_user=False)
 
         # create a course
         self.course = CourseFactory.create(org='MITx', course='999',

lms/djangoapps/instructor_task/tests/test_base.py

@@ -13,6 +13,7 @@ from django.contrib.auth.models import User
 from django.test.utils import override_settings
 
 from capa.tests.response_xml_factory import OptionResponseXMLFactory
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -207,8 +208,10 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         problem_xml = factory.build_xml(**factory_args)
         location = InstructorTaskTestCase.problem_location(problem_url_name)
         item = self.module_store.get_item(location)
-        item.data = problem_xml
-        self.module_store.update_item(item, '**replace_user**')
+        with self.module_store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, location.course_key):
+            item.data = problem_xml
+            self.module_store.update_item(item, self.user.id)
+            self.module_store.publish(location, self.user.id)
 
     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

@@ -17,6 +17,7 @@ from django.core.urlresolvers import reverse
 from capa.tests.response_xml_factory import (CodeResponseXMLFactory,
                                              CustomResponseXMLFactory)
 from xmodule.modulestore.tests.factories import ItemFactory
+from xmodule.modulestore import ModuleStoreEnum
 
 from courseware.model_data import StudentModule
 
@@ -105,6 +106,9 @@ class TestRescoringTask(TestIntegrationTask):
         self.create_student('u4')
         self.logout()
 
+        # set up test user for performing test operations
+        self.setup_user()
+
     def render_problem(self, username, problem_url_name):
         """
         Use ajax interface to request html for a problem.
@@ -294,7 +298,9 @@ class TestRescoringTask(TestIntegrationTask):
                 InstructorTaskModuleTestCase.problem_location(problem_url_name)
             )
             descriptor.data = problem_xml
-            self.module_store.update_item(descriptor, '**replace_user**')
+            with self.module_store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, descriptor.location.course_key):
+                self.module_store.update_item(descriptor, self.user.id)
+                self.module_store.publish(descriptor.location, self.user.id)
         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

lms/envs/test.py

@@ -109,7 +109,6 @@ STATICFILES_DIRS += [
     if os.path.isdir(COMMON_TEST_DATA_ROOT / course_dir)
 ]
 
-MODULESTORE_BRANCH = 'draft-preferred'
 update_module_store_settings(
     MODULESTORE,
     module_store_options={