From f5abef88c2f13114c72f505cc2aaeb5ea54041aa Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 7 Jul 2014 10:44:55 -0400 Subject: [PATCH] LMS-2947 remove get_error_courses in Split. LMS-2950 move delete_course to Mongo. --- .../management/commands/clone_course.py | 3 +- .../management/commands/delete_course.py | 8 ++-- .../management/commands/import.py | 4 +- .../contentstore/tests/test_contentstore.py | 15 +++---- .../contentstore/tests/test_course_listing.py | 6 +-- .../contentstore/tests/test_crud.py | 2 +- .../tests/test_users_default_role.py | 6 +-- cms/djangoapps/contentstore/utils.py | 26 +++++------ cms/djangoapps/contentstore/views/tabs.py | 5 ++- .../student/tests/test_course_listing.py | 2 +- .../xmodule/xmodule/modulestore/__init__.py | 44 ++++++++++++++++++- .../lib/xmodule/xmodule/modulestore/mixed.py | 19 ++------ .../xmodule/xmodule/modulestore/mongo/base.py | 9 ---- .../xmodule/modulestore/mongo/draft.py | 12 +++++ .../xmodule/modulestore/split_mongo/split.py | 13 +++--- .../xmodule/modulestore/store_utilities.py | 22 ---------- .../commands/tests/test_git_add_course.py | 7 +-- lms/djangoapps/dashboard/sysadmin.py | 6 +-- 18 files changed, 103 insertions(+), 106 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/clone_course.py b/cms/djangoapps/contentstore/management/commands/clone_course.py index 8f87e04336..a18924418e 100644 --- a/cms/djangoapps/contentstore/management/commands/clone_course.py +++ b/cms/djangoapps/contentstore/management/commands/clone_course.py @@ -7,6 +7,7 @@ from student.roles import CourseInstructorRole, CourseStaffRole from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import SlashSeparatedCourseKey +from xmodule.modulestore import ModuleStoreEnum # @@ -38,7 +39,7 @@ class Command(BaseCommand): print("Cloning course {0} to {1}".format(source_course_id, dest_course_id)) with mstore.bulk_write_operations(dest_course_id): - if mstore.clone_course(source_course_id, dest_course_id, None): + if mstore.clone_course(source_course_id, dest_course_id, ModuleStoreEnum.UserID.mgmt_command): print("copying User permissions...") # purposely avoids auth.add_user b/c it doesn't have a caller to authorize CourseInstructorRole(dest_course_id).add_users( diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 89e88b9f9d..6f77c8a048 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -7,7 +7,7 @@ from contentstore.utils import delete_course_and_groups from opaque_keys.edx.keys import CourseKey from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import SlashSeparatedCourseKey - +from xmodule.modulestore import ModuleStoreEnum class Command(BaseCommand): help = '''Delete a MongoDB backed course''' @@ -28,6 +28,6 @@ class Command(BaseCommand): if commit: print('Actually going to delete the course from DB....') - if query_yes_no("Deleting course {0}. Confirm?".format(course_key), default="no"): - if query_yes_no("Are you sure. This action cannot be undone!", default="no"): - delete_course_and_groups(course_key, commit) + if query_yes_no("Deleting course {0}. Confirm?".format(course_key), default="no"): + if query_yes_no("Are you sure. This action cannot be undone!", default="no"): + delete_course_and_groups(course_key, ModuleStoreEnum.UserID.mgmt_command) diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index e0e1ed0c5d..a460aae3f7 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -8,6 +8,7 @@ from django_comment_common.utils import (seed_permissions_roles, from xmodule.modulestore.xml_importer import import_from_xml from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore +from xmodule.modulestore import ModuleStoreEnum class Command(BaseCommand): @@ -37,10 +38,9 @@ class Command(BaseCommand): data=data_dir, courses=course_dirs, dis=do_import_static)) - mstore = modulestore() _, course_items = import_from_xml( - mstore, "**replace_user**", data_dir, course_dirs, load_error_modules=False, + modulestore(), ModuleStoreEnum.UserID.mgmt_command, data_dir, course_dirs, load_error_modules=False, static_content_store=contentstore(), verbose=True, do_import_static=do_import_static, create_new_course_if_not_present=True, diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 260702b52a..5f396467ca 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -30,7 +30,6 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation -from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint @@ -632,7 +631,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.store.convert_to_draft(vertical.location, self.user.id) # delete the course - delete_course(self.store, content_store, course_id, commit=True) + self.store.delete_course(course_id, self.user.id) # assert that there's absolutely no non-draft modules in the course # this should also include all draft items @@ -697,7 +696,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): self.assertEqual(on_disk['course/2012_Fall'], own_metadata(course)) # remove old course - delete_course(self.store, content_store, course_id, commit=True) + self.store.delete_course(course_id, self.user.id) # reimport over old course self.check_import(root_dir, content_store, course_id) @@ -909,7 +908,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): # Delete the course from module store and reimport it - delete_course(self.store, content_store, course_id, commit=True) + self.store.delete_course(course_id, self.user.id) import_from_xml( self.store, self.user.id, root_dir, ['test_export_no_content_store'], @@ -997,7 +996,7 @@ class ContentStoreTest(ContentStoreTestCase): test_course_data = self.assert_created_course(number_suffix=uuid4().hex) course_id = _get_course_id(test_course_data) self.assertTrue(are_permissions_roles_seeded(course_id)) - delete_course_and_groups(course_id, commit=True) + delete_course_and_groups(course_id, self.user.id) # should raise an exception for checking permissions on deleted course with self.assertRaises(ItemNotFoundError): are_permissions_roles_seeded(course_id) @@ -1009,7 +1008,7 @@ class ContentStoreTest(ContentStoreTestCase): # unseed the forums for the first course course_id = _get_course_id(test_course_data) - delete_course_and_groups(course_id, commit=True) + delete_course_and_groups(course_id, self.user.id) # should raise an exception for checking permissions on deleted course with self.assertRaises(ItemNotFoundError): are_permissions_roles_seeded(course_id) @@ -1029,7 +1028,7 @@ class ContentStoreTest(ContentStoreTestCase): self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id)) self.assertTrue(self.user.roles.filter(name="Student", course_id=course_id)) # pylint: disable=no-member - delete_course_and_groups(course_id, commit=True) + delete_course_and_groups(course_id, self.user.id) # check that user's enrollment for this course is not deleted self.assertTrue(CourseEnrollment.is_enrolled(self.user, course_id)) # check that user has form role "Student" for this course even after deleting it @@ -1051,7 +1050,7 @@ class ContentStoreTest(ContentStoreTestCase): self.assertTrue(len(instructor_role.users_with_role()) > 0) # Now delete course and check that user not in instructor groups of this course - delete_course_and_groups(course_id, commit=True) + delete_course_and_groups(course_id, self.user.id) # Update our cached user since its roles have changed self.user = User.objects.get_by_natural_key(self.user.natural_key()[0]) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 7d8c726db9..94642b6591 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -145,7 +145,7 @@ class TestCourseListing(ModuleStoreTestCase): self.assertEqual(courses_list, courses_list_by_groups) # now delete this course and re-add user to instructor group of this course - delete_course_and_groups(course_key, commit=True) + delete_course_and_groups(course_key, self.user.id) CourseInstructorRole(course_key).add_users(self.user) @@ -237,7 +237,7 @@ class TestCourseListing(ModuleStoreTestCase): self.assertEqual(len(courses_list_by_groups), 2) # now delete first course (course_location_caps) and check that it is no longer accessible - delete_course_and_groups(course_location_caps, commit=True) + delete_course_and_groups(course_location_caps, self.user.id) # test that get courses through iterating all courses now returns one course courses_list = _accessible_courses_list(self.request) @@ -269,7 +269,7 @@ class TestCourseListing(ModuleStoreTestCase): course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun') self._create_course_with_access_groups(course_location, self.user) - store.delete_course(course_location) + store.delete_course(course_location, self.user.id) course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun') course = self._create_course_with_access_groups(course_location, self.user) diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index 0accd31b8e..60e850af28 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -162,7 +162,7 @@ class TemplateTests(unittest.TestCase): self.assertIsInstance(self.split_store.get_course(id_locator), CourseDescriptor) # and by guid self.assertIsInstance(self.split_store.get_item(guid_locator), CourseDescriptor) - self.split_store.delete_course(id_locator) + self.split_store.delete_course(id_locator, ModuleStoreEnum.UserID.test) # test can no longer retrieve by id self.assertRaises(ItemNotFoundError, self.split_store.get_course, id_locator) # but can by guid diff --git a/cms/djangoapps/contentstore/tests/test_users_default_role.py b/cms/djangoapps/contentstore/tests/test_users_default_role.py index 26ec08ab94..c02b6574f1 100644 --- a/cms/djangoapps/contentstore/tests/test_users_default_role.py +++ b/cms/djangoapps/contentstore/tests/test_users_default_role.py @@ -62,7 +62,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase): # check that user has his default "Student" forum role for this course self.assertTrue(self.user.roles.filter(name="Student", course_id=self.course_key)) # pylint: disable=no-member - delete_course_and_groups(self.course_key, commit=True) + delete_course_and_groups(self.course_key, self.user.id) # check that user's enrollment for this course is not deleted self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key)) @@ -80,7 +80,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase): self.assertTrue(self.user.roles.filter(name="Student", course_id=self.course_key)) # pylint: disable=no-member # delete this course and recreate this course with same user - delete_course_and_groups(self.course_key, commit=True) + delete_course_and_groups(self.course_key, self.user.id) resp = self._create_course_with_given_location(self.course_key) self.assertEqual(resp.status_code, 200) @@ -98,7 +98,7 @@ class TestUsersDefaultRole(ModuleStoreTestCase): # check that user has enrollment and his default "Student" forum role for this course self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course_key)) # delete this course and recreate this course with same user - delete_course_and_groups(self.course_key, commit=True) + delete_course_and_groups(self.course_key, self.user.id) # now create same course with different name case ('uppercase') new_course_key = self.course_key.replace(course=self.course_key.course.upper()) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 7bd69b668d..c61753efa3 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -11,12 +11,10 @@ from django.utils.translation import ugettext as _ from django.core.urlresolvers import reverse from xmodule.contentstore.content import StaticContent -from xmodule.contentstore.django import contentstore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location -from xmodule.modulestore.store_utilities import delete_course from student.roles import CourseInstructorRole, CourseStaffRole @@ -28,27 +26,25 @@ NOTES_PANEL = {"name": _("My Notes"), "type": "notes"} EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]]) -def delete_course_and_groups(course_id, commit=False): +def delete_course_and_groups(course_id, user_id): """ This deletes the courseware associated with a course_id as well as cleaning update_item the various user table stuff (groups, permissions, etc.) """ module_store = modulestore() - content_store = contentstore() with module_store.bulk_write_operations(course_id): - if delete_course(module_store, content_store, course_id, commit): + module_store.delete_course(course_id, user_id) - print 'removing User permissions from course....' - # in the django layer, we need to remove all the user permissions groups associated with this course - if commit: - try: - staff_role = CourseStaffRole(course_id) - staff_role.remove_users(*staff_role.users_with_role()) - instructor_role = CourseInstructorRole(course_id) - instructor_role.remove_users(*instructor_role.users_with_role()) - except Exception as err: - log.error("Error in deleting course groups for {0}: {1}".format(course_id, err)) + print 'removing User permissions from course....' + # in the django layer, we need to remove all the user permissions groups associated with this course + try: + staff_role = CourseStaffRole(course_id) + staff_role.remove_users(*staff_role.users_with_role()) + instructor_role = CourseInstructorRole(course_id) + instructor_role.remove_users(*instructor_role.users_with_role()) + except Exception as err: + log.error("Error in deleting course groups for {0}: {1}".format(course_id, err)) def get_lms_link_for_item(location, preview=False): diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 6fc0dbe43b..e251e3dca8 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -12,6 +12,7 @@ from django_future.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods from edxmako.shortcuts import render_to_response from xmodule.modulestore.django import modulestore +from xmodule.modulestore import ModuleStoreEnum from xmodule.tabs import CourseTabList, StaticTab, CourseTab, InvalidTabsException from opaque_keys.edx.keys import CourseKey, UsageKey @@ -192,7 +193,7 @@ def primitive_delete(course, num): # Note for future implementations: if you delete a static_tab, then Chris Dodge # points out that there's other stuff to delete beyond this element. # This code happens to not delete static_tab so it doesn't come up. - modulestore().update_item(course, '**replace_user**') + modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) def primitive_insert(course, num, tab_type, name): @@ -201,5 +202,5 @@ def primitive_insert(course, num, tab_type, name): new_tab = CourseTab.from_json({u'type': unicode(tab_type), u'name': unicode(name)}) tabs = course.tabs tabs.insert(num, new_tab) - modulestore().update_item(course, '**replace_user**') + modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 4625660913..970a6110d4 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -97,7 +97,7 @@ class TestCourseListing(ModuleStoreTestCase): course_location = SlashSeparatedCourseKey('testOrg', 'doomedCourse', 'RunBabyRun') self._create_course_with_access_groups(course_location) - mongo_store.delete_course(course_location) + mongo_store.delete_course(course_location, ModuleStoreEnum.UserID.test) course_location = SlashSeparatedCourseKey('testOrg', 'erroredCourse', 'RunBabyRun') course = self._create_course_with_access_groups(course_location) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 8470e598e1..bf975dde45 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -72,6 +72,21 @@ class ModuleStoreEnum(object): draft = 'draft-branch' published = 'published-branch' + class UserID(object): + """ + Values for user ID defaults + """ + # Note: we use negative values here to (try to) not conflict + # with user identifiers provided by an actual user service. + + # user ID to use for all management commands + mgmt_command = -1 + + # user ID to use for primitive commands + primitive_command = -2 + + # user ID to use for tests that do not have a django user available + test = -3 class PublishState(object): """ @@ -270,6 +285,18 @@ class ModuleStoreRead(object): """ pass + @abstractmethod + def compute_publish_state(self, xblock): + """ + Returns whether this xblock is draft, public, or private. + + Returns: + PublishState.draft - content is in the process of being edited, but still has a previous + version deployed to LMS + PublishState.public - content is locked and deployed to LMS + PublishState.private - content is editable and not deployed to LMS + """ + pass class ModuleStoreWrite(ModuleStoreRead): """ @@ -348,7 +375,7 @@ class ModuleStoreWrite(ModuleStoreRead): pass @abstractmethod - def delete_course(self, course_key, user_id=None): + def delete_course(self, course_key, user_id): """ Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions depending on the persistence layer and how tightly bound the xblocks are to the course. @@ -441,6 +468,12 @@ class ModuleStoreReadBase(ModuleStoreRead): None ) + def compute_publish_state(self, xblock): + """ + Returns PublishState.public since this is a read-only store. + """ + return PublishState.public + def heartbeat(self): """ Is this modulestore ready? @@ -553,6 +586,15 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id) return dest_course_id + def delete_course(self, course_key, user_id): + """ + This base method just deletes the assets. The lower level impls must do the actual deleting of + content. + """ + # delete the assets + self.contentstore.delete_all_course_assets(course_key) + super(ModuleStoreWriteBase, self).delete_course(course_key, user_id) + @contextmanager def bulk_write_operations(self, course_id): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 5fa139b44b..b3622effc4 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -229,16 +229,13 @@ class MixedModuleStore(ModuleStoreWriteBase): store = self._get_modulestore_for_courseid(course_id) return store.has_course(course_id, ignore_case) - def delete_course(self, course_key, user_id=None): + def delete_course(self, course_key, user_id): """ See xmodule.modulestore.__init__.ModuleStoreWrite.delete_course """ assert(isinstance(course_key, CourseKey)) store = self._get_modulestore_for_courseid(course_key) - if hasattr(store, 'delete_course'): - return store.delete_course(course_key, user_id) - else: - raise NotImplementedError(u"Cannot delete a course on store {}".format(store)) + return store.delete_course(course_key, user_id) def get_parent_location(self, location, **kwargs): """ @@ -290,10 +287,6 @@ class MixedModuleStore(ModuleStoreWriteBase): Returns: a CourseDescriptor """ store = self._get_modulestore_for_courseid(None) - - if not hasattr(store, 'create_course'): - raise NotImplementedError(u"Cannot create a course on store {}".format(store)) - return store.create_course(org, offering, user_id, fields, **kwargs) def clone_course(self, source_course_id, dest_course_id, user_id): @@ -456,13 +449,7 @@ class MixedModuleStore(ModuleStoreWriteBase): """ course_id = xblock.scope_ids.usage_id.course_key store = self._get_modulestore_for_courseid(course_id) - if hasattr(store, 'compute_publish_state'): - return store.compute_publish_state(xblock) - elif hasattr(store, 'publish'): - raise NotImplementedError(u"Cannot compute_publish_state on store {}".format(store)) - else: - # read-only store; so, everything's public - return PublishState.public + return store.compute_publish_state(xblock) def publish(self, location, user_id): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 2f71e83635..74374b241d 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -892,15 +892,6 @@ class MongoModuleStore(ModuleStoreWriteBase): return course - def delete_course(self, course_key, user_id=None): - """ - The impl removes all of the db records for the course. - :param course_key: - :param user_id: - """ - course_query = self._course_key_to_son(course_key) - self.collection.remove(course_query, multi=True) - def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}): """ Create the new xmodule but don't save it. Returns the new module. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index ca13204f01..1621f0370d 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -144,6 +144,18 @@ class DraftModuleStore(MongoModuleStore): del key['_id.revision'] return self.collection.find(key).count() > 0 + def delete_course(self, course_key, user_id): + """ + :param course_key: which course to delete + :param user_id: id of the user deleting the course + """ + # delete the assets + super(DraftModuleStore, self).delete_course(course_key, user_id) + + # delete all of the db records for the course + course_query = self._course_key_to_son(course_key) + self.collection.remove(course_query, multi=True) + def clone_course(self, source_course_id, dest_course_id, user_id): """ Only called if cloning within this store or if env doesn't set up mixed. diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index c70a524d3c..5cbf3633ac 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1380,7 +1380,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): return result - def delete_course(self, course_key, user_id=None): + def delete_course(self, course_key, user_id): """ Remove the given course from the course index. @@ -1395,6 +1395,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): log.info(u"deleting course from split-mongo: %s", course_key) self.db_connection.delete_course_index(index) + # We do NOT call the super class here since we need to keep the assets + # in case the course is later restored. + # super(SplitMongoModuleStore, self).delete_course(course_key, user_id) + def revert_to_published(self, location, user_id=None): """ Reverts an item to its last published version (recursively traversing all of its descendants). @@ -1407,13 +1411,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): """ raise NotImplementedError() - def get_errored_courses(self): - """ - This function doesn't make sense for the mongo modulestore, as structures - are loaded on demand, rather than up front - """ - return {} - def inherit_settings(self, block_map, block_json, inheriting_settings=None): """ Updates block_json with any inheritable setting set by an ancestor and recurses to children. diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index d1e40ade9c..cf69b605b8 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -85,25 +85,3 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text): logging.warning("Error producing regex substitution %r for text = %r.\n\nError msg = %s", source_course_id, text, str(exc)) return text - - -def delete_course(modulestore, contentstore, course_key, commit=False): - """ - This method will actually do the work to delete all content in a course in a MongoDB backed - courseware store. BE VERY CAREFUL, this is not reversable. - """ - - # check to see if the source course is actually there - if not modulestore.has_course(course_key): - raise Exception("Cannot find a course at {0}. Aborting".format(course_key)) - - if commit: - print "Deleting assets and thumbnails {}".format(course_key) - contentstore.delete_all_course_assets(course_key) - - # finally delete the course - print "Deleting {0}...".format(course_key) - if commit: - modulestore.delete_course(course_key, '**replace-user**') - - return True diff --git a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py index f3fb82b5da..a64a9c89cb 100644 --- a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py +++ b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py @@ -15,10 +15,9 @@ from django.core.management.base import CommandError from django.test.utils import override_settings from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE -from xmodule.contentstore.django import contentstore +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from opaque_keys.edx.locations import SlashSeparatedCourseKey -from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase import dashboard.git_import as git_import from dashboard.git_import import GitImportError @@ -161,9 +160,7 @@ class TestGitAddCourse(ModuleStoreTestCase): self.TEST_BRANCH) # Delete to test branching back to master - delete_course(def_ms, contentstore(), - self.TEST_BRANCH_COURSE, - True) + def_ms.delete_course(self.TEST_BRANCH_COURSE, ModuleStoreEnum.UserID.test) self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 79f790ac04..dfb1879108 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -38,10 +38,8 @@ from external_auth.models import ExternalAuthMap from external_auth.views import generate_password from student.models import CourseEnrollment, UserProfile, Registration import track.views -from xmodule.contentstore.django import contentstore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.xml import XMLModuleStore from opaque_keys.edx.locations import SlashSeparatedCourseKey @@ -591,9 +589,7 @@ class Courses(SysadminDashboardView): elif course_found and not is_xml_course: # delete course that is stored with mongodb backend - content_store = contentstore() - commit = True - delete_course(self.def_ms, content_store, course.id, commit) + self.def_ms.delete_course(course.id, request.user.id) # don't delete user permission groups, though self.msg += \ u"{0} {1} = {2} ({3})".format(