Skip to content

E
edx-platform
Commit 9728759d by Attiya Ishaque Committed by GitHub
Options

Merge pull request #13662 from edx/atiya/TNL-5621-Duplicate-discussion 

TNL-5621 Fix discussion topics in MIT course.
parents c873be0f 76e1d4a7
Showing with 80 additions and 48 deletions
common/static/common/js/discussion/views/discussion_topic_menu_view.js
...@@ -45,10 +45,12 @@ ...@@ -45,10 +45,12 @@
renderCategoryMap: function(map) { renderCategoryMap: function(map) {
var categoryTemplate = edx.HtmlUtils.template($('#new-post-menu-category-template').html()), var categoryTemplate = edx.HtmlUtils.template($('#new-post-menu-category-template').html()),
entryTemplate = edx.HtmlUtils.template($('#new-post-menu-entry-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, var entry,
html = ''; html = '',
if (_.has(map.entries, name)) { 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]; entry = map.entries[name];
html = entryTemplate({ html = entryTemplate({
text: name, text: name,
......
common/static/common/js/spec/discussion/view/discussion_thread_edit_view_spec.js
...@@ -97,7 +97,11 @@ ...@@ -97,7 +97,11 @@
beforeEach(function() { beforeEach(function() {
this.course_settings = new DiscussionCourseSettings({ this.course_settings = new DiscussionCourseSettings({
'category_map': { 'category_map': {
'children': ['Topic', 'General', 'Basic Question'], 'children': [ // eslint-disable-line quote-props
['Topic', 'entry'],
['General', 'entry'],
['Basic Question', 'entry']
],
'entries': { 'entries': {
'Topic': { 'Topic': {
'is_cohorted': true, 'is_cohorted': true,
......
common/static/common/js/spec/discussion/view/discussion_topic_menu_view_spec.js
...@@ -21,11 +21,11 @@ ...@@ -21,11 +21,11 @@
'Basic Question Types': { 'Basic Question Types': {
'subcategories': {}, 'subcategories': {},
'children': [ 'children': [
'Selection From Options', ['Selection From Options', 'entry'],
'Numerical Input', ['Numerical Input', 'entry'],
'Very long category name', ['Very long category name', 'entry'],
'Very very very very long category name', ['Very very very very long category name', 'entry'],
'Name with <em>HTML</em>' ['Name with <em>HTML</em>', 'entry']
], ],
'entries': { 'entries': {
'Selection From Options': { 'Selection From Options': {
...@@ -59,7 +59,7 @@ ...@@ -59,7 +59,7 @@
'Example Inline Discussion': { 'Example Inline Discussion': {
'subcategories': {}, 'subcategories': {},
'children': [ 'children': [
'What Are Your Goals for Creating a MOOC?' ['What Are Your Goals for Creating a MOOC?', 'entry']
], ],
'entries': { 'entries': {
'What Are Your Goals for Creating a MOOC?': { 'What Are Your Goals for Creating a MOOC?': {
...@@ -70,7 +70,10 @@ ...@@ -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': {} 'entries': {}
}, },
'is_cohorted': true 'is_cohorted': true
......
common/static/common/js/spec/discussion/view/new_post_view_spec.js
...@@ -35,7 +35,7 @@ ...@@ -35,7 +35,7 @@
beforeEach(function() { beforeEach(function() {
this.course_settings = new DiscussionCourseSettings({ this.course_settings = new DiscussionCourseSettings({
category_map: { category_map: {
children: ['Topic', 'General', 'Not Cohorted'], children: [['Topic', 'entry'], ['General', 'entry'], ['Not Cohorted', 'entry']],
entries: { entries: {
Topic: { Topic: {
is_cohorted: true, is_cohorted: true,
...@@ -172,7 +172,9 @@ ...@@ -172,7 +172,9 @@
'subcategories': { 'subcategories': {
'Week 1': { 'Week 1': {
'subcategories': {}, 'subcategories': {},
'children': ['Topic-Level Student-Visible Label'], 'children': [ // eslint-disable-line quote-props
['Topic-Level Student-Visible Label', 'entry']
],
'entries': { 'entries': {
'Topic-Level Student-Visible Label': { 'Topic-Level Student-Visible Label': {
'sort_key': null, 'sort_key': null,
...@@ -182,7 +184,10 @@ ...@@ -182,7 +184,10 @@
} }
} }
}, },
'children': ['General', 'Week 1'], 'children': [ // eslint-disable-line quote-props
['General', 'entry'],
['Week 1', 'subcategory']
],
'entries': { 'entries': {
'General': { 'General': {
'sort_key': 'General', 'sort_key': 'General',
......
common/static/common/js/spec_helpers/discussion_spec_helper.js
...@@ -54,7 +54,7 @@ ...@@ -54,7 +54,7 @@
DiscussionSpecHelper.createTestCourseSettings = function() { DiscussionSpecHelper.createTestCourseSettings = function() {
return new DiscussionCourseSettings({ return new DiscussionCourseSettings({
category_map: { category_map: {
children: ['Test Topic', 'Other Topic'], children: [['Test Topic', 'entry'], ['Other Topic', 'entry']],
entries: { entries: {
'Test Topic': { 'Test Topic': {
is_cohorted: true, is_cohorted: true,
......
lms/djangoapps/django_comment_client/constants.py 0 → 100644
"""
Constants for Discussions
"""
TYPE_ENTRY = 'entry' # A leaf node in a category hierarchy.
TYPE_SUBCATEGORY = 'subcategory' # A non-leaf node in a category hierarchy.
lms/djangoapps/django_comment_client/utils.py
...@@ -18,6 +18,7 @@ from xmodule.modulestore.django import modulestore ...@@ -18,6 +18,7 @@ from xmodule.modulestore.django import modulestore
from django_comment_common.models import Role, FORUM_ROLE_STUDENT 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.permissions import check_permissions_by_view, has_permission, get_team
from django_comment_client.settings import MAX_COMMENT_DEPTH 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 edxmako import lookup_template
from courseware import courses from courseware import courses
...@@ -221,10 +222,10 @@ def _filter_unstarted_categories(category_map, course): ...@@ -221,10 +222,10 @@ def _filter_unstarted_categories(category_map, course):
filtered_map["entries"] = {} filtered_map["entries"] = {}
filtered_map["subcategories"] = {} filtered_map["subcategories"] = {}
for child in unfiltered_map["children"]: for child, c_type in unfiltered_map["children"]:
if child in unfiltered_map["entries"]: if child in unfiltered_map["entries"] and c_type == TYPE_ENTRY:
if course.self_paced or unfiltered_map["entries"][child]["start_date"] <= now: 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] = {} filtered_map["entries"][child] = {}
for key in unfiltered_map["entries"][child]: for key in unfiltered_map["entries"][child]:
if key != "start_date": if key != "start_date":
...@@ -233,7 +234,7 @@ def _filter_unstarted_categories(category_map, course): ...@@ -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"]) log.debug(u"Filtering out:%s with start_date: %s", child, unfiltered_map["entries"][child]["start_date"])
else: else:
if course.self_paced or unfiltered_map["subcategories"][child]["start_date"] < now: 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] = {} filtered_map["subcategories"][child] = {}
unfiltered_queue.append(unfiltered_map["subcategories"][child]) unfiltered_queue.append(unfiltered_map["subcategories"][child])
filtered_queue.append(filtered_map["subcategories"][child]) filtered_queue.append(filtered_map["subcategories"][child])
...@@ -249,11 +250,11 @@ def _sort_map_entries(category_map, sort_alpha): ...@@ -249,11 +250,11 @@ def _sort_map_entries(category_map, sort_alpha):
for title, entry in category_map["entries"].items(): for title, entry in category_map["entries"].items():
if entry["sort_key"] is None and sort_alpha: if entry["sort_key"] is None and sort_alpha:
entry["sort_key"] = title entry["sort_key"] = title
things.append((title, entry)) things.append((title, entry, TYPE_ENTRY))
for title, category in category_map["subcategories"].items(): 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) _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): 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 ...@@ -276,13 +277,16 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude
>>> "id": "i4x-edx-eiorguegnru-course-foobarbaz" >>> "id": "i4x-edx-eiorguegnru-course-foobarbaz"
>>> } >>> }
>>> }, >>> },
>>> "children": ["General", "Getting Started"], >>> "children": [
>>> ["General", "entry"],
>>> ["Getting Started", "subcategory"]
>>> ],
>>> "subcategories": { >>> "subcategories": {
>>> "Getting Started": { >>> "Getting Started": {
>>> "subcategories": {}, >>> "subcategories": {},
>>> "children": [ >>> "children": [
>>> "Working with Videos", >>> ["Working with Videos", "entry"],
>>> "Videos on edX" >>> ["Videos on edX", "entry"]
>>> ], >>> ],
>>> "entries": { >>> "entries": {
>>> "Working with Videos": { >>> "Working with Videos": {
......
lms/static/js/groups/views/cohort_discussions_course_wide.js
...@@ -33,8 +33,11 @@ ...@@ -33,8 +33,11 @@
entries = courseWideDiscussions.entries, entries = courseWideDiscussions.entries,
children = courseWideDiscussions.children; children = courseWideDiscussions.children;
return HtmlUtils.joinHtml.apply(this, _.map(children, function(name) { return HtmlUtils.joinHtml.apply(this, _.map(children, function(child) {
var entry = entries[name]; // 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({ return subCategoryTemplate({
name: name, name: name,
id: entry.id, id: entry.id,
......
lms/static/js/groups/views/cohort_discussions_inline.js
...@@ -48,9 +48,12 @@ ...@@ -48,9 +48,12 @@
entries = inlineDiscussions.entries, entries = inlineDiscussions.entries,
subcategories = inlineDiscussions.subcategories; subcategories = inlineDiscussions.subcategories;
return HtmlUtils.joinHtml.apply(this, _.map(children, function(name) { return HtmlUtils.joinHtml.apply(this, _.map(children, function(child) {
var htmlSnippet = '', entry; var htmlSnippet = '',
if (entries && _.has(entries, name)) { 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]; entry = entries[name];
htmlSnippet = entryTemplate({ htmlSnippet = entryTemplate({
name: name, name: name,
......
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 ...@@ -89,7 +89,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
createMockCohortDiscussionsJson = function(allCohorted) { createMockCohortDiscussionsJson = function(allCohorted) {
return { return {
course_wide_discussions: { course_wide_discussions: {
children: ['Topic_C_1', 'Topic_C_2'], children: [['Topic_C_1', 'entry'], ['Topic_C_2', 'entry']],
entries: { entries: {
Topic_C_1: { Topic_C_1: {
sort_key: null, sort_key: null,
...@@ -107,7 +107,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers ...@@ -107,7 +107,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
subcategories: { subcategories: {
Topic_I_1: { Topic_I_1: {
subcategories: {}, subcategories: {},
children: ['Inline_Discussion_1', 'Inline_Discussion_2'], children: [['Inline_Discussion_1', 'entry'], ['Inline_Discussion_2', 'entry']],
entries: { entries: {
Inline_Discussion_1: { Inline_Discussion_1: {
sort_key: null, sort_key: null,
...@@ -122,7 +122,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers ...@@ -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 ...@@ -1488,7 +1488,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
topicsJson = { topicsJson = {
course_wide_discussions: { course_wide_discussions: {
children: ['Topic_C_1'], children: [['Topic_C_1', 'entry']],
entries: { entries: {
Topic_C_1: { Topic_C_1: {
sort_key: null, sort_key: null,
......
lms/templates/discussion/_filter_dropdown.html
<%page expression_filter="h"/> <%page expression_filter="h"/>
<%! <%!
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from lms.djangoapps.django_comment_client.constants import TYPE_ENTRY
from openedx.core.djangolib.markup import HTML from openedx.core.djangolib.markup import HTML
%> %>
<%def name="render_dropdown(map)"> <%def name="render_dropdown(map)">
% for child in map["children"]: % for child, c_type in map["children"]:
% if child in map["entries"]: % if child in map["entries"] and c_type == TYPE_ENTRY:
${HTML(render_entry(map["entries"], child))} ${HTML(render_entry(map["entries"], child))}
%else: %else:
${HTML(render_category(map["subcategories"], child))} ${HTML(render_category(map["subcategories"], child))}
......
openedx/core/djangoapps/course_groups/tests/test_views.py
...@@ -21,6 +21,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase ...@@ -21,6 +21,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore.tests.factories import ItemFactory 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 ..models import CourseUserGroup, CourseCohort
from ..views import ( from ..views import (
...@@ -1229,7 +1230,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): ...@@ -1229,7 +1230,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase):
start_date = response['inline_discussions']['subcategories']['Chapter']['start_date'] start_date = response['inline_discussions']['subcategories']['Chapter']['start_date']
expected_response = { expected_response = {
"course_wide_discussions": { "course_wide_discussions": {
'children': ['Topic B'], 'children': [['Topic B', TYPE_ENTRY]],
'entries': { 'entries': {
'Topic B': { 'Topic B': {
'sort_key': 'A', 'sort_key': 'A',
...@@ -1243,7 +1244,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): ...@@ -1243,7 +1244,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase):
'subcategories': { 'subcategories': {
'Chapter': { 'Chapter': {
'subcategories': {}, 'subcategories': {},
'children': ['Discussion'], 'children': [['Discussion', TYPE_ENTRY]],
'entries': { 'entries': {
'Discussion': { 'Discussion': {
'sort_key': None, 'sort_key': None,
...@@ -1256,7 +1257,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): ...@@ -1256,7 +1257,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase):
'start_date': start_date 'start_date': start_date
} }
}, },
'children': ['Chapter'] 'children': [['Chapter', TYPE_SUBCATEGORY]]
} }
} }
self.assertEqual(response, expected_response) self.assertEqual(response, expected_response)
openedx/core/djangoapps/course_groups/views.py
...@@ -24,6 +24,7 @@ from edxmako.shortcuts import render_to_response ...@@ -24,6 +24,7 @@ from edxmako.shortcuts import render_to_response
from . import cohorts 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.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 from .models import CourseUserGroup, CourseUserGroupPartitionGroup, CohortMembership
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -437,15 +438,15 @@ def cohort_discussion_topics(request, course_key_string): ...@@ -437,15 +438,15 @@ def cohort_discussion_topics(request, course_key_string):
>>> "id": "i4x-edx-eiorguegnru-course-foobarbaz" >>> "id": "i4x-edx-eiorguegnru-course-foobarbaz"
>>> } >>> }
>>> } >>> }
>>> "children": ["General"] >>> "children": ["General", "entry"]
>>> }, >>> },
>>> "inline_discussions" : { >>> "inline_discussions" : {
>>> "subcategories": { >>> "subcategories": {
>>> "Getting Started": { >>> "Getting Started": {
>>> "subcategories": {}, >>> "subcategories": {},
>>> "children": [ >>> "children": [
>>> "Working with Videos", >>> ["Working with Videos", "entry"],
>>> "Videos on edX" >>> ["Videos on edX", "entry"]
>>> ], >>> ],
>>> "entries": { >>> "entries": {
>>> "Working with Videos": { >>> "Working with Videos": {
...@@ -460,7 +461,7 @@ def cohort_discussion_topics(request, course_key_string): ...@@ -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): ...@@ -479,11 +480,11 @@ def cohort_discussion_topics(request, course_key_string):
course_wide_children = [] course_wide_children = []
inline_children = [] inline_children = []
for name in discussion_category_map['children']: for name, c_type in discussion_category_map['children']:
if name in course_wide_entries: if name in course_wide_entries and c_type == TYPE_ENTRY:
course_wide_children.append(name) course_wide_children.append([name, c_type])
else: else:
inline_children.append(name) inline_children.append([name, c_type])
discussion_topics['course_wide_discussions'] = { discussion_topics['course_wide_discussions'] = {
'entries': course_wide_entries, 'entries': course_wide_entries,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment