From 639ab0dc70c121ed1395d5cb9d0c8ec1c7c0c5b2 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 | 21 +++++++++++++++++++ lms/djangoapps/django_comment_client/utils.py | 15 +++++++++---- 2 files changed, 32 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 b4b2d0b7f6..e3fb9b6ef7 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/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 diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index f8d247fe35..800545bc3e 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/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