Skip to content

Commit

Permalink
Fix issues with duplicate discussion targets.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mtyaka committed Feb 10, 2015
1 parent ed6a987 commit cdae6fe
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
20 changes: 20 additions & 0 deletions lms/djangoapps/django_comment_client/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 9 additions & 4 deletions lms/djangoapps/django_comment_client/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit cdae6fe

Please sign in to comment.