From cdae6fe6b9935c7993d7fef2803bda17a37014aa Mon Sep 17 00:00:00 2001 From: Matjaz Gregoric Date: Tue, 10 Feb 2015 13:17:45 +0800 Subject: [PATCH] Fix issues with duplicate discussion targets. When two or more instances of Discussion XBlock were configured with the same discussion target (Category/Subcategory), only one of the blocks would be shown on the Course Discussion page. This was the source of several bugs when trying to edit discussion threads. This patch adds incrementing numbers to the title of each duplicate subcategory when rendering the Course Discussion to make sure that all of the threads are visible in Course Discussion. --- .../django_comment_client/tests/test_utils.py | 20 +++++++++++++++++++ lms/djangoapps/django_comment_client/utils.py | 13 ++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index d813cb5df2fd..6cc4b3b3024f 100644 --- 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 index e962d6d04278..f12c4c3da468 100644 --- 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