diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 3c5f60ca7c82..7ec66fed03b4 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -466,8 +466,19 @@ def sync_discussion_settings(course_key, user): discussion_config.provider_type = Provider.OPEN_EDX - discussion_config.enable_graded_units = discussion_settings['enable_graded_units'] - discussion_config.unit_level_visibility = discussion_settings['unit_level_visibility'] + fields = ["enable_graded_units", "unit_level_visibility", "enable_in_context", "posting_restrictions"] + # Plugin configuration is stored in the course settings under the provider name. + field_mappings = dict(zip(fields, fields)) | {"plugin_configuration": discussion_config.provider_type} + + for attr_name, settings_key in field_mappings.items(): + if settings_key in discussion_settings: + setattr(discussion_config, attr_name, discussion_settings[settings_key]) + + # This part is no longer needed in Verawood. + for tab in course.tabs: + if tab.tab_id == "discussion": + discussion_config.enabled = not tab.is_hidden + break discussion_config.save() LOGGER.info(f'Course import {course.id}: DiscussionsConfiguration synced as per course') except Exception as exc: # pylint: disable=broad-except diff --git a/cms/djangoapps/contentstore/tests/test_tasks.py b/cms/djangoapps/contentstore/tests/test_tasks.py index cf82a6d16571..66a4ce80343c 100644 --- a/cms/djangoapps/contentstore/tests/test_tasks.py +++ b/cms/djangoapps/contentstore/tests/test_tasks.py @@ -8,6 +8,7 @@ from unittest import mock from uuid import uuid4 +import ddt from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.test.utils import override_settings @@ -17,12 +18,19 @@ from organizations.tests.factories import OrganizationFactory from user_tasks.models import UserTaskArtifact, UserTaskStatus -from cms.djangoapps.contentstore.tasks import export_olx, update_special_exams_and_publish, rerun_course +from cms.djangoapps.contentstore.tasks import ( + export_olx, + update_special_exams_and_publish, + rerun_course, + sync_discussion_settings, +) from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from cms.djangoapps.contentstore.tests.utils import CourseTestCase from common.djangoapps.course_action_state.models import CourseRerunState from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.course_apps.toggles import EXAMS_IDA +from openedx.core.djangoapps.discussions.config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS +from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider from openedx.core.djangoapps.embargo.models import Country, CountryAccessRule, RestrictedCourse from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE @@ -199,3 +207,110 @@ def test_register_exams_failure(self, _mock_register_exams_proctoring, _mock_reg _mock_register_exams_proctoring.side_effect = Exception('boom!') update_special_exams_and_publish(str(self.course.id)) course_publish.assert_called() + + +@ddt.ddt +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) +class SyncDiscussionSettingsTaskTestCase(CourseTestCase): + """Tests for the `sync_discussion_settings` task.""" + + def setUp(self): + super().setUp() + self.discussion_config = DiscussionsConfiguration.objects.create(context_key=self.course.id) + + def _update_discussion_settings(self, discussions_settings: dict): + """Helper method to set discussion settings in the course.""" + self.course.discussions_settings = discussions_settings + modulestore().update_item(self.course, self.user.id) + + def test_sync_settings(self): + """Test syncing discussion settings to DiscussionsConfiguration.""" + self._update_discussion_settings( + { + "enable_graded_units": True, + "unit_level_visibility": False, + "enable_in_context": True, + "posting_restrictions": "enabled", + } + ) + + sync_discussion_settings(self.course.id, self.user) + + self.discussion_config.refresh_from_db() + assert self.discussion_config.enable_graded_units is True + assert self.discussion_config.unit_level_visibility is False + assert self.discussion_config.enable_in_context is True + assert self.discussion_config.posting_restrictions == "enabled" + assert self.discussion_config.provider_type == Provider.LEGACY + + def test_sync_plugin_configuration(self): + """Test syncing plugin configuration from provider settings.""" + # Set up course discussion settings with provider-specific config + provider_config = {"test_key": "test_value", "test_key_2": "test_value_2"} + self._update_discussion_settings({self.discussion_config.provider_type: provider_config}) + + sync_discussion_settings(self.course.id, self.user) + + self.discussion_config.refresh_from_db() + assert self.discussion_config.plugin_configuration == provider_config + + @ddt.data( + (False, True), # When the tab is visible, the discussion should be enabled. + (True, False), # When the tab is hidden, the discussion should be disabled. + ) + @ddt.unpack + def test_sync_discussion_tab_visibility(self, is_hidden: bool, expected_enabled: bool): + """Test syncing discussion enabled status based on tab visibility.""" + for tab in self.course.tabs: + if tab.tab_id == "discussion": + tab.is_hidden = is_hidden + break + modulestore().update_item(self.course, self.user.id) + + sync_discussion_settings(self.course.id, self.user) + + self.discussion_config.refresh_from_db() + assert self.discussion_config.enabled is expected_enabled + + @override_waffle_flag(ENABLE_NEW_STRUCTURE_DISCUSSIONS, active=True) + def test_auto_migrate_to_new_structure(self): + """Test automatic migration to the `OPEN_EDX` provider when new structure is enabled.""" + with self.assertLogs("cms.djangoapps.contentstore.tasks", level="INFO") as logs: + sync_discussion_settings(self.course.id, self.user) + + migration_log = f"New structure is enabled, also updating {self.course.id} to use new provider" + assert any(migration_log in log for log in logs.output) + + self.discussion_config.refresh_from_db() + assert self.discussion_config.provider_type == Provider.OPEN_EDX + + course = modulestore().get_course(self.course.id) + assert course.discussions_settings.get("provider_type") == Provider.OPEN_EDX + + @ddt.data( + {"provider_type": Provider.OPEN_EDX}, # Using the `provider_type` field. + {"provider": Provider.OPEN_EDX}, # Using the `provider` field as fallback. + ) + @override_waffle_flag(ENABLE_NEW_STRUCTURE_DISCUSSIONS, active=True) + def test_no_provider_migration_when_already_openedx(self, provider_settings: dict): + """Test no migration occurs when provider is already `OPEN_EDX`.""" + self._update_discussion_settings(provider_settings) + + with self.assertLogs("cms.djangoapps.contentstore.tasks", level="INFO") as logs: + sync_discussion_settings(self.course.id, self.user) + + migration_log = f"New structure is enabled, also updating {self.course.id} to use new provider" + assert not any(migration_log in log for log in logs.output) + + def test_handling_exceptions(self): + """Test that exceptions are caught and logged properly.""" + test_error_message = "Test error" + + with mock.patch.object(DiscussionsConfiguration.objects, "get", side_effect=Exception(test_error_message)): + with self.assertLogs("cms.djangoapps.contentstore.tasks", level="INFO") as logs: + sync_discussion_settings(self.course.id, self.user) + + expected_log = ( + f"Course import {self.course.id}: DiscussionsConfiguration sync failed: {test_error_message}" + ) + assert any(expected_log in log for log in logs.output)