feat: sync more discussion settings on course import

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

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)