Added support for deleting assets to delete_course management command
The command now has an option to delete assets. This is relevant for users of the split Mongo module store, which does not delete assets by default. Additionally, tests and docstrings have been updated. LEARNER-2460
This commit is contained in:
committed by
Clinton Blackburn
parent
0543748380
commit
56eee715c1
@@ -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))
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user