diff --git a/.tx/config b/.tx/config index 4f1355038a8b..26438e05f1b4 100644 --- a/.tx/config +++ b/.tx/config @@ -42,3 +42,9 @@ file_filter = conf/locale//LC_MESSAGES/messages.po source_file = conf/locale/en/LC_MESSAGES/messages.po source_lang = en type = PO + +[edx-platform.wiki] +file_filter = conf/locale//LC_MESSAGES/wiki.po +source_file = conf/locale/en/LC_MESSAGES/wiki.po +source_lang = en +type = PO diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5006cb0cb227..c02f57ae6610 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -52,6 +52,12 @@ Blades: Fix comparison of float numbers. BLD-434. Blades: Allow regexp strings as the correct answer to a string response question. BLD-475. +Common: MixedModulestore is now the only approved access to the persistence layer + - takes a new parameter 'reference_type' which can be 'Location' or 'Locator'. Mixed + then tries to ensure that every reference in any xblock gets converted to that type on + retrieval. Because we're moving to Locators, the default is Locator; so, you should change + all existing configurations to 'Location' (unless you're using split) + Common: Add feature flags to allow developer use of pure XBlocks - ALLOW_ALL_ADVANCED_COMPONENTS disables the hard-coded list of advanced components in Studio, and allows any xblock to be added as an diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index 314793db72ac..9dfd00a0da96 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -56,7 +56,7 @@ def get_course_updates(location, provided_id): return course_upd_collection -def update_course_updates(location, update, passed_id=None): +def update_course_updates(location, update, passed_id=None, user=None): """ Either add or update the given course update. It will add it if the passed_id is absent or None. It will update it if it has an passed_id which has a valid value. Until updates have distinct values, the passed_id is the location url + an index @@ -102,7 +102,7 @@ def update_course_updates(location, update, passed_id=None): # update db record course_updates.data = html.tostring(course_html_parsed) - modulestore('direct').update_item(location, course_updates.data) + modulestore('direct').update_item(course_updates, user.id if user else None) return { "id": idx, @@ -125,7 +125,7 @@ def _course_info_content(html_parsed): # pylint: disable=unused-argument -def delete_course_update(location, update, passed_id): +def delete_course_update(location, update, passed_id, user): """ Delete the given course_info update from the db. Returns the resulting course_updates b/c their ids change. @@ -158,7 +158,7 @@ def delete_course_update(location, update, passed_id): # update db record course_updates.data = html.tostring(course_html_parsed) store = modulestore('direct') - store.update_item(location, course_updates.data) + store.update_item(course_updates, user.id) return get_course_updates(location, None) diff --git a/cms/djangoapps/contentstore/features/video-editor.py b/cms/djangoapps/contentstore/features/video-editor.py index 4036f10351a6..b9137e9fb277 100644 --- a/cms/djangoapps/contentstore/features/video-editor.py +++ b/cms/djangoapps/contentstore/features/video-editor.py @@ -44,7 +44,7 @@ def correct_video_settings(_step): ['HTML5 Transcript', '', False], ['Show Transcript', 'True', False], ['Start Time', '00:00:00', False], - ['Transcript Download Allowed', 'False', False], + # ['Transcript Download Allowed', 'False', False], ['Video Download Allowed', 'False', False], ['Video Sources', '', False], ['Youtube ID', 'OEoXaMPEzfM', False], diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index d28a3ab3de40..b904a65afc61 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -47,8 +47,6 @@ from django_comment_common.utils import are_permissions_roles_seeded from xmodule.exceptions import InvalidVersionError -import datetime -from pytz import UTC from uuid import uuid4 from pymongo import MongoClient from student.models import CourseEnrollment @@ -126,11 +124,7 @@ def check_components_on_page(self, component_types, expected_types): course.advanced_modules = component_types - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - course.save() - - store.update_metadata(course.location, own_metadata(course)) + store.update_item(course, self.user.id) # just pick one vertical descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0] @@ -269,7 +263,7 @@ def test_draft_metadata(self): self.assertIn('graceperiod', own_metadata(html_module)) self.assertEqual(html_module.graceperiod, new_graceperiod) - draft_store.update_metadata(html_module.location, own_metadata(html_module)) + draft_store.update_item(html_module, self.user.id) # read back to make sure it reads as 'own-metadata' html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) @@ -385,8 +379,7 @@ def test_create_static_tab_and_rename(self): self.assertEqual(course.tabs, expected_tabs) item.display_name = 'Updated' - item.save() - module_store.update_metadata(item.location, own_metadata(item)) + module_store.update_item(item, self.user.id) course = module_store.get_item(course_location) @@ -834,9 +827,9 @@ def test_portable_link_rewrites_during_clone_course(self): html_module = module_store.get_instance(source_location.course_id, html_module_location) self.assertIsInstance(html_module.data, basestring) - new_data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format( + new_data = html_module.data = html_module.data.replace('/static/', '/c4x/{0}/{1}/asset/'.format( source_location.org, source_location.course)) - module_store.update_item(html_module_location, new_data) + module_store.update_item(html_module, self.user.id) html_module = module_store.get_instance(source_location.course_id, html_module_location) self.assertEqual(new_data, html_module.data) @@ -858,22 +851,18 @@ def test_illegal_draft_crud_ops(self): draft_store = modulestore('draft') direct_store = modulestore('direct') - CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') + course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') location = Location('i4x://MITx/999/chapter/neuvo') # Ensure draft mongo store does not allow us to create chapters either directly or via convert to draft self.assertRaises(InvalidVersionError, draft_store.create_and_save_xmodule, location) direct_store.create_and_save_xmodule(location) self.assertRaises(InvalidVersionError, draft_store.convert_to_draft, location) + chapter = draft_store.get_instance(course.id, location) + chapter.data = 'chapter data' - self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data') - - # taking advantage of update_children and other functions never checking that the ids are valid - self.assertRaises(InvalidVersionError, draft_store.update_children, location, - ['i4x://MITx/999/problem/doesntexist']) - - self.assertRaises(InvalidVersionError, draft_store.update_metadata, location, - {'due': datetime.datetime.now(UTC)}) + with self.assertRaises(InvalidVersionError): + draft_store.update_item(chapter, self.user.id) self.assertRaises(InvalidVersionError, draft_store.unpublish, location) @@ -992,8 +981,8 @@ def test_export_course(self, mock_get): sequential = module_store.get_item(Location(['i4x', 'edX', 'toy', 'sequential', 'vertical_sequential', None])) private_location_no_draft = private_vertical.location.replace(revision=None) - module_store.update_children(sequential.location, sequential.children + - [private_location_no_draft.url()]) + sequential.children.append(private_location_no_draft.url()) + module_store.update_item(sequential, self.user.id) # read back the sequential, to make sure we have a pointer to sequential = module_store.get_item(Location(['i4x', 'edX', 'toy', @@ -1285,31 +1274,6 @@ def test_prefetch_children(self): self.assertFalse(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]) in course.system.module_data) - def test_export_course_with_unknown_metadata(self): - module_store = modulestore('direct') - content_store = contentstore() - - import_from_xml(module_store, 'common/test/data/', ['toy']) - location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') - - root_dir = path(mkdtemp_clean()) - - course = module_store.get_item(location) - - metadata = own_metadata(course) - # add a bool piece of unknown metadata so we can verify we don't throw an exception - metadata['new_metadata'] = True - - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - course.save() - module_store.update_metadata(location, metadata) - - print 'Exporting to tempdir = {0}'.format(root_dir) - - # export out to a tempdir - export_to_xml(module_store, content_store, location, root_dir, 'test_export') - def test_export_course_without_content_store(self): module_store = modulestore('direct') content_store = contentstore() @@ -1319,16 +1283,7 @@ def test_export_course_without_content_store(self): import_from_xml(module_store, 'common/test/data/', ['toy']) location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') - # Add a sequence - stub_location = Location(['i4x', 'edX', 'toy', 'sequential', 'vertical_sequential']) - sequential = module_store.get_item(stub_location) - module_store.update_children(sequential.location, sequential.children) - - # Get course and export it without a content_store - - course = module_store.get_item(location) - course.save() root_dir = path(mkdtemp_clean()) @@ -1343,7 +1298,7 @@ def test_export_course_without_content_store(self): module_store, root_dir, ['test_export_no_content_store'], draft_store=None, static_content_store=None, - target_location_namespace=course.location + target_location_namespace=location ) # Verify reimported course @@ -1810,7 +1765,8 @@ def test_metadata_inheritance(self): # crate a new module and add it as a child to a vertical module_store.create_and_save_xmodule(new_component_location) parent = verticals[0] - module_store.update_children(parent.location, parent.children + [new_component_location.url()]) + parent.children.append(new_component_location.url()) + module_store.update_item(parent, self.user.id) # flush the cache module_store.refresh_cached_metadata_inheritance_tree(new_component_location) @@ -1827,8 +1783,7 @@ def test_metadata_inheritance(self): # now let's define an override at the leaf node level # new_module.graceperiod = timedelta(1) - new_module.save() - module_store.update_metadata(new_module.location, own_metadata(new_module)) + module_store.update_item(new_module, self.user.id) # flush the cache and refetch module_store.refresh_cached_metadata_inheritance_tree(new_component_location) @@ -1942,10 +1897,7 @@ def test_metadata_not_persistence(self): delattr(self.video_descriptor, field_name) self.assertNotIn('html5_sources', own_metadata(self.video_descriptor)) - get_modulestore(location).update_metadata( - location, - own_metadata(self.video_descriptor) - ) + get_modulestore(location).update_item(self.video_descriptor, '**replace_user**') module = get_modulestore(location).get_item(location) self.assertNotIn('html5_sources', own_metadata(module)) @@ -2001,7 +1953,7 @@ def _course_factory_create_course(): Creates a course via the CourseFactory and returns the locator for it. """ course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course') - return loc_mapper().translate_location(course.location.course_id, course.location, True, True) + return loc_mapper().translate_location(course.id, course.location, False, True) def _get_course_id(test_course_data): diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index bfe244820d9b..9b33654b70f8 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -73,32 +73,32 @@ def test_update_and_fetch(self): jsondetails.syllabus = "bar" # encode - decode to convert date fields and other data which changes form self.assertEqual( - CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).syllabus, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).syllabus, jsondetails.syllabus, "After set syllabus" ) jsondetails.overview = "Overview" self.assertEqual( - CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).overview, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).overview, jsondetails.overview, "After set overview" ) jsondetails.intro_video = "intro_video" self.assertEqual( - CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).intro_video, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).intro_video, jsondetails.intro_video, "After set intro_video" ) jsondetails.effort = "effort" self.assertEqual( - CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).effort, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).effort, jsondetails.effort, "After set effort" ) jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC()) self.assertEqual( - CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).start_date, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).start_date, jsondetails.start_date ) jsondetails.course_image_name = "an_image.jpg" self.assertEqual( - CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).course_image_name, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__, self.user).course_image_name, jsondetails.course_image_name ) @@ -120,8 +120,8 @@ def test_marketing_site_fetch(self): self.assertContains(response, "Introducing Your Course") self.assertContains(response, "Course Image") - self.assertNotContains(response,"Course Overview") - self.assertNotContains(response,"Course Introduction Video") + self.assertNotContains(response, "Course Overview") + self.assertNotContains(response, "Course Introduction Video") self.assertNotContains(response, "Requirements") def test_regular_site_fetch(self): @@ -141,8 +141,8 @@ def test_regular_site_fetch(self): self.assertContains(response, "Introducing Your Course") self.assertContains(response, "Course Image") - self.assertContains(response,"Course Overview") - self.assertContains(response,"Course Introduction Video") + self.assertContains(response, "Course Overview") + self.assertContains(response, "Course Introduction Video") self.assertContains(response, "Requirements") @@ -241,67 +241,74 @@ def test_fetch_grader(self): def test_update_from_json(self): test_grader = CourseGradingModel.fetch(self.course_locator) - altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Noop update") test_grader.graders[0]['weight'] = test_grader.graders[0].get('weight') * 2 - altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Weight[0] * 2") test_grader.grade_cutoffs['D'] = 0.3 - altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "cutoff add D") test_grader.grace_period = {'hours': 4, 'minutes': 5, 'seconds': 0} - altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__, self.user) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "4 hour grace period") def test_update_grader_from_json(self): test_grader = CourseGradingModel.fetch(self.course_locator) - altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1]) + altered_grader = CourseGradingModel.update_grader_from_json( + self.course_locator, test_grader.graders[1], self.user + ) self.assertDictEqual(test_grader.graders[1], altered_grader, "Noop update") test_grader.graders[1]['min_count'] = test_grader.graders[1].get('min_count') + 2 - altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1]) + altered_grader = CourseGradingModel.update_grader_from_json( + self.course_locator, test_grader.graders[1], self.user) self.assertDictEqual(test_grader.graders[1], altered_grader, "min_count[1] + 2") test_grader.graders[1]['drop_count'] = test_grader.graders[1].get('drop_count') + 1 - altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1]) + altered_grader = CourseGradingModel.update_grader_from_json( + self.course_locator, test_grader.graders[1], self.user) self.assertDictEqual(test_grader.graders[1], altered_grader, "drop_count[1] + 2") def test_update_cutoffs_from_json(self): test_grader = CourseGradingModel.fetch(self.course_locator) - CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs) + CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs, self.user) # Unlike other tests, need to actually perform a db fetch for this test since update_cutoffs_from_json # simply returns the cutoffs you send into it, rather than returning the db contents. altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "Noop update") test_grader.grade_cutoffs['D'] = 0.3 - CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs) + CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs, self.user) altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "cutoff add D") test_grader.grade_cutoffs['Pass'] = 0.75 - CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs) + CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs, self.user) altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "cutoff change 'Pass'") def test_delete_grace_period(self): test_grader = CourseGradingModel.fetch(self.course_locator) - CourseGradingModel.update_grace_period_from_json(self.course_locator, test_grader.grace_period) + CourseGradingModel.update_grace_period_from_json( + self.course_locator, test_grader.grace_period, self.user + ) # update_grace_period_from_json doesn't return anything, so query the db for its contents. altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertEqual(test_grader.grace_period, altered_grader.grace_period, "Noop update") test_grader.grace_period = {'hours': 15, 'minutes': 5, 'seconds': 30} - CourseGradingModel.update_grace_period_from_json(self.course_locator, test_grader.grace_period) + CourseGradingModel.update_grace_period_from_json( + self.course_locator, test_grader.grace_period, self.user) altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grace_period, altered_grader.grace_period, "Adding in a grace period") test_grader.grace_period = {'hours': 1, 'minutes': 10, 'seconds': 0} # Now delete the grace period - CourseGradingModel.delete_grace_period(self.course_locator) + CourseGradingModel.delete_grace_period(self.course_locator, self.user) # update_grace_period_from_json doesn't return anything, so query the db for its contents. altered_grader = CourseGradingModel.fetch(self.course_locator) # Once deleted, the grace period should simply be None @@ -317,7 +324,7 @@ def test_update_section_grader_type(self): self.assertEqual(False, descriptor.graded) # Change the default grader type to Homework, which should also mark the section as graded - CourseGradingModel.update_section_grader_type(self.course, 'Homework') + CourseGradingModel.update_section_grader_type(self.course, 'Homework', self.user) descriptor = get_modulestore(self.course.location).get_item(self.course.location) section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator) @@ -326,7 +333,7 @@ def test_update_section_grader_type(self): self.assertEqual(True, descriptor.graded) # Change the grader type back to notgraded, which should also unmark the section as graded - CourseGradingModel.update_section_grader_type(self.course, 'notgraded') + CourseGradingModel.update_section_grader_type(self.course, 'notgraded', self.user) descriptor = get_modulestore(self.course.location).get_item(self.course.location) section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator) @@ -439,19 +446,27 @@ def test_fetch_initial_fields(self): self.assertIn('xqa_key', test_model, 'xqa_key field ') def test_update_from_json(self): - test_model = CourseMetadata.update_from_json(self.course, { - "advertised_start": "start A", - "days_early_for_beta": 2 - }) + test_model = CourseMetadata.update_from_json( + self.course, + { + "advertised_start": "start A", + "days_early_for_beta": 2, + }, + user=self.user + ) self.update_check(test_model) # try fresh fetch to ensure persistence fresh = modulestore().get_item(self.course_location) test_model = CourseMetadata.fetch(fresh) self.update_check(test_model) # now change some of the existing metadata - test_model = CourseMetadata.update_from_json(fresh, { - "advertised_start": "start B", - "display_name": "jolly roger"} + test_model = CourseMetadata.update_from_json( + fresh, + { + "advertised_start": "start B", + "display_name": "jolly roger", + }, + user=self.user ) self.assertIn('display_name', test_model, 'Missing editable metadata field') self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value") @@ -468,9 +483,9 @@ def update_check(self, test_model): def test_delete_key(self): test_model = CourseMetadata.update_from_json( - self.fullcourse, { - "unsetKeys": ['showanswer', 'xqa_key'] - } + self.fullcourse, + {"unsetKeys": ['showanswer', 'xqa_key']}, + user=self.user ) # ensure no harm self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in') diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index 6094c7cf3b97..290b90e4bc2a 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -172,7 +172,7 @@ def test_block_generations(self): ) first_problem.max_attempts = 3 first_problem.save() # decache the above into the kvs - updated_problem = modulestore('split').update_item(first_problem, 'testbot') + updated_problem = modulestore('split').update_item(first_problem, '**replace_user**') self.assertIsNotNone(updated_problem.previous_version) self.assertEqual(updated_problem.previous_version, first_problem.update_version) self.assertNotEqual(updated_problem.update_version, first_problem.update_version) diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py index e7babd25fd49..0bdcb69bbc3f 100644 --- a/cms/djangoapps/contentstore/tests/test_orphan.py +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -4,7 +4,6 @@ import json from contentstore.tests.utils import CourseTestCase from xmodule.modulestore.django import editable_modulestore, loc_mapper -from django.core.urlresolvers import reverse from student.models import CourseEnrollment class TestOrphan(CourseTestCase): @@ -35,7 +34,7 @@ def _create_item(self, category, name, data, metadata, parent_category, parent_n parent_location = self.course.location.replace(category=parent_category, name=parent_name) parent = editable_modulestore('direct').get_item(parent_location) parent.children.append(location.url()) - editable_modulestore('direct').update_children(parent_location, parent.children) + editable_modulestore('direct').update_item(parent, self.user.id) def test_mongo_orphan(self): """ diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index bc8c2aa8fa3d..97c4cddc221b 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -3,11 +3,13 @@ """ import time import mock +import unittest from django.test.utils import override_settings from django.core.cache import cache from django.core.urlresolvers import reverse from django.conf import settings +from django.contrib.auth.models import User from contentstore.tests.utils import parse_json, user, registration, AjaxEnabledTestClient from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -19,6 +21,7 @@ from freezegun import freeze_time + @override_settings(MODULESTORE=TEST_MODULESTORE) class ContentStoreTestCase(ModuleStoreTestCase): def _login(self, email, password): @@ -119,6 +122,32 @@ def test_create_account(self): self.create_account(self.username, self.email, self.pw) self.activate_user(self.email) + def test_create_account_username_already_exists(self): + User.objects.create_user(self.username, self.email, self.pw) + resp = self._create_account(self.username, "abc@def.com", "password") + # we have a constraint on unique usernames, so this should fail + self.assertEqual(resp.status_code, 400) + + def test_create_account_pw_already_exists(self): + User.objects.create_user(self.username, self.email, self.pw) + resp = self._create_account("abcdef", "abc@def.com", self.pw) + # we can have two users with the same password, so this should succeed + self.assertEqual(resp.status_code, 200) + + @unittest.skipUnless(settings.SOUTH_TESTS_MIGRATE, "South migrations required") + def test_create_account_email_already_exists(self): + User.objects.create_user(self.username, self.email, self.pw) + resp = self._create_account("abcdef", self.email, "password") + # This is tricky. Django's user model doesn't have a constraint on + # unique email addresses, but we *add* that constraint during the + # migration process: + # see common/djangoapps/student/migrations/0004_add_email_index.py + # + # The behavior we *want* is for this account creation request + # to fail, due to this uniqueness constraint, but the request will + # succeed if the migrations have not run. + self.assertEqual(resp.status_code, 400) + def test_login(self): self.create_account(self.username, self.email, self.pw) diff --git a/cms/djangoapps/contentstore/transcripts_utils.py b/cms/djangoapps/contentstore/transcripts_utils.py index 79024bc4faef..68f1b29d0db8 100644 --- a/cms/djangoapps/contentstore/transcripts_utils.py +++ b/cms/djangoapps/contentstore/transcripts_utils.py @@ -17,7 +17,6 @@ from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore from xmodule.modulestore import Location -from xmodule.modulestore.inheritance import own_metadata from .utils import get_modulestore @@ -280,16 +279,16 @@ def generate_srt_from_sjson(sjson_subs, speed): return output -def save_module(item): +def save_module(item, user): """ Proceed with additional save operations. """ item.save() store = get_modulestore(Location(item.id)) - store.update_metadata(item.id, own_metadata(item)) + store.update_item(item, user.id if user else None) -def copy_or_rename_transcript(new_name, old_name, item, delete_old=False): +def copy_or_rename_transcript(new_name, old_name, item, delete_old=False, user=None): """ Renames `old_name` transcript file in storage to `new_name`. @@ -303,12 +302,12 @@ def copy_or_rename_transcript(new_name, old_name, item, delete_old=False): transcripts = contentstore().find(content_location).data save_subs_to_store(json.loads(transcripts), new_name, item) item.sub = new_name - save_module(item) + save_module(item, user) if delete_old: remove_subs_from_store(old_name, item) -def manage_video_subtitles_save(old_item, new_item): +def manage_video_subtitles_save(old_item, new_item, user): """ Does some specific things, that can be done only on save. @@ -340,7 +339,7 @@ def manage_video_subtitles_save(old_item, new_item): # copy_or_rename_transcript changes item.sub of module try: # updates item.sub with `video_id`, if it is successful. - copy_or_rename_transcript(video_id, sub_name, new_item) + copy_or_rename_transcript(video_id, sub_name, new_item, user=user) except NotFoundError: # subtitles file `sub_name` is not presented in the system. Nothing to copy or rename. log.debug( diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index fbe04186c43c..29874583c455 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -222,16 +222,6 @@ def compute_unit_state(unit): return UnitState.public -def update_item(location, value): - """ - If value is None, delete the db entry. Otherwise, update it using the correct modulestore. - """ - if value is None: - get_modulestore(location).delete_item(location) - else: - get_modulestore(location).update_item(location, value) - - def add_extra_panel_tab(tab_type, course): """ Used to add the panel tab to a course if it does not exist. diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 123ff08f9a0a..352ead8cce20 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -11,9 +11,6 @@ from django.core.exceptions import PermissionDenied from xmodule.modulestore.django import loc_mapper -from xmodule.modulestore.inheritance import own_metadata - - from ..utils import get_modulestore from .access import has_course_access from xmodule.course_module import CourseDescriptor @@ -51,8 +48,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_ # from the template. if not course_module.checklists: course_module.checklists = CourseDescriptor.checklists.default - course_module.save() - modulestore.update_metadata(old_location, own_metadata(course_module)) + modulestore.update_item(course_module, request.user.id) expanded_checklists = expand_all_action_urls(course_module) if json_request: @@ -81,7 +77,7 @@ def checklists_handler(request, tag=None, package_id=None, branch=None, version_ # not default course_module.checklists = course_module.checklists course_module.save() - modulestore.update_metadata(old_location, own_metadata(course_module)) + modulestore.update_item(course_module, request.user.id) expanded_checklist = expand_checklist_action_url(course_module, persisted_checklist) return JsonResponse(expanded_checklist) else: diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e930e33c1a5e..782ea03c3b0f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -20,7 +20,6 @@ from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore, loc_mapper -from xmodule.modulestore.inheritance import own_metadata from xmodule.contentstore.content import StaticContent from xmodule.modulestore.exceptions import ( @@ -163,8 +162,7 @@ def course_listing(request): """ List all courses available to the logged in user """ - # there's an index on category which will be used if none of its antecedents are set - courses = modulestore('direct').get_items(Location(None, None, None, 'course', None)) + courses = modulestore('direct').get_courses() # filter out courses that we don't have access too def course_filter(course): @@ -331,7 +329,7 @@ def create_new_course(request): definition_data=overview_template.get('data') ) - initialize_course_tabs(new_course) + initialize_course_tabs(new_course, request.user) new_location = loc_mapper().translate_location(new_course.location.course_id, new_course.location, False, True) # can't use auth.add_users here b/c it requires request.user to already have Instructor perms in this course @@ -417,7 +415,7 @@ def course_info_update_handler(request, tag=None, package_id=None, branch=None, return JsonResponse(get_course_updates(updates_location, provided_id)) elif request.method == 'DELETE': try: - return JsonResponse(delete_course_update(updates_location, request.json, provided_id)) + return JsonResponse(delete_course_update(updates_location, request.json, provided_id, request.user)) except: return HttpResponseBadRequest( "Failed to delete", @@ -426,7 +424,7 @@ def course_info_update_handler(request, tag=None, package_id=None, branch=None, # can be either and sometimes django is rewriting one to the other: elif request.method in ('POST', 'PUT'): try: - return JsonResponse(update_course_updates(updates_location, request.json, provided_id)) + return JsonResponse(update_course_updates(updates_location, request.json, provided_id, request.user)) except: return HttpResponseBadRequest( "Failed to save", @@ -479,7 +477,7 @@ def settings_handler(request, tag=None, package_id=None, branch=None, version_gu ) else: # post or put, doesn't matter. return JsonResponse( - CourseDetails.update_from_json(locator, request.json), + CourseDetails.update_from_json(locator, request.json, request.user), encoder=CourseSettingsEncoder ) @@ -526,15 +524,15 @@ def grading_handler(request, tag=None, package_id=None, branch=None, version_gui # None implies update the whole model (cutoffs, graceperiod, and graders) not a specific grader if grader_index is None: return JsonResponse( - CourseGradingModel.update_from_json(locator, request.json), + CourseGradingModel.update_from_json(locator, request.json, request.user), encoder=CourseSettingsEncoder ) else: return JsonResponse( - CourseGradingModel.update_grader_from_json(locator, request.json) + CourseGradingModel.update_grader_from_json(locator, request.json, request.user) ) elif request.method == "DELETE" and grader_index is not None: - CourseGradingModel.delete_grader(locator, grader_index) + CourseGradingModel.delete_grader(locator, grader_index, request.user) return JsonResponse() @@ -625,7 +623,8 @@ def advanced_settings_handler(request, package_id=None, branch=None, version_gui return JsonResponse(CourseMetadata.update_from_json( course_module, request.json, - filter_tabs=filter_tabs + filter_tabs=filter_tabs, + user=request.user, )) except (TypeError, ValueError) as err: return HttpResponseBadRequest( @@ -743,10 +742,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs): course.tabs.append({"type": "pdf_textbooks"}) course.pdf_textbooks = textbooks - store.update_metadata( - course.location, - own_metadata(course) - ) + store.update_item(course, request.user.id) return JsonResponse(course.pdf_textbooks) elif request.method == 'POST': # create a new textbook for the course @@ -764,7 +760,7 @@ def textbooks_list_handler(request, tag=None, package_id=None, branch=None, vers tabs = course.tabs tabs.append({"type": "pdf_textbooks"}) course.tabs = tabs - store.update_metadata(course.location, own_metadata(course)) + store.update_item(course, request.user.id) resp = JsonResponse(textbook, status=201) resp["Location"] = locator.url_reverse('textbooks', textbook["id"]) return resp @@ -815,10 +811,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non course.pdf_textbooks = new_textbooks else: course.pdf_textbooks.append(new_textbook) - store.update_metadata( - course.location, - own_metadata(course) - ) + store.update_item(course, request.user.id) return JsonResponse(new_textbook, status=201) elif request.method == 'DELETE': if not textbook: @@ -827,10 +820,7 @@ def textbooks_detail_handler(request, tid, tag=None, package_id=None, branch=Non new_textbooks = course.pdf_textbooks[0:i] new_textbooks.extend(course.pdf_textbooks[i + 1:]) course.pdf_textbooks = new_textbooks - store.update_metadata( - course.location, - own_metadata(course) - ) + store.update_item(course, request.user.id) return JsonResponse() diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 8d80d9e0bba5..372ba7596943 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -9,7 +9,6 @@ from static_replace import replace_static_urls from xmodule_modifiers import wrap_xblock -from django.conf import settings from django.core.exceptions import PermissionDenied from django.contrib.auth.decorators import login_required from django.http import HttpResponseBadRequest, HttpResponse @@ -18,9 +17,8 @@ from xblock.fields import Scope from xblock.fragment import Fragment -from xblock.core import XBlock -import xmodule.x_module +import xmodule from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from xmodule.modulestore.inheritance import own_metadata @@ -159,7 +157,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid delete_children = str_to_bool(request.REQUEST.get('recurse', 'False')) delete_all_versions = str_to_bool(request.REQUEST.get('all_versions', 'False')) - return _delete_item_at_location(old_location, delete_children, delete_all_versions) + return _delete_item_at_location(old_location, delete_children, delete_all_versions, request.user) else: # Since we have a package_id, we are updating an existing xblock. return _save_item( request, @@ -184,7 +182,8 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid dest_location = _duplicate_item( parent_location, duplicate_source_location, - request.json.get('display_name') + request.json.get('display_name'), + request.user, ) course_location = loc_mapper().translate_locator_to_location(BlockUsageLocator(parent_locator), get_course=True) dest_locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True) @@ -232,7 +231,8 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta modulestore().convert_to_draft(item_location) if data: - store.update_item(item_location, data) + # TODO Allow any scope.content fields not just "data" (exactly like the get below this) + existing_item.data = data else: data = existing_item.get_explicitly_set_fields_by_scope(Scope.content) @@ -242,9 +242,9 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta for child_locator in children ] - store.update_children(item_location, children_ids) + existing_item.children = children_ids - # cdodge: also commit any metadata which might have been passed along + # also commit any metadata which might have been passed along if nullout is not None or metadata is not None: # the postback is not the complete metadata, as there's system metadata which is # not presented to the end-user for editing. So let's use the original (existing_item) and @@ -269,14 +269,11 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta return JsonResponse({"error": "Invalid data"}, 400) field.write_to(existing_item, value) - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - existing_item.save() - # commit to datastore - store.update_metadata(item_location, own_metadata(existing_item)) - if existing_item.category == 'video': - manage_video_subtitles_save(existing_item, existing_item) + manage_video_subtitles_save(existing_item, existing_item, request.user) + + # commit to datastore + store.update_item(existing_item, request.user.id) result = { 'id': unicode(usage_loc), @@ -285,7 +282,7 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta } if grader_type is not None: - result.update(CourseGradingModel.update_section_grader_type(existing_item, grader_type)) + result.update(CourseGradingModel.update_section_grader_type(existing_item, grader_type, request.user)) # Make public after updating the xblock, in case the caller asked # for both an update and a publish. @@ -339,14 +336,15 @@ def _create_item(request): # TODO replace w/ nicer accessor if not 'detached' in parent.runtime.load_block_type(category)._class_tags: - get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()]) + parent.children.append(dest_location.url()) + get_modulestore(parent.location).update_item(parent, request.user.id) course_location = loc_mapper().translate_locator_to_location(parent_locator, get_course=True) locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True) return JsonResponse({"locator": unicode(locator)}) -def _duplicate_item(parent_location, duplicate_source_location, display_name=None): +def _duplicate_item(parent_location, duplicate_source_location, display_name=None, user=None): """ Duplicate an existing xblock as a child of the supplied parent_location. """ @@ -373,13 +371,15 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non system=source_item.runtime, ) + dest_module = get_modulestore(category).get_item(dest_location) # Children are not automatically copied over (and not all xblocks have a 'children' attribute). # Because DAGs are not fully supported, we need to actually duplicate each child as well. if source_item.has_children: - copied_children = [] + dest_module.children = [] for child in source_item.children: - copied_children.append(_duplicate_item(dest_location, Location(child)).url()) - get_modulestore(dest_location).update_children(dest_location, copied_children) + dupe = _duplicate_item(dest_location, Location(child), user=user) + dest_module.children.append(dupe.url()) + get_modulestore(dest_location).update_item(dest_module, user.id if user else None) if not 'detached' in source_item.runtime.load_block_type(category)._class_tags: parent = get_modulestore(parent_location).get_item(parent_location) @@ -390,12 +390,12 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non parent.children.insert(source_index + 1, dest_location.url()) else: parent.children.append(dest_location.url()) - get_modulestore(parent_location).update_children(parent_location, parent.children) + get_modulestore(parent_location).update_item(parent, user.id if user else None) return dest_location -def _delete_item_at_location(item_location, delete_children=False, delete_all_versions=False): +def _delete_item_at_location(item_location, delete_children=False, delete_all_versions=False, user=None): """ Deletes the item at with the given Location. @@ -406,22 +406,19 @@ def _delete_item_at_location(item_location, delete_children=False, delete_all_ve item = store.get_item(item_location) if delete_children: - _xmodule_recurse(item, lambda i: store.delete_item(i.location, delete_all_versions)) + _xmodule_recurse(item, lambda i: store.delete_item(i.location, delete_all_versions=delete_all_versions)) else: - store.delete_item(item.location, delete_all_versions) + store.delete_item(item.location, delete_all_versions=delete_all_versions) # cdodge: we need to remove our parent's pointer to us so that it is no longer dangling if delete_all_versions: parent_locs = modulestore('direct').get_parent_locations(item_location, None) + item_url = item_location.url() for parent_loc in parent_locs: parent = modulestore('direct').get_item(parent_loc) - item_url = item_location.url() - if item_url in parent.children: - children = parent.children - children.remove(item_url) - parent.children = children - modulestore('direct').update_children(parent.location, parent.children) + parent.children.remove(item_url) + modulestore('direct').update_item(parent, user.id if user else None) return JsonResponse() @@ -452,7 +449,7 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid if request.user.is_staff: items = modulestore().get_orphans(old_location, 'draft') for item in items: - modulestore('draft').delete_item(item, True) + modulestore('draft').delete_item(item, delete_all_versions=True) return JsonResponse({'deleted': items}) else: raise PermissionDenied() diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 4a34886ebc9f..aa479e2e351d 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -10,8 +10,6 @@ from django_future.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods from edxmako.shortcuts import render_to_response -from xmodule.modulestore import Location -from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.locator import BlockUsageLocator @@ -23,7 +21,7 @@ __all__ = ['tabs_handler'] -def initialize_course_tabs(course): +def initialize_course_tabs(course, user): """ set up the default tabs I've added this because when we add static tabs, the LMS either expects a None for the tabs list or @@ -47,7 +45,7 @@ def initialize_course_tabs(course): {"type": "progress", "name": _("Progress")}, ] - modulestore('direct').update_metadata(course.location.url(), own_metadata(course)) + modulestore('direct').update_item(course, user.id) @expect_json @login_required @@ -123,14 +121,14 @@ def get_location_for_tab(tab): # OK, re-assemble the static tabs in the new order course_item.tabs = reordered_tabs - modulestore('direct').update_metadata(course_item.location, own_metadata(course_item)) + modulestore('direct').update_item(course_item, request.user.id) return JsonResponse() else: raise NotImplementedError('Creating or changing tab content is not supported.') elif request.method == 'GET': # assume html # see tabs have been uninitialized (e.g. supporting courses created before tab support in studio) if course_item.tabs is None or len(course_item.tabs) == 0: - initialize_course_tabs(course_item) + initialize_course_tabs(course_item, request.user) # first get all static tabs from the tabs list # we do this because this is also the order in which items are displayed in the LMS @@ -179,7 +177,7 @@ def primitive_delete(course, num): # Note for future implementations: if you delete a static_tab, then Chris Dodge # points out that there's other stuff to delete beyond this element. # This code happens to not delete static_tab so it doesn't come up. - modulestore('direct').update_metadata(course.location, own_metadata(course)) + modulestore('direct').update_item(course, '**replace_user**') def primitive_insert(course, num, tab_type, name): @@ -188,5 +186,5 @@ def primitive_insert(course, num, tab_type, name): new_tab = {u'type': unicode(tab_type), u'name': unicode(name)} tabs = course.tabs tabs.insert(num, new_tab) - modulestore('direct').update_metadata(course.location, own_metadata(course)) + modulestore('direct').update_item(course, '**replace_user**') diff --git a/cms/djangoapps/contentstore/tests/test_access.py b/cms/djangoapps/contentstore/views/tests/test_access.py similarity index 99% rename from cms/djangoapps/contentstore/tests/test_access.py rename to cms/djangoapps/contentstore/views/tests/test_access.py index 1fbdbdf84851..d0918195f843 100644 --- a/cms/djangoapps/contentstore/tests/test_access.py +++ b/cms/djangoapps/contentstore/views/tests/test_access.py @@ -11,6 +11,7 @@ from student.auth import add_users from contentstore.views.access import get_user_role + class RolesTest(TestCase): """ Tests for user roles. diff --git a/cms/djangoapps/contentstore/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py similarity index 99% rename from cms/djangoapps/contentstore/tests/test_assets.py rename to cms/djangoapps/contentstore/views/tests/test_assets.py index f5a204504d58..b4af6f9fb46b 100644 --- a/cms/djangoapps/contentstore/tests/test_assets.py +++ b/cms/djangoapps/contentstore/views/tests/test_assets.py @@ -10,7 +10,7 @@ from io import BytesIO from pytz import UTC import json -from .utils import CourseTestCase +from contentstore.tests.utils import CourseTestCase from contentstore.views import assets from xmodule.contentstore.content import StaticContent from xmodule.modulestore import Location @@ -114,6 +114,7 @@ def assert_correct_sort_response(self, url, sort, direction): self.assertGreaterEqual(name1, name2) self.assertGreaterEqual(name2, name3) + class UploadTestCase(AssetsTestCase): """ Unit tests for uploading a file diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/views/tests/test_checklists.py similarity index 97% rename from cms/djangoapps/contentstore/tests/test_checklists.py rename to cms/djangoapps/contentstore/views/tests/test_checklists.py index 864fe23c8333..aa480a525ced 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/views/tests/test_checklists.py @@ -1,12 +1,11 @@ """ Unit tests for checklist methods in views.py. """ from contentstore.utils import get_modulestore from contentstore.views.checklist import expand_checklist_action_url -from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.django import loc_mapper import json -from .utils import CourseTestCase +from contentstore.tests.utils import CourseTestCase class ChecklistTestCase(CourseTestCase): @@ -54,7 +53,7 @@ def test_get_checklists(self): # Save the changed `checklists` to the underlying KeyValueStore before updating the modulestore self.course.save() modulestore = get_modulestore(self.course.location) - modulestore.update_metadata(self.course.location, own_metadata(self.course)) + modulestore.update_item(self.course, self.user.id) self.assertEqual(self.get_persisted_checklists(), None) response = self.client.get(self.checklists_url) self.assertEqual(payload, response.content) @@ -113,7 +112,6 @@ def test_update_checklists_index(self): self.assertEqual('CourseOutline', get_first_item(persisted_checklist).get('action_url')) self.compare_checklists(persisted_checklist, returned_checklist) - def test_update_checklists_delete_unsupported(self): """ Delete operation is not supported. """ update_url = self.location.url_reverse('checklists/', '100') diff --git a/cms/djangoapps/contentstore/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py similarity index 100% rename from cms/djangoapps/contentstore/tests/test_course_index.py rename to cms/djangoapps/contentstore/views/tests/test_course_index.py index 496b9b70569f..4ca23ee43b64 100644 --- a/cms/djangoapps/contentstore/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -9,6 +9,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore import parsers + class TestCourseIndex(CourseTestCase): """ Unit tests for getting the list of courses and the course outline. @@ -25,7 +26,6 @@ def setUp(self): display_name='dotted.course.name-2', ) - def check_index_and_outline(self, authed_client): """ Test getting the list of courses and then pulling up their outlines diff --git a/cms/djangoapps/contentstore/tests/test_course_updates.py b/cms/djangoapps/contentstore/views/tests/test_course_updates.py similarity index 98% rename from cms/djangoapps/contentstore/tests/test_course_updates.py rename to cms/djangoapps/contentstore/views/tests/test_course_updates.py index 5ee5f1289bb7..bc3075668ac8 100644 --- a/cms/djangoapps/contentstore/tests/test_course_updates.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_updates.py @@ -53,7 +53,6 @@ def get_response(content, date): content, json.loads(refetched.content)['content'], "get w/ provided id" ) - # now put in an evil update content = '
    ' payload = get_response(content, 'January 11, 2013') @@ -123,7 +122,7 @@ def test_no_ol_course_update(self): modulestore('direct').create_and_save_xmodule(location) course_updates = modulestore('direct').get_item(location) course_updates.data = 'bad news' - modulestore('direct').update_item(location, course_updates.data) + modulestore('direct').update_item(course_updates, self.user.id) init_content = '' diff --git a/cms/djangoapps/contentstore/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py similarity index 98% rename from cms/djangoapps/contentstore/tests/test_import_export.py rename to cms/djangoapps/contentstore/views/tests/test_import_export.py index 85df894cd414..e01e6ba565a1 100644 --- a/cms/djangoapps/contentstore/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -12,7 +12,7 @@ from uuid import uuid4 from pymongo import MongoClient -from .utils import CourseTestCase +from contentstore.tests.utils import CourseTestCase from django.test.utils import override_settings from django.conf import settings from xmodule.modulestore.django import loc_mapper @@ -25,6 +25,7 @@ log = logging.getLogger(__name__) + @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class ImportTestCase(CourseTestCase): """ @@ -178,7 +179,7 @@ def test_unsafe_tar(self): def try_tar(tarpath): with open(tarpath) as tar: - args = { "name": tarpath, "course-data": [tar] } + args = {"name": tarpath, "course-data": [tar]} resp = self.client.post(self.url, args) self.assertEquals(resp.status_code, 400) self.assertTrue("SuspiciousFileOperation" in resp.content) diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py similarity index 100% rename from cms/djangoapps/contentstore/tests/test_item.py rename to cms/djangoapps/contentstore/views/tests/test_item.py diff --git a/cms/djangoapps/contentstore/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py similarity index 100% rename from cms/djangoapps/contentstore/tests/test_tabs.py rename to cms/djangoapps/contentstore/views/tests/test_tabs.py diff --git a/cms/djangoapps/contentstore/tests/test_textbooks.py b/cms/djangoapps/contentstore/views/tests/test_textbooks.py similarity index 96% rename from cms/djangoapps/contentstore/tests/test_textbooks.py rename to cms/djangoapps/contentstore/views/tests/test_textbooks.py index d1fe60635440..7312d0c70ac8 100644 --- a/cms/djangoapps/contentstore/tests/test_textbooks.py +++ b/cms/djangoapps/contentstore/views/tests/test_textbooks.py @@ -1,9 +1,7 @@ import json from unittest import TestCase -from .utils import CourseTestCase -from django.core.urlresolvers import reverse +from contentstore.tests.utils import CourseTestCase from contentstore.utils import get_modulestore -from xmodule.modulestore.inheritance import own_metadata from contentstore.views.course import ( validate_textbooks_json, validate_textbook_json, TextbookValidationError) @@ -58,11 +56,8 @@ def test_view_index_xhr_content(self): } ] self.course.pdf_textbooks = content - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - self.course.save() store = get_modulestore(self.course.location) - store.update_metadata(self.course.location, own_metadata(self.course)) + store.update_item(self.course, self.user.id) resp = self.client.get( self.url, @@ -200,7 +195,7 @@ def setUp(self): # MongoKeyValueStore before we update the mongo datastore. self.course.save() self.store = get_modulestore(self.course.location) - self.store.update_metadata(self.course.location, own_metadata(self.course)) + self.store.update_item(self.course, self.user.id) self.url_nonexist = self.course_locator.url_reverse("textbooks", "20") def test_get_1(self): diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py similarity index 91% rename from cms/djangoapps/contentstore/tests/test_transcripts.py rename to cms/djangoapps/contentstore/views/tests/test_transcripts.py index f4c7f773adf0..f92e8e7f216f 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -63,13 +63,13 @@ def setUp(self): self.item_locator, self.item_location = self._get_locator(resp) self.assertEqual(resp.status_code, 200) + self.item = modulestore().get_item(self.item_location) # hI10vDNYz4M - valid Youtube ID with transcripts. # JMD_ifUUfsU, AKqURZnYqpk, DYpADpL7jAY - valid Youtube IDs without transcripts. - data = '