diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 1870bce5bd..07e52cda5a 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -1,48 +1,38 @@ -""" - Command for deleting courses - - Arguments: - arg1 (str): Course key of the course to delete - - Returns: - none -""" from django.core.management.base import BaseCommand, CommandError from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from contentstore.utils import delete_course +from xmodule.contentstore.django import contentstore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore - from .prompt import query_yes_no class Command(BaseCommand): """ Delete a MongoDB backed course + Example usage: $ ./manage.py cms delete_course 'course-v1:edX+DemoX+Demo_Course' --settings=devstack $ ./manage.py cms delete_course 'course-v1:edX+DemoX+Demo_Course' --keep-instructors --settings=devstack + $ ./manage.py cms delete_course 'course-v1:edX+DemoX+Demo_Course' --remove-assets --settings=devstack Note: - keep-instructors option is added in effort to delete duplicate courses safely. - There happens to be courses with difference of casing in ids, for example - course-v1:DartmouthX+DART.ENGL.01.X+2016_T1 is a duplicate of course-v1:DartmouthX+DART.ENGL.01.x+2016_T1 - (Note the differene in 'x' of course number). These two are independent courses in MongoDB. - Current MYSQL setup is case-insensitive which essentially means there are not - seperate entries (in all course related mysql tables, but here we are concerned about accesses) - for duplicate courses. - This option will make us able to delete course (duplicate one) from - mongo while perserving course's related access data in mysql. + The keep-instructors option is useful for resolving issues that arise when a course run's ID is duplicated + in a case-insensitive manner. MongoDB is case-sensitive, but MySQL is case-insensitive. This results in + course-v1:edX+DemoX+1t2017 being treated differently in MongoDB from course-v1:edX+DemoX+1T2017 (capital 'T'). + + If you need to remove a duplicate that has resulted from casing issues, use the --keep-instructors flag + to ensure that permissions for the remaining course run are not deleted. + + Use the remove-assets option to ensure all assets are deleted. This is especially relevant to users of the + split Mongo modulestore. """ - help = '''Delete a MongoDB backed course''' + help = 'Delete a MongoDB backed course' def add_arguments(self, parser): - """ - Add arguments to the command parser. - """ - parser.add_argument('course_key', help="ID of the course to delete.") + parser.add_argument('course_key', help='ID of the course to delete.') parser.add_argument( '--keep-instructors', @@ -51,17 +41,30 @@ class Command(BaseCommand): help='Do not remove permissions of users and groups for course', ) + parser.add_argument( + '--remove-assets', + action='store_true', + help='Remove all assets associated with the course. ' + 'Be careful! These assets may be associated with another course', + ) + def handle(self, *args, **options): try: course_key = CourseKey.from_string(options['course_key']) except InvalidKeyError: - raise CommandError("Invalid course_key: '%s'." % options['course_key']) + raise CommandError('Invalid course_key: {}'.format(options['course_key'])) if not modulestore().get_course(course_key): - raise CommandError("Course with '%s' key not found." % options['course_key']) + raise CommandError('Course not found: {}'.format(options['course_key'])) - print 'Going to delete the %s course from DB....' % options['course_key'] - 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"): + print('Preparing to delete course %s from module store....' % options['course_key']) + + if query_yes_no('Are you sure you want to delete course {}?'.format(course_key), default='no'): + if query_yes_no('Are you sure? This action cannot be undone!', default='no'): delete_course(course_key, ModuleStoreEnum.UserID.mgmt_command, options['keep_instructors']) - print "Deleted course {}".format(course_key) + + if options['remove_assets']: + contentstore().delete_all_course_assets(course_key) + print('Deleted assets for course'.format(course_key)) + + print('Deleted course {}'.format(course_key)) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py b/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py index c4ad14c8e9..dc72d78e36 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py @@ -1,88 +1,76 @@ -""" -Unittests for deleting a course in an chosen modulestore -""" - import mock +from django.core.management import CommandError, call_command -from opaque_keys.edx.keys import CourseKey -from django.core.management import call_command, CommandError -from django.contrib.auth.models import User -from contentstore.tests.utils import CourseTestCase -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.django import modulestore from student.roles import CourseInstructorRole +from student.tests.factories import UserFactory +from xmodule.contentstore.content import StaticContent +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE +from xmodule.modulestore.tests.factories import CourseFactory -class DeleteCourseTest(CourseTestCase): +class DeleteCourseTests(ModuleStoreTestCase): """ Test for course deleting functionality of the 'delete_course' command """ - + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE YESNO_PATCH_LOCATION = 'contentstore.management.commands.delete_course.query_yes_no' - def setUp(self): - super(DeleteCourseTest, self).setUp() + def test_invalid_course_key(self): + course_run_key = 'foo/TestX/TS01/2015_Q7' + expected_error_message = 'Invalid course_key: ' + course_run_key + with self.assertRaisesRegexp(CommandError, expected_error_message): + call_command('delete_course', course_run_key) - org = 'TestX' - course_number = 'TS01' - course_run = '2015_Q1' + def test_course_not_found(self): + course_run_key = 'TestX/TS01/2015_Q7' + expected_error_message = 'Course not found: ' + course_run_key + with self.assertRaisesRegexp(CommandError, expected_error_message): + call_command('delete_course', course_run_key) - # Create a course using split modulestore - self.course = CourseFactory.create( - org=org, - number=course_number, - run=course_run - ) + def test_asset_and_course_deletion(self): + course_run = CourseFactory() + self.assertIsNotNone(modulestore().get_course(course_run.id)) - def test_invalid_key_not_found(self): - """ - Test for when a course key is malformed - """ - errstring = "Invalid course_key: 'foo/TestX/TS01/2015_Q7'." - with self.assertRaisesRegexp(CommandError, errstring): - call_command('delete_course', 'foo/TestX/TS01/2015_Q7') - - def test_course_key_not_found(self): - """ - Test for when a non-existing course key is entered - """ - errstring = "Course with 'TestX/TS01/2015_Q7' key not found." - with self.assertRaisesRegexp(CommandError, errstring): - call_command('delete_course', 'TestX/TS01/2015_Q7') - - def test_course_deleted(self): - """ - Testing if the entered course was deleted - """ - course_key = CourseKey.from_string('/'.join(["TestX", "TS01", "2015_Q1"])) - #Test if the course that is about to be deleted exists - self.assertIsNotNone(modulestore().get_course(course_key)) + store = contentstore() + asset_key = course_run.id.make_asset_key('asset', 'test.txt') + content = StaticContent(asset_key, 'test.txt', 'plain/text', 'test data') + store.save(content) + __, asset_count = store.get_all_content_for_course(course_run.id) + assert asset_count == 1 with mock.patch(self.YESNO_PATCH_LOCATION) as patched_yes_no: patched_yes_no.return_value = True - call_command('delete_course', 'TestX/TS01/2015_Q1') - self.assertIsNone(modulestore().get_course(course_key)) + call_command('delete_course', str(course_run.id)) - def test_course_deletion_with_keep_instructors(self): - """ - Tests that deleting course with keep-instructors option do not remove instructors from course. - """ - instructor_user = User.objects.create( - username='test_instructor', - email='test_email@example.com' - ) - self.assertIsNotNone(instructor_user) + assert modulestore().get_course(course_run.id) is None - # Add and verify instructor role for the course - instructor_role = CourseInstructorRole(self.course.id) - instructor_role.add_users(instructor_user) - self.assertTrue(instructor_role.has_user(instructor_user)) + __, asset_count = store.get_all_content_for_course(course_run.id) + assert asset_count == 1 - # Verify the course we are about to delete exists in the modulestore - self.assertIsNotNone(modulestore().get_course(self.course.id)) + def test_keep_instructors(self): + course_run = CourseFactory() + instructor = UserFactory() + CourseInstructorRole(course_run.id).add_users(instructor) with mock.patch(self.YESNO_PATCH_LOCATION, return_value=True): - call_command('delete_course', 'TestX/TS01/2015_Q1', '--keep-instructors') + call_command('delete_course', str(course_run.id), '--keep-instructors') - self.assertIsNone(modulestore().get_course(self.course.id)) - self.assertTrue(instructor_role.has_user(instructor_user)) + assert CourseInstructorRole(course_run.id).has_user(instructor) + + def test_remove_assets(self): + course_run = CourseFactory() + store = contentstore() + + asset_key = course_run.id.make_asset_key('asset', 'test.txt') + content = StaticContent(asset_key, 'test.txt', 'plain/text', 'test data') + store.save(content) + __, asset_count = store.get_all_content_for_course(course_run.id) + assert asset_count == 1 + + with mock.patch(self.YESNO_PATCH_LOCATION, return_value=True): + call_command('delete_course', str(course_run.id), '--remove-assets') + + __, asset_count = store.get_all_content_for_course(course_run.id) + assert asset_count == 0