diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 20146caf1f..eea41c3a43 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -51,6 +51,7 @@ from common.djangoapps.course_action_state.models import CourseRerunState from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.util.monitoring import monitor_import_failure from openedx.core.djangoapps.content.learning_sequences.api import key_supports_outlines +from openedx.core.djangoapps.discussions.tasks import update_unit_discussion_state_from_discussion_blocks from openedx.core.djangoapps.embargo.models import CountryAccessRule, RestrictedCourse from openedx.core.lib.extract_tar import safetar_extractall from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order @@ -119,6 +120,7 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i store = modulestore() with store.default_store('split'): store.clone_course(source_course_key, destination_course_key, user_id, fields=fields) + update_unit_discussion_state_from_discussion_blocks(destination_course_key, user_id) # set initial permissions for the user to access the course. initialize_permissions(destination_course_key, User.objects.get(id=user_id)) diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index 18bb009f40..93dbff3fa0 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -10,8 +10,10 @@ from openedx_events.learning.data import CourseDiscussionConfigurationData, Disc from openedx_events.learning.signals import COURSE_DISCUSSIONS_CHANGED from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore +from .config.waffle import ENABLE_NEW_STRUCTURE_DISCUSSIONS -from .models import DiscussionsConfiguration +from .models import DiscussionsConfiguration, Provider +from .utils import get_accessible_discussion_xblocks_by_course_id log = logging.getLogger(__name__) @@ -115,3 +117,68 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc contexts=contexts, ) return config_data + + +def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, user_id: int, force=False) -> None: + """ + Migrate existing courses to the new mechanism for linking discussion to units. + + This will iterate over an existing course's discussion xblocks and mark the units + they are in as discussable. + + Args: + course_key (CourseKey): CourseKey for course. + user_id (int): User id for the user performing this operation. + force (bool): Force migration of data even if not using legacy provider + + """ + store = modulestore() + course = store.get_course(course_key) + provider = course.discussions_settings.get('provider', None) + # Only migrate to the new discussion provider if the current provider is the legacy provider. + if provider is not None and provider != Provider.LEGACY and not force: + return + + log.info(f"Migrating legacy discussion config for {course_key}") + + with store.bulk_operations(course_key): + discussion_blocks = get_accessible_discussion_xblocks_by_course_id(course_key, include_all=True) + discussible_units = { + discussion_block.parent + for discussion_block in discussion_blocks + if discussion_block.parent.block_type == 'vertical' + } + log.info(f"Found {len(discussible_units)} discussible unit(s) in {course_key}") + 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}) + } + subsections_with_discussions = set() + for vertical in verticals: + if vertical.location in discussible_units: + vertical.discussion_enabled = True + subsections_with_discussions.add(vertical.parent) + else: + vertical.discussion_enabled = False + store.update_item(vertical, user_id) + + # If there are any graded subsections that have discussion units, + # then enable discussions for graded subsections for the course + enable_graded_subsections = bool(graded_subsections & subsections_with_discussions) + + # If the new discussions experience is enabled globally, + # then also set up the new provider for the course. + if ENABLE_NEW_STRUCTURE_DISCUSSIONS.is_enabled(): + log.info(f"New structure is enabled, also updating {course_key} to use new provider") + course = store.get_course(course_key) + provider = Provider.OPEN_EDX + course.discussions_settings['provider'] = provider + course.discussions_settings['enable_graded_units'] = enable_graded_subsections + course.discussions_settings['unit_level_visibility'] = True + 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() diff --git a/openedx/core/djangoapps/discussions/tests/test_tasks.py b/openedx/core/djangoapps/discussions/tests/test_tasks.py index 1d0c3a9d03..a529aff549 100644 --- a/openedx/core/djangoapps/discussions/tests/test_tasks.py +++ b/openedx/core/djangoapps/discussions/tests/test_tasks.py @@ -3,12 +3,16 @@ Tests for discussions tasks. """ import ddt import mock - - +from edx_toggles.toggles.testutils import override_waffle_flag from openedx_events.learning.data import DiscussionTopicContext -from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course -from xmodule.modulestore.tests.django_utils import TEST_DATA_MONGO_AMNESTY_MODULESTORE, ModuleStoreTestCase +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.tasks import ( + update_discussions_settings_from_course, + update_unit_discussion_state_from_discussion_blocks, +) +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -18,72 +22,55 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): """ Tests for the discussions settings update tasks """ - MODULESTORE = TEST_DATA_MONGO_AMNESTY_MODULESTORE def setUp(self): super().setUp() self.course = course = CourseFactory.create() self.course_key = course_key = self.course.id with self.store.bulk_operations(course_key): - self.section = ItemFactory.create( - parent_location=course.location, - category="chapter", - display_name="Section" - ) - self.sequence = ItemFactory.create( - parent_location=self.section.location, - category="sequential", - display_name="Sequence" - ) - self.unit = ItemFactory.create( - parent_location=self.sequence.location, - category="vertical", - display_name="Unit" - ) + self.section = ItemFactory.create(parent=course, category="chapter", display_name="Section") + self.sequence = ItemFactory.create(parent=self.section, category="sequential", display_name="Sequence") + self.unit = ItemFactory.create(parent=self.sequence, category="vertical", display_name="Unit") ItemFactory.create( - parent_location=self.sequence.location, + parent=self.sequence, category="vertical", display_name="Discussable Unit", discussion_enabled=True, ) ItemFactory.create( - parent_location=self.sequence.location, + parent=self.sequence, category="vertical", display_name="Non-Discussable Unit", discussion_enabled=False, ) - ItemFactory.create( - parent_location=self.unit.location, - category="html", - display_name="An HTML Module" - ) + ItemFactory.create(parent=self.unit, category="html", display_name="An HTML Module") graded_sequence = ItemFactory.create( - parent_location=self.section.location, + parent=self.section, category="sequential", display_name="Graded Sequence", graded=True, ) graded_unit = ItemFactory.create( - parent_location=graded_sequence.location, + parent=graded_sequence, category="vertical", - display_name="Graded Unit" + display_name="Graded Unit", ) ItemFactory.create( - parent_location=graded_sequence.location, + parent=graded_sequence, category="vertical", display_name="Discussable Graded Unit", discussion_enabled=True, ) ItemFactory.create( - parent_location=graded_sequence.location, + parent=graded_sequence, category="vertical", display_name="Non-Discussable Graded Unit", discussion_enabled=False, ) ItemFactory.create( - parent_location=graded_unit.location, + parent=graded_unit, category="html", - display_name="Graded HTML Module" + display_name="Graded HTML Module", ) def update_course_field(self, **update): @@ -92,14 +79,14 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): """ for key, value in update.items(): setattr(self.course, key, value) - self.update_course(self.course, self.user.id) + 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.update_course(self.course, self.user.id) + self.store.update_item(self.course, self.user.id) def test_default(self): """ @@ -166,3 +153,155 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): units_in_config = {context.title for context in config_data.contexts} assert present_units <= units_in_config assert not missing_units & units_in_config + + +@ddt.ddt +class MigrateUnitDiscussionStateFromXBlockTestCase(ModuleStoreTestCase): + """ + Tests for the discussions settings update tasks + """ + + def setUp(self): + super().setUp() + self.course = course = CourseFactory.create() + self.course_key = course_key = self.course.id + with self.store.bulk_operations(course_key): + section = ItemFactory.create( + parent=course, category="chapter", display_name="Section" + ) + sequence = ItemFactory.create( + parent=section, category="sequential", display_name="Sequence" + ) + self.unit_discussible = unit_discussible = ItemFactory.create( + parent=sequence, + category="vertical", + display_name="Discussable Unit", + ) + unit_non_discussible = ItemFactory.create( + parent=sequence, + category="vertical", + display_name="Non-Discussable Unit", + discussion_enabled=False, + ) + graded_sequence = ItemFactory.create( + parent=section, + category="sequential", + display_name="Graded Sequence", + graded=True, + ) + self.graded_unit_discussible = graded_unit_discussible = ItemFactory.create( + parent=graded_sequence, + category="vertical", + display_name="Discussable Graded Unit", + ) + graded_unit_non_discussible = ItemFactory.create( + parent=graded_sequence, + category="vertical", + display_name="Non-Discussable Graded Unit", + ) + self.discussible = {unit_discussible.display_name, graded_unit_discussible.display_name} + self.non_discussible = { + unit_non_discussible.display_name, + graded_unit_non_discussible.display_name, + } + self.graded = { + graded_unit_discussible.display_name, + graded_unit_non_discussible.display_name, + } + + def add_discussion_block(self, units): + """ + Add a discussion block to the specified units. + """ + for unit in units: + ItemFactory.create( + parent=unit, + category='discussion', + discussion_id=f'id-{unit.location}', + discussion_target=f'Target {unit.display_name}', + 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'}) + update_unit_discussion_state_from_discussion_blocks(self.course.id, self.user.id) + mock_get_discussion_blocks.assert_not_called() + + @mock.patch('openedx.core.djangoapps.discussions.tasks.get_accessible_discussion_xblocks_by_course_id') + @ddt.data(None, 'legacy') + def test_course_using_legacy(self, provider, mock_get_discussion_blocks): + self.update_discussions_settings({'provider': provider}) + update_unit_discussion_state_from_discussion_blocks(self.course.id, self.user.id) + mock_get_discussion_blocks.assert_called() + + def get_discussible_and_non_discussible_blocks(self): + """ + Get a set of display names of the discussible and non-discussible blocks in a course. + """ + discussible = { + item.display_name + for item in self.store.get_items( + self.course.id, + qualifiers={'block_type': 'vertical'}, + settings={'discussion_enabled': True}, + ) + } + non_discussible = { + item.display_name + for item in self.store.get_items( + self.course.id, + qualifiers={'block_type': 'vertical'}, + settings={'discussion_enabled': False}, + ) + } + 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) + discussible, non_discussible = self.get_discussible_and_non_discussible_blocks() + # A discussion block was not added to a graded unit, so it shouldn't be in the set of discussible blocks + assert discussible == (self.discussible - self.graded) + assert non_discussible == (self.non_discussible | self.graded) + self.assert_discussion_settings(enable_graded_units=False) + + @ddt.data(True, False) + def test_with_graded(self, new_structure_enabled): + self.add_discussion_block([self.unit_discussible, self.graded_unit_discussible]) + with override_waffle_flag(ENABLE_NEW_STRUCTURE_DISCUSSIONS, active=new_structure_enabled): + update_unit_discussion_state_from_discussion_blocks(self.course.id, self.user.id) + discussible, non_discussible = self.get_discussible_and_non_discussible_blocks() + # A discussion block was not added to a graded unit, so it shouldn't be in the set of discussible blocks + assert discussible == self.discussible + assert non_discussible == self.non_discussible + self.assert_discussion_settings(enable_graded_units=new_structure_enabled) + + @override_waffle_flag(ENABLE_NEW_STRUCTURE_DISCUSSIONS, active=True) + def test_with_new_structure(self): + update_unit_discussion_state_from_discussion_blocks(self.course.id, self.user.id) + self.assert_discussion_settings(provider=Provider.OPEN_EDX) + self.assert_discussion_settings(unit_level_visibility=True)