From 3cd3c60b590a52a4d9a2f9cbe421832c6633fbbf Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Wed, 7 Sep 2022 11:41:17 +0000 Subject: [PATCH] feat: enable unit-level control over discussions by default (#30903) This PR changes the default behaviour of the discussions experience by making the previous "unit-level visibility" the default mechanism for configuring discussions. Prior to this PR, under the new discussions configuration experience, all units would automatically get assigned a discussion topic and have discussions enabled for them (other than units in graded or exam subsections). However, if authors wanted they could enabled a custom visibility mode which would allow toggling discussions on or off on a per-unit level. This PR makes this custom visibility mode the standard behaviour (and eventually, only behaviour) and enables discussion for all units by default. This replicates the behaviour that already existed, however, now gives authors control over disabling discussions for individual units by default. It also removes the ability to disable discussions for all units (while still keeping course-wide discussions) enabled. --- .../views/tests/test_discussion_enabled.py | 14 +++++------ .../migrations/0014_auto_20220826_1107.py | 23 +++++++++++++++++++ openedx/core/djangoapps/discussions/models.py | 2 +- openedx/core/djangoapps/discussions/tasks.py | 2 +- .../discussions/tests/test_tasks.py | 22 ++++++++++-------- xmodule/course_module.py | 2 +- xmodule/vertical_block.py | 6 ++--- 7 files changed, 47 insertions(+), 24 deletions(-) create mode 100644 openedx/core/djangoapps/discussions/migrations/0014_auto_20220826_1107.py diff --git a/cms/djangoapps/contentstore/views/tests/test_discussion_enabled.py b/cms/djangoapps/contentstore/views/tests/test_discussion_enabled.py index 83f9d92349..e03f256848 100644 --- a/cms/djangoapps/contentstore/views/tests/test_discussion_enabled.py +++ b/cms/djangoapps/contentstore/views/tests/test_discussion_enabled.py @@ -91,20 +91,20 @@ class TestDiscussionEnabled(CourseTestCase): ) return resp - def test_discussion_enabled_false_initially(self): + def test_discussion_enabled_true_initially(self): """ - Tests discussion_enabled flag is False initially for vertical + Tests discussion_enabled flag is True initially for vertical """ - self.assertFalse(self.get_discussion_enabled_status(self.vertical)) - self.assertFalse(self.get_discussion_enabled_status(self.vertical_1)) + self.assertTrue(self.get_discussion_enabled_status(self.vertical)) + self.assertTrue(self.get_discussion_enabled_status(self.vertical_1)) def test_discussion_enabled_toggle(self): """ Tests discussion_enabled can be toggled. """ - self.set_discussion_enabled_status(self.vertical, True) - self.assertTrue(self.get_discussion_enabled_status(self.vertical)) - self.assertFalse(self.get_discussion_enabled_status(self.vertical_1)) + self.set_discussion_enabled_status(self.vertical, False) + self.assertFalse(self.get_discussion_enabled_status(self.vertical)) + self.assertTrue(self.get_discussion_enabled_status(self.vertical_1)) def test_non_course_author_cannot_get_or_set_discussion_enabled_flag(self): """ diff --git a/openedx/core/djangoapps/discussions/migrations/0014_auto_20220826_1107.py b/openedx/core/djangoapps/discussions/migrations/0014_auto_20220826_1107.py new file mode 100644 index 0000000000..2268782e89 --- /dev/null +++ b/openedx/core/djangoapps/discussions/migrations/0014_auto_20220826_1107.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.15 on 2022-08-26 11:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('discussions', '0013_auto_20220802_1154'), + ] + + operations = [ + migrations.AlterField( + model_name='discussionsconfiguration', + name='unit_level_visibility', + field=models.BooleanField(default=True, help_text='If enabled, discussions will need to be manually enabled for each unit.'), + ), + migrations.AlterField( + model_name='historicaldiscussionsconfiguration', + name='unit_level_visibility', + field=models.BooleanField(default=True, help_text='If enabled, discussions will need to be manually enabled for each unit.'), + ), + ] diff --git a/openedx/core/djangoapps/discussions/models.py b/openedx/core/djangoapps/discussions/models.py index f018cf6afa..1189102840 100644 --- a/openedx/core/djangoapps/discussions/models.py +++ b/openedx/core/djangoapps/discussions/models.py @@ -432,7 +432,7 @@ class DiscussionsConfiguration(TimeStampedModel): help_text=_("If enabled, discussion topics will be created for graded units as well.") ) unit_level_visibility = models.BooleanField( - default=False, + default=True, help_text=_("If enabled, discussions will need to be manually enabled for each unit.") ) plugin_configuration = JSONField( diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index 2af578755c..b8d498c4f9 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -80,7 +80,7 @@ def update_discussions_settings_from_course(course_key: CourseKey) -> CourseDisc 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', False) + unit_level_visibility = course.discussions_settings.get('unit_level_visibility', True) enable_graded_units = course.discussions_settings.get('enable_graded_units', False) contexts = [] if supports_in_context: diff --git a/openedx/core/djangoapps/discussions/tests/test_tasks.py b/openedx/core/djangoapps/discussions/tests/test_tasks.py index 7d8e854181..4950f287cd 100644 --- a/openedx/core/djangoapps/discussions/tests/test_tasks.py +++ b/openedx/core/djangoapps/discussions/tests/test_tasks.py @@ -108,10 +108,10 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): config_data = update_discussions_settings_from_course(self.course.id) assert config_data.course_key == self.course.id assert config_data.enable_graded_units is False - assert config_data.unit_level_visibility is False + assert config_data.unit_level_visibility is True assert config_data.provider_type is not None assert config_data.plugin_configuration == {} - assert len(config_data.contexts) == 4 + assert {context.title for context in config_data.contexts} == {"General", "Unit", "Discussable Unit"} def test_general_topics(self): """ @@ -122,7 +122,7 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): "Test Topic": {"id": "test-topic"}, }) config_data = update_discussions_settings_from_course(self.course.id) - assert len(config_data.contexts) == 5 + assert len(config_data.contexts) == 4 assert DiscussionTopicContext( title="General", external_id="general-topic", @@ -135,15 +135,17 @@ class UpdateDiscussionsSettingsFromCourseTestCase(ModuleStoreTestCase): ) in config_data.contexts @ddt.data( + ({}, 3, {"Unit", "Discussable Unit"}, + {"Graded Unit", "Non-Discussable Unit", "Discussable Graded Unit", "Non-Discussable Graded Unit"}), ({"enable_in_context": False}, 1, set(), {"Unit", "Graded Unit"}), - ({"enable_graded_units": False}, 4, {"Unit", "Discussable Unit", "Non-Discussable Unit"}, + ({"unit_level_visibility": False, "enable_graded_units": False}, 4, + {"Unit", "Discussable Unit", "Non-Discussable Unit"}, {"Graded Unit"}), - ({"enable_graded_units": True}, 7, {"Unit", "Graded Unit", "Discussable Graded Unit"}, set()), - ({"unit_level_visibility": True}, 2, {"Discussable Unit"}, - {"Unit", "Graded Unit", "Non-Discussable Unit", "Discussable Graded Unit", "Non-Discussable Graded Unit"}), - ({"unit_level_visibility": True, "enable_graded_units": True}, 3, - {"Discussable Unit", "Discussable Graded Unit"}, - {"Graded Unit", "Non-Discussable Unit", "Non-Discussable Graded Unit"}), + ({"unit_level_visibility": False, "enable_graded_units": True}, 7, + {"Unit", "Graded Unit", "Discussable Graded Unit"}, set()), + ({"enable_graded_units": True}, 5, + {"Discussable Unit", "Discussable Graded Unit", "Graded Unit"}, + {"Non-Discussable Unit", "Non-Discussable Graded Unit"}), ) @ddt.unpack def test_custom_discussion_settings(self, settings, context_count, present_units, missing_units): diff --git a/xmodule/course_module.py b/xmodule/course_module.py index 9926e13aeb..d6afd01829 100644 --- a/xmodule/course_module.py +++ b/xmodule/course_module.py @@ -418,7 +418,7 @@ class CourseFields: # lint-amnesty, pylint: disable=missing-class-docstring default={ "enable_in_context": True, "enable_graded_units": False, - "unit_level_visibility": False, + "unit_level_visibility": True, } ) announcement = Date( diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index 03c7b37f9e..3efc1fdc15 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -41,10 +41,8 @@ class VerticalFields: discussion_enabled = Boolean( display_name=_("Enable in-context discussions for the Unit"), - help=_( - "Add discussion for the Unit." - ), - default=False, + help=_("Add discussion for the Unit."), + default=True, scope=Scope.settings, )