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

fix: discussion topic links not created in some cases (#31032)

Browse Source 
This commit attempts to fix cases where dicussion topic links aren't created
during a provider change.

It does so by eliminating areas where there could be desynchronisation between
the configuration the course configuration in Mogo and the discussion config
in MySQL.

The topic creation code now uses the database version of the config which is
more recent.
This commit is contained in:
Kshitij Sobti
2022-09-29 12:16:03 +00:00
committed by GitHub
parent 74a12ed796
commit 346257dadf
3 changed files with 66 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -326,18 +326,20 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
"""
save = False
updated_provider_type = validated_data.get('provider_type') or instance.provider_type
course = self._get_course()
for key in self.Meta.course_fields:
value = validated_data.get(key)
course_value = course.discussions_settings.get(key, None)
# 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
if value is not None and value != course_value:
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] = {
course.discussions_settings[updated_provider_type] = {
key: value
for key, value in new_plugin_config.items()
if (
@@ -346,7 +348,7 @@ class DiscussionsConfigurationSerializer(serializers.ModelSerializer):
)
}
if save:
modulestore().update_item(self._get_course(), self.context['user_id'])
modulestore().update_item(course, self.context['user_id'])
return instance

19
openedx/core/djangoapps/discussions/tasks.py
View File

@@ -85,11 +85,10 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc
with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key):
course = store.get_course(course_key)
provider = course.discussions_settings.get('provider', provider_type)
enable_in_context = course.discussions_settings.get('enable_in_context', True)
provider_config = course.discussions_settings.get(provider, {})
unit_level_visibility = course.discussions_settings.get('unit_level_visibility', True)
enable_graded_units = course.discussions_settings.get('enable_graded_units', False)
enable_in_context = discussions_config.enable_in_context
provider_config = discussions_config.plugin_configuration
unit_level_visibility = discussions_config.unit_level_visibility
enable_graded_units = discussions_config.enable_graded_units
contexts = []
if supports_in_context:
sorted_topics = sorted(
@@ -112,7 +111,7 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc
enable_in_context=enable_in_context,
enable_graded_units=enable_graded_units,
unit_level_visibility=unit_level_visibility,
provider_type=provider,
provider_type=provider_type,
plugin_configuration=provider_config,
contexts=contexts,
)
@@ -152,7 +151,11 @@ def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, u
verticals = store.get_items(course_key, qualifiers={'block_type': 'vertical'})
graded_subsections = {
block.location
for block in store.get_items(course_key, qualifies={'block_type': 'sequential'}, settings={'graded': True})
for block in store.get_items(
course_key,
qualifies={'block_type': 'sequential'},
settings={'graded': True}
)
}
subsections_with_discussions = set()
for vertical in verticals:
@@ -176,9 +179,9 @@ def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, u
course.discussions_settings['provider'] = provider
course.discussions_settings['enable_graded_units'] = enable_graded_subsections
course.discussions_settings['unit_level_visibility'] = True
store.update_item(course, user_id)
discussion_config = DiscussionsConfiguration.get(course_key)
discussion_config.provider_type = provider
discussion_config.enable_graded_units = enable_graded_subsections
discussion_config.unit_level_visibility = True
store.update_item(course, user_id)
discussion_config.save()

92
openedx/core/djangoapps/discussions/tests/test_tasks.py
View File

@@ -7,7 +7,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag
from openedx_events.learning.data import DiscussionTopicContext
from openedx.core.djangoapps.discussions.config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS
from openedx.core.djangoapps.discussions.models import Provider
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider
from openedx.core.djangoapps.discussions.tasks import (
update_discussions_settings_from_course,
update_unit_discussion_state_from_discussion_blocks,
@@ -16,16 +16,57 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
class DiscussionConfigUpdateMixin:
"""
Mixin for common methods used to update course discussion configuration.
"""
def update_course_field(self, **update):
"""
Update the test course using provided parameters.
"""
for key, value in update.items():
setattr(self.course, key, value)
self.store.update_item(self.course, self.user.id)
def update_discussions_settings(self, settings):
"""
Update course discussion settings based on the provided discussion settings.
"""
discussion_config = DiscussionsConfiguration.get(self.course.id)
for key, value in settings.items():
key = "provider_type" if key == "provider" else key
if value is not None:
setattr(discussion_config, key, value)
discussion_config.save()
self.course.discussions_settings.update(settings)
self.store.update_item(self.course, self.user.id)
def assert_discussion_settings(self, **settings):
"""
Assert that the provided settings have the provided values in the course's discussion settings.
"""
course = self.store.get_course(self.course.id)
discussion_config = DiscussionsConfiguration.get(self.course.id)
for key, value in settings.items():
assert course.discussions_settings.get(key, None) == value
key = "provider_type" if key == "provider" else key
assert getattr(discussion_config, key) == value
@ddt.ddt
@mock.patch('openedx.core.djangoapps.discussions.tasks.DiscussionsConfiguration', mock.Mock())
class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase):
class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase, DiscussionConfigUpdateMixin):
"""
Tests for the discussions settings update tasks
"""
def setUp(self):
super().setUp()
self.course = course = CourseFactory.create()
self.course = course = CourseFactory.create(
discussions_settings={
"provider": Provider.OPEN_EDX
}
)
self.course_key = course_key = self.course.id
with self.store.bulk_operations(course_key):
self.section = ItemFactory.create(parent=course, category="chapter", display_name="Section")
@@ -72,21 +113,9 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase):
category="html",
display_name="Graded HTML Module",
)
def update_course_field(self, **update):
"""
Update the test course using provided parameters.
"""
for key, value in update.items():
setattr(self.course, key, value)
self.store.update_item(self.course, self.user.id)
def update_discussions_settings(self, settings):
"""
Update course discussion settings based on the provided discussion settings.
"""
self.course.discussions_settings.update(settings)
self.store.update_item(self.course, self.user.id)
discussion_config = DiscussionsConfiguration.get(course_key)
discussion_config.provider_type = Provider.OPEN_EDX
discussion_config.save()
def test_default(self):
"""
@@ -156,7 +185,7 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase):
@ddt.ddt
class MigrateUnitDiscussionStateFromXBlockTestCase(ModuleStoreTestCase):
class MigrateUnitDiscussionStateFromXBlockTestCase(ModuleStoreTestCase, DiscussionConfigUpdateMixin):
"""
Tests for the discussions settings update tasks
"""
@@ -222,21 +251,6 @@ class MigrateUnitDiscussionStateFromXBlockTestCase(ModuleStoreTestCase):
discussion_category=f'Category {unit.display_name}',
)
def update_course_field(self, **update):
"""
Update the test course using provided parameters.
"""
for key, value in update.items():
setattr(self.course, key, value)
self.store.update_item(self.course, self.user.id)
def update_discussions_settings(self, settings):
"""
Update course discussion settings based on the provided discussion settings.
"""
self.course.discussions_settings.update(settings)
self.store.update_item(self.course, self.user.id)
@mock.patch('openedx.core.djangoapps.discussions.tasks.get_accessible_discussion_xblocks_by_course_id')
def test_course_not_using_legacy(self, mock_get_discussion_blocks):
self.update_discussions_settings({'provider': 'non-legacy'})
@@ -272,14 +286,6 @@ class MigrateUnitDiscussionStateFromXBlockTestCase(ModuleStoreTestCase):
}
return discussible, non_discussible
def assert_discussion_settings(self, **settings):
"""
Assert that the provided settings have the provided values in the course's discussion settings.
"""
course = self.store.get_course(self.course.id)
for key, value in settings.items():
assert course.discussions_settings.get(key, None) == value
def test_without_graded(self):
self.add_discussion_block([self.unit_discussible])
update_unit_discussion_state_from_discussion_blocks(self.course.id, self.user.id)