refactor: Remove set_course_discussion_settings helper
This commit is contained in:
@@ -47,7 +47,7 @@ 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
|
||||
get_course_discussion_settings
|
||||
)
|
||||
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
|
||||
from openedx.core.lib.teams_config import TeamsConfig
|
||||
@@ -1425,13 +1425,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 = get_course_discussion_settings(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
|
||||
|
||||
@@ -8,7 +8,7 @@ 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
|
||||
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
|
||||
|
||||
|
||||
class GroupIdAssertionMixin:
|
||||
@@ -108,12 +108,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 = get_course_discussion_settings(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
|
||||
|
||||
@@ -42,7 +42,6 @@ from openedx.core.djangoapps.django_comment_common.models import (
|
||||
from openedx.core.djangoapps.django_comment_common.utils import (
|
||||
get_course_discussion_settings,
|
||||
seed_permissions_roles,
|
||||
set_course_discussion_settings
|
||||
)
|
||||
from openedx.core.djangoapps.util.testing import ContentGroupTestCase
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
@@ -1697,12 +1696,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 = get_course_discussion_settings(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)
|
||||
|
||||
|
||||
|
||||
@@ -11,8 +11,8 @@ from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
|
||||
from openedx.core.djangoapps.django_comment_common.models import ForumsConfig, Role
|
||||
from openedx.core.djangoapps.django_comment_common.utils import (
|
||||
CourseDiscussionSettings,
|
||||
get_course_discussion_settings,
|
||||
seed_permissions_roles,
|
||||
set_course_discussion_settings
|
||||
)
|
||||
from openedx.core.lib.teams_config import TeamsConfig
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
@@ -108,12 +108,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 = get_course_discussion_settings(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}
|
||||
|
||||
@@ -37,7 +37,6 @@ from openedx.core.djangoapps.django_comment_common.models import (
|
||||
Role
|
||||
)
|
||||
from openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
|
||||
from openedx.core.djangoapps.django_comment_common.utils import set_course_discussion_settings
|
||||
|
||||
|
||||
def get_context(course, request, thread=None):
|
||||
@@ -542,7 +541,7 @@ class DiscussionSettingsSerializer(serializers.Serializer):
|
||||
if not any(field in validated_data for field in self.fields):
|
||||
raise ValidationError('Bad request')
|
||||
try:
|
||||
set_course_discussion_settings(course_key, **validated_data)
|
||||
instance.update(validated_data)
|
||||
except ValueError as e:
|
||||
raise ValidationError(str(e)) from e
|
||||
return instance
|
||||
|
||||
@@ -54,7 +54,6 @@ from openedx.core.djangoapps.django_comment_common.models import CourseDiscussio
|
||||
from openedx.core.djangoapps.django_comment_common.utils import (
|
||||
ThreadContext,
|
||||
get_course_discussion_settings,
|
||||
set_course_discussion_settings
|
||||
)
|
||||
from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
|
||||
from openedx.features.course_duration_limits.access import generate_course_expired_fragment
|
||||
@@ -941,7 +940,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).
|
||||
|
||||
@@ -203,7 +203,7 @@ class CourseCohortsSettings(models.Model):
|
||||
@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)
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ 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 openedx.core.djangoapps.django_comment_common.utils import get_course_discussion_settings
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
@@ -125,10 +125,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 = get_course_discussion_settings(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)
|
||||
|
||||
@@ -267,6 +267,26 @@ class CourseDiscussionSettings(models.Model):
|
||||
"""
|
||||
self._divided_discussions = json.dumps(value)
|
||||
|
||||
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):
|
||||
"""
|
||||
|
||||
@@ -10,7 +10,6 @@ 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
|
||||
@@ -111,13 +110,13 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
|
||||
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,
|
||||
)
|
||||
def test_update_course_discussion_settings(self):
|
||||
discussion_settings = get_course_discussion_settings(self.course.id)
|
||||
discussion_settings.update({
|
||||
'divided_discussions': ['cohorted_topic'],
|
||||
'division_scheme': CourseDiscussionSettings.ENROLLMENT_TRACK,
|
||||
'always_divide_inline_discussions': True,
|
||||
})
|
||||
discussion_settings = get_course_discussion_settings(self.course.id)
|
||||
assert CourseDiscussionSettings.ENROLLMENT_TRACK == discussion_settings.division_scheme
|
||||
assert ['cohorted_topic'] == discussion_settings.divided_discussions
|
||||
@@ -132,8 +131,9 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
|
||||
]
|
||||
invalid_value = 3.14
|
||||
|
||||
discussion_settings = get_course_discussion_settings(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__)
|
||||
|
||||
@@ -134,34 +134,3 @@ def get_course_discussion_settings(course_key):
|
||||
)
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user