Merge pull request #457 from open-craft/discussion-export-all-cohorted

Don't stop with --all when one of the course doesn't have cohorted discussions

lms/djangoapps/django_comment_client/management/commands/export_discussion_participation.py

@@ -20,6 +20,11 @@ import django_comment_client.utils as utils
 from xmodule.modulestore.django import modulestore
 
 
+class MissingCohortedConfigCommandError(CommandError): #pylint: disable=no-init
+    """ Raised when a command requires cohorted discussions configured, but none are found """
+    pass
+
+
 class DiscussionExportFields(object):
     """ Container class for field names """
     USER_ID = u"id"
@@ -153,7 +158,10 @@ class Command(BaseCommand):
                 raw_course_key,
                 dir_name / self.get_default_file_location(raw_course_key)
             ]
-            self.dump_one(*args, **options)
+            try:
+                self.dump_one(*args, **options)
+            except MissingCohortedConfigCommandError as e:
+                print('Error generating CSV for course {}: {}'.format(raw_course_key, e.message))
 
     def dump_one(self, *args, **options):
         if not args:
@@ -180,8 +188,9 @@ class Command(BaseCommand):
         if options.get(self.COHORTED_ONLY_PARAMETER, False):
             cohorted_discussions = course.cohort_config.get('cohorted_inline_discussions', None)
             if not cohorted_discussions:
-                raise CommandError("Only cohorted discussions are marked for export, "
-                                   "but no cohorted discussions found for the course")
+                raise MissingCohortedConfigCommandError(
+                        "Only cohorted discussions are marked for export, "
+                        "but no cohorted discussions found for the course")
             else:
                 target_discussion_ids = cohorted_discussions
 

lms/djangoapps/django_comment_client/tests/management/test_export_discussion_participation.py

@@ -96,6 +96,29 @@ class CommandTest(TestCase):
         self.assertIn('test_dir/social_stats_course-v1SandboxSandboxSandbox', calls[1][0][1])
         self.assertIn('test_dir/social_stats_course-v1TestTestyTestify', calls[2][0][1])
 
+    def test_all_cohortedonly_options_together(self, patched_get_course):
+        """ Ensure the 'all' option doesn't stop when one of the course doesn't have cohorted discussions """
+        self.set_up_default_mocks(patched_get_course)
+        type(patched_get_course.return_value).cohort_config = mock.PropertyMock(
+            return_value={'cohorted_inline_discussions': []}
+        )
+        self.command.get_all_courses = mock.Mock()
+        course_list = [mock.Mock() for __ in range(0, 3)]
+        locator_list = [
+            CourseLocator(org="edX", course="demoX", run="now"),
+            CourseLocator(org="Sandbox", course="Sandbox", run="Sandbox"),
+            CourseLocator(org="Test", course="Testy", run="Testify"),
+        ]
+        for index, course in enumerate(course_list):
+            course.location.course_key = locator_list[index]
+        self.command.get_all_courses.return_value = course_list
+        self.command.handle("test_dir", all=True, cohorted_only=True, dummy='test')
+        calls = patched_get_course.call_args_list
+        self.assertEqual(len(calls), 3)
+        self.assertEqual(calls[0][0][0], locator_list[0])
+        self.assertEqual(calls[1][0][0], locator_list[1])
+        self.assertEqual(calls[2][0][0], locator_list[2])
+
     @ddt.data("edX/demoX/now", "otherX/CourseX/later")
     def test_handle_writes_to_correct_location_when_output_file_not_specified(self, course_key, patched_get_course):
         """ Tests that when no explicit filename is given data is exported to default location """