refactor: Remove get_course_discussion_settings helper

This commit is contained in:
stvn
2021-04-27 14:47:31 -07:00
parent 146887df1b
commit 1b76999c2f
17 changed files with 77 additions and 100 deletions

View File

@@ -189,14 +189,14 @@ 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)

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 get_course_discussion_settings
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
@@ -125,7 +124,7 @@ def config_course_cohorts(
"""
set_course_cohorted(course.id, is_cohorted)
discussion_settings = get_course_discussion_settings(course.id)
discussion_settings = CourseDiscussionSettings.get(course.id)
discussion_settings.update({
'division_scheme': discussion_division_scheme,
})

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):
"""

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,27 @@ 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

View File

@@ -8,9 +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,
)
from common.djangoapps.student.models import CourseEnrollment, User
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
@@ -79,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
@@ -91,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
@@ -105,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_update_course_discussion_settings(self):
discussion_settings = get_course_discussion_settings(self.course.id)
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 = get_course_discussion_settings(self.course.id)
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
@@ -131,7 +128,7 @@ class CourseDiscussionSettingsTest(ModuleStoreTestCase):
]
invalid_value = 3.14
discussion_settings = get_course_discussion_settings(self.course.id)
discussion_settings = CourseDiscussionSettings.get(self.course.id)
for field in fields:
with pytest.raises(ValueError) as value_error:
discussion_settings.update({field['name']: invalid_value})

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,22 +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