diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index a460aae3f751444541f898fe4875fa84a7cca963..b0d550228b479ac26a7f916d47bbed4a039ac9c6 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -6,9 +6,9 @@ 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 -from xmodule.modulestore import ModuleStoreEnum class Command(BaseCommand): @@ -38,9 +38,10 @@ class Command(BaseCommand): data=data_dir, courses=course_dirs, dis=do_import_static)) + mstore = modulestore() _, course_items = import_from_xml( - modulestore(), ModuleStoreEnum.UserID.mgmt_command, 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, diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index 60e850af288e32a873347141351acc846944a8b2..8910b31fb5e4a02774937fb8140451b9d2a51f69 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -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', diff --git a/cms/djangoapps/contentstore/tests/test_export_git.py b/cms/djangoapps/contentstore/tests/test_export_git.py index 4d38b29618bcb472983934f5b776626f0774634f..7928a9ed222a6d9ab19fce6d0f04708063b9d2c8 100644 --- a/cms/djangoapps/contentstore/tests/test_export_git.py +++ b/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) diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index 3f8ed0b05da546033b467ef38106a09788cc4bdc..96e121ca9aa424eee9fe06cc05e725b8d00950cd 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/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 diff --git a/cms/djangoapps/contentstore/tests/test_import_draft_order.py b/cms/djangoapps/contentstore/tests/test_import_draft_order.py index 090bf69921478421fd8582be6667d1d861dbc1fe..dd2175b662a08eaa74fbc0363c4d3131abce4eb1 100644 --- a/cms/djangoapps/contentstore/tests/test_import_draft_order.py +++ b/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 diff --git a/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py b/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py index d87419ca3b2d3a9bbee88acd1f5f8a60b55d9107..56a690790817779974e888ab18608acdf3b095ee 100644 --- a/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py +++ b/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') diff --git a/cms/djangoapps/contentstore/tests/test_permissions.py b/cms/djangoapps/contentstore/tests/test_permissions.py index 8398ef57a835e7228135cd7c4b9abbadcfe021fd..0e08f1eb170098e033c904c131850d2f81187946 100644 --- a/cms/djangoapps/contentstore/tests/test_permissions.py +++ b/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') diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index f850c32b8e6b133d60afa5f11599040396ca5e3f..10ed2f580edc66f510a6ca02b5109dd721281f3d 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/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) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 9fddf53bcba3bc20bf69d1cb455e0270040b7637..a7a8932221be947d396d508c0866241b9e89ad56 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/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 = super(CourseTestCase, self).create_non_staff_authed_user_client() client = Client() if authenticate: - client.login(username=uname, password=password) + client.login(username=nonstaff.username, password=password) return client, nonstaff def populate_course(self): diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 63eb9f5aa0c8d1648dbd48d393c4ddd62d9dff4e..5c61efa9f93d2850fe3ac9214ac99aba33bdaa1d 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 4ff410fecbb8f27e37ab39d055077f79c4fd209b..b144a6e73350e47e4075453e873ab1b16d5b0caa 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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, ) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 4de9ad2dee0039e896c82212e92c20791da7fbf7..07d58a1613ff83078c602926b970d72bd30ef825 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/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 diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index 5d81505f30e7a76ac8d6cb6829e432fda94fb0df..40767941fefd59b7ae6d2297d3640505d1d094ac 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/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(), diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index 9c942dace3d73e0706c5fad6b72f9fef0bd6bd92..d1d700f6d2207c95af3b3767fa3e5488704bff63 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -137,7 +137,7 @@ class ContainerPageTestCase(StudioPageTestCase): """ 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**') + 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) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 2d4acc5e4633b70fb093f8decdfde00285888975..5666eeaa855cc1d8c08d02ad8a6a4ec227f5fa6b 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/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): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_unit_page.py b/cms/djangoapps/contentstore/views/tests/test_unit_page.py index 7882ad858ebb10ec0127684fe7cb6c7080dc9b69..967d6628aae3a39e625bc2391ca8d4f21dcc6c1c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_unit_page.py +++ b/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) diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 615895329fcb641f98283a6ec938146574241ff3..0db837f68ba6b63b858e9c7349a5a6e41ed3bd2c 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/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) diff --git a/common/djangoapps/contentserver/tests/test.py b/common/djangoapps/contentserver/tests/test.py index 38b80774a65d7fb4256915df3b5eb34934fc1d3a..bb4aae815e6caf674ce6dbae47f2e90cc3d14298 100644 --- a/common/djangoapps/contentserver/tests/test.py +++ b/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_authed_user_client() + 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 diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index fc312f0c63b49d51a37b66dd16673706822b48f5..79658b4b184d9496c9c1dc66814650a752374024 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -17,7 +17,6 @@ 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 opaque_keys.edx.locations import SlashSeparatedCourseKey @@ -80,7 +79,7 @@ class ShibSPTest(ModuleStoreTestCase): request_factory = RequestFactory() def setUp(self): - self.store = modulestore() + super(ShibSPTest, self).setUp(create_user=False) @unittest.skipUnless(settings.FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set") def test_exception_shib_login(self): @@ -383,7 +382,7 @@ class ShibSPTest(ModuleStoreTestCase): for domain in ["", "shib:https://idp.stanford.edu/"]: # set domains course.enrollment_domain = domain - self.store.update_item(course, '**replace_user**') + self.store.update_item(course, self.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' + @@ -452,11 +451,11 @@ class ShibSPTest(ModuleStoreTestCase): # create 2 course, one with limited enrollment one without shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only') shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/' - self.store.update_item(shib_course, '**replace_user**') + self.store.update_item(shib_course, self.user.id) 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**') + self.store.update_item(open_enroll_course, self.user.id) # create 3 kinds of students, external_auth matching shib_course, external_auth not matching, no external auth shib_student = UserFactory.create() @@ -522,7 +521,7 @@ class ShibSPTest(ModuleStoreTestCase): course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only') course.enrollment_domain = 'shib:https://idp.stanford.edu/' - self.store.update_item(course, '**replace_user**') + self.store.update_item(course, self.user.id) # use django test client for sessions and url processing # no enrollment before trying diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 52a2b93e99d73a16f4ae3668152ad1db45a19888..0cb1fb7847b367dc511fd89f9125c8c05cce6a39 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/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,11 @@ class ExternalAuthShibTest(ModuleStoreTestCase): Tests how login_user() interacts with ExternalAuth, in particular Shib """ def setUp(self): - self.store = modulestore() + super(ExternalAuthShibTest, self).setUp() 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**') + self.store.update_item(self.shib_course, 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', diff --git a/common/djangoapps/util/testing.py b/common/djangoapps/util/testing.py index 062b04c8a02c89f913d4a4ef9c730bbf76133576..fdd61a2a6afbf6a6042b7d9416e379128d0acb26 100644 --- a/common/djangoapps/util/testing.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index bf975dde454b480abe58f4ba4bd4cb26688bf0f3..1d16cafa7a1c99947d28db8a2793a4e9faee0f15 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -76,8 +76,8 @@ class ModuleStoreEnum(object): """ Values for user ID defaults """ - # Note: we use negative values here to (try to) not conflict - # with user identifiers provided by an actual user service. + # 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 @@ -307,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. @@ -323,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 @@ -342,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. @@ -532,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. @@ -547,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. diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index b3622effc4f79da041257a2ab1d26e53264d20a6..708838d6db016e0e2c40ee6a07fa93529ddb5c0c 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -273,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. @@ -319,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 @@ -346,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)) @@ -371,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. """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 74374b241da6c8b48f69ddcb2024f672fbc4d9a6..554c2a8a2b08756b577276fbd2db1d174eb1e9e3 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/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. @@ -983,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. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 1621f0370d3727cde475296fc07ffdd54959cf13..d4a5b197f71753a7b729b53c66c9b5faf3123cd4 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -431,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 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 8792f3b439ce183e032decd9f4e9b38be7205a9c..77453b3f7cdae85d383cf92a489f1eed7fd4b20a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/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,54 @@ class ModuleStoreTestCase(TestCase): `clear_existing_modulestores()` directly in your `setUp()` method. """ + def setUp(self, **kwargs): + super(ModuleStoreTestCase, self).setUp() - @staticmethod - def update_course(course): + 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_authed_user_client(self): + """ + 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() + return nonstaff, 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 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 17a574e8dfc6865b1c106baaf3fb8267121ff254..1a4fc522d241c87b25652162e2709f114e18b8c3 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/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,6 +52,7 @@ 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) @@ -65,7 +66,7 @@ class CourseFactory(XModuleFactory): # Save the attributes we just set new_course.save() # Update the data in the mongo datastore - store.update_item(new_course, '**replace_user**') + store.update_item(new_course, user_id) return new_course @@ -143,7 +144,7 @@ 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) assert isinstance(location, Location) assert location != parent_location @@ -175,11 +176,11 @@ class ItemFactory(XModuleFactory): # Save the attributes we just set module.save() - store.update_item(module, '**replace_user**') + store.update_item(module, user_id) if 'detached' not in module._class_tags: parent.children.append(location) - store.update_item(parent, '**replace_user**') + store.update_item(parent, 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) @@ -192,7 +193,7 @@ class ItemFactory(XModuleFactory): url_slug=location.name, ) ) - store.update_item(course, '**replace_user**') + store.update_item(course, user_id) return store.get_item(location) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py index e7490e14429831812b5a08b2e5ea6a7408c15ccb..899378b5ef1c97b27ef27ec4b57530fdc52551c1 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py +++ b/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: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 9485d9323bd29482627c7c177459d8caf77ed2eb..8add221bc196688482f013bf2dc9e155c9b869ad 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/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)) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index a2ab88f8d1d2cf6ceedcc7adb3574e80747dc0c8..e490b404a2f03e815dbadb810bd4c1957fc910d3 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -134,6 +134,14 @@ class TestMongoModuleStore(unittest.TestCase): # Destroy the test db. connection.drop_database(DB) + def setUp(self): + # make a copy for convenience + self.connection = TestMongoModuleStore.connection + self.dummy_user = ModuleStoreEnum.UserID.test + + def tearDown(self): + pass + def test_init(self): '''Make sure the db loads''' ids = list(self.connection[DB][COLLECTION].find({}, {'_id': True})) @@ -351,7 +359,7 @@ class TestMongoModuleStore(unittest.TestCase): # set toy course to share the wiki with simple course toy_course = self.draft_store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) toy_course.wiki_slug = 'simple' - self.draft_store.update_item(toy_course) + self.draft_store.update_item(toy_course, ModuleStoreEnum.UserID.test) # now toy_course should not be retrievable with old wiki_slug course_locations = self.draft_store.get_courses_for_wiki('toy') @@ -366,7 +374,7 @@ class TestMongoModuleStore(unittest.TestCase): # configure simple course to use unique wiki_slug. simple_course = self.draft_store.get_course(SlashSeparatedCourseKey('edX', 'simple', '2012_Fall')) simple_course.wiki_slug = 'edX.simple.2012_Fall' - self.draft_store.update_item(simple_course) + self.draft_store.update_item(simple_course, ModuleStoreEnum.UserID.test) # it should be retrievable with its new wiki_slug course_locations = self.draft_store.get_courses_for_wiki('edX.simple.2012_Fall') assert_equals(len(course_locations), 1) @@ -491,11 +499,10 @@ class TestMongoModuleStore(unittest.TestCase): """ course_location = Location('edx', 'direct', '2012_Fall', 'course', 'test_course') chapter_location = Location('edx', 'direct', '2012_Fall', 'chapter', 'test_chapter') - dummy_user = 123 # Create dummy direct only xblocks - self.draft_store.create_and_save_xmodule(course_location, user_id=dummy_user) - self.draft_store.create_and_save_xmodule(chapter_location, user_id=dummy_user) + self.draft_store.create_and_save_xmodule(course_location, user_id=self.dummy_user) + self.draft_store.create_and_save_xmodule(chapter_location, user_id=self.dummy_user) # Check that neither xblock has changes self.assertFalse(self.draft_store.has_changes(course_location)) @@ -506,29 +513,28 @@ class TestMongoModuleStore(unittest.TestCase): Tests that has_changes() only returns true when changes are present """ location = Location('edX', 'changes', '2012_Fall', 'vertical', 'test_vertical') - dummy_user = 123 # Create a dummy component to test against - self.draft_store.create_and_save_xmodule(location, user_id=dummy_user) + self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user) # Not yet published, so changes are present self.assertTrue(self.draft_store.has_changes(location)) # Publish and verify that there are no unpublished changes - self.draft_store.publish(location, dummy_user) + self.draft_store.publish(location, self.dummy_user) self.assertFalse(self.draft_store.has_changes(location)) # Change the component, then check that there now are changes component = self.draft_store.get_item(location) component.display_name = 'Changed Display Name' - self.draft_store.update_item(component, dummy_user) + self.draft_store.update_item(component, self.dummy_user) self.assertTrue(self.draft_store.has_changes(location)) # Publish and verify again - self.draft_store.publish(location, dummy_user) + self.draft_store.publish(location, self.dummy_user) self.assertFalse(self.draft_store.has_changes(location)) - def _create_test_tree(self, name, user_id=123): + def _create_test_tree(self, name, user_id=None): """ Creates and returns a tree with the following structure: Grandparent @@ -538,6 +544,9 @@ class TestMongoModuleStore(unittest.TestCase): Child Sibling """ + if user_id is None: + user_id = self.dummy_user + locations = { 'grandparent': Location('edX', 'tree', name, 'chapter', 'grandparent'), 'parent_sibling': Location('edX', 'tree', name, 'sequential', 'parent_sibling'), @@ -566,7 +575,6 @@ class TestMongoModuleStore(unittest.TestCase): """ Tests that has_changes() returns true on ancestors when a child is changed """ - dummy_user = 123 locations = self._create_test_tree('has_changes_ancestors') # Verify that there are no unpublished changes @@ -576,7 +584,7 @@ class TestMongoModuleStore(unittest.TestCase): # Change the child child = self.draft_store.get_item(locations['child']) child.display_name = 'Changed Display Name' - self.draft_store.update_item(child, user_id=dummy_user) + self.draft_store.update_item(child, user_id=self.dummy_user) # All ancestors should have changes, but not siblings self.assertTrue(self.draft_store.has_changes(locations['grandparent'])) @@ -586,7 +594,7 @@ class TestMongoModuleStore(unittest.TestCase): self.assertFalse(self.draft_store.has_changes(locations['child_sibling'])) # Publish the unit with changes - self.draft_store.publish(locations['parent'], dummy_user) + self.draft_store.publish(locations['parent'], self.dummy_user) # Verify that there are no unpublished changes for key in locations: @@ -596,7 +604,6 @@ class TestMongoModuleStore(unittest.TestCase): """ Tests that has_changes() returns false after a child is published only if all children are unchanged """ - dummy_user = 123 locations = self._create_test_tree('has_changes_publish_ancestors') # Verify that there are no unpublished changes @@ -608,22 +615,22 @@ class TestMongoModuleStore(unittest.TestCase): child_sibling = self.draft_store.get_item(locations['child_sibling']) child.display_name = 'Changed Display Name' child_sibling.display_name = 'Changed Display Name' - self.draft_store.update_item(child, user_id=dummy_user) - self.draft_store.update_item(child_sibling, user_id=dummy_user) + self.draft_store.update_item(child, user_id=self.dummy_user) + self.draft_store.update_item(child_sibling, user_id=self.dummy_user) # Verify that ancestors have changes self.assertTrue(self.draft_store.has_changes(locations['grandparent'])) self.assertTrue(self.draft_store.has_changes(locations['parent'])) # Publish one child - self.draft_store.publish(locations['child_sibling'], dummy_user) + self.draft_store.publish(locations['child_sibling'], self.dummy_user) # Verify that ancestors still have changes self.assertTrue(self.draft_store.has_changes(locations['grandparent'])) self.assertTrue(self.draft_store.has_changes(locations['parent'])) # Publish the other child - self.draft_store.publish(locations['child'], dummy_user) + self.draft_store.publish(locations['child'], self.dummy_user) # Verify that ancestors now have no changes self.assertFalse(self.draft_store.has_changes(locations['grandparent'])) @@ -634,7 +641,6 @@ class TestMongoModuleStore(unittest.TestCase): Tests that has_changes() returns true for the parent when a child with changes is added and false when that child is removed. """ - dummy_user = 123 locations = self._create_test_tree('has_changes_add_remove_child') # Test that the ancestors don't have changes @@ -643,10 +649,10 @@ class TestMongoModuleStore(unittest.TestCase): # Create a new child and attach it to parent new_child_location = Location('edX', 'tree', 'has_changes_add_remove_child', 'vertical', 'new_child') - self.draft_store.create_and_save_xmodule(new_child_location, user_id=dummy_user) + self.draft_store.create_and_save_xmodule(new_child_location, user_id=self.dummy_user) parent = self.draft_store.get_item(locations['parent']) parent.children += [new_child_location] - self.draft_store.update_item(parent, user_id=dummy_user) + self.draft_store.update_item(parent, user_id=self.dummy_user) # Verify that the ancestors now have changes self.assertTrue(self.draft_store.has_changes(locations['grandparent'])) @@ -655,7 +661,7 @@ class TestMongoModuleStore(unittest.TestCase): # Remove the child from the parent parent = self.draft_store.get_item(locations['parent']) parent.children = [locations['child'], locations['child_sibling']] - self.draft_store.update_item(parent, user_id=dummy_user) + self.draft_store.update_item(parent, user_id=self.dummy_user) # Verify that ancestors now have no changes self.assertFalse(self.draft_store.has_changes(locations['grandparent'])) @@ -665,15 +671,14 @@ class TestMongoModuleStore(unittest.TestCase): """ Tests that has_changes() returns true after editing the child of a vertical (both not direct only categories). """ - dummy_user = 123 parent_location = Location('edX', 'test', 'non_direct_only_children', 'vertical', 'parent') child_location = Location('edX', 'test', 'non_direct_only_children', 'html', 'child') - parent = self.draft_store.create_and_save_xmodule(parent_location, user_id=dummy_user) - child = self.draft_store.create_and_save_xmodule(child_location, user_id=dummy_user) + parent = self.draft_store.create_and_save_xmodule(parent_location, user_id=self.dummy_user) + child = self.draft_store.create_and_save_xmodule(child_location, user_id=self.dummy_user) parent.children += [child_location] - self.draft_store.update_item(parent, user_id=dummy_user) - self.draft_store.publish(parent_location, dummy_user) + self.draft_store.update_item(parent, user_id=self.dummy_user) + self.draft_store.publish(parent_location, self.dummy_user) # Verify that there are no changes self.assertFalse(self.draft_store.has_changes(parent_location)) @@ -681,7 +686,7 @@ class TestMongoModuleStore(unittest.TestCase): # Change the child child.display_name = 'Changed Display Name' - self.draft_store.update_item(child, user_id=123) + self.draft_store.update_item(child, user_id=self.dummy_user) # Verify that both parent and child have changes self.assertTrue(self.draft_store.has_changes(parent_location)) @@ -693,7 +698,7 @@ class TestMongoModuleStore(unittest.TestCase): """ create_user = 123 edit_user = 456 - locations = self._create_test_tree('update_edit_info_ancestors', create_user) + locations =self._create_test_tree('update_edit_info_ancestors', create_user) def check_node(location_key, after, before, edited_by, subtree_after, subtree_before, subtree_by): """ @@ -739,24 +744,23 @@ class TestMongoModuleStore(unittest.TestCase): Tests that edited_on and edited_by are set correctly during an update """ location = Location('edX', 'editInfoTest', '2012_Fall', 'html', 'test_html') - dummy_user = 123 # Create a dummy component to test against - self.draft_store.create_and_save_xmodule(location, user_id=dummy_user) + self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user) # Store the current edit time and verify that dummy_user created the component component = self.draft_store.get_item(location) - self.assertEqual(component.edited_by, dummy_user) + self.assertEqual(component.edited_by, self.dummy_user) old_edited_on = component.edited_on # Change the component component.display_name = component.display_name + ' Changed' - self.draft_store.update_item(component, dummy_user) + self.draft_store.update_item(component, self.dummy_user) updated_component = self.draft_store.get_item(location) # Verify the ordering of edit times and that dummy_user made the edit self.assertLess(old_edited_on, updated_component.edited_on) - self.assertEqual(updated_component.edited_by, dummy_user) + self.assertEqual(updated_component.edited_by, self.dummy_user) def test_update_published_info(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index 46c9c60e31f8b5e0ed07c02276f73c0b1ab155a2..e573acc78ee7cca3e299d0ff63e7e33030f0a4f5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 3345827215c10d6a2b57941b86ba34004b8b1339..57e5447186e359c1b1b431372f2e52f56daa51a3 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -441,6 +441,7 @@ class SplitModuleTest(unittest.TestCase): } }, } + @staticmethod def bootstrapDB(): ''' @@ -489,6 +490,9 @@ class SplitModuleTest(unittest.TestCase): destination = CourseLocator(org="testx", offering="wonderful", branch=BRANCH_NAME_PUBLISHED) split_store.xblock_publish("test@edx.org", to_publish, destination, [to_publish], None) + def setUp(self): + self.user_id = random.getrandbits(32) + def tearDown(self): """ Clear persistence between each test. @@ -817,7 +821,7 @@ class SplitModuleItemTests(SplitModuleTest): draft_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_DRAFT) published_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED) head = draft_course.make_usage_key('course', 'head12345') - dummy_user = 'testUser' + dummy_user = ModuleStoreEnum.UserID.test # Not yet published, so changes are present self.assertTrue(modulestore().has_changes(head)) @@ -1217,7 +1221,7 @@ class TestItemCrud(SplitModuleTest): problem.max_attempts = 4 problem.save() # decache above setting into the kvs - updated_problem = modulestore().update_item(problem, '**replace_user**') + updated_problem = modulestore().update_item(problem, self.user_id) # check that course version changed and course's previous is the other one self.assertEqual(updated_problem.definition_locator.definition_id, pre_def_id) self.assertNotEqual(updated_problem.location.version_guid, pre_version_guid) @@ -1232,7 +1236,7 @@ class TestItemCrud(SplitModuleTest): history_info = modulestore().get_course_history_info(current_course.location) self.assertEqual(history_info['previous_version'], pre_version_guid) - self.assertEqual(history_info['edited_by'], "**replace_user**") + self.assertEqual(history_info['edited_by'], self.user_id) def test_update_children(self): """ @@ -1249,7 +1253,7 @@ class TestItemCrud(SplitModuleTest): self.assertGreater(len(block.children), 0, "meaningless test") moved_child = block.children.pop() block.save() # decache model changes - updated_problem = modulestore().update_item(block, '**replace_user**') + updated_problem = modulestore().update_item(block, self.user_id) # check that course version changed and course's previous is the other one self.assertEqual(updated_problem.definition_locator.definition_id, pre_def_id) self.assertNotEqual(updated_problem.location.version_guid, pre_version_guid) @@ -1258,7 +1262,7 @@ class TestItemCrud(SplitModuleTest): locator = locator.course_key.make_usage_key('Chapter', "chapter1") other_block = modulestore().get_item(locator) other_block.children.append(moved_child) - other_updated = modulestore().update_item(other_block, '**replace_user**') + other_updated = modulestore().update_item(other_block, self.user_id) self.assertIn(moved_child.version_agnostic(), version_agnostic(other_updated.children)) def test_update_definition(self): @@ -1274,7 +1278,7 @@ class TestItemCrud(SplitModuleTest): block.grading_policy['GRADER'][0]['min_count'] = 13 block.save() # decache model changes - updated_block = modulestore().update_item(block, '**replace_user**') + updated_block = modulestore().update_item(block, self.user_id) self.assertNotEqual(updated_block.definition_locator.definition_id, pre_def_id) self.assertNotEqual(updated_block.location.version_guid, pre_version_guid) @@ -1320,7 +1324,7 @@ class TestItemCrud(SplitModuleTest): block.advertised_start = "Soon" block.save() # decache model changes - updated_block = modulestore().update_item(block, "**replace_user**") + updated_block = modulestore().update_item(block, self.user_id) self.assertNotEqual(updated_block.definition_locator.definition_id, pre_def_id) self.assertNotEqual(updated_block.location.version_guid, pre_version_guid) self.assertEqual(updated_block.grading_policy['GRADER'][0]['min_count'], 13) @@ -1330,13 +1334,13 @@ class TestItemCrud(SplitModuleTest): def test_delete_item(self): course = self.create_course_for_deletion() with self.assertRaises(ValueError): - modulestore().delete_item(course.location, 'deleting_user') + modulestore().delete_item(course.location, self.user_id) reusable_location = course.id.version_agnostic().for_branch(BRANCH_NAME_DRAFT) # delete a leaf problems = modulestore().get_items(reusable_location, category='problem') locn_to_del = problems[0].location - new_course_loc = modulestore().delete_item(locn_to_del, 'deleting_user') + new_course_loc = modulestore().delete_item(locn_to_del, self.user_id) deleted = locn_to_del.version_agnostic() self.assertFalse(modulestore().has_item(deleted)) with self.assertRaises(VersionConflictError): @@ -1347,7 +1351,7 @@ class TestItemCrud(SplitModuleTest): # delete a subtree nodes = modulestore().get_items(reusable_location, category='chapter') - new_course_loc = modulestore().delete_item(nodes[0].location, 'deleting_user') + new_course_loc = modulestore().delete_item(nodes[0].location, self.user_id) # check subtree def check_subtree(node): @@ -1582,7 +1586,6 @@ class TestPublish(SplitModuleTest): """ def setUp(self): SplitModuleTest.setUp(self) - self.user = random.getrandbits(32) def tearDown(self): SplitModuleTest.tearDown(self) @@ -1597,14 +1600,14 @@ class TestPublish(SplitModuleTest): chapter1 = source_course.make_usage_key('chapter', 'chapter1') chapter2 = source_course.make_usage_key('chapter', 'chapter2') chapter3 = source_course.make_usage_key('chapter', 'chapter3') - modulestore().xblock_publish(self.user, source_course, dest_course, [head], [chapter2, chapter3]) + modulestore().xblock_publish(self.user_id, source_course, dest_course, [head], [chapter2, chapter3]) expected = [head.block_id, chapter1.block_id] self._check_course( source_course, dest_course, expected, [chapter2.block_id, chapter3.block_id, "problem1", "problem3_2"] ) # add a child under chapter1 new_module = modulestore().create_item( - chapter1, "sequential", self.user, + chapter1, "sequential", self.user_id, fields={'display_name': 'new sequential'}, ) # remove chapter1 from expected b/c its pub'd version != the source anymore since source changed @@ -1613,7 +1616,7 @@ class TestPublish(SplitModuleTest): with self.assertRaises(ItemNotFoundError): modulestore().get_item(new_module.location.map_into_course(dest_course)) # publish it - modulestore().xblock_publish(self.user, source_course, dest_course, [new_module.location], None) + modulestore().xblock_publish(self.user_id, source_course, dest_course, [new_module.location], None) expected.append(new_module.location.block_id) # check that it is in the published course and that its parent is the chapter pub_module = modulestore().get_item(new_module.location.map_into_course(dest_course)) @@ -1622,10 +1625,10 @@ class TestPublish(SplitModuleTest): ) # ensure intentionally orphaned blocks work (e.g., course_info) new_module = modulestore().create_item( - source_course, "course_info", self.user, block_id="handouts" + source_course, "course_info", self.user_id, block_id="handouts" ) # publish it - modulestore().xblock_publish(self.user, source_course, dest_course, [new_module.location], None) + modulestore().xblock_publish(self.user_id, source_course, dest_course, [new_module.location], None) expected.append(new_module.location.block_id) # check that it is in the published course (no error means it worked) pub_module = modulestore().get_item(new_module.location.map_into_course(dest_course)) @@ -1644,15 +1647,15 @@ class TestPublish(SplitModuleTest): chapter3 = source_course.make_usage_key('chapter', 'chapter3') problem1 = source_course.make_usage_key('problem', 'problem1') with self.assertRaises(ItemNotFoundError): - modulestore().xblock_publish(self.user, source_course, destination_course, [chapter3], None) + modulestore().xblock_publish(self.user_id, source_course, destination_course, [chapter3], None) # publishing into a new branch w/o publishing the root destination_course = CourseLocator(org='testx', offering='GreekHero', branch=BRANCH_NAME_PUBLISHED) with self.assertRaises(ItemNotFoundError): - modulestore().xblock_publish(self.user, source_course, destination_course, [chapter3], None) + modulestore().xblock_publish(self.user_id, source_course, destination_course, [chapter3], None) # publishing a subdag w/o the parent already in course - modulestore().xblock_publish(self.user, source_course, destination_course, [head], [chapter3]) + modulestore().xblock_publish(self.user_id, source_course, destination_course, [head], [chapter3]) with self.assertRaises(ItemNotFoundError): - modulestore().xblock_publish(self.user, source_course, destination_course, [problem1], []) + modulestore().xblock_publish(self.user_id, source_course, destination_course, [problem1], []) def test_move_delete(self): """ @@ -1663,7 +1666,7 @@ class TestPublish(SplitModuleTest): head = source_course.make_usage_key('course', "head12345") chapter2 = source_course.make_usage_key('chapter', 'chapter2') problem1 = source_course.make_usage_key('problem', 'problem1') - modulestore().xblock_publish(self.user, source_course, dest_course, [head], [chapter2]) + modulestore().xblock_publish(self.user_id, source_course, dest_course, [head], [chapter2]) expected = ["head12345", "chapter1", "chapter3", "problem1", "problem3_2"] self._check_course(source_course, dest_course, expected, ["chapter2"]) # now move problem1 and delete problem3_2 @@ -1671,8 +1674,8 @@ class TestPublish(SplitModuleTest): chapter3 = modulestore().get_item(source_course.make_usage_key("chapter", "chapter3")) chapter1.children.append(problem1) chapter3.children.remove(problem1.map_into_course(chapter3.location.course_key)) - modulestore().delete_item(source_course.make_usage_key("problem", "problem3_2"), self.user) - modulestore().xblock_publish(self.user, source_course, dest_course, [head], [chapter2]) + modulestore().delete_item(source_course.make_usage_key("problem", "problem3_2"), self.user_id) + modulestore().xblock_publish(self.user_id, source_course, dest_course, [head], [chapter2]) expected = ["head12345", "chapter1", "chapter3", "problem1"] self._check_course(source_course, dest_course, expected, ["chapter2", "problem3_2"]) @@ -1681,7 +1684,7 @@ class TestPublish(SplitModuleTest): Check that the course has the expected blocks and does not have the unexpected blocks """ history_info = modulestore().get_course_history_info(dest_course_loc) - self.assertEqual(history_info['edited_by'], self.user) + self.assertEqual(history_info['edited_by'], self.user_id) for expected in expected_blocks: # since block_type has no impact on identity, we can just provide an empty string source = modulestore().get_item(source_course_loc.make_usage_key("", expected)) @@ -1690,8 +1693,8 @@ class TestPublish(SplitModuleTest): self.assertEqual(source.category, pub_copy.category) self.assertEqual(source.update_version, pub_copy.update_version) self.assertEqual( - self.user, pub_copy.edited_by, - "{} edited_by {} not {}".format(pub_copy.location, pub_copy.edited_by, self.user) + self.user_id, pub_copy.edited_by, + "{} edited_by {} not {}".format(pub_copy.location, pub_copy.edited_by, self.user_id) ) for field in source.fields.values(): if field.name == 'children': diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index 1b6c72a3fa41191b0e05b7c06c89037928da3c74..cc13bd838dd5f2d46b1cb810355f9ec7be415b2a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index d1d46364833dd65cc82dce828b83134cfd9a10c7..857f61d2380325ab6f440ea6bfca523ab7b588a0 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -169,7 +169,7 @@ def import_from_xml( # Creates a new course if it doesn't already exist if create_new_course_if_not_present and not store.has_course(dest_course_id, ignore_case=True): try: - store.create_course(dest_course_id.org, dest_course_id.offering) + store.create_course(dest_course_id.org, dest_course_id.offering, user_id) except InvalidLocationError: # course w/ same org and course exists log.debug( diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 40da66784002d9d3df6a09e9eafae5da45e7857c..9bd5810efe6dac1a447465b31dedaf115b8bec64 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/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() diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 88a2cb1b5c02de0e5a915fea7a637867d0592b7f..26e23207c93a4e86befc04a0cd59fc6471c1c700 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/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): diff --git a/lms/djangoapps/branding/tests.py b/lms/djangoapps/branding/tests.py index 0a623d086844fec8273244be975ecb8bcf536338..aceea7a318a33bd7f50983cae14ada2eac74e113 100644 --- a/lms/djangoapps/branding/tests.py +++ b/lms/djangoapps/branding/tests.py @@ -28,12 +28,12 @@ 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.store.update_item(self.course, self.user.id) @override_settings(FEATURES=FEATURES_WITH_STARTDATE) def test_none_user_index_access_with_startdate_fails(self): diff --git a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py index 933b85d8165a4eb0bc875e230633b7ec4b647cc6..2ecf952841f28ded41a5baa879248a3a173a8540 100644 --- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py +++ b/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()] diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index ef88da053de0f04fd7d01b1c16ee0993810e1f25..e0a0d0ee6ace809e0fdae49f260c860c300bb2a5 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/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) diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index fb4fc60d64e8280c5174612961930c8d646fa2e2..8c76b6844fc64b4e487716cfabde0427feb48fc4 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/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): """ diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 380572d226feedd2eee8f01afde6980221ab79c3..0b5ee2e19cf7c79a2b66e010a06e73d4c8b400fe 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/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): diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index 77670568bbbc284419ce4940939dd42c51575699..e54bc83d8e396c75c4d78be243874401de40480c 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -411,7 +411,7 @@ class TestTranscriptTranslationGetDispatch(TestVideo): self.course.static_asset_path = 'dummy/static' self.course.save() store = modulestore() - store.update_item(self.course, 'OEoXaMPEzfM') + store.update_item(self.course, self.user.id) # Test youtube style en request = Request.blank('/translation/en?videoId=12345') diff --git a/lms/djangoapps/courseware/tests/test_view_authentication.py b/lms/djangoapps/courseware/tests/test_view_authentication.py index 674d3b32f889188180b76e01e2ee4a9249bd47dc..92ebaf6867e8e6e79e899f162b6cb00b8aac845a 100644 --- a/lms/djangoapps/courseware/tests/test_view_authentication.py +++ b/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) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 3a4f73eaeeebedeacca2d918763e1dec7b39feac..e3e13ef9a90d3d2be505386a5cc8241053505334 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/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): diff --git a/lms/djangoapps/dashboard/tests/test_sysadmin.py b/lms/djangoapps/dashboard/tests/test_sysadmin.py index df89c210fe3034fcb6296d4ca2922b88eb8124fc..6f75c7a961cfeefb2963f504fa36d60fa58519d4 100644 --- a/lms/djangoapps/dashboard/tests/test_sysadmin.py +++ b/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) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index b77ee1f11cbc7bc999d8fa4e1f3cfb04a55f857d..f4cb6d2878b5e6305bf2cf050fb768be661664cf 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/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', diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py index a9881ccec5d27d84f10e19a009c8ae273b7fd7dc..31fb6b8107bf16d12a27acd2577af7932235d9fb 100644 --- a/lms/djangoapps/instructor_task/tests/test_base.py +++ b/lms/djangoapps/instructor_task/tests/test_base.py @@ -208,7 +208,7 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase): 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**') + self.module_store.update_item(item, self.user.id) def get_student_module(self, username, descriptor): """Get StudentModule object for test course, given the `username` and the problem's `descriptor`.""" diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index adfec85ee9bf7b6265ccd356d5f7dc763da77851..5bc6e1ff91004c34099e068154cd5ba3bc944d5d 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -105,6 +105,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 +297,7 @@ class TestRescoringTask(TestIntegrationTask): InstructorTaskModuleTestCase.problem_location(problem_url_name) ) descriptor.data = problem_xml - self.module_store.update_item(descriptor, '**replace_user**') + self.module_store.update_item(descriptor, 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