Merge pull request #385 from open-craft/duplicate-discussion-targets

Fix issues with duplicate discussion targets
open-craft · Feb 13, 2015 · f30ea13 · f30ea13
2 parents ed6a987 + ecb0813
commit f30ea13
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 12 deletions.
diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py
@@ -177,14 +177,6 @@ def update_thread(request, course_id, thread_id):
     if "commentable_id" in request.POST:
         course = get_course_with_access(request.user, 'load', course_key)
         commentable_ids = get_discussion_categories_ids(course)
-        # The commentable ID on thread creation may not be available in the
-        # topic menu, such as when the thread is concerning the current
-        # unit in context. In this case the topic menu will be blank, but the
-        # old commentable_id will be sent.
-        try:
-            commentable_ids.append(thread.commentable_id)
-        except AttributeError:
-            pass
         if request.POST.get("commentable_id") in commentable_ids:
             thread.commentable_id = request.POST["commentable_id"]
         else:

diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py
@@ -337,6 +337,26 @@ def check_cohorted(is_cohorted):
         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)
+
+        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()

diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py
@@ -183,11 +183,16 @@ def get_discussion_category_map(course):
             if node[level]["start_date"] > category_start_date:
                 node[level]["start_date"] = category_start_date
 
+        dupe_counters = defaultdict(lambda: 0)
         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]:
+                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