Adds in radio button patch request and text for page

EDUCATOR-229

common/djangoapps/django_comment_common/tests.py

@@ -120,7 +120,7 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
     def test_invalid_data_types(self):
         exception_msg_template = "Incorrect field type for `{}`. Type must be `{}`"
         fields = [
-            {'name': 'division_scheme', 'type': str},
+            {'name': 'division_scheme', 'type': basestring},
             {'name': 'always_divide_inline_discussions', 'type': bool},
             {'name': 'divided_discussions', 'type': list}
         ]

common/djangoapps/django_comment_common/utils.py

@@ -130,12 +130,13 @@ def set_course_discussion_settings(course_key, **kwargs):
     Returns:
         A CourseDiscussionSettings object.
     """
-    fields = {'division_scheme': str, 'always_divide_inline_discussions': bool, 'divided_discussions': list}
+    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/test/acceptance/pages/lms/instructor_dashboard.py

@@ -807,19 +807,19 @@ class DiscussionManagementSection(PageObject):
         Returns the ID of the selected discussion division scheme
         ("NOT_DIVIDED_SCHEME", "COHORT_SCHEME", or "ENROLLMENT_TRACK_SCHEME)".
         """
-        return self.q(css=self._bounded_selector('.division-scheme:checked')).first.attrs('id')[0]
+        return self.q(css=self._bounded_selector('.division-scheme:checked')).first.attrs('value')[0]
 
     def select_division_scheme(self, scheme):
         """
         Selects the radio button associated with the specified division scheme.
         """
-        self.q(css=self._bounded_selector("input#%s" % scheme)).first.click()
+        self.q(css=self._bounded_selector("input.%s" % scheme)).first.click()
 
     def division_scheme_visible(self, scheme):
         """
         Returns whether or not the specified scheme is visible as an option.
         """
-        return self.q(css=self._bounded_selector("input#%s" % scheme)).visible
+        return self.q(css=self._bounded_selector("input.%s" % scheme)).visible
 
 
 class MembershipPageAutoEnrollSection(PageObject):

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

@@ -99,7 +99,7 @@ class BaseDividedDiscussionTest(UniqueCourseTest, CohortTestMixin):
         Verify that the save confirmation message for the specified portion of the page is visible.
         """
         confirmation_message = self.discussion_management_page.get_divide_discussions_message(key=key)
-        self.assertEqual("Your changes have been saved.", confirmation_message)
+        self.assertIn("Your changes have been saved.", confirmation_message)
 
 
 @attr(shard=6)

lms/djangoapps/discussion/views.py

@@ -24,6 +24,9 @@ try:
 except ImportError:
     newrelic = None  # pylint: disable=invalid-name
 
+from django.views.decorators.csrf import ensure_csrf_cookie
+from django.views.decorators.http import require_http_methods
+
 from rest_framework import status
 
 from web_fragments.fragment import Fragment
@@ -36,10 +39,12 @@ from courseware.access import has_access
 from student.models import CourseEnrollment
 from xmodule.modulestore.django import modulestore
 
-from django_comment_common.utils import ThreadContext, get_course_discussion_settings
+from django_comment_common.utils import ThreadContext, get_course_discussion_settings, set_course_discussion_settings
 
+from django_comment_client.constants import TYPE_ENTRY
 from django_comment_client.permissions import has_permission, get_team
 from django_comment_client.utils import (
+    available_division_schemes,
     merge_dict,
     extract,
     strip_none,
@@ -57,6 +62,8 @@ import lms.lib.comment_client as cc
 from opaque_keys.edx.keys import CourseKey
 
 from contextlib import contextmanager
+from util.json_request import expect_json, JsonResponse
+
 
 THREADS_PER_PAGE = 20
 INLINE_THREADS_PER_PAGE = 20
@@ -684,3 +691,167 @@ class DiscussionBoardFragmentView(EdxFragmentView):
             return self.get_css_dependencies('style-discussion-main-rtl')
         else:
             return self.get_css_dependencies('style-discussion-main')
+
+
+@expect_json
+@login_required
+def discussion_topics(request, course_key_string):
+    """
+    The handler for divided 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 = utils.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)
+
+
+@require_http_methods(("GET", "PATCH"))
+@ensure_csrf_cookie
+@expect_json
+@login_required
+def course_discussions_settings_handler(request, course_key_string):
+    """
+    The restful handler for divided discussion setting requests. Requires JSON.
+    This will raise 404 if user is not staff.
+    GET
+        Returns the JSON representation of divided discussion settings for the course.
+    PATCH
+        Updates the divided discussion 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)
+    discussion_settings = get_course_discussion_settings(course_key)
+
+    if request.method == 'PATCH':
+        divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
+            course, discussion_settings
+        )
+
+        settings_to_change = {}
+
+        if 'divided_course_wide_discussions' in request.json or 'divided_inline_discussions' in request.json:
+            divided_course_wide_discussions = request.json.get(
+                'divided_course_wide_discussions', divided_course_wide_discussions
+            )
+            divided_inline_discussions = request.json.get(
+                'divided_inline_discussions', divided_inline_discussions
+            )
+            settings_to_change['divided_discussions'] = divided_course_wide_discussions + divided_inline_discussions
+
+        if 'always_divide_inline_discussions' in request.json:
+            settings_to_change['always_divide_inline_discussions'] = request.json.get(
+                'always_divide_inline_discussions'
+            )
+        if 'division_scheme' in request.json:
+            settings_to_change['division_scheme'] = request.json.get(
+                'division_scheme'
+            )
+
+        if not settings_to_change:
+            return JsonResponse({"error": unicode("Bad Request")}, 400)
+
+        try:
+            if settings_to_change:
+                discussion_settings = set_course_discussion_settings(course_key, **settings_to_change)
+
+        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)
+
+    divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
+        course, discussion_settings
+    )
+
+    return JsonResponse({
+        'id': discussion_settings.id,
+        'divided_inline_discussions': divided_inline_discussions,
+        'divided_course_wide_discussions': divided_course_wide_discussions,
+        'always_divide_inline_discussions': discussion_settings.always_divide_inline_discussions,
+        'division_scheme': discussion_settings.division_scheme,
+        'available_division_schemes': available_division_schemes(course_key)
+    })
+
+
+def get_divided_discussions(course, discussion_settings):
+    """
+    Returns the course-wide and inline divided discussion ids separately.
+    """
+    divided_course_wide_discussions = []
+    divided_inline_discussions = []
+
+    course_wide_discussions = [topic['id'] for __, topic in course.discussion_topics.items()]
+    all_discussions = utils.get_discussion_categories_ids(course, None, include_all=True)
+
+    for divided_discussion_id in discussion_settings.divided_discussions:
+        if divided_discussion_id in course_wide_discussions:
+            divided_course_wide_discussions.append(divided_discussion_id)
+        elif divided_discussion_id in all_discussions:
+            divided_inline_discussions.append(divided_discussion_id)
+
+    return divided_course_wide_discussions, divided_inline_discussions

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -14,6 +14,7 @@ from edxmako import add_lookup
 
 from django_comment_client.tests.factories import RoleFactory
 from django_comment_client.tests.unicode import UnicodeTestMixin
+from django_comment_client.tests.utils import topic_name_to_id, config_course_discussions
 from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY
 import django_comment_client.utils as utils
 from lms.lib.comment_client.utils import perform_request, CommentClientMaintenanceError
@@ -26,7 +27,7 @@ from courseware.tests.factories import InstructorFactory
 from courseware.tabs import get_course_tab_list
 from openedx.core.djangoapps.course_groups import cohorts
 from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted
-from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts, config_course_discussions, topic_name_to_id, CohortFactory
+from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts, CohortFactory
 from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory
 from openedx.core.djangoapps.content.course_structures.models import CourseStructure
 from openedx.core.djangoapps.util.testing import ContentGroupTestCase
@@ -1489,7 +1490,7 @@ class GroupIdForUserTestCase(ModuleStoreTestCase):
 
 @attr(shard=1)
 class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
-    """ Test the course_discussion_division_enabled method. """
+    """ Test the course_discussion_division_enabled and available_division_schemes methods. """
 
     def setUp(self):
         super(CourseDiscussionDivisionEnabledTestCase, self).setUp()
@@ -1504,6 +1505,7 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
     def test_discussion_division_disabled(self):
         course_discussion_settings = get_course_discussion_settings(self.course.id)
         self.assertFalse(utils.course_discussion_division_enabled(course_discussion_settings))
+        self.assertEqual([], utils.available_division_schemes(self.course.id))
 
     def test_discussion_division_by_cohort(self):
         set_discussion_division_settings(
@@ -1511,11 +1513,13 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
         )
         # Because cohorts are disabled, discussion division is not enabled.
         self.assertFalse(utils.course_discussion_division_enabled(get_course_discussion_settings(self.course.id)))
+        self.assertEqual([], utils.available_division_schemes(self.course.id))
         # Now enable cohorts, which will cause discussions to be divided.
         set_discussion_division_settings(
             self.course.id, enable_cohorts=True, division_scheme=CourseDiscussionSettings.COHORT
         )
         self.assertTrue(utils.course_discussion_division_enabled(get_course_discussion_settings(self.course.id)))
+        self.assertEqual([CourseDiscussionSettings.COHORT], utils.available_division_schemes(self.course.id))
 
     def test_discussion_division_by_enrollment_track(self):
         set_discussion_division_settings(
@@ -1523,10 +1527,12 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
         )
         # Only a single enrollment track exists, so discussion division is not enabled.
         self.assertFalse(utils.course_discussion_division_enabled(get_course_discussion_settings(self.course.id)))
+        self.assertEqual([], utils.available_division_schemes(self.course.id))
 
         # Now create a second CourseMode, which will cause discussions to be divided.
         CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED)
         self.assertTrue(utils.course_discussion_division_enabled(get_course_discussion_settings(self.course.id)))
+        self.assertEqual([CourseDiscussionSettings.ENROLLMENT_TRACK], utils.available_division_schemes(self.course.id))
 
 
 @attr(shard=1)

lms/djangoapps/django_comment_client/tests/utils.py

@@ -5,9 +5,11 @@ from mock import patch
 
 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 django_comment_common.utils import seed_permissions_roles, set_course_discussion_settings, CourseDiscussionSettings
 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.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 
@@ -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/django_comment_client/utils.py

@@ -860,6 +860,24 @@ def course_discussion_division_enabled(course_discussion_settings):
     return _get_course_division_scheme(course_discussion_settings) != CourseDiscussionSettings.NONE
 
 
+def available_division_schemes(course_key):
+    """
+    Returns a list of possible discussion division schemes for this course.
+    This takes into account if cohorts are enabled and if there are multiple
+    enrollment tracks. If no schemes are available, returns an empty list.
+    Args:
+        course_key: CourseKey
+
+    Returns: list of possible division schemes (for example, CourseDiscussionSettings.COHORT)
+    """
+    available_schemes = []
+    if is_course_cohorted(course_key):
+        available_schemes.append(CourseDiscussionSettings.COHORT)
+    if len(_get_enrollment_track_groups(course_key)) > 1:
+        available_schemes.append(CourseDiscussionSettings.ENROLLMENT_TRACK)
+    return available_schemes
+
+
 def _get_course_division_scheme(course_discussion_settings):
     division_scheme = course_discussion_settings.division_scheme
     if (

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

@@ -6,7 +6,8 @@
             defaults: {
                 divided_inline_discussions: [],
                 divided_course_wide_discussions: [],
-                always_divide_inline_discussions: false
+                always_divide_inline_discussions: false,
+                division_scheme: 'none'
             }
         });
         return CourseDiscussionsSettingsModel;

lms/static/js/discussions_management/views/discussions.js

@@ -3,11 +3,24 @@
     define(['jquery', 'underscore', 'backbone', 'gettext',
         'js/discussions_management/views/divided_discussions_inline',
         'js/discussions_management/views/divided_discussions_course_wide',
-        'edx-ui-toolkit/js/utils/html-utils'
+        'edx-ui-toolkit/js/utils/html-utils',
+        'edx-ui-toolkit/js/utils/string-utils',
+        'js/models/notification',
+        'js/views/notification'
     ],
 
-        function($, _, Backbone, gettext, InlineDiscussionsView, CourseWideDiscussionsView, HtmlUtils) {
+        function($, _, Backbone, gettext, InlineDiscussionsView, CourseWideDiscussionsView, HtmlUtils, StringUtils) {
+            /* global NotificationModel, NotificationView */
+
+            var hiddenClass = 'is-hidden';
+            var cohort = 'cohort';
+            var none = 'none';
+            var enrollmentTrack = 'enrollment_track';
+
             var DiscussionsView = Backbone.View.extend({
+                events: {
+                    'click .division-scheme': 'divisionSchemeChanged'
+                },
 
                 initialize: function(options) {
                     this.template = HtmlUtils.template($('#discussions-tpl').text());
@@ -16,16 +29,145 @@
                 },
 
                 render: function() {
-                    HtmlUtils.setHtml(this.$el, this.template({}));
-                    this.showDiscussionTopics();
+                    var numberAvailableSchemes = this.discussionSettings.attributes.available_division_schemes.length;
+                    HtmlUtils.setHtml(this.$el, this.template({
+                        availableSchemes: this.getDivisionSchemeData(this.discussionSettings.attributes.division_scheme), //  eslint-disable-line max-len
+                        layoutClass: numberAvailableSchemes === 1 ? 'two-column-layout' : 'three-column-layout'
+                    }));
+                    this.updateTopicVisibility(this.getSelectedScheme(), this.getTopicNav());
+                    this.renderTopics();
                     return this;
                 },
 
+                getDivisionSchemeData: function(selectedScheme) {
+                    return [
+                        {
+                            key: none,
+                            displayName: gettext('Not divided'),
+                            descriptiveText: gettext('Discussions are unified; all learners interact with posts from other learners, regardless of the group they are in.'), //  eslint-disable-line max-len
+                            selected: selectedScheme === none,
+                            enabled: true // always leave none enabled
+                        },
+                        {
+                            key: enrollmentTrack,
+                            displayName: gettext('Enrollment Tracks'),
+                            descriptiveText: gettext('Use enrollment tracks as the basis for dividing discussions. All learners, regardless of their enrollment track, see the same discussion topics, but within divided topics, only learners who are in the same enrollment track see and respond to each others’ posts.'), //  eslint-disable-line max-len
+                            selected: selectedScheme === enrollmentTrack,
+                            enabled: this.isSchemeAvailable(enrollmentTrack) || selectedScheme === enrollmentTrack
+                        },
+                        {
+                            key: cohort,
+                            displayName: gettext('Cohorts'),
+                            descriptiveText: gettext('Use cohorts as the basis for dividing discussions. All learners, regardless of cohort, see the same discussion topics, but within divided topics, only members of the same cohort see and respond to each others’ posts. '), //  eslint-disable-line max-len
+                            selected: selectedScheme === cohort,
+                            enabled: this.isSchemeAvailable(cohort) || selectedScheme === cohort
+                        }
+
+                    ];
+                },
+
+                isSchemeAvailable: function(scheme) {
+                    return this.discussionSettings.attributes.available_division_schemes.indexOf(scheme) !== -1;
+                },
+
+                showMessage: function(message, type) {
+                    var model = new NotificationModel({type: type || 'confirmation', title: message});
+                    this.removeNotification();
+                    this.notification = new NotificationView({
+                        model: model
+                    });
+                    this.$('.division-scheme-container').prepend(this.notification.$el);
+                    this.notification.render();
+                },
+
+                removeNotification: function() {
+                    if (this.notification) {
+                        this.notification.remove();
+                    }
+                },
+
+                getSelectedScheme: function() {
+                    return this.$('input[name="division-scheme"]:checked').val();
+                },
+
+                getTopicNav: function() {
+                    return this.$('.topic-division-nav');
+                },
+
+                divisionSchemeChanged: function() {
+                    var selectedScheme = this.getSelectedScheme(),
+                        topicNav = this.getTopicNav(),
+                        fieldData = {
+                            division_scheme: selectedScheme
+                        };
+
+                    this.updateTopicVisibility(selectedScheme, topicNav);
+                    this.saveDivisionScheme(topicNav, fieldData, selectedScheme);
+                },
+
+                saveDivisionScheme: function($element, fieldData, selectedScheme) {
+                    var self = this,
+                        discussionSettingsModel = this.discussionSettings,
+                        showErrorMessage,
+                        details = '';
+
+                    this.removeNotification();
+                    showErrorMessage = function(message) {
+                        self.showMessage(message, 'error');
+                    };
+
+                    discussionSettingsModel.save(
+                        fieldData, {patch: true, wait: true}
+                    ).done(function() {
+                        switch (selectedScheme) {
+                        case none:
+                            details = gettext('Discussion topics in the course are not divided.');
+                            break;
+                        case enrollmentTrack:
+                            details = gettext('Any divided discussion topics are divided based on enrollment track.'); //  eslint-disable-line max-len
+                            break;
+                        case cohort:
+                            details = gettext('Any divided discussion topics are divided based on cohort.');
+                            break;
+                        default:
+                            break;
+                        }
+                        self.showMessage(
+                            StringUtils.interpolate(
+                                gettext('Your changes have been saved. {details}'),
+                                {details: details},
+                                true
+                            )
+                        );
+                    }).fail(function(result) {
+                        var errorMessage = null,
+                            jsonResponse;
+                        try {
+                            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 have encountered an error. Refresh your browser and then try again.'); //  eslint-disable-line max-len
+                        }
+                        showErrorMessage(errorMessage);
+                    });
+                },
+
+                updateTopicVisibility: function(selectedScheme, topicNav) {
+                    if (selectedScheme === none) {
+                        topicNav.addClass(hiddenClass);
+                    } else {
+                        topicNav.removeClass(hiddenClass);
+                    }
+                },
+
                 getSectionCss: function(section) {
                     return ".instructor-nav .nav-item [data-section='" + section + "']";
                 },
 
-                showDiscussionTopics: function() {
+                renderTopics: function() {
                     var dividedDiscussionsElement = this.$('.discussions-nav');
                     if (!this.CourseWideDiscussionsView) {
                         this.CourseWideDiscussionsView = new CourseWideDiscussionsView({

lms/static/js/spec/groups/views/discussions_spec.js

@@ -20,22 +20,26 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
 
             createMockDiscussionsSettingsJson = function(dividedInlineDiscussions,
                                                          dividedCourseWideDiscussions,
-                                                         alwaysDivideInlineDiscussions) {
+                                                         alwaysDivideInlineDiscussions,
+                                                         availableDivisionSchemes) {
                 return {
                     id: 0,
                     divided_inline_discussions: dividedInlineDiscussions || [],
                     divided_course_wide_discussions: dividedCourseWideDiscussions || [],
-                    always_divide_inline_discussions: alwaysDivideInlineDiscussions || false
+                    always_divide_inline_discussions: alwaysDivideInlineDiscussions || false,
+                    available_division_schemes: availableDivisionSchemes || ['cohort']
                 };
             };
 
             createMockDiscussionsSettings = function(dividedInlineDiscussions,
                                                      dividedCourseWideDiscussions,
-                                                     alwaysDivideInlineDiscussions) {
+                                                     alwaysDivideInlineDiscussions,
+                                                     availableDivisionSchemes) {
                 return new CourseDiscussionsSettingsModel(
                     createMockDiscussionsSettingsJson(dividedInlineDiscussions,
                                                       dividedCourseWideDiscussions,
-                                                      alwaysDivideInlineDiscussions)
+                                                      alwaysDivideInlineDiscussions,
+                                                      availableDivisionSchemes)
                 );
             };
 
@@ -132,14 +136,14 @@ define(['backbone', 'jquery', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers
                 expect($courseWideDiscussionsForm.text()).
                     toContain('Course-Wide Discussion Topics');
                 expect($courseWideDiscussionsForm.text()).
-                    toContain('Select the course-wide discussion topics that you want to divide by cohort.');
+                    toContain('Select the course-wide discussion topics that you want to divide.');
 
                 // Should see the inline discussions form and its content
                 expect($inlineDiscussionsForm.length).toBe(1);
                 expect($inlineDiscussionsForm.text()).
                     toContain('Content-Specific Discussion Topics');
                 expect($inlineDiscussionsForm.text()).
-                    toContain('Specify whether content-specific discussion topics are divided by cohort.');
+                    toContain('Specify whether content-specific discussion topics are divided.');
             };
 
             beforeEach(function() {

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

@@ -1241,6 +1241,36 @@
 // --------------------
 .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: 33%;
+    }
+
+    .two-column-layout {
+       max-width: 50%;
+    }
+
+    .field-message {
+        font-size: 13px;
+    }
+  }
+
+  // cohort management
   .form-submit {
     @include idashbutton($uxpl-blue-base);
     @include font-size(14);

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 <%- layoutClass %> <% if (!availableSchemes[i].enabled) { %>is-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/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 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/divided-discussions-inline.underscore

@@ -6,17 +6,17 @@
             <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>
+                    <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" <%- alwaysDivideInlineDiscussions ? 'checked="checked"' : '' %>/>
-                            <span class="all-inline-discussions"><%- gettext('Always cohort content-specific discussion topics') %></span>
+                            <span class="all-inline-discussions"><%- gettext('Always divide content-specific discussion topics') %></span>
                         </label>
                     </div>
                     <div class="divide_inline_discussions">
                         <label>
                             <input type="radio" name="inline" class="check-cohort-inline-discussions" <%- alwaysDivideInlineDiscussions ? '' : 'checked="checked"' %>/>
-                            <span class="all-inline-discussions"><%- gettext('Cohort selected content-specific discussion topics') %></span>
+                            <span class="all-inline-discussions"><%- gettext('Divide the selected content-specific discussion topics') %></span>
                         </label>
                     </div>
                     <hr class="divider divider-lv1" />

lms/urls.py

@@ -509,7 +509,7 @@ urlpatterns += (
         r'^courses/{}/discussions/settings$'.format(
             settings.COURSE_KEY_PATTERN,
         ),
-        'openedx.core.djangoapps.course_groups.views.course_discussions_settings_handler',
+        'lms.djangoapps.discussion.views.course_discussions_settings_handler',
         name='course_discussions_settings',
     ),
 
@@ -560,7 +560,7 @@ urlpatterns += (
         r'^courses/{}/discussion/topics$'.format(
             settings.COURSE_KEY_PATTERN,
         ),
-        'openedx.core.djangoapps.course_groups.views.discussion_topics',
+        'lms.djangoapps.discussion.views.discussion_topics',
         name='discussion_topics',
     ),
     url(

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

@@ -63,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
@@ -93,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:
@@ -136,52 +101,10 @@ def config_course_cohorts_legacy(
 
 
 # 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
-
-
-# pylint: disable=dangerous-default-value
 def config_course_cohorts(
         course,
         is_cohorted,
+        discussion_division_scheme=CourseDiscussionSettings.COHORT,
         auto_cohorts=[],
         manual_cohorts=[],
 ):
@@ -191,6 +114,8 @@ def config_course_cohorts(
     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.
 
@@ -201,7 +126,7 @@ def config_course_cohorts(
     set_course_cohorted(course.id, is_cohorted)
     set_course_discussion_settings(
         course.id,
-        division_scheme=CourseDiscussionSettings.COHORT,
+        division_scheme=discussion_division_scheme,
     )
 
     for cohort_name in auto_cohorts:

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

@@ -350,7 +350,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 +392,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"]
         )

openedx/core/djangoapps/course_groups/views.py

@@ -15,11 +15,7 @@ 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 django_comment_common.models import CourseDiscussionSettings
-from django_comment_common.utils import get_course_discussion_settings, set_course_discussion_settings
 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 opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from util.json_request import JsonResponse, expect_json
@@ -74,22 +70,6 @@ def _get_course_cohort_settings_representation(cohort_id, is_cohorted):
     }
 
 
-def _get_course_discussion_settings_representation(course, course_discussion_settings):
-    """
-    Returns a JSON representation of a course discussion settings.
-    """
-    divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
-        course, course_discussion_settings
-    )
-
-    return {
-        'id': course_discussion_settings.id,
-        'divided_inline_discussions': divided_inline_discussions,
-        'divided_course_wide_discussions': divided_course_wide_discussions,
-        'always_divide_inline_discussions': course_discussion_settings.always_divide_inline_discussions,
-    }
-
-
 def _get_cohort_representation(cohort, course):
     """
     Returns a JSON representation of a cohort.
@@ -107,80 +87,6 @@ def _get_cohort_representation(cohort, course):
     }
 
 
-def get_divided_discussions(course, discussion_settings):
-    """
-    Returns the course-wide and inline divided discussion ids separately.
-    """
-    divided_course_wide_discussions = []
-    divided_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 divided_discussion_id in discussion_settings.divided_discussions:
-        if divided_discussion_id in course_wide_discussions:
-            divided_course_wide_discussions.append(divided_discussion_id)
-        elif divided_discussion_id in all_discussions:
-            divided_inline_discussions.append(divided_discussion_id)
-
-    return divided_course_wide_discussions, divided_inline_discussions
-
-
-@require_http_methods(("GET", "PATCH"))
-@ensure_csrf_cookie
-@expect_json
-@login_required
-def course_discussions_settings_handler(request, course_key_string):
-    """
-    The restful handler for divided discussion setting requests. Requires JSON.
-    This will raise 404 if user is not staff.
-    GET
-        Returns the JSON representation of divided discussion settings for the course.
-    PATCH
-        Updates the divided discussion 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)
-    discussion_settings = get_course_discussion_settings(course_key)
-
-    if request.method == 'PATCH':
-        divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
-            course, discussion_settings
-        )
-
-        settings_to_change = {}
-
-        if 'divided_course_wide_discussions' in request.json or 'divided_inline_discussions' in request.json:
-            divided_course_wide_discussions = request.json.get(
-                'divided_course_wide_discussions', divided_course_wide_discussions
-            )
-            divided_inline_discussions = request.json.get(
-                'divided_inline_discussions', divided_inline_discussions
-            )
-            settings_to_change['divided_discussions'] = divided_course_wide_discussions + divided_inline_discussions
-
-        if 'always_divide_inline_discussions' in request.json:
-            settings_to_change['always_divide_inline_discussions'] = request.json.get(
-                'always_divide_inline_discussions'
-            )
-
-        if not settings_to_change:
-            return JsonResponse({"error": unicode("Bad Request")}, 400)
-
-        try:
-            if settings_to_change:
-                discussion_settings = set_course_discussion_settings(course_key, **settings_to_change)
-
-        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_discussion_settings_representation(
-        course,
-        discussion_settings
-    ))
-
-
 @require_http_methods(("GET", "PATCH"))
 @ensure_csrf_cookie
 @expect_json
@@ -196,7 +102,7 @@ def course_cohort_settings_handler(request, course_key_string):
     """
     course_key = CourseKey.from_string(course_key_string)
     # Although this course data is not used this method will return 404 is user is not staff
-    course = get_course_with_access(request.user, 'staff', course_key)
+    get_course_with_access(request.user, 'staff', course_key)
 
     if request.method == 'PATCH':
         if 'is_cohorted' not in request.json:
@@ -205,9 +111,6 @@ def course_cohort_settings_handler(request, course_key_string):
         is_cohorted = request.json.get('is_cohorted')
         try:
             cohorts.set_course_cohorted(course_key, is_cohorted)
-            scheme = CourseDiscussionSettings.COHORT if is_cohorted else CourseDiscussionSettings.NONE
-            scheme_settings = {'division_scheme': scheme}
-            set_course_discussion_settings(course_key, **scheme_settings)
         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)
@@ -464,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 discussion_topics(request, course_key_string):
-    """
-    The handler for divided 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)