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

Merge PR #27343 bd03/api/legacy

Browse Source 
* Commits:
  docs: Remove completed TODO discussions item
  feat: Add support to Discussions API for legacy cohort settings
  refactor: Remove get_course_discussion_settings helper
  refactor: Remove set_course_discussion_settings helper
  refactor: Add better DRF support to legacy discussions settings
This commit is contained in:
stvn
2021-04-30 12:33:13 -07:00
parent 39bca7bba4 718458115a
commit 9f1debac14
18 changed files with 240 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
lms/djangoapps/discussion/django_comment_client/base/tests.py
View File

@@ -47,7 +47,6 @@ from openedx.core.djangoapps.django_comment_common.models import (
from openedx.core.djangoapps.django_comment_common.utils import (
ThreadContext,
seed_permissions_roles,
set_course_discussion_settings
)
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
from openedx.core.lib.teams_config import TeamsConfig
@@ -1425,13 +1424,13 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto
If dividing by cohorts, create and assign users to a cohort.
"""
enable_cohorts = True if scheme is CourseDiscussionSettings.COHORT else False
set_course_discussion_settings(
self.course.id,
enable_cohorts=enable_cohorts,
divided_discussions=[],
always_divide_inline_discussions=True,
division_scheme=scheme,
)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
discussion_settings.update({
'enable_cohorts': enable_cohorts,
'divided_discussions': [],
'always_divide_inline_discussions': True,
'division_scheme': scheme,
})
set_course_cohorted(self.course.id, enable_cohorts)
@classmethod

3
lms/djangoapps/discussion/django_comment_client/permissions.py
View File

@@ -14,7 +14,6 @@ from openedx.core.djangoapps.django_comment_common.models import (
CourseDiscussionSettings,
all_permissions_for_user_in_course
)
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
from openedx.core.lib.cache_utils import request_cached
@@ -145,7 +144,7 @@ def _check_conditions_permissions(user, permissions, course_id, content, user_gr
# or a course has divided discussions, but the current user's content group does not equal
# the content group of the commenter/poster,
# then the current user does not have group edit permissions.
division_scheme = get_course_discussion_settings(course_id).division_scheme
division_scheme = CourseDiscussionSettings.get(course_id).division_scheme
if (division_scheme is CourseDiscussionSettings.NONE
or user_group_id is None
or content_user_group is None

13
lms/djangoapps/discussion/django_comment_client/tests/group_id.py
View File

@@ -8,7 +8,6 @@ from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
from lms.djangoapps.teams.tests.factories import CourseTeamFactory
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.utils import set_course_discussion_settings
class GroupIdAssertionMixin:
@@ -108,12 +107,12 @@ class CohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
def test_cohorted_topic_enrollment_track_invalid_group_id(self, mock_request):
CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.AUDIT)
CourseModeFactory.create(course_id=self.course.id, mode_slug=CourseMode.VERIFIED)
set_course_discussion_settings(
course_key=self.course.id,
divided_discussions=['cohorted_topic'],
division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK,
always_divide_inline_discussions=True,
)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
discussion_settings.update({
'divided_discussions': ['cohorted_topic'],
'division_scheme': CourseDiscussionSettings.ENROLLMENT_TRACK,
'always_divide_inline_discussions': True,
})
invalid_id = -1000
response = self.call_view(mock_request, "cohorted_topic", self.moderator, invalid_id) # lint-amnesty, pylint: disable=assignment-from-no-return

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

@@ -39,11 +39,7 @@ from openedx.core.djangoapps.django_comment_common.models import (
ForumsConfig,
assign_role
)
from openedx.core.djangoapps.django_comment_common.utils import (
get_course_discussion_settings,
seed_permissions_roles,
set_course_discussion_settings
)
from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles
from openedx.core.djangoapps.util.testing import ContentGroupTestCase
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
@@ -1387,7 +1383,7 @@ class GroupIdForUserTestCase(ModuleStoreTestCase):
)
def test_discussion_division_disabled(self):
course_discussion_settings = get_course_discussion_settings(self.course.id)
course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.NONE == course_discussion_settings.division_scheme
assert utils.get_group_id_for_user(self.test_user, course_discussion_settings) is None
@@ -1395,7 +1391,7 @@ class GroupIdForUserTestCase(ModuleStoreTestCase):
set_discussion_division_settings(
self.course.id, enable_cohorts=True, division_scheme=CourseDiscussionSettings.COHORT
)
course_discussion_settings = get_course_discussion_settings(self.course.id)
course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.COHORT == course_discussion_settings.division_scheme
assert self.test_cohort.id == utils.get_group_id_for_user(self.test_user, course_discussion_settings)
@@ -1403,7 +1399,7 @@ class GroupIdForUserTestCase(ModuleStoreTestCase):
set_discussion_division_settings(
self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK
)
course_discussion_settings = get_course_discussion_settings(self.course.id)
course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.ENROLLMENT_TRACK == course_discussion_settings.division_scheme
assert (- 2) == utils.get_group_id_for_user(self.test_user, course_discussion_settings)
@@ -1422,7 +1418,7 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
)
def test_discussion_division_disabled(self):
course_discussion_settings = get_course_discussion_settings(self.course.id)
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)
@@ -1431,13 +1427,13 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
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(get_course_discussion_settings(self.course.id))
assert not utils.course_discussion_division_enabled(CourseDiscussionSettings.get(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(get_course_discussion_settings(self.course.id))
assert utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
assert [CourseDiscussionSettings.COHORT] == utils.available_division_schemes(self.course.id)
def test_discussion_division_by_enrollment_track(self):
@@ -1445,12 +1441,12 @@ class CourseDiscussionDivisionEnabledTestCase(ModuleStoreTestCase):
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(get_course_discussion_settings(self.course.id))
assert not utils.course_discussion_division_enabled(CourseDiscussionSettings.get(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(get_course_discussion_settings(self.course.id))
assert utils.course_discussion_division_enabled(CourseDiscussionSettings.get(self.course.id))
assert [CourseDiscussionSettings.ENROLLMENT_TRACK] == utils.available_division_schemes(self.course.id)
@@ -1474,7 +1470,7 @@ class GroupNameTestCase(ModuleStoreTestCase):
)
def test_discussion_division_disabled(self):
course_discussion_settings = get_course_discussion_settings(self.course.id)
course_discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert {} == utils.get_group_names_by_id(course_discussion_settings)
assert utils.get_group_name((- 1000), course_discussion_settings) is None
@@ -1482,7 +1478,7 @@ class GroupNameTestCase(ModuleStoreTestCase):
set_discussion_division_settings(
self.course.id, enable_cohorts=True, division_scheme=CourseDiscussionSettings.COHORT
)
course_discussion_settings = get_course_discussion_settings(self.course.id)
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_2.name == utils.get_group_name(self.test_cohort_2.id, course_discussion_settings)
# Test also with a group_id that doesn't exist.
@@ -1492,7 +1488,7 @@ class GroupNameTestCase(ModuleStoreTestCase):
set_discussion_division_settings(
self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK
)
course_discussion_settings = get_course_discussion_settings(self.course.id)
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 'verified course' == utils.get_group_name((- 2), course_discussion_settings)
@@ -1697,12 +1693,12 @@ def set_discussion_division_settings(
COHORT is the default division_scheme, as no other schemes were supported at
the time that the unit tests were originally written.
"""
set_course_discussion_settings(
course_key=course_key,
divided_discussions=divided_discussions,
division_scheme=division_scheme,
always_divide_inline_discussions=always_divide_inline_discussions,
)
discussion_settings = CourseDiscussionSettings.get(course_key)
discussion_settings.update({
'divided_discussions': divided_discussions,
'division_scheme': division_scheme,
'always_divide_inline_discussions': always_divide_inline_discussions,
})
set_course_cohorted(course_key, enable_cohorts)

22
lms/djangoapps/discussion/django_comment_client/tests/utils.py
View File

@@ -8,12 +8,9 @@ from unittest.mock import patch
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory
from common.djangoapps.util.testing import UrlResetMixin
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.models import ForumsConfig, Role
from openedx.core.djangoapps.django_comment_common.utils import (
CourseDiscussionSettings,
seed_permissions_roles,
set_course_discussion_settings
)
from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles
from openedx.core.lib.teams_config import TeamsConfig
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
@@ -108,12 +105,15 @@ def config_course_discussions(
"""Convert name to id."""
return topic_name_to_id(course, name)
set_course_discussion_settings(
course.id,
divided_discussions=[to_id(name) for name in divided_discussions],
always_divide_inline_discussions=always_divide_inline_discussions,
division_scheme=CourseDiscussionSettings.COHORT,
)
discussion_settings = CourseDiscussionSettings.get(course.id)
discussion_settings.update({
'divided_discussions': [
to_id(name)
for name in divided_discussions
],
'always_divide_inline_discussions': always_divide_inline_discussions,
'division_scheme': CourseDiscussionSettings.COHORT,
})
course.discussion_topics = {name: {"sort_key": "A", "id": to_id(name)}
for name in discussion_topics}

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

@@ -33,7 +33,6 @@ from openedx.core.djangoapps.django_comment_common.models import (
DiscussionsIdMapping,
Role
)
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
from openedx.core.lib.cache_utils import request_cached
from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore
@@ -358,7 +357,7 @@ def get_discussion_category_map(course, user, divided_only_if_explicit=False, ex
xblocks = get_accessible_discussion_xblocks(course, user)
discussion_settings = get_course_discussion_settings(course.id)
discussion_settings = CourseDiscussionSettings.get(course.id)
discussion_division_enabled = course_discussion_division_enabled(discussion_settings)
divided_discussion_ids = discussion_settings.divided_discussions
@@ -797,7 +796,7 @@ def prepare_content(content, course_key, is_staff=False, discussion_division_ena
del endorsement["user_id"]
if discussion_division_enabled is None:
discussion_division_enabled = course_discussion_division_enabled(get_course_discussion_settings(course_key))
discussion_division_enabled = course_discussion_division_enabled(CourseDiscussionSettings.get(course_key))
for child_content_key in ["children", "endorsed_responses", "non_endorsed_responses"]:
if child_content_key in content:
@@ -816,7 +815,7 @@ def prepare_content(content, course_key, is_staff=False, discussion_division_ena
if discussion_division_enabled:
# Augment the specified thread info to include the group name if a group id is present.
if content.get('group_id') is not None:
course_discussion_settings = get_course_discussion_settings(course_key)
course_discussion_settings = CourseDiscussionSettings.get(course_key)
if group_names_by_id:
content['group_name'] = group_names_by_id.get(content.get('group_id'))
else:
@@ -843,7 +842,7 @@ def get_group_id_for_comments_service(request, course_key, commentable_id=None):
Raises:
ValueError if the requested group_id is invalid
"""
course_discussion_settings = get_course_discussion_settings(course_key)
course_discussion_settings = CourseDiscussionSettings.get(course_key)
if commentable_id is None or is_commentable_divided(course_key, commentable_id, course_discussion_settings):
if request.method == "GET":
requested_group_id = request.GET.get('group_id')
@@ -870,7 +869,7 @@ def get_group_id_for_user_from_cache(user, course_id):
Caches the results of get_group_id_for_user, but serializes the course_id
instead of the course_discussions_settings object as cache keys.
"""
return get_group_id_for_user(user, get_course_discussion_settings(course_id))
return get_group_id_for_user(user, CourseDiscussionSettings.get(course_id))
def get_group_id_for_user(user, course_discussion_settings):
@@ -922,7 +921,7 @@ def is_commentable_divided(course_key, commentable_id, course_discussion_setting
Http404 if the course doesn't exist.
"""
if not course_discussion_settings:
course_discussion_settings = get_course_discussion_settings(course_key)
course_discussion_settings = CourseDiscussionSettings.get(course_key)
course = get_course_by_id(course_key)

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

@@ -51,6 +51,7 @@ from lms.djangoapps.discussion.rest_api.utils import discussion_open_for_user
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
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.signals import (
comment_created,
comment_deleted,
@@ -61,7 +62,6 @@ from openedx.core.djangoapps.django_comment_common.signals import (
thread_edited,
thread_voted
)
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
from openedx.core.djangoapps.user_api.accounts.api import get_account_settings
from openedx.core.djangoapps.user_api.accounts.views import \
AccountViewSet # lint-amnesty, pylint: disable=unused-import
@@ -126,7 +126,7 @@ def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
course_key = CourseKey.from_string(cc_thread["course_id"])
course = _get_course(course_key, request.user)
context = get_context(course, request, cc_thread)
course_discussion_settings = get_course_discussion_settings(course_key)
course_discussion_settings = CourseDiscussionSettings.get(course_key)
if (
not context["is_requester_privileged"] and
cc_thread["group_id"] and
@@ -590,7 +590,7 @@ def get_thread_list(
"user_id": str(request.user.id),
"group_id": (
None if context["is_requester_privileged"] else
get_group_id_for_user(request.user, get_course_discussion_settings(course.id))
get_group_id_for_user(request.user, CourseDiscussionSettings.get(course.id))
),
"page": page,
"per_page": page_size,
@@ -875,7 +875,7 @@ def create_thread(request, thread_data):
context = get_context(course, request)
_check_initializable_thread_fields(thread_data, context)
discussion_settings = get_course_discussion_settings(course_key)
discussion_settings = CourseDiscussionSettings.get(course_key)
if (
"group_id" not in thread_data and
is_commentable_divided(course_key, thread_data.get("topic_id"), discussion_settings)

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

@@ -11,6 +11,7 @@ from six.moves.urllib.parse import urlencode, urlunparse
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,
@@ -29,12 +30,12 @@ from openedx.core.djangoapps.django_comment_common.comment_client.thread import
from openedx.core.djangoapps.django_comment_common.comment_client.user import User as CommentClientUser
from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError
from openedx.core.djangoapps.django_comment_common.models import (
CourseDiscussionSettings,
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_MODERATOR,
Role
)
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
def get_context(course, request, thread=None):
@@ -59,7 +60,7 @@ def get_context(course, request, thread=None):
requester = request.user
cc_requester = CommentClientUser.from_django_user(requester).retrieve()
cc_requester["course_id"] = course.id
course_discussion_settings = get_course_discussion_settings(course.id)
course_discussion_settings = CourseDiscussionSettings.get(course.id)
return {
"course": course,
"request": request,
@@ -466,57 +467,83 @@ 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 __init__(self, *args, **kwargs):
self.course = kwargs.pop('course')
self.discussion_settings = kwargs.pop('discussion_settings')
super().__init__(*args, **kwargs)
def validate(self, attrs):
def to_internal_value(self, data: dict) -> dict:
"""
Validate the fields in combination.
Transform the *incoming* primitive data into a native value.
"""
if not any(field in attrs for field in self.fields):
raise ValidationError('Bad request')
settings_to_change = {}
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
self.course, self.discussion_settings
)
if any(item in attrs for item in ('divided_course_wide_discussions', 'divided_inline_discussions')):
divided_course_wide_discussions = attrs.get(
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 = attrs.get('divided_inline_discussions', divided_inline_discussions)
settings_to_change['divided_discussions'] = divided_course_wide_discussions + divided_inline_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 attrs:
settings_to_change[item] = attrs[item]
attrs['settings_to_change'] = settings_to_change
return attrs
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):
"""
Overriden create abstract method
This method intentionally left empty
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
def update(self, instance, validated_data):
def update(self, instance: CourseDiscussionSettings, validated_data: dict) -> CourseDiscussionSettings:
"""
Overriden update abstract method
Update and save an existing instance
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
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):

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

@@ -16,7 +16,6 @@ from rest_framework.response import Response
from rest_framework.views import APIView
from rest_framework.viewsets import ViewSet
from common.djangoapps.util.json_request import JsonResponse
from lms.djangoapps.discussion.django_comment_client.utils import available_division_schemes
from lms.djangoapps.discussion.rest_api.api import (
create_comment,
@@ -48,10 +47,7 @@ from lms.djangoapps.discussion.views import get_divided_discussions
from lms.djangoapps.instructor.access import update_forum_role
from openedx.core.djangoapps.django_comment_common import comment_client
from openedx.core.djangoapps.django_comment_common.models import Role
from openedx.core.djangoapps.django_comment_common.utils import (
get_course_discussion_settings,
set_course_discussion_settings
)
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.user_api.accounts.permissions import CanReplaceUsername, CanRetireUser
from openedx.core.djangoapps.user_api.models import UserRetirementStatus
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
@@ -756,22 +752,6 @@ class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView):
parser_classes = (JSONParser, MergePatchParser,)
permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser)
def _get_representation(self, course, course_key, discussion_settings):
"""
Return a serialized representation of the course discussion settings.
"""
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
course, discussion_settings
)
return JsonResponse({
'id': discussion_settings.id,
'divided_inline_discussions': divided_inline_discussions,
'divided_course_wide_discussions': divided_course_wide_discussions,
'always_divide_inline_discussions': discussion_settings.always_divide_inline_discussions,
'division_scheme': discussion_settings.division_scheme,
'available_division_schemes': available_division_schemes(course_key)
})
def _get_request_kwargs(self, course_id):
return dict(course_id=course_id)
@@ -787,8 +767,17 @@ class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView):
course_key = form.cleaned_data['course_key']
course = form.cleaned_data['course']
discussion_settings = get_course_discussion_settings(course_key)
return self._get_representation(course, course_key, discussion_settings)
discussion_settings = CourseDiscussionSettings.get(course_key)
serializer = DiscussionSettingsSerializer(
discussion_settings,
context={
'course': course,
'settings': discussion_settings,
},
partial=True,
)
response = Response(serializer.data)
return response
def patch(self, request, course_id):
"""
@@ -804,24 +793,20 @@ class CourseDiscussionSettingsAPIView(DeveloperErrorViewMixin, APIView):
course = form.cleaned_data['course']
course_key = form.cleaned_data['course_key']
discussion_settings = get_course_discussion_settings(course_key)
discussion_settings = CourseDiscussionSettings.get(course_key)
serializer = DiscussionSettingsSerializer(
discussion_settings,
context={
'course': course,
'settings': discussion_settings,
},
data=request.data,
partial=True,
course=course,
discussion_settings=discussion_settings
)
if not serializer.is_valid():
raise ValidationError(serializer.errors)
settings_to_change = serializer.validated_data['settings_to_change']
try:
discussion_settings = set_course_discussion_settings(course_key, **settings_to_change)
except ValueError as e:
raise ValidationError(str(e)) # lint-amnesty, pylint: disable=raise-missing-from
serializer.save()
return Response(status=status.HTTP_204_NO_CONTENT)
@@ -907,7 +892,7 @@ class CourseDiscussionRolesAPIView(DeveloperErrorViewMixin, APIView):
role = form.cleaned_data['role']
data = {'course_id': course_id, 'users': role.users.all()}
context = {'course_discussion_settings': get_course_discussion_settings(course_id)}
context = {'course_discussion_settings': CourseDiscussionSettings.get(course_id)}
serializer = DiscussionRolesListSerializer(data, context=context)
return Response(serializer.data)
@@ -937,6 +922,6 @@ class CourseDiscussionRolesAPIView(DeveloperErrorViewMixin, APIView):
role = form.cleaned_data['role']
data = {'course_id': course_id, 'users': role.users.all()}
context = {'course_discussion_settings': get_course_discussion_settings(course_id)}
context = {'course_discussion_settings': CourseDiscussionSettings.get(course_id)}
serializer = DiscussionRolesListSerializer(data, context=context)
return Response(serializer.data)

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

@@ -51,11 +51,7 @@ from lms.djangoapps.discussion.exceptions import TeamDiscussionHiddenFromUserExc
from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context
from lms.djangoapps.teams import api as team_api
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.utils import (
ThreadContext,
get_course_discussion_settings,
set_course_discussion_settings
)
from openedx.core.djangoapps.django_comment_common.utils import ThreadContext
from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
from openedx.features.course_duration_limits.access import generate_course_expired_fragment
from xmodule.modulestore.django import modulestore
@@ -76,7 +72,7 @@ def make_course_settings(course, user, include_category_map=True):
Generate a JSON-serializable model for course settings, which will be used to initialize a
DiscussionCourseSettings object on the client.
"""
course_discussion_settings = get_course_discussion_settings(course.id)
course_discussion_settings = CourseDiscussionSettings.get(course.id)
group_names_by_id = get_group_names_by_id(course_discussion_settings)
course_setting = {
'is_discussion_division_enabled': course_discussion_division_enabled(course_discussion_settings),
@@ -225,7 +221,7 @@ def inline_discussion(request, course_key, discussion_id):
with function_trace('determine_group_permissions'):
is_staff = has_permission(request.user, 'openclose_thread', course.id)
course_discussion_settings = get_course_discussion_settings(course.id)
course_discussion_settings = CourseDiscussionSettings.get(course.id)
group_names_by_id = get_group_names_by_id(course_discussion_settings)
course_is_divided = course_discussion_settings.division_scheme is not CourseDiscussionSettings.NONE
@@ -378,7 +374,7 @@ def _find_thread(request, course, discussion_id, thread_id):
# verify that the thread belongs to the requesting student's group
is_moderator = has_permission(request.user, "see_all_cohorts", course.id)
course_discussion_settings = get_course_discussion_settings(course.id)
course_discussion_settings = CourseDiscussionSettings.get(course.id)
if is_commentable_divided(course.id, discussion_id, course_discussion_settings) and not is_moderator:
user_group_id = get_group_id_for_user(request.user, course_discussion_settings)
if getattr(thread, "group_id", None) is not None and user_group_id != thread.group_id:
@@ -489,7 +485,7 @@ def _create_discussion_board_context(request, base_context, thread=None):
add_courseware_context(threads, course, user)
with function_trace("get_cohort_info"):
course_discussion_settings = get_course_discussion_settings(course_key)
course_discussion_settings = CourseDiscussionSettings.get(course_key)
user_group_id = get_group_id_for_user(user, course_discussion_settings)
context.update({
@@ -563,7 +559,7 @@ def create_user_profile_context(request, course_key, user_id):
).order_by("name").values_list("name", flat=True).distinct()
with function_trace("get_cohort_info"):
course_discussion_settings = get_course_discussion_settings(course_key)
course_discussion_settings = CourseDiscussionSettings.get(course_key)
user_group_id = get_group_id_for_user(request.user, course_discussion_settings)
context = _create_base_discussion_view_context(request, course_key)
@@ -909,7 +905,7 @@ def course_discussions_settings_handler(request, course_key_string):
"""
course_key = CourseKey.from_string(course_key_string)
course = get_course_with_access(request.user, 'staff', course_key)
discussion_settings = get_course_discussion_settings(course_key)
discussion_settings = CourseDiscussionSettings.get(course_key)
if request.method == 'PATCH':
divided_course_wide_discussions, divided_inline_discussions = get_divided_discussions(
@@ -941,7 +937,7 @@ def course_discussions_settings_handler(request, course_key_string):
try:
if settings_to_change:
discussion_settings = set_course_discussion_settings(course_key, **settings_to_change)
discussion_settings.update(settings_to_change)
except ValueError as err:
# Note: error message not translated because it is not exposed to the user (UI prevents this state).

4
lms/djangoapps/instructor/views/api.py
View File

@@ -77,7 +77,6 @@ from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.courses import get_course_with_access
from lms.djangoapps.courseware.models import StudentModule
from lms.djangoapps.discussion.django_comment_client.utils import (
get_course_discussion_settings,
get_group_id_for_user,
get_group_name,
has_forum_access
@@ -103,6 +102,7 @@ from lms.djangoapps.instructor_task.models import ReportStore
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted
from openedx.core.djangoapps.django_comment_common.models import (
CourseDiscussionSettings,
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR,
@@ -2131,7 +2131,7 @@ def list_forum_members(request, course_id):
except Role.DoesNotExist:
users = []
course_discussion_settings = get_course_discussion_settings(course_id)
course_discussion_settings = CourseDiscussionSettings.get(course_id)
def extract_user_info(user):
""" Convert user to dict for json rendering. """

6
openedx/core/djangoapps/course_groups/models.py
View File

@@ -189,21 +189,21 @@ class CourseCohortsSettings(models.Model):
# in reality the default value at the time that cohorting is enabled for a course comes from
# course_module.always_cohort_inline_discussions (via `migrate_cohort_settings`).
# DEPRECATED-- DO NOT USE: Instead use `CourseDiscussionSettings.always_divide_inline_discussions`
# via `get_course_discussion_settings` or `set_course_discussion_settings`.
# via `CourseDiscussionSettings.get` or `CourseDiscussionSettings.update`.
always_cohort_inline_discussions = models.BooleanField(default=False)
@property
def cohorted_discussions(self):
"""
DEPRECATED-- DO NOT USE. Instead use `CourseDiscussionSettings.divided_discussions`
via `get_course_discussion_settings`.
via `CourseDiscussionSettings.get`.
"""
return json.loads(self._cohorted_discussions)
@cohorted_discussions.setter
def cohorted_discussions(self, value):
"""
DEPRECATED-- DO NOT USE. Instead use `CourseDiscussionSettings` via `set_course_discussion_settings`.
DEPRECATED-- DO NOT USE. Instead use `CourseDiscussionSettings.update`
"""
self._cohorted_discussions = json.dumps(value)

9
openedx/core/djangoapps/course_groups/tests/helpers.py
View File

@@ -10,7 +10,6 @@ from factory.django import DjangoModelFactory
from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.utils import set_course_discussion_settings
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
@@ -125,10 +124,10 @@ def config_course_cohorts(
"""
set_course_cohorted(course.id, is_cohorted)
set_course_discussion_settings(
course.id,
division_scheme=discussion_division_scheme,
)
discussion_settings = CourseDiscussionSettings.get(course.id)
discussion_settings.update({
'division_scheme': discussion_division_scheme,
})
for cohort_name in auto_cohorts:
cohort = CohortFactory(course_id=course.id, name=cohort_name)

5
openedx/core/djangoapps/course_groups/tests/test_views.py
View File

@@ -14,7 +14,6 @@ from django.test.client import RequestFactory
from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import InstructorFactory
from common.djangoapps.student.tests.factories import StaffFactory
@@ -196,13 +195,13 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase):
expected_response = self.get_expected_response()
expected_response['is_cohorted'] = False
assert response == expected_response
assert CourseDiscussionSettings.NONE == get_course_discussion_settings(self.course.id).division_scheme
assert CourseDiscussionSettings.NONE == CourseDiscussionSettings.get(self.course.id).division_scheme
expected_response['is_cohorted'] = True
response = self.patch_handler(self.course, data=expected_response, handler=course_cohort_settings_handler)
assert response == expected_response
assert CourseDiscussionSettings.NONE == get_course_discussion_settings(self.course.id).division_scheme
assert CourseDiscussionSettings.NONE == CourseDiscussionSettings.get(self.course.id).division_scheme
def test_update_settings_with_missing_field(self):
"""

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

@@ -12,6 +12,8 @@ from rest_framework.response import Response
from rest_framework.views import APIView
from common.djangoapps.student.roles import CourseStaffRole
from lms.djangoapps.discussion.rest_api.serializers import DiscussionSettingsSerializer
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.core.lib.courses import get_course_by_id
from xmodule.modulestore.django import modulestore
@@ -111,7 +113,6 @@ class LegacySettingsSerializer(serializers.BaseSerializer):
payload = {
key: value
for key, value in data.items()
if key in self.Meta.fields
}
return payload
@@ -124,6 +125,20 @@ class LegacySettingsSerializer(serializers.BaseSerializer):
for field in instance.fields.values()
if field.name in self.Meta.fields
}
discussion_settings = CourseDiscussionSettings.get(instance.id)
serializer = DiscussionSettingsSerializer(
discussion_settings,
context={
'course': instance,
'settings': discussion_settings,
},
partial=True,
)
settings.update({
key: value
for key, value in serializer.data.items()
if key != 'id'
})
return settings
def update(self, instance, validated_data: dict):
@@ -131,11 +146,26 @@ class LegacySettingsSerializer(serializers.BaseSerializer):
Update and save an existing instance
"""
save = False
for field in self.Meta.fields:
if field in validated_data:
value = validated_data[field]
cohort_settings = {}
for field, value in validated_data.items():
if field in self.Meta.fields:
setattr(instance, field, value)
save = True
else:
cohort_settings[field] = value
if cohort_settings:
discussion_settings = CourseDiscussionSettings.get(instance.id)
serializer = DiscussionSettingsSerializer(
discussion_settings,
context={
'course': instance,
'settings': discussion_settings,
},
data=cohort_settings,
partial=True,
)
if serializer.is_valid(raise_exception=True):
serializer.save()
if save:
modulestore().update_item(instance, self.context['user_id'])
return instance
@@ -296,8 +326,6 @@ class DiscussionsConfigurationView(APIView):
def post(self, request, course_key_string: str, **_kwargs) -> Response:
"""
Handle HTTP/POST requests
TODO: Should we cleanup orphaned LTI config when swapping to cs_comments_service?
"""
course_key = _validate_course_key(course_key_string)
configuration = DiscussionsConfiguration.get(course_key)

42
openedx/core/djangoapps/django_comment_common/models.py
View File

@@ -16,6 +16,7 @@ from jsonfield.fields import JSONField
from opaque_keys.edx.django.models import CourseKeyField
from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager
from openedx.core.lib.cache_utils import request_cached
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import GlobalStaff
from xmodule.modulestore.django import modulestore
@@ -267,6 +268,47 @@ class CourseDiscussionSettings(models.Model):
"""
self._divided_discussions = json.dumps(value)
@request_cached()
@classmethod
def get(cls, course_key):
"""
Get and/or create settings
"""
try:
course_discussion_settings = cls.objects.get(course_id=course_key)
except cls.DoesNotExist:
from openedx.core.djangoapps.course_groups.cohorts import get_legacy_discussion_settings
legacy_discussion_settings = get_legacy_discussion_settings(course_key)
course_discussion_settings, _ = cls.objects.get_or_create(
course_id=course_key,
defaults={
'always_divide_inline_discussions': legacy_discussion_settings['always_cohort_inline_discussions'],
'divided_discussions': legacy_discussion_settings['cohorted_discussions'],
'division_scheme': cls.COHORT if legacy_discussion_settings['is_cohorted'] else cls.NONE
},
)
return course_discussion_settings
def update(self, validated_data: dict):
"""
Set discussion settings for a course
Returns:
A CourseDiscussionSettings object
"""
fields = {
'division_scheme': (str,)[0],
'always_divide_inline_discussions': bool,
'divided_discussions': list,
}
for field, field_type in fields.items():
if field in validated_data:
if not isinstance(validated_data[field], field_type):
raise ValueError(f"Incorrect field type for `{field}`. Type must be `{field_type.__name__}`")
setattr(self, field, validated_data[field])
self.save()
return self
class DiscussionsIdMapping(models.Model):
"""

29
openedx/core/djangoapps/django_comment_common/tests.py
View File

@@ -8,10 +8,6 @@ from opaque_keys.edx.locator import CourseLocator
from openedx.core.djangoapps.course_groups.cohorts import CourseCohortsSettings
from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role
from openedx.core.djangoapps.django_comment_common.utils import (
get_course_discussion_settings,
set_course_discussion_settings
)
from common.djangoapps.student.models import CourseEnrollment, User
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
@@ -80,7 +76,7 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
self.course = CourseFactory.create()
def test_get_course_discussion_settings(self):
discussion_settings = get_course_discussion_settings(self.course.id)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.NONE == discussion_settings.division_scheme
assert [] == discussion_settings.divided_discussions
assert not discussion_settings.always_divide_inline_discussions
@@ -92,7 +88,7 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
'cohorted_discussions': ['foo']
}
modulestore().update_item(self.course, ModuleStoreEnum.UserID.system)
discussion_settings = get_course_discussion_settings(self.course.id)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.COHORT == discussion_settings.division_scheme
assert ['foo'] == discussion_settings.divided_discussions
assert discussion_settings.always_divide_inline_discussions
@@ -106,19 +102,19 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
'cohorted_discussions': ['foo', 'bar']
}
)
discussion_settings = get_course_discussion_settings(self.course.id)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.COHORT == discussion_settings.division_scheme
assert ['foo', 'bar'] == discussion_settings.divided_discussions
assert discussion_settings.always_divide_inline_discussions
def test_set_course_discussion_settings(self):
set_course_discussion_settings(
course_key=self.course.id,
divided_discussions=['cohorted_topic'],
division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK,
always_divide_inline_discussions=True,
)
discussion_settings = get_course_discussion_settings(self.course.id)
def test_update_course_discussion_settings(self):
discussion_settings = CourseDiscussionSettings.get(self.course.id)
discussion_settings.update({
'divided_discussions': ['cohorted_topic'],
'division_scheme': CourseDiscussionSettings.ENROLLMENT_TRACK,
'always_divide_inline_discussions': True,
})
discussion_settings = CourseDiscussionSettings.get(self.course.id)
assert CourseDiscussionSettings.ENROLLMENT_TRACK == discussion_settings.division_scheme
assert ['cohorted_topic'] == discussion_settings.divided_discussions
assert discussion_settings.always_divide_inline_discussions
@@ -132,8 +128,9 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
]
invalid_value = 3.14
discussion_settings = CourseDiscussionSettings.get(self.course.id)
for field in fields:
with pytest.raises(ValueError) as value_error:
set_course_discussion_settings(self.course.id, **{field['name']: invalid_value})
discussion_settings.update({field['name']: invalid_value})
assert str(value_error.value) == exception_msg_template.format(field['name'], field['type'].__name__)

55
openedx/core/djangoapps/django_comment_common/utils.py
View File

@@ -2,21 +2,16 @@
"""
Common comment client utility functions.
"""
from contracts import new_contract
from openedx.core.djangoapps.course_groups.cohorts import get_legacy_discussion_settings
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_GROUP_MODERATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_STUDENT,
CourseDiscussionSettings,
Role
)
from openedx.core.lib.cache_utils import request_cached
new_contract('basestring', str)
@@ -115,53 +110,3 @@ def are_permissions_roles_seeded(course_id):
return False
return True
@request_cached()
def get_course_discussion_settings(course_key):
try:
course_discussion_settings = CourseDiscussionSettings.objects.get(course_id=course_key)
except CourseDiscussionSettings.DoesNotExist:
legacy_discussion_settings = get_legacy_discussion_settings(course_key)
course_discussion_settings, _ = CourseDiscussionSettings.objects.get_or_create(
course_id=course_key,
defaults={
'always_divide_inline_discussions': legacy_discussion_settings['always_cohort_inline_discussions'],
'divided_discussions': legacy_discussion_settings['cohorted_discussions'],
'division_scheme': CourseDiscussionSettings.COHORT if legacy_discussion_settings['is_cohorted']
else CourseDiscussionSettings.NONE
}
)
return course_discussion_settings
def set_course_discussion_settings(course_key, **kwargs):
"""
Set discussion settings for a course.
Arguments:
course_key: CourseKey
always_divide_inline_discussions (bool): If inline discussions should always be divided.
divided_discussions (list): List of discussion ids.
division_scheme (str): `CourseDiscussionSettings.NONE`, `CourseDiscussionSettings.COHORT`,
or `CourseDiscussionSettings.ENROLLMENT_TRACK`
Returns:
A CourseDiscussionSettings object.
"""
fields = {
'division_scheme': (str,)[0],
'always_divide_inline_discussions': bool,
'divided_discussions': list,
}
course_discussion_settings = get_course_discussion_settings(course_key)
for field, field_type in fields.items():
if field in kwargs:
if not isinstance(kwargs[field], field_type):
raise ValueError(f"Incorrect field type for `{field}`. Type must be `{field_type.__name__}`")
setattr(course_discussion_settings, field, kwargs[field])
course_discussion_settings.save()
return course_discussion_settings