diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 549e827256..4831bce834 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -31,11 +31,11 @@ from django_comment_client.base.views import ( get_thread_created_event_data, track_forum_event, ) -from django_comment_client.utils import get_accessible_discussion_modules +from django_comment_client.utils import get_accessible_discussion_modules, is_commentable_cohorted from lms.lib.comment_client.comment import Comment from lms.lib.comment_client.thread import Thread from lms.lib.comment_client.utils import CommentClientRequestError -from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, is_commentable_cohorted +from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id def _get_course_or_404(course_key, user): diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 1521b0d303..4966806a3b 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -23,7 +23,6 @@ from openedx.core.djangoapps.course_groups.cohorts import ( is_course_cohorted, get_cohort_id, get_course_cohorts, - is_commentable_cohorted ) from courseware.tabs import EnrolledTab from courseware.access import has_access @@ -36,7 +35,8 @@ from django_comment_client.utils import ( extract, strip_none, add_courseware_context, - get_group_id_for_comments_service + get_group_id_for_comments_service, + is_commentable_cohorted ) import django_comment_client.utils as utils import lms.lib.comment_client as cc diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 83a45bfb6c..c1dc4bcaee 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -16,12 +16,16 @@ import django_comment_client.utils as utils from courseware.tests.factories import InstructorFactory from courseware.tabs import get_course_tab_list +from openedx.core.djangoapps.course_groups import cohorts from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings +from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts, topic_name_to_id from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.util.testing import ContentGroupTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.django import modulestore +from opaque_keys.edx.locator import CourseLocator @attr('shard_1') @@ -1100,3 +1104,108 @@ class DiscussionTabTestCase(ModuleStoreTestCase): with self.settings(FEATURES={'CUSTOM_COURSES_EDX': True}): self.assertFalse(self.discussion_tab_present(self.enrolled_user)) + + +class IsCommentableCohortedTestCase(ModuleStoreTestCase): + """ + Test the is_commentable_cohorted function. + """ + + MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE + + def setUp(self): + """ + Make sure that course is reloaded every time--clear out the modulestore. + """ + super(IsCommentableCohortedTestCase, self).setUp() + self.toy_course_key = CourseLocator("edX", "toy", "2012_Fall", deprecated=True) + + def test_is_commentable_cohorted(self): + course = modulestore().get_course(self.toy_course_key) + self.assertFalse(cohorts.is_course_cohorted(course.id)) + + def to_id(name): + """Helper for topic_name_to_id that uses course.""" + return topic_name_to_id(course, name) + + # no topics + self.assertFalse( + utils.is_commentable_cohorted(course.id, to_id("General")), + "Course doesn't even have a 'General' topic" + ) + + # not cohorted + config_course_cohorts(course, is_cohorted=False, discussion_topics=["General", "Feedback"]) + + self.assertFalse( + utils.is_commentable_cohorted(course.id, to_id("General")), + "Course isn't cohorted" + ) + + # cohorted, but top level topics aren't + config_course_cohorts(course, is_cohorted=True, discussion_topics=["General", "Feedback"]) + + self.assertTrue(cohorts.is_course_cohorted(course.id)) + self.assertFalse( + utils.is_commentable_cohorted(course.id, to_id("General")), + "Course is cohorted, but 'General' isn't." + ) + + # cohorted, including "Feedback" top-level topics aren't + config_course_cohorts( + course, + is_cohorted=True, + discussion_topics=["General", "Feedback"], + cohorted_discussions=["Feedback"] + ) + + self.assertTrue(cohorts.is_course_cohorted(course.id)) + self.assertFalse( + utils.is_commentable_cohorted(course.id, to_id("General")), + "Course is cohorted, but 'General' isn't." + ) + self.assertTrue( + utils.is_commentable_cohorted(course.id, to_id("Feedback")), + "Feedback was listed as cohorted. Should be." + ) + + def test_is_commentable_cohorted_inline_discussion(self): + course = modulestore().get_course(self.toy_course_key) + self.assertFalse(cohorts.is_course_cohorted(course.id)) + + def to_id(name): # pylint: disable=missing-docstring + return topic_name_to_id(course, name) + + config_course_cohorts( + course, + is_cohorted=True, + discussion_topics=["General", "Feedback"], + cohorted_discussions=["Feedback", "random_inline"] + ) + self.assertTrue( + utils.is_commentable_cohorted(course.id, to_id("random")), + "By default, Non-top-level discussion is always cohorted in cohorted courses." + ) + + # if always_cohort_inline_discussions is set to False, non-top-level discussion are always + # non cohorted unless they are explicitly set in cohorted_discussions + config_course_cohorts( + course, + is_cohorted=True, + discussion_topics=["General", "Feedback"], + cohorted_discussions=["Feedback", "random_inline"], + always_cohort_inline_discussions=False + ) + self.assertFalse( + utils.is_commentable_cohorted(course.id, to_id("random")), + "Non-top-level discussion is not cohorted if always_cohort_inline_discussions is False." + ) + self.assertTrue( + utils.is_commentable_cohorted(course.id, to_id("random_inline")), + "If always_cohort_inline_discussions set to False, Non-top-level discussion is " + "cohorted if explicitly set in cohorted_discussions." + ) + self.assertTrue( + utils.is_commentable_cohorted(course.id, to_id("Feedback")), + "If always_cohort_inline_discussions set to False, top-level discussion are not affected." + ) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index f403be3c9d..a91fa4b9cb 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -19,10 +19,11 @@ from django_comment_client.permissions import check_permissions_by_view, has_per from django_comment_client.settings import MAX_COMMENT_DEPTH from edxmako import lookup_template +from courseware import courses from courseware.access import has_access from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.course_groups.cohorts import ( - get_course_cohort_settings, get_cohort_by_id, get_cohort_id, is_commentable_cohorted, is_course_cohorted + get_course_cohort_settings, get_cohort_by_id, get_cohort_id, is_course_cohorted ) from openedx.core.djangoapps.course_groups.models import CourseUserGroup @@ -660,3 +661,37 @@ def is_comment_too_deep(parent): (parent and parent["depth"] >= MAX_COMMENT_DEPTH) ) ) + + +def is_commentable_cohorted(course_key, commentable_id): + """ + Args: + course_key: CourseKey + commentable_id: string + + Returns: + Bool: is this commentable cohorted? + + Raises: + Http404 if the course doesn't exist. + """ + course = courses.get_course_by_id(course_key) + course_cohort_settings = get_course_cohort_settings(course_key) + + if not course_cohort_settings.is_cohorted: + # this is the easy case :) + ans = False + elif ( + commentable_id in course.top_level_discussion_topic_ids or + course_cohort_settings.always_cohort_inline_discussions is False + ): + # top level discussions have to be manually configured as cohorted + # (default is not). + # Same thing for inline discussions if the default is explicitly set to False in settings + ans = commentable_id in course_cohort_settings.cohorted_discussions + else: + # inline discussions are cohorted by default + ans = True + + log.debug(u"is_commentable_cohorted(%s, %s) = {%s}", course_key, commentable_id, ans) + return ans diff --git a/openedx/core/djangoapps/course_groups/cohorts.py b/openedx/core/djangoapps/course_groups/cohorts.py index 8fd0b23231..aec125daa3 100644 --- a/openedx/core/djangoapps/course_groups/cohorts.py +++ b/openedx/core/djangoapps/course_groups/cohorts.py @@ -121,42 +121,6 @@ def get_cohort_id(user, course_key, use_cached=False): return None if cohort is None else cohort.id -def is_commentable_cohorted(course_key, commentable_id): - """ - Args: - course_key: CourseKey - commentable_id: string - - Returns: - Bool: is this commentable cohorted? - - Raises: - Http404 if the course doesn't exist. - """ - course = courses.get_course_by_id(course_key) - course_cohort_settings = get_course_cohort_settings(course_key) - - if not course_cohort_settings.is_cohorted: - # this is the easy case :) - ans = False - elif ( - commentable_id in course.top_level_discussion_topic_ids or - course_cohort_settings.always_cohort_inline_discussions is False - ): - # top level discussions have to be manually configured as cohorted - # (default is not). - # Same thing for inline discussions if the default is explicitly set to False in settings - ans = commentable_id in course_cohort_settings.cohorted_discussions - else: - # inline discussions are cohorted by default - ans = True - - log.debug(u"is_commentable_cohorted({0}, {1}) = {2}".format( - course_key, commentable_id, ans - )) - return ans - - def get_cohorted_commentables(course_key): """ Given a course_key return a set of strings representing cohorted commentables. diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index 7445e443c4..e07721b585 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -470,95 +470,6 @@ class TestCohorts(ModuleStoreTestCase): {cohort1.id: cohort1.name, cohort2.id: cohort2.name} ) - def test_is_commentable_cohorted(self): - course = modulestore().get_course(self.toy_course_key) - self.assertFalse(cohorts.is_course_cohorted(course.id)) - - def to_id(name): - return topic_name_to_id(course, name) - - # no topics - self.assertFalse( - cohorts.is_commentable_cohorted(course.id, to_id("General")), - "Course doesn't even have a 'General' topic" - ) - - # not cohorted - config_course_cohorts(course, is_cohorted=False, discussion_topics=["General", "Feedback"]) - - self.assertFalse( - cohorts.is_commentable_cohorted(course.id, to_id("General")), - "Course isn't cohorted" - ) - - # cohorted, but top level topics aren't - config_course_cohorts(course, is_cohorted=True, discussion_topics=["General", "Feedback"]) - - self.assertTrue(cohorts.is_course_cohorted(course.id)) - self.assertFalse( - cohorts.is_commentable_cohorted(course.id, to_id("General")), - "Course is cohorted, but 'General' isn't." - ) - - # cohorted, including "Feedback" top-level topics aren't - config_course_cohorts( - course, - is_cohorted=True, - discussion_topics=["General", "Feedback"], - cohorted_discussions=["Feedback"] - ) - - self.assertTrue(cohorts.is_course_cohorted(course.id)) - self.assertFalse( - cohorts.is_commentable_cohorted(course.id, to_id("General")), - "Course is cohorted, but 'General' isn't." - ) - self.assertTrue( - cohorts.is_commentable_cohorted(course.id, to_id("Feedback")), - "Feedback was listed as cohorted. Should be." - ) - - def test_is_commentable_cohorted_inline_discussion(self): - course = modulestore().get_course(self.toy_course_key) - self.assertFalse(cohorts.is_course_cohorted(course.id)) - - def to_id(name): # pylint: disable=missing-docstring - return topic_name_to_id(course, name) - - config_course_cohorts( - course, - is_cohorted=True, - discussion_topics=["General", "Feedback"], - cohorted_discussions=["Feedback", "random_inline"] - ) - self.assertTrue( - cohorts.is_commentable_cohorted(course.id, to_id("random")), - "By default, Non-top-level discussion is always cohorted in cohorted courses." - ) - - # if always_cohort_inline_discussions is set to False, non-top-level discussion are always - # non cohorted unless they are explicitly set in cohorted_discussions - config_course_cohorts( - course, - is_cohorted=True, - discussion_topics=["General", "Feedback"], - cohorted_discussions=["Feedback", "random_inline"], - always_cohort_inline_discussions=False - ) - self.assertFalse( - cohorts.is_commentable_cohorted(course.id, to_id("random")), - "Non-top-level discussion is not cohorted if always_cohort_inline_discussions is False." - ) - self.assertTrue( - cohorts.is_commentable_cohorted(course.id, to_id("random_inline")), - "If always_cohort_inline_discussions set to False, Non-top-level discussion is " - "cohorted if explicitly set in cohorted_discussions." - ) - self.assertTrue( - cohorts.is_commentable_cohorted(course.id, to_id("Feedback")), - "If always_cohort_inline_discussions set to False, top-level discussion are not affected." - ) - def test_get_cohorted_commentables(self): """ Make sure cohorts.get_cohorted_commentables() correctly returns a list of strings representing cohorted