Merge pull request #15037 from edx/christina/ed11-simplification

Feature branch: dividing discussions by enrollment tracks

cms/envs/common.py

@@ -239,7 +239,7 @@ FEATURES = {
     'ALLOW_PUBLIC_ACCOUNT_CREATION': True,
 
     # Whether or not the dynamic EnrollmentTrackUserPartition should be registered.
-    'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': False,
+    'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': True,
 }
 
 ENABLE_JASMINE = False

common/djangoapps/django_comment_common/migrations/0005_coursediscussionsettings.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import openedx.core.djangoapps.xmodule_django.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('django_comment_common', '0004_auto_20161117_1209'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='CourseDiscussionSettings',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('course_id', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(help_text=b'Which course are these settings associated with?', unique=True, max_length=255, db_index=True)),
+                ('always_divide_inline_discussions', models.BooleanField(default=False)),
+                ('_divided_discussions', models.TextField(null=True, db_column=b'divided_discussions', blank=True)),
+                ('division_scheme', models.CharField(default=b'none', max_length=20, choices=[(b'none', b'None'), (b'cohort', b'Cohort'), (b'enrollment_track', b'Enrollment Track')])),
+            ],
+        ),
+    ]

common/djangoapps/django_comment_common/models.py

+import json
 import logging
 
 from config_models.models import ConfigurationModel
@@ -162,3 +163,30 @@ class ForumsConfig(ConfigurationModel):
     def __unicode__(self):
         """Simple representation so the admin screen looks less ugly."""
         return u"ForumsConfig: timeout={}".format(self.connection_timeout)
+
+
+class CourseDiscussionSettings(models.Model):
+    course_id = CourseKeyField(
+        unique=True,
+        max_length=255,
+        db_index=True,
+        help_text="Which course are these settings associated with?",
+    )
+    always_divide_inline_discussions = models.BooleanField(default=False)
+    _divided_discussions = models.TextField(db_column='divided_discussions', null=True, blank=True)  # JSON list
+
+    COHORT = 'cohort'
+    ENROLLMENT_TRACK = 'enrollment_track'
+    NONE = 'none'
+    ASSIGNMENT_TYPE_CHOICES = ((NONE, 'None'), (COHORT, 'Cohort'), (ENROLLMENT_TRACK, 'Enrollment Track'))
+    division_scheme = models.CharField(max_length=20, choices=ASSIGNMENT_TYPE_CHOICES, default=NONE)
+
+    @property
+    def divided_discussions(self):
+        """Jsonify the divided_discussions"""
+        return json.loads(self._divided_discussions)
+
+    @divided_discussions.setter
+    def divided_discussions(self, value):
+        """Un-Jsonify the divided_discussions"""
+        self._divided_discussions = json.dumps(value)

common/djangoapps/django_comment_common/tests.py

-from django.test import TestCase
-from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from nose.plugins.attrib import attr
 
+from django.test import TestCase
 from django_comment_common.models import Role
+from models import CourseDiscussionSettings
+from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from openedx.core.djangoapps.course_groups.cohorts import CourseCohortsSettings
 from student.models import CourseEnrollment, User
+from utils import get_course_discussion_settings, set_course_discussion_settings
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
 
 
+@attr(shard=1)
 class RoleAssignmentTest(TestCase):
     """
     Basic checks to make sure our Roles get assigned and unassigned as students
@@ -55,3 +64,73 @@ class RoleAssignmentTest(TestCase):
     #     )
     #     self.assertNotIn(student_role, self.student_user.roles.all())
     #     self.assertIn(student_role, another_student.roles.all())
+
+
+@attr(shard=1)
+class CourseDiscussionSettingsTest(ModuleStoreTestCase):
+
+    def setUp(self):
+        super(CourseDiscussionSettingsTest, self).setUp()
+        self.course = CourseFactory.create()
+
+    def test_get_course_discussion_settings(self):
+        discussion_settings = get_course_discussion_settings(self.course.id)
+        self.assertEqual(CourseDiscussionSettings.NONE, discussion_settings.division_scheme)
+        self.assertEqual([], discussion_settings.divided_discussions)
+        self.assertFalse(discussion_settings.always_divide_inline_discussions)
+
+    def test_get_course_discussion_settings_legacy_settings(self):
+        self.course.cohort_config = {
+            'cohorted': True,
+            'always_cohort_inline_discussions': True,
+            'cohorted_discussions': ['foo']
+        }
+        modulestore().update_item(self.course, ModuleStoreEnum.UserID.system)
+        discussion_settings = get_course_discussion_settings(self.course.id)
+        self.assertEqual(CourseDiscussionSettings.COHORT, discussion_settings.division_scheme)
+        self.assertEqual(['foo'], discussion_settings.divided_discussions)
+        self.assertTrue(discussion_settings.always_divide_inline_discussions)
+
+    def test_get_course_discussion_settings_cohort_settings(self):
+        CourseCohortsSettings.objects.get_or_create(
+            course_id=self.course.id,
+            defaults={
+                'is_cohorted': True,
+                'always_cohort_inline_discussions': True,
+                'cohorted_discussions': ['foo', 'bar']
+            }
+        )
+        discussion_settings = get_course_discussion_settings(self.course.id)
+        self.assertEqual(CourseDiscussionSettings.COHORT, discussion_settings.division_scheme)
+        self.assertEqual(['foo', 'bar'], discussion_settings.divided_discussions)
+        self.assertTrue(discussion_settings.always_divide_inline_discussions)
+
+    def test_set_course_discussion_settings(self):
+        set_course_discussion_settings(
+            course_key=self.course.id,
+            divided_discussions=['cohorted_topic'],
+            division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK,
+            always_divide_inline_discussions=True,
+        )
+        discussion_settings = get_course_discussion_settings(self.course.id)
+        self.assertEqual(CourseDiscussionSettings.ENROLLMENT_TRACK, discussion_settings.division_scheme)
+        self.assertEqual(['cohorted_topic'], discussion_settings.divided_discussions)
+        self.assertTrue(discussion_settings.always_divide_inline_discussions)
+
+    def test_invalid_data_types(self):
+        exception_msg_template = "Incorrect field type for `{}`. Type must be `{}`"
+        fields = [
+            {'name': 'division_scheme', 'type': basestring},
+            {'name': 'always_divide_inline_discussions', 'type': bool},
+            {'name': 'divided_discussions', 'type': list}
+        ]
+        invalid_value = 3.14
+
+        for field in fields:
+            with self.assertRaises(ValueError) as value_error:
+                set_course_discussion_settings(self.course.id, **{field['name']: invalid_value})
+
+            self.assertEqual(
+                value_error.exception.message,
+                exception_msg_template.format(field['name'], field['type'].__name__)
+            )

common/djangoapps/django_comment_common/utils.py

@@ -9,6 +9,10 @@ from django_comment_common.models import (
     FORUM_ROLE_STUDENT,
     Role
 )
+from openedx.core.djangoapps.course_groups.cohorts import get_legacy_discussion_settings
+from request_cache.middleware import request_cached
+
+from .models import CourseDiscussionSettings
 
 
 class ThreadContext(object):
@@ -91,3 +95,48 @@ def are_permissions_roles_seeded(course_id):
             return False
 
     return True
+
+
+@request_cached
+def get_course_discussion_settings(course_key):
+    try:
+        course_discussion_settings = CourseDiscussionSettings.objects.get(course_id=course_key)
+    except CourseDiscussionSettings.DoesNotExist:
+        legacy_discussion_settings = get_legacy_discussion_settings(course_key)
+        course_discussion_settings, _ = CourseDiscussionSettings.objects.get_or_create(
+            course_id=course_key,
+            defaults={
+                'always_divide_inline_discussions': legacy_discussion_settings['always_cohort_inline_discussions'],
+                'divided_discussions': legacy_discussion_settings['cohorted_discussions'],
+                'division_scheme': CourseDiscussionSettings.COHORT if legacy_discussion_settings['is_cohorted']
+                else CourseDiscussionSettings.NONE
+            }
+        )
+
+    return course_discussion_settings
+
+
+def set_course_discussion_settings(course_key, **kwargs):
+    """
+    Set discussion settings for a course.
+
+    Arguments:
+        course_key: CourseKey
+        always_divide_inline_discussions (bool): If inline discussions should always be divided.
+        divided_discussions (list): List of discussion ids.
+        division_scheme (str): `CourseDiscussionSettings.NONE`, `CourseDiscussionSettings.COHORT`,
+            or `CourseDiscussionSettings.ENROLLMENT_TRACK`
+
+    Returns:
+        A CourseDiscussionSettings object.
+    """
+    fields = {'division_scheme': basestring, 'always_divide_inline_discussions': bool, 'divided_discussions': list}
+    course_discussion_settings = get_course_discussion_settings(course_key)
+    for field, field_type in fields.items():
+        if field in kwargs:
+            if not isinstance(kwargs[field], field_type):
+                raise ValueError("Incorrect field type for `{}`. Type must be `{}`".format(field, field_type.__name__))
+            setattr(course_discussion_settings, field, kwargs[field])
+
+    course_discussion_settings.save()
+    return course_discussion_settings

common/lib/xmodule/xmodule/partitions/partitions_service.py

@@ -133,7 +133,7 @@ class PartitionService(object):
         if self._cache and (cache_key in self._cache):
             return self._cache[cache_key]
 
-        user_partition = self._get_user_partition(user_partition_id)
+        user_partition = self.get_user_partition(user_partition_id)
         if user_partition is None:
             raise ValueError(
                 "Configuration problem!  No user_partition with id {0} "
@@ -148,7 +148,7 @@ class PartitionService(object):
 
         return group_id
 
-    def _get_user_partition(self, user_partition_id):
+    def get_user_partition(self, user_partition_id):
         """
         Look for a user partition with a matching id in the course's partitions.
         Note that this method can return an inactive user partition.

common/static/common/js/discussion/views/discussion_thread_list_view.js

@@ -32,8 +32,8 @@
                 this.retrieveFollowed = function() {
                     return DiscussionThreadListView.prototype.retrieveFollowed.apply(self, arguments);
                 };
-                this.chooseCohort = function() {
-                    return DiscussionThreadListView.prototype.chooseCohort.apply(self, arguments);
+                this.chooseGroup = function() {
+                    return DiscussionThreadListView.prototype.chooseGroup.apply(self, arguments);
                 };
                 this.chooseFilter = function() {
                     return DiscussionThreadListView.prototype.chooseFilter.apply(self, arguments);
@@ -85,7 +85,7 @@
                 'click .forum-nav-thread-link': 'threadSelected',
                 'click .forum-nav-load-more-link': 'loadMorePages',
                 'change .forum-nav-filter-main-control': 'chooseFilter',
-                'change .forum-nav-filter-cohort-control': 'chooseCohort'
+                'change .forum-nav-filter-cohort-control': 'chooseGroup'
             };
 
             DiscussionThreadListView.prototype.initialize = function(options) {
@@ -194,7 +194,7 @@
                 edx.HtmlUtils.append(
                     this.$el,
                     this.template({
-                        isCohorted: this.courseSettings.get('is_cohorted'),
+                        isDiscussionDivisionEnabled: this.courseSettings.get('is_discussion_division_enabled'),
                         isPrivilegedUser: DiscussionUtil.isPrivilegedUser()
                     })
                 );
@@ -404,7 +404,7 @@
                         return $(elem).data('discussion-id');
                     }).get();
                     this.retrieveDiscussions(discussionIds);
-                    return this.$('.forum-nav-filter-cohort').toggle($item.data('cohorted') === true);
+                    return this.$('.forum-nav-filter-cohort').toggle($item.data('divided') === true);
                 }
             };
 
@@ -413,7 +413,7 @@
                 return this.retrieveFirstPage();
             };
 
-            DiscussionThreadListView.prototype.chooseCohort = function() {
+            DiscussionThreadListView.prototype.chooseGroup = function() {
                 this.group_id = this.$('.forum-nav-filter-cohort-control :selected').val();
                 return this.retrieveFirstPage();
             };

common/static/common/js/discussion/views/discussion_topic_menu_view.js

@@ -35,7 +35,7 @@
                         '[data-discussion-id="' + this.getCurrentTopicId() + '"]'
                     ));
                 } else if ($general.length > 0) {
-                    this.setTopic($general);
+                    this.setTopic($general.first());
                 } else {
                     this.setTopic(this.$('.post-topic option').first());
                 }

common/static/common/js/discussion/views/new_post_view.js

@@ -51,7 +51,7 @@
                     threadTypeTemplate;
                 context = _.clone(this.course_settings.attributes);
                 _.extend(context, {
-                    cohort_options: this.getCohortOptions(),
+                    group_options: this.getGroupOptions(),
                     is_commentable_divided: this.is_commentable_divided,
                     mode: this.mode,
                     startHeader: this.startHeader,
@@ -84,15 +84,15 @@
                 return this.mode === 'tab';
             };
 
-            NewPostView.prototype.getCohortOptions = function() {
+            NewPostView.prototype.getGroupOptions = function() {
                 var userGroupId;
-                if (this.course_settings.get('is_cohorted') && DiscussionUtil.isPrivilegedUser()) {
+                if (this.course_settings.get('is_discussion_division_enabled') && DiscussionUtil.isPrivilegedUser()) {
                     userGroupId = $('#discussion-container').data('user-group-id');
-                    return _.map(this.course_settings.get('cohorts'), function(cohort) {
+                    return _.map(this.course_settings.get('groups'), function(group) {
                         return {
-                            value: cohort.id,
-                            text: cohort.name,
-                            selected: cohort.id === userGroupId
+                            value: group.id,
+                            text: group.name,
+                            selected: group.id === userGroupId
                         };
                     });
                 } else {
@@ -112,7 +112,7 @@
             };
 
             NewPostView.prototype.toggleGroupDropdown = function($target) {
-                if ($target.data('cohorted')) {
+                if ($target.data('divided')) {
                     $('.js-group-select').prop('disabled', false);
                     return $('.group-selector-wrapper').removeClass('disabled');
                 } else {

common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js

@@ -62,7 +62,7 @@
                 '                            <li' +
                 '                                class="forum-nav-browse-menu-item"' +
                 '                                data-discussion-id="child"' +
-                '                                data-cohorted="false"' +
+                '                                data-divided="false"' +
                 '                            >' +
                 '                                <a href="#" class="forum-nav-browse-title">Child</a>' +
                 '                            </li>' +
@@ -70,7 +70,7 @@
                 '                    <li' +
                 '                        class="forum-nav-browse-menu-item"' +
                 '                        data-discussion-id="sibling"' +
-                '                        data-cohorted="false"' +
+                '                        data-divided="false"' +
                 '                    >' +
                 '                        <a href="#" class="forum-nav-browse-title">Sibling</a>' +
                 '                    </li>' +
@@ -79,7 +79,7 @@
                 '            <li' +
                 '                class="forum-nav-browse-menu-item"' +
                 '                data-discussion-id="other"' +
-                '                data-cohorted="true"' +
+                '                data-divided="true"' +
                 '            >' +
                 '                <a href="#" class="forum-nav-browse-title">Other Category</a>' +
                 '            </li>' +
@@ -95,11 +95,11 @@
                 '                    <option value="flagged">Flagged</option>' +
                 '                </select>' +
                 '            </label>' +
-                '            <% if (isCohorted && isPrivilegedUser) { %>' +
+                '            <% if (isDiscussionDivisionEnabled && isPrivilegedUser) { %>' +
                 '            <label class="forum-nav-filter-cohort">' +
-                '                <span class="sr">Cohort:</span>' +
+                '                <span class="sr">Group:</span>' +
                 '                <select class="forum-nav-filter-cohort-control">' +
-                '                    <option value="">in all cohorts</option>' +
+                '                    <option value="">in all groups</option>' +
                 '                    <option value="1">Cohort1</option>' +
                 '                    <option value="2">Cohort2</option>' +
                 '                </select>' +
@@ -164,7 +164,7 @@
                 collection: this.discussion,
                 el: $('#fixture-element'),
                 courseSettings: new DiscussionCourseSettings({
-                    is_cohorted: true
+                    is_discussion_division_enabled: true
                 })
             });
             return this.view.render();
@@ -199,7 +199,7 @@
                         collection: discussion,
                         showThreadPreview: true,
                         courseSettings: new DiscussionCourseSettings({
-                            is_cohorted: true
+                            is_discussion_division_enabled: true
                         })
                     },
                     props
@@ -233,7 +233,7 @@
             });
         });
 
-        describe('cohort selector', function() {
+        describe('group selector', function() {
             it('should not be visible to students', function() {
                 return expect(this.view.$('.forum-nav-filter-cohort-control:visible')).not.toExist();
             });

common/static/common/js/spec/discussion/view/new_post_view_spec.js

@@ -31,7 +31,7 @@
                 return expect(group_disabled).toEqual(true);
             }
         };
-        describe('cohort selector', function() {
+        describe('group selector', function() {
             beforeEach(function() {
                 this.course_settings = new DiscussionCourseSettings({
                     category_map: {
@@ -53,8 +53,8 @@
                     },
                     allow_anonymous: false,
                     allow_anonymous_to_peers: false,
-                    is_cohorted: true,
-                    cohorts: [
+                    is_discussion_division_enabled: true,
+                    groups: [
                         {
                             id: 1,
                             name: 'Cohort1'
@@ -75,15 +75,15 @@
             it('is not visible to students', function() {
                 return checkVisibility(this.view, false, false, true);
             });
-            it('allows TAs to see the cohort selector when the topic is cohorted', function() {
+            it('allows TAs to see the group selector when the topic is divided', function() {
                 DiscussionSpecHelper.makeTA();
                 return checkVisibility(this.view, true, false, true);
             });
-            it('allows moderators to see the cohort selector when the topic is cohorted', function() {
+            it('allows moderators to see the group selector when the topic is divided', function() {
                 DiscussionSpecHelper.makeModerator();
                 return checkVisibility(this.view, true, false, true);
             });
-            it('only enables the cohort selector when applicable', function() {
+            it('only enables the group selector when applicable', function() {
                 DiscussionSpecHelper.makeModerator();
                 checkVisibility(this.view, true, false, true);
 
@@ -95,7 +95,7 @@
                 $('.post-topic').trigger('change');
                 return checkVisibility(this.view, true, false, false);
             });
-            it('allows the user to make a cohort selection', function() {
+            it('allows the user to make a group selection', function() {
                 var expectedGroupId,
                     self = this;
                 DiscussionSpecHelper.makeModerator();
@@ -116,23 +116,23 @@
                 });
             });
         });
-        describe('always cohort inline discussions ', function() {
+        describe('always divide inline discussions ', function() {
             beforeEach(function() {
                 this.course_settings = new DiscussionCourseSettings({
-                    'category_map': {
-                        'children': [],
-                        'entries': {}
+                    category_map: {
+                        children: [],
+                        entries: {}
                     },
-                    'allow_anonymous': false,
-                    'allow_anonymous_to_peers': false,
-                    'is_cohorted': true,
-                    'cohorts': [
+                    allow_anonymous: false,
+                    allow_anonymous_to_peers: false,
+                    is_discussion_division_enabled: true,
+                    groups: [
                         {
-                            'id': 1,
-                            'name': 'Cohort1'
+                            id: 1,
+                            name: 'Cohort1'
                         }, {
-                            'id': 2,
-                            'name': 'Cohort2'
+                            id: 2,
+                            name: 'Cohort2'
                         }
                     ]
                 });
@@ -143,12 +143,12 @@
                     mode: 'tab'
                 });
             });
-            it('disables the cohort menu if it is set false', function() {
+            it('disables the group menu if it is set false', function() {
                 DiscussionSpecHelper.makeModerator();
                 this.view.is_commentable_divided = false;
                 return checkVisibility(this.view, true, true, true);
             });
-            it('enables the cohort menu if it is set true', function() {
+            it('enables the group menu if it is set true', function() {
                 DiscussionSpecHelper.makeModerator();
                 this.view.is_commentable_divided = true;
                 return checkVisibility(this.view, true, false, true);

common/static/common/js/spec_helpers/discussion_spec_helper.js

@@ -67,7 +67,7 @@
                             }
                         }
                     },
-                    is_cohorted: true,
+                    is_discussion_division_enabled: true,
                     allow_anonymous: false,
                     allow_anonymous_to_peers: false
                 },
@@ -170,7 +170,7 @@
                 '                            <li' +
                 '                                class="forum-nav-browse-menu-item"' +
                 '                                data-discussion-id="child"' +
-                '                                data-cohorted="false"' +
+                '                                data-divided="false"' +
                 '                            >' +
                 '                                <a href="#" class="forum-nav-browse-title">Child</a>' +
                 '                            </li>' +
@@ -178,7 +178,7 @@
                 '                    <li' +
                 '                        class="forum-nav-browse-menu-item"' +
                 '                        data-discussion-id="sibling"' +
-                '                        data-cohorted="false"' +
+                '                        data-divided="false"' +
                 '                    >' +
                 '                        <a href="#" class="forum-nav-browse-title">Sibling</a>' +
                 '                    </li>' +
@@ -187,7 +187,7 @@
                 '            <li' +
                 '                class="forum-nav-browse-menu-item"' +
                 '                data-discussion-id="other"' +
-                '                data-cohorted="true"' +
+                '                data-divided="true"' +
                 '            >' +
                 '                <a href="#" class="forum-nav-browse-title">Other Category</a>' +
                 '            </li>' +
@@ -203,11 +203,11 @@
                 '                    <option value="flagged">Flagged</option>' +
                 '                </select>' +
                 '            </label>' +
-                '            <% if (isCohorted && isPrivilegedUser) { %>' +
+                '            <% if (isDiscussionDivisionEnabled && isPrivilegedUser) { %>' +
                 '            <label class="forum-nav-filter-cohort">' +
-                '                <span class="sr">Cohort:</span>' +
+                '                <span class="sr">Group:</span>' +
                 '                <select class="forum-nav-filter-cohort-control">' +
-                '                    <option value="">in all cohorts</option>' +
+                '                    <option value="">in all groups</option>' +
                 '                    <option value="1">Cohort1</option>' +
                 '                    <option value="2">Cohort2</option>' +
                 '                </select>' +

common/static/common/templates/discussion/new-post-menu-entry.underscore

-<option class="topic-title" data-discussion-id="<%- id %>" data-cohorted="<%- is_divided %>"><%- text %></option>
+<option class="topic-title" data-discussion-id="<%- id %>" data-divided="<%- is_divided %>"><%- text %></option>

common/static/common/templates/discussion/new-post.underscore

@@ -9,7 +9,7 @@
     <% } %>
     <ul class="post-errors" style="display: none"></ul>
     <div class="forum-new-post-form-wrapper"></div>
-    <% if (cohort_options) { %>
+    <% if (group_options) { %>
     <div class="post-field group-selector-wrapper <% if (!is_commentable_divided) { print('disabled'); } %>">
         <label class="field-label">
             <span class="field-label-text">
@@ -17,12 +17,12 @@
                 <%- gettext("Visible to") %>
             </span>
             <div class="field-help" id="field_help_visible_to">
-                <%- gettext("Discussion admins, moderators, and TAs can make their posts visible to all students or specify a single cohort.") %>
+                <%- gettext("Discussion admins, moderators, and TAs can make their posts visible to all students or specify a single group.") %>
             </div>
             <div class="field-input">
                 <select aria-describedby="field_help_visible_to" class="post-topic field-input js-group-select" name="group_id" <% if (!is_commentable_divided) { print("disabled"); } %>>
                     <option value=""><%- gettext("All Groups") %></option>
-                    <% _.each(cohort_options, function(opt) { %>
+                    <% _.each(group_options, function(opt) { %>
                     <option value="<%- opt.value %>" <% if (opt.selected) { print("selected"); } %>><%- opt.text %></option>
                     <% }); %>
                 </select>

common/test/acceptance/tests/discussion/helpers.py

@@ -76,17 +76,26 @@ class CohortTestMixin(object):
 
     def enable_cohorting(self, course_fixture):
         """
-        enables cohorting for the current course fixture.
+        Enables cohorting for the specified course fixture.
         """
-        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
-        data = json.dumps({'always_cohort_inline_discussions': True})
+        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'
+        data = json.dumps({'is_cohorted': True})
         response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers)
+        self.assertTrue(response.ok, "Failed to enable cohorts")
+
+    def enable_always_divide_inline_discussions(self, course_fixture):
+        """
+        Enables "always_divide_inline_discussions" (but does not enabling cohorting).
+        """
+        discussions_url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/discussions/settings'
+        discussions_data = json.dumps({'always_divide_inline_discussions': True})
+        course_fixture.session.patch(discussions_url, data=discussions_data, headers=course_fixture.headers)
 
     def disable_cohorting(self, course_fixture):
         """
-        Disables cohorting for the current course fixture.
+        Disables cohorting for the specified course fixture.
         """
-        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
+        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'
         data = json.dumps({'is_cohorted': False})
         response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers)
         self.assertTrue(response.ok, "Failed to disable cohorts")

common/test/acceptance/tests/discussion/test_cohorts.py

@@ -47,6 +47,7 @@ class CohortedDiscussionTestMixin(BaseDiscussionMixin, CohortTestMixin):
 
         # Enable cohorts and verify that the post shows to cohort only.
         self.enable_cohorting(self.course_fixture)
+        self.enable_always_divide_inline_discussions(self.course_fixture)
         self.refresh_thread_page(self.thread_id)
         self.assertEquals(
             self.thread_page.get_group_visibility_label(),

common/test/acceptance/tests/lms/test_lms_cohorted_courseware_search.py

@@ -7,7 +7,6 @@ import uuid
 
 from nose.plugins.attrib import attr
 
-from common.test.acceptance.fixtures import LMS_BASE_URL
 from common.test.acceptance.fixtures.course import XBlockFixtureDesc
 from common.test.acceptance.pages.common.auto_auth import AutoAuthPage
 from common.test.acceptance.pages.common.logout import LogoutPage
@@ -17,12 +16,13 @@ from common.test.acceptance.pages.lms.staff_view import StaffCoursewarePage
 from common.test.acceptance.pages.studio.component_editor import ComponentVisibilityEditorView
 from common.test.acceptance.pages.studio.overview import CourseOutlinePage as StudioCourseOutlinePage
 from common.test.acceptance.pages.studio.settings_group_configurations import GroupConfigurationsPage
+from common.test.acceptance.tests.discussion.helpers import CohortTestMixin
 from common.test.acceptance.tests.helpers import remove_file
 from common.test.acceptance.tests.studio.base_studio_test import ContainerBase
 
 
 @attr(shard=1)
-class CoursewareSearchCohortTest(ContainerBase):
+class CoursewareSearchCohortTest(ContainerBase, CohortTestMixin):
     """
     Test courseware search.
     """
@@ -132,15 +132,6 @@ class CoursewareSearchCohortTest(ContainerBase):
             )
         )
 
-    def enable_cohorting(self, course_fixture):
-        """
-        Enables cohorting for the current course.
-        """
-        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
-        data = json.dumps({'is_cohorted': True})
-        response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers)
-        self.assertTrue(response.ok, "Failed to enable cohorts")
-
     def create_content_groups(self):
         """
         Creates two content groups in Studio Group Configurations Settings.

common/test/acceptance/tests/lms/test_lms_help.py

@@ -2,17 +2,15 @@
 Test Help links in LMS
 """
 
-import json
-
-from common.test.acceptance.fixtures import LMS_BASE_URL
 from common.test.acceptance.fixtures.course import CourseFixture
 from common.test.acceptance.pages.lms.instructor_dashboard import InstructorDashboardPage
+from common.test.acceptance.tests.discussion.helpers import CohortTestMixin
 from common.test.acceptance.tests.helpers import assert_opened_help_link_is_correct, url_for_help
 from common.test.acceptance.tests.lms.test_lms_instructor_dashboard import BaseInstructorDashboardTest
 from common.test.acceptance.tests.studio.base_studio_test import ContainerBase
 
 
-class TestCohortHelp(ContainerBase):
+class TestCohortHelp(ContainerBase, CohortTestMixin):
     """
     Tests help links in Cohort page
     """
@@ -74,15 +72,6 @@ class TestCohortHelp(ContainerBase):
         )
         self.verify_help_link(href)
 
-    def enable_cohorting(self, course_fixture):
-        """
-        Enables cohorting for the current course.
-        """
-        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
-        data = json.dumps({'is_cohorted': True})
-        response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers)
-        self.assertTrue(response.ok, "Failed to enable cohorts")
-
 
 class InstructorDashboardHelp(BaseInstructorDashboardTest):
     """

common/test/acceptance/tests/test_cohorted_courseware.py

@@ -2,12 +2,9 @@
 End-to-end test for cohorted courseware. This uses both Studio and LMS.
 """
 
-import json
-
 from bok_choy.page_object import XSS_INJECTION
 from nose.plugins.attrib import attr
 
-from common.test.acceptance.fixtures import LMS_BASE_URL
 from common.test.acceptance.fixtures.course import XBlockFixtureDesc
 from common.test.acceptance.pages.common.auto_auth import AutoAuthPage
 from common.test.acceptance.pages.common.utils import add_enrollment_course_modes, enroll_user_track
@@ -15,6 +12,7 @@ from common.test.acceptance.pages.lms.courseware import CoursewarePage
 from common.test.acceptance.pages.lms.instructor_dashboard import InstructorDashboardPage
 from common.test.acceptance.pages.studio.component_editor import ComponentVisibilityEditorView
 from common.test.acceptance.pages.studio.settings_group_configurations import GroupConfigurationsPage
+from common.test.acceptance.tests.discussion.helpers import CohortTestMixin
 from common.test.acceptance.tests.lms.test_lms_user_preview import verify_expected_problem_visibility
 from studio.base_studio_test import ContainerBase
 
@@ -23,7 +21,7 @@ VERIFIED_TRACK = "Verified"
 
 
 @attr(shard=5)
-class EndToEndCohortedCoursewareTest(ContainerBase):
+class EndToEndCohortedCoursewareTest(ContainerBase, CohortTestMixin):
     """
     End-to-end of cohorted courseware.
     """
@@ -113,15 +111,6 @@ class EndToEndCohortedCoursewareTest(ContainerBase):
             )
         )
 
-    def enable_cohorting(self, course_fixture):
-        """
-        Enables cohorting for the current course.
-        """
-        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
-        data = json.dumps({'is_cohorted': True})
-        response = course_fixture.session.patch(url, data=data, headers=course_fixture.headers)
-        self.assertTrue(response.ok, "Failed to enable cohorts")
-
     def create_content_groups(self):
         """
         Creates two content groups in Studio Group Configurations Settings.

lms/djangoapps/discussion/static/discussion/js/spec/discussion_board_factory_spec.js

@@ -41,10 +41,10 @@ define(
                     thread_pages: [],
                     contentInfo: null,
                     courseSettings: {
-                        is_cohorted: false,
+                        is_discussion_division_enabled: false,
                         allow_anonymous: false,
                         allow_anonymous_to_peers: false,
-                        cohorts: [],
+                        groups: [],
                         category_map: {}
                     }
                 });

lms/djangoapps/discussion_api/api.py

 """
 Discussion API internal interface
 """
+import itertools
 from collections import defaultdict
+from enum import Enum
 from urllib import urlencode
 from urlparse import urlunparse
 
-from django.core.exceptions import ValidationError
-from django.core.urlresolvers import reverse
-from django.http import Http404
-import itertools
-from enum import Enum
-from openedx.core.djangoapps.user_api.accounts.views import AccountViewSet
-
-from rest_framework.exceptions import PermissionDenied
-
-from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
-from opaque_keys import InvalidKeyError
-from opaque_keys.edx.locator import CourseKey
 from courseware.courses import get_course_with_access
-
-from discussion_api.exceptions import ThreadNotFoundError, CommentNotFoundError, DiscussionDisabledError
+from discussion_api.exceptions import CommentNotFoundError, DiscussionDisabledError, ThreadNotFoundError
 from discussion_api.forms import CommentActionsForm, ThreadActionsForm
 from discussion_api.permissions import (
     can_delete,
     get_editable_fields,
     get_initializable_comment_fields,
-    get_initializable_thread_fields,
-)
-from discussion_api.serializers import CommentSerializer, ThreadSerializer, get_context, DiscussionTopicSerializer
-from django_comment_client.base.views import (
-    track_comment_created_event,
-    track_thread_created_event,
-    track_voted_event,
+    get_initializable_thread_fields
 )
+from discussion_api.serializers import CommentSerializer, DiscussionTopicSerializer, ThreadSerializer, get_context
+from django.core.exceptions import ValidationError
+from django.core.urlresolvers import reverse
+from django.http import Http404
+from django_comment_client.base.views import track_comment_created_event, track_thread_created_event, track_voted_event
+from django_comment_client.utils import get_accessible_discussion_xblocks, get_group_id_for_user, is_commentable_divided
 from django_comment_common.signals import (
-    thread_created,
-    thread_edited,
-    thread_deleted,
-    thread_voted,
     comment_created,
+    comment_deleted,
     comment_edited,
     comment_voted,
-    comment_deleted,
+    thread_created,
+    thread_deleted,
+    thread_edited,
+    thread_voted
 )
-from django_comment_client.utils import get_accessible_discussion_xblocks, is_commentable_divided, get_group_id_for_user
+from django_comment_common.utils import get_course_discussion_settings
+from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 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.lib.exceptions import CourseNotFoundError, PageNotFoundError, DiscussionNotFoundError
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.locator import CourseKey
+from openedx.core.djangoapps.user_api.accounts.views import AccountViewSet
+from openedx.core.lib.exceptions import CourseNotFoundError, DiscussionNotFoundError, PageNotFoundError
+from rest_framework.exceptions import PermissionDenied
 
 
 class DiscussionTopic(object):
@@ -109,12 +103,13 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
         course_key = CourseKey.from_string(cc_thread["course_id"])
         course = _get_course(course_key, request.user)
         context = get_context(course, request, cc_thread)
+        course_discussion_settings = get_course_discussion_settings(course_key)
         if (
                 not context["is_requester_privileged"] and
                 cc_thread["group_id"] and
-                is_commentable_divided(course.id, cc_thread["commentable_id"])
+                is_commentable_divided(course.id, cc_thread["commentable_id"], course_discussion_settings)
         ):
-            requester_group_id = get_group_id_for_user(request.user, course.id)
+            requester_group_id = get_group_id_for_user(request.user, course_discussion_settings)
             if requester_group_id is not None and cc_thread["group_id"] != requester_group_id:
                 raise ThreadNotFoundError("Thread not found.")
         return cc_thread, context
@@ -546,7 +541,7 @@ def get_thread_list(
         "user_id": unicode(request.user.id),
         "group_id": (
             None if context["is_requester_privileged"] else
-            get_group_id_for_user(request.user, course.id)
+            get_group_id_for_user(request.user, get_course_discussion_settings(course.id))
         ),
         "page": page,
         "per_page": page_size,
@@ -828,12 +823,13 @@ def create_thread(request, thread_data):
 
     context = get_context(course, request)
     _check_initializable_thread_fields(thread_data, context)
+    discussion_settings = get_course_discussion_settings(course_key)
     if (
             "group_id" not in thread_data and
-            is_commentable_divided(course_key, thread_data.get("topic_id"))
+            is_commentable_divided(course_key, thread_data.get("topic_id"), discussion_settings)
     ):
         thread_data = thread_data.copy()
-        thread_data["group_id"] = get_group_id_for_user(user, course_key)
+        thread_data["group_id"] = get_group_id_for_user(user, discussion_settings)
     serializer = ThreadSerializer(data=thread_data, context=context)
     actions_form = ThreadActionsForm(thread_data)
     if not (serializer.is_valid() and actions_form.is_valid()):

lms/djangoapps/discussion_api/permissions.py

@@ -77,7 +77,7 @@ def get_editable_fields(cc_content, context):
         ret |= {"following", "read"}
         if _is_author_or_privileged(cc_content, context):
             ret |= {"topic_id", "type", "title"}
-        if context["is_requester_privileged"] and context["course"].is_cohorted:
+        if context["is_requester_privileged"] and context["discussion_division_enabled"]:
             ret |= {"group_id"}
 
     # Comment fields

lms/djangoapps/discussion_api/serializers.py

@@ -4,30 +4,20 @@ Discussion API serializers
 from urllib import urlencode
 from urlparse import urlunparse
 
+from discussion_api.permissions import NON_UPDATABLE_COMMENT_FIELDS, NON_UPDATABLE_THREAD_FIELDS, get_editable_fields
+from discussion_api.render import render_body
 from django.contrib.auth.models import User as DjangoUser
 from django.core.exceptions import ValidationError
 from django.core.urlresolvers import reverse
-
-from rest_framework import serializers
-
-from discussion_api.permissions import (
-    NON_UPDATABLE_COMMENT_FIELDS,
-    NON_UPDATABLE_THREAD_FIELDS,
-    get_editable_fields,
-)
-from discussion_api.render import render_body
 from django_comment_client.utils import is_comment_too_deep
-from django_comment_common.models import (
-    FORUM_ROLE_ADMINISTRATOR,
-    FORUM_ROLE_COMMUNITY_TA,
-    FORUM_ROLE_MODERATOR,
-    Role,
-)
+from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR, Role
+from django_comment_common.utils import get_course_discussion_settings
+from lms.djangoapps.django_comment_client.utils import course_discussion_division_enabled, get_group_names_by_id
 from lms.lib.comment_client.comment import Comment
 from lms.lib.comment_client.thread import Thread
 from lms.lib.comment_client.user import User as CommentClientUser
 from lms.lib.comment_client.utils import CommentClientRequestError
-from openedx.core.djangoapps.course_groups.cohorts import get_cohort_names
+from rest_framework import serializers
 
 
 def get_context(course, request, thread=None):
@@ -52,12 +42,13 @@ def get_context(course, request, thread=None):
     requester = request.user
     cc_requester = CommentClientUser.from_django_user(requester).retrieve()
     cc_requester["course_id"] = course.id
+    course_discussion_settings = get_course_discussion_settings(course.id)
     return {
         "course": course,
         "request": request,
         "thread": thread,
-        # For now, the only groups are cohorts
-        "group_ids_to_names": get_cohort_names(course),
+        "discussion_division_enabled": course_discussion_division_enabled(course_discussion_settings),
+        "group_ids_to_names": get_group_names_by_id(course_discussion_settings),
         "is_requester_privileged": requester.id in staff_user_ids or requester.id in ta_user_ids,
         "staff_user_ids": staff_user_ids,
         "ta_user_ids": ta_user_ids,

lms/djangoapps/discussion_api/tests/test_api.py

 """
 Tests for Discussion API internal interface
 """
-from datetime import datetime, timedelta
 import itertools
-from urlparse import parse_qs, urlparse, urlunparse
+from datetime import datetime, timedelta
 from urllib import urlencode
+from urlparse import parse_qs, urlparse, urlunparse
 
 import ddt
-import httpretty
 import mock
 from nose.plugins.attrib import attr
-from pytz import UTC
-
-from django.core.exceptions import ValidationError
-from django.test.client import RequestFactory
-
-from rest_framework.exceptions import PermissionDenied
-
-from opaque_keys.edx.locator import CourseLocator
 
+import httpretty
 from common.test.utils import MockSignalHandlerMixin, disable_signal
 from courseware.tests.factories import BetaTesterFactory, StaffFactory
 from discussion_api import api
@@ -30,31 +22,37 @@ from discussion_api.api import (
     get_comment_list,
     get_course,
     get_course_topics,
+    get_thread,
     get_thread_list,
     update_comment,
-    update_thread,
-    get_thread,
+    update_thread
 )
-from discussion_api.exceptions import DiscussionDisabledError, ThreadNotFoundError, CommentNotFoundError
+from discussion_api.exceptions import CommentNotFoundError, DiscussionDisabledError, ThreadNotFoundError
 from discussion_api.tests.utils import (
     CommentsServiceMockMixin,
     make_minimal_cs_comment,
     make_minimal_cs_thread,
-    make_paginated_api_response,
+    make_paginated_api_response
 )
+from django.core.exceptions import ValidationError
+from django.test.client import RequestFactory
+from django_comment_client.tests.utils import ForumsEnableMixin
 from django_comment_common.models import (
     FORUM_ROLE_ADMINISTRATOR,
     FORUM_ROLE_COMMUNITY_TA,
     FORUM_ROLE_MODERATOR,
     FORUM_ROLE_STUDENT,
-    Role,
+    Role
 )
-from django_comment_client.tests.utils import ForumsEnableMixin
+from opaque_keys.edx.locator import CourseLocator
 from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup
 from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
 from openedx.core.lib.exceptions import CourseNotFoundError, PageNotFoundError
+from pytz import UTC
+from rest_framework.exceptions import PermissionDenied
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
 from util.testing import UrlResetMixin
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -591,6 +589,8 @@ class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMix
         self.request.user = self.user
         CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
         self.author = UserFactory.create()
+        self.course.cohort_config = {"cohorted": False}
+        modulestore().update_item(self.course, ModuleStoreEnum.UserID.test)
         self.cohort = CohortFactory.create(course_id=self.course.id)
 
     def get_thread_list(
@@ -662,6 +662,8 @@ class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMix
         })
 
     def test_thread_content(self):
+        self.course.cohort_config = {"cohorted": True}
+        modulestore().update_item(self.course, ModuleStoreEnum.UserID.test)
         source_threads = [
             make_minimal_cs_thread({
                 "id": "test_thread_id_0",

lms/djangoapps/discussion_api/tests/test_permissions.py

@@ -25,6 +25,7 @@ def _get_context(requester_id, is_requester_privileged, is_cohorted=False, threa
         "cc_requester": User(id=requester_id),
         "is_requester_privileged": is_requester_privileged,
         "course": CourseFactory(cohort_config={"cohorted": is_cohorted}),
+        "discussion_division_enabled": is_cohorted,
         "thread": thread,
     }
 

lms/djangoapps/discussion_api/tests/test_serializers.py

@@ -5,33 +5,30 @@ import itertools
 from urlparse import urlparse
 
 import ddt
-import httpretty
 import mock
 from nose.plugins.attrib import attr
 
-from django.test.client import RequestFactory
-
+import httpretty
 from discussion_api.serializers import CommentSerializer, ThreadSerializer, get_context
-from discussion_api.tests.utils import (
-    CommentsServiceMockMixin,
-    make_minimal_cs_thread,
-    make_minimal_cs_comment,
-)
+from discussion_api.tests.utils import CommentsServiceMockMixin, make_minimal_cs_comment, make_minimal_cs_thread
+from django.test.client import RequestFactory
+from django_comment_client.tests.utils import ForumsEnableMixin
 from django_comment_common.models import (
     FORUM_ROLE_ADMINISTRATOR,
     FORUM_ROLE_COMMUNITY_TA,
     FORUM_ROLE_MODERATOR,
     FORUM_ROLE_STUDENT,
-    Role,
+    Role
 )
 from lms.lib.comment_client.comment import Comment
 from lms.lib.comment_client.thread import Thread
+from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
 from student.tests.factories import UserFactory
 from util.testing import UrlResetMixin
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
-from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
-from django_comment_client.tests.utils import ForumsEnableMixin
 
 
 @ddt.ddt
@@ -209,6 +206,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe
         self.assertEqual(serialized["pinned"], False)
 
     def test_group(self):
+        self.course.cohort_config = {"cohorted": True}
+        modulestore().update_item(self.course, ModuleStoreEnum.UserID.test)
         cohort = CohortFactory.create(course_id=self.course.id)
         serialized = self.serialize(self.make_cs_content({"group_id": cohort.id}))
         self.assertEqual(serialized["group_id"], cohort.id)

lms/djangoapps/django_comment_client/tests/group_id.py

 import json
 import re
+
+from course_modes.models import CourseMode
+from course_modes.tests.factories import CourseModeFactory
+from django_comment_common.models import CourseDiscussionSettings
+from django_comment_common.utils import set_course_discussion_settings
 from lms.djangoapps.teams.tests.factories import CourseTeamFactory
 
 
@@ -94,11 +99,22 @@ class CohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
 
     def test_cohorted_topic_moderator_with_invalid_group_id(self, mock_request):
         invalid_id = self.student_cohort.id + self.moderator_cohort.id
-        try:
-            response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id)
-            self.assertEqual(response.status_code, 500)
-        except ValueError:
-            pass  # In mock request mode, server errors are not captured
+        response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id)
+        self.assertEqual(response.status_code, 500)
+
+    def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_request):
+        CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.AUDIT)
+        CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED)
+        set_course_discussion_settings(
+            course_key=self.course.id,
+            divided_discussions=['cohorted_topic'],
+            division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK,
+            always_divide_inline_discussions=True,
+        )
+
+        invalid_id = -1000
+        response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id)
+        self.assertEqual(response.status_code, 500)
 
 
 class NonCohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):

lms/djangoapps/django_comment_client/tests/utils.py

@@ -3,13 +3,15 @@ Utilities for tests within the django_comment_client module.
 """
 from mock import patch
 
+from django_comment_common.models import ForumsConfig, Role
+from django_comment_common.utils import CourseDiscussionSettings, seed_permissions_roles, set_course_discussion_settings
 from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
-from django_comment_common.models import Role, ForumsConfig
-from django_comment_common.utils import seed_permissions_roles
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
 from util.testing import UrlResetMixin
-from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
 
 
 class ForumsEnableMixin(object):
@@ -63,3 +65,58 @@ class CohortedTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCa
             course_id=self.course.id,
             users=[self.moderator]
         )
+
+
+# pylint: disable=dangerous-default-value
+def config_course_discussions(
+        course,
+        discussion_topics={},
+        divided_discussions=[],
+        always_divide_inline_discussions=False
+):
+        """
+        Set discussions and configure divided discussions for a course.
+
+        Arguments:
+            course: CourseDescriptor
+            discussion_topics (Dict): Discussion topic names. Picks ids and
+                sort_keys automatically.
+            divided_discussions: Discussion topics to divide. Converts the
+                list to use the same ids as discussion topic names.
+            always_divide_inline_discussions (bool): Whether inline discussions
+                should be divided by default.
+
+        Returns:
+            Nothing -- modifies course in place.
+        """
+
+        def to_id(name):
+            """Convert name to id."""
+            return topic_name_to_id(course, name)
+
+        set_course_discussion_settings(
+            course.id,
+            divided_discussions=[to_id(name) for name in divided_discussions],
+            always_divide_inline_discussions=always_divide_inline_discussions,
+            division_scheme=CourseDiscussionSettings.COHORT,
+        )
+
+        course.discussion_topics = dict((name, {"sort_key": "A", "id": to_id(name)})
+                                        for name in discussion_topics)
+        try:
+            # Not implemented for XMLModulestore, which is used by test_cohorts.
+            modulestore().update_item(course, ModuleStoreEnum.UserID.test)
+        except NotImplementedError:
+            pass
+
+
+def topic_name_to_id(course, name):
+    """
+    Given a discussion topic name, return an id for that name (includes
+    course and url_name).
+    """
+    return "{course}_{run}_{name}".format(
+        course=course.location.course,
+        run=course.url_name,
+        name=name
+    )

lms/djangoapps/instructor/tests/test_api.py

@@ -67,7 +67,7 @@ from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError
 from certificates.tests.factories import GeneratedCertificateFactory
 from certificates.models import CertificateStatuses
 
-from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings
+from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted
 from openedx.core.lib.xblock_utils import grade_histogram
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
@@ -2701,7 +2701,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment
         cohorted, and does not when the course is not cohorted.
         """
         url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)})
-        set_course_cohort_settings(self.course.id, is_cohorted=is_cohorted)
+        set_course_cohorted(self.course.id, is_cohorted)
 
         response = self.client.post(url, {})
         res_json = json.loads(response.content)

lms/djangoapps/instructor/tests/test_ecommerce.py

@@ -30,7 +30,7 @@ class TestECommerceDashboardViews(SiteMixin, SharedModuleStoreTestCase):
 
         # URL for instructor dash
         cls.url = reverse('instructor_dashboard', kwargs={'course_id': cls.course.id.to_deprecated_string()})
-        cls.ecommerce_link = '<button type="button" class="btn-link" data-section="e-commerce">E-Commerce</button>'
+        cls.ecommerce_link = '<button type="button" class="btn-link e-commerce" data-section="e-commerce">E-Commerce</button>'
 
     def setUp(self):
         super(TestECommerceDashboardViews, self).setUp()

lms/djangoapps/instructor/tests/test_email.py

@@ -31,7 +31,7 @@ class TestNewInstructorDashboardEmailViewMongoBacked(SharedModuleStoreTestCase):
         # URL for instructor dash
         cls.url = reverse('instructor_dashboard', kwargs={'course_id': cls.course.id.to_deprecated_string()})
         # URL for email view
-        cls.email_link = '<button type="button" class="btn-link" data-section="send_email">Email</button>'
+        cls.email_link = '<button type="button" class="btn-link send_email" data-section="send_email">Email</button>'
 
     def setUp(self):
         super(TestNewInstructorDashboardEmailViewMongoBacked, self).setUp()
@@ -126,7 +126,7 @@ class TestNewInstructorDashboardEmailViewXMLBacked(SharedModuleStoreTestCase):
         # URL for instructor dash
         cls.url = reverse('instructor_dashboard', kwargs={'course_id': cls.course_key.to_deprecated_string()})
         # URL for email view
-        cls.email_link = '<button type="button" class="btn-link" data-section="send_email">Email</button>'
+        cls.email_link = '<button type="button" class="btn-link send_email" data-section="send_email">Email</button>'
 
     def setUp(self):
         super(TestNewInstructorDashboardEmailViewXMLBacked, self).setUp()
@@ -138,7 +138,7 @@ class TestNewInstructorDashboardEmailViewXMLBacked(SharedModuleStoreTestCase):
         # URL for instructor dash
         self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course_key.to_deprecated_string()})
         # URL for email view
-        self.email_link = '<button type="button" class="btn-link" data-section="send_email">Email</button>'
+        self.email_link = '<button type="button" class="btn-link send_email" data-section="send_email">Email</button>'
 
     def tearDown(self):
         super(TestNewInstructorDashboardEmailViewXMLBacked, self).tearDown()

lms/djangoapps/instructor/tests/test_proctoring.py

@@ -27,7 +27,7 @@ class TestProctoringDashboardViews(SharedModuleStoreTestCase):
 
         # URL for instructor dash
         cls.url = reverse('instructor_dashboard', kwargs={'course_id': cls.course.id.to_deprecated_string()})
-        button = '<button type="button" class="btn-link" data-section="special_exams">Special Exams</button>'
+        button = '<button type="button" class="btn-link special_exams" data-section="special_exams">Special Exams</button>'
         cls.proctoring_link = button
 
     def setUp(self):

lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py

@@ -228,7 +228,7 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssT
         Test analytics dashboard message is shown
         """
         response = self.client.get(self.url)
-        analytics_section = '<li class="nav-item"><button type="button" class="btn-link" data-section="instructor_analytics">Analytics</button></li>'  # pylint: disable=line-too-long
+        analytics_section = '<li class="nav-item"><button type="button" class="btn-link instructor_analytics" data-section="instructor_analytics">Analytics</button></li>'  # pylint: disable=line-too-long
         self.assertIn(analytics_section, response.content)
 
         # link to dashboard shown
@@ -327,7 +327,7 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssT
         """
         ora_section = (
             '<li class="nav-item">'
-            '<button type="button" class="btn-link" data-section="open_response_assessment">'
+            '<button type="button" class="btn-link open_response_assessment" data-section="open_response_assessment">'
             'Open Responses'
             '</button>'
             '</li>'

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -2,61 +2,61 @@
 Instructor Dashboard Views
 """
 
-import logging
 import datetime
-from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import CourseKey
+import logging
 import uuid
-import pytz
 
-from django.contrib.auth.decorators import login_required
-from django.views.decorators.http import require_POST
-from django.utils.translation import ugettext as _, ugettext_noop
-from django.views.decorators.csrf import ensure_csrf_cookie
-from django.views.decorators.cache import cache_control
-from edxmako.shortcuts import render_to_response
-from django.core.urlresolvers import reverse
-from django.utils.html import escape
-from django.http import Http404, HttpResponseServerError
-from django.conf import settings
-from util.json_request import JsonResponse
 from mock import patch
 
-from openedx.core.lib.xblock_utils import wrap_xblock
-from openedx.core.lib.url_utils import quote_slashes
-from xmodule.html_module import HtmlDescriptor
-from xmodule.modulestore.django import modulestore
-from xmodule.tabs import CourseTab
-from xblock.field_data import DictFieldData
-from xblock.fields import ScopeIds
-from courseware.access import has_access
-from courseware.courses import get_course_by_id, get_studio_url
-from django_comment_client.utils import has_forum_access
-from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR
-from openedx.core.djangoapps.course_groups.cohorts import get_course_cohorts, is_course_cohorted, DEFAULT_COHORT_NAME
-from student.models import CourseEnrollment
-from shoppingcart.models import Coupon, PaidCourseRegistration, CourseRegCodeItem
-from course_modes.models import CourseMode, CourseModesArchive
-from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole
-from lms.djangoapps.courseware.module_render import get_module_by_usage_id
+import pytz
+from bulk_email.models import BulkEmailFlag
+from certificates import api as certs_api
 from certificates.models import (
     CertificateGenerationConfiguration,
-    CertificateWhitelist,
-    GeneratedCertificate,
-    CertificateStatuses,
     CertificateGenerationHistory,
     CertificateInvalidation,
+    CertificateStatuses,
+    CertificateWhitelist,
+    GeneratedCertificate
 )
-from certificates import api as certs_api
-from bulk_email.models import BulkEmailFlag
-
-from class_dashboard.dashboard_data import get_section_display_name, get_array_section_has_problem
-from .tools import get_units_with_due_date, title_or_url
+from class_dashboard.dashboard_data import get_array_section_has_problem, get_section_display_name
+from course_modes.models import CourseMode, CourseModesArchive
+from courseware.access import has_access
+from courseware.courses import get_course_by_id, get_studio_url
+from django.conf import settings
+from django.contrib.auth.decorators import login_required
+from django.core.urlresolvers import reverse
+from django.http import Http404, HttpResponseServerError
+from django.utils.html import escape
+from django.utils.translation import ugettext as _
+from django.utils.translation import ugettext_noop
+from django.views.decorators.cache import cache_control
+from django.views.decorators.csrf import ensure_csrf_cookie
+from django.views.decorators.http import require_POST
+from django_comment_client.utils import available_division_schemes, has_forum_access
+from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, CourseDiscussionSettings
+from edxmako.shortcuts import render_to_response
+from lms.djangoapps.courseware.module_render import get_module_by_usage_id
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from openedx.core.djangoapps.course_groups.cohorts import DEFAULT_COHORT_NAME, get_course_cohorts, is_course_cohorted
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse
-
 from openedx.core.djangolib.markup import HTML, Text
+from openedx.core.lib.url_utils import quote_slashes
+from openedx.core.lib.xblock_utils import wrap_xblock
+from shoppingcart.models import Coupon, CourseRegCodeItem, PaidCourseRegistration
+from student.models import CourseEnrollment
+from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole
+from util.json_request import JsonResponse
+from xblock.field_data import DictFieldData
+from xblock.fields import ScopeIds
+from xmodule.html_module import HtmlDescriptor
+from xmodule.modulestore.django import modulestore
+from xmodule.tabs import CourseTab
+
+from .tools import get_units_with_due_date, title_or_url
 
 log = logging.getLogger(__name__)
 
@@ -125,6 +125,7 @@ def instructor_dashboard_2(request, course_id):
         _section_course_info(course, access),
         _section_membership(course, access, is_white_label),
         _section_cohort_management(course, access),
+        _section_discussions_management(course, access),
         _section_student_admin(course, access),
         _section_data_download(course, access),
     ]
@@ -513,7 +514,6 @@ def _section_cohort_management(course, access):
         ),
         'cohorts_url': reverse('cohorts', kwargs={'course_key_string': unicode(course_key)}),
         'upload_cohorts_csv_url': reverse('add_users_to_cohorts', kwargs={'course_id': unicode(course_key)}),
-        'discussion_topics_url': reverse('cohort_discussion_topics', kwargs={'course_key_string': unicode(course_key)}),
         'verified_track_cohorting_url': reverse(
             'verified_track_cohorting', kwargs={'course_key_string': unicode(course_key)}
         ),
@@ -521,6 +521,24 @@ def _section_cohort_management(course, access):
     return section_data
 
 
+def _section_discussions_management(course, access):
+    """ Provide data for the corresponding discussion management section """
+    course_key = course.id
+    enrollment_track_schemes = available_division_schemes(course_key)
+    section_data = {
+        'section_key': 'discussions_management',
+        'section_display_name': _('Discussions'),
+        'is_hidden': (not is_course_cohorted(course_key) and
+                      CourseDiscussionSettings.ENROLLMENT_TRACK not in enrollment_track_schemes),
+        'discussion_topics_url': reverse('discussion_topics', kwargs={'course_key_string': unicode(course_key)}),
+        'course_discussion_settings': reverse(
+            'course_discussions_settings',
+            kwargs={'course_key_string': unicode(course_key)}
+        ),
+    }
+    return section_data
+
+
 def _is_small_course(course_key):
     """ Compares against MAX_ENROLLMENT_INSTR_BUTTONS to determine if course enrollment is considered small. """
     is_small_course = False

lms/envs/common.py

@@ -379,7 +379,7 @@ FEATURES = {
     'ENABLE_COOKIE_CONSENT': False,
 
     # Whether or not the dynamic EnrollmentTrackUserPartition should be registered.
-    'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': False,
+    'ENABLE_ENROLLMENT_TRACK_USER_PARTITION': True,
 
     # Enable one click program purchase
     # See LEARNER-493
@@ -1759,6 +1759,7 @@ REQUIRE_JS_PATH_OVERRIDES = {
     'js/student_profile/views/learner_profile_factory': 'js/student_profile/views/learner_profile_factory.js',
     'js/courseware/courseware_factory': 'js/courseware/courseware_factory.js',
     'js/groups/views/cohorts_dashboard_factory': 'js/groups/views/cohorts_dashboard_factory.js',
+    'js/groups/discussions_management/discussions_dashboard_factory': 'js/discussions_management/views/discussions_dashboard_factory.js',
     'draggabilly': 'js/vendor/draggabilly.js',
     'hls': 'common/js/vendor/hls.js'
 }

lms/static/js/groups/models/cohort_discussions.js → lms/static/js/discussions_management/models/course_discussions_detail.js

 (function(define) {
     'use strict';
     define(['backbone'], function(Backbone) {
-        var DiscussionTopicsSettingsModel = Backbone.Model.extend({
+        var CourseDiscussionTopicDetailsModel = Backbone.Model.extend({
             defaults: {
                 course_wide_discussions: {},
                 inline_discussions: {}
             }
         });
-        return DiscussionTopicsSettingsModel;
+        return CourseDiscussionTopicDetailsModel;
     });
 }).call(this, define || RequireJS.define);

lms/static/js/discussions_management/models/course_discussions_settings.js

+(function(define) {
+    'use strict';
+    define(['backbone'], function(Backbone) {
+        var CourseDiscussionsSettingsModel = Backbone.Model.extend({
+            idAttribute: 'id',
+            defaults: {
+                divided_inline_discussions: [],
+                divided_course_wide_discussions: [],
+                always_divide_inline_discussions: false,
+                division_scheme: 'none'
+            }
+        });
+        return CourseDiscussionsSettingsModel;
+    });
+}).call(this, define || RequireJS.define);

lms/static/js/discussions_management/views/discussions_dashboard_factory.js

+(function(define) {
+    'use strict';
+    define('js/discussions_management/views/discussions_dashboard_factory',
+        ['jquery', 'js/discussions_management/views/discussions',
+            'js/discussions_management/models/course_discussions_detail',
+            'js/discussions_management/models/course_discussions_settings'],
+        function($, DiscussionsView, CourseDiscussionTopicDetailsModel, CourseDiscussionsSettingsModel) {
+            return function() {
+                var courseDiscussionSettings = new CourseDiscussionsSettingsModel(),
+                    discussionTopicsSettings = new CourseDiscussionTopicDetailsModel(),
+                    $discussionsManagementElement = $('.discussions-management'),
+                    discussionsView;
+
+                courseDiscussionSettings.url = $discussionsManagementElement.data('course-discussion-settings-url');
+                discussionTopicsSettings.url = $discussionsManagementElement.data('discussion-topics-url');
+
+                discussionsView = new DiscussionsView({
+                    el: $discussionsManagementElement,
+                    discussionSettings: courseDiscussionSettings,
+                    context: {
+                        courseDiscussionTopicDetailsModel: discussionTopicsSettings
+                    }
+                });
+
+                courseDiscussionSettings.fetch().done(function() {
+                    discussionTopicsSettings.fetch().done(function() {
+                        discussionsView.render();
+                    });
+                });
+            };
+        });
+}).call(this, define || RequireJS.define);
+

lms/static/js/groups/views/cohort_discussions.js → lms/static/js/discussions_management/views/divided_discussions.js

 (function(define) {
     'use strict';
     define(['jquery', 'underscore', 'backbone', 'gettext', 'js/models/notification', 'js/views/notification'],
-        function($, _, Backbone) {
-            var CohortDiscussionConfigurationView = Backbone.View.extend({
+        function($, _, Backbone, gettext) {
+            /* global NotificationModel, NotificationView */
+            var DividedDiscussionConfigurationView = Backbone.View.extend({
 
                 /**
                 * Add/Remove the disabled attribute on given element.
@@ -14,53 +15,53 @@
                 },
 
                 /**
-                * Returns the cohorted discussions list.
+                * Returns the divided discussions list.
                 * @param {string} selector - To select the discussion elements whose ids to return.
-                * @returns {Array} - Cohorted discussions.
+                * @returns {Array} - Divided discussions.
                 */
-                getCohortedDiscussions: function(selector) {
+                getDividedDiscussions: function(selector) {
                     var self = this,
-                        cohortedDiscussions = [];
+                        dividedDiscussions = [];
 
                     _.each(self.$(selector), function(topic) {
-                        cohortedDiscussions.push($(topic).data('id'));
+                        dividedDiscussions.push($(topic).data('id'));
                     });
-                    return cohortedDiscussions;
+                    return dividedDiscussions;
                 },
 
                 /**
-                * Save the cohortSettings' changed attributes to the server via PATCH method.
+                * Save the discussionSettings' changed attributes to the server via PATCH method.
                 * It shows the error message(s) if any.
                 * @param {object} $element - Messages would be shown before this element.
                 * @param {object} fieldData - Data to update on the server.
                 */
                 saveForm: function($element, fieldData) {
                     var self = this,
-                        cohortSettingsModel = this.cohortSettings,
+                        discussionSettingsModel = this.discussionSettings,
                         saveOperation = $.Deferred(),
                         showErrorMessage;
-
-                    showErrorMessage = function(message, $element) {
+                    showErrorMessage = function(message) {
                         self.showMessage(message, $element, 'error');
                     };
                     this.removeNotification();
 
-                    cohortSettingsModel.save(
+                    discussionSettingsModel.save(
                         fieldData, {patch: true, wait: true}
                     ).done(function() {
                         saveOperation.resolve();
                     }).fail(function(result) {
-                        var errorMessage = null;
+                        var errorMessage = null,
+                            jsonResponse;
                         try {
-                            var jsonResponse = JSON.parse(result.responseText);
+                            jsonResponse = JSON.parse(result.responseText);
                             errorMessage = jsonResponse.error;
                         } catch (e) {
                             // Ignore the exception and show the default error message instead.
                         }
                         if (!errorMessage) {
-                            errorMessage = gettext("We've encountered an error. Refresh your browser and then try again.");
+                            errorMessage = gettext("We've encountered an error. Refresh your browser and then try again."); // eslint-disable-line max-len
                         }
-                        showErrorMessage(errorMessage, $element);
+                        showErrorMessage(errorMessage);
                         saveOperation.reject();
                     });
                     return saveOperation.promise();
@@ -92,6 +93,6 @@
                 }
 
             });
-            return CohortDiscussionConfigurationView;
+            return DividedDiscussionConfigurationView;
         });
 }).call(this, define || RequireJS.define);

lms/static/js/groups/views/cohort_discussions_course_wide.js → lms/static/js/discussions_management/views/divided_discussions_course_wide.js

 (function(define) {
     'use strict';
-    define(['jquery', 'underscore', 'backbone', 'gettext', 'js/groups/views/cohort_discussions',
+    define(['jquery', 'underscore', 'backbone', 'gettext', 'js/discussions_management/views/divided_discussions',
         'edx-ui-toolkit/js/utils/html-utils'],
-            function($, _, Backbone, gettext, CohortDiscussionConfigurationView, HtmlUtils) {
-                var CourseWideDiscussionsView = CohortDiscussionConfigurationView.extend({
+            function($, _, Backbone, gettext, DividedDiscussionConfigurationView, HtmlUtils) {
+                var CourseWideDiscussionsView = DividedDiscussionConfigurationView.extend({
                     events: {
                         'change .check-discussion-subcategory-course-wide': 'discussionCategoryStateChanged',
                         'click .cohort-course-wide-discussions-form .action-save': 'saveCourseWideDiscussionsForm'
                     },
 
                     initialize: function(options) {
-                        this.template = HtmlUtils.template($('#cohort-discussions-course-wide-tpl').text());
-                        this.cohortSettings = options.cohortSettings;
+                        this.template = HtmlUtils.template($('#divided-discussions-course-wide-tpl').text());
+                        this.discussionSettings = options.discussionSettings;
                     },
 
                     render: function() {
-                        HtmlUtils.setHtml(this.$('.cohort-course-wide-discussions-nav'), this.template({
+                        HtmlUtils.setHtml(this.$('.course-wide-discussions-nav'), this.template({
                             courseWideTopicsHtml: this.getCourseWideDiscussionsHtml(
                                 this.model.get('course_wide_discussions')
                             )
@@ -56,25 +56,27 @@
                     },
 
                     /**
-                     * Sends the cohorted_course_wide_discussions to the server and renders the view.
+                     * Sends the courseWideDividedDiscussions to the server and renders the view.
                      */
                     saveCourseWideDiscussionsForm: function(event) {
-                        event.preventDefault();
-
                         var self = this,
-                            courseWideCohortedDiscussions = self.getCohortedDiscussions(
+                            courseWideDividedDiscussions = self.getDividedDiscussions(
                                 '.check-discussion-subcategory-course-wide:checked'
                             ),
-                            fieldData = {cohorted_course_wide_discussions: courseWideCohortedDiscussions};
+                            fieldData = {divided_course_wide_discussions: courseWideDividedDiscussions};
+
+                        event.preventDefault();
 
                         self.saveForm(self.$('.course-wide-discussion-topics'), fieldData)
                         .done(function() {
                             self.model.fetch()
                                 .done(function() {
                                     self.render();
-                                    self.showMessage(gettext('Your changes have been saved.'), self.$('.course-wide-discussion-topics'));
+                                    self.showMessage(gettext('Your changes have been saved.'),
+                                                     self.$('.course-wide-discussion-topics')
+                                    );
                                 }).fail(function() {
-                                    var errorMessage = gettext("We've encountered an error. Refresh your browser and then try again.");
+                                    var errorMessage = gettext("We've encountered an error. Refresh your browser and then try again."); // eslint-disable-line max-len
                                     self.showMessage(errorMessage, self.$('.course-wide-discussion-topics'), 'error');
                                 });
                         });

lms/static/js/groups/views/cohort_discussions_inline.js → lms/static/js/discussions_management/views/divided_discussions_inline.js

 (function(define) {
     'use strict';
-    define(['jquery', 'underscore', 'backbone', 'gettext', 'js/groups/views/cohort_discussions',
+    define(['jquery', 'underscore', 'backbone', 'gettext', 'js/discussions_management/views/divided_discussions',
         'edx-ui-toolkit/js/utils/html-utils', 'js/vendor/jquery.qubit'],
-            function($, _, Backbone, gettext, CohortDiscussionConfigurationView, HtmlUtils) {
-                var InlineDiscussionsView = CohortDiscussionConfigurationView.extend({
+            function($, _, Backbone, gettext, DividedDiscussionConfigurationView, HtmlUtils) {
+                var InlineDiscussionsView = DividedDiscussionConfigurationView.extend({
                     events: {
                         'change .check-discussion-category': 'setSaveButton',
                         'change .check-discussion-subcategory-inline': 'setSaveButton',
@@ -13,17 +13,19 @@
                     },
 
                     initialize: function(options) {
-                        this.template = HtmlUtils.template($('#cohort-discussions-inline-tpl').text());
-                        this.cohortSettings = options.cohortSettings;
+                        this.template = HtmlUtils.template($('#divided-discussions-inline-tpl').text());
+                        this.discussionSettings = options.discussionSettings;
                     },
 
                     render: function() {
-                        var alwaysCohortInlineDiscussions = this.cohortSettings.get('always_cohort_inline_discussions'),
-                            inline_discussions = this.model.get('inline_discussions');
+                        var inlineDiscussions = this.model.get('inline_discussions'),
+                            alwaysDivideInlineDiscussions = this.discussionSettings.get(
+                                'always_divide_inline_discussions'
+                            );
 
-                        HtmlUtils.setHtml(this.$('.cohort-inline-discussions-nav'), this.template({
-                            inlineDiscussionTopicsHtml: this.getInlineDiscussionsHtml(inline_discussions),
-                            alwaysCohortInlineDiscussions: alwaysCohortInlineDiscussions
+                        HtmlUtils.setHtml(this.$('.inline-discussions-nav'), this.template({
+                            inlineDiscussionTopicsHtml: this.getInlineDiscussionsHtml(inlineDiscussions),
+                            alwaysDivideInlineDiscussions: alwaysDivideInlineDiscussions
                         }));
 
                         // Provides the semantics for a nested list of tri-state checkboxes.
@@ -32,7 +34,7 @@
                         // based on the checked values of any checkboxes in child elements of the DOM.
                         this.$('ul.inline-topics').qubit();
 
-                        this.setElementsEnabled(alwaysCohortInlineDiscussions, true);
+                        this.setElementsEnabled(alwaysDivideInlineDiscussions, true);
                     },
 
                     /**
@@ -99,45 +101,48 @@
                     *
                     * Enable/Disable the category and sub-category checkboxes.
                     * Enable/Disable the save button.
-                    * @param {bool} enable_checkboxes - The flag to enable/disable the checkboxes.
-                    * @param {bool} enable_save_button - The flag to enable/disable the save button.
+                    * @param {bool} enableCheckboxes - The flag to enable/disable the checkboxes.
+                    * @param {bool} enableSaveButton - The flag to enable/disable the save button.
                     */
-                    setElementsEnabled: function(enable_checkboxes, enable_save_button) {
-                        this.setDisabled(this.$('.check-discussion-category'), enable_checkboxes);
-                        this.setDisabled(this.$('.check-discussion-subcategory-inline'), enable_checkboxes);
-                        this.setDisabled(this.$('.cohort-inline-discussions-form .action-save'), enable_save_button);
+                    setElementsEnabled: function(enableCheckboxes, enableSaveButton) {
+                        this.setDisabled(this.$('.check-discussion-category'), enableCheckboxes);
+                        this.setDisabled(this.$('.check-discussion-subcategory-inline'), enableCheckboxes);
+                        this.setDisabled(this.$('.cohort-inline-discussions-form .action-save'), enableSaveButton);
                     },
 
                     /**
                     * Enables the save button for inline discussions.
                     */
-                    setSaveButton: function(event) {
+                    setSaveButton: function() {
                         this.setDisabled(this.$('.cohort-inline-discussions-form .action-save'), false);
                     },
 
                     /**
-                    * Sends the cohorted_inline_discussions to the server and renders the view.
+                    * Sends the dividedInlineDiscussions to the server and renders the view.
                     */
                     saveInlineDiscussionsForm: function(event) {
-                        event.preventDefault();
-
                         var self = this,
-                            cohortedInlineDiscussions = self.getCohortedDiscussions(
+                            dividedInlineDiscussions = self.getDividedDiscussions(
                                 '.check-discussion-subcategory-inline:checked'
                             ),
                             fieldData = {
-                                cohorted_inline_discussions: cohortedInlineDiscussions,
-                                always_cohort_inline_discussions: self.$('.check-all-inline-discussions').prop('checked')
+                                divided_inline_discussions: dividedInlineDiscussions,
+                                always_divide_inline_discussions: self.$(
+                                    '.check-all-inline-discussions'
+                                ).prop('checked')
                             };
 
+                        event.preventDefault();
+
                         self.saveForm(self.$('.inline-discussion-topics'), fieldData)
                             .done(function() {
                                 self.model.fetch()
                                     .done(function() {
                                         self.render();
-                                        self.showMessage(gettext('Your changes have been saved.'), self.$('.inline-discussion-topics'));
+                                        self.showMessage(gettext('Your changes have been saved.'),
+                                            self.$('.inline-discussion-topics'));
                                     }).fail(function() {
-                                        var errorMessage = gettext("We've encountered an error. Refresh your browser and then try again.");
+                                        var errorMessage = gettext("We've encountered an error. Refresh your browser and then try again."); // eslint-disable-line max-len
                                         self.showMessage(errorMessage, self.$('.inline-discussion-topics'), 'error');
                                     });
                             });

lms/static/js/groups/models/course_cohort_settings.js

@@ -4,10 +4,7 @@
         var CourseCohortSettingsModel = Backbone.Model.extend({
             idAttribute: 'id',
             defaults: {
-                is_cohorted: false,
-                cohorted_inline_discussions: [],
-                cohorted_course_wide_discussions: [],
-                always_cohort_inline_discussions: false
+                is_cohorted: false
             }
         });
         return CourseCohortSettingsModel;

lms/static/js/groups/views/cohorts.js

 (function(define) {
     'use strict';
     define(['jquery', 'underscore', 'backbone', 'gettext', 'js/groups/models/cohort',
-            'js/groups/models/verified_track_settings',
-            'js/groups/views/cohort_editor', 'js/groups/views/cohort_form',
-            'js/groups/views/course_cohort_settings_notification',
-            'js/groups/views/cohort_discussions_inline', 'js/groups/views/cohort_discussions_course_wide',
-            'js/groups/views/verified_track_settings_notification',
-            'edx-ui-toolkit/js/utils/html-utils',
-            'js/views/file_uploader', 'js/models/notification', 'js/views/notification', 'string_utils'],
-        function($, _, Backbone, gettext, CohortModel, VerifiedTrackSettingsModel, CohortEditorView, CohortFormView,
-                CourseCohortSettingsNotificationView, InlineDiscussionsView, CourseWideDiscussionsView,
-                 VerifiedTrackSettingsNotificationView, HtmlUtils) {
-            var hiddenClass = 'is-hidden',
+        'js/groups/models/verified_track_settings',
+        'js/groups/views/cohort_editor', 'js/groups/views/cohort_form',
+        'js/groups/views/course_cohort_settings_notification',
+        'js/groups/views/verified_track_settings_notification',
+        'edx-ui-toolkit/js/utils/html-utils',
+        'js/views/base_dashboard_view',
+        'js/views/file_uploader', 'js/models/notification', 'js/views/notification',
+        'string_utils'],
+        function($, _, Backbone, gettext, CohortModel,
+                 VerifiedTrackSettingsModel,
+                 CohortEditorView, CohortFormView,
+                CourseCohortSettingsNotificationView,
+                 VerifiedTrackSettingsNotificationView, HtmlUtils, BaseDashboardView) {
+            var hiddenClass = 'hidden',
                 disabledClass = 'is-disabled',
                 enableCohortsSelector = '.cohorts-state';
 
-
-            var CohortsView = Backbone.View.extend({
+            var CohortsView = BaseDashboardView.extend({
                 events: {
                     'change .cohort-select': 'onCohortSelected',
                     'change .cohorts-state': 'onCohortsEnabledChanged',
@@ -25,12 +27,10 @@
                     'click .cohort-management-add-form .action-cancel': 'cancelAddCohortForm',
                     'click .link-cross-reference': 'showSection',
                     'click .toggle-cohort-management-secondary': 'showCsvUpload',
-                    'click .toggle-cohort-management-discussions': 'showDiscussionTopics'
                 },
 
                 initialize: function(options) {
                     var model = this.model;
-
                     this.template = HtmlUtils.template($('#cohorts-tpl').text());
                     this.selectorTemplate = HtmlUtils.template($('#cohort-selector-tpl').text());
                     this.context = options.context;
@@ -154,6 +154,7 @@
                     ).done(function() {
                         self.render();
                         self.renderCourseCohortSettingsNotificationView();
+                        self.pubSub.trigger('cohorts:state', fieldData);
                     }).fail(function(result) {
                         self.showNotification({
                             type: 'error',
@@ -306,27 +307,6 @@
                         }).render();
                     }
                 },
-                showDiscussionTopics: function(event) {
-                    event.preventDefault();
-
-                    $(event.currentTarget).addClass(hiddenClass);
-                    var cohortDiscussionsElement = this.$('.cohort-discussions-nav').removeClass(hiddenClass);
-
-                    if (!this.CourseWideDiscussionsView) {
-                        this.CourseWideDiscussionsView = new CourseWideDiscussionsView({
-                            el: cohortDiscussionsElement,
-                            model: this.context.discussionTopicsSettingsModel,
-                            cohortSettings: this.cohortSettings
-                        }).render();
-                    }
-                    if (!this.InlineDiscussionsView) {
-                        this.InlineDiscussionsView = new InlineDiscussionsView({
-                            el: cohortDiscussionsElement,
-                            model: this.context.discussionTopicsSettingsModel,
-                            cohortSettings: this.cohortSettings
-                        }).render();
-                    }
-                },
 
                 getSectionCss: function(section) {
                     return ".instructor-nav .nav-item [data-section='" + section + "']";

lms/static/js/groups/views/cohorts_dashboard_factory.js

 (function(define, undefined) {
     'use strict';
     define(['jquery', 'js/groups/views/cohorts', 'js/groups/collections/cohort', 'js/groups/models/course_cohort_settings',
-            'js/groups/models/cohort_discussions', 'js/groups/models/content_group'],
-        function($, CohortsView, CohortCollection, CourseCohortSettingsModel, DiscussionTopicsSettingsModel, ContentGroupModel) {
+        'js/groups/models/content_group'],
+        function($, CohortsView, CohortCollection, CourseCohortSettingsModel, ContentGroupModel) {
             return function(contentGroups, studioGroupConfigurationsUrl) {
                 var contentGroupModels = $.map(contentGroups, function(group) {
                     return new ContentGroupModel({
@@ -14,33 +14,27 @@
 
                 var cohorts = new CohortCollection(),
                     courseCohortSettings = new CourseCohortSettingsModel(),
-                    discussionTopicsSettings = new DiscussionTopicsSettingsModel();
+                    $cohortManagementElement = $('.cohort-management');
 
-                var cohortManagementElement = $('.cohort-management');
-
-                cohorts.url = cohortManagementElement.data('cohorts_url');
-                courseCohortSettings.url = cohortManagementElement.data('course_cohort_settings_url');
-                discussionTopicsSettings.url = cohortManagementElement.data('discussion-topics-url');
+                cohorts.url = $cohortManagementElement.data('cohorts_url');
+                courseCohortSettings.url = $cohortManagementElement.data('course_cohort_settings_url');
 
                 var cohortsView = new CohortsView({
-                    el: cohortManagementElement,
+                    el: $cohortManagementElement,
                     model: cohorts,
                     contentGroups: contentGroupModels,
                     cohortSettings: courseCohortSettings,
                     context: {
-                        discussionTopicsSettingsModel: discussionTopicsSettings,
-                        uploadCohortsCsvUrl: cohortManagementElement.data('upload_cohorts_csv_url'),
-                        verifiedTrackCohortingUrl: cohortManagementElement.data('verified_track_cohorting_url'),
+                        uploadCohortsCsvUrl: $cohortManagementElement.data('upload_cohorts_csv_url'),
+                        verifiedTrackCohortingUrl: $cohortManagementElement.data('verified_track_cohorting_url'),
                         studioGroupConfigurationsUrl: studioGroupConfigurationsUrl,
-                        isCcxEnabled: cohortManagementElement.data('is_ccx_enabled')
+                        isCcxEnabled: $cohortManagementElement.data('is_ccx_enabled')
                     }
                 });
 
                 cohorts.fetch().done(function() {
                     courseCohortSettings.fetch().done(function() {
-                        discussionTopicsSettings.fetch().done(function() {
-                            cohortsView.render();
-                        });
+                        cohortsView.render();
                     });
                 });
             };

lms/static/js/instructor_dashboard/discussions_management.js

+(function() {
+    'use strict';
+
+    function DiscussionsManagement($section) {
+        this.$section = $section;
+        this.$section.data('wrapper', this);
+    }
+
+    DiscussionsManagement.prototype.onClickTitle = function() {};
+
+    window.InstructorDashboard.sections.DiscussionsManagement = DiscussionsManagement;
+}).call(this);

lms/static/js/instructor_dashboard/instructor_dashboard.js

@@ -188,6 +188,9 @@ such that the value can be defined later than this assignment (file load order).
                 constructor: window.InstructorDashboard.sections.CohortManagement,
                 $element: idashContent.find('.' + CSS_IDASH_SECTION + '#cohort_management')
             }, {
+                constructor: window.InstructorDashboard.sections.DiscussionsManagement,
+                $element: idashContent.find('.' + CSS_IDASH_SECTION + '#discussions_management')
+            }, {
                 constructor: window.InstructorDashboard.sections.Certificates,
                 $element: idashContent.find('.' + CSS_IDASH_SECTION + '#certificates')
             }, {

lms/static/js/views/base_dashboard_view.js

+(function(define) {
+    'use strict';
+    define(['jquery', 'backbone'],
+        function($, Backbone) {
+            // This Base view is useful when eventing or other features are shared between two or more
+            // views. Included with this view in the pubSub object allowing for events to be triggered
+            // and shared with other views.
+            var BaseDashboardView = Backbone.View.extend({
+                pubSub: $.extend({}, Backbone.Events)
+            });
+            return BaseDashboardView;
+        });
+}).call(this, define || RequireJS.define);

lms/static/lms/js/build.js

@@ -28,6 +28,7 @@
             'js/edxnotes/views/page_factory',
             'js/financial-assistance/financial_assistance_form_factory',
             'js/groups/views/cohorts_dashboard_factory',
+            'js/discussions_management/views/discussions_dashboard_factory',
             'js/header_factory',
             'js/learner_dashboard/program_details_factory',
             'js/learner_dashboard/program_list_factory',

lms/static/lms/js/spec/main.js

@@ -731,6 +731,7 @@
         'js/spec/edxnotes/views/visibility_decorator_spec.js',
         'js/spec/financial-assistance/financial_assistance_form_view_spec.js',
         'js/spec/groups/views/cohorts_spec.js',
+        'js/spec/groups/views/discussions_spec.js',
         'js/spec/instructor_dashboard/certificates_bulk_exception_spec.js',
         'js/spec/instructor_dashboard/certificates_exception_spec.js',
         'js/spec/instructor_dashboard/certificates_invalidation_spec.js',

lms/static/sass/course/instructor/_instructor_2.scss

@@ -1181,49 +1181,6 @@
     }
   }
 
-  // cohort discussions interface.
-  .cohort-discussions-nav {
-
-    .cohort-course-wide-discussions-form {
-
-      .form-actions {
-        padding-top: ($baseline/2);
-      }
-    }
-
-    .category-title,
-    .topic-name,
-    .all-inline-discussions,
-    .always_cohort_inline_discussions,
-    .cohort_inline_discussions {
-      padding-left: ($baseline/2);
-    }
-
-    .always_cohort_inline_discussions,
-    .cohort_inline_discussions {
-      padding-top: ($baseline/2);
-    }
-
-    .category-item,
-    .subcategory-item {
-      padding-top: ($baseline/2);
-    }
-
-    .cohorted-text {
-      color: $uxpl-blue-base;
-    }
-
-    .discussions-wrapper {
-      @extend %ui-no-list;
-      padding: 0 ($baseline/2);
-
-      .subcategories {
-        padding: 0 ($baseline*1.5);
-      }
-    }
-  }
-
-
   .wrapper-tabs { // This applies to the tab-like interface that toggles between the student management and the group settings
     @extend %ui-no-list;
     @extend %ui-depth1;
@@ -1280,6 +1237,107 @@
   }
 }
 
+// view - discussions management
+// --------------------
+.instructor-dashboard-wrapper-2 section.idash-section#discussions_management {
+
+  .division-scheme-container {
+    // See https://css-tricks.com/snippets/css/a-guide-to-flexbox/
+    display: flex;
+    flex-direction: column;
+    justify-content: space-between;
+
+    .division-scheme {
+      font-size: 18px;
+    }
+
+    .division-scheme-item {
+      padding-left: 1%;
+      padding-right: 1%;
+      float: left;
+    }
+
+    .three-column-layout {
+       max-width: 30%;
+    }
+
+    .two-column-layout {
+       max-width: 47%;
+    }
+
+    .field-message {
+        font-size: 13px;
+    }
+  }
+
+  // cohort management
+  .form-submit {
+    @include idashbutton($uxpl-blue-base);
+    @include font-size(14);
+    @include line-height(14);
+    margin-right: ($baseline/2);
+    margin-bottom: 0;
+    text-shadow: none;
+  }
+
+  .discussions-management-supplemental {
+    @extend %t-copy-sub1;
+    margin-top: $baseline;
+    padding: ($baseline/2) $baseline;
+    background: $gray-l6;
+    border-radius: ($baseline/10);
+  }
+
+  // cohort discussions interface.
+  .discussions-nav {
+
+    .cohort-course-wide-discussions-form {
+
+      .form-actions {
+        padding-top: ($baseline/2);
+      }
+    }
+
+    .category-title,
+    .topic-name,
+    .all-inline-discussions,
+    .always_divide_inline_discussions,
+    .divide_inline_discussions {
+      padding-left: ($baseline/2);
+    }
+
+    .always_divide_inline_discussions,
+    .divide_inline_discussions {
+      padding-top: ($baseline/2);
+    }
+
+    .category-item,
+    .subcategory-item {
+      padding-top: ($baseline/2);
+    }
+
+    .divided-discussion-text{
+      color: $uxpl-blue-base;
+    }
+
+    .discussions-wrapper {
+      @extend %ui-no-list;
+      padding: 0 ($baseline/2);
+
+      .subcategories {
+        padding: 0 ($baseline*1.5);
+      }
+    }
+  }
+  .wrapper-tabs {
+    @extend %ui-no-list;
+    @extend %ui-depth1;
+    position: relative;
+    top: 1px;
+    padding: 0 $baseline;
+  }
+}
+
 
 // view - student admin
 // --------------------

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_divided']).lower()}"
+        data-divided="${str(entries[entry]['is_divided']).lower()}"
         role="option"
     >
          % if entry:

lms/templates/discussion/_thread_list_template.html

@@ -21,14 +21,14 @@
                     %endif
                 </select>
             ## safe-lint: disable=python-parse-error,python-wrap-html
-            </label>${"<% if (isCohorted && isPrivilegedUser) { %>" | n, decode.utf8}<label class="forum-nav-filter-cohort">
-                ## Translators: This labels a cohort menu in forum navigation
-                <span class="sr">${_("Cohort:")}</span>
+            </label>${"<% if (isDiscussionDivisionEnabled && isPrivilegedUser) { %>" | n, decode.utf8}<label class="forum-nav-filter-cohort">
+                ## Translators: This labels a group menu in forum navigation
+                <span class="sr">${_("Group:")}</span>
                 <select class="forum-nav-filter-cohort-control">
-                    <option value="">${_("in all cohorts")}</option>
-                        ## cohorts is not iterable sometimes because inline discussions xmodule doesn't pass it
-                        %for c in (cohorts or []):
-                            <option value="${c['id']}">${c['name']}</option>
+                    <option value="">${_("in all groups")}</option>
+                        ## groups is not iterable sometimes because inline discussions xmodule doesn't pass it
+                        %for group in (groups or []):
+                            <option value="${group['id']}">${group['name']}</option>
                         %endfor
                 </select>
             ## safe-lint: disable=python-parse-error,python-wrap-html

lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore

@@ -3,7 +3,7 @@
         <label>
             <input data-id="<%- id %>" class="check-discussion-subcategory-<%- type %>" type="checkbox" <%- is_divided ? 'checked="checked"' : '' %> />
             <span class="topic-name"><%- name %></span>
-            <span class="cohorted-text <%- is_divided ? '' : 'hidden'%>">- <%- gettext('Cohorted') %></span>
+            <span class="divided-discussion-text <%- is_divided ? '' : 'hidden'%>">- <%- gettext('Divided') %></span>
         </label>
     </div>
 </li>

lms/templates/instructor/instructor_dashboard_2/cohort_management.html

@@ -13,7 +13,6 @@ from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_
      data-cohorts_url="${section_data['cohorts_url']}"
      data-upload_cohorts_csv_url="${section_data['upload_cohorts_csv_url']}"
      data-course_cohort_settings_url="${section_data['course_cohort_settings_url']}"
-     data-discussion-topics-url="${section_data['discussion_topics_url']}"
      data-verified_track_cohorting_url="${section_data['verified_track_cohorting_url']}"
      data-is_ccx_enabled="${'true' if section_data['ccx_is_enabled'] else 'false'}"
 >

lms/templates/instructor/instructor_dashboard_2/cohorts.underscore

@@ -35,7 +35,7 @@
 
         <!-- Uploading a CSV file of cohort assignments. -->
         <button class="toggle-cohort-management-secondary" data-href="#cohort-management-file-upload"><%- gettext('Assign students to cohorts by uploading a CSV file') %></button>
-        <div class="cohort-management-file-upload csv-upload is-hidden" id="cohort-management-file-upload" tabindex="-1"></div>
+        <div class="cohort-management-file-upload csv-upload hidden" id="cohort-management-file-upload" tabindex="-1"></div>
 
         <div class="cohort-management-supplemental">
             <p class="">
@@ -49,15 +49,5 @@
                 %>
             </p>
         </div>
-
-        <hr class="divider divider-lv1" />
-
-        <!-- Discussion Topics. -->
-        <button class="toggle-cohort-management-discussions" data-href="#cohort-discussions-management"><%- gettext('Specify whether discussion topics are divided by cohort') %></button>
-        <div class="cohort-discussions-nav is-hidden" id="cohort-discussions-management" tabindex="-1">
-            <div class="cohort-course-wide-discussions-nav"></div>
-            <div class="cohort-inline-discussions-nav"></div>
-        </div>
-
     </div>
 <% } %>

lms/templates/instructor/instructor_dashboard_2/discussions.underscore

+<!-- Discussion Topics. -->
+<div class="discussions-nav" id="discussions-management" tabindex="-1">
+    <div class="hd hd-3 subsection-title" id="division-scheme-title"><%- gettext('Specify whether discussion topics are divided') %></div>
+    <div class="division-scheme-container">
+        <div class="division-scheme-items" role="group" aria-labelledby="division-scheme-title">
+            <% for (var i = 0; i < availableSchemes.length; i++) { %>
+                <div class="division-scheme-item <%- availableSchemes[i].key %> <%- layoutClass %> <% if (!availableSchemes[i].enabled) { %>hidden<% } %>">
+                    <label class="division-scheme-label">
+                        <input class="division-scheme <%- availableSchemes[i].key %>" type="radio" name="division-scheme"
+                               value="<%- availableSchemes[i].key %>" aria-describedby="<%- availableSchemes[i].key %>-description"
+                        <% if (availableSchemes[i].selected) { %>
+                        checked
+                        <% } %>
+                        >
+                        <%- availableSchemes[i].displayName %>
+                    </label>
+                    <p class='field-message' id="<%- availableSchemes[i].key %>-description"><%- availableSchemes[i].descriptiveText %></p>
+                </div>
+            <% } %>
+        </div>
+    </div>
+    <div class="topic-division-nav">
+        <div class="course-wide-discussions-nav"></div>
+        <div class="inline-discussions-nav"></div>
+    </div>
+</div>

lms/templates/instructor/instructor_dashboard_2/discussions_management.html

+<%page expression_filter="h" args="section_data"/>
+
+<%namespace name='static' file='../../static_content.html'/>
+<%!
+from django.utils.translation import ugettext as _
+from openedx.core.djangolib.js_utils import js_escaped_string, dump_js_escaped_json
+from courseware.courses import get_studio_url
+from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition
+%>
+
+
+<div class="discussions-management"
+     data-discussion-topics-url="${section_data['discussion_topics_url']}"
+     data-course-discussion-settings-url="${section_data['course_discussion_settings']}"
+>
+</div>
+
+<%block name="js_extra">
+<%static:require_module module_name="js/discussions_management/views/discussions_dashboard_factory" class_name="DiscussionsFactory">
+    DiscussionsFactory();
+</%static:require_module>
+</%block>

lms/templates/instructor/instructor_dashboard_2/cohort-discussions-course-wide.underscore → lms/templates/instructor/instructor_dashboard_2/divided-discussions-course-wide.underscore

-<h3 class="hd hd-3 subsection-title"><%- gettext('Specify whether discussion topics are divided by cohort') %></h3>
 <form action="" method="post" id="cohort-course-wide-discussions-form" class="cohort-course-wide-discussions-form">
-    <div class="wrapper cohort-management-supplemental">
+    <div class="wrapper discussions-management-supplemental">
         <div class="form-fields">
             <div class="form-field">
                 <div class="course-wide-discussion-topics">
                     <h4 class="hd hd-4 subsection-title"><%- gettext('Course-Wide Discussion Topics') %></h4>
-                    <p><%- gettext('Select the course-wide discussion topics that you want to divide by cohort.') %></p>
+                    <p><%- gettext('Select the course-wide discussion topics that you want to divide.') %></p>
                     <div class="field">
                         <ul class="discussions-wrapper"><%= HtmlUtils.ensureHtml(courseWideTopicsHtml) %></ul>
                     </div>

lms/templates/instructor/instructor_dashboard_2/cohort-discussions-inline.underscore → lms/templates/instructor/instructor_dashboard_2/divided-discussions-inline.underscore

 <hr class="divider divider-lv1" />
 
 <form action="" method="post" id="cohort-inline-discussions-form" class="cohort-inline-discussions-form">
-    <div class="wrapper cohort-management-supplemental">
+    <div class="wrapper discussions-management-supplemental">
         <div class="form-fields">
             <div class="form-field">
                 <div class="inline-discussion-topics">
                     <h4 class="hd hd-4 subsection-title"><%- gettext('Content-Specific Discussion Topics') %></h4>
-                    <p><%- gettext('Specify whether content-specific discussion topics are divided by cohort.') %></p>
-                    <div class="always_cohort_inline_discussions">
+                    <p><%- gettext('Specify whether content-specific discussion topics are divided.') %></p>
+                    <div class="always_divide_inline_discussions">
                         <label>
-                            <input type="radio" name="inline" class="check-all-inline-discussions" <%- alwaysCohortInlineDiscussions ? 'checked="checked"' : '' %>/>
-                            <span class="all-inline-discussions"><%- gettext('Always cohort content-specific discussion topics') %></span>
+                            <input type="radio" name="inline" class="check-all-inline-discussions" <%- alwaysDivideInlineDiscussions ? 'checked="checked"' : '' %>/>
+                            <span class="all-inline-discussions"><%- gettext('Always divide content-specific discussion topics') %></span>
                         </label>
                     </div>
-                    <div class="cohort_inline_discussions">
+                    <div class="divide_inline_discussions">
                         <label>
-                            <input type="radio" name="inline" class="check-cohort-inline-discussions" <%- alwaysCohortInlineDiscussions ? '' : 'checked="checked"' %>/>
-                            <span class="all-inline-discussions"><%- gettext('Cohort selected content-specific discussion topics') %></span>
+                            <input type="radio" name="inline" class="check-cohort-inline-discussions" <%- alwaysDivideInlineDiscussions ? '' : 'checked="checked"' %>/>
+                            <span class="all-inline-discussions"><%- gettext('Divide the selected content-specific discussion topics') %></span>
                         </label>
                     </div>
                     <hr class="divider divider-lv1" />

lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html

@@ -81,7 +81,7 @@ from openedx.core.djangolib.markup import HTML
 
 ## Include Underscore templates
 <%block name="header_extras">
-% for template_name in ["cohorts", "enrollment-code-lookup-links", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state", "cohort-discussions-inline", "cohort-discussions-course-wide", "cohort-discussions-category", "cohort-discussions-subcategory", "certificate-white-list", "certificate-white-list-editor", "certificate-bulk-white-list", "certificate-invalidation"]:
+% for template_name in ["cohorts", "discussions", "enrollment-code-lookup-links", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state", "divided-discussions-inline", "divided-discussions-course-wide", "cohort-discussions-category", "cohort-discussions-subcategory", "certificate-white-list", "certificate-white-list-editor", "certificate-bulk-white-list", "certificate-invalidation"]:
 <script type="text/template" id="${template_name}-tpl">
   <%static:include path="instructor/instructor_dashboard_2/${template_name}.underscore" />
 </script>
@@ -121,9 +121,10 @@ from openedx.core.djangolib.markup import HTML
         ## when the javascript loads, it clicks on the first section
         <ul class="instructor-nav">
           % for section_data in sections:
+            <% is_hidden = section_data.get('is_hidden', False) %>
             ## This is necessary so we don't scrape 'section_display_name' as a string.
             <% dname = section_data['section_display_name'] %>
-            <li class="nav-item"><button type="button" class="btn-link" data-section="${ section_data['section_key'] }">${_(dname)}</button></li>
+            <li class="nav-item"><button type="button" class="btn-link ${ section_data['section_key'] }${' hidden' if is_hidden else ''}" data-section="${ section_data['section_key'] }">${_(dname)}</button></li>
           % endfor
         </ul>
 
@@ -131,7 +132,8 @@ from openedx.core.djangolib.markup import HTML
         ## to keep this short, sections can be pulled out into their own files
 
         % for section_data in sections:
-          <section id="${ section_data['section_key'] }" class="idash-section" aria-labelledby="header-${section_data['section_key']}">
+          <% is_hidden = section_data.get('is_hidden', False) %>
+          <section id="${ section_data['section_key'] }" class="idash-section${' hidden' if hidden else ''}" aria-labelledby="header-${section_data['section_key']}">
               <h3 class="hd hd-3" id="header-${ section_data['section_key'] }">${ section_data['section_display_name'] }</h3>
               <%include file="${ section_data['section_key'] }.html" args="section_data=section_data" />
           </section>

lms/urls.py

@@ -504,6 +504,15 @@ urlpatterns += (
         include(COURSE_URLS)
     ),
 
+    # Discussions Management
+    url(
+        r'^courses/{}/discussions/settings$'.format(
+            settings.COURSE_KEY_PATTERN,
+        ),
+        'lms.djangoapps.discussion.views.course_discussions_settings_handler',
+        name='course_discussions_settings',
+    ),
+
     # Cohorts management
     url(
         r'^courses/{}/cohorts/settings$'.format(
@@ -548,11 +557,11 @@ urlpatterns += (
         name='debug_cohort_mgmt',
     ),
     url(
-        r'^courses/{}/cohorts/topics$'.format(
+        r'^courses/{}/discussion/topics$'.format(
             settings.COURSE_KEY_PATTERN,
         ),
-        'openedx.core.djangoapps.course_groups.views.cohort_discussion_topics',
-        name='cohort_discussion_topics',
+        'lms.djangoapps.discussion.views.discussion_topics',
+        name='discussion_topics',
     ),
     url(
         r'^courses/{}/verified_track_content/settings'.format(

openedx/core/djangoapps/course_groups/cohorts.py

@@ -118,32 +118,40 @@ def is_course_cohorted(course_key):
     Raises:
        Http404 if the course doesn't exist.
     """
-    return get_course_cohort_settings(course_key).is_cohorted
+    return _get_course_cohort_settings(course_key).is_cohorted
 
 
-def get_cohort_id(user, course_key, use_cached=False):
+def get_course_cohort_id(course_key):
     """
-    Given a course key and a user, return the id of the cohort that user is
-    assigned to in that course.  If they don't have a cohort, return None.
+    Given a course key, return the int id for the cohort settings.
+
+    Raises:
+        Http404 if the course doesn't exist.
     """
-    cohort = get_cohort(user, course_key, use_cached=use_cached)
-    return None if cohort is None else cohort.id
+    return _get_course_cohort_settings(course_key).id
 
 
-def get_cohorted_commentables(course_key):
+def set_course_cohorted(course_key, cohorted):
     """
-    Given a course_key return a set of strings representing cohorted commentables.
+    Given a course course and a boolean, sets whether or not the course is cohorted.
+
+    Raises:
+        Value error if `cohorted` is not a boolean
     """
+    if not isinstance(cohorted, bool):
+        raise ValueError("Cohorted must be a boolean")
+    course_cohort_settings = _get_course_cohort_settings(course_key)
+    course_cohort_settings.is_cohorted = cohorted
+    course_cohort_settings.save()
 
-    course_cohort_settings = get_course_cohort_settings(course_key)
 
-    if not course_cohort_settings.is_cohorted:
-        # this is the easy case :)
-        ans = set()
-    else:
-        ans = set(course_cohort_settings.cohorted_discussions)
-
-    return ans
+def get_cohort_id(user, course_key, use_cached=False):
+    """
+    Given a course key and a user, return the id of the cohort that user is
+    assigned to in that course.  If they don't have a cohort, return None.
+    """
+    cohort = get_cohort(user, course_key, use_cached=use_cached)
+    return None if cohort is None else cohort.id
 
 
 COHORT_CACHE_NAMESPACE = u"cohorts.get_cohort"
@@ -213,8 +221,7 @@ def get_cohort(user, course_key, assign=True, use_cached=False):
 
     # First check whether the course is cohorted (users shouldn't be in a cohort
     # in non-cohorted courses, but settings can change after course starts)
-    course_cohort_settings = get_course_cohort_settings(course_key)
-    if not course_cohort_settings.is_cohorted:
+    if not is_course_cohorted(course_key):
         return cache.setdefault(cache_key, None)
 
     # If course is cohorted, check if the user already has a cohort.
@@ -277,11 +284,7 @@ def migrate_cohort_settings(course):
     """
     cohort_settings, created = CourseCohortsSettings.objects.get_or_create(
         course_id=course.id,
-        defaults={
-            'is_cohorted': course.is_cohorted,
-            'cohorted_discussions': list(course.cohorted_discussions),
-            'always_cohort_inline_discussions': course.always_cohort_inline_discussions
-        }
+        defaults=_get_cohort_settings_from_modulestore(course)
     )
 
     # Add the new and update the existing cohorts
@@ -507,50 +510,49 @@ def is_last_random_cohort(user_group):
     return len(random_cohorts) == 1 and random_cohorts[0].name == user_group.name
 
 
-def set_course_cohort_settings(course_key, **kwargs):
+@request_cached
+def _get_course_cohort_settings(course_key):
     """
-    Set cohort settings for a course.
+    Return cohort settings for a course. NOTE that the only non-deprecated fields in
+    CourseCohortSettings are `course_id` and  `is_cohorted`. Other fields should only be used for
+    migration purposes.
 
     Arguments:
         course_key: CourseKey
-        is_cohorted (bool): If the course should be cohorted.
-        always_cohort_inline_discussions (bool): If inline discussions should always be cohorted.
-        cohorted_discussions (list): List of discussion ids.
 
     Returns:
-        A CourseCohortSettings object.
+        A CourseCohortSettings object. NOTE that the only non-deprecated field in
+        CourseCohortSettings are `course_id` and  `is_cohorted`. Other fields should only be used
+        for migration purposes.
 
     Raises:
         Http404 if course_key is invalid.
     """
-    fields = {'is_cohorted': bool, 'always_cohort_inline_discussions': bool, 'cohorted_discussions': list}
-    course_cohort_settings = get_course_cohort_settings(course_key)
-    for field, field_type in fields.items():
-        if field in kwargs:
-            if not isinstance(kwargs[field], field_type):
-                raise ValueError("Incorrect field type for `{}`. Type must be `{}`".format(field, field_type.__name__))
-            setattr(course_cohort_settings, field, kwargs[field])
-    course_cohort_settings.save()
+    try:
+        course_cohort_settings = CourseCohortsSettings.objects.get(course_id=course_key)
+    except CourseCohortsSettings.DoesNotExist:
+        course = courses.get_course_by_id(course_key)
+        course_cohort_settings = migrate_cohort_settings(course)
     return course_cohort_settings
 
 
-@request_cached
-def get_course_cohort_settings(course_key):
-    """
-    Return cohort settings for a course.
-
-    Arguments:
-        course_key: CourseKey
+def get_legacy_discussion_settings(course_key):
 
-    Returns:
-        A CourseCohortSettings object.
-
-    Raises:
-        Http404 if course_key is invalid.
-    """
     try:
         course_cohort_settings = CourseCohortsSettings.objects.get(course_id=course_key)
+        return {
+            'is_cohorted': course_cohort_settings.is_cohorted,
+            'cohorted_discussions': course_cohort_settings.cohorted_discussions,
+            'always_cohort_inline_discussions': course_cohort_settings.always_cohort_inline_discussions
+        }
     except CourseCohortsSettings.DoesNotExist:
         course = courses.get_course_by_id(course_key)
-        course_cohort_settings = migrate_cohort_settings(course)
-    return course_cohort_settings
+        return _get_cohort_settings_from_modulestore(course)
+
+
+def _get_cohort_settings_from_modulestore(course):
+    return {
+        'is_cohorted': course.is_cohorted,
+        'cohorted_discussions': list(course.cohorted_discussions),
+        'always_cohort_inline_discussions': course.always_cohort_inline_discussions
+    }

openedx/core/djangoapps/course_groups/models.py

@@ -168,6 +168,7 @@ class CourseUserGroupPartitionGroup(models.Model):
 class CourseCohortsSettings(models.Model):
     """
     This model represents cohort settings for courses.
+    The only non-deprecated fields are `is_cohorted` and `course_id`.
     """
     is_cohorted = models.BooleanField(default=False)
 
@@ -184,16 +185,23 @@ class CourseCohortsSettings(models.Model):
     # in reality the default value at the time that cohorting is enabled for a course comes from
     # course_module.always_cohort_inline_discussions (via `migrate_cohort_settings`).
     # pylint: disable=invalid-name
+    # DEPRECATED-- DO NOT USE: Instead use `CourseDiscussionSettings.always_divide_inline_discussions`
+    # via `get_course_discussion_settings` or `set_course_discussion_settings`.
     always_cohort_inline_discussions = models.BooleanField(default=False)
 
     @property
     def cohorted_discussions(self):
-        """Jsonify the cohorted_discussions"""
+        """
+        DEPRECATED-- DO NOT USE. Instead use `CourseDiscussionSettings.divided_discussions`
+        via `get_course_discussion_settings`.
+        """
         return json.loads(self._cohorted_discussions)
 
     @cohorted_discussions.setter
     def cohorted_discussions(self, value):
-        """Un-Jsonify the cohorted_discussions"""
+        """
+        DEPRECATED-- DO NOT USE. Instead use `CourseDiscussionSettings` via `set_course_discussion_settings`.
+        """
         self._cohorted_discussions = json.dumps(value)
 
 

openedx/core/djangoapps/course_groups/tests/helpers.py

@@ -2,16 +2,18 @@
 Helper methods for testing cohorts.
 """
 
-from factory import post_generation, Sequence
-from factory.django import DjangoModelFactory
 import json
 
+from django_comment_common.models import CourseDiscussionSettings
+from django_comment_common.utils import set_course_discussion_settings
+from factory import Sequence, post_generation
+from factory.django import DjangoModelFactory
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
-from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore.django import modulestore
 
-from ..cohorts import set_course_cohort_settings
-from ..models import CourseUserGroup, CourseCohort, CourseCohortsSettings, CohortMembership
+from ..cohorts import set_course_cohorted
+from ..models import CohortMembership, CourseCohort, CourseCohortsSettings, CourseUserGroup
 
 
 class CohortFactory(DjangoModelFactory):
@@ -61,25 +63,10 @@ class CourseCohortSettingsFactory(DjangoModelFactory):
     always_cohort_inline_discussions = False
 
 
-def topic_name_to_id(course, name):
-    """
-    Given a discussion topic name, return an id for that name (includes
-    course and url_name).
-    """
-    return "{course}_{run}_{name}".format(
-        course=course.location.course,
-        run=course.url_name,
-        name=name
-    )
-
-
 def config_course_cohorts_legacy(
         course,
-        discussions,
         cohorted,
-        cohorted_discussions=None,
-        auto_cohort_groups=None,
-        always_cohort_inline_discussions=None
+        auto_cohort_groups=None
 ):
     """
     Given a course with no discussion set up, add the discussions and set
@@ -91,39 +78,19 @@ def config_course_cohorts_legacy(
 
     Arguments:
         course: CourseDescriptor
-        discussions: list of topic names strings.  Picks ids and sort_keys
-            automatically.
         cohorted: bool.
-        cohorted_discussions: optional list of topic names.  If specified,
-            converts them to use the same ids as topic names.
         auto_cohort_groups: optional list of strings
                   (names of groups to put students into).
 
     Returns:
         Nothing -- modifies course in place.
     """
-    def to_id(name):
-        """
-        Helper method to convert a discussion topic name to a database identifier
-        """
-        return topic_name_to_id(course, name)
-
-    topics = dict((name, {"sort_key": "A",
-                          "id": to_id(name)})
-                  for name in discussions)
-
-    course.discussion_topics = topics
+    course.discussion_topics = {}
 
     config = {"cohorted": cohorted}
-    if cohorted_discussions is not None:
-        config["cohorted_discussions"] = [to_id(name)
-                                          for name in cohorted_discussions]
     if auto_cohort_groups is not None:
         config["auto_cohort_groups"] = auto_cohort_groups
 
-    if always_cohort_inline_discussions is not None:
-        config["always_cohort_inline_discussions"] = always_cohort_inline_discussions
-
     course.cohort_config = config
 
     try:
@@ -137,39 +104,29 @@ def config_course_cohorts_legacy(
 def config_course_cohorts(
         course,
         is_cohorted,
+        discussion_division_scheme=CourseDiscussionSettings.COHORT,
         auto_cohorts=[],
         manual_cohorts=[],
-        discussion_topics=[],
-        cohorted_discussions=[],
-        always_cohort_inline_discussions=False
 ):
     """
-    Set discussions and configure cohorts for a course.
+    Set and configure cohorts for a course.
 
     Arguments:
         course: CourseDescriptor
         is_cohorted (bool): Is the course cohorted?
+        discussion_division_scheme (String): the division scheme for discussions. Default is
+            CourseDiscussionSettings.COHORT.
         auto_cohorts (list): Names of auto cohorts to create.
         manual_cohorts (list): Names of manual cohorts to create.
-        discussion_topics (list): Discussion topic names. Picks ids and
-            sort_keys automatically.
-        cohorted_discussions: Discussion topics to cohort. Converts the
-            list to use the same ids as discussion topic names.
-        always_cohort_inline_discussions (bool): Whether inline discussions
-            should be cohorted by default.
 
     Returns:
         Nothing -- modifies course in place.
     """
-    def to_id(name):
-        """Convert name to id."""
-        return topic_name_to_id(course, name)
 
-    set_course_cohort_settings(
+    set_course_cohorted(course.id, is_cohorted)
+    set_course_discussion_settings(
         course.id,
-        is_cohorted=is_cohorted,
-        cohorted_discussions=[to_id(name) for name in cohorted_discussions],
-        always_cohort_inline_discussions=always_cohort_inline_discussions
+        division_scheme=discussion_division_scheme,
     )
 
     for cohort_name in auto_cohorts:
@@ -180,8 +137,6 @@ def config_course_cohorts(
         cohort = CohortFactory(course_id=course.id, name=cohort_name)
         CourseCohortFactory(course_user_group=cohort, assignment_type=CourseCohort.MANUAL)
 
-    course.discussion_topics = dict((name, {"sort_key": "A", "id": to_id(name)})
-                                    for name in discussion_topics)
     try:
         # Not implemented for XMLModulestore, which is used by test_cohorts.
         modulestore().update_item(course, ModuleStoreEnum.UserID.test)

openedx/core/djangoapps/course_groups/tests/test_cohorts.py

@@ -5,13 +5,12 @@ Tests for cohorts
 import ddt
 from mock import call, patch
 from nose.plugins.attrib import attr
-import before_after
 
+import before_after
 from django.contrib.auth.models import User
 from django.db import IntegrityError
 from django.http import Http404
 from django.test import TestCase
-
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
@@ -19,12 +18,9 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_MODULESTORE, ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import ToyCourseFactory
 
-from ..models import CourseUserGroup, CourseCohort, CourseUserGroupPartitionGroup
 from .. import cohorts
-from ..tests.helpers import (
-    topic_name_to_id, config_course_cohorts, config_course_cohorts_legacy,
-    CohortFactory, CourseCohortFactory, CourseCohortSettingsFactory
-)
+from ..models import CourseCohort, CourseUserGroup, CourseUserGroupPartitionGroup
+from ..tests.helpers import CohortFactory, CourseCohortFactory, config_course_cohorts, config_course_cohorts_legacy
 
 
 @attr(shard=2)
@@ -350,7 +346,6 @@ class TestCohorts(ModuleStoreTestCase):
         # This will have no effect on lms side as we are already done with migrations
         config_course_cohorts_legacy(
             course,
-            discussions=[],
             cohorted=True,
             auto_cohort_groups=["OtherGroup"]
         )
@@ -393,7 +388,6 @@ class TestCohorts(ModuleStoreTestCase):
         # This will have no effect on lms side as we are already done with migrations
         config_course_cohorts_legacy(
             course,
-            discussions=[],
             cohorted=True,
             auto_cohort_groups=["AutoGroup"]
         )
@@ -475,44 +469,6 @@ class TestCohorts(ModuleStoreTestCase):
             {cohort1.id: cohort1.name, cohort2.id: cohort2.name}
         )
 
-    def test_get_cohorted_commentables(self):
-        """
-        Make sure cohorts.get_cohorted_commentables() correctly returns a list of strings representing cohorted
-        commentables.  Also verify that we can't get the cohorted commentables from a course which does not exist.
-        """
-        course = modulestore().get_course(self.toy_course_key)
-
-        self.assertEqual(cohorts.get_cohorted_commentables(course.id), set())
-
-        config_course_cohorts(course, is_cohorted=True)
-        self.assertEqual(cohorts.get_cohorted_commentables(course.id), set())
-
-        config_course_cohorts(
-            course,
-            is_cohorted=True,
-            discussion_topics=["General", "Feedback"],
-            cohorted_discussions=["Feedback"]
-        )
-        self.assertItemsEqual(
-            cohorts.get_cohorted_commentables(course.id),
-            set([topic_name_to_id(course, "Feedback")])
-        )
-
-        config_course_cohorts(
-            course,
-            is_cohorted=True,
-            discussion_topics=["General", "Feedback"],
-            cohorted_discussions=["General", "Feedback"]
-        )
-        self.assertItemsEqual(
-            cohorts.get_cohorted_commentables(course.id),
-            set([topic_name_to_id(course, "General"), topic_name_to_id(course, "Feedback")])
-        )
-        self.assertRaises(
-            Http404,
-            lambda: cohorts.get_cohorted_commentables(SlashSeparatedCourseKey("course", "does_not", "exist"))
-        )
-
     def test_get_cohort_by_name(self):
         """
         Make sure cohorts.get_cohort_by_name() properly finds a cohort by name for a given course.  Also verify that it
@@ -672,59 +628,16 @@ class TestCohorts(ModuleStoreTestCase):
         # Note that the following get() will fail with MultipleObjectsReturned if race condition is not handled.
         self.assertEqual(first_cohort.users.get(), course_user)
 
-    def test_get_course_cohort_settings(self):
-        """
-        Test that cohorts.get_course_cohort_settings is working as expected.
-        """
-        course = modulestore().get_course(self.toy_course_key)
-        course_cohort_settings = cohorts.get_course_cohort_settings(course.id)
-
-        self.assertFalse(course_cohort_settings.is_cohorted)
-        self.assertEqual(course_cohort_settings.cohorted_discussions, [])
-        self.assertFalse(course_cohort_settings.always_cohort_inline_discussions)
-
-    def test_update_course_cohort_settings(self):
+    def test_set_cohorted_with_invalid_data_type(self):
         """
-        Test that cohorts.set_course_cohort_settings is working as expected.
+        Test that cohorts.set_course_cohorted raises exception if argument is not a boolean.
         """
         course = modulestore().get_course(self.toy_course_key)
-        CourseCohortSettingsFactory(course_id=course.id)
-
-        cohorts.set_course_cohort_settings(
-            course.id,
-            is_cohorted=False,
-            cohorted_discussions=['topic a id', 'topic b id'],
-            always_cohort_inline_discussions=True
-        )
-
-        course_cohort_settings = cohorts.get_course_cohort_settings(course.id)
 
-        self.assertFalse(course_cohort_settings.is_cohorted)
-        self.assertEqual(course_cohort_settings.cohorted_discussions, ['topic a id', 'topic b id'])
-        self.assertTrue(course_cohort_settings.always_cohort_inline_discussions)
+        with self.assertRaises(ValueError) as value_error:
+            cohorts.set_course_cohorted(course.id, 'not a boolean')
 
-    def test_update_course_cohort_settings_with_invalid_data_type(self):
-        """
-        Test that cohorts.set_course_cohort_settings raises exception if fields have incorrect data type.
-        """
-        course = modulestore().get_course(self.toy_course_key)
-        CourseCohortSettingsFactory(course_id=course.id)
-
-        exception_msg_tpl = "Incorrect field type for `{}`. Type must be `{}`"
-        fields = [
-            {'name': 'is_cohorted', 'type': bool},
-            {'name': 'always_cohort_inline_discussions', 'type': bool},
-            {'name': 'cohorted_discussions', 'type': list}
-        ]
-
-        for field in fields:
-            with self.assertRaises(ValueError) as value_error:
-                cohorts.set_course_cohort_settings(course.id, **{field['name']: ''})
-
-            self.assertEqual(
-                value_error.exception.message,
-                exception_msg_tpl.format(field['name'], field['type'].__name__)
-            )
+        self.assertEqual("Cohorted must be a boolean", value_error.exception.message)
 
 
 @attr(shard=2)

openedx/core/djangoapps/course_groups/views.py

@@ -5,6 +5,7 @@ Views related to course groups functionality.
 import logging
 import re
 
+from courseware.courses import get_course_with_access
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
 from django.core.paginator import EmptyPage, Paginator
@@ -14,13 +15,9 @@ from django.http import Http404, HttpResponseBadRequest
 from django.utils.translation import ugettext
 from django.views.decorators.csrf import ensure_csrf_cookie
 from django.views.decorators.http import require_http_methods, require_POST
+from edxmako.shortcuts import render_to_response
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
-
-from courseware.courses import get_course_with_access
-from edxmako.shortcuts import render_to_response
-from lms.djangoapps.django_comment_client.constants import TYPE_ENTRY
-from lms.djangoapps.django_comment_client.utils import get_discussion_categories_ids, get_discussion_category_map
 from util.json_request import JsonResponse, expect_json
 
 from . import cohorts
@@ -63,20 +60,13 @@ def unlink_cohort_partition_group(cohort):
 
 
 # pylint: disable=invalid-name
-def _get_course_cohort_settings_representation(course, course_cohort_settings):
+def _get_course_cohort_settings_representation(cohort_id, is_cohorted):
     """
     Returns a JSON representation of a course cohort settings.
     """
-    cohorted_course_wide_discussions, cohorted_inline_discussions = get_cohorted_discussions(
-        course, course_cohort_settings
-    )
-
     return {
-        'id': course_cohort_settings.id,
-        'is_cohorted': course_cohort_settings.is_cohorted,
-        'cohorted_inline_discussions': cohorted_inline_discussions,
-        'cohorted_course_wide_discussions': cohorted_course_wide_discussions,
-        'always_cohort_inline_discussions': course_cohort_settings.always_cohort_inline_discussions,
+        'id': cohort_id,
+        'is_cohorted': is_cohorted,
     }
 
 
@@ -97,25 +87,6 @@ def _get_cohort_representation(cohort, course):
     }
 
 
-def get_cohorted_discussions(course, course_settings):
-    """
-    Returns the course-wide and inline cohorted discussion ids separately.
-    """
-    cohorted_course_wide_discussions = []
-    cohorted_inline_discussions = []
-
-    course_wide_discussions = [topic['id'] for __, topic in course.discussion_topics.items()]
-    all_discussions = get_discussion_categories_ids(course, None, include_all=True)
-
-    for cohorted_discussion_id in course_settings.cohorted_discussions:
-        if cohorted_discussion_id in course_wide_discussions:
-            cohorted_course_wide_discussions.append(cohorted_discussion_id)
-        elif cohorted_discussion_id in all_discussions:
-            cohorted_inline_discussions.append(cohorted_discussion_id)
-
-    return cohorted_course_wide_discussions, cohorted_inline_discussions
-
-
 @require_http_methods(("GET", "PATCH"))
 @ensure_csrf_cookie
 @expect_json
@@ -130,45 +101,24 @@ def course_cohort_settings_handler(request, course_key_string):
         Updates the cohort settings for the course. Returns the JSON representation of updated settings.
     """
     course_key = CourseKey.from_string(course_key_string)
-    course = get_course_with_access(request.user, 'staff', course_key)
-    cohort_settings = cohorts.get_course_cohort_settings(course_key)
+    # Although this course data is not used this method will return 404 is user is not staff
+    get_course_with_access(request.user, 'staff', course_key)
 
     if request.method == 'PATCH':
-        cohorted_course_wide_discussions, cohorted_inline_discussions = get_cohorted_discussions(
-            course, cohort_settings
-        )
-
-        settings_to_change = {}
-
-        if 'is_cohorted' in request.json:
-            settings_to_change['is_cohorted'] = request.json.get('is_cohorted')
-
-        if 'cohorted_course_wide_discussions' in request.json or 'cohorted_inline_discussions' in request.json:
-            cohorted_course_wide_discussions = request.json.get(
-                'cohorted_course_wide_discussions', cohorted_course_wide_discussions
-            )
-            cohorted_inline_discussions = request.json.get(
-                'cohorted_inline_discussions', cohorted_inline_discussions
-            )
-            settings_to_change['cohorted_discussions'] = cohorted_course_wide_discussions + cohorted_inline_discussions
-
-        if 'always_cohort_inline_discussions' in request.json:
-            settings_to_change['always_cohort_inline_discussions'] = request.json.get(
-                'always_cohort_inline_discussions'
-            )
-
-        if not settings_to_change:
+        if 'is_cohorted' not in request.json:
             return JsonResponse({"error": unicode("Bad Request")}, 400)
 
+        is_cohorted = request.json.get('is_cohorted')
         try:
-            cohort_settings = cohorts.set_course_cohort_settings(
-                course_key, **settings_to_change
-            )
+            cohorts.set_course_cohorted(course_key, is_cohorted)
         except ValueError as err:
             # Note: error message not translated because it is not exposed to the user (UI prevents this state).
             return JsonResponse({"error": unicode(err)}, 400)
 
-    return JsonResponse(_get_course_cohort_settings_representation(course, cohort_settings))
+    return JsonResponse(_get_course_cohort_settings_representation(
+        cohorts.get_course_cohort_id(course_key),
+        cohorts.is_course_cohorted(course_key)
+    ))
 
 
 @require_http_methods(("GET", "PUT", "POST", "PATCH"))
@@ -417,81 +367,3 @@ def debug_cohort_mgmt(request, course_key_string):
         kwargs={'course_key': course_key.to_deprecated_string()}
     )}
     return render_to_response('/course_groups/debug.html', context)
-
-
-@expect_json
-@login_required
-def cohort_discussion_topics(request, course_key_string):
-    """
-    The handler for cohort discussion categories requests.
-    This will raise 404 if user is not staff.
-
-    Returns the JSON representation of discussion topics w.r.t categories for the course.
-
-    Example:
-        >>> example = {
-        >>>               "course_wide_discussions": {
-        >>>                   "entries": {
-        >>>                       "General": {
-        >>>                           "sort_key": "General",
-        >>>                           "is_divided": True,
-        >>>                           "id": "i4x-edx-eiorguegnru-course-foobarbaz"
-        >>>                       }
-        >>>                   }
-        >>>                   "children": ["General", "entry"]
-        >>>               },
-        >>>               "inline_discussions" : {
-        >>>                   "subcategories": {
-        >>>                       "Getting Started": {
-        >>>                           "subcategories": {},
-        >>>                           "children": [
-        >>>                               ["Working with Videos", "entry"],
-        >>>                               ["Videos on edX", "entry"]
-        >>>                           ],
-        >>>                           "entries": {
-        >>>                               "Working with Videos": {
-        >>>                                   "sort_key": None,
-        >>>                                   "is_divided": False,
-        >>>                                   "id": "d9f970a42067413cbb633f81cfb12604"
-        >>>                               },
-        >>>                               "Videos on edX": {
-        >>>                                   "sort_key": None,
-        >>>                                   "is_divided": False,
-        >>>                                   "id": "98d8feb5971041a085512ae22b398613"
-        >>>                               }
-        >>>                           }
-        >>>                       },
-        >>>                       "children": ["Getting Started", "subcategory"]
-        >>>                   },
-        >>>               }
-        >>>          }
-    """
-    course_key = CourseKey.from_string(course_key_string)
-    course = get_course_with_access(request.user, 'staff', course_key)
-
-    discussion_topics = {}
-    discussion_category_map = get_discussion_category_map(
-        course, request.user, divided_only_if_explicit=True, exclude_unstarted=False
-    )
-
-    # We extract the data for the course wide discussions from the category map.
-    course_wide_entries = discussion_category_map.pop('entries')
-
-    course_wide_children = []
-    inline_children = []
-
-    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, c_type])
-
-    discussion_topics['course_wide_discussions'] = {
-        'entries': course_wide_entries,
-        'children': course_wide_children
-    }
-
-    discussion_category_map['children'] = inline_children
-    discussion_topics['inline_discussions'] = discussion_category_map
-
-    return JsonResponse(discussion_topics)

openedx/core/djangoapps/verified_track_content/partition_scheme.py

 """
 UserPartitionScheme for enrollment tracks.
 """
-from django.conf import settings
-
+from course_modes.models import CourseMode
 from courseware.masquerade import (
     get_course_masquerade,
     get_masquerading_user_group,
     is_masquerading_as_specific_student
 )
-from course_modes.models import CourseMode
-from student.models import CourseEnrollment
+from django.conf import settings
 from opaque_keys.edx.keys import CourseKey
 from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse
-from xmodule.partitions.partitions import NoSuchUserPartitionGroupError, Group, UserPartition
-
+from student.models import CourseEnrollment
+from xmodule.partitions.partitions import Group, UserPartition
 
 # These IDs must be less than 100 so that they do not overlap with Groups in
 # CohortUserPartition or RandomUserPartitionScheme

openedx/core/djangoapps/verified_track_content/tests/test_models.py

@@ -4,23 +4,26 @@ Tests for Verified Track Cohorting models
 # pylint: disable=attribute-defined-outside-init
 # pylint: disable=no-member
 
-from django.test import TestCase
 import mock
 from mock import patch
 
+from django.test import TestCase
 from opaque_keys.edx.keys import CourseKey
-
-from openedx.core.djangoapps.course_groups.cohorts import get_cohort
+from openedx.core.djangoapps.course_groups.cohorts import (
+    DEFAULT_COHORT_NAME,
+    CourseCohort,
+    add_cohort,
+    get_cohort,
+    set_course_cohorted
+)
+from openedx.core.djangolib.testing.utils import skip_unless_lms
 from student.models import CourseMode
-from student.tests.factories import UserFactory, CourseEnrollmentFactory
+from student.tests.factories import CourseEnrollmentFactory, UserFactory
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
-from ..models import VerifiedTrackCohortedCourse, DEFAULT_VERIFIED_COHORT_NAME
+
+from ..models import DEFAULT_VERIFIED_COHORT_NAME, VerifiedTrackCohortedCourse
 from ..tasks import sync_cohort_with_mode
-from openedx.core.djangoapps.course_groups.cohorts import (
-    set_course_cohort_settings, add_cohort, CourseCohort, DEFAULT_COHORT_NAME
-)
-from openedx.core.djangolib.testing.utils import skip_unless_lms
 
 
 class TestVerifiedTrackCohortedCourse(TestCase):
@@ -88,7 +91,7 @@ class TestMoveToVerified(SharedModuleStoreTestCase):
 
     def _enable_cohorting(self):
         """ Turn on cohorting in the course. """
-        set_course_cohort_settings(self.course.id, is_cohorted=True)
+        set_course_cohorted(self.course.id, True)
 
     def _create_verified_cohort(self, name=DEFAULT_VERIFIED_COHORT_NAME):
         """ Create a verified cohort. """