diff --git a/common/static/common/js/discussion/views/discussion_topic_menu_view.js b/common/static/common/js/discussion/views/discussion_topic_menu_view.js index 547cbed1df..a98b5b91c7 100644 --- a/common/static/common/js/discussion/views/discussion_topic_menu_view.js +++ b/common/static/common/js/discussion/views/discussion_topic_menu_view.js @@ -45,10 +45,12 @@ renderCategoryMap: function(map) { var categoryTemplate = edx.HtmlUtils.template($('#new-post-menu-category-template').html()), entryTemplate = edx.HtmlUtils.template($('#new-post-menu-entry-template').html()), - mappedCategorySnippets = _.map(map.children, function(name) { + mappedCategorySnippets = _.map(map.children, function(child) { var entry, - html = ''; - if (_.has(map.entries, name)) { + html = '', + name = child[0], // child[0] is the category name + type = child[1]; // child[1] is the type (i.e. 'entry' or 'subcategory') + if (_.has(map.entries, name) && type === 'entry') { entry = map.entries[name]; html = entryTemplate({ text: name, diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js index abfe539bfd..6b3c3d2921 100644 --- a/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js @@ -97,7 +97,11 @@ beforeEach(function() { this.course_settings = new DiscussionCourseSettings({ 'category_map': { - 'children': ['Topic', 'General', 'Basic Question'], + 'children': [ // eslint-disable-line quote-props + ['Topic', 'entry'], + ['General', 'entry'], + ['Basic Question', 'entry'] + ], 'entries': { 'Topic': { 'is_cohorted': true, diff --git a/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js index c21ecf95ed..4c9a3e798a 100644 --- a/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js @@ -21,11 +21,11 @@ 'Basic Question Types': { 'subcategories': {}, 'children': [ - 'Selection From Options', - 'Numerical Input', - 'Very long category name', - 'Very very very very long category name', - 'Name with HTML' + ['Selection From Options', 'entry'], + ['Numerical Input', 'entry'], + ['Very long category name', 'entry'], + ['Very very very very long category name', 'entry'], + ['Name with HTML', 'entry'] ], 'entries': { 'Selection From Options': { @@ -59,7 +59,7 @@ 'Example Inline Discussion': { 'subcategories': {}, 'children': [ - 'What Are Your Goals for Creating a MOOC?' + ['What Are Your Goals for Creating a MOOC?', 'entry'] ], 'entries': { 'What Are Your Goals for Creating a MOOC?': { @@ -70,7 +70,10 @@ } } }, - 'children': ['Basic Question Types', 'Example Inline Discussion'], + 'children': [ // eslint-disable-line quote-props + ['Basic Question Types', 'subcategory'], + ['Example Inline Discussion', 'subcategory'] + ], 'entries': {} }, 'is_cohorted': true diff --git a/common/static/common/js/spec/discussion/view/new_post_view_spec.js b/common/static/common/js/spec/discussion/view/new_post_view_spec.js index 1708fd081d..6726036980 100644 --- a/common/static/common/js/spec/discussion/view/new_post_view_spec.js +++ b/common/static/common/js/spec/discussion/view/new_post_view_spec.js @@ -35,7 +35,7 @@ beforeEach(function() { this.course_settings = new DiscussionCourseSettings({ category_map: { - children: ['Topic', 'General', 'Not Cohorted'], + children: [['Topic', 'entry'], ['General', 'entry'], ['Not Cohorted', 'entry']], entries: { Topic: { is_cohorted: true, @@ -172,7 +172,9 @@ 'subcategories': { 'Week 1': { 'subcategories': {}, - 'children': ['Topic-Level Student-Visible Label'], + 'children': [ // eslint-disable-line quote-props + ['Topic-Level Student-Visible Label', 'entry'] + ], 'entries': { 'Topic-Level Student-Visible Label': { 'sort_key': null, @@ -182,7 +184,10 @@ } } }, - 'children': ['General', 'Week 1'], + 'children': [ // eslint-disable-line quote-props + ['General', 'entry'], + ['Week 1', 'subcategory'] + ], 'entries': { 'General': { 'sort_key': 'General', diff --git a/common/static/common/js/spec_helpers/discussion_spec_helper.js b/common/static/common/js/spec_helpers/discussion_spec_helper.js index bae2354e70..e1838b05ad 100644 --- a/common/static/common/js/spec_helpers/discussion_spec_helper.js +++ b/common/static/common/js/spec_helpers/discussion_spec_helper.js @@ -54,7 +54,7 @@ DiscussionSpecHelper.createTestCourseSettings = function() { return new DiscussionCourseSettings({ category_map: { - children: ['Test Topic', 'Other Topic'], + children: [['Test Topic', 'entry'], ['Other Topic', 'entry']], entries: { 'Test Topic': { is_cohorted: true, diff --git a/lms/djangoapps/django_comment_client/constants.py b/lms/djangoapps/django_comment_client/constants.py new file mode 100644 index 0000000000..1df32828cf --- /dev/null +++ b/lms/djangoapps/django_comment_client/constants.py @@ -0,0 +1,6 @@ +""" +Constants for Discussions +""" + +TYPE_ENTRY = 'entry' # A leaf node in a category hierarchy. +TYPE_SUBCATEGORY = 'subcategory' # A non-leaf node in a category hierarchy. diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 290daafb22..ad23b133b5 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -14,6 +14,7 @@ from edxmako import add_lookup from django_comment_client.tests.factories import RoleFactory from django_comment_client.tests.unicode import UnicodeTestMixin +from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY import django_comment_client.utils as utils from lms.lib.comment_client.utils import perform_request, CommentClientMaintenanceError from django_comment_common.models import ForumsConfig @@ -399,7 +400,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Topic C": {"id": "Topic_C", "sort_key": "Topic C", "is_cohorted": "Topic_C" in expected_ids}, }, "subcategories": {}, - "children": ["Topic A", "Topic B", "Topic C"] + "children": [("Topic A", TYPE_ENTRY), ("Topic B", TYPE_ENTRY), ("Topic C", TYPE_ENTRY)] } ) @@ -451,10 +452,10 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Chapter"] + "children": [("Chapter", TYPE_SUBCATEGORY)] } ) @@ -475,10 +476,10 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Chapter"] + "children": [("Chapter", TYPE_SUBCATEGORY)] } ) @@ -499,10 +500,10 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Chapter"] + "children": [("Chapter", TYPE_SUBCATEGORY)] }, cohorted_if_in_list=True ) @@ -526,12 +527,12 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion 1"], + "children": [("Discussion 1", TYPE_ENTRY)], "start_date": later, "sort_key": "Chapter 1" } }, - "children": ["Chapter 1"] + "children": [("Chapter 1", TYPE_SUBCATEGORY)] }, cohorted_if_in_list=True, exclude_unstarted=False @@ -565,7 +566,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion 1", "Discussion 2"] + "children": [("Discussion 1", TYPE_ENTRY), ("Discussion 2", TYPE_ENTRY)] }, "Chapter 2": { "entries": { @@ -588,7 +589,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] }, "Subsection 2": { "entries": { @@ -599,13 +600,13 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Subsection 1", "Subsection 2"] + "children": [("Subsection 1", TYPE_SUBCATEGORY), ("Subsection 2", TYPE_SUBCATEGORY)] } }, - "children": ["Discussion", "Section 1"] + "children": [("Discussion", TYPE_ENTRY), ("Section 1", TYPE_SUBCATEGORY)] }, "Chapter 3": { "entries": {}, @@ -619,13 +620,14 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Section 1"] + "children": [("Section 1", TYPE_SUBCATEGORY)] } }, - "children": ["Chapter 1", "Chapter 2", "Chapter 3"] + "children": [("Chapter 1", TYPE_SUBCATEGORY), ("Chapter 2", TYPE_SUBCATEGORY), + ("Chapter 3", TYPE_SUBCATEGORY)] } ) @@ -652,13 +654,16 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): 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) + chapter1_discussions_with_types = set([("Discussion A", TYPE_ENTRY), ("Discussion B", TYPE_ENTRY), + ("Discussion A (1)", TYPE_ENTRY), ("Discussion A (2)", TYPE_ENTRY)]) + self.assertEqual(set(chapter1["children"]), chapter1_discussions_with_types) 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) + subsection1_discussions_with_types = set([("Discussion", TYPE_ENTRY), ("Discussion (1)", TYPE_ENTRY)]) + self.assertEqual(set(subsection1["children"]), subsection1_discussions_with_types) self.assertEqual(set(subsection1["entries"].keys()), subsection1_discussions) def test_start_date_filter(self): @@ -685,7 +690,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion 1"] + "children": [("Discussion 1", TYPE_ENTRY)] }, "Chapter 2": { "entries": { @@ -696,10 +701,10 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Chapter 1", "Chapter 2"] + "children": [("Chapter 1", TYPE_SUBCATEGORY), ("Chapter 2", TYPE_SUBCATEGORY)] } ) @@ -735,7 +740,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion 1", "Discussion 2"] + "children": [("Discussion 1", TYPE_ENTRY), ("Discussion 2", TYPE_ENTRY)] }, "Chapter 2": { "entries": { @@ -758,7 +763,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] }, "Subsection 2": { "entries": { @@ -769,13 +774,13 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Subsection 1", "Subsection 2"] + "children": [("Subsection 1", TYPE_SUBCATEGORY), ("Subsection 2", TYPE_SUBCATEGORY)] } }, - "children": ["Discussion", "Section 1"] + "children": [("Discussion", TYPE_ENTRY), ("Section 1", TYPE_SUBCATEGORY)] }, "Chapter 3": { "entries": {}, @@ -789,13 +794,14 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Section 1"] + "children": [("Section 1", TYPE_SUBCATEGORY)] } }, - "children": ["Chapter 1", "Chapter 2", "Chapter 3"] + "children": [("Chapter 1", TYPE_SUBCATEGORY), ("Chapter 2", TYPE_SUBCATEGORY), + ("Chapter 3", TYPE_SUBCATEGORY)] } ) @@ -840,15 +846,15 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): }, "subcategories": {}, "children": [ - "Discussion 2", - "Discussion 5", - "Discussion 4", - "Discussion 1", - "Discussion 3" + ("Discussion 2", TYPE_ENTRY), + ("Discussion 5", TYPE_ENTRY), + ("Discussion 4", TYPE_ENTRY), + ("Discussion 1", TYPE_ENTRY), + ("Discussion 3", TYPE_ENTRY) ] } }, - "children": ["Chapter"] + "children": [("Chapter", TYPE_SUBCATEGORY)] } ) @@ -866,7 +872,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Topic C": {"id": "Topic_C", "sort_key": "A", "is_cohorted": False}, }, "subcategories": {}, - "children": ["Topic C", "Topic A", "Topic B"] + "children": [("Topic C", TYPE_ENTRY), ("Topic A", TYPE_ENTRY), ("Topic B", TYPE_ENTRY)] } ) @@ -913,15 +919,15 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): }, "subcategories": {}, "children": [ - "Discussion A", - "Discussion B", - "Discussion C", - "Discussion D", - "Discussion E" + ("Discussion A", TYPE_ENTRY), + ("Discussion B", TYPE_ENTRY), + ("Discussion C", TYPE_ENTRY), + ("Discussion D", TYPE_ENTRY), + ("Discussion E", TYPE_ENTRY) ] } }, - "children": ["Chapter"] + "children": [("Chapter", TYPE_SUBCATEGORY)] } ) @@ -950,7 +956,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion 1", "Discussion 2"] + "children": [("Discussion 1", TYPE_ENTRY), ("Discussion 2", TYPE_ENTRY)] }, "Chapter B": { "entries": { @@ -966,7 +972,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion 1", "Discussion 2"] + "children": [("Discussion 1", TYPE_ENTRY), ("Discussion 2", TYPE_ENTRY)] }, "Chapter C": { "entries": { @@ -977,10 +983,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): } }, "subcategories": {}, - "children": ["Discussion"] + "children": [("Discussion", TYPE_ENTRY)] } }, - "children": ["Chapter A", "Chapter B", "Chapter C"] + "children": [("Chapter A", TYPE_SUBCATEGORY), ("Chapter B", TYPE_SUBCATEGORY), + ("Chapter C", TYPE_SUBCATEGORY)] } ) @@ -1042,9 +1049,9 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'Week 1': { 'subcategories': {}, 'children': [ - 'Visible to Alpha', - 'Visible to Beta', - 'Visible to Everyone' + ('Visible to Alpha', 'entry'), + ('Visible to Beta', 'entry'), + ('Visible to Everyone', 'entry') ], 'entries': { 'Visible to Alpha': { @@ -1065,7 +1072,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase } } }, - 'children': ['General', 'Week 1'], + 'children': [('General', 'entry'), ('Week 1', 'subcategory')], 'entries': { 'General': { 'sort_key': 'General', @@ -1088,8 +1095,8 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'Week 1': { 'subcategories': {}, 'children': [ - 'Visible to Alpha', - 'Visible to Everyone' + ('Visible to Alpha', 'entry'), + ('Visible to Everyone', 'entry') ], 'entries': { 'Visible to Alpha': { @@ -1105,7 +1112,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase } } }, - 'children': ['General', 'Week 1'], + 'children': [('General', 'entry'), ('Week 1', 'subcategory')], 'entries': { 'General': { 'sort_key': 'General', @@ -1128,8 +1135,8 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'Week 1': { 'subcategories': {}, 'children': [ - 'Visible to Beta', - 'Visible to Everyone' + ('Visible to Beta', 'entry'), + ('Visible to Everyone', 'entry') ], 'entries': { 'Visible to Beta': { @@ -1145,7 +1152,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase } } }, - 'children': ['General', 'Week 1'], + 'children': [('General', 'entry'), ('Week 1', 'subcategory')], 'entries': { 'General': { 'sort_key': 'General', @@ -1168,7 +1175,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'Week 1': { 'subcategories': {}, 'children': [ - 'Visible to Everyone' + ('Visible to Everyone', 'entry') ], 'entries': { 'Visible to Everyone': { @@ -1179,7 +1186,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase } } }, - 'children': ['General', 'Week 1'], + 'children': [('General', 'entry'), ('Week 1', 'subcategory')], 'entries': { 'General': { 'sort_key': 'General', diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index b2ce37b9f6..f620c66700 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -18,6 +18,7 @@ from xmodule.modulestore.django import modulestore from django_comment_common.models import Role, FORUM_ROLE_STUDENT from django_comment_client.permissions import check_permissions_by_view, has_permission, get_team from django_comment_client.settings import MAX_COMMENT_DEPTH +from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from edxmako import lookup_template from courseware import courses @@ -221,10 +222,10 @@ def _filter_unstarted_categories(category_map, course): filtered_map["entries"] = {} filtered_map["subcategories"] = {} - for child in unfiltered_map["children"]: - if child in unfiltered_map["entries"]: + for child, c_type in unfiltered_map["children"]: + if child in unfiltered_map["entries"] and c_type == TYPE_ENTRY: if course.self_paced or unfiltered_map["entries"][child]["start_date"] <= now: - filtered_map["children"].append(child) + filtered_map["children"].append((child, c_type)) filtered_map["entries"][child] = {} for key in unfiltered_map["entries"][child]: if key != "start_date": @@ -233,7 +234,7 @@ def _filter_unstarted_categories(category_map, course): log.debug(u"Filtering out:%s with start_date: %s", child, unfiltered_map["entries"][child]["start_date"]) else: if course.self_paced or unfiltered_map["subcategories"][child]["start_date"] < now: - filtered_map["children"].append(child) + filtered_map["children"].append((child, c_type)) filtered_map["subcategories"][child] = {} unfiltered_queue.append(unfiltered_map["subcategories"][child]) filtered_queue.append(filtered_map["subcategories"][child]) @@ -249,11 +250,11 @@ def _sort_map_entries(category_map, sort_alpha): for title, entry in category_map["entries"].items(): if entry["sort_key"] is None and sort_alpha: entry["sort_key"] = title - things.append((title, entry)) + things.append((title, entry, TYPE_ENTRY)) for title, category in category_map["subcategories"].items(): - things.append((title, category)) + things.append((title, category, TYPE_SUBCATEGORY)) _sort_map_entries(category_map["subcategories"][title], sort_alpha) - category_map["children"] = [x[0] for x in sorted(things, key=lambda x: x[1]["sort_key"])] + category_map["children"] = [(x[0], x[2]) for x in sorted(things, key=lambda x: x[1]["sort_key"])] def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude_unstarted=True): @@ -276,13 +277,16 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude >>> "id": "i4x-edx-eiorguegnru-course-foobarbaz" >>> } >>> }, - >>> "children": ["General", "Getting Started"], + >>> "children": [ + >>> ["General", "entry"], + >>> ["Getting Started", "subcategory"] + >>> ], >>> "subcategories": { >>> "Getting Started": { >>> "subcategories": {}, >>> "children": [ - >>> "Working with Videos", - >>> "Videos on edX" + >>> ["Working with Videos", "entry"], + >>> ["Videos on edX", "entry"] >>> ], >>> "entries": { >>> "Working with Videos": { diff --git a/lms/static/js/groups/views/cohort_discussions_course_wide.js b/lms/static/js/groups/views/cohort_discussions_course_wide.js index 3a4b400ebc..99214d344e 100644 --- a/lms/static/js/groups/views/cohort_discussions_course_wide.js +++ b/lms/static/js/groups/views/cohort_discussions_course_wide.js @@ -33,8 +33,11 @@ entries = courseWideDiscussions.entries, children = courseWideDiscussions.children; - return HtmlUtils.joinHtml.apply(this, _.map(children, function(name) { - var entry = entries[name]; + return HtmlUtils.joinHtml.apply(this, _.map(children, function(child) { + // child[0] is the category name, child[1] is the type. + // For course wide discussions, the type is always 'entry' + var name = child[0], + entry = entries[name]; return subCategoryTemplate({ name: name, id: entry.id, diff --git a/lms/static/js/groups/views/cohort_discussions_inline.js b/lms/static/js/groups/views/cohort_discussions_inline.js index 9b0f5acb98..70d79ef61a 100644 --- a/lms/static/js/groups/views/cohort_discussions_inline.js +++ b/lms/static/js/groups/views/cohort_discussions_inline.js @@ -48,9 +48,12 @@ entries = inlineDiscussions.entries, subcategories = inlineDiscussions.subcategories; - return HtmlUtils.joinHtml.apply(this, _.map(children, function(name) { - var htmlSnippet = '', entry; - if (entries && _.has(entries, name)) { + return HtmlUtils.joinHtml.apply(this, _.map(children, function(child) { + var htmlSnippet = '', + name = child[0], // child[0] is the category name + type = child[1], // child[1] is the type (i.e. 'entry' or 'subcategory') + entry; + if (entries && _.has(entries, name) && type === 'entry') { entry = entries[name]; htmlSnippet = entryTemplate({ name: name, diff --git a/lms/static/js/spec/groups/views/cohorts_spec.js b/lms/static/js/spec/groups/views/cohorts_spec.js index 8b1fd433e8..a8cd39294c 100644 --- a/lms/static/js/spec/groups/views/cohorts_spec.js +++ b/lms/static/js/spec/groups/views/cohorts_spec.js @@ -89,7 +89,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers createMockCohortDiscussionsJson = function(allCohorted) { return { course_wide_discussions: { - children: ['Topic_C_1', 'Topic_C_2'], + children: [['Topic_C_1', 'entry'], ['Topic_C_2', 'entry']], entries: { Topic_C_1: { sort_key: null, @@ -107,7 +107,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers subcategories: { Topic_I_1: { subcategories: {}, - children: ['Inline_Discussion_1', 'Inline_Discussion_2'], + children: [['Inline_Discussion_1', 'entry'], ['Inline_Discussion_2', 'entry']], entries: { Inline_Discussion_1: { sort_key: null, @@ -122,7 +122,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers } } }, - children: ['Topic_I_1'] + children: [['Topic_I_1', 'subcategory']] } }; }; @@ -1488,7 +1488,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers topicsJson = { course_wide_discussions: { - children: ['Topic_C_1'], + children: [['Topic_C_1', 'entry']], entries: { Topic_C_1: { sort_key: null, diff --git a/lms/templates/discussion/_filter_dropdown.html b/lms/templates/discussion/_filter_dropdown.html index 4ed62fef3d..f18e8309d6 100644 --- a/lms/templates/discussion/_filter_dropdown.html +++ b/lms/templates/discussion/_filter_dropdown.html @@ -1,13 +1,13 @@ <%page expression_filter="h"/> <%! from django.utils.translation import ugettext as _ - +from lms.djangoapps.django_comment_client.constants import TYPE_ENTRY from openedx.core.djangolib.markup import HTML %> <%def name="render_dropdown(map)"> - % for child in map["children"]: - % if child in map["entries"]: + % for child, c_type in map["children"]: + % if child in map["entries"] and c_type == TYPE_ENTRY: ${HTML(render_entry(map["entries"], child))} %else: ${HTML(render_category(map["subcategories"], child))} diff --git a/openedx/core/djangoapps/course_groups/tests/test_views.py b/openedx/core/djangoapps/course_groups/tests/test_views.py index 4afecd87df..fcef92da52 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_views.py +++ b/openedx/core/djangoapps/course_groups/tests/test_views.py @@ -21,6 +21,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.modulestore.tests.factories import ItemFactory +from lms.djangoapps.django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from ..models import CourseUserGroup, CourseCohort from ..views import ( @@ -1229,7 +1230,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): start_date = response['inline_discussions']['subcategories']['Chapter']['start_date'] expected_response = { "course_wide_discussions": { - 'children': ['Topic B'], + 'children': [['Topic B', TYPE_ENTRY]], 'entries': { 'Topic B': { 'sort_key': 'A', @@ -1243,7 +1244,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): 'subcategories': { 'Chapter': { 'subcategories': {}, - 'children': ['Discussion'], + 'children': [['Discussion', TYPE_ENTRY]], 'entries': { 'Discussion': { 'sort_key': None, @@ -1256,7 +1257,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): 'start_date': start_date } }, - 'children': ['Chapter'] + 'children': [['Chapter', TYPE_SUBCATEGORY]] } } self.assertEqual(response, expected_response) diff --git a/openedx/core/djangoapps/course_groups/views.py b/openedx/core/djangoapps/course_groups/views.py index 026c86ad9d..acffbd0978 100644 --- a/openedx/core/djangoapps/course_groups/views.py +++ b/openedx/core/djangoapps/course_groups/views.py @@ -24,6 +24,7 @@ from edxmako.shortcuts import render_to_response from . import cohorts from lms.djangoapps.django_comment_client.utils import get_discussion_category_map, get_discussion_categories_ids +from lms.djangoapps.django_comment_client.constants import TYPE_ENTRY from .models import CourseUserGroup, CourseUserGroupPartitionGroup, CohortMembership log = logging.getLogger(__name__) @@ -437,15 +438,15 @@ def cohort_discussion_topics(request, course_key_string): >>> "id": "i4x-edx-eiorguegnru-course-foobarbaz" >>> } >>> } - >>> "children": ["General"] + >>> "children": ["General", "entry"] >>> }, >>> "inline_discussions" : { >>> "subcategories": { >>> "Getting Started": { >>> "subcategories": {}, >>> "children": [ - >>> "Working with Videos", - >>> "Videos on edX" + >>> ["Working with Videos", "entry"], + >>> ["Videos on edX", "entry"] >>> ], >>> "entries": { >>> "Working with Videos": { @@ -460,7 +461,7 @@ def cohort_discussion_topics(request, course_key_string): >>> } >>> } >>> }, - >>> "children": ["Getting Started"] + >>> "children": ["Getting Started", "subcategory"] >>> }, >>> } >>> } @@ -479,11 +480,11 @@ def cohort_discussion_topics(request, course_key_string): course_wide_children = [] inline_children = [] - for name in discussion_category_map['children']: - if name in course_wide_entries: - course_wide_children.append(name) + for name, c_type in discussion_category_map['children']: + if name in course_wide_entries and c_type == TYPE_ENTRY: + course_wide_children.append([name, c_type]) else: - inline_children.append(name) + inline_children.append([name, c_type]) discussion_topics['course_wide_discussions'] = { 'entries': course_wide_entries,