feat: Adds discussions settings for new discusions experience [BD-38] [TNL-8621] [BB-4854]

cms/djangoapps/models/settings/course_metadata.py

@@ -75,6 +75,7 @@ class CourseMetadata:
         'default_tab',
         'highlights_enabled_for_messaging',
         'is_onboarding_exam',
+        'discussions_settings',
     ]
 
     @classmethod

cms/envs/common.py

@@ -1512,7 +1512,6 @@
 
     # Discussion
     'openedx.core.djangoapps.django_comment_common',
-    'openedx.core.djangoapps.discussions',
 
     # for course creator table
     'django.contrib.admin',

common/lib/xmodule/xmodule/course_module.py

@@ -406,6 +406,16 @@ class CourseFields:  # lint-amnesty, pylint: disable=missing-class-docstring
             "If false, they are sorted chronologically by creation date and time."
         )
     )
+    discussions_settings = Dict(
+        display_name=_("Discussions Plugin Settings"),
+        scope=Scope.settings,
+        help=_("Settings for discussions plugins."),
+        default={
+            "enable_in_context": True,
+            "enable_graded_units": False,
+            "unit_level_visibility": False,
+        }
+    )
     announcement = Date(
         display_name=_("Course Announcement Date"),
         help=_("Enter the date to announce your course."),

lms/djangoapps/courseware/tests/test_self_paced_overrides.py

@@ -10,7 +10,7 @@
 from lms.djangoapps.courseware.access import has_access
 from lms.djangoapps.ccx.tests.test_overrides import inject_field_overrides
 from lms.djangoapps.courseware.field_overrides import OverrideFieldData, OverrideModulestoreFieldData
-from lms.djangoapps.discussion.django_comment_client.utils import get_accessible_discussion_xblocks
+from openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 

lms/djangoapps/discussion/django_comment_client/tests/test_utils.py

@@ -30,6 +30,13 @@
 from openedx.core.djangoapps.course_groups import cohorts
 from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted
 from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory, config_course_cohorts
+from openedx.core.djangoapps.discussions.utils import (
+    available_division_schemes,
+    get_accessible_discussion_xblocks,
+    get_discussion_categories_ids,
+    get_group_names_by_id,
+    has_required_keys,
+)
 from openedx.core.djangoapps.django_comment_common.comment_client.utils import (
     CommentClientMaintenanceError,
     perform_request,
@@ -200,7 +207,7 @@ def test_get_accessible_discussion_xblocks(self, modulestore_type, expected_disc
         assert test_discussion.location not in self.store.get_orphans(course.id)
 
         # Assert that there is only one discussion xblock in the course at the moment.
-        assert len(utils.get_accessible_discussion_xblocks(course, self.user)) == 1
+        assert len(get_accessible_discussion_xblocks(course, self.user)) == 1
 
         # The above call is request cached, so we need to clear it for this test.
         RequestCache.clear_all_namespaces()
@@ -211,7 +218,7 @@ def test_get_accessible_discussion_xblocks(self, modulestore_type, expected_disc
         # Assert that the discussion xblock is an orphan.
         assert orphan in self.store.get_orphans(course.id)
 
-        assert len(utils.get_accessible_discussion_xblocks(course, self.user)) == expected_discussion_xblocks
+        assert len(get_accessible_discussion_xblocks(course, self.user)) == expected_discussion_xblocks
 
 
 class CachedDiscussionIdMapTestCase(ModuleStoreTestCase):
@@ -277,8 +284,8 @@ def test_cache_raises_exception_if_discussion_id_not_cached(self):
             utils.get_cached_discussion_key(self.course.id, 'test_discussion_id')
 
     def test_xblock_does_not_have_required_keys(self):
-        assert utils.has_required_keys(self.discussion)
-        assert not utils.has_required_keys(self.bad_discussion)
+        assert has_required_keys(self.discussion)
+        assert not has_required_keys(self.bad_discussion)
 
     def verify_discussion_metadata(self):
         """Retrieves the metadata for self.discussion and self.discussion2 and verifies that it is correct"""
@@ -988,16 +995,15 @@ def test_sort_intermediates(self):
         )
 
     def test_ids_empty(self):
-        assert utils.get_discussion_categories_ids(self.course, self.user) == []
+        assert get_discussion_categories_ids(self.course, self.user) == []
 
     def test_ids_configured_topics(self):
         self.course.discussion_topics = {
             "Topic A": {"id": "Topic_A"},
             "Topic B": {"id": "Topic_B"},
             "Topic C": {"id": "Topic_C"}
         }
-        assert len(utils.get_discussion_categories_ids(self.course, self.user)) ==\
-               len(["Topic_A", "Topic_B", "Topic_C"])
+        assert len(get_discussion_categories_ids(self.course, self.user)) == len(["Topic_A", "Topic_B", "Topic_C"])
 
     def test_ids_inline(self):
         self.create_discussion("Chapter 1", "Discussion 1")
@@ -1006,7 +1012,7 @@ def test_ids_inline(self):
         self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
         self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion")
         self.create_discussion("Chapter 3 / Section 1", "Discussion")
-        assert len(utils.get_discussion_categories_ids(self.course, self.user)) ==\
+        assert len(get_discussion_categories_ids(self.course, self.user)) == \
                len(["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"])
 
     def test_ids_mixed(self):
@@ -1018,7 +1024,7 @@ def test_ids_mixed(self):
         self.create_discussion("Chapter 1", "Discussion 1")
         self.create_discussion("Chapter 2", "Discussion")
         self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
-        assert len(utils.get_discussion_categories_ids(self.course, self.user)) ==\
+        assert len(get_discussion_categories_ids(self.course, self.user)) == \
                len(["Topic_A", "Topic_B", "Topic_C", "discussion1", "discussion2", "discussion3"])
 
 
@@ -1421,34 +1427,34 @@ def setUp(self):
     def test_discussion_division_disabled(self):
         course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
         assert not utils.course_discussion_division_enabled(course_discussion_settings)
-        assert [] == utils.available_division_schemes(self.course.id)
+        assert [] == available_division_schemes(self.course.id)
 
     def test_discussion_division_by_cohort(self):
         set_discussion_division_settings(
             self.course.id, enable_cohorts=False, division_scheme=CourseDiscussionSettings.COHORT
         )
         # Because cohorts are disabled, discussion division is not enabled.
         assert not utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
-        assert [] == utils.available_division_schemes(self.course.id)
+        assert [] == available_division_schemes(self.course.id)
         # Now enable cohorts, which will cause discussions to be divided.
         set_discussion_division_settings(
             self.course.id, enable_cohorts=True, division_scheme=CourseDiscussionSettings.COHORT
         )
         assert utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
-        assert [CourseDiscussionSettings.COHORT] == utils.available_division_schemes(self.course.id)
+        assert [CourseDiscussionSettings.COHORT] == available_division_schemes(self.course.id)
 
     def test_discussion_division_by_enrollment_track(self):
         set_discussion_division_settings(
             self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK
         )
         # Only a single enrollment track exists, so discussion division is not enabled.
         assert not utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
-        assert [] == utils.available_division_schemes(self.course.id)
+        assert [] == available_division_schemes(self.course.id)
 
         # Now create a second CourseMode, which will cause discussions to be divided.
         CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED)
         assert utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
-        assert [CourseDiscussionSettings.ENROLLMENT_TRACK] == utils.available_division_schemes(self.course.id)
+        assert [CourseDiscussionSettings.ENROLLMENT_TRACK] == available_division_schemes(self.course.id)
 
 
 class GroupNameTestCase(ModuleStoreTestCase):
@@ -1472,15 +1478,15 @@ def setUp(self):
 
     def test_discussion_division_disabled(self):
         course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
-        assert {} == utils.get_group_names_by_id(course_discussion_settings)
+        assert {} == get_group_names_by_id(course_discussion_settings)
         assert utils.get_group_name((- 1000), course_discussion_settings) is None
 
     def test_discussion_division_by_cohort(self):
         set_discussion_division_settings(
             self.course.id, enable_cohorts=True, division_scheme=CourseDiscussionSettings.COHORT
         )
         course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
-        assert {self.test_cohort_1.id: self.test_cohort_1.name, self.test_cohort_2.id: self.test_cohort_2.name} == utils.get_group_names_by_id(course_discussion_settings)
+        assert {self.test_cohort_1.id: self.test_cohort_1.name, self.test_cohort_2.id: self.test_cohort_2.name} == get_group_names_by_id(course_discussion_settings)
         assert self.test_cohort_2.name == utils.get_group_name(self.test_cohort_2.id, course_discussion_settings)
         # Test also with a group_id that doesn't exist.
         assert utils.get_group_name((- 1000), course_discussion_settings) is None
@@ -1490,7 +1496,7 @@ def test_discussion_division_by_enrollment_track(self):
             self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK
         )
         course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
-        assert {(- 1): 'audit course', (- 2): 'verified course'} == utils.get_group_names_by_id(course_discussion_settings)
+        assert {(- 1): 'audit course', (- 2): 'verified course'} == get_group_names_by_id(course_discussion_settings)
 
         assert 'verified course' == utils.get_group_name((- 2), course_discussion_settings)
         # Test also with a group_id that doesn't exist.

lms/djangoapps/discussion/django_comment_client/utils.py

@@ -24,7 +24,15 @@
     has_permission
 )
 from lms.djangoapps.discussion.django_comment_client.settings import MAX_COMMENT_DEPTH
-from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, get_cohort_names, is_course_cohorted
+from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id
+from openedx.core.djangoapps.discussions.utils import (
+    get_accessible_discussion_xblocks,
+    get_accessible_discussion_xblocks_by_course_id,
+    get_course_division_scheme,
+    get_discussion_categories_ids,
+    get_group_names_by_id,
+    has_required_keys,
+)
 from openedx.core.djangoapps.django_comment_common.models import (
     FORUM_ROLE_COMMUNITY_TA,
     FORUM_ROLE_STUDENT,
@@ -116,45 +124,6 @@ def is_user_community_ta(user, course_id):
     return has_forum_access(user, course_id, FORUM_ROLE_COMMUNITY_TA)
 
 
-def has_required_keys(xblock):
-    """
-    Returns True iff xblock has the proper attributes for generating metadata
-    with get_discussion_id_map_entry()
-    """
-    for key in ('discussion_id', 'discussion_category', 'discussion_target'):
-        if getattr(xblock, key, None) is None:
-            log.debug(
-                "Required key '%s' not in discussion %s, leaving out of category map",
-                key,
-                xblock.location
-            )
-            return False
-    return True
-
-
-def get_accessible_discussion_xblocks(course, user, include_all=False):
-    """
-    Return a list of all valid discussion xblocks in this course that
-    are accessible to the given user.
-    """
-    include_all = getattr(user, 'is_community_ta', False)
-    return get_accessible_discussion_xblocks_by_course_id(course.id, user, include_all=include_all)
-
-
-@request_cached()
-def get_accessible_discussion_xblocks_by_course_id(course_id, user=None, include_all=False):  # pylint: disable=invalid-name
-    """
-    Return a list of all valid discussion xblocks in this course.
-    Checks for the given user's access if include_all is False.
-    """
-    all_xblocks = modulestore().get_items(course_id, qualifiers={'category': 'discussion'}, include_orphans=False)
-
-    return [
-        xblock for xblock in all_xblocks
-        if has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course_id))
-    ]
-
-
 def get_discussion_id_map_entry(xblock):
     """
     Returns a tuple of (discussion_id, metadata) suitable for inclusion in the results of get_discussion_id_map().
@@ -465,23 +434,6 @@ def discussion_category_id_access(course, user, discussion_id, xblock=None):
         return discussion_id in 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 = [
-        xblock.discussion_id for xblock in get_accessible_discussion_xblocks(course, user, include_all=include_all)
-    ]
-    return course.top_level_discussion_topic_ids + accessible_discussion_ids
-
-
 class JsonResponse(HttpResponse):
     """
     Django response object delivering JSON representations
@@ -881,7 +833,7 @@ def get_group_id_for_user(user, course_discussion_settings):
     If discussions are not divided, this method will return None.
     It will also return None if the user is in no group within the specified division_scheme.
     """
-    division_scheme = _get_course_division_scheme(course_discussion_settings)
+    division_scheme = get_course_division_scheme(course_discussion_settings)
     if division_scheme == CourseDiscussionSettings.COHORT:
         return get_cohort_id(user, course_discussion_settings.course_id)
     elif division_scheme == CourseDiscussionSettings.ENROLLMENT_TRACK:
@@ -960,51 +912,7 @@ def course_discussion_division_enabled(course_discussion_settings):
 
     Returns: True if discussion division is enabled for the course, else False
     """
-    return _get_course_division_scheme(course_discussion_settings) != CourseDiscussionSettings.NONE
-
-
-def available_division_schemes(course_key):
-    """
-    Returns a list of possible discussion division schemes for this course.
-    This takes into account if cohorts are enabled and if there are multiple
-    enrollment tracks. If no schemes are available, returns an empty list.
-    Args:
-        course_key: CourseKey
-
-    Returns: list of possible division schemes (for example, CourseDiscussionSettings.COHORT)
-    """
-    available_schemes = []
-    if is_course_cohorted(course_key):
-        available_schemes.append(CourseDiscussionSettings.COHORT)
-    if enrollment_track_group_count(course_key) > 1:
-        available_schemes.append(CourseDiscussionSettings.ENROLLMENT_TRACK)
-    return available_schemes
-
-
-def enrollment_track_group_count(course_key):
-    """
-    Returns the count of possible enrollment track division schemes for this course.
-    Args:
-        course_key: CourseKey
-    Returns:
-        Count of enrollment track division scheme
-    """
-    return len(_get_enrollment_track_groups(course_key))
-
-
-def _get_course_division_scheme(course_discussion_settings):
-    division_scheme = course_discussion_settings.division_scheme
-    if (
-        division_scheme == CourseDiscussionSettings.COHORT and
-        not is_course_cohorted(course_discussion_settings.course_id)
-    ):
-        division_scheme = CourseDiscussionSettings.NONE
-    elif (
-        division_scheme == CourseDiscussionSettings.ENROLLMENT_TRACK and
-        enrollment_track_group_count(course_discussion_settings.course_id) <= 1
-    ):
-        division_scheme = CourseDiscussionSettings.NONE
-    return division_scheme
+    return get_course_division_scheme(course_discussion_settings) != CourseDiscussionSettings.NONE
 
 
 def get_group_name(group_id, course_discussion_settings):
@@ -1023,38 +931,6 @@ def get_group_name(group_id, course_discussion_settings):
     return group_names_by_id[group_id] if group_id in group_names_by_id else None
 
 
-def get_group_names_by_id(course_discussion_settings):
-    """
-    Creates of a dict of group_id to learner-facing group names, for the division_scheme
-    in use as specified by course_discussion_settings.
-    Args:
-        course_discussion_settings: CourseDiscussionSettings model instance
-
-    Returns: dict of group_id to learner-facing group names. If no division_scheme
-    is in use, returns an empty dict.
-    """
-    division_scheme = _get_course_division_scheme(course_discussion_settings)
-    course_key = course_discussion_settings.course_id
-    if division_scheme == CourseDiscussionSettings.COHORT:
-        return get_cohort_names(get_course_by_id(course_key))
-    elif division_scheme == CourseDiscussionSettings.ENROLLMENT_TRACK:
-        # We negate the group_ids from dynamic partitions so that they will not conflict
-        # with cohort IDs (which are an auto-incrementing integer field, starting at 1).
-        return {-1 * group.id: group.name for group in _get_enrollment_track_groups(course_key)}
-    else:
-        return {}
-
-
-def _get_enrollment_track_groups(course_key):
-    """
-    Helper method that returns an array of the Groups in the EnrollmentTrackUserPartition for the given course.
-    If no such partition exists on the course, an empty array is returned.
-    """
-    partition_service = PartitionService(course_key)
-    partition = partition_service.get_user_partition(ENROLLMENT_TRACK_PARTITION_ID)
-    return partition.groups if partition else []
-
-
 def _verify_group_exists(group_id, course_discussion_settings):
     """
     Helper method that verifies the given group_id corresponds to a Group in the

lms/djangoapps/discussion/rest_api/api.py

@@ -19,6 +19,7 @@
 
 from lms.djangoapps.courseware.courses import get_course_with_access
 from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
+from openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks
 from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment
 from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
 from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError
@@ -62,7 +63,6 @@
     track_voted_event,
 )
 from ..django_comment_client.utils import (
-    get_accessible_discussion_xblocks,
     get_group_id_for_user,
     is_commentable_divided,
 )