diff --git a/common/static/common/js/discussion/discussion.js b/common/static/common/js/discussion/discussion.js index 4ef2949f89..bdd9f25956 100644 --- a/common/static/common/js/discussion/discussion.js +++ b/common/static/common/js/discussion/discussion.js @@ -36,7 +36,7 @@ this.pages = options.pages || 1; this.current_page = 1; this.sort_preference = options.sort; - this.is_commentable_cohorted = options.is_commentable_cohorted; + this.is_commentable_divided = options.is_commentable_divided; this.bind('add', function(item) { item.discussion = self; }); @@ -142,7 +142,7 @@ Content.loadContentInfos(response.annotated_content_info); self.pages = response.num_pages; self.current_page = response.page; - self.is_commentable_cohorted = response.is_commentable_cohorted; + self.is_commentable_divided = response.is_commentable_divided; return self.reset(new_collection); }, error: error diff --git a/common/static/common/js/discussion/views/discussion_inline_view.js b/common/static/common/js/discussion/views/discussion_inline_view.js index 6b36fbe2b3..fe9fdc3529 100644 --- a/common/static/common/js/discussion/views/discussion_inline_view.js +++ b/common/static/common/js/discussion/views/discussion_inline_view.js @@ -86,7 +86,7 @@ DiscussionUtil.loadRoles(response.roles); this.courseSettings = new DiscussionCourseSettings(response.course_settings); - this.is_commentable_cohorted = response.is_commentable_cohorted; + this.is_commentable_divided = response.is_commentable_divided; this.discussion = new Discussion(undefined, {pages: response.num_pages}); this.discussion.reset(response.discussion_data, { @@ -126,7 +126,7 @@ course_settings: this.courseSettings, topicId: discussionId, startHeader: this.startHeader, - is_commentable_cohorted: response.is_commentable_cohorted + is_commentable_divided: response.is_commentable_divided }); this.newPostView.render(); @@ -154,7 +154,7 @@ mode: 'inline', startHeader: this.startHeader, courseSettings: this.courseSettings, - is_commentable_cohorted: this.is_commentable_cohorted + is_commentable_divided: this.is_commentable_divided }); this.threadView.render(); this.listenTo(this.threadView.showView, 'thread:_delete', this.navigateToAllPosts); diff --git a/common/static/common/js/discussion/views/discussion_thread_show_view.js b/common/static/common/js/discussion/views/discussion_thread_show_view.js index 1f453ca577..787a5dba2b 100644 --- a/common/static/common/js/discussion/views/discussion_thread_show_view.js +++ b/common/static/common/js/discussion/views/discussion_thread_show_view.js @@ -31,7 +31,7 @@ DiscussionThreadShowView.__super__.initialize.call(this); this.mode = options.mode || 'inline'; this.startHeader = options.startHeader; - this.is_commentable_cohorted = options.is_commentable_cohorted; + this.is_commentable_divided = options.is_commentable_divided; if ((_ref = this.mode) !== 'tab' && _ref !== 'inline') { throw new Error('invalid mode: ' + this.mode); } @@ -42,7 +42,7 @@ mode: this.mode, startHeader: this.startHeader, flagged: this.model.isFlagged(), - is_commentable_cohorted: this.is_commentable_cohorted, + is_commentable_divided: this.is_commentable_divided, author_display: this.getAuthorDisplay(), cid: this.model.cid, readOnly: $('.discussion-module').data('read-only') diff --git a/common/static/common/js/discussion/views/discussion_thread_view.js b/common/static/common/js/discussion/views/discussion_thread_view.js index 8936643e6f..11405cd0ff 100644 --- a/common/static/common/js/discussion/views/discussion_thread_view.js +++ b/common/static/common/js/discussion/views/discussion_thread_view.js @@ -92,7 +92,7 @@ self.model = collection.get(id); } }); - this.is_commentable_cohorted = options.is_commentable_cohorted; + this.is_commentable_divided = options.is_commentable_divided; this.createShowView(); this.responses = new Comments(); this.loadedResponses = false; @@ -423,7 +423,7 @@ model: this.model, mode: this.mode, startHeader: this.startHeader, - is_commentable_cohorted: this.is_commentable_cohorted + is_commentable_divided: this.is_commentable_divided }); this.showView.bind('thread:_delete', this._delete); return this.showView.bind('thread:edit', this.edit); 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 a98b5b91c7..9e6797d322 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 @@ -55,7 +55,7 @@ html = entryTemplate({ text: name, id: entry.id, - is_cohorted: entry.is_cohorted + is_divided: entry.is_divided }); } else { // subcategory html = categoryTemplate({ diff --git a/common/static/common/js/discussion/views/new_post_view.js b/common/static/common/js/discussion/views/new_post_view.js index c60146b7f3..cfb851808a 100644 --- a/common/static/common/js/discussion/views/new_post_view.js +++ b/common/static/common/js/discussion/views/new_post_view.js @@ -41,7 +41,7 @@ throw new Error('invalid mode: ' + this.mode); } this.course_settings = options.course_settings; - this.is_commentable_cohorted = options.is_commentable_cohorted; + this.is_commentable_divided = options.is_commentable_divided; this.topicId = options.topicId; this.discussionBoardView = options.discussionBoardView; }; @@ -52,7 +52,7 @@ context = _.clone(this.course_settings.attributes); _.extend(context, { cohort_options: this.getCohortOptions(), - is_commentable_cohorted: this.is_commentable_cohorted, + is_commentable_divided: this.is_commentable_divided, mode: this.mode, startHeader: this.startHeader, form_id: this.mode + (this.topicId ? '-' + this.topicId : '') @@ -85,14 +85,14 @@ }; NewPostView.prototype.getCohortOptions = function() { - var userCohortId; + var userGroupId; if (this.course_settings.get('is_cohorted') && DiscussionUtil.isPrivilegedUser()) { - userCohortId = $('#discussion-container').data('user-cohort-id'); + userGroupId = $('#discussion-container').data('user-group-id'); return _.map(this.course_settings.get('cohorts'), function(cohort) { return { value: cohort.id, text: cohort.name, - selected: cohort.id === userCohortId + selected: cohort.id === userGroupId }; }); } else { 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 6b3c3d2921..ef1f711067 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 @@ -96,29 +96,29 @@ describe('renderComments', function() { beforeEach(function() { this.course_settings = new DiscussionCourseSettings({ - 'category_map': { - 'children': [ // eslint-disable-line quote-props + category_map: { + children: [ ['Topic', 'entry'], ['General', 'entry'], ['Basic Question', 'entry'] ], - 'entries': { - 'Topic': { - 'is_cohorted': true, - 'id': 'topic' + entries: { + Topic: { + is_divided: true, + id: 'topic' }, - 'General': { - 'sort_key': 'General', - 'is_cohorted': false, - 'id': '6.00.1x_General' + General: { + sort_key: 'General', + is_divided: false, + id: '6.00.1x_General' }, 'Basic Question': { - 'is_cohorted': false, - 'id': "6>00'1x\"Basic_Question" + is_divided: false, + id: "6>00'1x\"Basic_Question" } } }, - 'is_cohorted': true + is_cohorted: true }); }); diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_show_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_show_view_spec.js index 9ba9dd5d5f..ce603770a6 100644 --- a/common/static/common/js/spec/discussion/view/discussion_thread_show_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_thread_show_view_spec.js @@ -173,23 +173,23 @@ }); }); describe('cohorting', function() { - it('renders correctly for an uncohorted thread', function() { + it('renders correctly for a unified thread', function() { this.view.render(); return expect(this.view.$('.group-visibility-label').text().trim()) .toEqual('This post is visible to everyone.'); }); - it('renders correctly for a cohorted thread', function() { + it('renders correctly for a divided thread', function() { this.thread.set('group_id', '1'); this.thread.set('group_name', 'Mock Cohort'); - this.view.is_commentable_cohorted = true; + this.view.is_commentable_divided = true; this.view.render(); return expect(this.view.$('.group-visibility-label').text().trim()) .toEqual('This post is visible only to Mock Cohort.'); }); - it('renders correctly for a grouped uncohorted thread', function() { + it('renders correctly for a grouped unified thread', function() { this.thread.set('group_id', '1'); this.thread.set('group_name', 'Mock Cohort'); - this.view.is_commentable_cohorted = false; + this.view.is_commentable_divided = false; this.view.render(); return expect(this.view.$('.group-visibility-label').text().trim()) .toEqual('This post is visible to everyone.'); 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 4c9a3e798a..eaaec587e8 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 @@ -16,67 +16,67 @@ DiscussionSpecHelper.setUpGlobals(); DiscussionSpecHelper.setUnderscoreFixtures(); this.course_settings = new DiscussionCourseSettings({ - 'category_map': { - 'subcategories': { + category_map: { + subcategories: { 'Basic Question Types': { - 'subcategories': {}, - 'children': [ + subcategories: {}, + children: [ ['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': { + entries: { 'Selection From Options': { - 'sort_key': null, - 'is_cohorted': true, - 'id': 'cba3e4cd91d0466b9ac50926e495b76f' + sort_key: null, + is_divided: true, + id: 'cba3e4cd91d0466b9ac50926e495b76f' }, 'Numerical Input': { - 'sort_key': null, - 'is_cohorted': false, - 'id': 'c49f0dfb8fc94c9c8d9999cc95190c56' + sort_key: null, + is_divided: false, + id: 'c49f0dfb8fc94c9c8d9999cc95190c56' }, 'Very long category name': { - 'sort_key': null, - 'is_cohorted': false, - 'id': 'c49f0dfb8fc94c9c8d9999cc95190c59' + sort_key: null, + is_divided: false, + id: 'c49f0dfb8fc94c9c8d9999cc95190c59' }, 'Very very very very long category name': { - 'sort_key': null, - 'is_cohorted': false, - 'id': 'c49f0dfb8fc94c9c8d9999cc95190e32' + sort_key: null, + is_divided: false, + id: 'c49f0dfb8fc94c9c8d9999cc95190e32' }, 'Name with HTML': { - 'sort_key': null, - 'is_cohorted': false, - 'id': 'c49f0dfb8fc94c9c8d9999cc95190363' + sort_key: null, + is_divided: false, + id: 'c49f0dfb8fc94c9c8d9999cc95190363' } } }, 'Example Inline Discussion': { - 'subcategories': {}, - 'children': [ + subcategories: {}, + children: [ ['What Are Your Goals for Creating a MOOC?', 'entry'] ], - 'entries': { + entries: { 'What Are Your Goals for Creating a MOOC?': { - 'sort_key': null, - 'is_cohorted': true, - 'id': 'cba3e4cd91d0466b9ac50926e495b931' + sort_key: null, + is_divided: true, + id: 'cba3e4cd91d0466b9ac50926e495b931' } } } }, - 'children': [ // eslint-disable-line quote-props + children: [ ['Basic Question Types', 'subcategory'], ['Example Inline Discussion', 'subcategory'] ], - 'entries': {} + entries: {} }, - 'is_cohorted': true + 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 ea5653cf18..1d109de321 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 @@ -38,15 +38,15 @@ children: [['Topic', 'entry'], ['General', 'entry'], ['Not Cohorted', 'entry']], entries: { Topic: { - is_cohorted: true, + is_divided: true, id: 'topic' }, General: { - is_cohorted: true, + is_divided: true, id: 'general' }, 'Not Cohorted': { - is_cohorted: false, + is_divided: false, id: 'not-cohorted' } } @@ -145,20 +145,20 @@ }); it('disables the cohort menu if it is set false', function() { DiscussionSpecHelper.makeModerator(); - this.view.is_commentable_cohorted = false; + this.view.is_commentable_divided = false; return checkVisibility(this.view, true, true, true); }); it('enables the cohort menu if it is set true', function() { DiscussionSpecHelper.makeModerator(); - this.view.is_commentable_cohorted = true; + this.view.is_commentable_divided = true; return checkVisibility(this.view, true, false, true); }); it('is not visible to students when set false', function() { - this.view.is_commentable_cohorted = false; + this.view.is_commentable_divided = false; return checkVisibility(this.view, false, false, true); }); it('is not visible to students when set true', function() { - this.view.is_commentable_cohorted = true; + this.view.is_commentable_divided = true; return checkVisibility(this.view, false, false, true); }); }); @@ -166,33 +166,33 @@ var checkPostCancelReset; beforeEach(function() { this.course_settings = new DiscussionCourseSettings({ - 'allow_anonymous_to_peers': true, - 'allow_anonymous': true, - 'category_map': { - 'subcategories': { + allow_anonymous_to_peers: true, + allow_anonymous: true, + category_map: { + subcategories: { 'Week 1': { - 'subcategories': {}, - 'children': [ // eslint-disable-line quote-props + subcategories: {}, + children: [ ['Topic-Level Student-Visible Label', 'entry'] ], - 'entries': { + entries: { 'Topic-Level Student-Visible Label': { - 'sort_key': null, - 'is_cohorted': false, - 'id': '2b3a858d0c884eb4b272dbbe3f2ffddd' + sort_key: null, + is_divided: false, + id: '2b3a858d0c884eb4b272dbbe3f2ffddd' } } } }, - 'children': [ // eslint-disable-line quote-props + children: [ ['General', 'entry'], ['Week 1', 'subcategory'] ], - 'entries': { - 'General': { - 'sort_key': 'General', - 'is_cohorted': false, - 'id': 'i4x-waqastest-waqastest-course-waqastest' + entries: { + General: { + sort_key: 'General', + is_divided: false, + id: 'i4x-waqastest-waqastest-course-waqastest' } } } @@ -255,7 +255,7 @@ entries: { 'Topic-Level Student-Visible Label': { sort_key: null, - is_cohorted: false, + is_divided: false, id: '2b3a858d0c884eb4b272dbbe3f2ffddd' } } @@ -268,7 +268,7 @@ entries: { 'First topic': { sort_key: 'First topic', - is_cohorted: false, + is_divided: false, id: 'i4x-waqastest-waqastest-course-waqastest' } } 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 7a05b8d8ff..a5c7c48082 100644 --- a/common/static/common/js/spec_helpers/discussion_spec_helper.js +++ b/common/static/common/js/spec_helpers/discussion_spec_helper.js @@ -58,11 +58,11 @@ children: [['Test Topic', 'entry'], ['Other Topic', 'entry']], entries: { 'Test Topic': { - is_cohorted: true, + is_divided: true, id: 'test_topic' }, 'Other Topic': { - is_cohorted: true, + is_divided: true, id: 'other_topic' } } diff --git a/common/static/common/templates/discussion/new-post-menu-entry.underscore b/common/static/common/templates/discussion/new-post-menu-entry.underscore index d3f300f94c..854a95ab56 100644 --- a/common/static/common/templates/discussion/new-post-menu-entry.underscore +++ b/common/static/common/templates/discussion/new-post-menu-entry.underscore @@ -1 +1 @@ - + diff --git a/common/static/common/templates/discussion/new-post.underscore b/common/static/common/templates/discussion/new-post.underscore index 1376a2111c..aaa28611ed 100644 --- a/common/static/common/templates/discussion/new-post.underscore +++ b/common/static/common/templates/discussion/new-post.underscore @@ -10,7 +10,7 @@
<% if (cohort_options) { %> -
+
- > <% _.each(cohort_options, function(opt) { %> diff --git a/common/static/common/templates/discussion/thread-show.underscore b/common/static/common/templates/discussion/thread-show.underscore index 3caf6cbccd..faad477b13 100644 --- a/common/static/common/templates/discussion/thread-show.underscore +++ b/common/static/common/templates/discussion/thread-show.underscore @@ -70,7 +70,7 @@ %> <% } %>
- <% if (obj.group_name && is_commentable_cohorted) { %> + <% if (obj.group_name && is_commentable_divided) { %> <%- interpolate( gettext('This post is visible only to %(group_name)s.'), diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js index 0f8d313da6..467678b5b6 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_board_factory.js @@ -21,7 +21,7 @@ sortPreference = options.sortPreference, threads = options.threads, threadPages = options.threadPages, - isCommentableCohorted = options.isCommentableCohorted, + isCommentableDivided = options.isCommentableDivided, contentInfo = options.contentInfo, user = new DiscussionUser(userInfo), discussion, @@ -41,7 +41,7 @@ // Create a discussion model discussion = new Discussion(threads, {pages: threadPages, sort: sortPreference, - is_commentable_cohorted: isCommentableCohorted}); + is_commentable_divided: isCommentableDivided}); courseSettings = new DiscussionCourseSettings(options.courseSettings); // Create the discussion board view diff --git a/lms/djangoapps/discussion/static/discussion/js/discussion_router.js b/lms/djangoapps/discussion/static/discussion/js/discussion_router.js index d96258160a..353ff0de03 100644 --- a/lms/djangoapps/discussion/static/discussion/js/discussion_router.js +++ b/lms/djangoapps/discussion/static/discussion/js/discussion_router.js @@ -103,7 +103,7 @@ mode: 'tab', startHeader: this.startHeader, courseSettings: this.courseSettings, - is_commentable_cohorted: this.discussion.is_commentable_cohorted + is_commentable_divided: this.discussion.is_commentable_divided }); this.main.render(); this.main.on('thread:responses:rendered', function() { diff --git a/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html b/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html index 6382f5c816..f8f71ef129 100644 --- a/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html +++ b/lms/djangoapps/discussion/templates/discussion/discussion_board_fragment.html @@ -22,7 +22,7 @@ from openedx.core.djangolib.markup import HTML data-read-only="false" data-sort-preference="${sort_preference}" data-flag-moderator="${json.dumps(flag_moderator)}" - data-user-cohort-id="${user_cohort}"> + data-user-group-id="${user_group_id}">
diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index 9a15286f7c..3439b78328 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -32,7 +32,6 @@ from courseware.courses import get_course_with_access from courseware.views.views import CourseTabView from openedx.core.djangoapps.course_groups.cohorts import ( is_course_cohorted, - get_cohort_id, get_course_cohorts, ) from openedx.core.djangoapps.plugin_api.views import EdxFragmentView @@ -49,8 +48,10 @@ from django_comment_client.utils import ( strip_none, add_courseware_context, get_group_id_for_comments_service, - is_commentable_cohorted + is_commentable_divided, + get_group_id_for_user, ) + import django_comment_client.utils as utils import lms.lib.comment_client as cc @@ -222,7 +223,7 @@ def inline_discussion(request, course_key, discussion_id): add_courseware_context(threads, course, request.user) return utils.JsonResponse({ - 'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id), + 'is_commentable_divided': is_commentable_divided(course_key, discussion_id), 'discussion_data': threads, 'user_info': user_info, 'annotated_content_info': annotated_content_info, @@ -347,8 +348,8 @@ def _find_thread(request, course, discussion_id, thread_id): # verify that the thread belongs to the requesting student's cohort is_moderator = has_permission(request.user, "see_all_cohorts", course.id) - if is_commentable_cohorted(course.id, discussion_id) and not is_moderator: - user_group_id = get_cohort_id(request.user, course.id) + if is_commentable_divided(course.id, discussion_id) and not is_moderator: + user_group_id = get_group_id_for_user(request.user, course.id) if getattr(thread, "group_id", None) is not None and user_group_id != thread.group_id: return None @@ -423,7 +424,7 @@ def _create_discussion_board_context(request, course_key, discussion_id=None, th add_courseware_context(threads, course, user) with newrelic_function_trace("get_cohort_info"): - user_cohort_id = get_cohort_id(user, course_key) + user_group_id = get_group_id_for_user(user, course_key) context.update({ 'root_url': root_url, @@ -434,11 +435,11 @@ def _create_discussion_board_context(request, course_key, discussion_id=None, th 'annotated_content_info': annotated_content_info, 'is_moderator': has_permission(user, "see_all_cohorts", course_key), 'cohorts': course_settings["cohorts"], # still needed to render _thread_list_template - 'user_cohort': user_cohort_id, # read from container in NewPostView + 'user_group_id': user_group_id, # read from container in NewPostView 'sort_preference': cc_user.default_sort_key, 'category_map': course_settings["category_map"], 'course_settings': course_settings, - 'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id) + 'is_commentable_divided': is_commentable_divided(course_key, discussion_id) }) return context @@ -500,7 +501,7 @@ def user_profile(request, course_key, user_id): ).order_by("name").values_list("name", flat=True).distinct() with newrelic_function_trace("get_cohort_info"): - user_cohort_id = get_cohort_id(request.user, course_key) + user_group_id = get_group_id_for_user(request.user, course_key) context = _create_base_discussion_view_context(request, course_key) context.update({ @@ -508,7 +509,7 @@ def user_profile(request, course_key, user_id): 'django_user_roles': user_roles, 'profiled_user': profiled_user.to_dict(), 'threads': threads, - 'user_cohort': user_cohort_id, + 'user_group_id': user_group_id, 'annotated_content_info': annotated_content_info, 'page': query_params['page'], 'num_pages': query_params['num_pages'], diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 587eb66a2b..488ba21a6b 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -43,12 +43,11 @@ from django_comment_common.signals import ( comment_voted, comment_deleted, ) -from django_comment_client.utils import get_accessible_discussion_xblocks, is_commentable_cohorted +from django_comment_client.utils import get_accessible_discussion_xblocks, is_commentable_divided, get_group_id_for_user from lms.djangoapps.discussion_api.pagination import DiscussionAPIPagination from lms.lib.comment_client.comment import Comment from lms.lib.comment_client.thread import Thread from lms.lib.comment_client.utils import CommentClientRequestError -from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id from openedx.core.lib.exceptions import CourseNotFoundError, PageNotFoundError, DiscussionNotFoundError @@ -113,10 +112,10 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None): if ( not context["is_requester_privileged"] and cc_thread["group_id"] and - is_commentable_cohorted(course.id, cc_thread["commentable_id"]) + is_commentable_divided(course.id, cc_thread["commentable_id"]) ): - requester_cohort = get_cohort_id(request.user, course.id) - if requester_cohort is not None and cc_thread["group_id"] != requester_cohort: + requester_group_id = get_group_id_for_user(request.user, course.id) + if requester_group_id is not None and cc_thread["group_id"] != requester_group_id: raise ThreadNotFoundError("Thread not found.") return cc_thread, context except CommentClientRequestError: @@ -547,7 +546,7 @@ def get_thread_list( "user_id": unicode(request.user.id), "group_id": ( None if context["is_requester_privileged"] else - get_cohort_id(request.user, course.id) + get_group_id_for_user(request.user, course.id) ), "page": page, "per_page": page_size, @@ -831,10 +830,10 @@ def create_thread(request, thread_data): _check_initializable_thread_fields(thread_data, context) if ( "group_id" not in thread_data and - is_commentable_cohorted(course_key, thread_data.get("topic_id")) + is_commentable_divided(course_key, thread_data.get("topic_id")) ): thread_data = thread_data.copy() - thread_data["group_id"] = get_cohort_id(user, course_key) + thread_data["group_id"] = get_group_id_for_user(user, course_key) serializer = ThreadSerializer(data=thread_data, context=context) actions_form = ThreadActionsForm(thread_data) if not (serializer.is_valid() and actions_form.is_valid()): diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 1bc658553f..2a5e1d09d1 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -241,11 +241,11 @@ def create_thread(request, course_id, commentable_id): thread = cc.Thread(**params) - # Cohort the thread if required + # Divide the thread if required try: group_id = get_group_id_for_comments_service(request, course_key, commentable_id) except ValueError: - return HttpResponseServerError("Invalid cohort id") + return HttpResponseServerError("Invalid group id for commentable") if group_id is not None: thread.group_id = group_id diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 1316217340..b3c6a06029 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -390,12 +390,14 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): **kwargs ) - def assert_category_map_equals(self, expected, cohorted_if_in_list=False, exclude_unstarted=True): # pylint: disable=arguments-differ + def assert_category_map_equals(self, expected, divided_only_if_explicit=False, exclude_unstarted=True): # pylint: disable=arguments-differ """ Asserts the expected map with the map returned by get_discussion_category_map method. """ self.assertEqual( - utils.get_discussion_category_map(self.course, self.instructor, cohorted_if_in_list, exclude_unstarted), + utils.get_discussion_category_map( + self.course, self.instructor, divided_only_if_explicit, exclude_unstarted + ), expected ) @@ -413,9 +415,9 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): self.assert_category_map_equals( { "entries": { - "Topic A": {"id": "Topic_A", "sort_key": "Topic A", "is_cohorted": "Topic_A" in expected_ids}, - "Topic B": {"id": "Topic_B", "sort_key": "Topic B", "is_cohorted": "Topic_B" in expected_ids}, - "Topic C": {"id": "Topic_C", "sort_key": "Topic C", "is_cohorted": "Topic_C" in expected_ids}, + "Topic A": {"id": "Topic_A", "sort_key": "Topic A", "is_divided": "Topic_A" in expected_ids}, + "Topic B": {"id": "Topic_B", "sort_key": "Topic B", "is_divided": "Topic_B" in expected_ids}, + "Topic C": {"id": "Topic_C", "sort_key": "Topic C", "is_divided": "Topic_C" in expected_ids}, }, "subcategories": {}, "children": [("Topic A", TYPE_ENTRY), ("Topic B", TYPE_ENTRY), ("Topic C", TYPE_ENTRY)] @@ -466,7 +468,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion1", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -490,7 +492,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion1", "sort_key": None, - "is_cohorted": True, + "is_divided": True, } }, "subcategories": {}, @@ -514,7 +516,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion1", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -523,7 +525,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): }, "children": [("Chapter", TYPE_SUBCATEGORY)] }, - cohorted_if_in_list=True + divided_only_if_explicit=True ) def test_get_unstarted_discussion_xblocks(self): @@ -540,7 +542,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion1", "sort_key": None, - "is_cohorted": False, + "is_divided": False, "start_date": later } }, @@ -552,7 +554,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): }, "children": [("Chapter 1", TYPE_SUBCATEGORY)] }, - cohorted_if_in_list=True, + divided_only_if_explicit=True, exclude_unstarted=False ) @@ -564,7 +566,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion") self.create_discussion("Chapter 3 / Section 1", "Discussion") - def check_cohorted(is_cohorted): + def check_cohorted(is_divided): self.assert_category_map_equals( { @@ -575,12 +577,12 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion1", "sort_key": None, - "is_cohorted": is_cohorted, + "is_divided": is_divided, }, "Discussion 2": { "id": "discussion2", "sort_key": None, - "is_cohorted": is_cohorted, + "is_divided": is_divided, } }, "subcategories": {}, @@ -591,7 +593,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion3", "sort_key": None, - "is_cohorted": is_cohorted, + "is_divided": is_divided, } }, "subcategories": { @@ -603,7 +605,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion4", "sort_key": None, - "is_cohorted": is_cohorted, + "is_divided": is_divided, } }, "subcategories": {}, @@ -614,7 +616,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion5", "sort_key": None, - "is_cohorted": is_cohorted, + "is_divided": is_divided, } }, "subcategories": {}, @@ -634,7 +636,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion6", "sort_key": None, - "is_cohorted": is_cohorted, + "is_divided": is_divided, } }, "subcategories": {}, @@ -704,7 +706,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion1", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -715,7 +717,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion3", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -749,12 +751,12 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion1", "sort_key": None, - "is_cohorted": False, + "is_divided": False, }, "Discussion 2": { "id": "discussion2", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -765,7 +767,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion3", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": { @@ -777,7 +779,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion4", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -788,7 +790,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion5", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -808,7 +810,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion6", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -839,27 +841,27 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion1", "sort_key": "D", - "is_cohorted": False, + "is_divided": False, }, "Discussion 2": { "id": "discussion2", "sort_key": "A", - "is_cohorted": False, + "is_divided": False, }, "Discussion 3": { "id": "discussion3", "sort_key": "E", - "is_cohorted": False, + "is_divided": False, }, "Discussion 4": { "id": "discussion4", "sort_key": "C", - "is_cohorted": False, + "is_divided": False, }, "Discussion 5": { "id": "discussion5", "sort_key": "B", - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -885,9 +887,9 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): self.assert_category_map_equals( { "entries": { - "Topic A": {"id": "Topic_A", "sort_key": "B", "is_cohorted": False}, - "Topic B": {"id": "Topic_B", "sort_key": "C", "is_cohorted": False}, - "Topic C": {"id": "Topic_C", "sort_key": "A", "is_cohorted": False}, + "Topic A": {"id": "Topic_A", "sort_key": "B", "is_divided": False}, + "Topic B": {"id": "Topic_B", "sort_key": "C", "is_divided": False}, + "Topic C": {"id": "Topic_C", "sort_key": "A", "is_divided": False}, }, "subcategories": {}, "children": [("Topic C", TYPE_ENTRY), ("Topic A", TYPE_ENTRY), ("Topic B", TYPE_ENTRY)] @@ -912,27 +914,27 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion D": { "id": "discussion1", "sort_key": "Discussion D", - "is_cohorted": False, + "is_divided": False, }, "Discussion A": { "id": "discussion2", "sort_key": "Discussion A", - "is_cohorted": False, + "is_divided": False, }, "Discussion E": { "id": "discussion3", "sort_key": "Discussion E", - "is_cohorted": False, + "is_divided": False, }, "Discussion C": { "id": "discussion4", "sort_key": "Discussion C", - "is_cohorted": False, + "is_divided": False, }, "Discussion B": { "id": "discussion5", "sort_key": "Discussion B", - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -965,12 +967,12 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion3", "sort_key": None, - "is_cohorted": False, + "is_divided": False, }, "Discussion 2": { "id": "discussion5", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -981,12 +983,12 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion 1": { "id": "discussion4", "sort_key": None, - "is_cohorted": False, + "is_divided": False, }, "Discussion 2": { "id": "discussion1", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -997,7 +999,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): "Discussion": { "id": "discussion2", "sort_key": None, - "is_cohorted": False, + "is_divided": False, } }, "subcategories": {}, @@ -1074,17 +1076,17 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'Visible to Alpha': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'alpha_group_discussion' }, 'Visible to Beta': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'beta_group_discussion' }, 'Visible to Everyone': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'global_group_discussion' } } @@ -1094,7 +1096,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'General': { 'sort_key': 'General', - 'is_cohorted': False, + 'is_divided': False, 'id': 'i4x-org-number-course-run' } } @@ -1119,12 +1121,12 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'Visible to Alpha': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'alpha_group_discussion' }, 'Visible to Everyone': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'global_group_discussion' } } @@ -1134,7 +1136,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'General': { 'sort_key': 'General', - 'is_cohorted': False, + 'is_divided': False, 'id': 'i4x-org-number-course-run' } } @@ -1159,12 +1161,12 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'Visible to Beta': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'beta_group_discussion' }, 'Visible to Everyone': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'global_group_discussion' } } @@ -1174,7 +1176,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'General': { 'sort_key': 'General', - 'is_cohorted': False, + 'is_divided': False, 'id': 'i4x-org-number-course-run' } } @@ -1198,7 +1200,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'Visible to Everyone': { 'sort_key': None, - 'is_cohorted': False, + 'is_divided': False, 'id': 'global_group_discussion' } } @@ -1208,7 +1210,7 @@ class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase 'entries': { 'General': { 'sort_key': 'General', - 'is_cohorted': False, + 'is_divided': False, 'id': 'i4x-org-number-course-run' } } @@ -1273,9 +1275,9 @@ class DiscussionTabTestCase(ModuleStoreTestCase): self.assertFalse(self.discussion_tab_present(self.enrolled_user)) -class IsCommentableCohortedTestCase(ModuleStoreTestCase): +class IsCommentableDividedTestCase(ModuleStoreTestCase): """ - Test the is_commentable_cohorted function. + Test the is_commentable_divided function. """ MODULESTORE = TEST_DATA_MIXED_MODULESTORE @@ -1284,10 +1286,10 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): """ Make sure that course is reloaded every time--clear out the modulestore. """ - super(IsCommentableCohortedTestCase, self).setUp() + super(IsCommentableDividedTestCase, self).setUp() self.toy_course_key = ToyCourseFactory.create().id - def test_is_commentable_cohorted(self): + def test_is_commentable_divided(self): course = modulestore().get_course(self.toy_course_key) self.assertFalse(cohorts.is_course_cohorted(course.id)) @@ -1297,7 +1299,7 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): # no topics self.assertFalse( - utils.is_commentable_cohorted(course.id, to_id("General")), + utils.is_commentable_divided(course.id, to_id("General")), "Course doesn't even have a 'General' topic" ) @@ -1305,7 +1307,7 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): config_course_cohorts(course, is_cohorted=False, discussion_topics=["General", "Feedback"]) self.assertFalse( - utils.is_commentable_cohorted(course.id, to_id("General")), + utils.is_commentable_divided(course.id, to_id("General")), "Course isn't cohorted" ) @@ -1314,7 +1316,7 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): self.assertTrue(cohorts.is_course_cohorted(course.id)) self.assertFalse( - utils.is_commentable_cohorted(course.id, to_id("General")), + utils.is_commentable_divided(course.id, to_id("General")), "Course is cohorted, but 'General' isn't." ) @@ -1328,15 +1330,15 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): self.assertTrue(cohorts.is_course_cohorted(course.id)) self.assertFalse( - utils.is_commentable_cohorted(course.id, to_id("General")), + utils.is_commentable_divided(course.id, to_id("General")), "Course is cohorted, but 'General' isn't." ) self.assertTrue( - utils.is_commentable_cohorted(course.id, to_id("Feedback")), + utils.is_commentable_divided(course.id, to_id("Feedback")), "Feedback was listed as cohorted. Should be." ) - def test_is_commentable_cohorted_inline_discussion(self): + def test_is_commentable_divided_inline_discussion(self): course = modulestore().get_course(self.toy_course_key) self.assertFalse(cohorts.is_course_cohorted(course.id)) @@ -1350,7 +1352,7 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): cohorted_discussions=["Feedback", "random_inline"] ) self.assertFalse( - utils.is_commentable_cohorted(course.id, to_id("random")), + utils.is_commentable_divided(course.id, to_id("random")), "By default, Non-top-level discussions are not cohorted in a cohorted courses." ) @@ -1364,20 +1366,20 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): always_cohort_inline_discussions=False ) self.assertFalse( - utils.is_commentable_cohorted(course.id, to_id("random")), + utils.is_commentable_divided(course.id, to_id("random")), "Non-top-level discussion is not cohorted if always_cohort_inline_discussions is False." ) self.assertTrue( - utils.is_commentable_cohorted(course.id, to_id("random_inline")), + utils.is_commentable_divided(course.id, to_id("random_inline")), "If always_cohort_inline_discussions set to False, Non-top-level discussion is " "cohorted if explicitly set in cohorted_discussions." ) self.assertTrue( - utils.is_commentable_cohorted(course.id, to_id("Feedback")), + utils.is_commentable_divided(course.id, to_id("Feedback")), "If always_cohort_inline_discussions set to False, top-level discussion are not affected." ) - def test_is_commentable_cohorted_team(self): + def test_is_commentable_divided_team(self): course = modulestore().get_course(self.toy_course_key) self.assertFalse(cohorts.is_course_cohorted(course.id)) @@ -1386,8 +1388,8 @@ class IsCommentableCohortedTestCase(ModuleStoreTestCase): # Verify that team discussions are not cohorted, but other discussions are # if "always cohort inline discussions" is set to true. - self.assertFalse(utils.is_commentable_cohorted(course.id, team.discussion_topic_id)) - self.assertTrue(utils.is_commentable_cohorted(course.id, "random")) + self.assertFalse(utils.is_commentable_divided(course.id, team.discussion_topic_id)) + self.assertTrue(utils.is_commentable_divided(course.id, "random")) class PermissionsTestCase(ModuleStoreTestCase): diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index cd49c871d1..e2656367ba 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -258,7 +258,7 @@ def _sort_map_entries(category_map, sort_alpha): 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, divided_only_if_explicit=False, exclude_unstarted=True): """ Transform the list of this course's discussion xblocks into a recursive dictionary structure. This is used to render the discussion category map in the discussion tab sidebar for a given user. @@ -266,15 +266,15 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude Args: course: Course for which to get the ids. user: User to check for access. - cohorted_if_in_list (bool): If True, inline topics are marked is_cohorted only if they are - in course_cohort_settings.discussion_topics. + divided_only_if_explicit (bool): If True, inline topics are marked is_divided only if they are + explicitly listed in course_cohort_settings.discussion_topics. Example: >>> example = { >>> "entries": { >>> "General": { >>> "sort_key": "General", - >>> "is_cohorted": True, + >>> "is_divided": True, >>> "id": "i4x-edx-eiorguegnru-course-foobarbaz" >>> } >>> }, @@ -292,12 +292,12 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude >>> "entries": { >>> "Working with Videos": { >>> "sort_key": None, - >>> "is_cohorted": False, + >>> "is_divided": False, >>> "id": "d9f970a42067413cbb633f81cfb12604" >>> }, >>> "Videos on edX": { >>> "sort_key": None, - >>> "is_cohorted": False, + >>> "is_divided": False, >>> "id": "98d8feb5971041a085512ae22b398613" >>> } >>> } @@ -356,14 +356,14 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude if node[level]["start_date"] > category_start_date: node[level]["start_date"] = category_start_date - always_cohort_inline_discussions = ( # pylint: disable=invalid-name - not cohorted_if_in_list and course_cohort_settings.always_cohort_inline_discussions + divide_all_inline_discussions = ( # pylint: disable=invalid-name + not divided_only_if_explicit and course_cohort_settings.always_cohort_inline_discussions ) dupe_counters = defaultdict(lambda: 0) # counts the number of times we see each title for entry in entries: - is_entry_cohorted = ( + is_entry_divided = ( course_cohort_settings.is_cohorted and ( - always_cohort_inline_discussions or entry["id"] in course_cohort_settings.cohorted_discussions + divide_all_inline_discussions or entry["id"] in course_cohort_settings.cohorted_discussions ) ) @@ -376,7 +376,7 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude node[level]["entries"][title] = {"id": entry["id"], "sort_key": entry["sort_key"], "start_date": entry["start_date"], - "is_cohorted": is_entry_cohorted} + "is_divided": is_entry_divided} # 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 @@ -386,8 +386,9 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude "id": entry["id"], "sort_key": entry.get("sort_key", topic), "start_date": datetime.now(UTC()), - "is_cohorted": (course_cohort_settings.is_cohorted and - entry["id"] in course_cohort_settings.cohorted_discussions) + "is_divided": ( + course_cohort_settings.is_cohorted and entry["id"] in course_cohort_settings.cohorted_discussions + ) } _sort_map_entries(category_map, course.discussion_sort_alpha) @@ -740,7 +741,7 @@ def get_group_id_for_comments_service(request, course_key, commentable_id=None): Raises: ValueError if the requested group_id is invalid """ - if commentable_id is None or is_commentable_cohorted(course_key, commentable_id): + if commentable_id is None or is_commentable_divided(course_key, commentable_id): if request.method == "GET": requested_group_id = request.GET.get('group_id') elif request.method == "POST": @@ -755,7 +756,7 @@ def get_group_id_for_comments_service(request, course_key, commentable_id=None): raise ValueError else: # regular users always query with their own id. - group_id = get_cohort_id(request.user, course_key) + group_id = get_group_id_for_user(request.user, course_key) return group_id else: # Never pass a group_id to the comments service for a non-cohorted @@ -763,6 +764,14 @@ def get_group_id_for_comments_service(request, course_key, commentable_id=None): return None +def get_group_id_for_user(user, course_key): + """ + This method will be modified to consider Enrollment Tracks. + Currently pass-through to get_cohort_id. + """ + return get_cohort_id(user, course_key) + + def is_comment_too_deep(parent): """ Determine whether a comment with the given parent violates MAX_COMMENT_DEPTH @@ -777,14 +786,15 @@ def is_comment_too_deep(parent): ) -def is_commentable_cohorted(course_key, commentable_id): +def is_commentable_divided(course_key, commentable_id): """ Args: course_key: CourseKey commentable_id: string Returns: - Bool: is this commentable cohorted? + Bool: is this commentable divided, meaning that learners are divided into + groups (either Cohorts or Enrollment Tracks) and only see posts within their group? Raises: Http404 if the course doesn't exist. @@ -807,7 +817,7 @@ def is_commentable_cohorted(course_key, commentable_id): # inline discussions are cohorted by default ans = True - log.debug(u"is_commentable_cohorted(%s, %s) = {%s}", course_key, commentable_id, ans) + log.debug(u"is_commentable_divided(%s, %s) = {%s}", course_key, commentable_id, ans) return ans 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 99214d344e..71fdcd750e 100644 --- a/lms/static/js/groups/views/cohort_discussions_course_wide.js +++ b/lms/static/js/groups/views/cohort_discussions_course_wide.js @@ -41,7 +41,7 @@ return subCategoryTemplate({ name: name, id: entry.id, - is_cohorted: entry.is_cohorted, + is_divided: entry.is_divided, type: 'course-wide' }); })); diff --git a/lms/static/js/groups/views/cohort_discussions_inline.js b/lms/static/js/groups/views/cohort_discussions_inline.js index 70d79ef61a..817b07d58f 100644 --- a/lms/static/js/groups/views/cohort_discussions_inline.js +++ b/lms/static/js/groups/views/cohort_discussions_inline.js @@ -58,7 +58,7 @@ htmlSnippet = entryTemplate({ name: name, id: entry.id, - is_cohorted: entry.is_cohorted, + is_divided: entry.is_divided, type: 'inline' }); } else { // subcategory diff --git a/lms/static/js/spec/groups/views/cohorts_spec.js b/lms/static/js/spec/groups/views/cohorts_spec.js index f68b56d664..ba29285340 100644 --- a/lms/static/js/spec/groups/views/cohorts_spec.js +++ b/lms/static/js/spec/groups/views/cohorts_spec.js @@ -93,12 +93,12 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers entries: { Topic_C_1: { sort_key: null, - is_cohorted: true, + is_divided: true, id: 'Topic_C_1' }, Topic_C_2: { sort_key: null, - is_cohorted: false, + is_divided: false, id: 'Topic_C_2' } } @@ -111,12 +111,12 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers entries: { Inline_Discussion_1: { sort_key: null, - is_cohorted: true, + is_divided: true, id: 'Inline_Discussion_1' }, Inline_Discussion_2: { sort_key: null, - is_cohorted: allCohorted || false, + is_divided: allCohorted || false, id: 'Inline_Discussion_2' } } @@ -1492,7 +1492,7 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers entries: { Topic_C_1: { sort_key: null, - is_cohorted: true, + is_divided: true, id: 'Topic_C_1' } } diff --git a/lms/templates/discussion/_filter_dropdown.html b/lms/templates/discussion/_filter_dropdown.html index 4e27d513dc..386ee84565 100644 --- a/lms/templates/discussion/_filter_dropdown.html +++ b/lms/templates/discussion/_filter_dropdown.html @@ -20,7 +20,7 @@ from openedx.core.djangolib.markup import HTML class="forum-nav-browse-menu-item" data-discussion-id='${entries[entry]["id"]}' id='${entries[entry]["id"]}' - data-cohorted="${str(entries[entry]['is_cohorted']).lower()}" + data-cohorted="${str(entries[entry]['is_divided']).lower()}" role="option" > % if entry: diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore index 28b2dffb86..95ed87faf3 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore @@ -1,9 +1,9 @@
  • diff --git a/openedx/core/djangoapps/course_groups/tests/test_views.py b/openedx/core/djangoapps/course_groups/tests/test_views.py index f781a83ce7..835248deeb 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_views.py +++ b/openedx/core/djangoapps/course_groups/tests/test_views.py @@ -1255,7 +1255,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): 'entries': { 'Topic B': { 'sort_key': 'A', - 'is_cohorted': True, + 'is_divided': True, 'id': topic_name_to_id(self.course, "Topic B"), 'start_date': response['course_wide_discussions']['entries']['Topic B']['start_date'] } @@ -1269,7 +1269,7 @@ class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase): 'entries': { 'Discussion': { 'sort_key': None, - 'is_cohorted': True, + 'is_divided': True, 'id': topic_name_to_id(self.course, "Topic A"), 'start_date': start_date } diff --git a/openedx/core/djangoapps/course_groups/views.py b/openedx/core/djangoapps/course_groups/views.py index 6f69ebda90..43aa91b5ad 100644 --- a/openedx/core/djangoapps/course_groups/views.py +++ b/openedx/core/djangoapps/course_groups/views.py @@ -435,7 +435,7 @@ def cohort_discussion_topics(request, course_key_string): >>> "entries": { >>> "General": { >>> "sort_key": "General", - >>> "is_cohorted": True, + >>> "is_divided": True, >>> "id": "i4x-edx-eiorguegnru-course-foobarbaz" >>> } >>> } @@ -452,12 +452,12 @@ def cohort_discussion_topics(request, course_key_string): >>> "entries": { >>> "Working with Videos": { >>> "sort_key": None, - >>> "is_cohorted": False, + >>> "is_divided": False, >>> "id": "d9f970a42067413cbb633f81cfb12604" >>> }, >>> "Videos on edX": { >>> "sort_key": None, - >>> "is_cohorted": False, + >>> "is_divided": False, >>> "id": "98d8feb5971041a085512ae22b398613" >>> } >>> } @@ -472,7 +472,7 @@ def cohort_discussion_topics(request, course_key_string): discussion_topics = {} discussion_category_map = get_discussion_category_map( - course, request.user, cohorted_if_in_list=True, exclude_unstarted=False + course, request.user, divided_only_if_explicit=True, exclude_unstarted=False ) # We extract the data for the course wide discussions from the category map.