Merge pull request #9143 from edx/benmcmorran/move-is-commentable-cohorted
Move is_commentable_cohorted to django_comment_client
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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."
|
||||
)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user