Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "feat: Adds discussions settings for new discusions experience (#28749)"

Browse Source 
This reverts commit bb0c03123c.
This commit is contained in:
David Ormsbee
2021-10-27 09:45:06 -04:00
parent 249f4e6fbf
commit 5d0132305d
23 changed files with 344 additions and 592 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
cms/djangoapps/models/settings/course_metadata.py
View File

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

1
cms/envs/common.py
View File

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

10
common/lib/xmodule/xmodule/course_module.py
View File

@@ -406,16 +406,6 @@ 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."),

2
lms/djangoapps/courseware/tests/test_self_paced_overrides.py
View File

@@ -10,7 +10,7 @@ from common.djangoapps.student.tests.factories import BetaTesterFactory
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 openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks
from lms.djangoapps.discussion.django_comment_client.utils import get_accessible_discussion_xblocks
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory

40
lms/djangoapps/discussion/django_comment_client/tests/test_utils.py
View File

@@ -30,13 +30,6 @@ from lms.djangoapps.teams.tests.factories import CourseTeamFactory
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,
@@ -207,7 +200,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
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(get_accessible_discussion_xblocks(course, self.user)) == 1
assert len(utils.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()
@@ -218,7 +211,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
# Assert that the discussion xblock is an orphan.
assert orphan in self.store.get_orphans(course.id)
assert len(get_accessible_discussion_xblocks(course, self.user)) == expected_discussion_xblocks
assert len(utils.get_accessible_discussion_xblocks(course, self.user)) == expected_discussion_xblocks
class CachedDiscussionIdMapTestCase(ModuleStoreTestCase):
@@ -284,8 +277,8 @@ class CachedDiscussionIdMapTestCase(ModuleStoreTestCase):
utils.get_cached_discussion_key(self.course.id, 'test_discussion_id')
def test_xblock_does_not_have_required_keys(self):
assert has_required_keys(self.discussion)
assert not has_required_keys(self.bad_discussion)
assert utils.has_required_keys(self.discussion)
assert not utils.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"""
@@ -995,7 +988,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
)
def test_ids_empty(self):
assert get_discussion_categories_ids(self.course, self.user) == []
assert utils.get_discussion_categories_ids(self.course, self.user) == []
def test_ids_configured_topics(self):
self.course.discussion_topics = {
@@ -1003,7 +996,8 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
"Topic B": {"id": "Topic_B"},
"Topic C": {"id": "Topic_C"}
}
assert len(get_discussion_categories_ids(self.course, self.user)) == len(["Topic_A", "Topic_B", "Topic_C"])
assert len(utils.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")
@@ -1012,7 +1006,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
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(get_discussion_categories_ids(self.course, self.user)) == \
assert len(utils.get_discussion_categories_ids(self.course, self.user)) ==\
len(["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"])
def test_ids_mixed(self):
@@ -1024,7 +1018,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
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(get_discussion_categories_ids(self.course, self.user)) == \
assert len(utils.get_discussion_categories_ids(self.course, self.user)) ==\
len(["Topic_A", "Topic_B", "Topic_C", "discussion1", "discussion2", "discussion3"])
@@ -1427,7 +1421,7 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
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 [] == available_division_schemes(self.course.id)
assert [] == utils.available_division_schemes(self.course.id)
def test_discussion_division_by_cohort(self):
set_discussion_division_settings(
@@ -1435,13 +1429,13 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
)
# Because cohorts are disabled, discussion division is not enabled.
assert not utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
assert [] == available_division_schemes(self.course.id)
assert [] == utils.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] == available_division_schemes(self.course.id)
assert [CourseDiscussionSettings.COHORT] == utils.available_division_schemes(self.course.id)
def test_discussion_division_by_enrollment_track(self):
set_discussion_division_settings(
@@ -1449,12 +1443,12 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
)
# 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 [] == available_division_schemes(self.course.id)
assert [] == utils.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] == available_division_schemes(self.course.id)
assert [CourseDiscussionSettings.ENROLLMENT_TRACK] == utils.available_division_schemes(self.course.id)
class GroupNameTestCase(ModuleStoreTestCase):
@@ -1478,7 +1472,7 @@ class GroupNameTestCase(ModuleStoreTestCase):
def test_discussion_division_disabled(self):
course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert {} == get_group_names_by_id(course_discussion_settings)
assert {} == utils.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):
@@ -1486,7 +1480,7 @@ class GroupNameTestCase(ModuleStoreTestCase):
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} == 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} == utils.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
@@ -1496,7 +1490,7 @@ class GroupNameTestCase(ModuleStoreTestCase):
self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK
)
course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert {(- 1): 'audit course', (- 2): 'verified course'} == get_group_names_by_id(course_discussion_settings)
assert {(- 1): 'audit course', (- 2): 'verified course'} == utils.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.

146
lms/djangoapps/discussion/django_comment_client/utils.py
View File

@@ -24,15 +24,7 @@ from lms.djangoapps.discussion.django_comment_client.permissions import (
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
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.course_groups.cohorts import get_cohort_id, get_cohort_names, is_course_cohorted
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_STUDENT,
@@ -124,6 +116,45 @@ 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().
@@ -434,6 +465,23 @@ 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
@@ -833,7 +881,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:
@@ -912,7 +960,51 @@ 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
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
def get_group_name(group_id, course_discussion_settings):
@@ -931,6 +1023,38 @@ 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

2
lms/djangoapps/discussion/rest_api/api.py
View File

@@ -19,7 +19,6 @@ from rest_framework.request import Request
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
@@ -63,6 +62,7 @@ from ..django_comment_client.base.views import (
track_voted_event,
)
from ..django_comment_client.utils import (
get_accessible_discussion_xblocks,
get_group_id_for_user,
is_commentable_divided,
)

87
lms/djangoapps/discussion/rest_api/serializers.py
View File

@@ -13,12 +13,13 @@ from rest_framework import serializers
from common.djangoapps.student.models import get_user_by_username_or_email
from lms.djangoapps.discussion.django_comment_client.utils import (
available_division_schemes,
course_discussion_division_enabled,
get_group_id_for_user,
get_group_name,
get_group_names_by_id,
is_comment_too_deep,
)
from openedx.core.djangoapps.discussions.utils import get_group_names_by_id
from lms.djangoapps.discussion.rest_api.permissions import (
NON_UPDATABLE_COMMENT_FIELDS,
NON_UPDATABLE_THREAD_FIELDS,
@@ -26,6 +27,7 @@ from lms.djangoapps.discussion.rest_api.permissions import (
get_editable_fields,
)
from lms.djangoapps.discussion.rest_api.render import render_body
from lms.djangoapps.discussion.views import get_divided_discussions
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.user import User as CommentClientUser
@@ -519,6 +521,89 @@ class DiscussionTopicSerializer(serializers.Serializer):
pass # lint-amnesty, pylint: disable=unnecessary-pass
class DiscussionSettingsSerializer(serializers.Serializer):
"""
Serializer for course discussion settings.
"""
divided_discussions = serializers.ListField(
child=serializers.CharField(),
write_only=True,
)
divided_course_wide_discussions = serializers.ListField(
child=serializers.CharField(),
read_only=True,
)
divided_inline_discussions = serializers.ListField(
child=serializers.CharField(),
read_only=True,
)
always_divide_inline_discussions = serializers.BooleanField()
division_scheme = serializers.CharField()
def to_internal_value(self, data: dict) -> dict:
"""
Transform the *incoming* primitive data into a native value.
"""
payload = super().to_internal_value(data) or {}
course = self.context['course']
instance = self.context['settings']
if any(item in data for item in ('divided_course_wide_discussions', 'divided_inline_discussions')):
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
course, instance
)
divided_course_wide_discussions = data.get(
'divided_course_wide_discussions',
divided_course_wide_discussions
)
divided_inline_discussions = data.get('divided_inline_discussions', divided_inline_discussions)
try:
payload['divided_discussions'] = divided_course_wide_discussions + divided_inline_discussions
except TypeError as error:
raise ValidationError(str(error)) from error
for item in ('always_divide_inline_discussions', 'division_scheme'):
if item in data:
payload[item] = data[item]
return payload
def to_representation(self, instance: CourseDiscussionSettings) -> dict:
"""
Return a serialized representation of the course discussion settings.
"""
payload = super().to_representation(instance)
course = self.context['course']
instance = self.context['settings']
course_key = course.id
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
course, instance
)
payload = {
'id': instance.id,
'divided_inline_discussions': divided_inline_discussions,
'divided_course_wide_discussions': divided_course_wide_discussions,
'always_divide_inline_discussions': instance.always_divide_inline_discussions,
'division_scheme': instance.division_scheme,
'available_division_schemes': available_division_schemes(course_key)
}
return payload
def create(self, validated_data):
"""
This method intentionally left empty
"""
def update(self, instance: CourseDiscussionSettings, validated_data: dict) -> CourseDiscussionSettings:
"""
Update and save an existing instance
"""
if not any(field in validated_data for field in self.fields):
raise ValidationError('Bad request')
try:
instance.update(validated_data)
except ValueError as e:
raise ValidationError(str(e)) from e
return instance
class DiscussionRolesSerializer(serializers.Serializer):
"""
Serializer for course discussion roles.

4
lms/djangoapps/discussion/rest_api/views.py
View File

@@ -16,10 +16,10 @@ from rest_framework.parsers import JSONParser
from rest_framework.response import Response
from rest_framework.views import APIView
from rest_framework.viewsets import ViewSet
from xmodule.modulestore.django import modulestore
from lms.djangoapps.course_goals.models import UserActivity
from lms.djangoapps.instructor.access import update_forum_role
from openedx.core.djangoapps.discussions.serializers import DiscussionSettingsSerializer
from openedx.core.djangoapps.django_comment_common import comment_client
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role
from openedx.core.djangoapps.user_api.accounts.permissions import CanReplaceUsername, CanRetireUser
@@ -27,7 +27,6 @@ from openedx.core.djangoapps.user_api.models import UserRetirementStatus
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.core.lib.api.parsers import MergePatchParser
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
from xmodule.modulestore.django import modulestore
from ..rest_api.api import (
create_comment,
create_thread,
@@ -52,6 +51,7 @@ from ..rest_api.forms import (
from ..rest_api.serializers import (
DiscussionRolesListSerializer,
DiscussionRolesSerializer,
DiscussionSettingsSerializer,
)
log = logging.getLogger(__name__)

2
lms/djangoapps/discussion/tasks.py
View File

@@ -22,9 +22,9 @@ from six.moves.urllib.parse import urljoin
import openedx.core.djangoapps.django_comment_common.comment_client as cc
from common.djangoapps.track import segment
from lms.djangoapps.discussion.django_comment_client.utils import (
get_accessible_discussion_xblocks_by_course_id,
permalink
)
from openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks_by_course_id
from openedx.core.djangoapps.ace_common.message import BaseMessageType
from openedx.core.djangoapps.ace_common.template_context import get_base_template_context
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

34
lms/djangoapps/discussion/views.py
View File

@@ -4,6 +4,7 @@ Views handling read (GET) requests for the Discussion tab and inline discussions
import logging
from functools import wraps
from django.conf import settings
from django.contrib.auth.decorators import login_required
@@ -14,12 +15,12 @@ from django.shortcuts import render
from django.template.context_processors import csrf
from django.template.loader import render_to_string
from django.urls import reverse
from django.utils.translation import get_language_bidi, ugettext_lazy as _
from django.utils.translation import get_language_bidi
from django.utils.translation import ugettext_lazy as _
from django.views.decorators.cache import cache_control
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_GET, require_http_methods
from edx_django_utils.monitoring import function_trace
from functools import wraps
from opaque_keys.edx.keys import CourseKey
from rest_framework import status
from web_fragments.fragment import Fragment
@@ -37,22 +38,18 @@ from lms.djangoapps.discussion.django_comment_client.constants import TYPE_ENTRY
from lms.djangoapps.discussion.django_comment_client.permissions import has_permission
from lms.djangoapps.discussion.django_comment_client.utils import (
add_courseware_context,
available_division_schemes,
course_discussion_division_enabled,
extract,
get_group_id_for_comments_service,
get_group_id_for_user,
get_group_names_by_id,
is_commentable_divided,
strip_none,
)
from lms.djangoapps.discussion.exceptions import TeamDiscussionHiddenFromUserException
from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context
from lms.djangoapps.teams import api as team_api
from openedx.core.djangoapps.discussions.utils import (
available_division_schemes,
get_discussion_categories_ids,
get_divided_discussions,
get_group_names_by_id,
)
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.utils import ThreadContext
from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
@@ -166,7 +163,7 @@ def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS
# If not provided with a discussion id, filter threads by commentable ids
# which are accessible to the current user.
if discussion_id is None:
discussion_category_ids = set(get_discussion_categories_ids(course, request.user))
discussion_category_ids = set(utils.get_discussion_categories_ids(course, request.user))
threads = [
thread for thread in threads
if thread.get('commentable_id') in discussion_category_ids
@@ -960,6 +957,25 @@ def course_discussions_settings_handler(request, course_key_string):
})
def get_divided_discussions(course, discussion_settings):
"""
Returns the course-wide and inline divided discussion ids separately.
"""
divided_course_wide_discussions = []
divided_inline_discussions = []
course_wide_discussions = [topic['id'] for __, topic in course.discussion_topics.items()]
all_discussions = utils.get_discussion_categories_ids(course, None, include_all=True)
for divided_discussion_id in discussion_settings.divided_discussions:
if divided_discussion_id in course_wide_discussions:
divided_course_wide_discussions.append(divided_discussion_id)
elif divided_discussion_id in all_discussions:
divided_inline_discussions.append(divided_discussion_id)
return divided_course_wide_discussions, divided_inline_discussions
def _check_team_discussion_access(request, course, discussion_id):
"""
Helper function to check if the discussion is visible to the user,

3
lms/djangoapps/instructor/views/instructor_dashboard.py
View File

@@ -49,11 +49,10 @@ from lms.djangoapps.certificates.models import (
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.courses import get_studio_url
from lms.djangoapps.courseware.module_render import get_module_by_usage_id
from lms.djangoapps.discussion.django_comment_client.utils import has_forum_access
from lms.djangoapps.discussion.django_comment_client.utils import available_division_schemes, has_forum_access
from lms.djangoapps.grades.api import is_writable_gradebook_enabled
from openedx.core.djangoapps.course_groups.cohorts import DEFAULT_COHORT_NAME, get_course_cohorts, is_course_cohorted
from openedx.core.djangoapps.discussions.config.waffle_utils import legacy_discussion_experience_enabled
from openedx.core.djangoapps.discussions.utils import available_division_schemes
from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, CourseDiscussionSettings
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.verified_track_content.models import VerifiedTrackCohortedCourse

8
openedx/core/djangoapps/discussions/apps.py
View File

@@ -15,15 +15,9 @@ class DiscussionsConfig(AppConfig):
name = 'openedx.core.djangoapps.discussions'
plugin_app = {
PluginURLs.CONFIG: {
# TODO: Remove the LMS path once its usage has been removed from frontend-app-course-authoring.
ProjectType.LMS: {
PluginURLs.NAMESPACE: '',
PluginURLs.REGEX: r'^discussions/api/',
PluginURLs.RELATIVE_PATH: 'urls',
},
ProjectType.CMS: {
PluginURLs.NAMESPACE: '',
PluginURLs.REGEX: r'^api/discussions/',
PluginURLs.REGEX: r'^discussions/',
PluginURLs.RELATIVE_PATH: 'urls',
},
},

20
openedx/core/djangoapps/discussions/docs/decisions/0004-in-context-discussions-linking.rst
View File

@@ -50,7 +50,7 @@ Consideration
-------------
Most of these configuration entries would be right at home in the
``DiscussionsConfiguration`` model in ``plugin_settings``, however since they need
`DiscussionsConfiguration` model in `plugin_settings`, however since they need
to be available during course import-export, they should be stored in the
course object itself.
@@ -59,13 +59,13 @@ using its usage key. This same mechanism can be used to associate a Unit usage
key with a corresponding discussion id.
However the current mechanism has a few issues. It is stored as a JSON
structure in the ``DiscussionsIdMapping`` model which has course id and a mapping
structure in the `DiscussionsIdMapping` model which has course id and a mapping
of the discussion id to the xblock usage key in a single dict.
This is OK for the existing setup because this is just a caching mechanism and
the source of truth for this mapping is the XBlock itself, which stores the
discussion id. On course publish this information is cached to
``DiscussionsIdMapping``.
`DiscussionsIdMapping`.
For the new discussions system though, this mapping would be the source of
truth for the link between discussions and units, so we should use a model
@@ -76,12 +76,12 @@ Decision
Since the discussions settings need to be stored in the course structure we
should create a new JSON structure in the course that matches the structure
of ``plugin_settings``. This can then be used to store not just the settings
of `plugin_settings`. This can then be used to store not just the settings
for the inbuilt discussions provider, but for any discussions provider in the
future.
When a course is published, we can copy over all the ``plugin_settings`` to the
course in a JSON field called ``discussions_settings`` with the following
When a course is published, we can copy over all the `plugin_settings` to the
course in a JSON field called `dicussions_settings` with the following
structure:
.. code-block:: JSON
@@ -95,7 +95,7 @@ structure:
}
}
The ``edx-next`` key here represents the provider id, allowing for potentially
The `edx-next` key here represents the provider id, allowing for potentially
multiple provider configs to coexist in case of switching providers etc.
Settings outside this key are those that are applicable to all providers. Note
that they may not be supported by all providers though, in which case they will
@@ -103,16 +103,16 @@ simply be ignored.
To store Unit-level discussions settings, we can simply add a boolean field
to the Unit block that specifies whether it is discussable or not. To be
consistent with the above names we can call this field ``discussions_enabled``.
consistent with the above names we can call this field `discussions_enabled`.
A signal can be created using the new Hooks extension system proposed in OEP-50
that is triggered when discussions settings change. This signal can encapsulate
all the data needed for setting up discussions from the modulestore. It can
traverse through all teh Units in the course that match the criterion from the
discussions settings and provide the needed details as part of the signal data.
dicussions settings and provide the needed details as part of the signal data.
A handler for the above signal, we create the discussion topics in
``cs_comments_service`` and add a mapping. If an existing unit with discussions
`cs_comments_service` and add a mapping. If an existing unit with discussions
is removed, we will disable the link but not delete the data.
The discussion grouping at subsections will simply combine the topics from all

72
openedx/core/djangoapps/discussions/migrations/0006_auto_20211006_0441.py
View File

@@ -1,72 +0,0 @@
# Generated by Django 2.2.24 on 2021-10-06 04:41
from django.db import migrations, models
import django_mysql.models
class Migration(migrations.Migration):
dependencies = [
('discussions', '0005_auto_20210910_0940'),
]
operations = [
migrations.AddField(
model_name='discussionsconfiguration',
name='enable_graded_units',
field=models.BooleanField(default=False, help_text='If enabled, discussion topics will be created for graded units as well.'),
),
migrations.AddField(
model_name='discussionsconfiguration',
name='enable_in_context',
field=models.BooleanField(default=True, help_text='If enabled, discussion topics will be created for each non-graded unit in the course. A UI for discussions will show up with each unit.'),
),
migrations.AddField(
model_name='discussionsconfiguration',
name='unit_level_visibility',
field=models.BooleanField(default=False, help_text='If enabled, discussions will need to be manually enabled for each unit.'),
),
migrations.AddField(
model_name='historicaldiscussionsconfiguration',
name='enable_graded_units',
field=models.BooleanField(default=False, help_text='If enabled, discussion topics will be created for graded units as well.'),
),
migrations.AddField(
model_name='historicaldiscussionsconfiguration',
name='enable_in_context',
field=models.BooleanField(default=True, help_text='If enabled, discussion topics will be created for each non-graded unit in the course. A UI for discussions will show up with each unit.'),
),
migrations.AddField(
model_name='historicaldiscussionsconfiguration',
name='unit_level_visibility',
field=models.BooleanField(default=False, help_text='If enabled, discussions will need to be manually enabled for each unit.'),
),
migrations.AlterField(
model_name='discussionsconfiguration',
name='provider_type',
field=models.CharField(default='legacy', help_text="The discussion tool/provider's id", max_length=100, verbose_name='Discussion provider'),
),
migrations.AlterField(
model_name='historicaldiscussionsconfiguration',
name='provider_type',
field=models.CharField(default='legacy', help_text="The discussion tool/provider's id", max_length=100, verbose_name='Discussion provider'),
),
migrations.AlterField(
model_name='providerfilter',
name='allow',
field=django_mysql.models.ListCharField(models.CharField(
choices=[('legacy', 'legacy'), ('ed-discuss', 'ed-discuss'), ('inscribe', 'inscribe'),
('piazza', 'piazza'), ('yellowdig', 'yellowdig')], max_length=20), blank=True,
help_text='Comma-separated list of providers to allow, eg: legacy,ed-discuss,inscribe,piazza,yellowdig',
max_length=63, size=3, verbose_name='Allow List'),
),
migrations.AlterField(
model_name='providerfilter',
name='deny',
field=django_mysql.models.ListCharField(models.CharField(
choices=[('legacy', 'legacy'), ('ed-discuss', 'ed-discuss'), ('inscribe', 'inscribe'),
('piazza', 'piazza'), ('yellowdig', 'yellowdig')], max_length=20), blank=True,
help_text='Comma-separated list of providers to deny, eg: legacy,ed-discuss,inscribe,piazza,yellowdig',
max_length=63, size=3, verbose_name='Deny List'),
),
]

44
openedx/core/djangoapps/discussions/models.py
View File

@@ -4,15 +4,14 @@ Provide django models to back the discussions app
from __future__ import annotations
import logging
from enum import Enum
from collections import namedtuple
from typing import List, Type, TypeVar
from django.conf import settings
from django.core.exceptions import ValidationError
from django.db import models
from django.utils.translation import gettext_lazy as _
from django_mysql.models import ListCharField
from enum import Enum
from jsonfield import JSONField
from lti_consumer.models import LtiConfiguration
from model_utils.models import TimeStampedModel
@@ -233,14 +232,17 @@ AVAILABLE_PROVIDER_MAP = {
}
def get_supported_providers() -> List[str]:
def get_supported_providers() -> list[str]:
"""
Return the list of supported discussion providers
TODO: Load this from entry points?
"""
return list(AVAILABLE_PROVIDER_MAP.keys())
providers = [
'legacy',
'piazza',
]
return providers
class ProviderFilter(StackedConfigurationModel):
@@ -302,7 +304,7 @@ class ProviderFilter(StackedConfigurationModel):
)
@property
def available_providers(self) -> List[str]:
def available_providers(self) -> list[str]:
"""
Return a filtered list of available providers
"""
@@ -322,15 +324,12 @@ class ProviderFilter(StackedConfigurationModel):
return _providers
@classmethod
def get_available_providers(cls, course_key: CourseKey) -> List[str]:
def get_available_providers(cls, course_key: CourseKey) -> list[str]:
_filter = cls.current(course_key=course_key)
providers = _filter.available_providers
return providers
T = TypeVar('T', bound='DiscussionsConfiguration')
class DiscussionsConfiguration(TimeStampedModel):
"""
Associates a learning context with discussion provider and configuration
@@ -357,21 +356,6 @@ class DiscussionsConfiguration(TimeStampedModel):
null=True,
help_text=_("The LTI configuration data for this context/provider."),
)
enable_in_context = models.BooleanField(
default=True,
help_text=_(
"If enabled, discussion topics will be created for each non-graded unit in the course. "
"A UI for discussions will show up with each unit."
)
)
enable_graded_units = models.BooleanField(
default=False,
help_text=_("If enabled, discussion topics will be created for graded units as well.")
)
unit_level_visibility = models.BooleanField(
default=False,
help_text=_("If enabled, discussions will need to be manually enabled for each unit.")
)
plugin_configuration = JSONField(
blank=True,
default={},
@@ -382,7 +366,6 @@ class DiscussionsConfiguration(TimeStampedModel):
max_length=100,
verbose_name=_("Discussion provider"),
help_text=_("The discussion tool/provider's id"),
default=DEFAULT_PROVIDER_TYPE,
)
history = HistoricalRecords()
@@ -424,8 +407,9 @@ class DiscussionsConfiguration(TimeStampedModel):
configuration = cls.get(context_key)
return configuration.enabled
# pylint: disable=undefined-variable
@classmethod
def get(cls: Type[T], context_key: CourseKey) -> T:
def get(cls, context_key: CourseKey) -> cls:
"""
Lookup a model by context_key
"""
@@ -439,12 +423,14 @@ class DiscussionsConfiguration(TimeStampedModel):
)
return configuration
# pylint: enable=undefined-variable
@property
def available_providers(self) -> List[str]:
def available_providers(self) -> list[str]:
return ProviderFilter.current(course_key=self.context_key).available_providers
@classmethod
def get_available_providers(cls, context_key: CourseKey) -> List[str]:
def get_available_providers(cls, context_key: CourseKey) -> list[str]:
return ProviderFilter.current(course_key=context_key).available_providers
@classmethod

171
openedx/core/djangoapps/discussions/serializers.py
View File

@@ -1,16 +1,17 @@
"""
Serializers for Discussion views.
"""
from django.core.exceptions import ValidationError
from lti_consumer.api import get_lti_pii_sharing_state_for_course
from lti_consumer.models import LtiConfiguration
from opaque_keys.edx.keys import CourseKey
from rest_framework import serializers
from xmodule.modulestore.django import modulestore
from lms.djangoapps.discussion.rest_api.serializers import DiscussionSettingsSerializer
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore
from .models import AVAILABLE_PROVIDER_MAP, DEFAULT_PROVIDER_TYPE, DiscussionsConfiguration, Features
from .utils import available_division_schemes, get_divided_discussions
class LtiSerializer(serializers.ModelSerializer):
@@ -171,23 +172,10 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
class Meta:
model = DiscussionsConfiguration
course_fields = [
'provider_type',
'enable_in_context',
'enable_graded_units',
'unit_level_visibility',
]
fields = [
'enabled',
] + course_fields
def _get_course(self):
"""
Get course and save it in the context, so it doesn't need to be reloaded.
"""
if self.context.get('course') is None:
self.context['course'] = get_course_by_id(self.instance.context_key)
return self.context['course']
'provider_type',
]
def create(self, validated_data):
"""
@@ -199,11 +187,13 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
"""
Transform the *incoming* primitive data into a native value.
"""
payload = super().to_internal_value(data)
payload.update({
payload = {
'context_key': data.get('course_key', ''),
'enabled': data.get('enabled', False),
'lti_configuration': data.get('lti_configuration', {}),
'plugin_configuration': data.get('plugin_configuration', {}),
})
'provider_type': data.get('provider_type', DEFAULT_PROVIDER_TYPE),
}
return payload
def to_representation(self, instance: DiscussionsConfiguration) -> dict:
@@ -247,17 +237,14 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
"""
Update and save an existing instance
"""
# This needs to check which fields have changed, so do it before
# fields are copied over.
instance = self._update_course_configuration(instance, validated_data)
instance = self._update_plugin_configuration(instance, validated_data)
for key in self.Meta.fields:
value = validated_data.get(key)
if value is not None:
setattr(instance, key, value)
# _update_* helpers assume `enabled` and `provider_type`
# have already been set
instance = self._update_lti(instance, validated_data)
instance = self._update_lti(instance, validated_data, instance.context_key)
instance = self._update_plugin_configuration(instance, validated_data)
instance.save()
return instance
@@ -265,6 +252,7 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
self,
instance: DiscussionsConfiguration,
validated_data: dict,
course_key: CourseKey
) -> DiscussionsConfiguration:
"""
Update LtiConfiguration
@@ -280,7 +268,7 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
data=lti_configuration_data,
partial=True,
context={
'pii_sharing_allowed': get_lti_pii_sharing_state_for_course(instance.context_key),
'pii_sharing_allowed': get_lti_pii_sharing_state_for_course(course_key),
}
)
if lti_serializer.is_valid(raise_exception=True):
@@ -296,140 +284,23 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
"""
Create/update legacy provider settings
"""
plugin_configuration = validated_data.pop('plugin_configuration', {})
updated_provider_type = validated_data.get('provider_type') or instance.provider_type
will_support_legacy = bool(
updated_provider_type == 'legacy'
)
if will_support_legacy:
course_key = instance.context_key
course = get_course_by_id(course_key)
legacy_settings = LegacySettingsSerializer(
self._get_course(),
course,
context={
'user_id': self.context['user_id'],
},
data=plugin_configuration,
data=validated_data.get('plugin_configuration', {}),
)
if legacy_settings.is_valid(raise_exception=True):
legacy_settings.save()
instance.plugin_configuration = {
"group_at_subsection": plugin_configuration.get("group_at_subsection", False)
}
instance.plugin_configuration = {}
else:
instance.plugin_configuration = plugin_configuration
return instance
def _update_course_configuration(
self,
instance: DiscussionsConfiguration,
validated_data: dict,
) -> DiscussionsConfiguration:
"""
Update configuration settings that are stored in the course.
"""
save = False
updated_provider_type = validated_data.get('provider_type') or instance.provider_type
for key in self.Meta.course_fields:
value = validated_data.get(key)
# Delay loading course till we know something has actually been updated
if value is not None and value != getattr(instance, key):
self._get_course().discussions_settings[key] = value
save = True
new_plugin_config = validated_data.get('plugin_configuration', None)
if new_plugin_config and new_plugin_config != instance.plugin_configuration:
save = True
# Any fields here that aren't already stored in the course structure
# or in other models should be stored here.
self._get_course().discussions_settings[updated_provider_type] = {
key: value
for key, value in new_plugin_config.items()
if (
key not in LegacySettingsSerializer.Meta.fields and
key not in LegacySettingsSerializer.Meta.fields_cohorts
)
}
if save:
modulestore().update_item(self._get_course(), self.context['user_id'])
return instance
class DiscussionSettingsSerializer(serializers.Serializer):
"""
Serializer for course discussion settings.
"""
divided_discussions = serializers.ListField(
child=serializers.CharField(),
write_only=True,
)
divided_course_wide_discussions = serializers.ListField(
child=serializers.CharField(),
read_only=True,
)
divided_inline_discussions = serializers.ListField(
child=serializers.CharField(),
read_only=True,
)
always_divide_inline_discussions = serializers.BooleanField()
division_scheme = serializers.CharField()
def to_internal_value(self, data: dict) -> dict:
"""
Transform the *incoming* primitive data into a native value.
"""
payload = super().to_internal_value(data) or {}
course = self.context['course']
instance = self.context['settings']
if any(item in data for item in ('divided_course_wide_discussions', 'divided_inline_discussions')):
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
course, instance
)
divided_course_wide_discussions = data.get(
'divided_course_wide_discussions',
divided_course_wide_discussions
)
divided_inline_discussions = data.get('divided_inline_discussions', divided_inline_discussions)
try:
payload['divided_discussions'] = divided_course_wide_discussions + divided_inline_discussions
except TypeError as error:
raise ValidationError(str(error)) from error
for item in ('always_divide_inline_discussions', 'division_scheme'):
if item in data:
payload[item] = data[item]
return payload
def to_representation(self, instance: CourseDiscussionSettings) -> dict:
"""
Return a serialized representation of the course discussion settings.
"""
payload = super().to_representation(instance)
course = self.context['course']
instance = self.context['settings']
course_key = course.id
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
course, instance
)
payload = {
'id': instance.id,
'divided_inline_discussions': divided_inline_discussions,
'divided_course_wide_discussions': divided_course_wide_discussions,
'always_divide_inline_discussions': instance.always_divide_inline_discussions,
'division_scheme': instance.division_scheme,
'available_division_schemes': available_division_schemes(course_key)
}
return payload
def create(self, validated_data):
"""
This method intentionally left empty
"""
def update(self, instance: CourseDiscussionSettings, validated_data: dict) -> CourseDiscussionSettings:
"""
Update and save an existing instance
"""
if not any(field in validated_data for field in self.fields):
raise ValidationError('Bad request')
try:
instance.update(validated_data)
except ValueError as e:
raise ValidationError(str(e)) from e
instance.plugin_configuration = validated_data.get('plugin_configuration') or {}
return instance

4
openedx/core/djangoapps/discussions/tests/test_models.py
View File

@@ -162,7 +162,7 @@ class DiscussionsConfigurationModelTest(TestCase):
assert configuration.enabled # by default
assert configuration.lti_configuration is None
assert len(configuration.plugin_configuration.keys()) == 0
assert configuration.provider_type == 'legacy'
assert not configuration.provider_type
def test_get_with_values(self):
"""
@@ -236,7 +236,7 @@ class DiscussionsConfigurationModelTest(TestCase):
assert configuration.enabled
assert not configuration.lti_configuration
assert not configuration.plugin_configuration
assert configuration.provider_type == 'legacy'
assert not configuration.provider_type
def test_get_explicit(self):
"""

88
openedx/core/djangoapps/discussions/tests/test_views.py
View File

@@ -2,20 +2,23 @@
Test app view logic
"""
# pylint: disable=test-inherits-tests
import itertools
from contextlib import contextmanager
from datetime import datetime, timedelta, timezone
import unittest
from contextlib import contextmanager
import ddt
from django.conf import settings
from django.core.exceptions import ValidationError
from django.urls import reverse
from lti_consumer.models import CourseAllowPIISharingInLTIFlag
from rest_framework import status
from rest_framework.test import APITestCase
from xmodule.modulestore.tests.django_utils import CourseUserType
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import CourseUserType, ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from ..models import AVAILABLE_PROVIDER_MAP, DEFAULT_CONFIG_ENABLED, DEFAULT_PROVIDER_TYPE
DATA_LEGACY_COHORTS = {
@@ -42,6 +45,7 @@ DATA_LTI_CONFIGURATION = {
}
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'URLs are only configured in LMS')
class ApiTest(ModuleStoreTestCase, APITestCase):
"""
Test basic API operations
@@ -290,34 +294,30 @@ class DataTest(AuthorizedApiTest):
"""
Check validation of basic configuration
"""
response = self._post(payload)
assert status.is_client_error(response.status_code)
assert 'enabled' in response.json()
with self.assertRaises(ValidationError):
response = self._post(payload)
response = self._get()
self._assert_defaults(response)
@ddt.data(
*DATA_LTI_CONFIGURATION.items()
)
@ddt.unpack
def test_post_lti_valid(self, key, value):
def test_post_lti_valid(self):
"""
Check we can set LTI configuration
"""
provider_type = 'piazza'
payload = {
'enabled': True,
'provider_type': provider_type,
'lti_configuration': {
key: value,
for key, value in DATA_LTI_CONFIGURATION.items():
payload = {
'enabled': True,
'provider_type': provider_type,
'lti_configuration': {
key: value,
}
}
}
self._post(payload)
response = self._get()
data = response.json()
assert data['enabled']
assert data['provider_type'] == provider_type
assert data['lti_configuration'][key] == value
response = self._post(payload)
response = self._get()
data = response.json()
assert data['enabled']
assert data['provider_type'] == provider_type
assert data['lti_configuration'][key] == value
def test_post_lti_invalid(self):
"""
@@ -481,48 +481,6 @@ class DataTest(AuthorizedApiTest):
assert data['provider_type'] == 'legacy'
assert not data['plugin_configuration']['allow_anonymous']
@ddt.data(
*itertools.product(
["enable_in_context", "enable_graded_units", "unit_level_visibility"],
[True, False],
),
("provider_type", "piazza"),
)
@ddt.unpack
def test_change_course_fields(self, field, value):
"""
Test changing fields that are saved to the course
"""
payload = {
field: value
}
response = self._post(payload)
data = response.json()
assert data[field] == value
course = self.store.get_course(self.course.id)
assert course.discussions_settings[field] == value
def test_change_plugin_configuration(self):
"""
Test changing plugin config that is saved to the course
"""
payload = {
"provider_type": "piazza",
"plugin_configuration": {
"allow_anonymous": False,
"custom_field": "custom_value",
},
}
response = self._post(payload)
data = response.json()
assert data["plugin_configuration"] == payload["plugin_configuration"]
course = self.store.get_course(self.course.id)
# Only configuration fields not stored in the course, or
# directly in the model should be stored here.
assert course.discussions_settings["piazza"] == {
"custom_field": "custom_value",
}
@ddt.data(*[
user_type.name for user_type in CourseUserType
if user_type not in { # pylint: disable=undefined-variable

2
openedx/core/djangoapps/discussions/urls.py
View File

@@ -8,7 +8,7 @@ from .views import DiscussionsConfigurationView
urlpatterns = [
url(
r'^v0/(?P<course_key_string>.+)$',
r'^api/v0/(?P<course_key_string>.+)$',
DiscussionsConfigurationView.as_view(),
name='discussions',
),

191
openedx/core/djangoapps/discussions/utils.py
View File

@@ -1,191 +0,0 @@
"""
Shared utility code related to discussions.
"""
import logging
from typing import Dict, List, Optional, Tuple
from opaque_keys.edx.keys import CourseKey
from lms.djangoapps.courseware.access import has_access
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_names, is_course_cohorted
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.lib.cache_utils import request_cached
from openedx.core.lib.courses import get_course_by_id
from openedx.core.lib.xblock_builtin.xblock_discussion.xblock_discussion import DiscussionXBlock
from openedx.core.types import User
from xmodule.course_module import CourseBlock
from xmodule.modulestore.django import modulestore
from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID, Group
from xmodule.partitions.partitions_service import PartitionService
log = logging.getLogger(__name__)
def get_divided_discussions(
course: CourseBlock,
discussion_settings: CourseDiscussionSettings,
) -> Tuple[List[str], List[str]]:
"""
Returns the course-wide and inline divided discussion ids separately.
"""
divided_course_wide_discussions = []
divided_inline_discussions = []
course_wide_discussions = [topic['id'] for __, topic in course.discussion_topics.items()]
all_discussions = get_discussion_categories_ids(course, None)
for divided_discussion_id in discussion_settings.divided_discussions:
if divided_discussion_id in course_wide_discussions:
divided_course_wide_discussions.append(divided_discussion_id)
elif divided_discussion_id in all_discussions:
divided_inline_discussions.append(divided_discussion_id)
return divided_course_wide_discussions, divided_inline_discussions
def get_discussion_categories_ids(course: CourseBlock, user: Optional[User]) -> List[str]:
"""
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.
"""
accessible_discussion_ids = [
xblock.discussion_id for xblock in get_accessible_discussion_xblocks(course, user)
]
return course.top_level_discussion_topic_ids + accessible_discussion_ids
def get_accessible_discussion_xblocks(
course: CourseBlock,
user: Optional[User],
) -> List[DiscussionXBlock]:
"""
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: CourseKey,
user: Optional[User] = None,
include_all: bool = False
) -> List[DiscussionXBlock]:
"""
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 available_division_schemes(course_key: CourseKey) -> List[str]:
"""
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 has_required_keys(xblock: DiscussionXBlock):
"""
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 enrollment_track_group_count(course_key: CourseKey) -> int:
"""
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_enrollment_track_groups(course_key: CourseKey) -> List[Group]:
"""
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 get_group_names_by_id(course_discussion_settings: CourseDiscussionSettings) -> Dict[str, str]:
"""
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_course_division_scheme(course_discussion_settings: CourseDiscussionSettings) -> str:
"""
Returns the division scheme used by the course, from the course discussion settings.
Args:
course_discussion_settings (CourseDiscussionSettings): An instance of the CourseDiscussionSettings model
Returns:
(string) Returns 'cohort', 'enrollment_track' or 'none'
depending on the division scheme used by the course.
"""
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

3
openedx/core/djangoapps/discussions/views.py
View File

@@ -107,8 +107,7 @@ class DiscussionsConfigurationView(APIView):
partial=True,
)
if serializer.is_valid(raise_exception=True):
new_provider_type = serializer.validated_data.get('provider_type', None)
if new_provider_type is not None and new_provider_type != configuration.provider_type:
if serializer.validated_data['provider_type'] != configuration.provider_type:
check_course_permissions(course, request.user, 'change_provider')
serializer.save()

1
setup.py
View File

@@ -115,7 +115,6 @@ setup(
# consolidate the multiple discussions-related Django apps and
# either put them in the openedx/ dir, or in another repo entirely.
"discussion = lms.djangoapps.discussion.apps:DiscussionConfig",
"discussions = openedx.core.djangoapps.discussions.apps:DiscussionsConfig",
"olx_rest_api = openedx.core.djangoapps.olx_rest_api.apps:OlxRestApiAppConfig",
"plugins = openedx.core.djangoapps.plugins.apps:PluginsConfig",
"theming = openedx.core.djangoapps.theming.apps:ThemingConfig",