Custom error messages for delete course command

This is a fix for
https://openedx.atlassian.net/browse/PLAT-619
added custom CommandError messages for invalid syntax and
invalid course keys.

cms/djangoapps/contentstore/management/commands/delete_course.py

-###
-### Script for cloning a course
-###
+"""
+    Command for deleting courses
+
+    Arguments:
+        arg1 (str): Course key of the course to delete
+        arg2 (str): 'commit'
+
+    Returns:
+        none
+"""
 from django.core.management.base import BaseCommand, CommandError
 from .prompt import query_yes_no
 from contentstore.utils import delete_course_and_groups
@@ -8,27 +15,56 @@ from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
+
+
+def print_out_all_courses():
+    """
+    Print out all the courses available in the course_key format so that
+    the user can correct any course_key mistakes
+    """
+    courses = modulestore().get_courses_keys()
+    print 'Available courses:'
+    for course in courses:
+        print str(course)
+    print ''
 
 
 class Command(BaseCommand):
+    """
+    Delete a MongoDB backed course
+    """
     help = '''Delete a MongoDB backed course'''
 
     def handle(self, *args, **options):
-        if len(args) != 1 and len(args) != 2:
-            raise CommandError("delete_course requires one or more arguments: <course_id> |commit|")
+        if len(args) == 0:
+            raise CommandError("Arguments missing: 'org/number/run commit'")
 
-        try:
-            course_key = CourseKey.from_string(args[0])
-        except InvalidKeyError:
-            course_key = SlashSeparatedCourseKey.from_deprecated_string(args[0])
+        if len(args) == 1:
+            if args[0] == 'commit':
+                raise CommandError("Delete_course requires a course_key <org/number/run> argument.")
+            else:
+                raise CommandError("Delete_course requires a commit argument at the end")
+        elif len(args) == 2:
+            try:
+                course_key = CourseKey.from_string(args[0])
+            except InvalidKeyError:
+                try:
+                    course_key = SlashSeparatedCourseKey.from_deprecated_string(args[0])
+                except InvalidKeyError:
+                    raise CommandError("Invalid course_key: '%s'. Proper syntax: 'org/number/run commit' " % args[0])
+            if args[1] != 'commit':
+                raise CommandError("Delete_course requires a commit argument at the end")
+        elif len(args) > 2:
+            raise CommandError("Too many arguments! Expected <course_key> <commit>")
 
-        commit = False
-        if len(args) == 2:
-            commit = args[1] == 'commit'
+        print_out_all_courses()
 
-        if commit:
-            print('Actually going to delete the course from DB....')
+        if not modulestore().get_course(course_key):
+            raise CommandError("Course with '%s' key not found." % args[0])
 
-            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)
+        print 'Actually going to delete the %s course from DB....' % args[0]
+        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)
+                print_out_all_courses()

cms/djangoapps/contentstore/management/commands/tests/test_delete_course.py

+"""
+Unittests for deleting a course in an chosen modulestore
+"""
+
+import unittest
+import mock
+
+from django.core.management import CommandError
+from contentstore.management.commands.delete_course import Command  # pylint: disable=import-error
+from contentstore.tests.utils import CourseTestCase  # pylint: disable=import-error
+from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.django import modulestore
+
+
+class TestArgParsing(unittest.TestCase):
+    """
+    Tests for parsing arguments for the 'delete_course'  management command
+    """
+
+    def setUp(self):
+        super(TestArgParsing, self).setUp()
+
+        self.command = Command()
+
+    def test_no_args(self):
+        """
+        Testing 'delete_course' command with no arguments provided
+        """
+        errstring = "Arguments missing: 'org/number/run commit'"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle()
+
+    def test_no_course_key(self):
+        """
+        Testing 'delete_course' command with no course key provided
+        """
+        errstring = "Delete_course requires a course_key <org/number/run> argument."
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("commit")
+
+    def test_commit_argument(self):
+        """
+        Testing 'delete_course' command without 'commit' argument
+        """
+        errstring = "Delete_course requires a commit argument at the end"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("TestX/TS01/run")
+
+    def test_invalid_course_key(self):
+        """
+        Testing 'delete_course' command with an invalid course key argument
+        """
+        errstring = "Invalid course_key: 'TestX/TS01'. Proper syntax: 'org/number/run commit' "
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("TestX/TS01", "commit")
+
+    def test_missing_commit_argument(self):
+        """
+        Testing 'delete_course' command with misspelled 'commit' argument
+        """
+        errstring = "Delete_course requires a commit argument at the end"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("TestX/TS01/run", "comit")
+
+    def test_too_many_arguments(self):
+        """
+        Testing 'delete_course' command with more than 2 arguments
+        """
+        errstring = "Too many arguments! Expected <course_key> <commit>"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("TestX/TS01/run", "commit", "invalid")
+
+
+class DeleteCourseTest(CourseTestCase):
+    """
+    Test for course deleting functionality of the 'delete_course' command
+    """
+
+    YESNO_PATCH_LOCATION = 'contentstore.management.commands.delete_course.query_yes_no'
+
+    def setUp(self):
+        super(DeleteCourseTest, self).setUp()
+
+        self.command = Command()
+
+        org = 'TestX'
+        course_number = 'TS01'
+        course_run = '2015_Q1'
+
+        # Create a course using split modulestore
+        self.course = CourseFactory.create(
+            org=org,
+            number=course_number,
+            run=course_run
+        )
+
+    def test_courses_keys_listing(self):
+        """
+        Test if the command lists out available course key courses
+        """
+        courses = [str(key) for key in modulestore().get_courses_keys()]
+        self.assertIn("TestX/TS01/2015_Q1", courses)
+
+    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):
+            self.command.handle("TestX/TS01/2015_Q7", "commit")
+
+    def test_course_deleted(self):
+        """
+        Testing if the entered course was deleted
+        """
+        with mock.patch(self.YESNO_PATCH_LOCATION) as patched_yes_no:
+            patched_yes_no.return_value = True
+            self.command.handle("TestX/TS01/2015_Q1", "commit")
+            courses = [unicode(key) for key in modulestore().get_courses_keys()]
+            self.assertNotIn("TestX/TS01/2015_Q1", courses)

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -281,6 +281,21 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         return courses.values()
 
     @strip_key
+    def get_courses_keys(self, **kwargs):
+        '''
+        Returns a list containing the top level XModuleDescriptors keys of the courses in this modulestore.
+        '''
+        courses = {}
+        for store in self.modulestores:
+            # filter out ones which were fetched from earlier stores but locations may not be ==
+            for course in store.get_courses(**kwargs):
+                course_id = self._clean_locator_for_mapping(course.id)
+                if course_id not in courses:
+                    # course is indeed unique. save it in result
+                    courses[course_id] = course
+        return courses.keys()
+
+    @strip_key
     def get_libraries(self, **kwargs):
         """
         Returns a list containing the top level XBlock of the libraries (LibraryRoot) in this modulestore.