Merge pull request #7081 from open-craft/nonunique-discussion-targets

Fix issues with non-unique discussion targets

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -359,6 +359,27 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.course.cohort_config = {"cohorted": True}
         check_cohorted(True)
 
+    def test_tree_with_duplicate_targets(self):
+        self.create_discussion("Chapter 1", "Discussion A")
+        self.create_discussion("Chapter 1", "Discussion B")
+        self.create_discussion("Chapter 1", "Discussion A")  # duplicate
+        self.create_discussion("Chapter 1", "Discussion A")  # another duplicate
+        self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
+        self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")  # duplicate
+
+        category_map = utils.get_discussion_category_map(self.course, self.user)
+
+        chapter1 = category_map["subcategories"]["Chapter 1"]
+        chapter1_discussions = set(["Discussion A", "Discussion B", "Discussion A (1)", "Discussion A (2)"])
+        self.assertEqual(set(chapter1["children"]), chapter1_discussions)
+        self.assertEqual(set(chapter1["entries"].keys()), chapter1_discussions)
+
+        chapter2 = category_map["subcategories"]["Chapter 2"]
+        subsection1 = chapter2["subcategories"]["Section 1"]["subcategories"]["Subsection 1"]
+        subsection1_discussions = set(["Discussion", "Discussion (1)"])
+        self.assertEqual(set(subsection1["children"]), subsection1_discussions)
+        self.assertEqual(set(subsection1["entries"].keys()), subsection1_discussions)
+
     def test_start_date_filter(self):
         now = datetime.now()
         later = datetime.max

lms/djangoapps/django_comment_client/utils.py

@@ -196,11 +196,18 @@ def get_discussion_category_map(course, user):
             if node[level]["start_date"] > category_start_date:
                 node[level]["start_date"] = category_start_date
 
+        dupe_counters = defaultdict(lambda: 0)  # counts the number of times we see each title
         for entry in entries:
-            node[level]["entries"][entry["title"]] = {"id": entry["id"],
-                                                      "sort_key": entry["sort_key"],
-                                                      "start_date": entry["start_date"],
-                                                      "is_cohorted": is_course_cohorted}
+            title = entry["title"]
+            if node[level]["entries"][title]:
+                # If we've already seen this title, append an incrementing number to disambiguate
+                # the category from other categores sharing the same title in the course discussion UI.
+                dupe_counters[title] += 1
+                title = u"{title} ({counter})".format(title=title, counter=dupe_counters[title])
+            node[level]["entries"][title] = {"id": entry["id"],
+                                             "sort_key": entry["sort_key"],
+                                             "start_date": entry["start_date"],
+                                             "is_cohorted": is_course_cohorted}
 
     # TODO.  BUG! : course location is not unique across multiple course runs!
     # (I think Kevin already noticed this)  Need to send course_id with requests, store it