From bcc226821691e34e1bd0962e790ce9b53080e501 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 3 Jul 2013 11:23:44 -0400 Subject: [PATCH 1/4] add code to actually go through and delete the draft modules --- .../xmodule/modulestore/store_utilities.py | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index 6beffcb71d..89dca0c325 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -102,7 +102,27 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele return True +def _delete_modules_except_course(modulestore, modules, source_location, commit): + """ + This helper method will just enumerate through a list of modules and delete them, except for the + top-level course module + """ + for module in modules: + if module.category != 'course': + print "Deleting {0}...".format(module.location) + if commit: + # sanity check. Make sure we're not deleting a module in the incorrect course + if module.location.org != source_location.org or module.location.course != source_location.course: + raise Exception('Module {0} is not in same namespace as {1}. This should not happen! Aborting...'.format(module.location, source_location)) + modulestore.delete_item(module.location) + + def delete_course(modulestore, contentstore, source_location, 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. + """ + # first check to see if the modulestore is Mongo backed if not isinstance(modulestore, MongoModuleStore): raise Exception("Expected a MongoModuleStore in the runtime. Aborting....") @@ -131,12 +151,11 @@ def delete_course(modulestore, contentstore, source_location, commit=False): # then delete all course modules modules = modulestore.get_items([source_location.tag, source_location.org, source_location.course, None, None, None]) + _delete_modules_except_course(modulestore, modules, source_location, commit) - for module in modules: - if module.category != 'course': # save deleting the course module for last - print "Deleting {0}...".format(module.location) - if commit: - modulestore.delete_item(module.location) + # then delete all draft course modules + modules = modulestore.get_items([source_location.tag, source_location.org, source_location.course, None, None, 'draft']) + _delete_modules_except_course(modulestore, modules, source_location, commit) # finally delete the top-level course module itself print "Deleting {0}...".format(source_location) @@ -144,4 +163,3 @@ def delete_course(modulestore, contentstore, source_location, commit=False): modulestore.delete_item(source_location) return True - From b18fb97eede5dc0e9b8cacc142b918da23bf5cc1 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 3 Jul 2013 11:24:30 -0400 Subject: [PATCH 2/4] extend tests to assert that all draft content is cleaned up. Also add assertion that assets are deleted as well --- .../contentstore/tests/test_contentstore.py | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index b946aac6bb..83b300b4fb 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -612,18 +612,42 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertEqual(resp.status_code, 400) def test_delete_course(self): + """ + This test will import a course, make a draft item, and delete it. This will also assert that the + draft content is also deleted + """ module_store = modulestore('direct') - import_from_xml(module_store, 'common/test/data/', ['full']) - content_store = contentstore() + draft_store = modulestore('draft') + + import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store) location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012') + # verify that we actually have assets + assets = content_store.get_all_content_for_course(location) + self.assertNotEquals(len(assets), 0) + + # get a vertical (and components in it) to put into 'draft' + vertical = module_store.get_item(Location(['i4x', 'edX', 'full', + 'vertical', 'vertical_66', None]), depth=1) + + draft_store.clone_item(vertical.location, vertical.location) + for child in vertical.get_children(): + draft_store.clone_item(child.location, child.location) + + # delete the course delete_course(module_store, content_store, location, commit=True) - items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None])) + # assert that there's absolutely no non-draft modules in the course + # this should also include all draft items + items = draft_store.get_items(Location(['i4x', 'edX', 'full', None, None])) self.assertEqual(len(items), 0) + # assert that all content in the asset library is also deleted + assets = content_store.get_all_content_for_course(location) + self.assertEqual(len(assets), 0) + def verify_content_existence(self, store, root_dir, location, dirname, category_name, filename_suffix=''): filesystem = OSFS(root_dir / 'test_export') self.assertTrue(filesystem.exists(dirname)) From e92799bb2edf032bc5c81ba36b62532f56f1b566 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 3 Jul 2013 16:12:23 -0400 Subject: [PATCH 3/4] switch from prints to logging.debug. Also refactor some duplicate code --- .../xmodule/modulestore/store_utilities.py | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index 89dca0c325..e568405593 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -2,6 +2,8 @@ from xmodule.contentstore.content import StaticContent from xmodule.modulestore import Location from xmodule.modulestore.mongo import MongoModuleStore +import logging + def clone_course(modulestore, contentstore, source_location, dest_location, delete_original=False): # first check to see if the modulestore is Mongo backed @@ -109,7 +111,7 @@ def _delete_modules_except_course(modulestore, modules, source_location, commit) """ for module in modules: if module.category != 'course': - print "Deleting {0}...".format(module.location) + logging.debug("Deleting {0}...".format(module.location)) if commit: # sanity check. Make sure we're not deleting a module in the incorrect course if module.location.org != source_location.org or module.location.course != source_location.course: @@ -117,6 +119,18 @@ def _delete_modules_except_course(modulestore, modules, source_location, commit) modulestore.delete_item(module.location) +def _delete_assets(contentstore, assets, commit): + """ + This helper method will enumerate through a list of assets and delete them + """ + for asset in assets: + asset_loc = Location(asset["_id"]) + id = StaticContent.get_id_from_location(asset_loc) + logging.debug("Deleting {0}...".format(id)) + if commit: + contentstore.delete(id) + + def delete_course(modulestore, contentstore, source_location, commit=False): """ This method will actually do the work to delete all content in a course in a MongoDB backed @@ -133,21 +147,11 @@ def delete_course(modulestore, contentstore, source_location, commit=False): # first delete all of the thumbnails thumbs = contentstore.get_all_content_thumbnails_for_course(source_location) - for thumb in thumbs: - thumb_loc = Location(thumb["_id"]) - id = StaticContent.get_id_from_location(thumb_loc) - print "Deleting {0}...".format(id) - if commit: - contentstore.delete(id) + _delete_assets(contentstore, thumbs, commit) # then delete all of the assets assets = contentstore.get_all_content_for_course(source_location) - for asset in assets: - asset_loc = Location(asset["_id"]) - id = StaticContent.get_id_from_location(asset_loc) - print "Deleting {0}...".format(id) - if commit: - contentstore.delete(id) + _delete_assets(contentstore, assets, commit) # then delete all course modules modules = modulestore.get_items([source_location.tag, source_location.org, source_location.course, None, None, None]) From 3acff0a77ab0ab14e0d55e380f98cfa77977fc18 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 3 Jul 2013 16:27:30 -0400 Subject: [PATCH 4/4] remove unnecessary type check on the module store. XML module stores will throw an exception when deleting an item... --- common/lib/xmodule/xmodule/modulestore/store_utilities.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index e568405593..2ac80b2b8b 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -137,10 +137,6 @@ def delete_course(modulestore, contentstore, source_location, commit=False): courseware store. BE VERY CAREFUL, this is not reversable. """ - # first check to see if the modulestore is Mongo backed - if not isinstance(modulestore, MongoModuleStore): - raise Exception("Expected a MongoModuleStore in the runtime. Aborting....") - # check to see if the source course is actually there if not modulestore.has_item(source_location): raise Exception("Cannot find a course at {0}. Aborting".format(source_location))