From e6c75529c09f932abddfa843c49fb7e90fdbaa5f Mon Sep 17 00:00:00 2001 From: muzaffaryousaf Date: Tue, 17 Feb 2015 16:51:27 +0500 Subject: [PATCH] Cohort discussion topics via UI in instructor dashboard. TNL-1256 --- common/lib/django_test_client_utils.py | 49 ++ .../pages/lms/instructor_dashboard.py | 121 ++++ .../acceptance/tests/discussion/helpers.py | 3 +- .../discussion/test_cohort_management.py | 309 ++++++++++- .../tests/test_cohorted_courseware.py | 33 +- .../django_comment_client/tests/test_utils.py | 98 +++- lms/djangoapps/django_comment_client/utils.py | 74 ++- .../instructor/views/instructor_dashboard.py | 2 +- .../js/groups/models/cohort_discussions.js | 14 + .../groups/models/course_cohort_settings.js | 3 +- .../js/groups/views/cohort_discussions.js | 99 ++++ .../views/cohort_discussions_course_wide.js | 83 +++ .../groups/views/cohort_discussions_inline.js | 145 +++++ lms/static/js/groups/views/cohort_editor.js | 3 +- lms/static/js/groups/views/cohorts.js | 31 +- .../views/cohorts_dashboard_factory.js} | 18 +- .../js/spec/groups/views/cohorts_spec.js | 515 +++++++++++++++++- lms/static/js/spec/main.js | 21 + lms/static/js/vendor/jquery.qubit.js | 97 ++++ .../sass/course/instructor/_instructor_2.scss | 65 ++- .../cohort-discussions-category.underscore | 10 + .../cohort-discussions-course-wide.underscore | 19 + .../cohort-discussions-inline.underscore | 38 ++ .../cohort-discussions-subcategory.underscore | 9 + .../cohort-group-header.underscore | 7 +- .../cohort_management.html | 4 +- .../instructor_dashboard_2/cohorts.underscore | 10 + .../instructor_dashboard_2.html | 8 +- lms/urls.py | 3 + .../course_groups/tests/test_views.py | 219 +++++++- .../core/djangoapps/course_groups/views.py | 153 +++++- 31 files changed, 2118 insertions(+), 145 deletions(-) create mode 100644 common/lib/django_test_client_utils.py create mode 100644 lms/static/js/groups/models/cohort_discussions.js create mode 100644 lms/static/js/groups/views/cohort_discussions.js create mode 100644 lms/static/js/groups/views/cohort_discussions_course_wide.js create mode 100644 lms/static/js/groups/views/cohort_discussions_inline.js rename lms/static/js/{factories/cohorts_factory.js => groups/views/cohorts_dashboard_factory.js} (68%) create mode 100755 lms/static/js/vendor/jquery.qubit.js create mode 100644 lms/templates/instructor/instructor_dashboard_2/cohort-discussions-category.underscore create mode 100644 lms/templates/instructor/instructor_dashboard_2/cohort-discussions-course-wide.underscore create mode 100644 lms/templates/instructor/instructor_dashboard_2/cohort-discussions-inline.underscore create mode 100644 lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore diff --git a/common/lib/django_test_client_utils.py b/common/lib/django_test_client_utils.py new file mode 100644 index 0000000000..47d57dec65 --- /dev/null +++ b/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) diff --git a/common/test/acceptance/pages/lms/instructor_dashboard.py b/common/test/acceptance/pages/lms/instructor_dashboard.py index 9f4b1d3527..e43d58c001 100644 --- a/common/test/acceptance/pages/lms/instructor_dashboard.py +++ b/common/test/acceptance/pages/lms/instructor_dashboard.py @@ -105,6 +105,10 @@ class CohortManagementSection(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').present @@ -466,6 +470,123 @@ class CohortManagementSection(PageObject): 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). diff --git a/common/test/acceptance/tests/discussion/helpers.py b/common/test/acceptance/tests/discussion/helpers.py index a5fba485b0..269148b20d 100644 --- a/common/test/acceptance/tests/discussion/helpers.py +++ b/common/test/acceptance/tests/discussion/helpers.py @@ -62,10 +62,9 @@ class CohortTestMixin(object): """ 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.post(url, data=data, headers=course_fixture.headers) + 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): """ Adds a cohort by name, returning its ID. diff --git a/common/test/acceptance/tests/discussion/test_cohort_management.py b/common/test/acceptance/tests/discussion/test_cohort_management.py index 1b247c823a..48f452f0fa 100644 --- a/common/test/acceptance/tests/discussion/test_cohort_management.py +++ b/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 @@ -122,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. @@ -635,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): """ diff --git a/common/test/acceptance/tests/test_cohorted_courseware.py b/common/test/acceptance/tests/test_cohorted_courseware.py index cd1df402b1..11097684d5 100644 --- a/common/test/acceptance/tests/test_cohorted_courseware.py +++ b/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): """ @@ -219,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() diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index ea6da4523d..b67a0ad2d7 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -1,8 +1,9 @@ # -*- 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 @@ -179,7 +180,7 @@ 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 = {} @@ -198,6 +199,15 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): **kwargs ) + def assert_category_map_equals(self, expected, cohorted_if_in_list=False, exclude_unstarted=True): + """ + Asserts the expected map with the map returned by get_discussion_category_map method. + """ + self.assertEqual( + utils.get_discussion_category_map(self.course, cohorted_if_in_list, exclude_unstarted), + expected + ) + def test_empty(self): self.assert_category_map_equals({"entries": {}, "subcategories": {}, "children": []}) @@ -276,6 +286,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") @@ -401,8 +490,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) @@ -440,6 +529,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") diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 3aa99ecc15..d502a4c128 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -61,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. @@ -71,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)) ] @@ -146,10 +145,50 @@ 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) @@ -162,7 +201,7 @@ def get_discussion_category_map(course, user): 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,7 +257,7 @@ 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": course_cohort_settings.is_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 @@ -225,16 +273,22 @@ def get_discussion_category_map(course, user): _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 diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index a7ed90df09..5b44f8ace2 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -345,8 +345,8 @@ def _section_cohort_management(course, access): kwargs={'course_key_string': unicode(course_key)} ), 'cohorts_url': reverse('cohorts', kwargs={'course_key_string': unicode(course_key)}), - 'advanced_settings_url': get_studio_url(course, 'settings/advanced'), '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 diff --git a/lms/static/js/groups/models/cohort_discussions.js b/lms/static/js/groups/models/cohort_discussions.js new file mode 100644 index 0000000000..90af5a9207 --- /dev/null +++ b/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); diff --git a/lms/static/js/groups/models/course_cohort_settings.js b/lms/static/js/groups/models/course_cohort_settings.js index a87eeceec7..74be792614 100644 --- a/lms/static/js/groups/models/course_cohort_settings.js +++ b/lms/static/js/groups/models/course_cohort_settings.js @@ -9,7 +9,8 @@ var edx = edx || {}; idAttribute: 'id', defaults: { is_cohorted: false, - cohorted_discussions: [], + cohorted_inline_discussions: [], + cohorted_course_wide_discussions:[], always_cohort_inline_discussions: true } }); diff --git a/lms/static/js/groups/views/cohort_discussions.js b/lms/static/js/groups/views/cohort_discussions.js new file mode 100644 index 0000000000..b20c37d1f8 --- /dev/null +++ b/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 +); diff --git a/lms/static/js/groups/views/cohort_discussions_course_wide.js b/lms/static/js/groups/views/cohort_discussions_course_wide.js new file mode 100644 index 0000000000..97a06fea84 --- /dev/null +++ b/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); diff --git a/lms/static/js/groups/views/cohort_discussions_inline.js b/lms/static/js/groups/views/cohort_discussions_inline.js new file mode 100644 index 0000000000..036b2dafc6 --- /dev/null +++ b/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); diff --git a/lms/static/js/groups/views/cohort_editor.js b/lms/static/js/groups/views/cohort_editor.js index 448b398546..8795c9baf5 100644 --- a/lms/static/js/groups/views/cohort_editor.js +++ b/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 })); }, diff --git a/lms/static/js/groups/views/cohorts.js b/lms/static/js/groups/views/cohorts.js index a8184bece6..0d5714d901 100644 --- a/lms/static/js/groups/views/cohorts.js +++ b/lms/static/js/groups/views/cohorts.js @@ -1,7 +1,8 @@ var edx = edx || {}; (function($, _, Backbone, gettext, interpolate_text, CohortModel, CohortEditorView, CohortFormView, - CourseCohortSettingsNotificationView, NotificationModel, NotificationView, FileUploaderView) { + CourseCohortSettingsNotificationView, NotificationModel, NotificationView, FileUploaderView, + InlineDiscussionsView, CourseWideDiscussionsView) { 'use strict'; var hiddenClass = 'is-hidden', @@ -17,7 +18,8 @@ var edx = edx || {}; '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) { @@ -120,7 +122,7 @@ var edx = edx || {}; fieldData = {is_cohorted: this.getCohortsEnabled()}; cohortSettings = this.cohortSettings; cohortSettings.save( - fieldData, {wait: true} + fieldData, {patch: true, wait: true} ).done(function() { self.render(); self.renderCourseCohortSettingsNotificationView(); @@ -277,6 +279,27 @@ 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 + "']"; @@ -284,4 +307,4 @@ var edx = edx || {}; }); }).call(this, $, _, Backbone, gettext, interpolate_text, edx.groups.CohortModel, edx.groups.CohortEditorView, edx.groups.CohortFormView, edx.groups.CourseCohortSettingsNotificationView, NotificationModel, NotificationView, - FileUploaderView); + FileUploaderView, edx.groups.InlineDiscussionsView, edx.groups.CourseWideDiscussionsView); diff --git a/lms/static/js/factories/cohorts_factory.js b/lms/static/js/groups/views/cohorts_dashboard_factory.js similarity index 68% rename from lms/static/js/factories/cohorts_factory.js rename to lms/static/js/groups/views/cohorts_dashboard_factory.js index 3d04e1fd91..3ddf229220 100644 --- a/lms/static/js/factories/cohorts_factory.js +++ b/lms/static/js/groups/views/cohorts_dashboard_factory.js @@ -1,33 +1,39 @@ ;(function (define, undefined) { 'use strict'; - define(['jquery', 'js/groups/views/cohorts', 'js/groups/collections/cohort', 'js/groups/models/course_cohort_settings'], + 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(); + 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'), - studioAdvancedSettingsUrl: cohortManagementElement.data('advanced-settings-url'), studioGroupConfigurationsUrl: studioGroupConfigurationsUrl } }); + cohorts.fetch().done(function() { courseCohortSettings.fetch().done(function() { - cohortsView.render(); - }) + discussionTopicsSettings.fetch().done(function() { + cohortsView.render(); + }); + }); }); }; }); diff --git a/lms/static/js/spec/groups/views/cohorts_spec.js b/lms/static/js/spec/groups/views/cohorts_spec.js index 6106c16f2c..175b43f369 100644 --- a/lms/static/js/spec/groups/views/cohorts_spec.js +++ b/lms/static/js/spec/groups/views/cohorts_spec.js @@ -1,10 +1,13 @@ 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', - 'js/groups/models/course_cohort_settings', 'js/utils/animation', - 'js/groups/views/course_cohort_settings_notification' + '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, CourseCohortSettingsNotificationView) { + CourseCohortSettingsModel, AnimationUtil, Qubit, CourseCohortSettingsNotificationView, DiscussionTopicsSettingsModel, + CohortDiscussionsView, CohortCourseWideDiscussionsView, CohortInlineDiscussionsView) { 'use strict'; describe("Cohorts View", function () { @@ -14,7 +17,16 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe 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; + 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'; @@ -54,23 +66,71 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ]; }; - createMockCohortSettingsJson = function (isCohorted, cohortedDiscussions, alwaysCohortInlineDiscussions) { + createMockCohortSettingsJson = function (isCohorted, cohortedInlineDiscussions, cohortedCourseWideDiscussions, alwaysCohortInlineDiscussions) { return { id: 0, is_cohorted: isCohorted || false, - cohorted_discussions: cohortedDiscussions || [], + cohorted_inline_discussions: cohortedInlineDiscussions || [], + cohorted_course_wide_discussions: cohortedCourseWideDiscussions || [], always_cohort_inline_discussions: alwaysCohortInlineDiscussions || true }; }; - createMockCohortSettings = function (isCohorted, cohortedDiscussions, alwaysCohortInlineDiscussions) { + createMockCohortSettings = function (isCohorted, cohortedInlineDiscussions, cohortedCourseWideDiscussions, alwaysCohortInlineDiscussions) { return new CourseCohortSettingsModel( - createMockCohortSettingsJson(isCohorted, cohortedDiscussions, alwaysCohortInlineDiscussions) + 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, cohortSettings; + var cohortsJson, cohorts, contentGroups, cohortSettings, cohortDiscussions; options = options || {}; cohortsJson = options.cohorts ? {cohorts: options.cohorts} : createMockCohorts(); cohorts = new CohortCollection(cohortsJson, {parse: true}); @@ -78,17 +138,23 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe 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(); @@ -195,6 +261,41 @@ 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; }; @@ -208,6 +309,10 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe 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'); }); @@ -221,6 +326,8 @@ 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() { @@ -260,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); @@ -287,23 +398,23 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe cohortsView.$('.cohorts-state').prop('checked', true).change(); AjaxHelpers.expectJsonRequest( - requests, 'PUT', '/mock_service/cohorts/settings', - createMockCohortSettingsJson(true, [], true) + requests, 'PATCH', '/mock_service/cohorts/settings', + {is_cohorted: true} ); AjaxHelpers.respondWithJson( requests, - createMockCohortSettingsJson(true) + {is_cohorted: true} ); expect(cohortsView.$('.cohorts-state').prop('checked')).toBeTruthy(); cohortsView.$('.cohorts-state').prop('checked', false).change(); AjaxHelpers.expectJsonRequest( - requests, 'PUT', '/mock_service/cohorts/settings', - createMockCohortSettingsJson(false, [], true) + requests, 'PATCH', '/mock_service/cohorts/settings', + {is_cohorted: false} ); AjaxHelpers.respondWithJson( requests, - createMockCohortSettingsJson(false) + {is_cohorted: false} ); expect(cohortsView.$('.cohorts-state').prop('checked')).toBeFalsy(); }); @@ -960,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); + }); + + }); + + }); }); }); diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index 0c2b9fcbca..d3932fc64b 100644 --- a/lms/static/js/spec/main.js +++ b/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', @@ -68,6 +69,10 @@ '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', @@ -300,6 +305,22 @@ 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'] diff --git a/lms/static/js/vendor/jquery.qubit.js b/lms/static/js/vendor/jquery.qubit.js new file mode 100755 index 0000000000..7f85712708 --- /dev/null +++ b/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)); diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index bb9c40cad2..f5ad96107a 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -725,19 +725,18 @@ 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; } } + .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(); @@ -924,8 +923,9 @@ } } - // CSV-based file upload for auto cohort assigning - .toggle-cohort-management-secondary { + // 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; } @@ -1071,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; diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-category.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-category.underscore new file mode 100644 index 0000000000..735f4f0ad6 --- /dev/null +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-category.underscore @@ -0,0 +1,10 @@ +
  • +
    + +
    + + +
  • diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-course-wide.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-course-wide.underscore new file mode 100644 index 0000000000..7cc647389f --- /dev/null +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-course-wide.underscore @@ -0,0 +1,19 @@ +

    <%- gettext('Specify whether discussion topics are divided by cohort') %>

    +
    +
    +
    +
    +
    +

    <%- gettext('Course-Wide Discussion Topics') %>

    +

    <%- gettext('Select the course-wide discussion topics that you want to divide by cohort.') %>

    +
    +
      <%= courseWideTopics %>
    +
    +
    +
    +
    +
    + +
    +
    +
    diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-inline.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-inline.underscore new file mode 100644 index 0000000000..f70225d3d7 --- /dev/null +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-inline.underscore @@ -0,0 +1,38 @@ +
    + +
    +
    +
    +
    +
    +

    <%- gettext('Content-Specific Discussion Topics') %>

    +

    <%- gettext('Specify whether content-specific discussion topics are divided by cohort.') %>

    +
    + +
    +
    + +
    +
    +
    + <% if ( inlineDiscussionTopics ) { %> +
      <%= inlineDiscussionTopics %>
    + <% } else { %> + <%- gettext('No content-specific discussion topics exist.') %> + <% } %> +
    +
    +
    +
    +
    +
    + +
    +
    +
    diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore new file mode 100644 index 0000000000..28b2dffb86 --- /dev/null +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-discussions-subcategory.underscore @@ -0,0 +1,9 @@ +
  • +
    + +
    +
  • diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-group-header.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-group-header.underscore index e9cac34abf..b5e441859e 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort-group-header.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/cohort-group-header.underscore @@ -18,9 +18,4 @@ <%- gettext("What does this mean?") %> <% } %> -
    - <% if (studioAdvancedSettingsUrl !== "None") { %> - <%- gettext("Edit settings in Studio") %> - <% } %> -
    - \ No newline at end of file + diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort_management.html b/lms/templates/instructor/instructor_dashboard_2/cohort_management.html index 715d16e3d1..0b3ca814cd 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohort_management.html +++ b/lms/templates/instructor/instructor_dashboard_2/cohort_management.html @@ -7,9 +7,9 @@
    @@ -34,7 +34,7 @@ ]; (function (require) { - require(['js/factories/cohorts_factory'], function (CohortsFactory) { + require(['js/groups/views/cohorts_dashboard_factory'], function (CohortsFactory) { CohortsFactory(contentGroups, '${get_studio_url(course, 'group_configurations') | h}'); }); }).call(this, require || RequireJS.require); diff --git a/lms/templates/instructor/instructor_dashboard_2/cohorts.underscore b/lms/templates/instructor/instructor_dashboard_2/cohorts.underscore index 3c5e28d5fe..26925bb7d7 100644 --- a/lms/templates/instructor/instructor_dashboard_2/cohorts.underscore +++ b/lms/templates/instructor/instructor_dashboard_2/cohorts.underscore @@ -47,5 +47,15 @@ ) %>

    + +
    + + + <%- gettext('Specify whether discussion topics are divided by cohort') %> + + <% } %> diff --git a/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html b/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html index 9cac21bb1d..6148f285d3 100644 --- a/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html +++ b/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html @@ -48,6 +48,8 @@ + + <%static:js group='module-descriptor-js'/> <%static:js group='instructor_dash'/> <%static:js group='application'/> @@ -63,6 +65,10 @@ + + + + @@ -71,7 +77,7 @@ ## Include Underscore templates <%block name="header_extras"> -% for template_name in ["cohorts", "cohort-editor", "cohort-group-header", "cohort-selector", "cohort-form", "notification", "cohort-state"]: +% 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"]: diff --git a/lms/urls.py b/lms/urls.py index 445aecfdcd..97a9a74e4e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -396,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), diff --git a/openedx/core/djangoapps/course_groups/tests/test_views.py b/openedx/core/djangoapps/course_groups/tests/test_views.py index 95bec032c0..900ce98889 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_views.py +++ b/openedx/core/djangoapps/course_groups/tests/test_views.py @@ -6,20 +6,22 @@ Tests for course group views import json from collections import namedtuple +from datetime import datetime 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 opaque_keys.edx.locations import SlashSeparatedCourseKey +from xmodule.modulestore.tests.factories import ItemFactory from ..models import CourseUserGroup, CourseCohort from ..views import ( course_cohort_settings_handler, cohort_handler, users_in_cohort, add_users_to_cohort, remove_user_from_cohort, - link_cohort_to_partition_group + link_cohort_to_partition_group, cohort_discussion_topics ) from ..cohorts import ( get_cohort, get_cohort_by_name, get_cohort_by_id, @@ -91,6 +93,33 @@ 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 + + # 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 `handler` for a given `course` and return its response as a dict. @@ -121,56 +150,143 @@ class CohortViewsTestCase(ModuleStoreTestCase): 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, "PUT", [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_discussions = ['Topic A', 'Topic B'] - config_course_cohorts(self.course, is_cohorted=True, cohorted_discussions=cohorted_discussions) + cohorted_inline_discussions, cohorted_course_wide_discussions = self.create_cohorted_discussions() response = self.get_handler(self.course, handler=course_cohort_settings_handler) - response['cohorted_discussions'].sort() + expected_response = self.get_expected_response() - expected_response = { - 'is_cohorted': True, - 'always_cohort_inline_discussions': True, - 'cohorted_discussions': [topic_name_to_id(self.course, name) for name in cohorted_discussions], - 'id': 1 - } - expected_response['cohorted_discussions'].sort() + 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_settings(self): + def test_update_is_cohorted_settings(self): """ - Verify that course_cohort_settings_handler is working for HTTP POST. + 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 = { - 'is_cohorted': True, - 'always_cohort_inline_discussions': True, - 'cohorted_discussions': [], - 'id': 1 - } + expected_response = self.get_expected_response() + self.assertEqual(response, expected_response) expected_response['is_cohorted'] = False - response = self.put_handler(self.course, data=expected_response, handler=course_cohort_settings_handler) + 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) @@ -180,7 +296,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase): """ config_course_cohorts(self.course, is_cohorted=True) - response = self.put_handler(self.course, expected_response_code=400, handler=course_cohort_settings_handler) + 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): @@ -189,7 +305,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase): """ config_course_cohorts(self.course, is_cohorted=True) - response = self.put_handler( + response = self.patch_handler( self.course, data={'is_cohorted': ''}, expected_response_code=400, @@ -1075,3 +1191,58 @@ 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) + + +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) diff --git a/openedx/core/djangoapps/course_groups/views.py b/openedx/core/djangoapps/course_groups/views.py index e3c89f5345..f49e278e4e 100644 --- a/openedx/core/djangoapps/course_groups/views.py +++ b/openedx/core/djangoapps/course_groups/views.py @@ -22,6 +22,7 @@ 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__) @@ -61,14 +62,19 @@ def unlink_cohort_partition_group(cohort): # pylint: disable=invalid-name -def _get_course_cohort_settings_representation(course_cohort_settings): +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_discussions': course_cohort_settings.cohorted_discussions, + '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, } @@ -89,7 +95,26 @@ def _get_cohort_representation(cohort, course): } -@require_http_methods(("GET", "PUT", "POST")) +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, 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 @@ -99,27 +124,49 @@ def course_cohort_settings_handler(request, course_key_string): This will raise 404 if user is not staff. GET Returns the JSON representation of cohort settings for the course. - PUT or POST + PATCH Updates the cohort settings for the course. Returns the JSON representation of updated settings. """ course_key = CourseKey.from_string(course_key_string) - get_course_with_access(request.user, 'staff', course_key) - if request.method == 'GET': - cohort_settings = cohorts.get_course_cohort_settings(course_key) - return JsonResponse(_get_course_cohort_settings_representation(cohort_settings)) - else: - is_cohorted = request.json.get('is_cohorted') - if is_cohorted is None: - # Note: error message not translated because it is not exposed to the user (UI prevents this state). - return JsonResponse({"error": "Bad Request"}, 400) + 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, is_cohorted=is_cohorted) + 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(cohort_settings)) + return JsonResponse(_get_course_cohort_settings_representation(course, cohort_settings)) @require_http_methods(("GET", "PUT", "POST", "PATCH")) @@ -362,3 +409,79 @@ def debug_cohort_mgmt(request, course_key_string): kwargs={'course_key': course_key.to_deprecated_string()} )} return render_to_response('/course_groups/debug.html', context) + + +@expect_json +@login_required +def cohort_discussion_topics(request, course_key_string): + """ + The handler for cohort discussion categories requests. + This will raise 404 if user is not staff. + + Returns the JSON representation of discussion topics w.r.t categories for the course. + + Example: + >>> example = { + >>> "course_wide_discussions": { + >>> "entries": { + >>> "General": { + >>> "sort_key": "General", + >>> "is_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, 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)