From c5d1807c8137e7c7243910ca2b73a032976f3042 Mon Sep 17 00:00:00 2001 From: Sagirov Evgeniy <34642612+UvgenGen@users.noreply.github.com> Date: Wed, 6 Sep 2023 17:01:31 +0300 Subject: [PATCH] feat!: remove most Old Mongo functionality (#31134) This commit leaves behind just enough Old Mongo (DraftModulestore) functionality to allow read-only access to static assets and the root CourseBlock. It removes: * create/update operations * child/parent traversal * inheritance related code It also removes or converts tests for this functionality. The ability to read from the root CourseBlock was maintained for backwards compatibility, since top-level course settings are often stored here, and this is used by various parts of the codebase, like displaying dashboards and re-building CourseOverview models. Any attempt to read the contents of a course by getting the CourseBlock's children will return an empty list (i.e. it will look empty). This commit does _not_ delete content on MongoDB or run any sort of data migration or cleanup. --- .../commands/tests/test_cleanup_assets.py | 4 + .../commands/tests/test_create_course.py | 50 +- .../commands/tests/test_export_all_courses.py | 5 + .../commands/tests/test_fix_not_found.py | 8 - .../commands/tests/test_force_publish.py | 9 - .../management/commands/tests/test_import.py | 21 - .../tests/test_import_pure_xblock.py | 25 +- .../contentstore/tests/test_libraries.py | 25 - cms/djangoapps/contentstore/tests/utils.py | 6 +- .../contentstore/views/tests/test_block.py | 295 +++---- .../views/tests/test_container_page.py | 31 +- .../contentstore/views/tests/utils.py | 4 +- cms/lib/xblock/test/test_authoring_mixin.py | 22 - .../student/tests/test_certificates.py | 7 +- .../student/tests/test_course_listing.py | 23 +- .../course_api/tests/test_serializers.py | 33 +- .../tests/test_load_override_data.py | 10 +- .../course_wiki/tests/test_access.py | 8 +- .../course_wiki/tests/test_middleware.py | 12 +- lms/djangoapps/courseware/tests/helpers.py | 9 +- .../courseware/tests/test_lti_integration.py | 3 +- .../courseware/tests/test_video_handlers.py | 2 + .../courseware/tests/test_video_mongo.py | 94 ++- lms/djangoapps/courseware/tests/tests.py | 6 +- .../instructor/tests/test_enrollment.py | 15 +- .../lti_provider/tests/test_outcomes.py | 61 +- .../tests/data/mongo_course.json | 67 ++ .../tests/test_course_overviews.py | 268 +++--- .../course_overviews/tests/test_tasks.py | 7 +- xmodule/modulestore/__init__.py | 2 +- xmodule/modulestore/mongo/base.py | 436 +--------- xmodule/modulestore/mongo/draft.py | 558 +------------ xmodule/modulestore/tests/test_assetstore.py | 70 +- .../test_cross_modulestore_import_export.py | 5 +- .../tests/test_mixed_modulestore.py | 393 ++------- xmodule/modulestore/tests/test_mongo.py | 789 +----------------- .../tests/test_mongo_call_count.py | 17 - xmodule/modulestore/tests/test_publish.py | 245 +----- .../modulestore/tests/test_xml_importer.py | 118 +-- xmodule/tests/test_course_metadata_utils.py | 46 +- 40 files changed, 663 insertions(+), 3146 deletions(-) create mode 100644 openedx/core/djangoapps/content/course_overviews/tests/data/mongo_course.json diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py b/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py index dfed3bb69c..e31d566543 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py @@ -4,6 +4,7 @@ or with filename which starts with "._") """ +from unittest import skip from django.conf import settings from django.core.management import call_command from opaque_keys.edx.keys import CourseKey @@ -20,6 +21,9 @@ from xmodule.modulestore.xml_importer import import_course_from_xml TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT +@skip("OldMongo Deprecation") +# This test worked only for Old Mongo +# Can later be converted to work with Split class ExportAllCourses(ModuleStoreTestCase): """ Tests assets cleanup for all courses. diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py b/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py index 40d1493f94..f821c8a6d5 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py @@ -4,7 +4,6 @@ Unittests for creating a course in an chosen modulestore from io import StringIO -import ddt from django.core.management import CommandError, call_command from django.test import TestCase @@ -40,27 +39,28 @@ class TestArgParsing(TestCase): call_command('create_course', "mongo", "fake@example.com", "org", "course", "run") -@ddt.ddt class TestCreateCourse(ModuleStoreTestCase): """ Unit tests for creating a course in either old mongo or split mongo via command line """ - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_all_stores_user_email(self, store): + def test_all_stores_user_email(self): call_command( "create_course", - store, + ModuleStoreEnum.Type.split, str(self.user.email), "org", "course", "run", "dummy-course-name" ) new_key = modulestore().make_course_key("org", "course", "run") self.assertTrue( modulestore().has_course(new_key), - f"Could not find course in {store}" + f"Could not find course in {ModuleStoreEnum.Type.split}" ) # pylint: disable=protected-access - self.assertEqual(store, modulestore()._get_modulestore_for_courselike(new_key).get_modulestore_type()) + self.assertEqual( + ModuleStoreEnum.Type.split, + modulestore()._get_modulestore_for_courselike(new_key).get_modulestore_type() + ) def test_duplicate_course(self): """ @@ -85,8 +85,7 @@ class TestCreateCourse(ModuleStoreTestCase): expected = "Course already exists" self.assertIn(out.getvalue().strip(), expected) - @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) - def test_get_course_with_different_case(self, default_store): + def test_get_course_with_different_case(self): """ Tests that course can not be accessed with different case. @@ -98,21 +97,20 @@ class TestCreateCourse(ModuleStoreTestCase): org = 'org1' number = 'course1' run = 'run1' - with self.store.default_store(default_store): - lowercase_course_id = self.store.make_course_key(org, number, run) - with self.store.bulk_operations(lowercase_course_id, ignore_case=True): - # Create course with lowercase key & Verify that store returns course. - self.store.create_course( - lowercase_course_id.org, - lowercase_course_id.course, - lowercase_course_id.run, - self.user.id - ) - course = self.store.get_course(lowercase_course_id) - self.assertIsNotNone(course, 'Course not found using lowercase course key.') - self.assertEqual(str(course.id), str(lowercase_course_id)) + lowercase_course_id = self.store.make_course_key(org, number, run) + with self.store.bulk_operations(lowercase_course_id, ignore_case=True): + # Create course with lowercase key & Verify that store returns course. + self.store.create_course( + lowercase_course_id.org, + lowercase_course_id.course, + lowercase_course_id.run, + self.user.id + ) + course = self.store.get_course(lowercase_course_id) + self.assertIsNotNone(course, 'Course not found using lowercase course key.') + self.assertEqual(str(course.id), str(lowercase_course_id)) - # Verify store does not return course with different case. - uppercase_course_id = self.store.make_course_key(org.upper(), number.upper(), run.upper()) - course = self.store.get_course(uppercase_course_id) - self.assertIsNone(course, 'Course should not be accessed with uppercase course id.') + # Verify store does not return course with different case. + uppercase_course_id = self.store.make_course_key(org.upper(), number.upper(), run.upper()) + course = self.store.get_course(uppercase_course_id) + self.assertIsNone(course, 'Course should not be accessed with uppercase course id.') diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_export_all_courses.py b/cms/djangoapps/contentstore/management/commands/tests/test_export_all_courses.py index 262becacbf..5c01abde69 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_export_all_courses.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_export_all_courses.py @@ -5,6 +5,7 @@ Test for export all courses. import shutil from tempfile import mkdtemp +from unittest import skip from cms.djangoapps.contentstore.management.commands.export_all_courses import export_courses_to_output_path from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order @@ -13,6 +14,10 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-a from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order +@skip("OldMongo Deprecation") +# This test fails for split modulestre +# AttributeError: 'MixedModuleStore' object has no attribute 'collection' +# split module store has no 'collection' attribute. class ExportAllCourses(ModuleStoreTestCase): """ Tests exporting all courses. diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_fix_not_found.py b/cms/djangoapps/contentstore/management/commands/tests/test_fix_not_found.py index 8750a4f59e..b9372d1520 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_fix_not_found.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_fix_not_found.py @@ -23,14 +23,6 @@ class TestFixNotFound(ModuleStoreTestCase): with self.assertRaisesRegex(CommandError, msg): call_command('fix_not_found') - def test_fix_not_found_non_split(self): - """ - The management command doesn't work on non split courses - """ - course = CourseFactory.create(default_store=ModuleStoreEnum.Type.mongo) - with self.assertRaisesRegex(CommandError, "The owning modulestore does not support this command."): - call_command("fix_not_found", str(course.id)) - def test_fix_not_found(self): course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) BlockFactory.create(category='chapter', parent_location=course.location) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_force_publish.py b/cms/djangoapps/contentstore/management/commands/tests/test_force_publish.py index 43305ff11f..ea16707d7e 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_force_publish.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_force_publish.py @@ -58,15 +58,6 @@ class TestForcePublish(SharedModuleStoreTestCase): with self.assertRaisesRegex(CommandError, errstring): call_command('force_publish', 'course-v1:org+course+run') - def test_force_publish_non_split(self): - """ - Test 'force_publish' command doesn't work on non split courses - """ - course = CourseFactory.create(default_store=ModuleStoreEnum.Type.mongo) - errstring = 'The owning modulestore does not support this command.' - with self.assertRaisesRegex(CommandError, errstring): - call_command('force_publish', str(course.id)) - class TestForcePublishModifications(ModuleStoreTestCase): """ diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_import.py b/cms/djangoapps/contentstore/management/commands/tests/test_import.py index 3e00def458..5b90973b04 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_import.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_import.py @@ -11,7 +11,6 @@ from django.core.management import call_command from path import Path as path from openedx.core.djangoapps.django_comment_common.utils import are_permissions_roles_seeded -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order @@ -73,23 +72,3 @@ class TestImport(ModuleStoreTestCase): # Now load up the course with a similar course_id and verify it loads call_command('import', self.content_dir, self.course_dir) self.assertIsNotNone(store.get_course(self.truncated_key)) - - def test_existing_course_with_different_modulestore(self): - """ - Checks that a course that originally existed in old mongo can be re-imported when - split is the default modulestore. - """ - with modulestore().default_store(ModuleStoreEnum.Type.mongo): - call_command('import', self.content_dir, self.good_dir) - - # Clear out the modulestore mappings, else when the next import command goes to create a destination - # course_key, it will find the existing course and return the mongo course_key. To reproduce TNL-1362, - # the destination course_key needs to be the one for split modulestore. - modulestore().mappings = {} - - with modulestore().default_store(ModuleStoreEnum.Type.split): - call_command('import', self.content_dir, self.good_dir) - course = modulestore().get_course(self.base_course_key) - # With the bug, this fails because the chapter's course_key is the split mongo form, - # while the course's course_key is the old mongo form. - self.assertEqual(str(course.location.course_key), str(course.children[0].course_key)) diff --git a/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py b/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py index a4a4368bdf..0a0e8663ba 100644 --- a/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py +++ b/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py @@ -7,9 +7,6 @@ from django.conf import settings from xblock.core import XBlock from xblock.fields import String -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.mongo.draft import as_draft from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.xml_importer import import_course_from_xml @@ -41,14 +38,6 @@ class XBlockImportTest(ModuleStoreTestCase): 'set by xml' ) - @XBlock.register_temp_plugin(StubXBlock) - def test_import_draft(self): - self._assert_import( - 'pure_xblock_draft', - 'set by xml', - has_draft=True - ) - def _assert_import(self, course_dir, expected_field_val, has_draft=False): """ Import a course from XML, then verify that the XBlock was loaded @@ -66,22 +55,12 @@ class XBlockImportTest(ModuleStoreTestCase): """ # It is necessary to use the "old mongo" modulestore because split doesn't work # with the "has_draft" logic below. - store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) # pylint: disable=protected-access courses = import_course_from_xml( - store, self.user.id, TEST_DATA_DIR, [course_dir], create_if_not_present=True + self.store, self.user.id, TEST_DATA_DIR, [course_dir], create_if_not_present=True ) xblock_location = courses[0].id.make_usage_key('stubxblock', 'xblock_test') - if has_draft: - xblock_location = as_draft(xblock_location) - - xblock = store.get_item(xblock_location) + xblock = self.store.get_item(xblock_location) self.assertTrue(isinstance(xblock, StubXBlock)) self.assertEqual(xblock.test_field, expected_field_val) - - if has_draft: - draft_xblock = store.get_item(xblock_location) - self.assertTrue(getattr(draft_xblock, 'is_draft', False)) - self.assertTrue(isinstance(draft_xblock, StubXBlock)) - self.assertEqual(draft_xblock.test_field, expected_field_val) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index eb9b109cdd..bb692c0160 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -1014,28 +1014,3 @@ class TestOverrides(LibraryTestCase): self.assertEqual(self.lc_block.source_library_version, duplicate.source_library_version) problem2_in_course = store.get_item(duplicate.children[0]) self.assertEqual(problem2_in_course.display_name, self.original_display_name) - - -class TestIncompatibleModuleStore(LibraryTestCase): - """ - Tests for proper validation errors with an incompatible course modulestore. - """ - - def setUp(self): - super().setUp() - # Create a course in an incompatible modulestore. - with modulestore().default_store(ModuleStoreEnum.Type.mongo): - self.course = CourseFactory.create() - - # Add a LibraryContent block to the course: - self.lc_block = self._add_library_content_block(self.course, self.lib_key) - - def test_incompatible_modulestore(self): - """ - Verifies that, if a user is using a modulestore that doesn't support libraries, - a validation error will be produced. - """ - validation = self.lc_block.validate() - self.assertEqual(validation.summary.type, validation.summary.ERROR) - self.assertIn( - "This course does not support content libraries.", validation.summary.text) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 0536db1405..bb1ca02052 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -12,7 +12,7 @@ from opaque_keys.edx.keys import AssetKey from xmodule.contentstore.django import contentstore from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.utils import ProceduralCourseTestMixin from xmodule.tests.test_transcripts_utils import YoutubeVideoHTMLResponse @@ -73,7 +73,7 @@ class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase): Base class for Studio tests that require a logged in user and a course. Also provides helper methods for manipulating and verifying the course. """ - MODULESTORE = TEST_DATA_MONGO_MODULESTORE + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): """ @@ -123,7 +123,7 @@ class CourseTestCase(ProceduralCourseTestMixin, ModuleStoreTestCase): SEQUENTIAL = 'vertical_sequential' DRAFT_HTML = 'draft_html' DRAFT_VIDEO = 'draft_video' - LOCKED_ASSET_KEY = AssetKey.from_string('/c4x/edX/toy/asset/sample_static.html') + LOCKED_ASSET_KEY = AssetKey.from_string('asset-v1:edX+toy+2012_Fall+type@asset+block@sample_static.html') def assertCoursesEqual(self, course1_id, course2_id): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index a25d0670b4..ce5771ef83 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -111,13 +111,11 @@ class ItemTest(CourseTestCase): self.course_key = self.course.id self.usage_key = self.course.location - def get_item_from_modulestore(self, usage_key, verify_is_draft=False): + def get_item_from_modulestore(self, usage_key): """ Get the item referenced by the UsageKey from the modulestore """ item = self.store.get_item(usage_key) - if verify_is_draft: - self.assertTrue(getattr(item, "is_draft", False)) return item def response_usage_key(self, response): @@ -540,9 +538,8 @@ class GetItemTest(ItemTest): class DeleteItem(ItemTest): """Tests for '/xblock' DELETE url.""" - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_delete_static_page(self, store): - course = CourseFactory.create(default_store=store) + def test_delete_static_page(self): + course = CourseFactory.create() # Add static tab resp = self.create_xblock( category="static_tab", parent_usage_key=course.location @@ -589,7 +586,7 @@ class TestCreateItem(ItemTest): parent_usage_key=vert_usage_key, category="problem", boilerplate=template_id ) prob_usage_key = self.response_usage_key(resp) - problem = self.get_item_from_modulestore(prob_usage_key, verify_is_draft=True) + problem = self.get_item_from_modulestore(prob_usage_key) # check against the template template = ProblemBlock.get_template(template_id) self.assertEqual(problem.data, template["data"]) @@ -807,9 +804,7 @@ class TestDuplicateItem(ItemTest, DuplicateHelper, OpenEdxEventsTestMixin): self.html_usage_key = self.response_usage_key(resp) # Create a second sequential just (testing children of children) - self.create_xblock( - parent_usage_key=self.chapter_usage_key, category="sequential2" - ) + self.create_xblock(parent_usage_key=self.chapter_usage_key, category='sequential') def test_duplicate_equality(self): """ @@ -976,10 +971,10 @@ class TestMoveItem(ItemTest): if not default_store: default_store = self.store.default_modulestore.get_modulestore_type() - self.course = CourseFactory.create(default_store=default_store) + course = CourseFactory.create(default_store=default_store) # Create group configurations - self.course.user_partitions = [ + course.user_partitions = [ UserPartition( 0, "first_partition", @@ -987,18 +982,18 @@ class TestMoveItem(ItemTest): [Group("0", "alpha"), Group("1", "beta")], ) ] - self.store.update_item(self.course, self.user.id) + self.store.update_item(course, self.user.id) # Create a parent chapter chap1 = self.create_xblock( - parent_usage_key=self.course.location, + parent_usage_key=course.location, display_name="chapter1", category="chapter", ) self.chapter_usage_key = self.response_usage_key(chap1) chap2 = self.create_xblock( - parent_usage_key=self.course.location, + parent_usage_key=course.location, display_name="chapter2", category="chapter", ) @@ -1053,6 +1048,8 @@ class TestMoveItem(ItemTest): ) self.split_test_usage_key = self.response_usage_key(resp) + self.course = self.store.get_item(course.location) + def setup_and_verify_content_experiment(self, partition_id): """ Helper method to set up group configurations to content experiment. @@ -1060,9 +1057,7 @@ class TestMoveItem(ItemTest): Arguments: partition_id (int): User partition id. """ - split_test = self.get_item_from_modulestore( - self.split_test_usage_key, verify_is_draft=True - ) + split_test = self.get_item_from_modulestore(self.split_test_usage_key) # Initially, no user_partition_id is set, and the split_test has no children. self.assertEqual(split_test.user_partition_id, -1) @@ -1073,9 +1068,7 @@ class TestMoveItem(ItemTest): reverse_usage_url("xblock_handler", self.split_test_usage_key), data={"metadata": {"user_partition_id": str(partition_id)}}, ) - split_test = self.get_item_from_modulestore( - self.split_test_usage_key, verify_is_draft=True - ) + split_test = self.get_item_from_modulestore(self.split_test_usage_key) self.assertEqual(split_test.user_partition_id, partition_id) self.assertEqual( len(split_test.children), @@ -1141,15 +1134,11 @@ class TestMoveItem(ItemTest): self.assertIn(source_usage_key, target_parent.children) self.assertNotIn(source_usage_key, source_parent.children) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_move_component(self, store_type): + def test_move_component(self): """ Test move component with different xblock types. - - Arguments: - store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. """ - self.setup_course(default_store=store_type) + self.setup_course() for source_usage_key, target_usage_key in [ (self.html_usage_key, self.vert2_usage_key), (self.vert_usage_key, self.seq2_usage_key), @@ -1391,9 +1380,7 @@ class TestMoveItem(ItemTest): reverse_usage_url("xblock_handler", child_split_test_usage_key), data={"metadata": {"user_partition_id": str(0)}}, ) - child_split_test = self.get_item_from_modulestore( - self.split_test_usage_key, verify_is_draft=True - ) + child_split_test = self.get_item_from_modulestore(self.split_test_usage_key) # Try to move content experiment further down the level to a child group A nested inside main group A. response = self._move_component( @@ -1469,6 +1456,7 @@ class TestMoveItem(ItemTest): """ group1 = self.course.user_partitions[0].groups[0] group2 = self.course.user_partitions[0].groups[1] + vert1 = self.store.get_item(self.vert_usage_key) vert2 = self.store.get_item(self.vert2_usage_key) html = self.store.get_item(self.html_usage_key) @@ -1481,10 +1469,12 @@ class TestMoveItem(ItemTest): html.runtime._services["partitions"] = partitions_service # lint-amnesty, pylint: disable=protected-access # Set access settings so html will contradict vert2 when moved into that unit + vert1.group_access = {self.course.user_partitions[0].id: [group2.id]} vert2.group_access = {self.course.user_partitions[0].id: [group1.id]} html.group_access = {self.course.user_partitions[0].id: [group2.id]} - self.store.update_item(html, self.user.id) - self.store.update_item(vert2, self.user.id) + vert1 = self.store.update_item(vert1, self.user.id) + vert2 = self.store.update_item(vert2, self.user.id) + html = self.store.update_item(html, self.user.id) # Verify that there is no warning when html is in a non contradicting unit validation = html.validate() @@ -1493,7 +1483,7 @@ class TestMoveItem(ItemTest): # Now move it and confirm that the html component has been moved into vertical 2 self.assert_move_item(self.html_usage_key, self.vert2_usage_key) html.parent = self.vert2_usage_key - self.store.update_item(html, self.user.id) + html = self.store.update_item(html, self.user.id) validation = html.validate() self.assertEqual(len(validation.messages), 1) self._verify_validation_message( @@ -1505,7 +1495,7 @@ class TestMoveItem(ItemTest): # Move the html component back and confirm that the warning is gone again self.assert_move_item(self.html_usage_key, self.vert_usage_key) html.parent = self.vert_usage_key - self.store.update_item(html, self.user.id) + html = self.store.update_item(html, self.user.id) validation = html.validate() self.assertEqual(len(validation.messages), 0) @@ -1527,16 +1517,12 @@ class TestMoveItem(ItemTest): insert_at, ) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_move_and_discard_changes(self, store_type): + def test_move_and_discard_changes(self): """ Verifies that discard changes operation brings moved component back to source location and removes the component from target location. - - Arguments: - store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. """ - self.setup_course(default_store=store_type) + self.setup_course() old_parent_loc = self.store.get_parent_location(self.html_usage_key) @@ -1594,15 +1580,11 @@ class TestMoveItem(ItemTest): self.assertIn(self.html_usage_key, source_parent.children) self.assertNotIn(self.html_usage_key, target_parent.children) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_move_item_not_found(self, store_type=ModuleStoreEnum.Type.mongo): + def test_move_item_not_found(self): """ Test that an item not found exception raised when an item is not found when getting the item. - - Arguments: - store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. """ - self.setup_course(default_store=store_type) + self.setup_course() data = { "move_source_locator": str( @@ -1752,30 +1734,25 @@ class TestEditItem(TestEditItemSetup): self.client.ajax_post( self.problem_update_url, data={"metadata": {"rerandomize": "onreset"}} ) - problem = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) - self.assertEqual(problem.rerandomize, "onreset") + problem = self.get_item_from_modulestore(self.problem_usage_key) + self.assertEqual(problem.rerandomize, 'onreset') self.client.ajax_post( self.problem_update_url, data={"metadata": {"rerandomize": None}} ) - problem = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) - self.assertEqual(problem.rerandomize, "never") + problem = self.get_item_from_modulestore(self.problem_usage_key) + self.assertEqual(problem.rerandomize, 'never') def test_null_field(self): """ Sending null in for a field 'deletes' it """ - problem = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) + problem = self.get_item_from_modulestore(self.problem_usage_key) self.assertIsNotNone(problem.markdown) - self.client.ajax_post(self.problem_update_url, data={"nullout": ["markdown"]}) - problem = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True + self.client.ajax_post( + self.problem_update_url, + data={'nullout': ['markdown']} ) + problem = self.get_item_from_modulestore(self.problem_usage_key) self.assertIsNone(problem.markdown) def test_date_fields(self): @@ -1831,9 +1808,7 @@ class TestEditItem(TestEditItemSetup): } }, ) - problem = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) + problem = self.get_item_from_modulestore(self.problem_usage_key) self.assertEqual(problem.display_name, new_display_name) self.assertEqual(problem.max_attempts, new_max_attempts) @@ -2052,9 +2027,7 @@ class TestEditItem(TestEditItemSetup): }, ) self.assertFalse(self._is_location_published(self.problem_usage_key)) - draft = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) + draft = self.get_item_from_modulestore(self.problem_usage_key) self.assertEqual(draft.display_name, new_display_name) # Publish the item @@ -2112,9 +2085,7 @@ class TestEditItem(TestEditItemSetup): self.client.ajax_post( self.problem_update_url, data={"metadata": {"due": "2077-10-10T04:00Z"}} ) - updated_draft = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) + updated_draft = self.get_item_from_modulestore(self.problem_usage_key) self.assertEqual(updated_draft.due, datetime(2077, 10, 10, 4, 0, tzinfo=UTC)) self.assertIsNone(published.due) # Fetch the published version again to make sure the due date is still unset. @@ -2154,9 +2125,7 @@ class TestEditItem(TestEditItemSetup): ) # Both published and draft content should be different - draft = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) + draft = self.get_item_from_modulestore(self.problem_usage_key) self.assertNotEqual(draft.data, published.data) # Get problem by 'xblock_handler' @@ -2174,9 +2143,7 @@ class TestEditItem(TestEditItemSetup): self.assertEqual(resp.status_code, 200) # Both published and draft content should still be different - draft = self.get_item_from_modulestore( - self.problem_usage_key, verify_is_draft=True - ) + draft = self.get_item_from_modulestore(self.problem_usage_key) self.assertNotEqual(draft.data, published.data) # Fetch the published version again to make sure the data is correct. published = modulestore().get_item( @@ -2209,18 +2176,6 @@ class TestEditItem(TestEditItemSetup): self._verify_published_with_no_draft(unit_usage_key) self._verify_published_with_no_draft(html_usage_key) - # Make a draft for the unit and verify that the problem also has a draft - resp = self.client.ajax_post( - unit_update_url, - data={ - "id": str(unit_usage_key), - "metadata": {}, - }, - ) - self.assertEqual(resp.status_code, 200) - self._verify_published_with_draft(unit_usage_key) - self._verify_published_with_draft(html_usage_key) - def test_field_value_errors(self): """ Test that if the user's input causes a ValueError on an XBlock field, @@ -2346,9 +2301,7 @@ class TestEditSplitModule(ItemTest): ) # Verify the partition_id was saved. - split_test = self.get_item_from_modulestore( - self.split_test_usage_key, verify_is_draft=True - ) + split_test = self.get_item_from_modulestore(self.split_test_usage_key) self.assertEqual(partition_id, split_test.user_partition_id) return split_test @@ -2356,7 +2309,7 @@ class TestEditSplitModule(ItemTest): """ Verifies the number of children of the split_test instance. """ - split_test = self.get_item_from_modulestore(self.split_test_usage_key, True) + split_test = self.get_item_from_modulestore(self.split_test_usage_key) self.assertEqual(expected_number, len(split_test.children)) return split_test @@ -2365,9 +2318,7 @@ class TestEditSplitModule(ItemTest): Test that verticals are created for the configuration groups when a spit test block is edited. """ - split_test = self.get_item_from_modulestore( - self.split_test_usage_key, verify_is_draft=True - ) + split_test = self.get_item_from_modulestore(self.split_test_usage_key) # Initially, no user_partition_id is set, and the split_test has no children. self.assertEqual(-1, split_test.user_partition_id) self.assertEqual(0, len(split_test.children)) @@ -2377,12 +2328,8 @@ class TestEditSplitModule(ItemTest): # Verify that child verticals have been set to match the groups self.assertEqual(2, len(split_test.children)) - vertical_0 = self.get_item_from_modulestore( - split_test.children[0], verify_is_draft=True - ) - vertical_1 = self.get_item_from_modulestore( - split_test.children[1], verify_is_draft=True - ) + vertical_0 = self.get_item_from_modulestore(split_test.children[0]) + vertical_1 = self.get_item_from_modulestore(split_test.children[1]) self.assertEqual("vertical", vertical_0.category) self.assertEqual("vertical", vertical_1.category) self.assertEqual( @@ -2407,9 +2354,7 @@ class TestEditSplitModule(ItemTest): """ Test that concise outline for split test component gives display name as group name. """ - split_test = self.get_item_from_modulestore( - self.split_test_usage_key, verify_is_draft=True - ) + split_test = self.get_item_from_modulestore(self.split_test_usage_key) # Initially, no user_partition_id is set, and the split_test has no children. self.assertEqual(split_test.user_partition_id, -1) self.assertEqual(len(split_test.children), 0) @@ -2451,15 +2396,9 @@ class TestEditSplitModule(ItemTest): self.assertEqual(5, len(split_test.children)) self.assertEqual(initial_vertical_0_location, split_test.children[0]) self.assertEqual(initial_vertical_1_location, split_test.children[1]) - vertical_0 = self.get_item_from_modulestore( - split_test.children[2], verify_is_draft=True - ) - vertical_1 = self.get_item_from_modulestore( - split_test.children[3], verify_is_draft=True - ) - vertical_2 = self.get_item_from_modulestore( - split_test.children[4], verify_is_draft=True - ) + vertical_0 = self.get_item_from_modulestore(split_test.children[2]) + vertical_1 = self.get_item_from_modulestore(split_test.children[3]) + vertical_2 = self.get_item_from_modulestore(split_test.children[4]) # Verify that the group_id_to child mapping is correct. self.assertEqual(3, len(split_test.group_id_to_child)) @@ -3106,39 +3045,25 @@ class TestXBlockInfo(ItemTest): json_response = json.loads(resp.content.decode("utf-8")) self.validate_course_xblock_info(json_response, course_outline=True) - @ddt.data( - (ModuleStoreEnum.Type.split, 3, 3), - (ModuleStoreEnum.Type.mongo, 8, 12), - ) - @ddt.unpack - def test_xblock_outline_handler_mongo_calls( - self, store_type, chapter_queries, chapter_queries_1 - ): - with self.store.default_store(store_type): - course = CourseFactory.create() - chapter = BlockFactory.create( - parent_location=course.location, - category="chapter", - display_name="Week 1", - ) - outline_url = reverse_usage_url("xblock_outline_handler", chapter.location) - with check_mongo_calls(chapter_queries): - self.client.get(outline_url, HTTP_ACCEPT="application/json") + def test_xblock_outline_handler_mongo_calls(self): + course = CourseFactory.create() + chapter = BlockFactory.create( + parent_location=course.location, category='chapter', display_name='Week 1' + ) + outline_url = reverse_usage_url('xblock_outline_handler', chapter.location) + with check_mongo_calls(3): + self.client.get(outline_url, HTTP_ACCEPT='application/json') - sequential = BlockFactory.create( - parent_location=chapter.location, - category="sequential", - display_name="Sequential 1", - ) + sequential = BlockFactory.create( + parent_location=chapter.location, category='sequential', display_name='Sequential 1' + ) - BlockFactory.create( - parent_location=sequential.location, - category="vertical", - display_name="Vertical 1", - ) - # calls should be same after adding two new children for split only. - with check_mongo_calls(chapter_queries_1): - self.client.get(outline_url, HTTP_ACCEPT="application/json") + BlockFactory.create( + parent_location=sequential.location, category='vertical', display_name='Vertical 1' + ) + # calls should be same after adding two new children for split only. + with check_mongo_calls(3): + self.client.get(outline_url, HTTP_ACCEPT='application/json') def test_entrance_exam_chapter_xblock_info(self): chapter = BlockFactory.create( @@ -3264,32 +3189,26 @@ class TestXBlockInfo(ItemTest): ) self.validate_component_xblock_info(xblock_info) - @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) - def test_validate_start_date(self, store_type): + def test_validate_start_date(self): """ Validate if start-date year is less than 1900 reset the date to DEFAULT_START_DATE. """ - with self.store.default_store(store_type): - course = CourseFactory.create() - chapter = BlockFactory.create( - parent_location=course.location, - category="chapter", - display_name="Week 1", - ) + course = CourseFactory.create() + chapter = BlockFactory.create( + parent_location=course.location, category='chapter', display_name='Week 1' + ) - chapter.start = datetime(year=1899, month=1, day=1, tzinfo=UTC) + chapter.start = datetime(year=1899, month=1, day=1, tzinfo=UTC) - xblock_info = create_xblock_info( - chapter, - include_child_info=True, - include_children_predicate=ALWAYS, - include_ancestor_info=True, - user=self.user, - ) + xblock_info = create_xblock_info( + chapter, + include_child_info=True, + include_children_predicate=ALWAYS, + include_ancestor_info=True, + user=self.user + ) - self.assertEqual( - xblock_info["start"], DEFAULT_START_DATE.strftime("%Y-%m-%dT%H:%M:%SZ") - ) + self.assertEqual(xblock_info['start'], DEFAULT_START_DATE.strftime('%Y-%m-%dT%H:%M:%SZ')) def test_highlights_enabled(self): self.course.highlights_enabled_for_messaging = True @@ -3489,9 +3408,11 @@ class TestSpecialExamXBlockInfo(ItemTest): user_id=user_id, highlights=["highlight"], ) + # get updated course + self.course = self.store.get_item(self.course.location) self.course.enable_proctored_exams = True self.course.save() - self.store.update_item(self.course, self.user.id) + self.course = self.store.update_item(self.course, self.user.id) def test_proctoring_is_enabled_for_course(self): course = modulestore().get_item(self.course.location) @@ -3517,7 +3438,7 @@ class TestSpecialExamXBlockInfo(ItemTest): category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_exam=True, + is_proctored_enabled=True, is_time_limited=True, default_time_limit_minutes=100, is_onboarding_exam=False, @@ -3561,7 +3482,7 @@ class TestSpecialExamXBlockInfo(ItemTest): category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_exam=False, + is_proctored_enabled=False, is_time_limited=False, is_onboarding_exam=False, ) @@ -3589,7 +3510,7 @@ class TestSpecialExamXBlockInfo(ItemTest): category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_exam=False, + is_proctored_enabled=False, is_time_limited=False, is_onboarding_exam=False, ) @@ -3849,9 +3770,8 @@ class TestXBlockPublishingInfo(ItemTest): xblock_info = self._get_xblock_info(empty_chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.unscheduled) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_chapter_self_paced_default_start_date(self, store_type): - course = CourseFactory.create(default_store=store_type) + def test_chapter_self_paced_default_start_date(self): + course = CourseFactory.create() course.self_paced = True self.store.update_item(course, self.user.id) chapter = self._create_child(course, "chapter", "Test Chapter") @@ -3939,29 +3859,15 @@ class TestXBlockPublishingInfo(ItemTest): ) def test_partially_released_section(self): - chapter = self._create_child(self.course, "chapter", "Test Chapter") - released_sequential = self._create_child( - chapter, "sequential", "Released Sequential" - ) - self._create_child( - released_sequential, "vertical", "Released Unit", publish_item=True - ) - self._create_child( - released_sequential, "vertical", "Staff Only Unit", staff_only=True - ) + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + released_sequential = self._create_child(chapter, 'sequential', "Released Sequential") + self._create_child(released_sequential, 'vertical', "Released Unit", publish_item=True) + self._create_child(released_sequential, 'vertical', "Staff Only Unit 1", staff_only=True) self._set_release_date(chapter.location, datetime.now(UTC) - timedelta(days=1)) - published_sequential = self._create_child( - chapter, "sequential", "Published Sequential" - ) - self._create_child( - published_sequential, "vertical", "Published Unit", publish_item=True - ) - self._create_child( - published_sequential, "vertical", "Staff Only Unit", staff_only=True - ) - self._set_release_date( - published_sequential.location, datetime.now(UTC) + timedelta(days=1) - ) + published_sequential = self._create_child(chapter, 'sequential', "Published Sequential") + self._create_child(published_sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(published_sequential, 'vertical', "Staff Only Unit 2", staff_only=True) + self._set_release_date(published_sequential.location, datetime.now(UTC) + timedelta(days=1)) xblock_info = self._get_xblock_info(chapter.location) # Verify the state of the released sequential @@ -4191,8 +4097,7 @@ class TestXBlockPublishingInfo(ItemTest): xblock_info, True, path=self.FIRST_UNIT_PATH ) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_self_paced_item_visibility_state(self, store_type): + def test_self_paced_item_visibility_state(self): """ Test that in self-paced course, item has `live` visibility state. Test that when item was initially in `scheduled` state in instructor mode, change course pacing to self-paced, @@ -4200,7 +4105,7 @@ class TestXBlockPublishingInfo(ItemTest): """ # Create course, chapter and setup future release date to make chapter in scheduled state - course = CourseFactory.create(default_store=store_type) + course = CourseFactory.create() chapter = self._create_child(course, "chapter", "Test Chapter") self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index 7bc2616515..d3bf988d2a 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -31,29 +31,30 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): def setUp(self): super().setUp() - self.vertical = self._create_block(self.sequential.location, 'vertical', 'Unit') - self.html = self._create_block(self.vertical.location, "html", "HTML") - self.child_container = self._create_block(self.vertical.location, 'split_test', 'Split Test') - self.child_vertical = self._create_block(self.child_container.location, 'vertical', 'Child Vertical') - self.video = self._create_block(self.child_vertical.location, "video", "My Video") + self.vertical = self._create_block(self.sequential, 'vertical', 'Unit') + self.html = self._create_block(self.vertical, "html", "HTML") + self.child_container = self._create_block(self.vertical, 'split_test', 'Split Test') + self.child_vertical = self._create_block(self.child_container, 'vertical', 'Child Vertical') + self.video = self._create_block(self.child_vertical, "video", "My Video") self.store = modulestore() past = datetime.datetime(1970, 1, 1, tzinfo=UTC) future = datetime.datetime.now(UTC) + datetime.timedelta(days=1) self.released_private_vertical = self._create_block( - parent_location=self.sequential.location, category='vertical', display_name='Released Private Unit', + parent=self.sequential, category='vertical', display_name='Released Private Unit', start=past) self.unreleased_private_vertical = self._create_block( - parent_location=self.sequential.location, category='vertical', display_name='Unreleased Private Unit', + parent=self.sequential, category='vertical', display_name='Unreleased Private Unit', start=future) self.released_public_vertical = self._create_block( - parent_location=self.sequential.location, category='vertical', display_name='Released Public Unit', + parent=self.sequential, category='vertical', display_name='Released Public Unit', start=past) self.unreleased_public_vertical = self._create_block( - parent_location=self.sequential.location, category='vertical', display_name='Unreleased Public Unit', + parent=self.sequential, category='vertical', display_name='Unreleased Public Unit', start=future) self.store.publish(self.unreleased_public_vertical.location, self.user.id) self.store.publish(self.released_public_vertical.location, self.user.id) + self.store.publish(self.vertical.location, self.user.id) def test_container_html(self): self._test_html_content( @@ -81,8 +82,8 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): 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 = self._create_block(self.child_container.location, "wrapper", "Wrapper") - self._create_block(draft_container.location, "html", "Child HTML") + draft_container = self._create_block(self.child_container, "wrapper", "Wrapper") + self._create_block(draft_container, "html", "Child HTML") def test_container_html(xblock): self._test_html_content( @@ -177,12 +178,12 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): self.validate_preview_html(self.child_container, self.container_view) self.validate_preview_html(self.child_vertical, self.reorderable_child_view) - def _create_block(self, parent_location, category, display_name, **kwargs): + def _create_block(self, parent, category, display_name, **kwargs): """ creates a block in the module store, without publishing it. """ return BlockFactory.create( - parent_location=parent_location, + parent=parent, category=category, display_name=display_name, publish_item=False, @@ -194,7 +195,7 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): """ Verify that a public container rendered as a child of the container page returns the expected HTML. """ - empty_child_container = self._create_block(self.vertical.location, 'split_test', 'Split Test') + empty_child_container = self._create_block(self.vertical, 'split_test', 'Split Test 1') 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_add=False) @@ -202,7 +203,7 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): """ Verify that a draft container rendered as a child of the container page returns the expected HTML. """ - empty_child_container = self._create_block(self.vertical.location, 'split_test', 'Split Test') + empty_child_container = self._create_block(self.vertical, 'split_test', 'Split Test 1') self.validate_preview_html(empty_child_container, self.reorderable_child_view, can_add=False) @patch( diff --git a/cms/djangoapps/contentstore/views/tests/utils.py b/cms/djangoapps/contentstore/views/tests/utils.py index c1a745f2e2..979f8322e9 100644 --- a/cms/djangoapps/contentstore/views/tests/utils.py +++ b/cms/djangoapps/contentstore/views/tests/utils.py @@ -18,9 +18,9 @@ class StudioPageTestCase(CourseTestCase): def setUp(self): super().setUp() - self.chapter = BlockFactory.create(parent_location=self.course.location, + self.chapter = BlockFactory.create(parent=self.course, category='chapter', display_name="Week 1") - self.sequential = BlockFactory.create(parent_location=self.chapter.location, + self.sequential = BlockFactory.create(parent=self.chapter, category='sequential', display_name="Lesson 1") def get_page_html(self, xblock): diff --git a/cms/lib/xblock/test/test_authoring_mixin.py b/cms/lib/xblock/test/test_authoring_mixin.py index 97b92546d6..f3721a622d 100644 --- a/cms/lib/xblock/test/test_authoring_mixin.py +++ b/cms/lib/xblock/test/test_authoring_mixin.py @@ -158,14 +158,6 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): [self.STAFF_LOCKED, self.CONTENT_GROUPS_TITLE, self.ENROLLMENT_GROUPS_TITLE] ) - def test_html_populated_partition_staff_locked(self): - self.create_content_groups(self.pet_groups) - self.set_staff_only(self.vertical_location) - self.verify_visibility_view_contains( - self.video_location, - [self.STAFF_LOCKED, self.CONTENT_GROUPS_TITLE, 'Cat Lovers', 'Dog Lovers'] - ) - def test_html_false_content_group(self): self.create_content_groups(self.pet_groups) self.set_group_access(self.video_location, ['false_group_id']) @@ -178,20 +170,6 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): [self.STAFF_LOCKED] ) - def test_html_false_content_group_staff_locked(self): - self.create_content_groups(self.pet_groups) - self.set_staff_only(self.vertical_location) - self.set_group_access(self.video_location, ['false_group_id']) - self.verify_visibility_view_contains( - self.video_location, - [ - 'Cat Lovers', - 'Dog Lovers', - self.STAFF_LOCKED, - self.GROUP_NO_LONGER_EXISTS - ] - ) - @override_settings(FEATURES=FEATURES_WITH_ENROLLMENT_TRACK_DISABLED) def test_enrollment_tracks_disabled(self): """ diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index 8447987249..3a69f64b2b 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -9,7 +9,7 @@ from django.test.utils import override_settings from django.urls import reverse from pytz import UTC from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, SharedModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.data import CertificatesDisplayBehaviors @@ -31,7 +31,7 @@ FUTURE_DATE = datetime.datetime.now(UTC) + datetime.timedelta(days=2) class CertificateDisplayTestBase(SharedModuleStoreTestCase): """Tests display of certificates on the student dashboard. """ - MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE USERNAME = "test_user" PASSWORD = "password" @@ -93,15 +93,16 @@ class CertificateDashboardMessageDisplayTest(CertificateDisplayTestBase): def _check_message(self, visible_date): # lint-amnesty, pylint: disable=missing-function-docstring response = self.client.get(reverse('dashboard')) - test_message = 'Your grade and certificate will be ready after' is_past = visible_date < datetime.datetime.now(UTC) if is_past: + test_message = 'Your grade and certificate will be ready after' self.assertNotContains(response, test_message) self.assertNotContains(response, "View Test_Certificate") else: + test_message = 'Congratulations! Your certificate is ready.' self.assertContains(response, test_message) self.assertNotContains(response, "View Test_Certificate") diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index e26763939f..2f7b6c41d1 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -103,12 +103,13 @@ class TestCourseListing(ModuleStoreTestCase, MilestonesTestCaseMixin): """ Test the course list for regular staff when get_course returns an ErrorBlock """ - # pylint: disable=protected-access - mongo_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) - course_key = mongo_store.make_course_key('Org1', 'Course1', 'Run1') - self._create_course_with_access_groups(course_key, default_store=ModuleStoreEnum.Type.mongo) + store = modulestore() + course_key = store.make_course_key('Org1', 'Course1', 'Run1') + self._create_course_with_access_groups(course_key) - with mock.patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', mock.Mock(side_effect=Exception)): + with mock.patch( + 'xmodule.modulestore.split_mongo.caching_descriptor_system.SplitMongoKVS', mock.Mock(side_effect=Exception) + ): assert isinstance(modulestore().get_course(course_key), ErrorBlock) # Invalidate (e.g., delete) the corresponding CourseOverview, forcing get_course to be called. @@ -122,14 +123,14 @@ class TestCourseListing(ModuleStoreTestCase, MilestonesTestCaseMixin): Create good courses, courses that won't load, and deleted courses which still have roles. Test course listing. """ - mongo_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) # lint-amnesty, pylint: disable=protected-access + store = modulestore() - good_location = mongo_store.make_course_key('testOrg', 'testCourse', 'RunBabyRun') - self._create_course_with_access_groups(good_location, default_store=ModuleStoreEnum.Type.mongo) + good_location = store.make_course_key('testOrg', 'testCourse', 'RunBabyRun') + self._create_course_with_access_groups(good_location) - course_location = mongo_store.make_course_key('testOrg', 'doomedCourse', 'RunBabyRun') - self._create_course_with_access_groups(course_location, default_store=ModuleStoreEnum.Type.mongo) - mongo_store.delete_course(course_location, ModuleStoreEnum.UserID.test) + course_location = store.make_course_key('testOrg', 'doomedCourse', 'RunBabyRun') + self._create_course_with_access_groups(course_location) + store.delete_course(course_location, ModuleStoreEnum.UserID.test) courses_list = list(get_course_enrollments(self.student, None, [])) assert len(courses_list) == 1, courses_list diff --git a/lms/djangoapps/course_api/tests/test_serializers.py b/lms/djangoapps/course_api/tests/test_serializers.py index 60971df577..ec11fc05de 100644 --- a/lms/djangoapps/course_api/tests/test_serializers.py +++ b/lms/djangoapps/course_api/tests/test_serializers.py @@ -2,7 +2,6 @@ Test data created by CourseSerializer and CourseDetailSerializer """ - from datetime import datetime from unittest import TestCase, mock @@ -12,7 +11,7 @@ from rest_framework.request import Request from rest_framework.test import APIRequestFactory from xblock.core import XBlock from xmodule.course_block import DEFAULT_START_DATE -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_ONLY_SPLIT_MODULESTORE_DRAFT_PREFERRED, ModuleStoreTestCase from xmodule.modulestore.tests.factories import check_mongo_calls from lms.djangoapps.certificates.api import can_show_certificate_available_date_field @@ -32,7 +31,7 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase): maxDiff = 5000 # long enough to show mismatched dicts, in case of error serializer_class = CourseSerializer - MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE + MODULESTORE = TEST_DATA_ONLY_SPLIT_MODULESTORE_DRAFT_PREFERRED ENABLED_SIGNALS = ['course_published'] def setUp(self): @@ -41,10 +40,10 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase): self.honor_user = self.create_user('honor', is_staff=False) self.request_factory = APIRequestFactory() - course_id = 'edX/toy/2012_Fall' - banner_image_uri = '/c4x/edX/toy/asset/images_course_image.jpg' + course_id = 'course-v1:edX+toy+2012_Fall' + banner_image_uri = '/asset-v1:edX+toy+2012_Fall+type@asset+block@images_course_image.jpg' banner_image_absolute_uri = 'http://testserver' + banner_image_uri - image_path = '/c4x/edX/toy/asset/just_a_test.jpg' + image_path = '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg' image_url = 'http://testserver' + image_path self.expected_data = { 'id': course_id, @@ -55,19 +54,15 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase): 'media': { 'banner_image': { 'uri': banner_image_uri, - 'uri_absolute': banner_image_absolute_uri, - }, - 'course_image': { - 'uri': image_path, - }, - 'course_video': { - 'uri': 'http://www.youtube.com/watch?v=test_youtube_id', + 'uri_absolute': banner_image_absolute_uri }, + 'course_image': {'uri': image_path}, + 'course_video': {'uri': 'http://www.youtube.com/watch?v=test_youtube_id'}, 'image': { 'raw': image_url, 'small': image_url, - 'large': image_url, - }, + 'large': image_url + } }, 'start': '2015-07-17T12:00:00Z', 'start_type': 'timestamp', @@ -75,11 +70,11 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase): 'end': '2015-09-19T18:00:00Z', 'enrollment_start': '2015-06-15T00:00:00Z', 'enrollment_end': '2015-07-15T00:00:00Z', - 'blocks_url': 'http://testserver/api/courses/v2/blocks/?course_id=edX%2Ftoy%2F2012_Fall', + 'blocks_url': 'http://testserver/api/courses/v2/blocks/?course_id=course-v1%3AedX%2Btoy%2B2012_Fall', 'effort': '6 hours', 'pacing': 'instructor', 'mobile_available': True, - 'hidden': True, # because it's an old mongo course + 'hidden': False, 'invitation_only': False, # 'course_id' is a deprecated field, please use 'id' instead. @@ -126,14 +121,14 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase): advertised_start='The Ides of March' ) result = self._get_result(course) - assert result['course_id'] == 'edX/custom/2012_Fall' + assert result['course_id'] == 'course-v1:edX+custom+2012_Fall' assert result['start_type'] == 'string' assert result['start_display'] == 'The Ides of March' def test_empty_start(self): course = self.create_course(start=DEFAULT_START_DATE, course='custom') result = self._get_result(course) - assert result['course_id'] == 'edX/custom/2012_Fall' + assert result['course_id'] == 'course-v1:edX+custom+2012_Fall' assert result['start_type'] == 'empty' assert result['start_display'] is None diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py b/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py index e522bc5b65..d0fef5065b 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_load_override_data.py @@ -8,7 +8,7 @@ import datetime import ddt import pytz from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import ToyCourseFactory from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -32,7 +32,7 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): """ Test proper behavior for OverrideDataTransformer """ - MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE @classmethod def setUpClass(cls): @@ -52,10 +52,10 @@ class TestOverrideDataTransformer(ModuleStoreTestCase): self.learner.id, subsection.location, 'html', 'new_component' ) CourseEnrollmentFactory.create(user=self.learner, course_id=self.course_key, is_active=True) - self.block = self.store.create_child( - self.learner2.id, subsection.location, 'html', 'new_component' - ) CourseEnrollmentFactory.create(user=self.learner2, course_id=self.course_key, is_active=True) + self.block = self.store.create_child( + self.learner2.id, subsection.location, 'html', 'new_component_2' + ) @ddt.data(*REQUESTED_FIELDS) def test_transform(self, field): diff --git a/lms/djangoapps/course_wiki/tests/test_access.py b/lms/djangoapps/course_wiki/tests/test_access.py index 86c6e54b8a..daf51c7c00 100644 --- a/lms/djangoapps/course_wiki/tests/test_access.py +++ b/lms/djangoapps/course_wiki/tests/test_access.py @@ -5,7 +5,7 @@ Tests for wiki permissions from django.contrib.auth.models import Group from wiki.models import URLPath -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import InstructorFactory @@ -18,14 +18,14 @@ from common.djangoapps.student.tests.factories import UserFactory class TestWikiAccessBase(ModuleStoreTestCase): """Base class for testing wiki access.""" - MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): super().setUp() self.wiki = get_or_create_root() - self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course', metadata={'use_unique_wiki_id': 'false'}) # lint-amnesty, pylint: disable=line-too-long + self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course') # lint-amnesty, pylint: disable=line-too-long self.course_math101_staff = self.create_staff_for_course(self.course_math101) wiki_math101 = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101)) @@ -33,7 +33,7 @@ class TestWikiAccessBase(ModuleStoreTestCase): wiki_math101_page_page = self.create_urlpath(wiki_math101_page, 'Grandchild') self.wiki_math101_pages = [wiki_math101, wiki_math101_page, wiki_math101_page_page] - self.course_math101b = CourseFactory.create(org='org', number='math101b', display_name='Course', metadata={'use_unique_wiki_id': 'true'}) # lint-amnesty, pylint: disable=line-too-long + self.course_math101b = CourseFactory.create(org='org', number='math101b', display_name='Course') # lint-amnesty, pylint: disable=line-too-long self.course_math101b_staff = self.create_staff_for_course(self.course_math101b) wiki_math101b = self.create_urlpath(self.wiki, course_wiki_slug(self.course_math101b)) diff --git a/lms/djangoapps/course_wiki/tests/test_middleware.py b/lms/djangoapps/course_wiki/tests/test_middleware.py index 16937cc772..d9fa62608c 100644 --- a/lms/djangoapps/course_wiki/tests/test_middleware.py +++ b/lms/djangoapps/course_wiki/tests/test_middleware.py @@ -5,7 +5,7 @@ Tests for wiki middleware. from django.test.client import Client from wiki.models import URLPath -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import InstructorFactory @@ -14,7 +14,7 @@ from lms.djangoapps.course_wiki.views import get_or_create_root class TestWikiAccessMiddleware(ModuleStoreTestCase): """Tests for WikiAccessMiddleware.""" - MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): """Test setup.""" @@ -22,7 +22,7 @@ class TestWikiAccessMiddleware(ModuleStoreTestCase): self.wiki = get_or_create_root() - self.course_math101 = CourseFactory.create(org='edx', number='math101', display_name='2014', metadata={'use_unique_wiki_id': 'false'}) # lint-amnesty, pylint: disable=line-too-long + self.course_math101 = CourseFactory.create(org='edx', number='math101', display_name='2014') self.course_math101_instructor = InstructorFactory(course_key=self.course_math101.id, username='instructor', password='secret') # lint-amnesty, pylint: disable=line-too-long self.wiki_math101 = URLPath.create_article(self.wiki, 'math101', title='math101') @@ -31,6 +31,6 @@ class TestWikiAccessMiddleware(ModuleStoreTestCase): def test_url_tranform(self): """Test that the correct prefix ('/courses/') is added to the urls in the wiki.""" - response = self.client.get('/courses/edx/math101/2014/wiki/math101/') - self.assertContains(response, '/courses/edx/math101/2014/wiki/math101/_edit/') - self.assertContains(response, '/courses/edx/math101/2014/wiki/math101/_settings/') + response = self.client.get('/courses/course-v1:edx+math101+2014/wiki/math101/') + self.assertContains(response, '/courses/course-v1:edx+math101+2014/wiki/math101/_edit/') + self.assertContains(response, '/courses/course-v1:edx+math101+2014/wiki/math101/_settings/') diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 577f8b2ca3..3a2ddbc456 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -31,7 +31,7 @@ from common.djangoapps.student.models import CourseEnrollment, Registration from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from common.djangoapps.util.date_utils import strftime_localized_html from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # lint-amnesty, pylint: disable=wrong-import-order from xmodule.tests import get_test_descriptor_system, get_test_system, prepare_block_runtime # lint-amnesty, pylint: disable=wrong-import-order @@ -47,15 +47,14 @@ class BaseTestXmodule(ModuleStoreTestCase): 1. CATEGORY 2. DATA or METADATA 3. MODEL_DATA - 4. COURSE_DATA and USER_COUNT if needed + 4. USER_COUNT if needed This class should not contain any tests, because CATEGORY should be defined in child class. """ - MODULESTORE = TEST_DATA_MONGO_MODULESTORE + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE USER_COUNT = 2 - COURSE_DATA = {} # Data from YAML xmodule/templates/NAME/default.yaml CATEGORY = "vertical" @@ -101,7 +100,7 @@ class BaseTestXmodule(ModuleStoreTestCase): self.item_url = str(self.block.location) def setup_course(self): # lint-amnesty, pylint: disable=missing-function-docstring - self.course = CourseFactory.create(data=self.COURSE_DATA) + self.course = CourseFactory.create() # Turn off cache. modulestore().request_cache = None diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index 036cc8da2d..c9a8a54507 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -16,7 +16,6 @@ from lms.djangoapps.courseware.views.views import get_course_lti_endpoints from openedx.core.lib.url_utils import quote_slashes from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=wrong-import-order class TestLTI(BaseTestXmodule): @@ -117,7 +116,7 @@ class TestLTI(BaseTestXmodule): self.addCleanup(patcher.stop) def test_lti_constructor(self): - generated_content = self.block.render(STUDENT_VIEW).content + generated_content = self.block.student_view(None).content expected_content = self.runtime.render_template('lti.html', self.expected_context) assert generated_content == expected_content diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index dd53d4262f..3af1de706d 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -146,6 +146,8 @@ class BaseTestVideoXBlock(BaseTestXmodule): # a lot of tests code, parse and set the values as fields. fields_data = VideoBlock.parse_video_xml(data) kwargs.update(fields_data) + kwargs.pop('source', None) + kwargs.get('metadata', {}).pop('source', None) super().initialize_module(**kwargs) def setUp(self): diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 452962c06f..fa660f133c 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -43,9 +43,8 @@ from xmodule.course_block import ( COURSE_VIDEO_SHARING_PER_VIDEO ) from xmodule.exceptions import NotFoundError -from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import own_metadata -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE +from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE # noinspection PyUnresolvedReferences from xmodule.tests.helpers import override_descriptor_system # pylint: disable=unused-import from xmodule.tests.test_import import DummySystem @@ -65,11 +64,6 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from .test_video_handlers import BaseTestVideoXBlock, TestVideo from .test_video_xml import SOURCE_XML, PUBLIC_SOURCE_XML -MODULESTORES = { - ModuleStoreEnum.Type.mongo: TEST_DATA_MONGO_MODULESTORE, - ModuleStoreEnum.Type.split: TEST_DATA_SPLIT_MODULESTORE, -} - TRANSCRIPT_FILE_SRT_DATA = """ 1 00:00:14,370 --> 00:00:16,530 @@ -88,7 +82,7 @@ class TestVideoYouTube(TestVideo): # lint-amnesty, pylint: disable=missing-clas def test_video_constructor(self): """Make sure that all parameters extracted correctly from xml""" - context = self.block.render(STUDENT_VIEW).content + context = self.block.student_view(None).content sources = ['example.mp4', 'example.webm'] expected_context = { @@ -173,7 +167,7 @@ class TestVideoNonYouTube(TestVideo): # pylint: disable=test-inherits-tests """Make sure that if the 'youtube' attribute is omitted in XML, then the template generates an empty string for the YouTube streams. """ - context = self.block.render(STUDENT_VIEW).content + context = self.block.student_view(None).content sources = ['example.mp4', 'example.webm'] expected_context = { @@ -328,7 +322,7 @@ class TestVideoPublicAccess(BaseTestVideoXBlock): 'is_public_sharing_enabled', return_value=is_public_sharing_enabled ): - content = self.block.render(STUDENT_VIEW).content + content = self.block.student_view(None).content context = get_context_dict_from_string(content) assert ('public_sharing_enabled' in context) == is_public_sharing_enabled assert ('public_video_url' in context) == is_public_sharing_enabled @@ -393,7 +387,7 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): # lint-amnesty, pylint: disable=redefined-outer-name SOURCE_XML = """