Added validation to discussion topic input in studio

common/lib/xmodule/xmodule/course_module.py

@@ -269,6 +269,53 @@ def get_available_providers():
     return available_providers
 
 
+class DiscussionTopicMapping(Dict):
+    """
+    DiscussionTopicMapping field, which includes validation of the topic id
+    """
+
+    _default = {}
+
+    def from_json(self, value):
+        """
+        Return python dict type. Performs validation on id key for
+        each discussion topic. Whenever Discussion Topic Mapping field is saved or
+        uploaded from data, this function is called to validate input mapping
+        """
+        if value is None:
+            return None
+        """
+        if statement checks to make sure value is a dict,
+        each topic in dict has a key id and the id value only has valid characters
+        """
+        if isinstance(value, dict):
+            for key in value:
+                if "id" in value[key].keys():
+                    if not self._validate_topic_id_value(value[key]["id"]):
+                        raise ValueError("For discussion topic ids, the only characters that are supported are the English alphabet(a-z, A-Z), integers(0-9), underscore, hyphen, and period.")
+                else:
+                    raise ValueError("All discussion topics must have an ID value set.")
+        else:
+            raise TypeError('Invalid JSON format found in Discussion Topic Mapping.' % type(value))
+        return value
+    enforce_type = from_json
+
+    def _validate_topic_id_value(self, value):
+        """
+        Function validates discussion topic ids. Will return true if
+        only valid characters(a-z,A-Z,0-9,underscore(_), hyphen(-), and period(.))
+        are used in id body
+        inputs: value(str)
+        """
+        for character in value:
+            if not (character.isalnum()
+                    or character == "-"
+                    or character == "_"
+                    or character == "."):
+                return False
+        return True
+
+
 class CourseFields(object):
     lti_passports = List(
         display_name=_("LTI Passports"),
@@ -360,7 +407,7 @@ class CourseFields(object):
         ),
         scope=Scope.settings
     )
-    discussion_topics = Dict(
+    discussion_topics = DiscussionTopicMapping(
         display_name=_("Discussion Topic Mapping"),
         help=_(
             'Enter discussion categories in the following format: "CategoryName": '

common/test/acceptance/tests/studio/test_studio_settings.py

@@ -454,25 +454,6 @@ class AdvancedSettingsValidationTest(StudioCourseTest):
             "Button text should change to 'Show Deprecated Settings' after the click"
         )
 
-    def test_multi_line_input(self):
-        """
-        Scenario: Test that advanced settings correctly shows the multi-line input
-            Given I am on the Advanced Course Settings page in Studio
-            When I create a JSON object as a value for "Discussion Topic Mapping"
-                Then it is displayed as formatted
-        """
-
-        inputs = {
-            "key": "value",
-            "key_2": "value_2"
-        }
-        json_input = json.dumps(inputs)
-        self.advanced_settings.set('Discussion Topic Mapping', json_input)
-        self.assertEqual(
-            self.advanced_settings.get('Discussion Topic Mapping'),
-            '{\n    "key": "value",\n    "key_2": "value_2"\n}'
-        )
-
     def test_automatic_quoting_of_non_json_value(self):
         """
         Scenario: Test that advanced settings automatically quotes the field input

lms/djangoapps/discussion/django_comment_client/tests/utils.py

@@ -120,7 +120,19 @@ def topic_name_to_id(course, name):
     """
     Given a discussion topic name, return an id for that name (includes
     course and url_name).
+    The set of valid characters for topic id is a smaller subset of valid
+    characters for topic name. Topic id can only have
+    these characters: a-z, A-Z, 0-9, underscore, hyphen, and period
+    Whereas, topic name character choice is much larger.
+    This function goes through all the characters in name and removes all invalid
+    characters and returns the resulting topic id.
     """
+    for index, indexed_character in enumerate(name):
+        if not (indexed_character.isalnum()
+                or indexed_character == "-"
+                or indexed_character == "_"
+                or indexed_character == "."):
+            name = name[:index] + "_" + name[index + 1:]
     return "{course}_{run}_{name}".format(
         course=course.location.course,
         run=course.url_name,

lms/djangoapps/discussion/rest_api/tests/test_views.py

@@ -1928,7 +1928,7 @@ class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStor
             start=datetime.now()
         )
         discussion_topics = {
-            "Topic B": {"id": "Topic B"},
+            "Topic B": {"id": "Topic_B"},
         }
         config_course_cohorts(self.course, is_cohorted=True)
         config_course_discussions(
@@ -2073,7 +2073,7 @@ class CourseDiscussionSettingsAPIViewTest(APITestCase, UrlResetMixin, ModuleStor
     def test_update_course_wide_discussion_settings(self):
         """Test whether the 'divided_course_wide_discussions' setting is updated."""
         discussion_topics = {
-            'Topic B': {'id': 'Topic B'}
+            'Topic B': {'id': 'Topic_B'}
         }
         config_course_cohorts(self.course, is_cohorted=True)
         config_course_discussions(self.course, discussion_topics=discussion_topics)