From 81b1e2276c11566b9d457ce2f7b39e605329f11c Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 6 Sep 2023 10:09:16 -0400 Subject: [PATCH] Revert "feat!: remove most Old Mongo functionality (#31134)" This reverts commit c5d1807c8137e7c7243910ca2b73a032976f3042. --- .../commands/tests/test_cleanup_assets.py | 4 - .../commands/tests/test_create_course.py | 52 +- .../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 | 297 ++++--- .../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 | 8 +- .../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 | 272 +++--- .../course_overviews/tests/test_tasks.py | 7 +- xmodule/modulestore/__init__.py | 2 +- xmodule/modulestore/mongo/base.py | 438 +++++++++- 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, 3150 insertions(+), 667 deletions(-) delete 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 e31d566543fe..dfed3bb69caa 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_cleanup_assets.py @@ -4,7 +4,6 @@ """ -from unittest import skip from django.conf import settings from django.core.management import call_command from opaque_keys.edx.keys import CourseKey @@ -21,9 +20,6 @@ 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 f821c8a6d561..40d1493f94b3 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_create_course.py @@ -4,6 +4,7 @@ from io import StringIO +import ddt from django.core.management import CommandError, call_command from django.test import TestCase @@ -39,28 +40,27 @@ def test_nonexistent_user_email(self): 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 """ - def test_all_stores_user_email(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_all_stores_user_email(self, store): call_command( "create_course", - ModuleStoreEnum.Type.split, + store, 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 {ModuleStoreEnum.Type.split}" + f"Could not find course in {store}" ) # pylint: disable=protected-access - self.assertEqual( - ModuleStoreEnum.Type.split, - modulestore()._get_modulestore_for_courselike(new_key).get_modulestore_type() - ) + self.assertEqual(store, modulestore()._get_modulestore_for_courselike(new_key).get_modulestore_type()) def test_duplicate_course(self): """ @@ -85,7 +85,8 @@ def test_duplicate_course(self): expected = "Course already exists" self.assertIn(out.getvalue().strip(), expected) - def test_get_course_with_different_case(self): + @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) + def test_get_course_with_different_case(self, default_store): """ Tests that course can not be accessed with different case. @@ -97,20 +98,21 @@ def test_get_course_with_different_case(self): org = 'org1' number = 'course1' run = 'run1' - 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.') + 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)) + + # 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 5c01abde69ab..262becacbfe5 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,7 +5,6 @@ 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 @@ -14,10 +13,6 @@ 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 b9372d15204b..8750a4f59e77 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,6 +23,14 @@ def test_no_args(self): 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 ea16707d7eb8..43305ff11f2d 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_force_publish.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_force_publish.py @@ -58,6 +58,15 @@ def test_course_key_not_found(self): 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 5b90973b0463..3e00def45852 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_import.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_import.py @@ -11,6 +11,7 @@ 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 @@ -72,3 +73,23 @@ def test_truncated_course_with_url(self): # 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 0a0e8663bae3..a4a4368bdf59 100644 --- a/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py +++ b/cms/djangoapps/contentstore/tests/test_import_pure_xblock.py @@ -7,6 +7,9 @@ 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 @@ -38,6 +41,14 @@ def test_import_public(self): '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 @@ -55,12 +66,22 @@ def _assert_import(self, course_dir, expected_field_val, has_draft=False): """ # 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( - self.store, self.user.id, TEST_DATA_DIR, [course_dir], create_if_not_present=True + 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') - xblock = self.store.get_item(xblock_location) + if has_draft: + xblock_location = as_draft(xblock_location) + + xblock = 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 bb692c016033..eb9b109cdd35 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -1014,3 +1014,28 @@ def test_duplicated_version(self): 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 bb1ca0205234..0536db140587 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -12,7 +12,7 @@ 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_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_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_SPLIT_MODULESTORE + MODULESTORE = TEST_DATA_MONGO_MODULESTORE def setUp(self): """ @@ -123,7 +123,7 @@ def save_course(self): SEQUENTIAL = 'vertical_sequential' DRAFT_HTML = 'draft_html' DRAFT_VIDEO = 'draft_video' - LOCKED_ASSET_KEY = AssetKey.from_string('asset-v1:edX+toy+2012_Fall+type@asset+block@sample_static.html') + LOCKED_ASSET_KEY = AssetKey.from_string('/c4x/edX/toy/asset/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 ce5771ef83e8..a25d0670b4be 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -111,11 +111,13 @@ def setUp(self): self.course_key = self.course.id self.usage_key = self.course.location - def get_item_from_modulestore(self, usage_key): + def get_item_from_modulestore(self, usage_key, verify_is_draft=False): """ 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): @@ -538,8 +540,9 @@ def assert_xblock_info(xblock, xblock_info): class DeleteItem(ItemTest): """Tests for '/xblock' DELETE url.""" - def test_delete_static_page(self): - course = CourseFactory.create() + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_delete_static_page(self, store): + course = CourseFactory.create(default_store=store) # Add static tab resp = self.create_xblock( category="static_tab", parent_usage_key=course.location @@ -586,7 +589,7 @@ def test_create_nicely(self): 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) + problem = self.get_item_from_modulestore(prob_usage_key, verify_is_draft=True) # check against the template template = ProblemBlock.get_template(template_id) self.assertEqual(problem.data, template["data"]) @@ -804,7 +807,9 @@ def setUp(self): 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='sequential') + self.create_xblock( + parent_usage_key=self.chapter_usage_key, category="sequential2" + ) def test_duplicate_equality(self): """ @@ -971,10 +976,10 @@ def setup_course(self, default_store=None): if not default_store: default_store = self.store.default_modulestore.get_modulestore_type() - course = CourseFactory.create(default_store=default_store) + self.course = CourseFactory.create(default_store=default_store) # Create group configurations - course.user_partitions = [ + self.course.user_partitions = [ UserPartition( 0, "first_partition", @@ -982,18 +987,18 @@ def setup_course(self, default_store=None): [Group("0", "alpha"), Group("1", "beta")], ) ] - self.store.update_item(course, self.user.id) + self.store.update_item(self.course, self.user.id) # Create a parent chapter chap1 = self.create_xblock( - parent_usage_key=course.location, + parent_usage_key=self.course.location, display_name="chapter1", category="chapter", ) self.chapter_usage_key = self.response_usage_key(chap1) chap2 = self.create_xblock( - parent_usage_key=course.location, + parent_usage_key=self.course.location, display_name="chapter2", category="chapter", ) @@ -1048,8 +1053,6 @@ def setup_course(self, default_store=None): ) 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. @@ -1057,7 +1060,9 @@ def setup_and_verify_content_experiment(self, partition_id): Arguments: partition_id (int): User partition id. """ - split_test = self.get_item_from_modulestore(self.split_test_usage_key) + split_test = self.get_item_from_modulestore( + self.split_test_usage_key, verify_is_draft=True + ) # Initially, no user_partition_id is set, and the split_test has no children. self.assertEqual(split_test.user_partition_id, -1) @@ -1068,7 +1073,9 @@ def setup_and_verify_content_experiment(self, partition_id): 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) + split_test = self.get_item_from_modulestore( + self.split_test_usage_key, verify_is_draft=True + ) self.assertEqual(split_test.user_partition_id, partition_id) self.assertEqual( len(split_test.children), @@ -1134,11 +1141,15 @@ def assert_move_item(self, source_usage_key, target_usage_key, target_index=None self.assertIn(source_usage_key, target_parent.children) self.assertNotIn(source_usage_key, source_parent.children) - def test_move_component(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_component(self, store_type): """ Test move component with different xblock types. + + Arguments: + store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. """ - self.setup_course() + self.setup_course(default_store=store_type) for source_usage_key, target_usage_key in [ (self.html_usage_key, self.vert2_usage_key), (self.vert_usage_key, self.seq2_usage_key), @@ -1380,7 +1391,9 @@ def test_can_not_move_content_experiment_into_its_children(self): 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) + child_split_test = self.get_item_from_modulestore( + self.split_test_usage_key, verify_is_draft=True + ) # Try to move content experiment further down the level to a child group A nested inside main group A. response = self._move_component( @@ -1456,7 +1469,6 @@ def test_move_component_nonsensical_access_restriction_validation(self): """ 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) @@ -1469,12 +1481,10 @@ def test_move_component_nonsensical_access_restriction_validation(self): 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]} - 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) + self.store.update_item(html, self.user.id) + self.store.update_item(vert2, self.user.id) # Verify that there is no warning when html is in a non contradicting unit validation = html.validate() @@ -1483,7 +1493,7 @@ def test_move_component_nonsensical_access_restriction_validation(self): # 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 - html = self.store.update_item(html, self.user.id) + self.store.update_item(html, self.user.id) validation = html.validate() self.assertEqual(len(validation.messages), 1) self._verify_validation_message( @@ -1495,7 +1505,7 @@ def test_move_component_nonsensical_access_restriction_validation(self): # 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 - html = self.store.update_item(html, self.user.id) + self.store.update_item(html, self.user.id) validation = html.validate() self.assertEqual(len(validation.messages), 0) @@ -1517,12 +1527,16 @@ def test_move_logging(self, mock_logger): insert_at, ) - def test_move_and_discard_changes(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_and_discard_changes(self, store_type): """ 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() + self.setup_course(default_store=store_type) old_parent_loc = self.store.get_parent_location(self.html_usage_key) @@ -1580,11 +1594,15 @@ def test_move_and_discard_changes(self): self.assertIn(self.html_usage_key, source_parent.children) self.assertNotIn(self.html_usage_key, target_parent.children) - def test_move_item_not_found(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_item_not_found(self, store_type=ModuleStoreEnum.Type.mongo): """ 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() + self.setup_course(default_store=store_type) data = { "move_source_locator": str( @@ -1734,25 +1752,30 @@ def test_delete_field(self): self.client.ajax_post( self.problem_update_url, data={"metadata": {"rerandomize": "onreset"}} ) - problem = self.get_item_from_modulestore(self.problem_usage_key) - self.assertEqual(problem.rerandomize, 'onreset') + problem = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) + 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) - self.assertEqual(problem.rerandomize, 'never') + problem = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) + 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) + problem = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) self.assertIsNotNone(problem.markdown) - self.client.ajax_post( - self.problem_update_url, - data={'nullout': ['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 ) - problem = self.get_item_from_modulestore(self.problem_usage_key) self.assertIsNone(problem.markdown) def test_date_fields(self): @@ -1808,7 +1831,9 @@ def test_update_generic_fields(self): } }, ) - problem = self.get_item_from_modulestore(self.problem_usage_key) + problem = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) self.assertEqual(problem.display_name, new_display_name) self.assertEqual(problem.max_attempts, new_max_attempts) @@ -2027,7 +2052,9 @@ def test_republish(self): }, ) self.assertFalse(self._is_location_published(self.problem_usage_key)) - draft = self.get_item_from_modulestore(self.problem_usage_key) + draft = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) self.assertEqual(draft.display_name, new_display_name) # Publish the item @@ -2085,7 +2112,9 @@ def _make_draft_content_different_from_published(self): 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) + updated_draft = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) 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. @@ -2125,7 +2154,9 @@ def test_published_and_draft_contents_with_update(self): ) # Both published and draft content should be different - draft = self.get_item_from_modulestore(self.problem_usage_key) + draft = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) self.assertNotEqual(draft.data, published.data) # Get problem by 'xblock_handler' @@ -2143,7 +2174,9 @@ def test_published_and_draft_contents_with_update(self): 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) + draft = self.get_item_from_modulestore( + self.problem_usage_key, verify_is_draft=True + ) self.assertNotEqual(draft.data, published.data) # Fetch the published version again to make sure the data is correct. published = modulestore().get_item( @@ -2176,6 +2209,18 @@ def test_publish_states_of_nested_xblocks(self): 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, @@ -2301,7 +2346,9 @@ def _update_partition_id(self, partition_id): ) # Verify the partition_id was saved. - split_test = self.get_item_from_modulestore(self.split_test_usage_key) + split_test = self.get_item_from_modulestore( + self.split_test_usage_key, verify_is_draft=True + ) self.assertEqual(partition_id, split_test.user_partition_id) return split_test @@ -2309,7 +2356,7 @@ def _assert_children(self, expected_number): """ Verifies the number of children of the split_test instance. """ - split_test = self.get_item_from_modulestore(self.split_test_usage_key) + split_test = self.get_item_from_modulestore(self.split_test_usage_key, True) self.assertEqual(expected_number, len(split_test.children)) return split_test @@ -2318,7 +2365,9 @@ def test_create_groups(self): 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) + split_test = self.get_item_from_modulestore( + self.split_test_usage_key, verify_is_draft=True + ) # 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)) @@ -2328,8 +2377,12 @@ def test_create_groups(self): # 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]) - vertical_1 = self.get_item_from_modulestore(split_test.children[1]) + 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 + ) self.assertEqual("vertical", vertical_0.category) self.assertEqual("vertical", vertical_1.category) self.assertEqual( @@ -2354,7 +2407,9 @@ def test_split_xblock_info_group_name(self): """ 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) + split_test = self.get_item_from_modulestore( + self.split_test_usage_key, verify_is_draft=True + ) # 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) @@ -2396,9 +2451,15 @@ def test_change_user_partition_id(self): 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]) - vertical_1 = self.get_item_from_modulestore(split_test.children[3]) - vertical_2 = self.get_item_from_modulestore(split_test.children[4]) + 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 + ) # Verify that the group_id_to child mapping is correct. self.assertEqual(3, len(split_test.group_id_to_child)) @@ -3045,25 +3106,39 @@ def test_json_responses(self): json_response = json.loads(resp.content.decode("utf-8")) self.validate_course_xblock_info(json_response, course_outline=True) - 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' - ) + @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") + + 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(3): - 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(chapter_queries_1): + self.client.get(outline_url, HTTP_ACCEPT="application/json") def test_entrance_exam_chapter_xblock_info(self): chapter = BlockFactory.create( @@ -3189,26 +3264,32 @@ def test_component_xblock_info(self): ) self.validate_component_xblock_info(xblock_info) - def test_validate_start_date(self): + @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) + def test_validate_start_date(self, store_type): """ Validate if start-date year is less than 1900 reset the date to DEFAULT_START_DATE. """ - course = CourseFactory.create() - chapter = BlockFactory.create( - parent_location=course.location, category='chapter', display_name='Week 1' - ) + with self.store.default_store(store_type): + 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 @@ -3408,11 +3489,9 @@ def setUp(self): 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.course = self.store.update_item(self.course, self.user.id) + self.store.update_item(self.course, self.user.id) def test_proctoring_is_enabled_for_course(self): course = modulestore().get_item(self.course.location) @@ -3438,7 +3517,7 @@ def test_special_exam_xblock_info( category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_enabled=True, + is_proctored_exam=True, is_time_limited=True, default_time_limit_minutes=100, is_onboarding_exam=False, @@ -3482,7 +3561,7 @@ def test_xblock_was_ever_proctortrack_proctored_exam( category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_enabled=False, + is_proctored_exam=False, is_time_limited=False, is_onboarding_exam=False, ) @@ -3510,7 +3589,7 @@ def test_xblock_was_never_proctortrack_proctored_exam( category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_enabled=False, + is_proctored_exam=False, is_time_limited=False, is_onboarding_exam=False, ) @@ -3770,8 +3849,9 @@ def test_empty_chapter(self): xblock_info = self._get_xblock_info(empty_chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.unscheduled) - def test_chapter_self_paced_default_start_date(self): - course = CourseFactory.create() + @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) course.self_paced = True self.store.update_item(course, self.user.id) chapter = self._create_child(course, "chapter", "Test Chapter") @@ -3859,15 +3939,29 @@ def test_unpublished_changes(self): ) 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 1", 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", 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 2", 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", 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 @@ -4097,7 +4191,8 @@ def test_locked_unit_staff_only_message(self): xblock_info, True, path=self.FIRST_UNIT_PATH ) - def test_self_paced_item_visibility_state(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_self_paced_item_visibility_state(self, store_type): """ 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, @@ -4105,7 +4200,7 @@ def test_self_paced_item_visibility_state(self): """ # Create course, chapter and setup future release date to make chapter in scheduled state - course = CourseFactory.create() + course = CourseFactory.create(default_store=store_type) 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 d3bf988d2a45..7bc261651544 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -31,30 +31,29 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): def setUp(self): super().setUp() - 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.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.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=self.sequential, category='vertical', display_name='Released Private Unit', + parent_location=self.sequential.location, category='vertical', display_name='Released Private Unit', start=past) self.unreleased_private_vertical = self._create_block( - parent=self.sequential, category='vertical', display_name='Unreleased Private Unit', + parent_location=self.sequential.location, category='vertical', display_name='Unreleased Private Unit', start=future) self.released_public_vertical = self._create_block( - parent=self.sequential, category='vertical', display_name='Released Public Unit', + parent_location=self.sequential.location, category='vertical', display_name='Released Public Unit', start=past) self.unreleased_public_vertical = self._create_block( - parent=self.sequential, category='vertical', display_name='Unreleased Public Unit', + parent_location=self.sequential.location, 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( @@ -82,8 +81,8 @@ def test_container_on_container_html(self): 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, "wrapper", "Wrapper") - self._create_block(draft_container, "html", "Child HTML") + draft_container = self._create_block(self.child_container.location, "wrapper", "Wrapper") + self._create_block(draft_container.location, "html", "Child HTML") def test_container_html(xblock): self._test_html_content( @@ -178,12 +177,12 @@ def test_draft_container_preview_html(self): 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, category, display_name, **kwargs): + def _create_block(self, parent_location, category, display_name, **kwargs): """ creates a block in the module store, without publishing it. """ return BlockFactory.create( - parent=parent, + parent_location=parent_location, category=category, display_name=display_name, publish_item=False, @@ -195,7 +194,7 @@ def test_public_child_container_preview_html(self): """ 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, 'split_test', 'Split Test 1') + empty_child_container = self._create_block(self.vertical.location, 'split_test', 'Split Test') published_empty_child_container = self.store.publish(empty_child_container.location, self.user.id) self.validate_preview_html(published_empty_child_container, self.reorderable_child_view, can_add=False) @@ -203,7 +202,7 @@ def test_draft_child_container_preview_html(self): """ 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, 'split_test', 'Split Test 1') + empty_child_container = self._create_block(self.vertical.location, 'split_test', 'Split Test') 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 979f8322e9b4..c1a745f2e2e0 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=self.course, + self.chapter = BlockFactory.create(parent_location=self.course.location, category='chapter', display_name="Week 1") - self.sequential = BlockFactory.create(parent=self.chapter, + self.sequential = BlockFactory.create(parent_location=self.chapter.location, 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 f3721a622d86..97b92546d6bb 100644 --- a/cms/lib/xblock/test/test_authoring_mixin.py +++ b/cms/lib/xblock/test/test_authoring_mixin.py @@ -158,6 +158,14 @@ def test_html_empty_partition_staff_locked(self): [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']) @@ -170,6 +178,20 @@ def test_html_false_content_group(self): [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 3a69f64b2b6c..844798724962 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -9,7 +9,7 @@ from django.urls import reverse from pytz import UTC from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.data import CertificatesDisplayBehaviors @@ -31,7 +31,7 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase): """Tests display of certificates on the student dashboard. """ - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE USERNAME = "test_user" PASSWORD = "password" @@ -93,16 +93,15 @@ def setUpClass(cls): 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 2f7b6c41d145..e26763939fc9 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -103,13 +103,12 @@ def test_errored_course_regular_access(self): """ Test the course list for regular staff when get_course returns an ErrorBlock """ - store = modulestore() - course_key = store.make_course_key('Org1', 'Course1', 'Run1') - self._create_course_with_access_groups(course_key) + # 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) - with mock.patch( - 'xmodule.modulestore.split_mongo.caching_descriptor_system.SplitMongoKVS', mock.Mock(side_effect=Exception) - ): + with mock.patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', 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. @@ -123,14 +122,14 @@ def test_course_listing_errored_deleted_courses(self): Create good courses, courses that won't load, and deleted courses which still have roles. Test course listing. """ - store = modulestore() + mongo_store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) # lint-amnesty, pylint: disable=protected-access - good_location = store.make_course_key('testOrg', 'testCourse', 'RunBabyRun') - self._create_course_with_access_groups(good_location) + good_location = mongo_store.make_course_key('testOrg', 'testCourse', 'RunBabyRun') + self._create_course_with_access_groups(good_location, default_store=ModuleStoreEnum.Type.mongo) - 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) + 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) 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 ec11fc05de66..60971df57741 100644 --- a/lms/djangoapps/course_api/tests/test_serializers.py +++ b/lms/djangoapps/course_api/tests/test_serializers.py @@ -2,6 +2,7 @@ Test data created by CourseSerializer and CourseDetailSerializer """ + from datetime import datetime from unittest import TestCase, mock @@ -11,7 +12,7 @@ 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_ONLY_SPLIT_MODULESTORE_DRAFT_PREFERRED, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import check_mongo_calls from lms.djangoapps.certificates.api import can_show_certificate_available_date_field @@ -31,7 +32,7 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase): maxDiff = 5000 # long enough to show mismatched dicts, in case of error serializer_class = CourseSerializer - MODULESTORE = TEST_DATA_ONLY_SPLIT_MODULESTORE_DRAFT_PREFERRED + MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE ENABLED_SIGNALS = ['course_published'] def setUp(self): @@ -40,10 +41,10 @@ def setUp(self): self.honor_user = self.create_user('honor', is_staff=False) self.request_factory = APIRequestFactory() - course_id = 'course-v1:edX+toy+2012_Fall' - banner_image_uri = '/asset-v1:edX+toy+2012_Fall+type@asset+block@images_course_image.jpg' + course_id = 'edX/toy/2012_Fall' + banner_image_uri = '/c4x/edX/toy/asset/images_course_image.jpg' banner_image_absolute_uri = 'http://testserver' + banner_image_uri - image_path = '/asset-v1:edX+toy+2012_Fall+type@asset+block@just_a_test.jpg' + image_path = '/c4x/edX/toy/asset/just_a_test.jpg' image_url = 'http://testserver' + image_path self.expected_data = { 'id': course_id, @@ -54,15 +55,19 @@ def setUp(self): 'media': { 'banner_image': { 'uri': banner_image_uri, - 'uri_absolute': banner_image_absolute_uri + 'uri_absolute': banner_image_absolute_uri, + }, + 'course_image': { + 'uri': image_path, + }, + 'course_video': { + 'uri': 'http://www.youtube.com/watch?v=test_youtube_id', }, - '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', @@ -70,11 +75,11 @@ def setUp(self): '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=course-v1%3AedX%2Btoy%2B2012_Fall', + 'blocks_url': 'http://testserver/api/courses/v2/blocks/?course_id=edX%2Ftoy%2F2012_Fall', 'effort': '6 hours', 'pacing': 'instructor', 'mobile_available': True, - 'hidden': False, + 'hidden': True, # because it's an old mongo course 'invitation_only': False, # 'course_id' is a deprecated field, please use 'id' instead. @@ -121,14 +126,14 @@ def test_advertised_start(self): advertised_start='The Ides of March' ) result = self._get_result(course) - assert result['course_id'] == 'course-v1:edX+custom+2012_Fall' + assert result['course_id'] == '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'] == 'course-v1:edX+custom+2012_Fall' + assert result['course_id'] == '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 d0fef5065b43..e522bc5b65d3 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 ddt import pytz from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_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_SPLIT_MODULESTORE + MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE @classmethod def setUpClass(cls): @@ -52,10 +52,10 @@ def setUp(self): self.learner.id, subsection.location, 'html', 'new_component' ) CourseEnrollmentFactory.create(user=self.learner, course_id=self.course_key, is_active=True) - 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' + self.learner2.id, subsection.location, 'html', 'new_component' ) + CourseEnrollmentFactory.create(user=self.learner2, course_id=self.course_key, is_active=True) @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 daf51c7c0054..86c6e54b8a9b 100644 --- a/lms/djangoapps/course_wiki/tests/test_access.py +++ b/lms/djangoapps/course_wiki/tests/test_access.py @@ -5,7 +5,7 @@ from django.contrib.auth.models import Group from wiki.models import URLPath -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import InstructorFactory @@ -18,14 +18,14 @@ class TestWikiAccessBase(ModuleStoreTestCase): """Base class for testing wiki access.""" - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE def setUp(self): super().setUp() self.wiki = get_or_create_root() - self.course_math101 = CourseFactory.create(org='org', number='math101', display_name='Course') # lint-amnesty, pylint: disable=line-too-long + 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_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 @@ def setUp(self): 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') # lint-amnesty, pylint: disable=line-too-long + 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_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 d9fa62608cb1..16937cc77289 100644 --- a/lms/djangoapps/course_wiki/tests/test_middleware.py +++ b/lms/djangoapps/course_wiki/tests/test_middleware.py @@ -5,7 +5,7 @@ from django.test.client import Client from wiki.models import URLPath -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import InstructorFactory @@ -14,7 +14,7 @@ class TestWikiAccessMiddleware(ModuleStoreTestCase): """Tests for WikiAccessMiddleware.""" - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE def setUp(self): """Test setup.""" @@ -22,7 +22,7 @@ def setUp(self): self.wiki = get_or_create_root() - self.course_math101 = CourseFactory.create(org='edx', number='math101', display_name='2014') + 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_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 @@ def setUp(self): def test_url_tranform(self): """Test that the correct prefix ('/courses/') is added to the urls in the wiki.""" - 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/') + 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/') diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 3a2ddbc45630..577f8b2ca323 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -31,7 +31,7 @@ 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_SPLIT_MODULESTORE, ModuleStoreTestCase # 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.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,14 +47,15 @@ class BaseTestXmodule(ModuleStoreTestCase): 1. CATEGORY 2. DATA or METADATA 3. MODEL_DATA - 4. USER_COUNT if needed + 4. COURSE_DATA and USER_COUNT if needed This class should not contain any tests, because CATEGORY should be defined in child class. """ - MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + MODULESTORE = TEST_DATA_MONGO_MODULESTORE USER_COUNT = 2 + COURSE_DATA = {} # Data from YAML xmodule/templates/NAME/default.yaml CATEGORY = "vertical" @@ -100,7 +101,7 @@ def initialize_module(self, runtime_kwargs=None, **kwargs): # lint-amnesty, pyl self.item_url = str(self.block.location) def setup_course(self): # lint-amnesty, pylint: disable=missing-function-docstring - self.course = CourseFactory.create() + self.course = CourseFactory.create(data=self.COURSE_DATA) # 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 c9a8a5450712..036cc8da2d7e 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -16,6 +16,7 @@ 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): @@ -116,7 +117,7 @@ def mocked_sign(self, *args, **kwargs): self.addCleanup(patcher.stop) def test_lti_constructor(self): - generated_content = self.block.student_view(None).content + generated_content = self.block.render(STUDENT_VIEW).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 3af1de706d4a..dd53d4262f98 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -146,8 +146,6 @@ def initialize_block(self, data=None, **kwargs): # 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 fa660f133c74..452962c06f53 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -43,8 +43,9 @@ 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_SPLIT_MODULESTORE +from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_MODULESTORE, 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 @@ -64,6 +65,11 @@ 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 @@ -82,7 +88,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.student_view(None).content + context = self.block.render(STUDENT_VIEW).content sources = ['example.mp4', 'example.webm'] expected_context = { @@ -167,7 +173,7 @@ def test_video_constructor(self): """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.student_view(None).content + context = self.block.render(STUDENT_VIEW).content sources = ['example.mp4', 'example.webm'] expected_context = { @@ -322,7 +328,7 @@ def test_context(self, is_public_sharing_enabled): 'is_public_sharing_enabled', return_value=is_public_sharing_enabled ): - content = self.block.student_view(None).content + content = self.block.render(STUDENT_VIEW).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 @@ -387,7 +393,7 @@ def test_get_html_track(self): # lint-amnesty, pylint: disable=redefined-outer-name SOURCE_XML = """