Merge pull request #7017 from edx/ammar/move-cohort-management-tnl-1268

Enable/disable cohorts from the instructor dashboard and move cohorts management to its own tab.

cms/djangoapps/models/settings/course_metadata.py

@@ -18,6 +18,7 @@ class CourseMetadata(object):
     # Should not be used directly. Instead the filtered_list method should
     # be used if the field needs to be filtered depending on the feature flag.
     FILTERED_LIST = [
+        'cohort_config',
         'xml_attributes',
         'start',
         'end',

common/lib/django_test_client_utils.py

@@ -0,0 +1,49 @@
+"""
+This file includes the monkey-patch for requests' PATCH method, as we are using
+older version of django that does not contains the PATCH method in its test client.
+"""
+from __future__ import unicode_literals
+
+from urlparse import urlparse
+
+from django.test.client import RequestFactory, Client, FakePayload
+
+
+BOUNDARY = 'BoUnDaRyStRiNg'
+MULTIPART_CONTENT = 'multipart/form-data; boundary=%s' % BOUNDARY
+
+
+def request_factory_patch(self, path, data={}, content_type=MULTIPART_CONTENT, **extra):
+    """
+    Construct a PATCH request.
+    """
+    patch_data = self._encode_data(data, content_type)
+
+    parsed = urlparse(path)
+    r = {
+        'CONTENT_LENGTH': len(patch_data),
+        'CONTENT_TYPE': content_type,
+        'PATH_INFO': self._get_path(parsed),
+        'QUERY_STRING': parsed[4],
+        'REQUEST_METHOD': 'PATCH',
+        'wsgi.input': FakePayload(patch_data),
+    }
+    r.update(extra)
+    return self.request(**r)
+
+
+def client_patch(self, path, data={}, content_type=MULTIPART_CONTENT, follow=False, **extra):
+    """
+    Send a resource to the server using PATCH.
+    """
+    response = super(Client, self).patch(path, data=data, content_type=content_type, **extra)
+    if follow:
+        response = self._handle_redirects(response, **extra)
+    return response
+
+
+if not hasattr(RequestFactory, 'patch'):
+    setattr(RequestFactory, 'patch', request_factory_patch)
+
+if not hasattr(Client, 'patch'):
+    setattr(Client, 'patch', client_patch)

common/lib/xmodule/xmodule/course_module.py

@@ -1046,6 +1046,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
     def is_cohorted(self):
         """
         Return whether the course is cohorted.
+
+        Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
         """
         config = self.cohort_config
         if config is None:
@@ -1057,6 +1059,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
     def auto_cohort(self):
         """
         Return whether the course is auto-cohorted.
+
+        Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
         """
         if not self.is_cohorted:
             return False
@@ -1070,6 +1074,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
         Return the list of groups to put students into.  Returns [] if not
         specified. Returns specified list even if is_cohorted and/or auto_cohort are
         false.
+
+        Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
         """
         if self.cohort_config is None:
             return []
@@ -1090,6 +1096,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
         Return the set of discussions that is explicitly cohorted.  It may be
         the empty set.  Note that all inline discussions are automatically
         cohorted based on the course's is_cohorted setting.
+
+        Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
         """
         config = self.cohort_config
         if config is None:
@@ -1103,6 +1111,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
         This allow to change the default behavior of inline discussions cohorting. By
         setting this to False, all inline discussions are non-cohorted unless their
         ids are specified in cohorted_discussions.
+
+        Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
         """
         config = self.cohort_config
         if config is None:

common/lib/xmodule/xmodule/tests/test_import.py

@@ -643,6 +643,9 @@ class ImportTestCase(BaseCourseTestCase):
     def test_cohort_config(self):
         """
         Check that cohort config parsing works right.
+
+        Note: The cohort config on the CourseModule is no longer used.
+        See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
         """
         modulestore = XMLModuleStore(DATA_DIR, source_dirs=['toy'])
 

common/test/acceptance/pages/lms/instructor_dashboard.py

@@ -28,6 +28,15 @@ class InstructorDashboardPage(CoursePage):
         membership_section.wait_for_page()
         return membership_section
 
+    def select_cohort_management(self):
+        """
+        Selects the cohort management tab and returns the CohortManagementSection
+        """
+        self.q(css='a[data-section=cohort_management]').first.click()
+        cohort_management_section = CohortManagementSection(self.browser)
+        cohort_management_section.wait_for_page()
+        return cohort_management_section
+
     def select_data_download(self):
         """
         Selects the data download tab and returns a DataDownloadPage.
@@ -84,16 +93,10 @@ class MembershipPage(PageObject):
         """
         return MembershipPageAutoEnrollSection(self.browser)
 
-    def select_cohort_management_section(self):
-        """
-        Returns the MembershipPageCohortManagementSection page object.
-        """
-        return MembershipPageCohortManagementSection(self.browser)
 
-
-class MembershipPageCohortManagementSection(PageObject):
+class CohortManagementSection(PageObject):
     """
-    The cohort management subsection of the Membership section of the Instructor dashboard.
+    The Cohort Management section of the Instructor dashboard.
     """
     url = None
     csv_browse_button_selector_css = '.csv-upload #file-upload-form-file'
@@ -102,15 +105,19 @@ class MembershipPageCohortManagementSection(PageObject):
     no_content_group_button_css = '.cohort-management-details-association-course input.radio-no'
     select_content_group_button_css = '.cohort-management-details-association-course input.radio-yes'
     assignment_type_buttons_css = '.cohort-management-assignment-type-settings input'
+    discussion_form_selectors = {
+        'course-wide': '.cohort-course-wide-discussions-form',
+        'inline': '.cohort-inline-discussions-form'
+    }
 
     def is_browser_on_page(self):
-        return self.q(css='.cohort-management.membership-section').present
+        return self.q(css='.cohort-management').present
 
     def _bounded_selector(self, selector):
         """
         Return `selector`, but limited to the cohort management context.
         """
-        return '.cohort-management.membership-section {}'.format(selector)
+        return '.cohort-management {}'.format(selector)
 
     def _get_cohort_options(self):
         """
@@ -158,10 +165,10 @@ class MembershipPageCohortManagementSection(PageObject):
         Return assignment settings disabled message in case of default cohort.
         """
         query = self.q(css=self._bounded_selector('.copy-error'))
-        if query.present:
+        if query.visible:
             return query.text[0]
-        else:
-            return ''
+
+        return ''
 
     @property
     def cohort_name_in_header(self):
@@ -232,7 +239,11 @@ class MembershipPageCohortManagementSection(PageObject):
         Adds a new manual cohort with the specified name.
         If a content group should also be associated, the name of the content group should be specified.
         """
-        create_buttons = self.q(css=self._bounded_selector(".action-create"))
+        add_cohort_selector = self._bounded_selector(".action-create")
+
+        # We need to wait because sometime add cohort button is not in a state to be clickable.
+        self.wait_for_element_presence(add_cohort_selector, 'Add Cohort button is present.')
+        create_buttons = self.q(css=add_cohort_selector)
         # There are 2 create buttons on the page. The second one is only present when no cohort yet exists
         # (in which case the first is not visible). Click on the last present create button.
         create_buttons.results[len(create_buttons.results) - 1].click()
@@ -444,6 +455,145 @@ class MembershipPageCohortManagementSection(PageObject):
         file_input.send_keys(path)
         self.q(css=self._bounded_selector(self.csv_upload_button_selector_css)).first.click()
 
+    @property
+    def is_cohorted(self):
+        """
+        Returns the state of `Enable Cohorts` checkbox state.
+        """
+        return self.q(css=self._bounded_selector('.cohorts-state')).selected
+
+    @is_cohorted.setter
+    def is_cohorted(self, state):
+        """
+        Check/Uncheck the `Enable Cohorts` checkbox state.
+        """
+        if state != self.is_cohorted:
+            self.q(css=self._bounded_selector('.cohorts-state')).first.click()
+
+    def toggles_showing_of_discussion_topics(self):
+        """
+        Shows the discussion topics.
+        """
+        EmptyPromise(
+            lambda: self.q(css=self._bounded_selector('.toggle-cohort-management-discussions')).results != 0,
+            "Waiting for discussion section to show"
+        ).fulfill()
+
+        # If the discussion topic section has not yet been toggled on, click on the toggle link.
+        self.q(css=self._bounded_selector(".toggle-cohort-management-discussions")).click()
+
+    def discussion_topics_visible(self):
+        """
+        Returns the visibility status of cohort discussion controls.
+        """
+        EmptyPromise(
+            lambda: self.q(css=self._bounded_selector('.cohort-discussions-nav')).results != 0,
+            "Waiting for discussion section to show"
+        ).fulfill()
+
+        return (self.q(css=self._bounded_selector('.cohort-course-wide-discussions-nav')).visible and
+                self.q(css=self._bounded_selector('.cohort-inline-discussions-nav')).visible)
+
+    def select_discussion_topic(self, key):
+        """
+        Selects discussion topic checkbox by clicking on it.
+        """
+        self.q(css=self._bounded_selector(".check-discussion-subcategory-%s" % key)).first.click()
+
+    def select_always_inline_discussion(self):
+        """
+        Selects the always_cohort_inline_discussions radio button.
+        """
+        self.q(css=self._bounded_selector(".check-all-inline-discussions")).first.click()
+
+    def always_inline_discussion_selected(self):
+        """
+        Returns the checked always_cohort_inline_discussions radio button.
+        """
+        return self.q(css=self._bounded_selector(".check-all-inline-discussions:checked"))
+
+    def cohort_some_inline_discussion_selected(self):
+        """
+        Returns the checked some_cohort_inline_discussions radio button.
+        """
+        return self.q(css=self._bounded_selector(".check-cohort-inline-discussions:checked"))
+
+    def select_cohort_some_inline_discussion(self):
+        """
+        Selects the cohort_some_inline_discussions radio button.
+        """
+        self.q(css=self._bounded_selector(".check-cohort-inline-discussions")).first.click()
+
+    def inline_discussion_topics_disabled(self):
+        """
+        Returns the status of inline discussion topics, enabled or disabled.
+        """
+        inline_topics = self.q(css=self._bounded_selector('.check-discussion-subcategory-inline'))
+        return all(topic.get_attribute('disabled') == 'true' for topic in inline_topics)
+
+    def is_save_button_disabled(self, key):
+        """
+        Returns the status for form's save button, enabled or disabled.
+        """
+        save_button_css = '%s %s' % (self.discussion_form_selectors[key], '.action-save')
+        disabled = self.q(css=self._bounded_selector(save_button_css)).attrs('disabled')
+        return disabled[0] == 'true'
+
+    def is_category_selected(self):
+        """
+        Returns the status for category checkboxes.
+        """
+        return self.q(css=self._bounded_selector('.check-discussion-category:checked')).is_present()
+
+    def get_cohorted_topics_count(self, key):
+        """
+        Returns the count for cohorted topics.
+        """
+        cohorted_topics = self.q(css=self._bounded_selector('.check-discussion-subcategory-%s:checked' % key))
+        return len(cohorted_topics.results)
+
+    def save_discussion_topics(self, key):
+        """
+        Saves the discussion topics.
+        """
+        save_button_css = '%s %s' % (self.discussion_form_selectors[key], '.action-save')
+        self.q(css=self._bounded_selector(save_button_css)).first.click()
+
+    def get_cohort_discussions_message(self, key, msg_type="confirmation"):
+        """
+        Returns the message related to modifying discussion topics.
+        """
+        title_css = "%s .message-%s .message-title" % (self.discussion_form_selectors[key], msg_type)
+
+        EmptyPromise(
+            lambda: self.q(css=self._bounded_selector(title_css)),
+            "Waiting for message to appear"
+        ).fulfill()
+
+        message_title = self.q(css=self._bounded_selector(title_css))
+
+        if len(message_title.results) == 0:
+            return ''
+        return message_title.first.text[0]
+
+    def cohort_discussion_heading_is_visible(self, key):
+        """
+        Returns the visibility of discussion topic headings.
+        """
+        form_heading_css = '%s %s' % (self.discussion_form_selectors[key], '.subsection-title')
+        discussion_heading = self.q(css=self._bounded_selector(form_heading_css))
+
+        if len(discussion_heading) == 0:
+            return False
+        return discussion_heading.first.text[0]
+
+    def cohort_management_controls_visible(self):
+        """
+        Return the visibility status of cohort management controls(cohort selector section etc).
+        """
+        return (self.q(css=self._bounded_selector('.cohort-management-nav')).visible and
+                self.q(css=self._bounded_selector('.wrapper-cohort-supplemental')).visible)
+
 
 class MembershipPageAutoEnrollSection(PageObject):
     """

common/test/acceptance/pages/studio/settings_advanced.py

@@ -154,7 +154,6 @@ class AdvancedSettingsPage(CoursePage):
             'cert_name_long',
             'cert_name_short',
             'certificates_display_behavior',
-            'cohort_config',
             'course_image',
             'cosmetic_display_price',
             'advertised_start',

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

@@ -60,13 +60,10 @@ class CohortTestMixin(object):
         """
         Disables cohorting for the current course fixture.
         """
-        course_fixture._update_xblock(course_fixture._course_location, {
-            "metadata": {
-                u"cohort_config": {
-                    "cohorted": False
-                },
-            },
-        })
+        url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings'  # pylint: disable=protected-access
+        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")
 
     def add_manual_cohort(self, course_fixture, cohort_name):
         """

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

@@ -11,7 +11,7 @@ from nose.plugins.attrib import attr
 from .helpers import CohortTestMixin
 from ..helpers import UniqueCourseTest, EventsTestMixin, create_user_partition_json
 from xmodule.partitions.partitions import Group
-from ...fixtures.course import CourseFixture
+from ...fixtures.course import CourseFixture, XBlockFixtureDesc
 from ...pages.lms.auto_auth import AutoAuthPage
 from ...pages.lms.instructor_dashboard import InstructorDashboardPage, DataDownloadPage
 from ...pages.studio.settings_advanced import AdvancedSettingsPage
@@ -63,8 +63,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         # go to the membership page on the instructor dashboard
         self.instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id)
         self.instructor_dashboard_page.visit()
-        membership_page = self.instructor_dashboard_page.select_membership()
-        self.cohort_management_page = membership_page.select_cohort_management_section()
+        self.cohort_management_page = self.instructor_dashboard_page.select_cohort_management()
 
     def verify_cohort_description(self, cohort_name, expected_description):
         """
@@ -123,22 +122,6 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         )
         group_settings_page.wait_for_page()
 
-    def test_link_to_studio(self):
-        """
-        Scenario: a link is present from the cohort configuration in the instructor dashboard
-        to the Studio Advanced Settings.
-
-        Given I have a course with a cohort defined
-        When I view the cohort in the LMS instructor dashboard
-        There is a link to take me to the Studio Advanced Settings for the course
-        """
-        self.cohort_management_page.select_cohort(self.manual_cohort_name)
-        self.cohort_management_page.select_edit_settings()
-        advanced_settings_page = AdvancedSettingsPage(
-            self.browser, self.course_info['org'], self.course_info['number'], self.course_info['run']
-        )
-        advanced_settings_page.wait_for_page()
-
     def test_add_students_to_cohort_success(self):
         """
         Scenario: When students are added to a cohort, the appropriate notification is shown.
@@ -441,9 +424,31 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
 
         self.assertTrue(self.cohort_management_page.is_assignment_settings_disabled)
 
-        message = "There must be one cohort to which students can be randomly assigned."
+        message = "There must be one cohort to which students can automatically be assigned."
         self.assertEqual(message, self.cohort_management_page.assignment_settings_message)
 
+    def test_cohort_enable_disable(self):
+        """
+        Scenario: Cohort Enable/Disable checkbox related functionality is working as intended.
+
+        Given I have a cohorted course with a user.
+        And I can see the `Enable Cohorts` checkbox is checked.
+        And cohort management controls are visible.
+        When I uncheck the `Enable Cohorts` checkbox.
+        Then cohort management controls are not visible.
+        And When I reload the page.
+        Then I can see the `Enable Cohorts` checkbox is unchecked.
+        And cohort management controls are not visible.
+        """
+        self.assertTrue(self.cohort_management_page.is_cohorted)
+        self.assertTrue(self.cohort_management_page.cohort_management_controls_visible())
+        self.cohort_management_page.is_cohorted = False
+        self.assertFalse(self.cohort_management_page.cohort_management_controls_visible())
+        self.browser.refresh()
+        self.cohort_management_page.wait_for_page()
+        self.assertFalse(self.cohort_management_page.is_cohorted)
+        self.assertFalse(self.cohort_management_page.cohort_management_controls_visible())
+
     def test_link_to_data_download(self):
         """
         Scenario: a link is present from the cohort configuration in
@@ -614,6 +619,297 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin
         self.assertEquals(expected_message, messages[0])
 
 
+@attr('shard_3')
+class CohortDiscussionTopicsTest(UniqueCourseTest, CohortTestMixin):
+    """
+    Tests for cohorting the inline and course-wide discussion topics.
+    """
+    def setUp(self):
+        """
+        Set up a discussion topics
+        """
+        super(CohortDiscussionTopicsTest, self).setUp()
+
+        self.discussion_id = "test_discussion_{}".format(uuid.uuid4().hex)
+        self.course_fixture = CourseFixture(**self.course_info).add_children(
+            XBlockFixtureDesc("chapter", "Test Section").add_children(
+                XBlockFixtureDesc("sequential", "Test Subsection").add_children(
+                    XBlockFixtureDesc("vertical", "Test Unit").add_children(
+                        XBlockFixtureDesc(
+                            "discussion",
+                            "Test Discussion",
+                            metadata={"discussion_id": self.discussion_id}
+                        )
+                    )
+                )
+            )
+        ).install()
+
+        # create course with single cohort and two content groups (user_partition of type "cohort")
+        self.cohort_name = "OnlyCohort"
+        self.setup_cohort_config(self.course_fixture)
+        self.cohort_id = self.add_manual_cohort(self.course_fixture, self.cohort_name)
+
+        # login as an instructor
+        self.instructor_name = "instructor_user"
+        self.instructor_id = AutoAuthPage(
+            self.browser, username=self.instructor_name, email="instructor_user@example.com",
+            course_id=self.course_id, staff=True
+        ).visit().get_user_id()
+
+        # go to the membership page on the instructor dashboard
+        self.instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id)
+        self.instructor_dashboard_page.visit()
+        self.cohort_management_page = self.instructor_dashboard_page.select_cohort_management()
+        self.cohort_management_page.wait_for_page()
+
+        self.course_wide_key = 'course-wide'
+        self.inline_key = 'inline'
+
+    def cohort_discussion_topics_are_visible(self):
+        """
+        Assert that discussion topics are visible with appropriate content.
+        """
+        self.cohort_management_page.toggles_showing_of_discussion_topics()
+        self.assertTrue(self.cohort_management_page.discussion_topics_visible())
+
+        self.assertEqual(
+            "Course-Wide Discussion Topics",
+            self.cohort_management_page.cohort_discussion_heading_is_visible(self.course_wide_key)
+        )
+        self.assertTrue(self.cohort_management_page.is_save_button_disabled(self.course_wide_key))
+
+        self.assertEqual(
+            "Content-Specific Discussion Topics",
+            self.cohort_management_page.cohort_discussion_heading_is_visible(self.inline_key)
+        )
+        self.assertTrue(self.cohort_management_page.is_save_button_disabled(self.inline_key))
+
+    def save_and_verify_discussion_topics(self, key):
+        """
+        Saves the discussion topics and the verify the changes.
+        """
+        # click on the inline save button.
+        self.cohort_management_page.save_discussion_topics(key)
+
+        # verifies that changes saved successfully.
+        confirmation_message = self.cohort_management_page.get_cohort_discussions_message(key=key)
+        self.assertEqual("Your changes have been saved.", confirmation_message)
+
+        # save button disabled again.
+        self.assertTrue(self.cohort_management_page.is_save_button_disabled(key))
+
+    def reload_page(self):
+        """
+        Refresh the page.
+        """
+        self.browser.refresh()
+        self.cohort_management_page.wait_for_page()
+
+        self.instructor_dashboard_page.select_cohort_management()
+        self.cohort_management_page.wait_for_page()
+
+        self.cohort_discussion_topics_are_visible()
+
+    def verify_discussion_topics_after_reload(self, key, cohorted_topics):
+        """
+        Verifies the changed topics.
+        """
+        self.reload_page()
+        self.assertEqual(self.cohort_management_page.get_cohorted_topics_count(key), cohorted_topics)
+
+    def test_cohort_course_wide_discussion_topic(self):
+        """
+        Scenario: cohort a course-wide discussion topic.
+
+        Given I have a course with a cohort defined,
+        And a course-wide discussion with disabled Save button.
+        When I click on the course-wide discussion topic
+        Then I see the enabled save button
+        When I click on save button
+        Then I see success message
+        When I reload the page
+        Then I see the discussion topic selected
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        cohorted_topics_before = self.cohort_management_page.get_cohorted_topics_count(self.course_wide_key)
+        self.cohort_management_page.select_discussion_topic(self.course_wide_key)
+
+        self.assertFalse(self.cohort_management_page.is_save_button_disabled(self.course_wide_key))
+
+        self.save_and_verify_discussion_topics(key=self.course_wide_key)
+        cohorted_topics_after = self.cohort_management_page.get_cohorted_topics_count(self.course_wide_key)
+
+        self.assertNotEqual(cohorted_topics_before, cohorted_topics_after)
+
+        self.verify_discussion_topics_after_reload(self.course_wide_key, cohorted_topics_after)
+
+    def test_always_cohort_inline_topic_enabled(self):
+        """
+        Scenario: Select the always_cohort_inline_topics radio button
+
+        Given I have a course with a cohort defined,
+        And a inline discussion topic with disabled Save button.
+        When I click on always_cohort_inline_topics
+        Then I see enabled save button
+        And I see disabled inline discussion topics
+        When I reload the page
+        Then I see the option enabled
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        # enable always inline discussion topics.
+        self.cohort_management_page.select_always_inline_discussion()
+
+        self.assertTrue(self.cohort_management_page.inline_discussion_topics_disabled())
+
+        self.reload_page()
+        self.assertIsNotNone(self.cohort_management_page.always_inline_discussion_selected())
+
+    def test_cohort_some_inline_topics_enabled(self):
+        """
+        Scenario: Select the cohort_some_inline_topics radio button
+
+        Given I have a course with a cohort defined,
+        And a inline discussion topic with disabled Save button.
+        When I click on cohort_some_inline_topics
+        Then I see enabled save button
+        And I see enabled inline discussion topics
+        When I reload the page
+        Then I see the option enabled
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        # enable some inline discussion topic radio button.
+        self.cohort_management_page.select_cohort_some_inline_discussion()
+        # I see that save button is enabled
+        self.assertFalse(self.cohort_management_page.is_save_button_disabled(self.inline_key))
+        # I see that inline discussion topics are enabled
+        self.assertFalse(self.cohort_management_page.inline_discussion_topics_disabled())
+
+        self.reload_page()
+        self.assertIsNotNone(self.cohort_management_page.cohort_some_inline_discussion_selected())
+
+    def test_cohort_inline_discussion_topic(self):
+        """
+        Scenario: cohort inline discussion topic.
+
+        Given I have a course with a cohort defined,
+        And a inline discussion topic with disabled Save button.
+        When I click on cohort_some_inline_discussion_topics
+        Then I see enabled saved button
+        And When I click on inline discussion topic
+        And I see enabled save button
+        And When i click save button
+        Then I see success message
+        When I reload the page
+        Then I see the discussion topic selected
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        # select some inline discussion topics radio button.
+        self.cohort_management_page.select_cohort_some_inline_discussion()
+
+        cohorted_topics_before = self.cohort_management_page.get_cohorted_topics_count(self.inline_key)
+        # check the discussion topic.
+        self.cohort_management_page.select_discussion_topic(self.inline_key)
+
+        # Save button enabled.
+        self.assertFalse(self.cohort_management_page.is_save_button_disabled(self.inline_key))
+
+        # verifies that changes saved successfully.
+        self.save_and_verify_discussion_topics(key=self.inline_key)
+
+        cohorted_topics_after = self.cohort_management_page.get_cohorted_topics_count(self.inline_key)
+        self.assertNotEqual(cohorted_topics_before, cohorted_topics_after)
+
+        self.verify_discussion_topics_after_reload(self.inline_key, cohorted_topics_after)
+
+    def test_verify_that_selecting_the_final_child_selects_category(self):
+        """
+        Scenario: Category should be selected on selecting final child.
+
+        Given I have a course with a cohort defined,
+        And a inline discussion with disabled Save button.
+        When I click on child topics
+        Then I see enabled saved button
+        Then I see parent category to be checked.
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        # enable some inline discussion topics.
+        self.cohort_management_page.select_cohort_some_inline_discussion()
+
+        # category should not be selected.
+        self.assertFalse(self.cohort_management_page.is_category_selected())
+
+        # check the discussion topic.
+        self.cohort_management_page.select_discussion_topic(self.inline_key)
+
+        # verify that category is selected.
+        self.assertTrue(self.cohort_management_page.is_category_selected())
+
+    def test_verify_that_deselecting_the_final_child_deselects_category(self):
+        """
+        Scenario: Category should be deselected on deselecting final child.
+
+        Given I have a course with a cohort defined,
+        And a inline discussion with disabled Save button.
+        When I click on final child topics
+        Then I see enabled saved button
+        Then I see parent category to be deselected.
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        # enable some inline discussion topics.
+        self.cohort_management_page.select_cohort_some_inline_discussion()
+
+        # category should not be selected.
+        self.assertFalse(self.cohort_management_page.is_category_selected())
+
+        # check the discussion topic.
+        self.cohort_management_page.select_discussion_topic(self.inline_key)
+
+        # verify that category is selected.
+        self.assertTrue(self.cohort_management_page.is_category_selected())
+
+        # un-check the discussion topic.
+        self.cohort_management_page.select_discussion_topic(self.inline_key)
+
+        # category should not be selected.
+        self.assertFalse(self.cohort_management_page.is_category_selected())
+
+    def test_verify_that_correct_subset_of_category_being_selected_after_save(self):
+        """
+        Scenario: Category should be selected on selecting final child.
+
+        Given I have a course with a cohort defined,
+        And a inline discussion with disabled Save button.
+        When I click on child topics
+        Then I see enabled saved button
+        When I select subset of category
+        And I click on save button
+        Then I see success message with
+        same sub-category being selected
+        """
+        self.cohort_discussion_topics_are_visible()
+
+        # enable some inline discussion topics.
+        self.cohort_management_page.select_cohort_some_inline_discussion()
+
+        # category should not be selected.
+        self.assertFalse(self.cohort_management_page.is_category_selected())
+
+        cohorted_topics_after = self.cohort_management_page.get_cohorted_topics_count(self.inline_key)
+
+        # verifies that changes saved successfully.
+        self.save_and_verify_discussion_topics(key=self.inline_key)
+
+        # verify changes after reload.
+        self.verify_discussion_topics_after_reload(self.inline_key, cohorted_topics_after)
+
+
 @attr('shard_3')
 class CohortContentGroupAssociationTest(UniqueCourseTest, CohortTestMixin):
     """
@@ -656,8 +952,7 @@ class CohortContentGroupAssociationTest(UniqueCourseTest, CohortTestMixin):
         # go to the membership page on the instructor dashboard
         self.instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id)
         self.instructor_dashboard_page.visit()
-        membership_page = self.instructor_dashboard_page.select_membership()
-        self.cohort_management_page = membership_page.select_cohort_management_section()
+        self.cohort_management_page = self.instructor_dashboard_page.select_cohort_management()
 
     def test_no_content_group_linked(self):
         """

common/test/acceptance/tests/test_cohorted_courseware.py

@@ -2,18 +2,17 @@
 End-to-end test for cohorted courseware. This uses both Studio and LMS.
 """
 
-from nose.plugins.attrib import attr
 import json
+from nose.plugins.attrib import attr
 
 from studio.base_studio_test import ContainerBase
 
 from ..pages.studio.settings_group_configurations import GroupConfigurationsPage
-from ..pages.studio.settings_advanced import AdvancedSettingsPage
 from ..pages.studio.auto_auth import AutoAuthPage as StudioAutoAuthPage
 from ..fixtures.course import XBlockFixtureDesc
+from ..fixtures import LMS_BASE_URL
 from ..pages.studio.component_editor import ComponentVisibilityEditorView
 from ..pages.lms.instructor_dashboard import InstructorDashboardPage
-from ..pages.lms.course_nav import CourseNavPage
 from ..pages.lms.courseware import CoursewarePage
 from ..pages.lms.auto_auth import AutoAuthPage as LmsAutoAuthPage
 from ..tests.lms.test_lms_user_preview import verify_expected_problem_visibility
@@ -80,28 +79,14 @@ class EndToEndCohortedCoursewareTest(ContainerBase):
             )
         )
 
-    def enable_cohorts_in_course(self):
+    def enable_cohorting(self, course_fixture):
         """
-        This turns on cohorts for the course. Currently this is still done through Advanced
-        Settings. Eventually it will be done in the LMS Instructor Dashboard.
+        Enables cohorting for the current course.
         """
-        advanced_settings = AdvancedSettingsPage(
-            self.browser,
-            self.course_info['org'],
-            self.course_info['number'],
-            self.course_info['run']
-        )
-
-        advanced_settings.visit()
-        cohort_config = '{"cohorted": true}'
-        advanced_settings.set('Cohort Configuration', cohort_config)
-        advanced_settings.refresh_and_wait_for_load()
-
-        self.assertEquals(
-            json.loads(cohort_config),
-            json.loads(advanced_settings.get('Cohort Configuration')),
-            'Wrong input for Cohort Configuration'
-        )
+        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):
         """
@@ -154,8 +139,7 @@ class EndToEndCohortedCoursewareTest(ContainerBase):
         """
         instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id)
         instructor_dashboard_page.visit()
-        membership_page = instructor_dashboard_page.select_membership()
-        cohort_management_page = membership_page.select_cohort_management_section()
+        cohort_management_page = instructor_dashboard_page.select_cohort_management()
 
         def add_cohort_with_student(cohort_name, content_group, student):
             cohort_management_page.add_cohort(cohort_name, content_group=content_group)
@@ -220,7 +204,7 @@ class EndToEndCohortedCoursewareTest(ContainerBase):
           And the student in Cohort B can see all the problems except the one linked to Content Group A
           And the student in the default cohort can ony see the problem that is unlinked to any Content Group
         """
-        self.enable_cohorts_in_course()
+        self.enable_cohorting(self.course_fixture)
         self.create_content_groups()
         self.link_problems_to_content_groups_and_publish()
         self.create_cohorts_and_assign_students()

lms/djangoapps/django_comment_client/forum/tests.py

@@ -309,18 +309,18 @@ class SingleThreadTestCase(ModuleStoreTestCase):
 @patch('requests.request')
 class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
     """
-    Ensures the number of modulestore queries is deterministic based on the
-    number of responses retrieved for a given discussion thread.
+    Ensures the number of modulestore queries and number of sql queries are
+    independent of the number of responses retrieved for a given discussion thread.
     """
     MODULESTORE = TEST_DATA_MONGO_MODULESTORE
 
     @ddt.data(
-        # old mongo with cache: number of responses plus 17.  TODO: O(n)!
-        (ModuleStoreEnum.Type.mongo, 1, 23, 18),
-        (ModuleStoreEnum.Type.mongo, 50, 366, 67),
+        # old mongo with cache: 15
+        (ModuleStoreEnum.Type.mongo, 1, 21, 15, 40, 27),
+        (ModuleStoreEnum.Type.mongo, 50, 315, 15, 628, 27),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, 1, 3, 3),
-        (ModuleStoreEnum.Type.split, 50, 3, 3),
+        (ModuleStoreEnum.Type.split, 1, 3, 3, 40, 27),
+        (ModuleStoreEnum.Type.split, 50, 3, 3, 628, 27),
     )
     @ddt.unpack
     def test_number_of_mongo_queries(
@@ -329,6 +329,8 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
             num_thread_responses,
             num_uncached_mongo_calls,
             num_cached_mongo_calls,
+            num_uncached_sql_queries,
+            num_cached_sql_queries,
             mock_request
     ):
         with modulestore().default_store(default_store):
@@ -370,15 +372,16 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
             backend='django.core.cache.backends.dummy.DummyCache',
             LOCATION='single_thread_local_cache'
         )
-        cached_calls = {
-            single_thread_dummy_cache: num_uncached_mongo_calls,
-            single_thread_local_cache: num_cached_mongo_calls
-        }
-        for single_thread_cache, expected_calls in cached_calls.items():
+        cached_calls = [
+            [single_thread_dummy_cache, num_uncached_mongo_calls, num_uncached_sql_queries],
+            [single_thread_local_cache, num_cached_mongo_calls, num_cached_sql_queries]
+        ]
+        for single_thread_cache, expected_mongo_calls, expected_sql_queries in cached_calls:
             single_thread_cache.clear()
             with patch("django_comment_client.permissions.CACHE", single_thread_cache):
-                with check_mongo_calls(expected_calls):
-                    call_single_thread()
+                with self.assertNumQueries(expected_sql_queries):
+                    with check_mongo_calls(expected_mongo_calls):
+                        call_single_thread()
             single_thread_cache.clear()
 
 

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -1,17 +1,24 @@
 # -*- coding: utf-8 -*-
-from datetime import datetime
+import datetime
 import json
+import mock
 from pytz import UTC
+from django.utils.timezone import UTC as django_utc
 
+from django_comment_client.tests.factories import RoleFactory
+from django_comment_client.tests.unicode import UnicodeTestMixin
+import django_comment_client.utils as utils
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from edxmako import add_lookup
-import mock
 
 from django_comment_client.tests.factories import RoleFactory
 from django_comment_client.tests.unicode import UnicodeTestMixin
 from django_comment_client.tests.utils import ContentGroupTestCase
 import django_comment_client.utils as utils
+
+from courseware.tests.factories import InstructorFactory
+from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -174,12 +181,13 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             # This test needs to use a course that has already started --
             # discussion topics only show up if the course has already started,
             # and the default start date for courses is Jan 1, 2030.
-            start=datetime(2012, 2, 3, tzinfo=UTC)
+            start=datetime.datetime(2012, 2, 3, tzinfo=UTC)
         )
         # Courses get a default discussion topic on creation, so remove it
         self.course.discussion_topics = {}
         self.course.save()
         self.discussion_num = 0
+        self.instructor = InstructorFactory(course_key=self.course.id)
         self.maxDiff = None  # pylint: disable=invalid-name
 
     def create_discussion(self, discussion_category, discussion_target, **kwargs):
@@ -193,6 +201,15 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             **kwargs
         )
 
+    def assert_category_map_equals(self, expected, cohorted_if_in_list=False, exclude_unstarted=True):  # pylint: disable=arguments-differ
+        """
+        Asserts the expected map with the map returned by get_discussion_category_map method.
+        """
+        self.assertEqual(
+            utils.get_discussion_category_map(self.course, self.instructor, cohorted_if_in_list, exclude_unstarted),
+            expected
+        )
+
     def test_empty(self):
         self.assert_category_map_equals({"entries": {}, "subcategories": {}, "children": []})
 
@@ -218,20 +235,35 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
 
         check_cohorted_topics([])  # default (empty) cohort config
 
-        self.course.cohort_config = {"cohorted": False, "cohorted_discussions": []}
+        set_course_cohort_settings(course_key=self.course.id, is_cohorted=False, cohorted_discussions=[])
         check_cohorted_topics([])
 
-        self.course.cohort_config = {"cohorted": True, "cohorted_discussions": []}
+        set_course_cohort_settings(course_key=self.course.id, is_cohorted=True, cohorted_discussions=[])
         check_cohorted_topics([])
 
-        self.course.cohort_config = {"cohorted": True, "cohorted_discussions": ["Topic_B", "Topic_C"]}
+        set_course_cohort_settings(
+            course_key=self.course.id,
+            is_cohorted=True,
+            cohorted_discussions=["Topic_B", "Topic_C"],
+            always_cohort_inline_discussions=False,
+        )
         check_cohorted_topics(["Topic_B", "Topic_C"])
 
-        self.course.cohort_config = {"cohorted": True, "cohorted_discussions": ["Topic_A", "Some_Other_Topic"]}
+        set_course_cohort_settings(
+            course_key=self.course.id,
+            is_cohorted=True,
+            cohorted_discussions=["Topic_A", "Some_Other_Topic"],
+            always_cohort_inline_discussions=False,
+        )
         check_cohorted_topics(["Topic_A"])
 
         # unlikely case, but make sure it works.
-        self.course.cohort_config = {"cohorted": False, "cohorted_discussions": ["Topic_A"]}
+        set_course_cohort_settings(
+            course_key=self.course.id,
+            is_cohorted=False,
+            cohorted_discussions=["Topic_A"],
+            always_cohort_inline_discussions=False,
+        )
         check_cohorted_topics([])
 
     def test_single_inline(self):
@@ -256,6 +288,85 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             }
         )
 
+    def test_inline_with_always_cohort_inline_discussion_flag(self):
+        self.create_discussion("Chapter", "Discussion")
+        set_course_cohort_settings(course_key=self.course.id, is_cohorted=True)
+
+        self.assert_category_map_equals(
+            {
+                "entries": {},
+                "subcategories": {
+                    "Chapter": {
+                        "entries": {
+                            "Discussion": {
+                                "id": "discussion1",
+                                "sort_key": None,
+                                "is_cohorted": True,
+                            }
+                        },
+                        "subcategories": {},
+                        "children": ["Discussion"]
+                    }
+                },
+                "children": ["Chapter"]
+            }
+        )
+
+    def test_inline_without_always_cohort_inline_discussion_flag(self):
+        self.create_discussion("Chapter", "Discussion")
+        set_course_cohort_settings(course_key=self.course.id, is_cohorted=True, always_cohort_inline_discussions=False)
+
+        self.assert_category_map_equals(
+            {
+                "entries": {},
+                "subcategories": {
+                    "Chapter": {
+                        "entries": {
+                            "Discussion": {
+                                "id": "discussion1",
+                                "sort_key": None,
+                                "is_cohorted": False,
+                            }
+                        },
+                        "subcategories": {},
+                        "children": ["Discussion"]
+                    }
+                },
+                "children": ["Chapter"]
+            },
+            cohorted_if_in_list=True
+        )
+
+    def test_get_unstarted_discussion_modules(self):
+        later = datetime.datetime(datetime.MAXYEAR, 1, 1, tzinfo=django_utc())
+
+        self.create_discussion("Chapter 1", "Discussion 1", start=later)
+
+        self.assert_category_map_equals(
+            {
+                "entries": {},
+                "subcategories": {
+                    "Chapter 1": {
+                        "entries": {
+                            "Discussion 1": {
+                                "id": "discussion1",
+                                "sort_key": None,
+                                "is_cohorted": False,
+                                "start_date": later
+                            }
+                        },
+                        "subcategories": {},
+                        "children": ["Discussion 1"],
+                        "start_date": later,
+                        "sort_key": "Chapter 1"
+                    }
+                },
+                "children": ["Chapter 1"]
+            },
+            cohorted_if_in_list=True,
+            exclude_unstarted=False
+        )
+
     def test_tree(self):
         self.create_discussion("Chapter 1", "Discussion 1")
         self.create_discussion("Chapter 1", "Discussion 2")
@@ -352,11 +463,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         check_cohorted(False)
 
         # explicitly disabled cohorting
-        self.course.cohort_config = {"cohorted": False}
+        set_course_cohort_settings(course_key=self.course.id, is_cohorted=False)
         check_cohorted(False)
 
         # explicitly enabled cohorting
-        self.course.cohort_config = {"cohorted": True}
+        set_course_cohort_settings(course_key=self.course.id, is_cohorted=True)
         check_cohorted(True)
 
     def test_tree_with_duplicate_targets(self):
@@ -381,8 +492,8 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.assertEqual(set(subsection1["entries"].keys()), subsection1_discussions)
 
     def test_start_date_filter(self):
-        now = datetime.now()
-        later = datetime.max
+        now = datetime.datetime.now()
+        later = datetime.datetime.max
         self.create_discussion("Chapter 1", "Discussion 1", start=now)
         self.create_discussion("Chapter 1", "Discussion 2 обсуждение", start=later)
         self.create_discussion("Chapter 2", "Discussion", start=now)
@@ -420,6 +531,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
                 "children": ["Chapter 1", "Chapter 2"]
             }
         )
+        self.maxDiff = None
 
     def test_sort_inline_explicit(self):
         self.create_discussion("Chapter", "Discussion 1", sort_key="D")

lms/djangoapps/django_comment_client/utils.py

@@ -19,8 +19,9 @@ from django_comment_client.permissions import check_permissions_by_view, cached_
 from edxmako import lookup_template
 
 from courseware.access import has_access
-from openedx.core.djangoapps.course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted, \
-    is_course_cohorted
+from openedx.core.djangoapps.course_groups.cohorts import (
+    get_course_cohort_settings, get_cohort_by_id, get_cohort_id, is_commentable_cohorted, is_course_cohorted
+)
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup
 
 
@@ -60,8 +61,7 @@ def has_forum_access(uname, course_id, rolename):
     return role.users.filter(username=uname).exists()
 
 
-# pylint: disable=invalid-name
-def get_accessible_discussion_modules(course, user):
+def get_accessible_discussion_modules(course, user, include_all=False):  # pylint: disable=invalid-name
     """
     Return a list of all valid discussion modules in this course that
     are accessible to the given user.
@@ -70,14 +70,14 @@ def get_accessible_discussion_modules(course, user):
 
     def has_required_keys(module):
         for key in ('discussion_id', 'discussion_category', 'discussion_target'):
-            if getattr(module, key) is None:
+            if getattr(module, key, None) is None:
                 log.warning("Required key '%s' not in discussion %s, leaving out of category map" % (key, module.location))
                 return False
         return True
 
     return [
         module for module in all_modules
-        if has_required_keys(module) and has_access(user, 'load', module, course.id)
+        if has_required_keys(module) and (include_all or has_access(user, 'load', module, course.id))
     ]
 
 
@@ -145,24 +145,63 @@ def _sort_map_entries(category_map, sort_alpha):
     category_map["children"] = [x[0] for x in sorted(things, key=lambda x: x[1]["sort_key"])]
 
 
-def get_discussion_category_map(course, user):
+def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude_unstarted=True):
     """
     Transform the list of this course's discussion modules into a recursive dictionary structure.  This is used
     to render the discussion category map in the discussion tab sidebar for a given user.
+
+    Args:
+        course: Course for which to get the ids.
+        user:  User to check for access.
+        cohorted_if_in_list (bool): If True, inline topics are marked is_cohorted only if they are
+            in course_cohort_settings.discussion_topics.
+
+    Example:
+        >>> example = {
+        >>>               "entries": {
+        >>>                   "General": {
+        >>>                       "sort_key": "General",
+        >>>                       "is_cohorted": True,
+        >>>                       "id": "i4x-edx-eiorguegnru-course-foobarbaz"
+        >>>                   }
+        >>>               },
+        >>>               "children": ["General", "Getting Started"],
+        >>>               "subcategories": {
+        >>>                   "Getting Started": {
+        >>>                       "subcategories": {},
+        >>>                       "children": [
+        >>>                           "Working with Videos",
+        >>>                           "Videos on edX"
+        >>>                       ],
+        >>>                       "entries": {
+        >>>                           "Working with Videos": {
+        >>>                               "sort_key": None,
+        >>>                               "is_cohorted": False,
+        >>>                               "id": "d9f970a42067413cbb633f81cfb12604"
+        >>>                           },
+        >>>                           "Videos on edX": {
+        >>>                               "sort_key": None,
+        >>>                               "is_cohorted": False,
+        >>>                               "id": "98d8feb5971041a085512ae22b398613"
+        >>>                           }
+        >>>                       }
+        >>>                   }
+        >>>               }
+        >>>          }
+
     """
     unexpanded_category_map = defaultdict(list)
 
     modules = get_accessible_discussion_modules(course, user)
 
-    is_course_cohorted = course.is_cohorted
-    cohorted_discussion_ids = course.cohorted_discussions
+    course_cohort_settings = get_course_cohort_settings(course.id)
 
     for module in modules:
         id = module.discussion_id
         title = module.discussion_target
         sort_key = module.sort_key
         category = " / ".join([x.strip() for x in module.discussion_category.split("/")])
-        #Handle case where module.start is None
+        # Handle case where module.start is None
         entry_start_date = module.start if module.start else datetime.max.replace(tzinfo=pytz.UTC)
         unexpanded_category_map[category].append({"title": title, "id": id, "sort_key": sort_key, "start_date": entry_start_date})
 
@@ -198,8 +237,17 @@ def get_discussion_category_map(course, user):
             if node[level]["start_date"] > category_start_date:
                 node[level]["start_date"] = category_start_date
 
+        always_cohort_inline_discussions = (  # pylint: disable=invalid-name
+            not cohorted_if_in_list and course_cohort_settings.always_cohort_inline_discussions
+        )
         dupe_counters = defaultdict(lambda: 0)  # counts the number of times we see each title
         for entry in entries:
+            is_entry_cohorted = (
+                course_cohort_settings.is_cohorted and (
+                    always_cohort_inline_discussions or entry["id"] in course_cohort_settings.cohorted_discussions
+                )
+            )
+
             title = entry["title"]
             if node[level]["entries"][title]:
                 # If we've already seen this title, append an incrementing number to disambiguate
@@ -209,29 +257,38 @@ def get_discussion_category_map(course, user):
             node[level]["entries"][title] = {"id": entry["id"],
                                              "sort_key": entry["sort_key"],
                                              "start_date": entry["start_date"],
-                                             "is_cohorted": is_course_cohorted}
+                                             "is_cohorted": is_entry_cohorted}
 
     # TODO.  BUG! : course location is not unique across multiple course runs!
     # (I think Kevin already noticed this)  Need to send course_id with requests, store it
     # in the backend.
     for topic, entry in course.discussion_topics.items():
-        category_map['entries'][topic] = {"id": entry["id"],
-                                          "sort_key": entry.get("sort_key", topic),
-                                          "start_date": datetime.now(UTC()),
-                                          "is_cohorted": is_course_cohorted and entry["id"] in cohorted_discussion_ids}
+        category_map['entries'][topic] = {
+            "id": entry["id"],
+            "sort_key": entry.get("sort_key", topic),
+            "start_date": datetime.now(UTC()),
+            "is_cohorted": (course_cohort_settings.is_cohorted and
+                            entry["id"] in course_cohort_settings.cohorted_discussions)
+        }
 
     _sort_map_entries(category_map, course.discussion_sort_alpha)
 
-    return _filter_unstarted_categories(category_map)
+    return _filter_unstarted_categories(category_map) if exclude_unstarted else category_map
 
 
-def get_discussion_categories_ids(course, user):
+def get_discussion_categories_ids(course, user, include_all=False):
     """
     Returns a list of available ids of categories for the course that
     are accessible to the given user.
+
+    Args:
+        course: Course for which to get the ids.
+        user:  User to check for access.
+        include_all (bool): If True, return all ids. Used by configuration views.
+
     """
     accessible_discussion_ids = [
-        module.discussion_id for module in get_accessible_discussion_modules(course, user)
+        module.discussion_id for module in get_accessible_discussion_modules(course, user, include_all=include_all)
     ]
     return course.top_level_discussion_topic_ids + accessible_discussion_ids
 
@@ -402,7 +459,7 @@ def add_courseware_context(content_list, course, user, id_map=None):
             content.update({"courseware_url": url, "courseware_title": title})
 
 
-def prepare_content(content, course_key, is_staff=False):
+def prepare_content(content, course_key, is_staff=False, course_is_cohorted=None):
     """
     This function is used to pre-process thread and comment models in various
     ways before adding them to the HTTP response.  This includes fixing empty
@@ -411,6 +468,12 @@ def prepare_content(content, course_key, is_staff=False):
 
     @TODO: not all response pre-processing steps are currently integrated into
     this function.
+
+    Arguments:
+        content (dict): A thread or comment.
+        course_key (CourseKey): The course key of the course.
+        is_staff (bool): Whether the user is a staff member.
+        course_is_cohorted (bool): Whether the course is cohorted.
     """
     fields = [
         'id', 'title', 'body', 'course_id', 'anonymous', 'anonymous_to_peers',
@@ -450,14 +513,18 @@ def prepare_content(content, course_key, is_staff=False):
         else:
             del endorsement["user_id"]
 
+    if course_is_cohorted is None:
+        course_is_cohorted = is_course_cohorted(course_key)
+
     for child_content_key in ["children", "endorsed_responses", "non_endorsed_responses"]:
         if child_content_key in content:
             children = [
-                prepare_content(child, course_key, is_staff) for child in content[child_content_key]
+                prepare_content(child, course_key, is_staff, course_is_cohorted=course_is_cohorted)
+                for child in content[child_content_key]
             ]
             content[child_content_key] = children
 
-    if is_course_cohorted(course_key):
+    if course_is_cohorted:
         # Augment the specified thread info to include the group name if a group id is present.
         if content.get('group_id') is not None:
             content['group_name'] = get_cohort_by_id(course_key, content.get('group_id')).name

lms/djangoapps/instructor/tests/test_api.py

@@ -58,6 +58,8 @@ from instructor.views.api import generate_unique_password
 from instructor.views.api import _split_input_list, common_exceptions_400
 from instructor_task.api_helper import AlreadyRunningError
 
+from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings
+
 from .test_tools import msk_from_problem_urlname
 from ..views.tools import get_extended_due
 
@@ -2002,8 +2004,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
         cohorted, and does not when the course is not cohorted.
         """
         url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)})
-        self.course.cohort_config = {'cohorted': is_cohorted}
-        self.store.update_item(self.course, self.instructor.id)
+        set_course_cohort_settings(self.course.id, is_cohorted=is_cohorted)
 
         response = self.client.get(url, {})
         res_json = json.loads(response.content)

lms/djangoapps/instructor/views/api.py

@@ -104,6 +104,7 @@ from .tools import (
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from opaque_keys import InvalidKeyError
+from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted
 
 log = logging.getLogger(__name__)
 
@@ -1002,7 +1003,7 @@ def get_students_features(request, course_id, csv=False):  # pylint: disable=red
         'goals': _('Goals'),
     }
 
-    if course.is_cohorted:
+    if is_course_cohorted(course.id):
         # Translators: 'Cohort' refers to a group of students within a course.
         query_features.append('cohort')
         query_features_names['cohort'] = _('Cohort')

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -65,9 +65,7 @@ def instructor_dashboard_2(request, course_id):
         'finance_admin': CourseFinanceAdminRole(course_key).has_user(request.user),
         'sales_admin': CourseSalesAdminRole(course_key).has_user(request.user),
         'staff': has_access(request.user, 'staff', course),
-        'forum_admin': has_forum_access(
-            request.user, course_key, FORUM_ROLE_ADMINISTRATOR
-        ),
+        'forum_admin': has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR),
     }
 
     if not access['staff']:
@@ -76,6 +74,7 @@ def instructor_dashboard_2(request, course_id):
     sections = [
         _section_course_info(course, access),
         _section_membership(course, access),
+        _section_cohort_management(course, access),
         _section_student_admin(course, access),
         _section_data_download(course, access),
         _section_analytics(course, access),
@@ -330,9 +329,24 @@ def _section_membership(course, access):
         'modify_access_url': reverse('modify_access', kwargs={'course_id': unicode(course_key)}),
         'list_forum_members_url': reverse('list_forum_members', kwargs={'course_id': unicode(course_key)}),
         'update_forum_role_membership_url': reverse('update_forum_role_membership', kwargs={'course_id': unicode(course_key)}),
-        'cohorts_ajax_url': reverse('cohorts', kwargs={'course_key_string': unicode(course_key)}),
-        'advanced_settings_url': get_studio_url(course, 'settings/advanced'),
+    }
+    return section_data
+
+
+def _section_cohort_management(course, access):
+    """ Provide data for the corresponding cohort management section """
+    course_key = course.id
+    section_data = {
+        'section_key': 'cohort_management',
+        'section_display_name': _('Cohorts'),
+        'access': access,
+        'course_cohort_settings_url': reverse(
+            'course_cohort_settings',
+            kwargs={'course_key_string': unicode(course_key)}
+        ),
+        '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)}),
     }
     return section_data
 

lms/djangoapps/instructor/views/legacy.py

@@ -56,6 +56,8 @@ from django.utils.translation import ugettext as _
 
 from microsite_configuration import microsite
 from opaque_keys.edx.locations import i4xEncoder
+from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted
+
 
 log = logging.getLogger(__name__)
 
@@ -451,6 +453,7 @@ def instructor_dashboard(request, course_id):
 
     context = {
         'course': course,
+        'course_is_cohorted': is_course_cohorted(course.id),
         'staff_access': True,
         'admin_access': request.user.is_staff,
         'instructor_access': instructor_access,

lms/djangoapps/instructor_task/tasks_helper.py

@@ -34,7 +34,7 @@ from lms.djangoapps.lms_xblock.runtime import LmsPartitionService
 from openedx.core.djangoapps.course_groups.cohorts import get_cohort
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup
 from opaque_keys.edx.keys import UsageKey
-from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
+from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted
 from student.models import CourseEnrollment
 
 
@@ -578,7 +578,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input,
     TASK_LOG.info(u'%s, Task type: %s, Starting task execution', task_info_string, action_name)
 
     course = get_course_by_id(course_id)
-    cohorts_header = ['Cohort Name'] if course.is_cohorted else []
+    course_is_cohorted = is_course_cohorted(course.id)
+    cohorts_header = ['Cohort Name'] if course_is_cohorted else []
 
     experiment_partitions = get_split_user_partitions(course.user_partitions)
     group_configs_header = [u'Experiment Group ({})'.format(partition.name) for partition in experiment_partitions]
@@ -632,7 +633,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input,
             }
 
             cohorts_group_name = []
-            if course.is_cohorted:
+            if course_is_cohorted:
                 group = get_cohort(student, course_id, assign=False)
                 cohorts_group_name.append(group.name if group else '')
 

lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee

@@ -176,6 +176,9 @@ setup_instructor_dashboard_sections = (idash_content) ->
   ,
     constructor: window.InstructorDashboard.sections.Metrics
     $element: idash_content.find ".#{CSS_IDASH_SECTION}#metrics"
+  ,
+    constructor: window.InstructorDashboard.sections.CohortManagement
+    $element: idash_content.find ".#{CSS_IDASH_SECTION}#cohort_management"
   ]
 
   sections_to_initialize.map ({constructor, $element}) ->

lms/static/js/edxnotes/views/toggle_notes_factory.js

@@ -2,7 +2,7 @@
 'use strict';
 define([
     'jquery', 'underscore', 'backbone', 'gettext',
-    'annotator_1.2.9', 'js/edxnotes/views/visibility_decorator'
+    'annotator_1.2.9', 'js/edxnotes/views/visibility_decorator', 'js/utils/animation'
 ], function($, _, Backbone, gettext, Annotator, EdxnotesVisibilityDecorator) {
     var ToggleNotesView = Backbone.View.extend({
         events: {
@@ -31,7 +31,7 @@ define([
         toggleHandler: function (event) {
             event.preventDefault();
             this.visibility = !this.visibility;
-            this.showActionMessage();
+            AnimationUtil.triggerAnimation(this.actionToggleMessage);
             this.toggleNotes(this.visibility);
         },
 
@@ -51,13 +51,6 @@ define([
             this.sendRequest();
         },
 
-        showActionMessage: function () {
-            // The following lines are necessary to re-trigger the CSS animation on span.action-toggle-message
-            this.actionToggleMessage.removeClass('is-fleeting');
-            this.actionToggleMessage.offset().width = this.actionToggleMessage.offset().width;
-            this.actionToggleMessage.addClass('is-fleeting');
-        },
-
         enableNotes: function () {
             _.each($('.edx-notes-wrapper'), EdxnotesVisibilityDecorator.enableNote);
             this.actionLink.addClass('is-active');

lms/static/js/groups/models/cohort_discussions.js

@@ -0,0 +1,14 @@
+var edx = edx || {};
+
+(function(Backbone) {
+    'use strict';
+
+    edx.groups = edx.groups || {};
+
+    edx.groups.DiscussionTopicsSettingsModel = Backbone.Model.extend({
+        defaults: {
+            course_wide_discussions: {},
+            inline_discussions: {}
+        }
+    });
+}).call(this, Backbone);

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

@@ -0,0 +1,17 @@
+var edx = edx || {};
+
+(function(Backbone) {
+    'use strict';
+
+    edx.groups = edx.groups || {};
+
+    edx.groups.CourseCohortSettingsModel = Backbone.Model.extend({
+        idAttribute: 'id',
+        defaults: {
+            is_cohorted: false,
+            cohorted_inline_discussions: [],
+            cohorted_course_wide_discussions:[],
+            always_cohort_inline_discussions: true
+        }
+    });
+}).call(this, Backbone);

lms/static/js/groups/views/cohort_discussions.js

@@ -0,0 +1,99 @@
+var edx = edx || {};
+
+(function ($, _, Backbone, gettext, interpolate_text, NotificationModel, NotificationView) {
+    'use strict';
+
+    edx.groups = edx.groups || {};
+
+    edx.groups.CohortDiscussionConfigurationView = Backbone.View.extend({
+
+        /**
+         * Add/Remove the disabled attribute on given element.
+         * @param {object} $element - The element to disable/enable.
+         * @param {bool} disable - The flag to add/remove 'disabled' attribute.
+         */
+        setDisabled: function($element, disable) {
+            $element.prop('disabled', disable ? 'disabled' : false);
+        },
+
+        /**
+         * Returns the cohorted discussions list.
+         * @param {string} selector - To select the discussion elements whose ids to return.
+         * @returns {Array} - Cohorted discussions.
+         */
+        getCohortedDiscussions: function(selector) {
+            var self=this,
+                cohortedDiscussions = [];
+
+            _.each(self.$(selector), function (topic) {
+                cohortedDiscussions.push($(topic).data('id'))
+            });
+            return cohortedDiscussions;
+        },
+
+        /**
+         * Save the cohortSettings' 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,
+                saveOperation = $.Deferred(),
+                showErrorMessage;
+
+            showErrorMessage = function (message, $element) {
+                self.showMessage(message, $element, 'error');
+            };
+            this.removeNotification();
+
+            cohortSettingsModel.save(
+                fieldData, {patch: true, wait: true}
+            ).done(function () {
+                saveOperation.resolve();
+            }).fail(function (result) {
+                var errorMessage = null;
+                try {
+                    var 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.");
+                }
+                showErrorMessage(errorMessage, $element);
+                saveOperation.reject();
+            });
+            return saveOperation.promise();
+        },
+
+        /**
+         * Shows the notification messages before given element using the NotificationModel.
+         * @param {string} message - Text message to show.
+         * @param {object} $element - Message would be shown before this element.
+         * @param {string} type - Type of message to show e.g. confirmation or error.
+         */
+        showMessage: function (message, $element, type) {
+            var model = new NotificationModel({type: type || 'confirmation', title: message});
+            this.removeNotification();
+            this.notification = new NotificationView({
+                model: model
+            });
+            $element.before(this.notification.$el);
+            this.notification.render();
+        },
+
+        /**
+         *Removes the notification messages.
+         */
+        removeNotification: function () {
+            if (this.notification) {
+                this.notification.remove();
+            }
+        }
+
+    });
+}).call(this, $, _, Backbone, gettext, interpolate_text, NotificationModel, NotificationView
+);

lms/static/js/groups/views/cohort_discussions_course_wide.js

@@ -0,0 +1,83 @@
+var edx = edx || {};
+
+(function ($, _, Backbone, gettext, interpolate_text, CohortDiscussionConfigurationView) {
+    'use strict';
+
+    edx.groups = edx.groups || {};
+
+    edx.groups.CourseWideDiscussionsView = CohortDiscussionConfigurationView.extend({
+        events: {
+            'change .check-discussion-subcategory-course-wide': 'discussionCategoryStateChanged',
+            'click .cohort-course-wide-discussions-form .action-save': 'saveCourseWideDiscussionsForm'
+        },
+
+        initialize: function (options) {
+            this.template = _.template($('#cohort-discussions-course-wide-tpl').text());
+            this.cohortSettings = options.cohortSettings;
+        },
+
+        render: function () {
+            this.$('.cohort-course-wide-discussions-nav').html(this.template({
+                courseWideTopics: this.getCourseWideDiscussionsHtml(
+                    this.model.get('course_wide_discussions')
+                )
+            }));
+            this.setDisabled(this.$('.cohort-course-wide-discussions-form .action-save'), true);
+        },
+
+        /**
+         * Returns the html list for course-wide discussion topics.
+         * @param {object} courseWideDiscussions - course-wide discussions object from server.
+         * @returns {Array} - HTML list for course-wide discussion topics.
+         */
+        getCourseWideDiscussionsHtml: function (courseWideDiscussions) {
+            var subCategoryTemplate = _.template($('#cohort-discussions-subcategory-tpl').html()),
+                entries = courseWideDiscussions.entries,
+                children = courseWideDiscussions.children;
+
+            return _.map(children, function (name) {
+                var entry = entries[name];
+                return subCategoryTemplate({
+                    name: name,
+                    id: entry.id,
+                    is_cohorted: entry.is_cohorted,
+                    type: 'course-wide'
+                });
+            }).join('');
+        },
+
+        /**
+         * Enables the save button for course-wide discussions.
+         */
+        discussionCategoryStateChanged: function(event) {
+            event.preventDefault();
+            this.setDisabled(this.$('.cohort-course-wide-discussions-form .action-save'), false);
+        },
+
+        /**
+         * Sends the cohorted_course_wide_discussions to the server and renders the view.
+         */
+        saveCourseWideDiscussionsForm: function (event) {
+            event.preventDefault();
+
+            var self = this,
+                courseWideCohortedDiscussions = self.getCohortedDiscussions(
+                    '.check-discussion-subcategory-course-wide:checked'
+                ),
+                fieldData = { cohorted_course_wide_discussions: courseWideCohortedDiscussions };
+
+            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'));
+                    }).fail(function() {
+                        var errorMessage = gettext("We've encountered an error. Refresh your browser and then try again.");
+                        self.showMessage(errorMessage, self.$('.course-wide-discussion-topics'), 'error')
+                    });
+            });
+        }
+
+    });
+}).call(this, $, _, Backbone, gettext, interpolate_text, edx.groups.CohortDiscussionConfigurationView);

lms/static/js/groups/views/cohort_discussions_inline.js

@@ -0,0 +1,145 @@
+var edx = edx || {};
+
+(function ($, _, Backbone, gettext, interpolate_text, CohortDiscussionConfigurationView) {
+    'use strict';
+
+    edx.groups = edx.groups || {};
+
+    edx.groups.InlineDiscussionsView = CohortDiscussionConfigurationView.extend({
+        events: {
+            'change .check-discussion-category': 'setSaveButton',
+            'change .check-discussion-subcategory-inline': 'setSaveButton',
+            'click .cohort-inline-discussions-form .action-save': 'saveInlineDiscussionsForm',
+            'change .check-all-inline-discussions': 'setAllInlineDiscussions',
+            'change .check-cohort-inline-discussions': 'setSomeInlineDiscussions'
+        },
+
+        initialize: function (options) {
+            this.template = _.template($('#cohort-discussions-inline-tpl').text());
+            this.cohortSettings = options.cohortSettings;
+        },
+
+        render: function () {
+            var alwaysCohortInlineDiscussions = this.cohortSettings.get('always_cohort_inline_discussions');
+
+            this.$('.cohort-inline-discussions-nav').html(this.template({
+                inlineDiscussionTopics: this.getInlineDiscussionsHtml(this.model.get('inline_discussions')),
+                alwaysCohortInlineDiscussions:alwaysCohortInlineDiscussions
+            }));
+
+            // Provides the semantics for a nested list of tri-state checkboxes.
+            // When attached to a jQuery element it listens for change events to
+            // input[type=checkbox] elements, and updates the checked and indeterminate
+            // based on the checked values of any checkboxes in child elements of the DOM.
+            this.$('ul.inline-topics').qubit();
+
+            this.setElementsEnabled(alwaysCohortInlineDiscussions, true);
+        },
+
+        /**
+         * Generate html list for inline discussion topics.
+         * @params {object} inlineDiscussions - inline discussions object from server.
+         * @returns {Array} - HTML for inline discussion topics.
+         */
+        getInlineDiscussionsHtml: function (inlineDiscussions) {
+            var categoryTemplate = _.template($('#cohort-discussions-category-tpl').html()),
+                entryTemplate = _.template($('#cohort-discussions-subcategory-tpl').html()),
+                isCategoryCohorted = false,
+                children = inlineDiscussions.children,
+                entries = inlineDiscussions.entries,
+                subcategories = inlineDiscussions.subcategories;
+
+            return _.map(children, function (name) {
+                var html = '', entry;
+                if (entries && _.has(entries, name)) {
+                    entry = entries[name];
+                    html = entryTemplate({
+                        name: name,
+                        id: entry.id,
+                        is_cohorted: entry.is_cohorted,
+                        type: 'inline'
+                    });
+                } else { // subcategory
+                    html = categoryTemplate({
+                        name: name,
+                        entries: this.getInlineDiscussionsHtml(subcategories[name]),
+                        isCategoryCohorted: isCategoryCohorted
+                    });
+                }
+                return html;
+            }, this).join('');
+        },
+
+        /**
+         * Enable/Disable the inline discussion elements.
+         *
+         * Disables the category and sub-category checkboxes.
+         * Enables the save button.
+         */
+        setAllInlineDiscussions: function(event) {
+            event.preventDefault();
+            this.setElementsEnabled(($(event.currentTarget).prop('checked')), false);
+        },
+
+        /**
+         * Enables the inline discussion elements.
+         *
+         * Enables the category and sub-category checkboxes.
+         * Enables the save button.
+         */
+        setSomeInlineDiscussions: function(event) {
+            event.preventDefault();
+            this.setElementsEnabled(!($(event.currentTarget).prop('checked')), false);
+        },
+
+        /**
+         * Enable/Disable the inline discussion elements.
+         *
+         * 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.
+         */
+        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);
+        },
+
+        /**
+         * Enables the save button for inline discussions.
+         */
+        setSaveButton: function(event) {
+            this.setDisabled(this.$('.cohort-inline-discussions-form .action-save'), false);
+        },
+
+        /**
+         * Sends the cohorted_inline_discussions to the server and renders the view.
+         */
+        saveInlineDiscussionsForm: function (event) {
+            event.preventDefault();
+
+            var self = this,
+                cohortedInlineDiscussions = self.getCohortedDiscussions(
+                    '.check-discussion-subcategory-inline:checked'
+                ),
+                fieldData= {
+                    cohorted_inline_discussions: cohortedInlineDiscussions,
+                    always_cohort_inline_discussions: self.$('.check-all-inline-discussions').prop('checked')
+                };
+
+            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'));
+                        }).fail(function() {
+                            var errorMessage = gettext("We've encountered an error. Refresh your browser and then try again.");
+                            self.showMessage(errorMessage, self.$('.inline-discussion-topics'), 'error')
+                        });
+                });
+        }
+
+    });
+}).call(this, $, _, Backbone, gettext, interpolate_text, edx.groups.CohortDiscussionConfigurationView);

lms/static/js/groups/views/cohort_editor.js

@@ -44,8 +44,7 @@ var edx = edx || {};
 
         renderGroupHeader: function() {
             this.$('.cohort-management-group-header').html(this.groupHeaderTemplate({
-                cohort: this.model,
-                studioAdvancedSettingsUrl: this.context.studioAdvancedSettingsUrl
+                cohort: this.model
             }));
         },
 

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

@@ -1,7 +1,8 @@
 var edx = edx || {};
 
 (function($, _, Backbone, gettext, interpolate_text, CohortModel, CohortEditorView, CohortFormView,
-          NotificationModel, NotificationView, FileUploaderView) {
+          CourseCohortSettingsNotificationView, NotificationModel, NotificationView, FileUploaderView,
+          InlineDiscussionsView, CourseWideDiscussionsView) {
     'use strict';
 
     var hiddenClass = 'is-hidden',
@@ -12,11 +13,13 @@ var edx = edx || {};
     edx.groups.CohortsView = Backbone.View.extend({
         events : {
             'change .cohort-select': 'onCohortSelected',
+            'change .cohorts-state': 'onCohortsEnabledChanged',
             'click .action-create': 'showAddCohortForm',
             'click .cohort-management-add-form .action-save': 'saveAddCohortForm',
             'click .cohort-management-add-form .action-cancel': 'cancelAddCohortForm',
             'click .link-cross-reference': 'showSection',
-            'click .toggle-cohort-management-secondary': 'showCsvUpload'
+            'click .toggle-cohort-management-secondary': 'showCsvUpload',
+            'click .toggle-cohort-management-discussions': 'showDiscussionTopics'
         },
 
         initialize: function(options) {
@@ -26,19 +29,21 @@ var edx = edx || {};
             this.selectorTemplate = _.template($('#cohort-selector-tpl').text());
             this.context = options.context;
             this.contentGroups = options.contentGroups;
+            this.cohortSettings = options.cohortSettings;
             model.on('sync', this.onSync, this);
 
-            // Update cohort counts when the user clicks back on the membership tab
+            // Update cohort counts when the user clicks back on the cohort management tab
             // (for example, after uploading a csv file of cohort assignments and then
             // checking results on data download tab).
-            $(this.getSectionCss('membership')).click(function () {
+            $(this.getSectionCss('cohort_management')).click(function () {
                 model.fetch();
             });
         },
 
         render: function() {
             this.$el.html(this.template({
-                cohorts: this.model.models
+                cohorts: this.model.models,
+                cohortsEnabled: this.cohortSettings.get('is_cohorted')
             }));
             this.onSync();
             return this;
@@ -51,6 +56,14 @@ var edx = edx || {};
             }));
         },
 
+        renderCourseCohortSettingsNotificationView: function() {
+            var cohortStateMessageNotificationView = new CourseCohortSettingsNotificationView({
+                el: $('.cohort-state-message'),
+                cohortEnabled: this.getCohortsEnabled()
+            });
+            cohortStateMessageNotificationView.render();
+        },
+
         onSync: function(model, response, options) {
             var selectedCohort = this.lastSelectedCohortId && this.model.get(this.lastSelectedCohortId),
                 hasCohorts = this.model.length > 0,
@@ -98,6 +111,34 @@ var edx = edx || {};
             this.showCohortEditor(selectedCohort);
         },
 
+        onCohortsEnabledChanged: function(event) {
+            event.preventDefault();
+            this.saveCohortSettings();
+        },
+
+        saveCohortSettings: function() {
+            var self = this,
+                cohortSettings,
+                fieldData = {is_cohorted: this.getCohortsEnabled()};
+            cohortSettings = this.cohortSettings;
+            cohortSettings.save(
+                fieldData, {patch: true, wait: true}
+            ).done(function() {
+                self.render();
+                self.renderCourseCohortSettingsNotificationView();
+            }).fail(function(result) {
+                self.showNotification({
+                    type: 'error',
+                    title: gettext("We've encountered an error. Refresh your browser and then try again.")},
+                    self.$('.cohorts-state-section')
+                );
+            });
+        },
+
+        getCohortsEnabled: function() {
+            return this.$('.cohorts-state').prop('checked');
+        },
+
         showCohortEditor: function(cohort) {
             this.removeNotification();
             if (this.editor) {
@@ -167,9 +208,11 @@ var edx = edx || {};
 
         setCohortEditorVisibility: function(showEditor) {
             if (showEditor) {
+                this.$('.cohorts-state-section').removeClass(disabledClass).attr('aria-disabled', false);
                 this.$('.cohort-management-group').removeClass(hiddenClass);
                 this.$('.cohort-management-nav').removeClass(disabledClass).attr('aria-disabled', false);
             } else {
+                this.$('.cohorts-state-section').addClass(disabledClass).attr('aria-disabled', true);
                 this.$('.cohort-management-group').addClass(hiddenClass);
                 this.$('.cohort-management-nav').addClass(disabledClass).attr('aria-disabled', true);
             }
@@ -236,10 +279,32 @@ var edx = edx || {};
                 this.$('#file-upload-form-file').focus();
             }
         },
+        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 a[data-section='" + section + "']";
         }
     });
 }).call(this, $, _, Backbone, gettext, interpolate_text, edx.groups.CohortModel, edx.groups.CohortEditorView,
-    edx.groups.CohortFormView, NotificationModel, NotificationView, FileUploaderView);
+    edx.groups.CohortFormView, edx.groups.CourseCohortSettingsNotificationView, NotificationModel, NotificationView,
+    FileUploaderView, edx.groups.InlineDiscussionsView, edx.groups.CourseWideDiscussionsView);

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

@@ -0,0 +1,41 @@
+;(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'],
+        function($) {
+
+            return function(contentGroups, studioGroupConfigurationsUrl) {
+
+                var cohorts = new edx.groups.CohortCollection(),
+                    courseCohortSettings = new edx.groups.CourseCohortSettingsModel(),
+                    discussionTopicsSettings = new edx.groups.DiscussionTopicsSettingsModel();
+
+                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');
+                
+                var cohortsView = new edx.groups.CohortsView({
+                    el: cohortManagementElement,
+                    model: cohorts,
+                    contentGroups: contentGroups,
+                    cohortSettings: courseCohortSettings,
+                    context: {
+                        discussionTopicsSettingsModel: discussionTopicsSettings,
+                        uploadCohortsCsvUrl: cohortManagementElement.data('upload_cohorts_csv_url'),
+                        studioGroupConfigurationsUrl: studioGroupConfigurationsUrl
+                    }
+                });
+
+                cohorts.fetch().done(function() {
+                    courseCohortSettings.fetch().done(function() {
+                        discussionTopicsSettings.fetch().done(function() {
+                            cohortsView.render();
+                        });
+                    });
+                });
+            };
+    });
+}).call(this, define || RequireJS.define);
+

lms/static/js/groups/views/course_cohort_settings_notification.js

@@ -0,0 +1,31 @@
+var edx = edx || {};
+
+(function($, _, Backbone, gettext) {
+    'use strict';
+
+    edx.groups = edx.groups || {};
+
+    edx.groups.CourseCohortSettingsNotificationView = Backbone.View.extend({
+        initialize: function(options) {
+            this.template = _.template($('#cohort-state-tpl').text());
+            this.cohortEnabled = options.cohortEnabled;
+        },
+
+        render: function() {
+            this.$el.html(this.template({}));
+            this.showCohortStateMessage();
+            return this;
+        },
+
+        showCohortStateMessage: function () {
+            var actionToggleMessage = this.$('.action-toggle-message');
+
+            AnimationUtil.triggerAnimation(actionToggleMessage);
+            if (this.cohortEnabled) {
+                actionToggleMessage.text(gettext('Cohorts Enabled'));
+            } else {
+                actionToggleMessage.text(gettext('Cohorts Disabled'));
+            }
+        }
+    });
+}).call(this, $, _, Backbone, gettext);

lms/static/js/instructor_dashboard/cohort_management.js

@@ -0,0 +1,19 @@
+(function() {
+    var CohortManagement;
+
+    CohortManagement = (function() {
+
+    function CohortManagement($section) {
+      this.$section = $section;
+      this.$section.data('wrapper', this);
+    }
+
+    CohortManagement.prototype.onClickTitle = function() {};
+
+    return CohortManagement;
+
+    })();
+
+    window.InstructorDashboard.sections.CohortManagement = CohortManagement;
+
+}).call(this);

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

@@ -1,15 +1,32 @@
 define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpers/template_helpers',
-        'js/groups/views/cohorts', 'js/groups/collections/cohort', 'js/groups/models/content_group'],
-    function (Backbone, $, AjaxHelpers, TemplateHelpers, CohortsView, CohortCollection, ContentGroupModel) {
+        'js/groups/views/cohorts', 'js/groups/collections/cohort', 'js/groups/models/content_group',
+        'js/groups/models/course_cohort_settings', 'js/utils/animation', 'js/vendor/jquery.qubit',
+        'js/groups/views/course_cohort_settings_notification', 'js/groups/models/cohort_discussions',
+        'js/groups/views/cohort_discussions', 'js/groups/views/cohort_discussions_course_wide',
+        'js/groups/views/cohort_discussions_inline'
+        ],
+    function (Backbone, $, AjaxHelpers, TemplateHelpers, CohortsView, CohortCollection, ContentGroupModel,
+              CourseCohortSettingsModel, AnimationUtil, Qubit, CourseCohortSettingsNotificationView, DiscussionTopicsSettingsModel,
+              CohortDiscussionsView, CohortCourseWideDiscussionsView, CohortInlineDiscussionsView) {
         'use strict';
 
         describe("Cohorts View", function () {
             var catLoversInitialCount = 123, dogLoversInitialCount = 456, unknownUserMessage,
-                createMockCohort, createMockCohorts, createMockContentGroups, createCohortsView, cohortsView,
-                requests, respondToRefresh, verifyMessage, verifyNoMessage, verifyDetailedMessage, verifyHeader,
-                expectCohortAddRequest, getAddModal, selectContentGroup, clearContentGroup, saveFormAndExpectErrors,
-                MOCK_COHORTED_USER_PARTITION_ID, MOCK_UPLOAD_COHORTS_CSV_URL, MOCK_STUDIO_ADVANCED_SETTINGS_URL,
-                MOCK_STUDIO_GROUP_CONFIGURATIONS_URL, MOCK_MANUAL_ASSIGNMENT, MOCK_RANDOM_ASSIGNMENT;
+                createMockCohort, createMockCohorts, createMockContentGroups, createMockCohortSettingsJson,
+                createCohortsView, cohortsView, requests, respondToRefresh, verifyMessage, verifyNoMessage,
+                verifyDetailedMessage, verifyHeader, expectCohortAddRequest, getAddModal, selectContentGroup,
+                clearContentGroup, saveFormAndExpectErrors, createMockCohortSettings, MOCK_COHORTED_USER_PARTITION_ID,
+                MOCK_UPLOAD_COHORTS_CSV_URL, MOCK_STUDIO_ADVANCED_SETTINGS_URL, MOCK_STUDIO_GROUP_CONFIGURATIONS_URL,
+                MOCK_MANUAL_ASSIGNMENT, MOCK_RANDOM_ASSIGNMENT, createMockCohortDiscussionsJson,
+                createMockCohortDiscussions, showAndAssertDiscussionTopics;
+
+            // Selectors
+            var discussionsToggle ='.toggle-cohort-management-discussions',
+                inlineDiscussionsFormCss = '.cohort-inline-discussions-form',
+                courseWideDiscussionsFormCss = '.cohort-course-wide-discussions-form',
+                courseWideDiscussionsSaveButtonCss = '.cohort-course-wide-discussions-form .action-save',
+                inlineDiscussionsSaveButtonCss = '.cohort-inline-discussions-form .action-save',
+                inlineDiscussionsForm, courseWideDiscussionsForm;
 
             MOCK_MANUAL_ASSIGNMENT = 'manual';
             MOCK_RANDOM_ASSIGNMENT = 'random';
@@ -49,23 +66,95 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
                 ];
             };
 
+            createMockCohortSettingsJson = function (isCohorted, cohortedInlineDiscussions, cohortedCourseWideDiscussions, alwaysCohortInlineDiscussions) {
+                return {
+                    id: 0,
+                    is_cohorted: isCohorted || false,
+                    cohorted_inline_discussions: cohortedInlineDiscussions || [],
+                    cohorted_course_wide_discussions: cohortedCourseWideDiscussions || [],
+                    always_cohort_inline_discussions: alwaysCohortInlineDiscussions || true
+                };
+            };
+
+            createMockCohortSettings = function (isCohorted, cohortedInlineDiscussions, cohortedCourseWideDiscussions, alwaysCohortInlineDiscussions) {
+                return new CourseCohortSettingsModel(
+                    createMockCohortSettingsJson(isCohorted, cohortedInlineDiscussions, cohortedCourseWideDiscussions, alwaysCohortInlineDiscussions)
+                );
+            };
+
+            createMockCohortDiscussionsJson = function (allCohorted) {
+                return {
+                    course_wide_discussions: {
+                        children: ['Topic_C_1', 'Topic_C_2'],
+                        entries: {
+                            Topic_C_1: {
+                               sort_key: null,
+                               is_cohorted: true,
+                               id: 'Topic_C_1'
+                            },
+                            Topic_C_2: {
+                                sort_key: null,
+                                is_cohorted: false,
+                                id: 'Topic_C_2'
+                            }
+                        }
+                    },
+                    inline_discussions: {
+                        subcategories: {
+                            Topic_I_1: {
+                                subcategories: {},
+                                children: ['Inline_Discussion_1', 'Inline_Discussion_2'],
+                                entries: {
+                                    Inline_Discussion_1: {
+                                        sort_key: null,
+                                        is_cohorted: true,
+                                        id: 'Inline_Discussion_1'
+                                    },
+                                    Inline_Discussion_2: {
+                                        sort_key: null,
+                                        is_cohorted: allCohorted || false,
+                                        id: 'Inline_Discussion_2'
+                                    }
+                                }
+                            }
+                        },
+                        children: ['Topic_I_1']
+                    }
+                };
+            };
+
+            createMockCohortDiscussions = function (allCohorted) {
+                return new DiscussionTopicsSettingsModel(
+                    createMockCohortDiscussionsJson(allCohorted)
+                );
+            };
+
             createCohortsView = function (test, options) {
-                var cohortsJson, cohorts, contentGroups;
+                var cohortsJson, cohorts, contentGroups, cohortSettings, cohortDiscussions;
                 options = options || {};
                 cohortsJson = options.cohorts ? {cohorts: options.cohorts} : createMockCohorts();
                 cohorts = new CohortCollection(cohortsJson, {parse: true});
                 contentGroups = options.contentGroups || createMockContentGroups();
+                cohortSettings = options.cohortSettings || createMockCohortSettings(true);
+                cohortSettings.url = '/mock_service/cohorts/settings';
                 cohorts.url = '/mock_service/cohorts';
+
+                cohortDiscussions = options.cohortDiscussions || createMockCohortDiscussions();
+                cohortDiscussions.url = '/mock_service/cohorts/discussion/topics';
+
                 requests = AjaxHelpers.requests(test);
                 cohortsView = new CohortsView({
                     model: cohorts,
                     contentGroups: contentGroups,
+                    cohortSettings: cohortSettings,
                     context: {
+                        discussionTopicsSettingsModel: cohortDiscussions,
                         uploadCohortsCsvUrl: MOCK_UPLOAD_COHORTS_CSV_URL,
                         studioAdvancedSettingsUrl: MOCK_STUDIO_ADVANCED_SETTINGS_URL,
                         studioGroupConfigurationsUrl: MOCK_STUDIO_GROUP_CONFIGURATIONS_URL
                     }
                 });
+
                 cohortsView.render();
                 if (options && options.selectCohort) {
                     cohortsView.$('.cohort-select').val(options.selectCohort.toString()).change();
@@ -172,18 +261,58 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
                 verifyDetailedMessage(expectedTitle, 'error', errors);
             };
 
+            showAndAssertDiscussionTopics = function(that) {
+
+                createCohortsView(that);
+
+                // Should see the control to toggle cohort discussions.
+                expect(cohortsView.$(discussionsToggle)).not.toHaveClass('is-hidden');
+                // But discussions form should not be visible until toggle is clicked.
+                expect(cohortsView.$(inlineDiscussionsFormCss).length).toBe(0);
+                expect(cohortsView.$(courseWideDiscussionsFormCss).length).toBe(0);
+
+                expect(cohortsView.$(discussionsToggle).text()).
+                    toContain('Specify whether discussion topics are divided by cohort');
+
+                cohortsView.$(discussionsToggle).click();
+                // After toggle is clicked, it should be hidden.
+                expect(cohortsView.$(discussionsToggle)).toHaveClass('is-hidden');
+
+                // Should see the course wide discussions form and its content
+                courseWideDiscussionsForm = cohortsView.$(courseWideDiscussionsFormCss);
+                expect(courseWideDiscussionsForm.length).toBe(1);
+
+                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.');
+
+                // Should see the inline discussions form and its content
+                inlineDiscussionsForm = cohortsView.$(inlineDiscussionsFormCss);
+                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.');
+            };
+
             unknownUserMessage = function (name) {
                 return "Unknown user: " +  name;
             };
 
             beforeEach(function () {
-                setFixtures('<ul class="instructor-nav"><li class="nav-item"><<a href data-section="membership" class="active-section">Membership</a></li></ul><div></div>');
+                setFixtures('<ul class="instructor-nav"><li class="nav-item"><<a href data-section="cohort_management" class="active-section">Cohort Management</a></li></ul><div></div><div class="cohort-state-message"></div>');
                 TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohorts');
                 TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-form');
                 TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-selector');
                 TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-editor');
                 TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-group-header');
                 TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/notification');
+                TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-state');
+                TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-discussions-category');
+                TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory');
+                TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-discussions-course-wide');
+                TemplateHelpers.installTemplate('templates/instructor/instructor_dashboard_2/cohort-discussions-inline');
                 TemplateHelpers.installTemplate('templates/file-upload');
             });
 
@@ -197,12 +326,14 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
 
                 // If no cohorts have been created, can't upload a CSV file.
                 expect(cohortsView.$('.wrapper-cohort-supplemental')).toHaveClass('is-hidden');
+                // if no cohorts have been created, can't show the link to discussion topics.
+                expect(cohortsView.$('.cohort-discussions-nav')).toHaveClass('is-hidden');
             });
 
             it("syncs data when membership tab is clicked", function() {
                 createCohortsView(this, {selectCohort: 1});
                 verifyHeader(1, 'Cat Lovers', catLoversInitialCount);
-                $(cohortsView.getSectionCss("membership")).click();
+                $(cohortsView.getSectionCss("cohort_management")).click();
                 AjaxHelpers.expectRequest(requests, 'GET', '/mock_service/cohorts');
                 respondToRefresh(1001, 2);
                 verifyHeader(1, 'Cat Lovers', 1001);
@@ -236,6 +367,10 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
                     .toBe("Your file 'upload_file.txt' has been uploaded. Allow a few minutes for processing.");
             });
 
+            it('can show discussion topics if cohort exists', function () {
+                showAndAssertDiscussionTopics(this);
+            });
+
             describe("Cohort Selector", function () {
                 it('has no initial selection', function () {
                     createCohortsView(this);
@@ -255,6 +390,62 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
                 });
             });
 
+            describe("Course Cohort Settings", function () {
+                it('can enable and disable cohorting', function () {
+                    createCohortsView(this, {cohortSettings: createMockCohortSettings(false)});
+
+                    expect(cohortsView.$('.cohorts-state').prop('checked')).toBeFalsy();
+
+                    cohortsView.$('.cohorts-state').prop('checked', true).change();
+                    AjaxHelpers.expectJsonRequest(
+                        requests, 'PATCH', '/mock_service/cohorts/settings',
+                        {is_cohorted: true}
+                    );
+                    AjaxHelpers.respondWithJson(
+                        requests,
+                        {is_cohorted: true}
+                    );
+                    expect(cohortsView.$('.cohorts-state').prop('checked')).toBeTruthy();
+
+                    cohortsView.$('.cohorts-state').prop('checked', false).change();
+                    AjaxHelpers.expectJsonRequest(
+                        requests, 'PATCH', '/mock_service/cohorts/settings',
+                        {is_cohorted: false}
+                    );
+                    AjaxHelpers.respondWithJson(
+                        requests,
+                        {is_cohorted: false}
+                    );
+                    expect(cohortsView.$('.cohorts-state').prop('checked')).toBeFalsy();
+                });
+
+
+                it('shows an appropriate cohort status message', function () {
+                     var createCourseCohortSettingsNotificationView = function (is_cohorted) {
+                        var notificationView = new CourseCohortSettingsNotificationView({
+                            el: $('.cohort-state-message'),
+                            cohortEnabled: is_cohorted});
+                        notificationView.render();
+                        return notificationView;
+                     };
+
+                    var notificationView = createCourseCohortSettingsNotificationView(true);
+                    expect(notificationView.$('.action-toggle-message').text().trim()).toBe('Cohorts Enabled');
+
+                    notificationView = createCourseCohortSettingsNotificationView(false);
+                    expect(notificationView.$('.action-toggle-message').text().trim()).toBe('Cohorts Disabled');
+                });
+
+                it('shows an appropriate error message for HTTP500', function () {
+                    createCohortsView(this, {cohortSettings: createMockCohortSettings(false)});
+                    expect(cohortsView.$('.cohorts-state').prop('checked')).toBeFalsy();
+                    cohortsView.$('.cohorts-state').prop('checked', true).change();
+                    AjaxHelpers.respondWithError(requests, 500);
+                    var expectedTitle = "We've encountered an error. Refresh your browser and then try again."
+                    expect(cohortsView.$('.message-title').text().trim()).toBe(expectedTitle);
+                });
+            });
+
             describe("Cohort Group Header", function () {
                 it("renders header correctly", function () {
                     var cohortName = 'Transformers',
@@ -867,7 +1058,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
 
                         // We have a single random cohort so we should not be allowed to change it assignment type
                         expect(cohortsView.$('.cohort-management-assignment-type-settings')).toHaveClass('is-disabled');
-                        expect(cohortsView.$('.copy-error').text()).toContain("There must be one cohort to which students can be randomly assigned.");
+                        expect(cohortsView.$('.copy-error').text()).toContain("There must be one cohort to which students can automatically be assigned.");
                     });
 
                     it("cancel settings works", function() {
@@ -880,5 +1071,379 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
                     });
                 });
             });
+
+            describe("Discussion Topics", function() {
+                var createCourseWideView, createInlineView,
+                    inlineView, courseWideView, assertCohortedTopics;
+
+                createCourseWideView = function(that) {
+                    createCohortsView(that);
+
+                    courseWideView = new CohortCourseWideDiscussionsView({
+                        el: cohortsView.$('.cohort-discussions-nav').removeClass('is-hidden'),
+                        model: cohortsView.context.discussionTopicsSettingsModel,
+                        cohortSettings: cohortsView.cohortSettings
+                    });
+                    courseWideView.render();
+                };
+
+                createInlineView = function(that, discussionTopicsSettingsModel) {
+                    createCohortsView(that);
+
+                    inlineView = new CohortInlineDiscussionsView({
+                        el: cohortsView.$('.cohort-discussions-nav').removeClass('is-hidden'),
+                        model: discussionTopicsSettingsModel || cohortsView.context.discussionTopicsSettingsModel,
+                        cohortSettings: cohortsView.cohortSettings
+                    });
+                    inlineView.render();
+                };
+
+                assertCohortedTopics = function(view, type) {
+                    expect(view.$('.check-discussion-subcategory-' + type).length).toBe(2);
+                    expect(view.$('.check-discussion-subcategory-' + type + ':checked').length).toBe(1);
+                };
+
+                it('renders the view properly', function() {
+                    showAndAssertDiscussionTopics(this);
+                });
+
+                describe("Course Wide", function() {
+
+                    it('shows the "Save" button as disabled initially', function() {
+                        createCourseWideView(this);
+                        expect(courseWideView.$(courseWideDiscussionsSaveButtonCss).prop('disabled')).toBeTruthy();
+                    });
+
+                    it('has one cohorted and one non-cohorted topic', function() {
+                        createCourseWideView(this);
+
+                        assertCohortedTopics(courseWideView, 'course-wide');
+
+                        expect(courseWideView.$('.cohorted-text').length).toBe(2);
+                        expect(courseWideView.$('.cohorted-text.hidden').length).toBe(1);
+                    });
+
+                    it('enables the "Save" button after changing checkbox', function() {
+                        createCourseWideView(this);
+
+                        // save button is disabled.
+                        expect(courseWideView.$(courseWideDiscussionsSaveButtonCss).prop('disabled')).toBeTruthy();
+
+                        $(courseWideView.$('.check-discussion-subcategory-course-wide')[0]).prop('checked', false).change();
+
+                        // save button is enabled.
+                        expect(courseWideView.$(courseWideDiscussionsSaveButtonCss).prop('disabled')).toBeFalsy();
+                    });
+
+                    it('saves the topic successfully', function() {
+                        createCourseWideView(this);
+
+                        $(courseWideView.$('.check-discussion-subcategory-course-wide')[1]).prop('checked', 'checked').change();
+                        expect(courseWideView.$(courseWideDiscussionsSaveButtonCss).prop('disabled')).toBeFalsy();
+
+                        // Save the updated settings
+                        courseWideView.$('.action-save').click();
+
+                        // fake requests for cohort settings with PATCH method.
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'PATCH', '/mock_service/cohorts/settings',
+                            {cohorted_course_wide_discussions: ['Topic_C_1', 'Topic_C_2']}
+                        );
+                        AjaxHelpers.respondWithJson(
+                            requests,
+                            {cohorted_course_wide_discussions: ['Topic_C_1', 'Topic_C_2']}
+                        );
+
+                        // fake request for discussion/topics with GET method.
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'GET', '/mock_service/cohorts/discussion/topics'
+                        );
+                        AjaxHelpers.respondWithJson(
+                            requests,
+                            createMockCohortDiscussions()
+                        );
+
+                        // verify the success message.
+                        expect(courseWideView.$(courseWideDiscussionsSaveButtonCss).prop('disabled')).toBeTruthy();
+                        verifyMessage('Your changes have been saved.', 'confirmation');
+                    });
+
+                    it('shows an appropriate message when subsequent "GET" returns HTTP500', function() {
+                        createCourseWideView(this);
+
+                        $(courseWideView.$('.check-discussion-subcategory-course-wide')[1]).prop('checked', 'checked').change();
+                        expect(courseWideView.$(courseWideDiscussionsSaveButtonCss).prop('disabled')).toBeFalsy();
+
+                        // Save the updated settings
+                        courseWideView.$('.action-save').click();
+
+                        // fake requests for cohort settings with PATCH method.
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'PATCH', '/mock_service/cohorts/settings',
+                            {cohorted_course_wide_discussions: ['Topic_C_1', 'Topic_C_2']}
+                        );
+                        AjaxHelpers.respondWithJson(
+                            requests,
+                            {cohorted_course_wide_discussions: ['Topic_C_1', 'Topic_C_2']}
+                        );
+
+                        // fake request for discussion/topics with GET method.
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'GET', '/mock_service/cohorts/discussion/topics'
+                        );
+                        AjaxHelpers.respondWithError(requests, 500);
+
+                        var expectedTitle = "We've encountered an error. Refresh your browser and then try again.";
+                        expect(courseWideView.$('.message-title').text().trim()).toBe(expectedTitle);
+                    });
+
+                    it('shows an appropriate error message for HTTP500', function () {
+                        createCourseWideView(this);
+
+                        $(courseWideView.$('.check-discussion-subcategory-course-wide')[1]).prop('checked', 'checked').change();
+                        courseWideView.$('.action-save').click();
+
+                        AjaxHelpers.respondWithError(requests, 500);
+                        var expectedTitle = "We've encountered an error. Refresh your browser and then try again.";
+                        expect(courseWideView.$('.message-title').text().trim()).toBe(expectedTitle);
+                    });
+                });
+
+                describe("Inline", function() {
+                    var enableSaveButton, mockGetRequest, verifySuccess, mockPatchRequest;
+
+                    enableSaveButton = function() {
+                        // enable the inline discussion topics.
+                        inlineView.$('.check-cohort-inline-discussions').prop('checked', 'checked').change();
+
+                        $(inlineView.$('.check-discussion-subcategory-inline')[0]).prop('checked', 'checked').change();
+
+                        expect(inlineView.$(inlineDiscussionsSaveButtonCss).prop('disabled')).toBeFalsy();
+                    };
+
+                    verifySuccess = function() {
+                        // verify the success message.
+                        expect(inlineView.$(inlineDiscussionsSaveButtonCss).prop('disabled')).toBeTruthy();
+                        verifyMessage('Your changes have been saved.', 'confirmation');
+                    };
+
+                    mockPatchRequest = function(cohortedInlineDiscussions) {
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'PATCH', '/mock_service/cohorts/settings',
+                            {
+                                cohorted_inline_discussions: cohortedInlineDiscussions,
+                                always_cohort_inline_discussions: false
+                            }
+                        );
+                        AjaxHelpers.respondWithJson(
+                            requests,
+                            {
+                                cohorted_inline_discussions: cohortedInlineDiscussions,
+                                always_cohort_inline_discussions: false
+                            }
+                        );
+                    };
+
+                    mockGetRequest = function(allCohorted) {
+                        // fake request for discussion/topics with GET method.
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'GET', '/mock_service/cohorts/discussion/topics'
+                        );
+                        AjaxHelpers.respondWithJson(
+                            requests,
+                            createMockCohortDiscussions(allCohorted)
+                        );
+                    };
+
+                    it('shows the "Save" button as disabled initially', function() {
+                        createInlineView(this);
+                        expect(inlineView.$(inlineDiscussionsSaveButtonCss).prop('disabled')).toBeTruthy();
+                    });
+
+                    it('shows always cohort radio button as selected', function() {
+                        createInlineView(this);
+                        inlineView.$('.check-all-inline-discussions').prop('checked', 'checked').change();
+
+                        // verify always cohort inline discussions is being selected.
+                        expect(inlineView.$('.check-all-inline-discussions').prop('checked')).toBeTruthy();
+
+                        // verify that inline topics are disabled
+                        expect(inlineView.$('.check-discussion-subcategory-inline').prop('disabled')).toBeTruthy();
+                        expect(inlineView.$('.check-discussion-category').prop('disabled')).toBeTruthy();
+
+                        // verify that cohort some topics are not being selected.
+                        expect(inlineView.$('.check-cohort-inline-discussions').prop('checked')).toBeFalsy();
+                    });
+
+                    it('shows cohort some topics radio button as selected', function() {
+                        createInlineView(this);
+                        inlineView.$('.check-cohort-inline-discussions').prop('checked', 'checked').change();
+
+                        // verify some cohort inline discussions radio is being selected.
+                        expect(inlineView.$('.check-cohort-inline-discussions').prop('checked')).toBeTruthy();
+
+                        // verify always cohort radio is not selected.
+                        expect(inlineView.$('.check-all-inline-discussions').prop('checked')).toBeFalsy();
+
+                        // verify that inline topics are enabled
+                        expect(inlineView.$('.check-discussion-subcategory-inline').prop('disabled')).toBeFalsy();
+                        expect(inlineView.$('.check-discussion-category').prop('disabled')).toBeFalsy();
+                    });
+
+                    it('has cohorted and non-cohorted topics', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+                        assertCohortedTopics(inlineView, 'inline');
+                    });
+
+                    it('enables "Save" button after changing from always inline option', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+                    });
+
+                    it('saves the topic', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        // Save the updated settings
+                        inlineView.$('.action-save').click();
+
+                        mockPatchRequest(['Inline_Discussion_1']);
+                        mockGetRequest();
+
+                        verifySuccess();
+                    });
+
+                    it('selects the parent category when all children are selected', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        // parent category should be indeterminate.
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(0);
+                        expect(inlineView.$('.check-discussion-category:indeterminate').length).toBe(1);
+
+                        inlineView.$('.check-discussion-subcategory-inline').prop('checked', 'checked').change();
+                        // parent should be checked as we checked all children
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(1);
+                    });
+
+                    it('selects/deselects all children when a parent category is selected/deselected', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(0);
+
+                        inlineView.$('.check-discussion-category').prop('checked', 'checked').change();
+
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(1);
+                        expect(inlineView.$('.check-discussion-subcategory-inline:checked').length).toBe(2);
+
+                        // un-check the parent, all children should be unchecd.
+                        inlineView.$('.check-discussion-category').prop('checked', false).change();
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(0);
+                        expect(inlineView.$('.check-discussion-subcategory-inline:checked').length).toBe(0);
+                    });
+
+                    it('saves correctly when a subset of topics are selected within a category', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        // parent category should be indeterminate.
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(0);
+                        expect(inlineView.$('.check-discussion-category:indeterminate').length).toBe(1);
+
+                        // Save the updated settings
+                        inlineView.$('.action-save').click();
+
+                        mockPatchRequest(['Inline_Discussion_1']);
+                        mockGetRequest();
+
+                        verifySuccess();
+                        // parent category should be indeterminate.
+                        expect(inlineView.$('.check-discussion-category:indeterminate').length).toBe(1);
+                    });
+
+                    it('saves correctly when all child topics are selected within a category', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        // parent category should be indeterminate.
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(0);
+                        expect(inlineView.$('.check-discussion-category:indeterminate').length).toBe(1);
+
+                        inlineView.$('.check-discussion-subcategory-inline').prop('checked', 'checked').change();
+                        // Save the updated settings
+                        inlineView.$('.action-save').click();
+
+                        mockPatchRequest(['Inline_Discussion_1', 'Inline_Discussion_2']);
+                        mockGetRequest(true);
+
+                        verifySuccess();
+                        // parent category should be checked.
+                        expect(inlineView.$('.check-discussion-category:checked').length).toBe(1);
+                    });
+
+                    it('shows an appropriate message when no inline topics exist', function() {
+
+                        var topicsJson, discussionTopicsSettingsModel;
+
+                        topicsJson = {
+                            course_wide_discussions: {
+                                children: ['Topic_C_1'],
+                                entries: {
+                                    Topic_C_1: {
+                                       sort_key: null,
+                                       is_cohorted: true,
+                                       id: 'Topic_C_1'
+                                    }
+                                }
+                            },
+                            inline_discussions: {
+                                subcategories: {},
+                                children: []
+                            }
+                        };
+                        discussionTopicsSettingsModel = new DiscussionTopicsSettingsModel(topicsJson);
+
+                        createInlineView(this, discussionTopicsSettingsModel);
+
+                        var expectedTitle = "No content-specific discussion topics exist.";
+                        expect(inlineView.$('.no-topics').text().trim()).toBe(expectedTitle);
+                    });
+
+                    it('shows an appropriate message when subsequent "GET" returns HTTP500', function() {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        // Save the updated settings
+                        inlineView.$('.action-save').click();
+
+                        mockPatchRequest(['Inline_Discussion_1']);
+
+                        // fake request for discussion/topics with GET method.
+                        AjaxHelpers.expectJsonRequest(
+                            requests, 'GET', '/mock_service/cohorts/discussion/topics'
+                        );
+                        AjaxHelpers.respondWithError(requests, 500);
+
+                        var expectedTitle = "We've encountered an error. Refresh your browser and then try again.";
+                        expect(inlineView.$('.message-title').text().trim()).toBe(expectedTitle);
+                    });
+
+                    it('shows an appropriate error message for HTTP500', function () {
+                        createInlineView(this);
+                        enableSaveButton();
+
+                        $(inlineView.$('.check-discussion-subcategory-inline')[1]).prop('checked', 'checked').change();
+                        inlineView.$('.action-save').click();
+
+                        AjaxHelpers.respondWithError(requests, 500);
+                        var expectedTitle = "We've encountered an error. Refresh your browser and then try again.";
+                        expect(inlineView.$('.message-title').text().trim()).toBe(expectedTitle);
+                    });
+
+                });
+
+            });
         });
     });

lms/static/js/spec/main.js

@@ -60,6 +60,7 @@
             'history': 'js/vendor/history',
             'js/verify_student/photocapture': 'js/verify_student/photocapture',
             'js/staff_debug_actions': 'js/staff_debug_actions',
+            'js/vendor/jquery.qubit': 'js/vendor/jquery.qubit',
 
             // Backbone classes loaded explicitly until they are converted to use RequireJS
             'js/models/notification': 'js/models/notification',
@@ -67,6 +68,12 @@
             'js/views/notification': 'js/views/notification',
             'js/groups/models/cohort': 'js/groups/models/cohort',
             'js/groups/models/content_group': 'js/groups/models/content_group',
+            'js/groups/models/course_cohort_settings': 'js/groups/models/course_cohort_settings',
+            'js/groups/models/cohort_discussions': 'js/groups/models/cohort_discussions',
+            'js/groups/views/cohort_discussions': 'js/groups/views/cohort_discussions',
+            'js/groups/views/cohort_discussions_course_wide': 'js/groups/views/cohort_discussions_course_wide',
+            'js/groups/views/cohort_discussions_inline': 'js/groups/views/cohort_discussions_inline',
+            'js/groups/views/course_cohort_settings_notification': 'js/groups/views/course_cohort_settings_notification',
             'js/groups/collections/cohort': 'js/groups/collections/cohort',
             'js/groups/views/cohort_editor': 'js/groups/views/cohort_editor',
             'js/groups/views/cohort_form': 'js/groups/views/cohort_form',
@@ -294,6 +301,30 @@
                 exports: 'edx.groups.ContentGroupModel',
                 deps: ['backbone']
             },
+            'js/groups/models/course_cohort_settings': {
+                exports: 'edx.groups.CourseCohortSettingsModel',
+                deps: ['backbone']
+            },
+            'js/groups/models/cohort_discussions': {
+                exports: 'edx.groups.DiscussionTopicsSettingsModel',
+                deps: ['backbone']
+            },
+            'js/groups/views/cohort_discussions': {
+                exports: 'edx.groups.CohortDiscussionConfigurationView',
+                deps: ['backbone']
+            },
+            'js/groups/views/cohort_discussions_course_wide': {
+                exports: 'edx.groups.CourseWideDiscussionsView',
+                deps: ['backbone', 'js/groups/views/cohort_discussions']
+            },
+            'js/groups/views/cohort_discussions_inline': {
+                exports: 'edx.groups.InlineDiscussionsView',
+                deps: ['backbone', 'js/groups/views/cohort_discussions', 'js/vendor/jquery.qubit']
+            },
+            'js/groups/views/course_cohort_settings_notification': {
+                exports: 'edx.groups.CourseCohortSettingsNotificationView',
+                deps: ['backbone']
+            },
             'js/groups/collections/cohort': {
                 exports: 'edx.groups.CohortCollection',
                 deps: ['backbone', 'js/groups/models/cohort']

lms/static/js/utils/animation.js

@@ -0,0 +1,15 @@
+(function() {
+    this.AnimationUtil = (function() {
+        function AnimationUtil() {}
+        AnimationUtil.triggerAnimation = function(messageElement) {
+            // The following lines are necessary to re-trigger the CSS animation on span.action-toggle-message
+            // To see how it works, please see `Another JavaScript Method to Restart a CSS Animation`
+            // at https://css-tricks.com/restart-css-animation/
+            messageElement.removeClass('is-fleeting');
+            messageElement.offset().width = messageElement.offset().width;
+            messageElement.addClass('is-fleeting');
+        };
+        return AnimationUtil;
+    }).call(this);
+}).call(this);
+

lms/static/js/vendor/jquery.qubit.js

@@ -0,0 +1,97 @@
+/*
+** Checkboxes TreeView- jQuery
+** https://github.com/aexmachina/jquery-qubit
+**
+** Copyright (c) 2014 Simon Wade
+** The MIT License (MIT)
+** https://github.com/aexmachina/jquery-qubit/blob/master/LICENSE.txt
+**
+*/
+
+(function($) {
+  $.fn.qubit = function(options) {
+    return this.each(function() {
+      var qubit = new Qubit(this, options);
+    });
+  };
+  var Qubit = function(el) {
+    var self = this;
+    this.scope = $(el);
+    this.scope.on('change', 'input[type=checkbox]', function(e) {
+      if (!self.suspendListeners) {
+        self.process(e.target);
+      }
+    });
+    this.scope.find('input[type=checkbox]:checked').each(function() {
+      self.process(this);
+    });
+  };
+  Qubit.prototype = {
+    itemSelector: 'li',
+    process: function(checkbox) {
+      var checkbox = $(checkbox),
+          parentItems = checkbox.parentsUntil(this.scope, this.itemSelector);
+      try {
+        this.suspendListeners = true;
+        // all children inherit my state
+        parentItems.eq(0).find('input[type=checkbox]')
+          .filter(checkbox.prop('checked') ? ':not(:checked)' : ':checked')
+          .each(function() {
+            if (!$(this).parent().hasClass('hidden')) {
+              $(this).prop('checked', checkbox.prop('checked'));
+            }
+          })
+          .trigger('change');
+        this.processParents(checkbox);
+      } finally {
+        this.suspendListeners = false;
+      }
+    },
+    processParents: function() {
+      var self = this, changed = false;
+      this.scope.find('input[type=checkbox]').each(function() {
+        var $this = $(this),
+            parent = $this.closest(self.itemSelector),
+            children = parent.find('input[type=checkbox]').not($this),
+            numChecked = children.filter(function() {
+              return $(this).prop('checked') || $(this).prop('indeterminate');
+            }).length;
+
+        if (children.length) {
+          if (numChecked == 0) {
+            if (self.setChecked($this, false)) changed = true;
+          } else if (numChecked == children.length) {
+            if (self.setChecked($this, true)) changed = true;
+          } else {
+            if (self.setIndeterminate($this, true)) changed = true;
+          }
+        }
+        else {
+          if (self.setIndeterminate($this, false)) changed = true;
+        }
+      });
+      if (changed) this.processParents();
+    },
+    setChecked: function(checkbox, value, event) {
+      var changed = false;
+      if (checkbox.prop('indeterminate')) {
+        checkbox.prop('indeterminate', false);
+        changed = true;
+      }
+      if (checkbox.prop('checked') != value) {
+        checkbox.prop('checked', value).trigger('change');
+        changed = true;
+      }
+      return changed;
+    },
+    setIndeterminate: function(checkbox, value) {
+      if (value) {
+        checkbox.prop('checked', false);
+      }
+      if (checkbox.prop('indeterminate') != value) {
+        checkbox.prop('indeterminate', value);
+        return true;
+      }
+    }
+  };
+}(jQuery));

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

@@ -435,306 +435,6 @@
     }
   }
 
-  // cohort management
-  %cohort-management-form {
-
-    .form-fields {
-
-      .label,
-      .form-label,
-      .input,
-      .tip {
-        display: block;
-      }
-
-      .label,
-      .form-label {
-        @extend %t-title7;
-        @extend %t-weight4;
-        margin-bottom: ($baseline/2);
-      }
-
-      .tip {
-        @extend %t-copy-sub1;
-        margin-top: ($baseline/4);
-        color: $gray-l2;
-      }
-
-      .field-text {
-
-        // needed to reset poor input styling
-        input[type="text"] {
-          height: auto;
-        }
-
-        .input {
-          width: 100%;
-          padding: ($baseline/2) ($baseline*0.75);
-        }
-      }
-
-      .input-file {
-        margin-bottom: ($baseline/2);
-      }
-    }
-
-    .form-submit, .form-cancel {
-      display: inline-block;
-      vertical-align: middle;
-    }
-
-    .form-submit {
-      @include idashbutton($blue);
-      @include font-size(14);
-      @include line-height(14);
-      margin-right: ($baseline/2);
-      margin-bottom: 0;
-      text-shadow: none;
-    }
-
-    .form-cancel {
-      @extend %t-copy-sub1;
-    }
-  }
-
-  .cohort-management-nav {
-    @include clearfix();
-
-    .cohort-management-nav-form {
-      width: 60%;
-      @include float(left);
-    }
-
-    .cohort-select {
-      width: 100%;
-      margin-top: ($baseline/4);
-    }
-
-    .action-create {
-      @include idashbutton($blue);
-      @extend %t-weight4;
-      @include float(right);
-      @include text-align(right);
-      text-shadow: none;
-    }
-
-    // STATE: is disabled
-    &.is-disabled {
-
-
-      .cohort-select {
-        opacity: 0.25;
-      }
-
-      .action-create {
-        opacity: 0.50;
-      }
-    }
-  }
-
-  .cohort-management {
-
-    // specific message actions
-    .message .action-create {
-      @include idashbutton($blue);
-    }
-  }
-
-  // create or edit cohort
-  .cohort-management-settings,
-  .cohort-management-edit {
-    @extend %cohort-management-form;
-    margin-bottom: $baseline;
-
-    .form-title {
-      @extend %t-title5;
-      @extend %t-weight4;
-      padding: $baseline;
-      background: $gray-l5;
-      border-bottom: ($baseline/10) solid $gray-l4;
-    }
-
-    .form-field {
-      // padding: $baseline;
-    }
-
-    .form-actions {
-      padding: $baseline 0;
-
-      &.new-cohort-form {
-        padding: $baseline;
-      }
-    }
-  }
-
-  .cohort-management-assignment-type-settings {
-     &.is-disabled {
-        opacity: 0.50;
-      }
-  }
-
-  // cohort
-  .cohort-management-group-header {
-    padding: $baseline;
-    border-bottom: ($baseline/10) solid $gray-l4;
-    background: $gray-l5;
-
-    .group-header-title {
-      margin-bottom: ($baseline/2);
-      padding-bottom: ($baseline/2);
-      border-bottom: 1px solid $gray-l4;
-
-      &:hover, &:active, &:focus {
-
-        .action-edit-name {
-          opacity: 1.0;
-          pointer-events: auto;
-        }
-      }
-    }
-
-    .title-value, .group-count, .action-edit {
-      display: inline-block;
-      vertical-align: middle;
-    }
-
-    .title-value {
-      @extend %t-title5;
-      @extend %t-weight4;
-      @include margin-right($baseline/4);
-    }
-
-    .group-count {
-      @extend %t-title7;
-      @extend %t-weight4;
-    }
-
-    .action-edit-name {
-      @include idashbutton($gray-l3);
-      @include transition(opacity $tmg-f2 ease-in-out);
-      @include font-size(14);
-      @include line-height(14);
-      @include margin-left($baseline/2);
-      padding: ($baseline/4) ($baseline/2);
-      margin-bottom: 0;
-      opacity: 0.0;
-      pointer-events: none;
-    }
-  }
-
-  .cohort-management-group-setup {
-    @include clearfix();
-    @extend %t-copy-sub1;
-    color: $gray-l2;
-
-    .setup-value {
-      @include float(left);
-      @include margin-right(5%);
-      width: 70%;
-    }
-
-    .setup-actions {
-      @include float(right);
-      @include text-align(right);
-      width: 20%;
-    }
-  }
-
-  .cohort-management-group-add {
-    @extend %cohort-management-form;
-    border: 1px solid $gray-l5;
-
-    .message-title {
-      @extend %t-title7;
-    }
-
-    .form-title {
-      @extend %t-title6;
-      @extend %t-weight4;
-      margin-bottom: ($baseline/4);
-      padding: 0;
-      border: none;
-      background: transparent;
-    }
-
-    .form-introduction {
-      @extend %t-copy-sub1;
-      margin-bottom: $baseline;
-
-      p {
-        color: $gray-l1;
-      }
-    }
-
-    .form-fields {
-      padding: $baseline 0;
-    }
-
-    .form-actions {
-      padding: 0 0 $baseline 0;
-    }
-
-    .cohort-management-group-add-students {
-      min-height: ($baseline*10);
-      width: 100%;
-      padding: ($baseline/2) ($baseline*0.75);
-    }
-  }
-
-  // CSV-based file upload for auto cohort assigning
-  .toggle-cohort-management-secondary {
-    @extend %t-copy-sub1;
-  }
-
-  .cohort-management-file-upload {
-
-    .message-title {
-      @extend %t-title7;
-    }
-
-    .form-introduction {
-      @extend %t-copy-sub1;
-      margin-bottom: $baseline;
-
-      p {
-        color: $gray-l1;
-      }
-    }
-  }
-
-  .file-upload-form {
-    @extend %cohort-management-form;
-
-    .form-fields {
-      margin-bottom: $baseline;
-    }
-
-    .action-submit {
-      @include idashbutton($blue);
-      // needed to override very poor specificity and base rules for blue button
-      @include font-size(14);
-      margin-bottom: 0;
-      font-weight: 700;
-      text-shadow: none;
-    }
-  }
-
-  .cohort-management-supplemental {
-    @extend %t-copy-sub1;
-    margin-top: $baseline;
-    padding: ($baseline/2) $baseline;
-    background: $gray-l5;
-    border-radius: ($baseline/10);
-
-
-    .icon {
-      @include margin-right($baseline/4);
-      color: $gray-l1;
-    }
-  }
-
-
-
   .batch-enrollment, .batch-beta-testers {
     textarea {
       margin-top: 0.2em;
@@ -850,7 +550,6 @@
     display: block;
   }
 
-
   label[for="email-students"]:hover + .email-students-hint {
     display: block;
   }
@@ -971,6 +670,311 @@
       }
     }
   }
+}
+
+
+// view - cohort management
+// --------------------
+.instructor-dashboard-wrapper-2 section.idash-section#cohort_management {
+
+  // cohort management
+  %cohort-management-form {
+
+    .form-fields {
+
+      .label,
+      .form-label,
+      .input,
+      .tip {
+        display: block;
+      }
+
+      .label,
+      .form-label {
+        @extend %t-title7;
+        @extend %t-weight4;
+        margin-bottom: ($baseline/2);
+      }
+
+      .tip {
+        @extend %t-copy-sub1;
+        margin-top: ($baseline/4);
+        color: $gray-l2;
+      }
+
+      .field-text {
+
+        // needed to reset poor input styling
+        input[type="text"] {
+          height: auto;
+        }
+
+        .input {
+          width: 100%;
+          padding: ($baseline/2) ($baseline*0.75);
+        }
+      }
+
+      .input-file {
+        margin-bottom: ($baseline/2);
+      }
+    }
+
+    .form-submit, .form-cancel {
+      display: inline-block;
+      vertical-align: middle;
+    }
+
+    .form-cancel {
+      @extend %t-copy-sub1;
+    }
+  }
+  .form-submit {
+    @include idashbutton($blue);
+    @include font-size(14);
+    @include line-height(14);
+    margin-right: ($baseline/2);
+    margin-bottom: 0;
+    text-shadow: none;
+  }
+
+  .cohort-management-nav {
+    @include clearfix();
+
+    .cohort-management-nav-form {
+      width: 60%;
+      @include float(left);
+    }
+
+    .cohort-select {
+      width: 100%;
+      margin-top: ($baseline/4);
+    }
+
+    .action-create {
+      @include idashbutton($blue);
+      @extend %t-weight4;
+      @include float(right);
+      @include text-align(right);
+      text-shadow: none;
+    }
+
+    // STATE: is disabled
+    &.is-disabled {
+
+
+      .cohort-select {
+        opacity: 0.25;
+      }
+
+      .action-create {
+        opacity: 0.50;
+      }
+    }
+  }
+
+  .cohort-management {
+
+    // specific message actions
+    .message .action-create {
+      @include idashbutton($blue);
+    }
+  }
+
+  // create or edit cohort
+  .cohort-management-settings,
+  .cohort-management-edit {
+    @extend %cohort-management-form;
+    margin-bottom: $baseline;
+
+    .form-title {
+      @extend %t-title5;
+      @extend %t-weight4;
+      padding: $baseline;
+      background: $gray-l5;
+      border-bottom: ($baseline/10) solid $gray-l4;
+    }
+
+    .form-field {
+      // padding: $baseline;
+    }
+
+    .form-actions {
+      padding: $baseline 0;
+
+      &.new-cohort-form {
+        padding: $baseline;
+      }
+    }
+  }
+
+  .cohort-management-assignment-type-settings,
+  .cohorts-state-section {
+     &.is-disabled {
+        opacity: 0.25;
+      }
+  }
+
+  // cohort
+  .cohort-management-group-header {
+    padding: $baseline;
+    border-bottom: ($baseline/10) solid $gray-l4;
+    background: $gray-l5;
+
+    .group-header-title {
+      margin-bottom: ($baseline/2);
+      padding-bottom: ($baseline/2);
+      border-bottom: 1px solid $gray-l4;
+
+      &:hover, &:active, &:focus {
+
+        .action-edit-name {
+          opacity: 1.0;
+          pointer-events: auto;
+        }
+      }
+    }
+
+    .title-value, .group-count, .action-edit {
+      display: inline-block;
+      vertical-align: middle;
+    }
+
+    .title-value {
+      @extend %t-title5;
+      @extend %t-weight4;
+      @include margin-right($baseline/4);
+    }
+
+    .group-count {
+      @extend %t-title7;
+      @extend %t-weight4;
+    }
+
+    .action-edit-name {
+      @include idashbutton($gray-l3);
+      @include transition(opacity $tmg-f2 ease-in-out);
+      @include font-size(14);
+      @include line-height(14);
+      @include margin-left($baseline/2);
+      padding: ($baseline/4) ($baseline/2);
+      margin-bottom: 0;
+      opacity: 0.0;
+      pointer-events: none;
+    }
+  }
+
+  .cohort-management-group-setup {
+    @include clearfix();
+    @extend %t-copy-sub1;
+    color: $gray-l2;
+
+    .setup-value {
+      @include float(left);
+      @include margin-right(5%);
+      width: 70%;
+    }
+
+    .setup-actions {
+      @include float(right);
+      @include text-align(right);
+      width: 20%;
+    }
+  }
+
+  .cohort-management-group-add {
+    @extend %cohort-management-form;
+    border: 1px solid $gray-l5;
+
+    .message-title {
+      @extend %t-title7;
+    }
+
+    .form-title {
+      @extend %t-title6;
+      @extend %t-weight4;
+      margin-bottom: ($baseline/4);
+      padding: 0;
+      border: none;
+      background: transparent;
+    }
+
+    .form-introduction {
+      @extend %t-copy-sub1;
+      margin-bottom: $baseline;
+
+      p {
+        color: $gray-l1;
+      }
+    }
+
+    .form-fields {
+      padding: $baseline 0;
+    }
+
+    .form-actions {
+      padding: 0 0 $baseline 0;
+    }
+
+    .cohort-management-group-add-students {
+      min-height: ($baseline*10);
+      width: 100%;
+      padding: ($baseline/2) ($baseline*0.75);
+    }
+  }
+
+  // CSV-based file upload for auto cohort assigning and
+  // cohort the discussion topics.
+  .toggle-cohort-management-secondary, .toggle-cohort-management-discussions {
+    @extend %t-copy-sub1;
+  }
+
+  .cohort-management-file-upload {
+
+    .message-title {
+      @extend %t-title7;
+    }
+
+    .form-introduction {
+      @extend %t-copy-sub1;
+      margin-bottom: $baseline;
+
+      p {
+        color: $gray-l1;
+      }
+    }
+  }
+
+  .file-upload-form {
+    @extend %cohort-management-form;
+
+    .form-fields {
+      margin-bottom: $baseline;
+    }
+
+    .action-submit {
+      @include idashbutton($blue);
+      // needed to override very poor specificity and base rules for blue button
+      @include font-size(14);
+      margin-bottom: 0;
+      font-weight: 700;
+      text-shadow: none;
+    }
+  }
+
+  .cohort-management-supplemental {
+    @extend %t-copy-sub1;
+    margin-top: $baseline;
+    padding: ($baseline/2) $baseline;
+    background: $gray-l5;
+    border-radius: ($baseline/10);
+
+
+    .icon {
+      @include margin-right($baseline/4);
+      color: $gray-l1;
+    }
+  }
 
   .has-other-input-text { // Given to groups which have an 'other' input that appears when needed
     display: inline-block;
@@ -1067,6 +1071,49 @@
     }
   }
 
+  // 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: $link-color;
+    }
+
+    .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;
@@ -1118,6 +1165,7 @@
   }
 }
 
+
 // view - student admin
 // --------------------
 .instructor-dashboard-wrapper-2 section.idash-section#student_admin > {

lms/templates/courseware/legacy_instructor_dashboard.html

@@ -372,7 +372,7 @@ function goto( mode)
 
 %if modeflag.get('Manage Groups'):
   %if instructor_access:
-    %if course.is_cohorted:
+    %if course_is_cohorted:
       <p class="is-deprecated">${_("To manage beta tester roles and cohorts, visit the Membership section of the Instructor Dashboard.")}</p>
     %else:
       <p class="is-deprecated">${_("To manage beta tester roles, visit the Membership section of the Instructor Dashboard.")}</p>

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

@@ -0,0 +1,10 @@
+<li class="discussion-category-item">
+    <div class="category-item">
+    <label>
+        <input type="checkbox" class="check-discussion-category"  <%- isCategoryCohorted ? 'checked="checked"' : '' %>/>
+        <span class="category-title"><%- name %></span>
+    </label>
+    </div>
+
+    <ul class="wrapper-tabs subcategories"><%= entries %></ul>
+</li>

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

@@ -0,0 +1,19 @@
+<h3 class="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="form-fields">
+            <div class="form-field">
+                <div class="course-wide-discussion-topics">
+                    <h3 class="subsection-title"><%- gettext('Course-Wide Discussion Topics') %></h3>
+                    <p><%- gettext('Select the course-wide discussion topics that you want to divide by cohort.') %></p>
+                    <div class="field">
+                        <ul class="discussions-wrapper"><%= courseWideTopics %></ul>
+                    </div>
+                </div>
+            </div>
+        </div>
+        <div class="form-actions">
+            <button class="form-submit button action-primary action-save"><%- gettext('Save') %></button>
+        </div>
+    </div>
+</form>

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

@@ -0,0 +1,38 @@
+<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="form-fields">
+            <div class="form-field">
+                <div class="inline-discussion-topics">
+                    <h3 class="subsection-title"><%- gettext('Content-Specific Discussion Topics') %></h3>
+                    <p><%- gettext('Specify whether content-specific discussion topics are divided by cohort.') %></p>
+                    <div class="always_cohort_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>
+                        </label>
+                    </div>
+                    <div class="cohort_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>
+                        </label>
+                    </div>
+                    <hr class="divider divider-lv1" />
+                    <div class="field">
+                        <% if ( inlineDiscussionTopics ) { %>
+                           <ul class="inline-topics discussions-wrapper"><%= inlineDiscussionTopics %></ul>
+                        <% } else {  %>
+                           <span class="no-topics"><%- gettext('No content-specific discussion topics exist.') %></span>
+                        <% } %>
+                    </div>
+                </div>
+            </div>
+        </div>
+        <hr class="divider divider-lv1" />
+        <div class="form-actions">
+            <button class="form-submit button action-primary action-save"><%- gettext('Save') %></button>
+        </div>
+    </div>
+</form>

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

@@ -0,0 +1,9 @@
+<li class="discussion-subcategory-item">
+    <div class="subcategory-item">
+        <label>
+            <input data-id="<%- id %>" class="check-discussion-subcategory-<%- type %>" type="checkbox" <%- is_cohorted ? 'checked="checked"' : '' %> />
+            <span class="topic-name"><%- name %></span>
+            <span class="cohorted-text <%- is_cohorted ? '' : 'hidden'%>">- <%- gettext('Cohorted') %></span>
+        </label>
+    </div>
+</li>

lms/templates/instructor/instructor_dashboard_2/cohort-form.underscore

@@ -35,19 +35,19 @@
                     <div class="cohort-management-assignment-type-settings field field-radio">
                 <% } %>
                     <h4 class="form-label">
-                        <%- gettext('Students in this cohort are:') %>
+                        <%- gettext('Cohort Assignment Method') %>
                     </h4>
                     <label>
-                        <input type="radio" class="type-random" name="cohort-assignment-type" value="random"  <%- assignment_type == 'random' ? 'checked="checked"' : '' %>/> <%- gettext("Randomly Assigned") %>
+                        <input type="radio" class="type-random" name="cohort-assignment-type" value="random"  <%- assignment_type == 'random' ? 'checked="checked"' : '' %>/> <%- gettext("Automatic") %>
                     </label>
                     <label>
-                        <input type="radio" class="type-manual" name="cohort-assignment-type" value="manual"  <%- assignment_type == 'manual' || isNewCohort ? 'checked="checked"' : '' %>/> <%- gettext("Manually Assigned") %>
+                        <input type="radio" class="type-manual" name="cohort-assignment-type" value="manual"  <%- assignment_type == 'manual' || isNewCohort ? 'checked="checked"' : '' %>/> <%- gettext("Manual") %>
                     </label>
                 </div>
                 <% if (isDefaultCohort) { %>
                     <p class="copy-error">
                         <i class="icon fa fa-exclamation-triangle" aria-hidden="true"></i>
-                        <%- gettext("There must be one cohort to which students can be randomly assigned.") %>
+                        <%- gettext("There must be one cohort to which students can automatically be assigned.") %>
                     </p>
                 <% } %>
                 <hr class="divider divider-lv1">

lms/templates/instructor/instructor_dashboard_2/cohort-group-header.underscore

@@ -18,9 +18,4 @@
             <a href="http://edx.readthedocs.org/projects/edx-partner-course-staff/en/latest/cohorts/cohorts_overview.html#all-automated-assignment" class="incontext-help action-secondary action-help"><%- gettext("What does this mean?") %></a>
         <% } %>
     </div>
-    <div class="setup-actions">
-        <% if (studioAdvancedSettingsUrl !== "None") { %>
-            <a href="<%= studioAdvancedSettingsUrl %>" class="action-secondary action-edit"><%- gettext("Edit settings in Studio") %></a>
-        <% } %>
-    </div>
-</div>
+</div>

lms/templates/instructor/instructor_dashboard_2/cohort-state.underscore

@@ -0,0 +1,4 @@
+<div class="nav-utilities">
+    <span class="action-toggle-message" aria-live="polite"></span>
+</div>
+

lms/templates/instructor/instructor_dashboard_2/cohort_management.html

@@ -0,0 +1,45 @@
+<%! from django.utils.translation import ugettext as _ %>
+<%page args="section_data"/>
+<%! from courseware.courses import get_studio_url %>
+<%! from microsite_configuration import microsite %>
+<%! from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition %>
+
+
+<div class="cohort-management"
+     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']}"
+>
+</div>
+
+
+<%block name="headextra">
+    <%
+    cohorted_user_partition = get_cohorted_user_partition(course.id)
+    content_groups = cohorted_user_partition.groups if cohorted_user_partition else []
+    %>
+    <script type="text/javascript">
+        $(document).ready(function() {
+
+            var cohortUserPartitionId = ${cohorted_user_partition.id if cohorted_user_partition else 'null'},
+                contentGroups = [
+                    % for content_group in content_groups:
+                        new edx.groups.ContentGroupModel({
+                            id: ${content_group.id},
+                            name: "${content_group.name | h}",
+                            user_partition_id: cohortUserPartitionId
+                        }),
+                    % endfor
+                ];
+
+            (function (require) {
+                require(['js/groups/views/cohorts_dashboard_factory'], function (CohortsFactory) {
+                    CohortsFactory(contentGroups, '${get_studio_url(course, 'group_configurations') | h}');
+                });
+            }).call(this, require || RequireJS.require);
+        });
+    </script>
+
+</%block>
+<div class="cohort-state-message"></div>

lms/templates/instructor/instructor_dashboard_2/cohorts.underscore

@@ -1,50 +1,61 @@
-<h2 class="section-title">
-    <span class="value"><%- gettext('Cohort Management') %></span>
-    <span class="description"></span>
-</h2>
-
-<div class="cohort-management-nav">
-    <h3 class="subsection-title"><%- gettext('Assign students to cohorts manually') %></h3>
-    <form action="" method="post" name="" id="cohort-management-nav-form" class="cohort-management-nav-form">
-
-        <div class="cohort-management-nav-form-select field field-select">
-            <label for="cohort-select" class="label sr"><%- gettext("Select a cohort group to manage") %></label>
-            <select class="input cohort-select" name="cohort-select" id="cohort-select"></select>
-        </div>
-
-        <div class="form-actions">
-            <button class="form-submit button action-primary action-view sr"><%- gettext('View Cohort') %></button>
-        </div>
-    </form>
-
-    <a href="" class="action-primary action-create">
-        <i class="icon fa fa-plus" aria-hidden="true"></i>
-        <%- gettext('Add Cohort') %>
-    </a>
+<div class="cohorts-state-section">
+    <label> <input type="checkbox" class="cohorts-state" value="Cohorts-State" <%- cohortsEnabled ? 'checked="checked"' : '' %> /> <%- gettext('Enable Cohorts') %></label>
 </div>
 
-<!-- Add modal -->
-<div class="cohort-management-add-form"></div>
+<% if (cohortsEnabled) { %>
+    <div class="cohort-management-nav">
+        <hr class="divider divider-lv1" />
+        <form action="" method="post" name="" id="cohort-management-nav-form" class="cohort-management-nav-form">
 
-<!-- individual group -->
-<div class="cohort-management-group"></div>
+            <div class="cohort-management-nav-form-select field field-select">
+                <label for="cohort-select" class="label sr"><%- gettext("Select a cohort to manage") %></label>
+                <select class="input cohort-select" name="cohort-select" id="cohort-select"></select>
+            </div>
 
-<div class="wrapper-cohort-supplemental">
+            <div class="form-actions">
+                <button class="form-submit button action-primary action-view sr"><%- gettext('View Cohort') %></button>
+            </div>
+        </form>
 
-    <hr class="divider divider-lv1" />
-
-    <!-- Uploading a CSV file of cohort assignments. -->
-    <a class="toggle-cohort-management-secondary" href="#cohort-management-file-upload"><%- gettext('Assign students to cohorts by uploading a CSV file') %></a>
-    <div class="cohort-management-file-upload csv-upload is-hidden" id="cohort-management-file-upload"></div>
-
-    <div class="cohort-management-supplemental">
-        <p class="">
-            <i class="icon fa fa-info-circle" aria-hidden="true"></i>
-            <%= interpolate(
-            gettext('To review student cohort assignments or see the results of uploading a CSV file, download course profile information or cohort results on %(link_start)s the Data Download page. %(link_end)s'),
-                {link_start: '<a href="" class="link-cross-reference" data-section="data_download">', link_end: '</a>'},
-                true
-            ) %>
-        </p>
+        <button class="button action-primary action-create">
+            <i class="icon fa fa-plus" aria-hidden="true"></i>
+            <%- gettext('Add Cohort') %>
+        </button>
     </div>
-</div>
+
+    <!-- Add modal -->
+    <div class="cohort-management-add-form"></div>
+
+    <!-- individual group -->
+    <div class="cohort-management-group"></div>
+
+    <div class="wrapper-cohort-supplemental">
+
+        <hr class="divider divider-lv1" />
+
+        <!-- Uploading a CSV file of cohort assignments. -->
+        <a class="toggle-cohort-management-secondary" href="#cohort-management-file-upload"><%- gettext('Assign students to cohorts by uploading a CSV file') %></a>
+        <div class="cohort-management-file-upload csv-upload is-hidden" id="cohort-management-file-upload"></div>
+
+        <div class="cohort-management-supplemental">
+            <p class="">
+                <i class="icon fa fa-info-circle" aria-hidden="true"></i>
+                <%= interpolate(
+                gettext('To review student cohort assignments or see the results of uploading a CSV file, download course profile information or cohort results on %(link_start)s the Data Download page. %(link_end)s'),
+                    {link_start: '<a href="" class="link-cross-reference" data-section="data_download">', link_end: '</a>'},
+                    true
+                ) %>
+            </p>
+        </div>
+
+        <hr class="divider divider-lv1" />
+
+        <!-- Discussion Topics. -->
+        <a class="toggle-cohort-management-discussions" href="#cohort-discussions-management"><%- gettext('Specify whether discussion topics are divided by cohort') %></a>
+        <div class="cohort-discussions-nav is-hidden" id="cohort-management-discussion-topics">
+            <div class="cohort-course-wide-discussions-nav"></div>
+            <div class="cohort-inline-discussions-nav"></div>
+        </div>
+
+    </div>
+<% } %>

lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html

@@ -48,25 +48,36 @@
   <script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/tinymce.full.min.js')}"></script>
   <script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js')}"></script>
   <script type="text/javascript" src="${static.url('js/vendor/jQuery-File-Upload/js/jquery.fileupload.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/vendor/jquery.qubit.js')}"></script>
+
   <%static:js group='module-descriptor-js'/>
   <%static:js group='instructor_dash'/>
   <%static:js group='application'/>
 
   ## Backbone classes declared explicitly until RequireJS is supported
+  <script type="text/javascript" src="${static.url('js/instructor_dashboard/ecommerce.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/instructor_dashboard/cohort_management.js')}"></script>
   <script type="text/javascript" src="${static.url('js/models/notification.js')}"></script>
   <script type="text/javascript" src="${static.url('js/views/notification.js')}"></script>
   <script type="text/javascript" src="${static.url('js/views/file_uploader.js')}"></script>
   <script type="text/javascript" src="${static.url('js/groups/models/cohort.js')}"></script>
   <script type="text/javascript" src="${static.url('js/groups/models/content_group.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/groups/models/course_cohort_settings.js')}"></script>
   <script type="text/javascript" src="${static.url('js/groups/collections/cohort.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/groups/views/course_cohort_settings_notification.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/groups/models/cohort_discussions.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/groups/views/cohort_discussions.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/groups/views/cohort_discussions_course_wide.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/groups/views/cohort_discussions_inline.js')}"></script>
   <script type="text/javascript" src="${static.url('js/groups/views/cohort_form.js')}"></script>
   <script type="text/javascript" src="${static.url('js/groups/views/cohort_editor.js')}"></script>
   <script type="text/javascript" src="${static.url('js/groups/views/cohorts.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/utils/animation.js')}"></script>
 </%block>
 
 ## Include Underscore templates
 <%block name="header_extras">
-% for template_name in ["cohorts", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification"]:
+% for template_name in ["cohorts", "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"]:
 <script type="text/template" id="${template_name}-tpl">
   <%static:include path="instructor/instructor_dashboard_2/${template_name}.underscore" />
 </script>
@@ -126,3 +137,4 @@
     </section>
   </div>
 </section>
+

lms/templates/instructor/instructor_dashboard_2/membership.html

@@ -245,53 +245,3 @@
   %endif
 
 </div>
-
-% if course.is_cohorted:
-    <hr class="divider" />
-    <div class="cohort-management membership-section"
-         data-ajax_url="${section_data['cohorts_ajax_url']}"
-         data-advanced-settings-url="${section_data['advanced_settings_url']}"
-         data-upload_cohorts_csv_url="${section_data['upload_cohorts_csv_url']}"
-    >
-    </div>
-
-    <%block name="headextra">
-        <%
-        cohorted_user_partition = get_cohorted_user_partition(course.id)
-        content_groups = cohorted_user_partition.groups if cohorted_user_partition else []
-        %>
-        <script>
-        $(document).ready(function() {
-            var cohortManagementElement = $('.cohort-management');
-            if (cohortManagementElement.length > 0) {
-                var cohorts = new edx.groups.CohortCollection(),
-                    cohortUserPartitionId = ${cohorted_user_partition.id if cohorted_user_partition else 'null'},
-                    contentGroups = [
-                        % for content_group in content_groups:
-                            new edx.groups.ContentGroupModel({
-                                id: ${content_group.id},
-                                name: "${content_group.name | h}",
-                                user_partition_id: cohortUserPartitionId
-                            }),
-                        % endfor
-                    ];
-                cohorts.url = cohortManagementElement.data('ajax_url');
-                var cohortsView = new edx.groups.CohortsView({
-                    el: cohortManagementElement,
-                    model: cohorts,
-                    contentGroups: contentGroups,
-                    context: {
-                        uploadCohortsCsvUrl: cohortManagementElement.data('upload_cohorts_csv_url'),
-                        studioAdvancedSettingsUrl: cohortManagementElement.data('advanced-settings-url'),
-                        studioGroupConfigurationsUrl: '${get_studio_url(course, 'group_configurations') | h}'
-                    }
-                });
-                cohorts.fetch().done(function() {
-                    cohortsView.render();
-                });
-            }
-        });
-        </script>
-
-    </%block>
-% endif

lms/urls.py

@@ -379,6 +379,9 @@ if settings.COURSEWARE_ENABLED:
             'open_ended_grading.views.take_action_on_flags', name='open_ended_flagged_problems_take_action'),
 
         # Cohorts management
+        url(r'^courses/{}/cohorts/settings$'.format(settings.COURSE_KEY_PATTERN),
+            'openedx.core.djangoapps.course_groups.views.course_cohort_settings_handler',
+            name="course_cohort_settings"),
         url(r'^courses/{}/cohorts/(?P<cohort_id>[0-9]+)?$'.format(settings.COURSE_KEY_PATTERN),
             'openedx.core.djangoapps.course_groups.views.cohort_handler', name="cohorts"),
         url(r'^courses/{}/cohorts/(?P<cohort_id>[0-9]+)$'.format(settings.COURSE_KEY_PATTERN),
@@ -393,6 +396,9 @@ if settings.COURSEWARE_ENABLED:
         url(r'^courses/{}/cohorts/debug$'.format(settings.COURSE_KEY_PATTERN),
             'openedx.core.djangoapps.course_groups.views.debug_cohort_mgmt',
             name="debug_cohort_mgmt"),
+        url(r'^courses/{}/cohorts/topics$'.format(settings.COURSE_KEY_PATTERN),
+            'openedx.core.djangoapps.course_groups.views.cohort_discussion_topics',
+            name='cohort_discussion_topics'),
 
         # Open Ended Notifications
         url(r'^courses/{}/open_ended_notifications$'.format(settings.COURSE_ID_PATTERN),

openedx/core/djangoapps/course_groups/cohorts.py

@@ -5,7 +5,6 @@ forums, and to the cohort admin views.
 
 import logging
 import random
-import json
 
 from django.db import transaction
 from django.db.models.signals import post_save, m2m_changed
@@ -15,7 +14,9 @@ from django.utils.translation import ugettext as _
 
 from courseware import courses
 from eventtracking import tracker
+from request_cache.middleware import RequestCache
 from student.models import get_user_by_username_or_email
+
 from .models import CourseUserGroup, CourseCohort, CourseCohortsSettings, CourseUserGroupPartitionGroup
 
 
@@ -72,12 +73,9 @@ def _cohort_membership_changed(sender, **kwargs):
         tracker.emit(event_name, event)
 
 
-# A 'default cohort' is an auto-cohort that is automatically created for a course if no auto_cohort_groups have been
-# specified. It is intended to be used in a cohorted-course for users who have yet to be assigned to a cohort.
-# Note 1: If an administrator chooses to configure a cohort with the same name, the said cohort will be used as
-#         the "default cohort".
-# Note 2: If auto_cohort_groups are configured after the 'default cohort' has been created and populated, the
-#         stagnant 'default cohort' will still remain (now as a manual cohort) with its previously assigned students.
+# A 'default cohort' is an auto-cohort that is automatically created for a course if no cohort with automatic
+# assignment have been specified. It is intended to be used in a cohorted-course for users who have yet to be assigned
+# to a cohort.
 # Translation Note: We are NOT translating this string since it is the constant identifier for the "default group"
 #                   and needed across product boundaries.
 DEFAULT_COHORT_NAME = "Default Group"
@@ -111,15 +109,15 @@ def is_course_cohorted(course_key):
     Raises:
        Http404 if the course doesn't exist.
     """
-    return courses.get_course_by_id(course_key).is_cohorted
+    return get_course_cohort_settings(course_key).is_cohorted
 
 
-def get_cohort_id(user, course_key):
+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)
+    cohort = get_cohort(user, course_key, use_cached=use_cached)
     return None if cohort is None else cohort.id
 
 
@@ -136,18 +134,19 @@ def is_commentable_cohorted(course_key, commentable_id):
         Http404 if the course doesn't exist.
     """
     course = courses.get_course_by_id(course_key)
+    course_cohort_settings = get_course_cohort_settings(course_key)
 
-    if not course.is_cohorted:
+    if not course_cohort_settings.is_cohorted:
         # this is the easy case :)
         ans = False
     elif (
             commentable_id in course.top_level_discussion_topic_ids or
-            course.always_cohort_inline_discussions is False
+            course_cohort_settings.always_cohort_inline_discussions is False
     ):
         # top level discussions have to be manually configured as cohorted
         # (default is not).
         # Same thing for inline discussions if the default is explicitly set to False in settings
-        ans = commentable_id in course.cohorted_discussions
+        ans = commentable_id in course_cohort_settings.cohorted_discussions
     else:
         # inline discussions are cohorted by default
         ans = True
@@ -163,27 +162,32 @@ def get_cohorted_commentables(course_key):
     Given a course_key return a set of strings representing cohorted commentables.
     """
 
-    course = courses.get_course_by_id(course_key)
+    course_cohort_settings = get_course_cohort_settings(course_key)
 
-    if not course.is_cohorted:
+    if not course_cohort_settings.is_cohorted:
         # this is the easy case :)
         ans = set()
     else:
-        ans = course.cohorted_discussions
+        ans = set(course_cohort_settings.cohorted_discussions)
 
     return ans
 
 
 @transaction.commit_on_success
-def get_cohort(user, course_key, assign=True):
+def get_cohort(user, course_key, assign=True, use_cached=False):
     """
     Given a Django user and a CourseKey, return the user's cohort in that
     cohort.
 
+    The cohort for the user is cached for the duration of a request. Pass
+    use_cached=True to use the cached value instead of fetching from the
+    database.
+
     Arguments:
         user: a Django User object.
         course_key: CourseKey
         assign (bool): if False then we don't assign a group to user
+        use_cached (bool): Whether to use the cached value or fetch from database.
 
     Returns:
         A CourseUserGroup object if the course is cohorted and the User has a
@@ -192,27 +196,37 @@ def get_cohort(user, course_key, assign=True):
     Raises:
        ValueError if the CourseKey doesn't exist.
     """
+    request_cache = RequestCache.get_request_cache()
+    cache_key = u"cohorts.get_cohort.{}.{}".format(user.id, course_key)
+
+    if use_cached and cache_key in request_cache.data:
+        return request_cache.data[cache_key]
+
+    request_cache.data.pop(cache_key, None)
+
     # 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)
-    try:
-        course = courses.get_course_by_id(course_key)
-    except Http404:
-        raise ValueError("Invalid course_key")
-
-    if not course.is_cohorted:
-        return None
+    course_cohort_settings = get_course_cohort_settings(course_key)
+    if not course_cohort_settings.is_cohorted:
+        return request_cache.data.setdefault(cache_key, None)
 
+    # If course is cohorted, check if the user already has a cohort.
     try:
-        return CourseUserGroup.objects.get(
+        cohort = CourseUserGroup.objects.get(
             course_id=course_key,
             group_type=CourseUserGroup.COHORT,
             users__id=user.id,
         )
+        return request_cache.data.setdefault(cache_key, cohort)
     except CourseUserGroup.DoesNotExist:
-        # Didn't find the group.  We'll go on to create one if needed.
+        # Didn't find the group. If we do not want to assign, return here.
         if not assign:
+            # Do not cache the cohort here, because in the next call assign
+            # may be True, and we will have to assign the user a cohort.
             return None
 
+    # Otherwise assign the user a cohort.
+    course = courses.get_course(course_key)
     cohorts = get_course_cohorts(course, assignment_type=CourseCohort.RANDOM)
     if cohorts:
         cohort = local_random().choice(cohorts)
@@ -225,7 +239,7 @@ def get_cohort(user, course_key, assign=True):
 
     user.course_groups.add(cohort)
 
-    return cohort
+    return request_cache.data.setdefault(cache_key, cohort)
 
 
 def migrate_cohort_settings(course):
@@ -233,12 +247,11 @@ def migrate_cohort_settings(course):
     Migrate all the cohort settings associated with this course from modulestore to mysql.
     After that we will never touch modulestore for any cohort related settings.
     """
-    course_id = course.location.course_key
     cohort_settings, created = CourseCohortsSettings.objects.get_or_create(
-        course_id=course_id,
+        course_id=course.id,
         defaults={
             'is_cohorted': course.is_cohorted,
-            'cohorted_discussions': json.dumps(list(course.cohorted_discussions)),
+            'cohorted_discussions': list(course.cohorted_discussions),
             'always_cohort_inline_discussions': course.always_cohort_inline_discussions
         }
     )
@@ -247,14 +260,14 @@ def migrate_cohort_settings(course):
     if created:
         # Update the manual cohorts already present in CourseUserGroup
         manual_cohorts = CourseUserGroup.objects.filter(
-            course_id=course_id,
+            course_id=course.id,
             group_type=CourseUserGroup.COHORT
         ).exclude(name__in=course.auto_cohort_groups)
         for cohort in manual_cohorts:
             CourseCohort.create(course_user_group=cohort)
 
         for group_name in course.auto_cohort_groups:
-            CourseCohort.create(cohort_name=group_name, course_id=course_id, assignment_type=CourseCohort.RANDOM)
+            CourseCohort.create(cohort_name=group_name, course_id=course.id, assignment_type=CourseCohort.RANDOM)
 
     return cohort_settings
 
@@ -324,7 +337,8 @@ def add_cohort(course_key, name, assignment_type):
         raise ValueError("Invalid course_key")
 
     cohort = CourseCohort.create(
-        cohort_name=name, course_id=course.id,
+        cohort_name=name,
+        course_id=course.id,
         assignment_type=assignment_type
     ).course_user_group
 
@@ -392,18 +406,33 @@ def add_user_to_cohort(cohort, username_or_email):
     return (user, previous_cohort_name)
 
 
-def get_group_info_for_cohort(cohort):
+def get_group_info_for_cohort(cohort, use_cached=False):
     """
     Get the ids of the group and partition to which this cohort has been linked
     as a tuple of (int, int).
 
     If the cohort has not been linked to any group/partition, both values in the
     tuple will be None.
+
+    The partition group info is cached for the duration of a request. Pass
+    use_cached=True to use the cached value instead of fetching from the
+    database.
     """
-    res = CourseUserGroupPartitionGroup.objects.filter(course_user_group=cohort)
-    if len(res):
-        return res[0].group_id, res[0].partition_id
-    return None, None
+    request_cache = RequestCache.get_request_cache()
+    cache_key = u"cohorts.get_group_info_for_cohort.{}".format(cohort.id)
+
+    if use_cached and cache_key in request_cache.data:
+        return request_cache.data[cache_key]
+
+    request_cache.data.pop(cache_key, None)
+
+    try:
+        partition_group = CourseUserGroupPartitionGroup.objects.get(course_user_group=cohort)
+        return request_cache.data.setdefault(cache_key, (partition_group.group_id, partition_group.partition_id))
+    except CourseUserGroupPartitionGroup.DoesNotExist:
+        pass
+
+    return request_cache.data.setdefault(cache_key, (None, None))
 
 
 def set_assignment_type(user_group, assignment_type):
@@ -413,7 +442,7 @@ def set_assignment_type(user_group, assignment_type):
     course_cohort = user_group.cohort
 
     if is_default_cohort(user_group) and course_cohort.assignment_type != assignment_type:
-        raise ValueError(_("There must be one cohort to which students can be randomly assigned."))
+        raise ValueError(_("There must be one cohort to which students can automatically be assigned."))
 
     course_cohort.assignment_type = assignment_type
     course_cohort.save()
@@ -438,3 +467,51 @@ def is_default_cohort(user_group):
     )
 
     return len(random_cohorts) == 1 and random_cohorts[0].name == user_group.name
+
+
+def set_course_cohort_settings(course_key, **kwargs):
+    """
+    Set cohort settings for a course.
+
+    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.
+
+    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()
+    return course_cohort_settings
+
+
+def get_course_cohort_settings(course_key):
+    """
+    Return cohort settings for a course.
+
+    Arguments:
+        course_key: CourseKey
+
+    Returns:
+        A CourseCohortSettings object.
+
+    Raises:
+        Http404 if course_key is invalid.
+    """
+    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

openedx/core/djangoapps/course_groups/management/commands/tests/test_remove_users_from_multiple_cohorts.py

@@ -36,10 +36,10 @@ class TestMultipleCohortUsers(ModuleStoreTestCase):
         """
         # set two auto_cohort_groups for both courses
         config_course_cohorts(
-            self.course1, [], cohorted=True, auto_cohort_groups=["Course1AutoGroup1", "Course1AutoGroup2"]
+            self.course1, is_cohorted=True, auto_cohorts=["Course1AutoGroup1", "Course1AutoGroup2"]
         )
         config_course_cohorts(
-            self.course2, [], cohorted=True, auto_cohort_groups=["Course2AutoGroup1", "Course2AutoGroup2"]
+            self.course2, is_cohorted=True, auto_cohorts=["Course2AutoGroup1", "Course2AutoGroup2"]
         )
 
         # get the cohorts from the courses, which will cause auto cohorts to be created

openedx/core/djangoapps/course_groups/migrations/0004_auto__del_field_coursecohortssettings_cohorted_discussions__add_field_.py

@@ -0,0 +1,91 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Changed 'CourseCohortsSettings.cohorted_discussions' to 'CourseCohortsSettings._cohorted_discussions' without
+        # changing db column name
+        pass
+
+    def backwards(self, orm):
+        # Changed 'CourseCohortsSettings.cohorted_discussions' to 'CourseCohortsSettings._cohorted_discussions' without
+        # changing db column name
+        pass
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'course_groups.coursecohort': {
+            'Meta': {'object_name': 'CourseCohort'},
+            'assignment_type': ('django.db.models.fields.CharField', [], {'default': "'manual'", 'max_length': '20'}),
+            'course_user_group': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'cohort'", 'unique': 'True', 'to': "orm['course_groups.CourseUserGroup']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        },
+        'course_groups.coursecohortssettings': {
+            'Meta': {'object_name': 'CourseCohortsSettings'},
+            '_cohorted_discussions': ('django.db.models.fields.TextField', [], {'null': 'True', 'db_column': "'cohorted_discussions'", 'blank': 'True'}),
+            'always_cohort_inline_discussions': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'unique': 'True', 'max_length': '255', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_cohorted': ('django.db.models.fields.BooleanField', [], {'default': 'False'})
+        },
+        'course_groups.courseusergroup': {
+            'Meta': {'unique_together': "(('name', 'course_id'),)", 'object_name': 'CourseUserGroup'},
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
+            'group_type': ('django.db.models.fields.CharField', [], {'max_length': '20'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'users': ('django.db.models.fields.related.ManyToManyField', [], {'db_index': 'True', 'related_name': "'course_groups'", 'symmetrical': 'False', 'to': "orm['auth.User']"})
+        },
+        'course_groups.courseusergrouppartitiongroup': {
+            'Meta': {'object_name': 'CourseUserGroupPartitionGroup'},
+            'course_user_group': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['course_groups.CourseUserGroup']", 'unique': 'True'}),
+            'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'group_id': ('django.db.models.fields.IntegerField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'partition_id': ('django.db.models.fields.IntegerField', [], {}),
+            'updated_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'})
+        }
+    }
+
+    complete_apps = ['course_groups']

openedx/core/djangoapps/course_groups/models.py

@@ -1,3 +1,8 @@
+"""
+Django models related to course groups functionality.
+"""
+
+import json
 import logging
 
 from django.contrib.auth.models import User
@@ -81,11 +86,21 @@ class CourseCohortsSettings(models.Model):
         help_text="Which course are these settings associated with?",
     )
 
-    cohorted_discussions = models.TextField(null=True, blank=True)  # JSON list
+    _cohorted_discussions = models.TextField(db_column='cohorted_discussions', null=True, blank=True)  # JSON list
 
     # pylint: disable=invalid-name
     always_cohort_inline_discussions = models.BooleanField(default=True)
 
+    @property
+    def cohorted_discussions(self):
+        """Jsonify the cohorted_discussions"""
+        return json.loads(self._cohorted_discussions)
+
+    @cohorted_discussions.setter
+    def cohorted_discussions(self, value):
+        """Un-Jsonify the cohorted_discussions"""
+        self._cohorted_discussions = json.dumps(value)
+
 
 class CourseCohort(models.Model):
     """

openedx/core/djangoapps/course_groups/partition_scheme.py

@@ -22,7 +22,7 @@ class CohortPartitionScheme(object):
 
     # pylint: disable=unused-argument
     @classmethod
-    def get_group_for_user(cls, course_key, user, user_partition, track_function=None):
+    def get_group_for_user(cls, course_key, user, user_partition, track_function=None, use_cached=True):
         """
         Returns the Group from the specified user partition to which the user
         is assigned, via their cohort membership and any mappings from cohorts
@@ -48,12 +48,12 @@ class CohortPartitionScheme(object):
                     return None
             return None
 
-        cohort = get_cohort(user, course_key)
+        cohort = get_cohort(user, course_key, use_cached=use_cached)
         if cohort is None:
             # student doesn't have a cohort
             return None
 
-        group_id, partition_id = get_group_info_for_cohort(cohort)
+        group_id, partition_id = get_group_info_for_cohort(cohort, use_cached=use_cached)
         if partition_id is None:
             # cohort isn't mapped to any partition group.
             return None

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

@@ -4,11 +4,14 @@ Helper methods for testing cohorts.
 import factory
 from factory import post_generation, Sequence
 from factory.django import DjangoModelFactory
+import json
+
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
 
-from ..models import CourseUserGroup, CourseCohort
+from ..cohorts import set_course_cohort_settings
+from ..models import CourseUserGroup, CourseCohort, CourseCohortsSettings
 
 
 class CohortFactory(DjangoModelFactory):
@@ -40,6 +43,19 @@ class CourseCohortFactory(DjangoModelFactory):
     assignment_type = 'manual'
 
 
+class CourseCohortSettingsFactory(DjangoModelFactory):
+    """
+    Factory for constructing mock course cohort settings.
+    """
+    FACTORY_FOR = CourseCohortsSettings
+
+    is_cohorted = False
+    course_id = SlashSeparatedCourseKey("dummy", "dummy", "dummy")
+    cohorted_discussions = json.dumps([])
+    # pylint: disable=invalid-name
+    always_cohort_inline_discussions = True
+
+
 def topic_name_to_id(course, name):
     """
     Given a discussion topic name, return an id for that name (includes
@@ -52,7 +68,7 @@ def topic_name_to_id(course, name):
     )
 
 
-def config_course_cohorts(
+def config_course_cohorts_legacy(
         course,
         discussions,
         cohorted,
@@ -62,7 +78,11 @@ def config_course_cohorts(
 ):
     """
     Given a course with no discussion set up, add the discussions and set
-    the cohort config appropriately.
+    the cohort config on the course descriptor.
+
+    Since cohort settings are now stored in models.CourseCohortSettings,
+    this is only used for testing data migration from the CourseDescriptor
+    to the table.
 
     Arguments:
         course: CourseDescriptor
@@ -90,7 +110,6 @@ def config_course_cohorts(
     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
 
@@ -104,3 +123,59 @@ def config_course_cohorts(
         modulestore().update_item(course, ModuleStoreEnum.UserID.test)
     except NotImplementedError:
         pass
+
+
+# pylint: disable=dangerous-default-value
+def config_course_cohorts(
+        course,
+        is_cohorted,
+        auto_cohorts=[],
+        manual_cohorts=[],
+        discussion_topics=[],
+        cohorted_discussions=[],
+        always_cohort_inline_discussions=True  # pylint: disable=invalid-name
+):
+    """
+    Set discussions and configure cohorts for a course.
+
+    Arguments:
+        course: CourseDescriptor
+        is_cohorted (bool): Is the course cohorted?
+        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(
+        course.id,
+        is_cohorted=is_cohorted,
+        cohorted_discussions=[to_id(name) for name in cohorted_discussions],
+        always_cohort_inline_discussions=always_cohort_inline_discussions
+    )
+
+    for cohort_name in auto_cohorts:
+        cohort = CohortFactory(course_id=course.id, name=cohort_name)
+        CourseCohortFactory(course_user_group=cohort, assignment_type=CourseCohort.RANDOM)
+
+    for cohort_name in manual_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)
+    except NotImplementedError:
+        pass

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

@@ -2,14 +2,14 @@
 Tests for cohorts
 """
 # pylint: disable=no-member
+import ddt
+from mock import call, patch
 
-from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import IntegrityError
 from django.http import Http404
 from django.test import TestCase
 from django.test.utils import override_settings
-from mock import call, patch
 
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from student.models import CourseEnrollment
@@ -19,8 +19,10 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTO
 
 from ..models import CourseUserGroup, CourseCohort, CourseUserGroupPartitionGroup
 from .. import cohorts
-from ..tests.helpers import topic_name_to_id, config_course_cohorts, CohortFactory, CourseCohortFactory
-
+from ..tests.helpers import (
+    topic_name_to_id, config_course_cohorts, config_course_cohorts_legacy,
+    CohortFactory, CourseCohortFactory, CourseCohortSettingsFactory
+)
 
 @patch("openedx.core.djangoapps.course_groups.cohorts.tracker")
 class TestCohortSignals(TestCase):
@@ -120,6 +122,7 @@ class TestCohortSignals(TestCase):
         self.assertFalse(mock_tracker.emit.called)
 
 
+@ddt.ddt
 class TestCohorts(ModuleStoreTestCase):
     """
     Test the cohorts feature
@@ -146,12 +149,10 @@ class TestCohorts(ModuleStoreTestCase):
         Make sure cohorts.is_course_cohorted() correctly reports if a course is cohorted or not.
         """
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
         self.assertFalse(cohorts.is_course_cohorted(course.id))
 
-        config_course_cohorts(course, [], cohorted=True)
+        config_course_cohorts(course, is_cohorted=True)
 
-        self.assertTrue(course.is_cohorted)
         self.assertTrue(cohorts.is_course_cohorted(course.id))
 
         # Make sure we get a Http404 if there's no course
@@ -164,18 +165,18 @@ class TestCohorts(ModuleStoreTestCase):
         invalid course key.
         """
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         user = UserFactory(username="test", email="a@b.com")
         self.assertIsNone(cohorts.get_cohort_id(user, course.id))
 
-        config_course_cohorts(course, discussions=[], cohorted=True)
+        config_course_cohorts(course, is_cohorted=True)
         cohort = CohortFactory(course_id=course.id, name="TestCohort")
         cohort.users.add(user)
         self.assertEqual(cohorts.get_cohort_id(user, course.id), cohort.id)
 
         self.assertRaises(
-            ValueError,
+            Http404,
             lambda: cohorts.get_cohort_id(user, SlashSeparatedCourseKey("course", "does_not", "exist"))
         )
 
@@ -209,7 +210,7 @@ class TestCohorts(ModuleStoreTestCase):
 
         self.assertEqual(cohorts.get_assignment_type(cohort), CourseCohort.RANDOM)
 
-        exception_msg = "There must be one cohort to which students can be randomly assigned."
+        exception_msg = "There must be one cohort to which students can automatically be assigned."
         with self.assertRaises(ValueError) as context_manager:
             cohorts.set_assignment_type(cohort, CourseCohort.MANUAL)
 
@@ -221,7 +222,7 @@ class TestCohorts(ModuleStoreTestCase):
         """
         course = modulestore().get_course(self.toy_course_key)
         self.assertEqual(course.id, self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         user = UserFactory(username="test", email="a@b.com")
         other_user = UserFactory(username="test2", email="a2@b.com")
@@ -237,35 +238,55 @@ class TestCohorts(ModuleStoreTestCase):
         )
 
         # Make the course cohorted...
-        config_course_cohorts(course, discussions=[], cohorted=True)
+        config_course_cohorts(course, is_cohorted=True)
 
         self.assertEquals(
             cohorts.get_cohort(user, course.id).id,
             cohort.id,
             "user should be assigned to the correct cohort"
         )
+
         self.assertEquals(
             cohorts.get_cohort(other_user, course.id).id,
             cohorts.get_cohort_by_name(course.id, cohorts.DEFAULT_COHORT_NAME).id,
             "other_user should be assigned to the default cohort"
         )
 
+    @ddt.data(
+        (True, 2),
+        (False, 6),
+    )
+    @ddt.unpack
+    def test_get_cohort_sql_queries(self, use_cached, num_sql_queries):
+        """
+        Test number of queries by cohorts.get_cohort() with and without caching.
+        """
+        course = modulestore().get_course(self.toy_course_key)
+        config_course_cohorts(course, is_cohorted=True)
+        cohort = CohortFactory(course_id=course.id, name="TestCohort")
+
+        user = UserFactory(username="test", email="a@b.com")
+        cohort.users.add(user)
+
+        with self.assertNumQueries(num_sql_queries):
+            for __ in range(3):
+                cohorts.get_cohort(user, course.id, use_cached=use_cached)
+
     def test_get_cohort_with_assign(self):
         """
         Make sure cohorts.get_cohort() returns None if no group is already
         assigned to a user instead of assigning/creating a group automatically
         """
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         user = UserFactory(username="test", email="a@b.com")
 
         # Add an auto_cohort_group to the course...
         config_course_cohorts(
             course,
-            discussions=[],
-            cohorted=True,
-            auto_cohort_groups=["AutoGroup"]
+            is_cohorted=True,
+            auto_cohorts=["AutoGroup"]
         )
 
         # get_cohort should return None as no group is assigned to user
@@ -274,13 +295,13 @@ class TestCohorts(ModuleStoreTestCase):
         # get_cohort should return a group for user
         self.assertEquals(cohorts.get_cohort(user, course.id).name, "AutoGroup")
 
-    def test_cohorting_with_auto_cohort_groups(self):
+    def test_cohorting_with_auto_cohorts(self):
         """
-        Make sure cohorts.get_cohort() does the right thing with auto_cohort_groups.
+        Make sure cohorts.get_cohort() does the right thing.
         If there are auto cohort groups then a user should be assigned one.
         """
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         user1 = UserFactory(username="test", email="a@b.com")
         user2 = UserFactory(username="test2", email="a2@b.com")
@@ -293,9 +314,8 @@ class TestCohorts(ModuleStoreTestCase):
         # Add an auto_cohort_group to the course...
         config_course_cohorts(
             course,
-            discussions=[],
-            cohorted=True,
-            auto_cohort_groups=["AutoGroup"]
+            is_cohorted=True,
+            auto_cohorts=["AutoGroup"]
         )
 
         self.assertEquals(cohorts.get_cohort(user1, course.id).id, cohort.id, "user1 should stay put")
@@ -315,16 +335,15 @@ class TestCohorts(ModuleStoreTestCase):
         # Add an auto_cohort_group to the course...
         config_course_cohorts(
             course,
-            discussions=[],
-            cohorted=True,
-            auto_cohort_groups=["AutoGroup"]
+            is_cohorted=True,
+            auto_cohorts=["AutoGroup"]
         )
 
         self.assertEquals(cohorts.get_cohort(user1, course.id).name, "AutoGroup", "user1 should be auto-cohorted")
 
         # Now set the auto_cohort_group to something different
         # This will have no effect on lms side as we are already done with migrations
-        config_course_cohorts(
+        config_course_cohorts_legacy(
             course,
             discussions=[],
             cohorted=True,
@@ -339,15 +358,15 @@ class TestCohorts(ModuleStoreTestCase):
             cohorts.get_cohort(user1, course.id).name, "AutoGroup", "user1 should still be in originally placed cohort"
         )
 
-    def test_cohorting_with_no_auto_cohort_groups(self):
+    def test_cohorting_with_no_auto_cohorts(self):
         """
-        Make sure cohorts.get_cohort() does the right thing with auto_cohort_groups.
-        If there are not auto cohort groups then a user should be assigned to Default Cohort Group.
+        Make sure cohorts.get_cohort() does the right thing.
+        If there are not auto cohorts then a user should be assigned to Default Cohort Group.
         Also verifies that cohort config changes on studio/moduletore side will
         not be reflected on lms after the migrations are done.
         """
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         user1 = UserFactory(username="test", email="a@b.com")
         user2 = UserFactory(username="test2", email="a2@b.com")
@@ -355,9 +374,8 @@ class TestCohorts(ModuleStoreTestCase):
         # Make the auto_cohort_group list empty
         config_course_cohorts(
             course,
-            discussions=[],
-            cohorted=True,
-            auto_cohort_groups=[]
+            is_cohorted=True,
+            auto_cohorts=[]
         )
 
         self.assertEquals(
@@ -368,7 +386,7 @@ class TestCohorts(ModuleStoreTestCase):
 
         # Add an auto_cohort_group to the course
         # This will have no effect on lms side as we are already done with migrations
-        config_course_cohorts(
+        config_course_cohorts_legacy(
             course,
             discussions=[],
             cohorted=True,
@@ -393,11 +411,11 @@ class TestCohorts(ModuleStoreTestCase):
         Make sure cohorts.get_cohort() randomizes properly.
         """
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         groups = ["group_{0}".format(n) for n in range(5)]
         config_course_cohorts(
-            course, discussions=[], cohorted=True, auto_cohort_groups=groups
+            course, is_cohorted=True, auto_cohorts=groups
         )
 
         # Assign 100 users to cohorts
@@ -423,7 +441,7 @@ class TestCohorts(ModuleStoreTestCase):
         Tests get_course_cohorts returns an empty list when no cohorts exist.
         """
         course = modulestore().get_course(self.toy_course_key)
-        config_course_cohorts(course, [], cohorted=True)
+        config_course_cohorts(course, is_cohorted=True)
         self.assertEqual([], cohorts.get_course_cohorts(course))
 
     def test_get_course_cohorts(self):
@@ -432,8 +450,9 @@ class TestCohorts(ModuleStoreTestCase):
         """
         course = modulestore().get_course(self.toy_course_key)
         config_course_cohorts(
-            course, [], cohorted=True,
-            auto_cohort_groups=["AutoGroup1", "AutoGroup2"]
+            course,
+            is_cohorted=True,
+            auto_cohorts=["AutoGroup1", "AutoGroup2"]
         )
 
         # add manual cohorts to course 1
@@ -445,7 +464,7 @@ class TestCohorts(ModuleStoreTestCase):
 
     def test_is_commentable_cohorted(self):
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         def to_id(name):
             return topic_name_to_id(course, name)
@@ -457,7 +476,7 @@ class TestCohorts(ModuleStoreTestCase):
         )
 
         # not cohorted
-        config_course_cohorts(course, ["General", "Feedback"], cohorted=False)
+        config_course_cohorts(course, is_cohorted=False, discussion_topics=["General", "Feedback"])
 
         self.assertFalse(
             cohorts.is_commentable_cohorted(course.id, to_id("General")),
@@ -465,9 +484,9 @@ class TestCohorts(ModuleStoreTestCase):
         )
 
         # cohorted, but top level topics aren't
-        config_course_cohorts(course, ["General", "Feedback"], cohorted=True)
+        config_course_cohorts(course, is_cohorted=True, discussion_topics=["General", "Feedback"])
 
-        self.assertTrue(course.is_cohorted)
+        self.assertTrue(cohorts.is_course_cohorted(course.id))
         self.assertFalse(
             cohorts.is_commentable_cohorted(course.id, to_id("General")),
             "Course is cohorted, but 'General' isn't."
@@ -475,12 +494,13 @@ class TestCohorts(ModuleStoreTestCase):
 
         # cohorted, including "Feedback" top-level topics aren't
         config_course_cohorts(
-            course, ["General", "Feedback"],
-            cohorted=True,
+            course,
+            is_cohorted=True,
+            discussion_topics=["General", "Feedback"],
             cohorted_discussions=["Feedback"]
         )
 
-        self.assertTrue(course.is_cohorted)
+        self.assertTrue(cohorts.is_course_cohorted(course.id))
         self.assertFalse(
             cohorts.is_commentable_cohorted(course.id, to_id("General")),
             "Course is cohorted, but 'General' isn't."
@@ -492,14 +512,15 @@ class TestCohorts(ModuleStoreTestCase):
 
     def test_is_commentable_cohorted_inline_discussion(self):
         course = modulestore().get_course(self.toy_course_key)
-        self.assertFalse(course.is_cohorted)
+        self.assertFalse(cohorts.is_course_cohorted(course.id))
 
         def to_id(name):  # pylint: disable=missing-docstring
             return topic_name_to_id(course, name)
 
         config_course_cohorts(
-            course, ["General", "Feedback"],
-            cohorted=True,
+            course,
+            is_cohorted=True,
+            discussion_topics=["General", "Feedback"],
             cohorted_discussions=["Feedback", "random_inline"]
         )
         self.assertTrue(
@@ -510,8 +531,9 @@ class TestCohorts(ModuleStoreTestCase):
         # if always_cohort_inline_discussions is set to False, non-top-level discussion are always
         # non cohorted unless they are explicitly set in cohorted_discussions
         config_course_cohorts(
-            course, ["General", "Feedback"],
-            cohorted=True,
+            course,
+            is_cohorted=True,
+            discussion_topics=["General", "Feedback"],
             cohorted_discussions=["Feedback", "random_inline"],
             always_cohort_inline_discussions=False
         )
@@ -538,12 +560,13 @@ class TestCohorts(ModuleStoreTestCase):
 
         self.assertEqual(cohorts.get_cohorted_commentables(course.id), set())
 
-        config_course_cohorts(course, [], cohorted=True)
+        config_course_cohorts(course, is_cohorted=True)
         self.assertEqual(cohorts.get_cohorted_commentables(course.id), set())
 
         config_course_cohorts(
-            course, ["General", "Feedback"],
-            cohorted=True,
+            course,
+            is_cohorted=True,
+            discussion_topics=["General", "Feedback"],
             cohorted_discussions=["Feedback"]
         )
         self.assertItemsEqual(
@@ -552,8 +575,9 @@ class TestCohorts(ModuleStoreTestCase):
         )
 
         config_course_cohorts(
-            course, ["General", "Feedback"],
-            cohorted=True,
+            course,
+            is_cohorted=True,
+            discussion_topics=["General", "Feedback"],
             cohorted_discussions=["General", "Feedback"]
         )
         self.assertItemsEqual(
@@ -685,12 +709,67 @@ class TestCohorts(ModuleStoreTestCase):
             lambda: cohorts.add_user_to_cohort(first_cohort, "non_existent_username")
         )
 
+    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.assertTrue(course_cohort_settings.always_cohort_inline_discussions)
+
+    def test_update_course_cohort_settings(self):
+        """
+        Test that cohorts.set_course_cohort_settings is working as expected.
+        """
+        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=False
+        )
+
+        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.assertFalse(course_cohort_settings.always_cohort_inline_discussions)
+
+    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__)
+            )
+
+
+@ddt.ddt
 class TestCohortsAndPartitionGroups(ModuleStoreTestCase):
-    MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE
     """
     Test Cohorts and Partitions Groups.
     """
+    MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE
 
     def setUp(self):
         """
@@ -748,6 +827,25 @@ class TestCohortsAndPartitionGroups(ModuleStoreTestCase):
             (None, None),
         )
 
+    @ddt.data(
+        (True, 1),
+        (False, 3),
+    )
+    @ddt.unpack
+    def test_get_group_info_for_cohort_queries(self, use_cached, num_sql_queries):
+        """
+        Basic test of the partition_group_info accessor function
+        """
+        # create a link for the cohort in the db
+        self._link_cohort_partition_group(
+            self.first_cohort,
+            self.partition_id,
+            self.group1_id
+        )
+        with self.assertNumQueries(num_sql_queries):
+            for __ in range(3):
+                self.assertIsNotNone(cohorts.get_group_info_for_cohort(self.first_cohort, use_cached=use_cached))
+
     def test_multiple_cohorts(self):
         """
         Test that multiple cohorts can be linked to the same partition group

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

@@ -41,7 +41,7 @@ class TestCohortPartitionScheme(ModuleStoreTestCase):
 
         self.course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall")
         self.course = modulestore().get_course(self.course_key)
-        config_course_cohorts(self.course, [], cohorted=True)
+        config_course_cohorts(self.course, is_cohorted=True)
 
         self.groups = [Group(10, 'Group 10'), Group(20, 'Group 20')]
         self.user_partition = UserPartition(
@@ -63,6 +63,7 @@ class TestCohortPartitionScheme(ModuleStoreTestCase):
                 self.course_key,
                 self.student,
                 partition or self.user_partition,
+                use_cached=False
             ),
             group
         )

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

@@ -3,30 +3,36 @@ Tests for course group views
 """
 # pylint: disable=attribute-defined-outside-init
 # pylint: disable=no-member
-from collections import namedtuple
 import json
 
 from collections import namedtuple
+from datetime import datetime
+from unittest import skipUnless
+
+from django.conf import settings
 from django.contrib.auth.models import User
 from django.http import Http404
 from django.test.client import RequestFactory
-
+import django_test_client_utils  # monkey-patch for PATCH request method.
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
-from xmodule.modulestore.django import modulestore
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from xmodule.modulestore.tests.factories import ItemFactory
 
 from ..models import CourseUserGroup, CourseCohort
 from ..views import (
-    cohort_handler, users_in_cohort, add_users_to_cohort, remove_user_from_cohort, link_cohort_to_partition_group
+    course_cohort_settings_handler, cohort_handler, users_in_cohort, add_users_to_cohort, remove_user_from_cohort,
+    link_cohort_to_partition_group, cohort_discussion_topics
 )
 from ..cohorts import (
     get_cohort, get_cohort_by_name, get_cohort_by_id,
     DEFAULT_COHORT_NAME, get_group_info_for_cohort
 )
-from .helpers import config_course_cohorts, CohortFactory, CourseCohortFactory
+from .helpers import (
+    config_course_cohorts, config_course_cohorts_legacy, CohortFactory, CourseCohortFactory, topic_name_to_id
+)
 
 
 class CohortViewsTestCase(ModuleStoreTestCase):
@@ -90,49 +96,241 @@ class CohortViewsTestCase(ModuleStoreTestCase):
         view_args.insert(0, request)
         self.assertRaises(Http404, view, *view_args)
 
+    def create_cohorted_discussions(self):
+        cohorted_inline_discussions = ['Topic A']
+        cohorted_course_wide_discussions = ["Topic B"]
+        cohorted_discussions = cohorted_inline_discussions + cohorted_course_wide_discussions
 
-class CohortHandlerTestCase(CohortViewsTestCase):
-    """
-    Tests the `cohort_handler` view.
-    """
-    def get_cohort_handler(self, course, cohort=None):
+        # inline discussion
+        ItemFactory.create(
+            parent_location=self.course.location,
+            category="discussion",
+            discussion_id=topic_name_to_id(self.course, "Topic A"),
+            discussion_category="Chapter",
+            discussion_target="Discussion",
+            start=datetime.now()
+        )
+        # course-wide discussion
+        discussion_topics = {
+            "Topic B": {"id": "Topic B"},
+        }
+
+        config_course_cohorts(
+            self.course,
+            is_cohorted=True,
+            discussion_topics=discussion_topics,
+            cohorted_discussions=cohorted_discussions
+        )
+        return cohorted_inline_discussions, cohorted_course_wide_discussions
+
+    def get_handler(self, course, cohort=None, expected_response_code=200, handler=cohort_handler):
         """
-        Call a GET on `cohort_handler` for a given `course` and return its response as a
-        dict. If `cohort` is specified, only information for that specific cohort is returned.
+        Call a GET on `handler` for a given `course` and return its response as a dict.
+        Raise an exception if response status code is not as expected.
         """
         request = RequestFactory().get("dummy_url")
         request.user = self.staff_user
         if cohort:
-            response = cohort_handler(request, unicode(course.id), cohort.id)
+            response = handler(request, unicode(course.id), cohort.id)
         else:
-            response = cohort_handler(request, unicode(course.id))
-        self.assertEqual(response.status_code, 200)
+            response = handler(request, unicode(course.id))
+        self.assertEqual(response.status_code, expected_response_code)
         return json.loads(response.content)
 
-    def put_cohort_handler(self, course, cohort=None, data=None, expected_response_code=200):
+    def put_handler(self, course, cohort=None, data=None, expected_response_code=200, handler=cohort_handler):
         """
-        Call a PUT on `cohort_handler` for a given `course` and return its response as a
-        dict. If `cohort` is not specified, a new cohort is created. If `cohort` is specified,
-        the existing cohort is updated.
+        Call a PUT on `handler` for a given `course` and return its response as a dict.
+        Raise an exception if response status code is not as expected.
         """
         if not isinstance(data, basestring):
             data = json.dumps(data or {})
         request = RequestFactory().put(path="dummy path", data=data, content_type="application/json")
         request.user = self.staff_user
         if cohort:
-            response = cohort_handler(request, unicode(course.id), cohort.id)
+            response = handler(request, unicode(course.id), cohort.id)
         else:
-            response = cohort_handler(request, unicode(course.id))
+            response = handler(request, unicode(course.id))
         self.assertEqual(response.status_code, expected_response_code)
         return json.loads(response.content)
 
+    def patch_handler(self, course, cohort=None, data=None, expected_response_code=200, handler=cohort_handler):
+        """
+        Call a PATCH on `handler` for a given `course` and return its response as a dict.
+        Raise an exception if response status code is not as expected.
+        """
+        if not isinstance(data, basestring):
+            data = json.dumps(data or {})
+
+        request = RequestFactory().patch(path="dummy path", data=data, content_type="application/json")
+        request.user = self.staff_user
+        if cohort:
+            response = handler(request, unicode(course.id), cohort.id)
+        else:
+            response = handler(request, unicode(course.id))
+        self.assertEqual(response.status_code, expected_response_code)
+        return json.loads(response.content)
+
+
+class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase):
+    """
+    Tests the `course_cohort_settings_handler` view.
+    """
+
+    def get_expected_response(self):
+        """
+        Returns the static response dict.
+        """
+        return {
+            'is_cohorted': True,
+            'always_cohort_inline_discussions': True,
+            'cohorted_inline_discussions': [],
+            'cohorted_course_wide_discussions': [],
+            'id': 1
+        }
+
+    def test_non_staff(self):
+        """
+        Verify that we cannot access course_cohort_settings_handler if we're a non-staff user.
+        """
+        self._verify_non_staff_cannot_access(course_cohort_settings_handler, "GET", [unicode(self.course.id)])
+        self._verify_non_staff_cannot_access(course_cohort_settings_handler, "PATCH", [unicode(self.course.id)])
+
+    def test_get_settings(self):
+        """
+        Verify that course_cohort_settings_handler is working for HTTP GET.
+        """
+        cohorted_inline_discussions, cohorted_course_wide_discussions = self.create_cohorted_discussions()
+
+        response = self.get_handler(self.course, handler=course_cohort_settings_handler)
+        expected_response = self.get_expected_response()
+
+        expected_response['cohorted_inline_discussions'] = [topic_name_to_id(self.course, name)
+                                                            for name in cohorted_inline_discussions]
+        expected_response['cohorted_course_wide_discussions'] = [topic_name_to_id(self.course, name)
+                                                                 for name in cohorted_course_wide_discussions]
+
+        self.assertEqual(response, expected_response)
+
+    def test_update_is_cohorted_settings(self):
+        """
+        Verify that course_cohort_settings_handler is working for is_cohorted via HTTP PATCH.
+        """
+        config_course_cohorts(self.course, is_cohorted=True)
+
+        response = self.get_handler(self.course, handler=course_cohort_settings_handler)
+
+        expected_response = self.get_expected_response()
+
+        self.assertEqual(response, expected_response)
+
+        expected_response['is_cohorted'] = False
+        response = self.patch_handler(self.course, data=expected_response, handler=course_cohort_settings_handler)
+
+        self.assertEqual(response, expected_response)
+
+    def test_update_always_cohort_inline_discussion_settings(self):
+        """
+        Verify that course_cohort_settings_handler is working for always_cohort_inline_discussions via HTTP PATCH.
+        """
+        config_course_cohorts(self.course, is_cohorted=True)
+
+        response = self.get_handler(self.course, handler=course_cohort_settings_handler)
+
+        expected_response = self.get_expected_response()
+
+        self.assertEqual(response, expected_response)
+
+        expected_response['always_cohort_inline_discussions'] = False
+        response = self.patch_handler(self.course, data=expected_response, handler=course_cohort_settings_handler)
+
+        self.assertEqual(response, expected_response)
+
+    def test_update_course_wide_discussion_settings(self):
+        """
+        Verify that course_cohort_settings_handler is working for cohorted_course_wide_discussions via HTTP PATCH.
+        """
+        # course-wide discussion
+        discussion_topics = {
+            "Topic B": {"id": "Topic B"},
+        }
+
+        config_course_cohorts(self.course, is_cohorted=True, discussion_topics=discussion_topics)
+
+        response = self.get_handler(self.course, handler=course_cohort_settings_handler)
+
+        expected_response = self.get_expected_response()
+        self.assertEqual(response, expected_response)
+
+        expected_response['cohorted_course_wide_discussions'] = [topic_name_to_id(self.course, "Topic B")]
+        response = self.patch_handler(self.course, data=expected_response, handler=course_cohort_settings_handler)
+
+        self.assertEqual(response, expected_response)
+
+    def test_update_inline_discussion_settings(self):
+        """
+        Verify that course_cohort_settings_handler is working for cohorted_inline_discussions via HTTP PATCH.
+        """
+        config_course_cohorts(self.course, is_cohorted=True)
+
+        response = self.get_handler(self.course, handler=course_cohort_settings_handler)
+
+        expected_response = self.get_expected_response()
+        self.assertEqual(response, expected_response)
+
+        now = datetime.now()
+        # inline discussion
+        ItemFactory.create(
+            parent_location=self.course.location,
+            category="discussion",
+            discussion_id="Topic_A",
+            discussion_category="Chapter",
+            discussion_target="Discussion",
+            start=now
+        )
+
+        expected_response['cohorted_inline_discussions'] = ["Topic_A"]
+        response = self.patch_handler(self.course, data=expected_response, handler=course_cohort_settings_handler)
+
+        self.assertEqual(response, expected_response)
+
+    def test_update_settings_with_missing_field(self):
+        """
+        Verify that course_cohort_settings_handler return HTTP 400 if required data field is missing from post data.
+        """
+        config_course_cohorts(self.course, is_cohorted=True)
+
+        response = self.patch_handler(self.course, expected_response_code=400, handler=course_cohort_settings_handler)
+        self.assertEqual("Bad Request", response.get("error"))
+
+    def test_update_settings_with_invalid_field_data_type(self):
+        """
+        Verify that course_cohort_settings_handler return HTTP 400 if field data type is incorrect.
+        """
+        config_course_cohorts(self.course, is_cohorted=True)
+
+        response = self.patch_handler(
+            self.course,
+            data={'is_cohorted': ''},
+            expected_response_code=400,
+            handler=course_cohort_settings_handler
+        )
+        self.assertEqual(
+            "Incorrect field type for `{}`. Type must be `{}`".format('is_cohorted', bool.__name__),
+            response.get("error")
+        )
+
+
+class CohortHandlerTestCase(CohortViewsTestCase):
+    """
+    Tests the `cohort_handler` view.
+    """
     def verify_lists_expected_cohorts(self, expected_cohorts, response_dict=None):
         """
         Verify that the server response contains the expected_cohorts.
         If response_dict is None, the list of cohorts is requested from the server.
         """
         if response_dict is None:
-            response_dict = self.get_cohort_handler(self.course)
+            response_dict = self.get_handler(self.course)
 
         self.assertEqual(
             response_dict.get("cohorts"),
@@ -191,23 +389,21 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         """
         Verify that auto cohorts are included in the response.
         """
-        config_course_cohorts(self.course, [], cohorted=True,
-                              auto_cohort_groups=["AutoGroup1", "AutoGroup2"])
+        config_course_cohorts(self.course, is_cohorted=True, auto_cohorts=["AutoGroup1", "AutoGroup2"])
 
-        # Will create cohort1, cohort2, and cohort3. Auto cohorts remain uncreated.
+        # Will create manual cohorts cohort1, cohort2, and cohort3.
         self._create_cohorts()
-        # Get the cohorts from the course, which will cause auto cohorts to be created.
-        actual_cohorts = self.get_cohort_handler(self.course)
+        actual_cohorts = self.get_handler(self.course)
         # Get references to the created auto cohorts.
         auto_cohort_1 = get_cohort_by_name(self.course.id, "AutoGroup1")
         auto_cohort_2 = get_cohort_by_name(self.course.id, "AutoGroup2")
         expected_cohorts = [
+            CohortHandlerTestCase.create_expected_cohort(auto_cohort_1, 0, CourseCohort.RANDOM),
+            CohortHandlerTestCase.create_expected_cohort(auto_cohort_2, 0, CourseCohort.RANDOM),
             CohortHandlerTestCase.create_expected_cohort(self.cohort1, 3, CourseCohort.MANUAL),
             CohortHandlerTestCase.create_expected_cohort(self.cohort2, 2, CourseCohort.MANUAL),
             CohortHandlerTestCase.create_expected_cohort(self.cohort3, 2, CourseCohort.MANUAL),
             CohortHandlerTestCase.create_expected_cohort(self.cohort4, 2, CourseCohort.RANDOM),
-            CohortHandlerTestCase.create_expected_cohort(auto_cohort_1, 0, CourseCohort.RANDOM),
-            CohortHandlerTestCase.create_expected_cohort(auto_cohort_2, 0, CourseCohort.RANDOM),
         ]
         self.verify_lists_expected_cohorts(expected_cohorts, actual_cohorts)
 
@@ -218,8 +414,8 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         # verify the default cohort is not created when the course is not cohorted
         self.verify_lists_expected_cohorts([])
 
-        # create a cohorted course without any auto_cohort_groups
-        config_course_cohorts(self.course, [], cohorted=True)
+        # create a cohorted course without any auto_cohorts
+        config_course_cohorts(self.course, is_cohorted=True)
 
         # verify the default cohort is not yet created until a user is assigned
         self.verify_lists_expected_cohorts([])
@@ -235,7 +431,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
 
         # verify the default cohort is automatically created
         default_cohort = get_cohort_by_name(self.course.id, DEFAULT_COHORT_NAME)
-        actual_cohorts = self.get_cohort_handler(self.course)
+        actual_cohorts = self.get_handler(self.course)
         self.verify_lists_expected_cohorts(
             [CohortHandlerTestCase.create_expected_cohort(default_cohort, len(users), CourseCohort.RANDOM)],
             actual_cohorts,
@@ -243,7 +439,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
 
         # set auto_cohort_groups
         # these cohort config will have not effect on lms side as we are already done with migrations
-        config_course_cohorts(self.course, [], cohorted=True, auto_cohort_groups=["AutoGroup"])
+        config_course_cohorts_legacy(self.course, [], cohorted=True, auto_cohort_groups=["AutoGroup"])
 
         # We should expect the DoesNotExist exception because above cohort config have
         # no effect on lms side so as a result there will be no AutoGroup cohort present
@@ -255,7 +451,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         Tests that information for just a single cohort can be requested.
         """
         self._create_cohorts()
-        response_dict = self.get_cohort_handler(self.course, self.cohort2)
+        response_dict = self.get_handler(self.course, self.cohort2)
         self.assertEqual(
             response_dict,
             {
@@ -298,7 +494,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         """
         new_cohort_name = "New cohort unassociated to content groups"
         request_data = {'name': new_cohort_name, 'assignment_type': CourseCohort.RANDOM}
-        response_dict = self.put_cohort_handler(self.course, data=request_data)
+        response_dict = self.put_handler(self.course, data=request_data)
         self.verify_contains_added_cohort(response_dict, new_cohort_name, assignment_type=CourseCohort.RANDOM)
 
         new_cohort_name = "New cohort linked to group"
@@ -308,7 +504,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
             'user_partition_id': 1,
             'group_id': 2
         }
-        response_dict = self.put_cohort_handler(self.course, data=data)
+        response_dict = self.put_handler(self.course, data=data)
         self.verify_contains_added_cohort(
             response_dict,
             new_cohort_name,
@@ -320,14 +516,14 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         """
         Verify that we cannot create a cohort without specifying a name.
         """
-        response_dict = self.put_cohort_handler(self.course, expected_response_code=400)
+        response_dict = self.put_handler(self.course, expected_response_code=400)
         self.assertEqual("Cohort name must be specified.", response_dict.get("error"))
 
     def test_create_new_cohort_missing_assignment_type(self):
         """
         Verify that we cannot create a cohort without specifying an assignment type.
         """
-        response_dict = self.put_cohort_handler(self.course, data={'name': 'COHORT NAME'}, expected_response_code=400)
+        response_dict = self.put_handler(self.course, data={'name': 'COHORT NAME'}, expected_response_code=400)
         self.assertEqual("Assignment type must be specified.", response_dict.get("error"))
 
     def test_create_new_cohort_existing_name(self):
@@ -335,7 +531,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         Verify that we cannot add a cohort with the same name as an existing cohort.
         """
         self._create_cohorts()
-        response_dict = self.put_cohort_handler(
+        response_dict = self.put_handler(
             self.course, data={'name': self.cohort1.name, 'assignment_type': CourseCohort.MANUAL},
             expected_response_code=400
         )
@@ -346,7 +542,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         Verify that we cannot create a cohort with a group_id if the user_partition_id is not also specified.
         """
         data = {'name': "Cohort missing user_partition_id", 'assignment_type': CourseCohort.MANUAL, 'group_id': 2}
-        response_dict = self.put_cohort_handler(self.course, data=data, expected_response_code=400)
+        response_dict = self.put_handler(self.course, data=data, expected_response_code=400)
         self.assertEqual(
             "If group_id is specified, user_partition_id must also be specified.", response_dict.get("error")
         )
@@ -360,7 +556,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         self._create_cohorts()
         updated_name = self.cohort1.name + "_updated"
         data = {'name': updated_name, 'assignment_type': CourseCohort.MANUAL}
-        response_dict = self.put_cohort_handler(self.course, self.cohort1, data=data)
+        response_dict = self.put_handler(self.course, self.cohort1, data=data)
         self.assertEqual(updated_name, get_cohort_by_id(self.course.id, self.cohort1.id).name)
         self.assertEqual(updated_name, response_dict.get("name"))
         self.assertEqual(CourseCohort.MANUAL, response_dict.get("assignment_type"))
@@ -372,7 +568,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         # Create a new cohort with random assignment
         cohort_name = 'I AM A RANDOM COHORT'
         data = {'name': cohort_name, 'assignment_type': CourseCohort.RANDOM}
-        response_dict = self.put_cohort_handler(self.course, data=data)
+        response_dict = self.put_handler(self.course, data=data)
 
         self.assertEqual(cohort_name, response_dict.get("name"))
         self.assertEqual(CourseCohort.RANDOM, response_dict.get("assignment_type"))
@@ -381,7 +577,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         newly_created_cohort = get_cohort_by_name(self.course.id, cohort_name)
         cohort_name = 'I AM AN UPDATED RANDOM COHORT'
         data = {'name': cohort_name, 'assignment_type': CourseCohort.RANDOM}
-        response_dict = self.put_cohort_handler(self.course, newly_created_cohort, data=data)
+        response_dict = self.put_handler(self.course, newly_created_cohort, data=data)
 
         self.assertEqual(cohort_name, get_cohort_by_id(self.course.id, newly_created_cohort.id).name)
         self.assertEqual(cohort_name, response_dict.get("name"))
@@ -394,7 +590,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         # Create a new cohort with random assignment
         cohort_name = 'I AM A RANDOM COHORT'
         data = {'name': cohort_name, 'assignment_type': CourseCohort.RANDOM}
-        response_dict = self.put_cohort_handler(self.course, data=data)
+        response_dict = self.put_handler(self.course, data=data)
 
         self.assertEqual(cohort_name, response_dict.get("name"))
         self.assertEqual(CourseCohort.RANDOM, response_dict.get("assignment_type"))
@@ -402,9 +598,9 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         # Try to update the assignment type of newly created random cohort
         cohort = get_cohort_by_name(self.course.id, cohort_name)
         data = {'name': cohort_name, 'assignment_type': CourseCohort.MANUAL}
-        response_dict = self.put_cohort_handler(self.course, cohort, data=data, expected_response_code=400)
+        response_dict = self.put_handler(self.course, cohort, data=data, expected_response_code=400)
         self.assertEqual(
-            'There must be one cohort to which students can be randomly assigned.', response_dict.get("error")
+            'There must be one cohort to which students can automatically be assigned.', response_dict.get("error")
         )
 
     def test_update_cohort_group_id(self):
@@ -419,7 +615,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
             'group_id': 2,
             'user_partition_id': 3
         }
-        response_dict = self.put_cohort_handler(self.course, self.cohort1, data=data)
+        response_dict = self.put_handler(self.course, self.cohort1, data=data)
         self.assertEqual((2, 3), get_group_info_for_cohort(self.cohort1))
         self.assertEqual(2, response_dict.get("group_id"))
         self.assertEqual(3, response_dict.get("user_partition_id"))
@@ -434,7 +630,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         link_cohort_to_partition_group(self.cohort1, 5, 0)
         self.assertEqual((0, 5), get_group_info_for_cohort(self.cohort1))
         data = {'name': self.cohort1.name, 'assignment_type': CourseCohort.RANDOM, 'group_id': None}
-        response_dict = self.put_cohort_handler(self.course, self.cohort1, data=data)
+        response_dict = self.put_handler(self.course, self.cohort1, data=data)
         self.assertEqual((None, None), get_group_info_for_cohort(self.cohort1))
         self.assertIsNone(response_dict.get("group_id"))
         self.assertIsNone(response_dict.get("user_partition_id"))
@@ -452,7 +648,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
             'group_id': 2,
             'user_partition_id': 3
         }
-        self.put_cohort_handler(self.course, self.cohort4, data=data)
+        self.put_handler(self.course, self.cohort4, data=data)
         self.assertEqual((2, 3), get_group_info_for_cohort(self.cohort4))
 
         data = {
@@ -461,7 +657,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
             'group_id': 1,
             'user_partition_id': 3
         }
-        self.put_cohort_handler(self.course, self.cohort4, data=data)
+        self.put_handler(self.course, self.cohort4, data=data)
         self.assertEqual((1, 3), get_group_info_for_cohort(self.cohort4))
 
     def test_update_cohort_missing_user_partition_id(self):
@@ -470,7 +666,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
         """
         self._create_cohorts()
         data = {'name': self.cohort1.name, 'assignment_type': CourseCohort.RANDOM, 'group_id': 2}
-        response_dict = self.put_cohort_handler(self.course, self.cohort1, data=data, expected_response_code=400)
+        response_dict = self.put_handler(self.course, self.cohort1, data=data, expected_response_code=400)
         self.assertEqual(
             "If group_id is specified, user_partition_id must also be specified.", response_dict.get("error")
         )
@@ -998,3 +1194,59 @@ class RemoveUserFromCohortTestCase(CohortViewsTestCase):
         cohort = CohortFactory(course_id=self.course.id, users=[user])
         response_dict = self.request_remove_user_from_cohort(user.username, cohort)
         self.verify_removed_user_from_cohort(user.username, response_dict, cohort)
+
+
+@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS')
+class CourseCohortDiscussionTopicsTestCase(CohortViewsTestCase):
+    """
+    Tests the `cohort_discussion_topics` view.
+    """
+
+    def test_non_staff(self):
+        """
+        Verify that we cannot access cohort_discussion_topics if we're a non-staff user.
+        """
+        self._verify_non_staff_cannot_access(cohort_discussion_topics, "GET", [unicode(self.course.id)])
+
+    def test_get_discussion_topics(self):
+        """
+        Verify that course_cohort_settings_handler is working for HTTP GET.
+        """
+        # create inline & course-wide discussion to verify the different map.
+        self.create_cohorted_discussions()
+
+        response = self.get_handler(self.course, handler=cohort_discussion_topics)
+        start_date = response['inline_discussions']['subcategories']['Chapter']['start_date']
+        expected_response = {
+            "course_wide_discussions": {
+                'children': ['Topic B'],
+                'entries': {
+                    'Topic B': {
+                        'sort_key': 'A',
+                        'is_cohorted': True,
+                        'id': topic_name_to_id(self.course, "Topic B"),
+                        'start_date': response['course_wide_discussions']['entries']['Topic B']['start_date']
+                    }
+                }
+            },
+            "inline_discussions": {
+                'subcategories': {
+                    'Chapter': {
+                        'subcategories': {},
+                        'children': ['Discussion'],
+                        'entries': {
+                            'Discussion': {
+                                'sort_key': None,
+                                'is_cohorted': True,
+                                'id': topic_name_to_id(self.course, "Topic A"),
+                                'start_date': start_date
+                            }
+                        },
+                        'sort_key': 'Chapter',
+                        'start_date': start_date
+                    }
+                },
+                'children': ['Chapter']
+            }
+        }
+        self.assertEqual(response, expected_response)

openedx/core/djangoapps/course_groups/views.py

@@ -1,3 +1,7 @@
+"""
+Views related to course groups functionality.
+"""
+
 from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.http import require_POST
 from django.contrib.auth.models import User
@@ -12,11 +16,13 @@ from django.utils.translation import ugettext
 import logging
 import re
 
+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 . import cohorts
+from lms.djangoapps.django_comment_client.utils import get_discussion_category_map, get_discussion_categories_ids
 from .models import CourseUserGroup, CourseUserGroupPartitionGroup
 
 log = logging.getLogger(__name__)
@@ -55,6 +61,24 @@ def unlink_cohort_partition_group(cohort):
     CourseUserGroupPartitionGroup.objects.filter(course_user_group=cohort).delete()
 
 
+# pylint: disable=invalid-name
+def _get_course_cohort_settings_representation(course, course_cohort_settings):
+    """
+    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,
+    }
+
+
 def _get_cohort_representation(cohort, course):
     """
     Returns a JSON representation of a cohort.
@@ -71,6 +95,80 @@ 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
+@login_required
+def course_cohort_settings_handler(request, course_key_string):
+    """
+    The restful handler for cohort setting requests. Requires JSON.
+    This will raise 404 if user is not staff.
+    GET
+        Returns the JSON representation of cohort settings for the course.
+    PATCH
+        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)
+
+    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:
+            return JsonResponse({"error": unicode("Bad Request")}, 400)
+
+        try:
+            cohort_settings = cohorts.set_course_cohort_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_cohort_settings_representation(course, cohort_settings))
+
+
 @require_http_methods(("GET", "PUT", "POST", "PATCH"))
 @ensure_csrf_cookie
 @expect_json
@@ -306,8 +404,86 @@ def debug_cohort_mgmt(request, course_key_string):
     # add staff check to make sure it's safe if it's accidentally deployed.
     get_course_with_access(request.user, 'staff', course_key)
 
-    context = {'cohorts_ajax_url': reverse(
+    context = {'cohorts_url': reverse(
         'cohorts',
         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_cohorted": True,
+        >>>                           "id": "i4x-edx-eiorguegnru-course-foobarbaz"
+        >>>                       }
+        >>>                   }
+        >>>                   "children": ["General"]
+        >>>               },
+        >>>               "inline_discussions" : {
+        >>>                   "subcategories": {
+        >>>                       "Getting Started": {
+        >>>                           "subcategories": {},
+        >>>                           "children": [
+        >>>                               "Working with Videos",
+        >>>                               "Videos on edX"
+        >>>                           ],
+        >>>                           "entries": {
+        >>>                               "Working with Videos": {
+        >>>                                   "sort_key": None,
+        >>>                                   "is_cohorted": False,
+        >>>                                   "id": "d9f970a42067413cbb633f81cfb12604"
+        >>>                               },
+        >>>                               "Videos on edX": {
+        >>>                                   "sort_key": None,
+        >>>                                   "is_cohorted": False,
+        >>>                                   "id": "98d8feb5971041a085512ae22b398613"
+        >>>                               }
+        >>>                           }
+        >>>                       },
+        >>>                       "children": ["Getting Started"]
+        >>>                   },
+        >>>               }
+        >>>          }
+    """
+    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, cohorted_if_in_list=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 in discussion_category_map['children']:
+        if name in course_wide_entries:
+            course_wide_children.append(name)
+        else:
+            inline_children.append(name)
+
+    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)