Skip to content

Commit 3453e7d

Browse files
authored
fix: discussion tab visibility on import (#38084)
* fix: discussion tab visibility on import * fix: quality issues * refactor: move the logic to update_discussions_settings_from_course in task * test: add test * fix: remove old code * fix: issue * chore: update openedx-events version
1 parent ae87974 commit 3453e7d

File tree

8 files changed

+74
-4
lines changed

8 files changed

+74
-4
lines changed

openedx/core/djangoapps/discussions/handlers.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,17 @@ def update_course_discussion_config(configuration: CourseDiscussionConfiguration
9696
log.info(f"Course {course_key} doesn't have discussion configuration model yet. Creating a new one.")
9797
DiscussionsConfiguration(
9898
context_key=course_key,
99+
enabled=configuration.enabled,
99100
provider_type=provider_id,
100101
plugin_configuration=configuration.plugin_configuration,
101102
enable_in_context=configuration.enable_in_context,
102103
enable_graded_units=configuration.enable_graded_units,
103104
unit_level_visibility=configuration.unit_level_visibility,
104105
).save()
106+
else:
107+
DiscussionsConfiguration.objects.filter(
108+
context_key=course_key,
109+
).update(enabled=configuration.enabled)
105110

106111

107112
COURSE_DISCUSSIONS_CHANGED.connect(handle_course_discussion_config_update)

openedx/core/djangoapps/discussions/tasks.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,20 @@ def update_discussions_settings_from_course(
8888
)
8989
contexts.extend(list(discussable_units))
9090

91+
# Derive enabled from the discussion tab's is_hidden state.
92+
# When a course is imported or rerun, course.tabs are copied verbatim
93+
# from the source but DiscussionsConfiguration is not, so we pass the
94+
# tab state through config_data for the handler to reconcile.
95+
enabled = True
96+
if course:
97+
for tab in course.tabs:
98+
if getattr(tab, 'tab_id', None) == 'discussion':
99+
enabled = not tab.is_hidden
100+
break
101+
91102
config_data = CourseDiscussionConfigurationData(
92103
course_key=course_key,
104+
enabled=enabled,
93105
enable_in_context=enable_in_context,
94106
enable_graded_units=enable_graded_units,
95107
unit_level_visibility=unit_level_visibility,

openedx/core/djangoapps/discussions/tests/test_handlers.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ def test_configuration_for_new_course(self):
5555
config_data = CourseDiscussionConfigurationData(
5656
course_key=new_key,
5757
provider_type="openedx",
58+
plugin_configuration={},
5859
)
5960
assert not DiscussionsConfiguration.objects.filter(context_key=new_key).exists()
6061
update_course_discussion_config(config_data)
@@ -191,3 +192,33 @@ def test_enabled_units_change(self):
191192
assert existing_topic_link.title == "Section 10|Subsection 10|Unit 10"
192193
# If there is no stored context, then continue using the Unit name.
193194
assert existing_topic_link_2.title == "Unit 11"
195+
196+
def test_new_config_uses_enabled_from_configuration(self):
197+
"""
198+
When creating a new DiscussionsConfiguration, the handler should use
199+
the enabled value from the configuration data.
200+
"""
201+
new_key = CourseKey.from_string("course-v1:test+test+disabled")
202+
config_data = CourseDiscussionConfigurationData(
203+
course_key=new_key,
204+
provider_type="openedx",
205+
enabled=False,
206+
plugin_configuration={},
207+
)
208+
update_course_discussion_config(config_data)
209+
db_config = DiscussionsConfiguration.objects.get(context_key=new_key)
210+
assert db_config.enabled is False
211+
212+
def test_existing_config_updated_enabled_from_configuration(self):
213+
"""
214+
When the configuration already exists, the handler should update
215+
enabled from the configuration data.
216+
"""
217+
config_data = CourseDiscussionConfigurationData(
218+
course_key=self.course_key,
219+
provider_type="openedx",
220+
enabled=False,
221+
)
222+
update_course_discussion_config(config_data)
223+
db_config = DiscussionsConfiguration.objects.get(context_key=self.course_key)
224+
assert db_config.enabled is False

openedx/core/djangoapps/discussions/tests/test_tasks.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,28 @@ def test_custom_discussion_settings(self, settings, context_count, present_units
184184
assert present_units <= units_in_config
185185
assert not missing_units & units_in_config
186186

187+
@ddt.data(
188+
# (tab_is_hidden, expected_enabled)
189+
(True, False), # import with hidden tab → disabled
190+
(False, True), # import with visible tab → enabled
191+
)
192+
@ddt.unpack
193+
def test_config_data_enabled_from_discussion_tab(self, tab_is_hidden, expected_enabled):
194+
"""
195+
update_discussions_settings_from_course should set config_data.enabled
196+
based on the discussion tab's is_hidden state.
197+
"""
198+
course = self.store.get_course(self.course.id)
199+
for tab in course.tabs:
200+
if getattr(tab, 'tab_id', None) == 'discussion':
201+
tab['is_hidden'] = tab_is_hidden
202+
break
203+
self.store.update_item(course, self.user.id)
204+
205+
config_data = update_discussions_settings_from_course(self.course.id)
206+
207+
assert config_data.enabled is expected_enabled
208+
187209

188210
@ddt.ddt
189211
class MigrateUnitDiscussionStateFromXBlockTestCase(ModuleStoreTestCase, DiscussionConfigUpdateMixin):

requirements/edx/base.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ openedx-django-require==3.0.0
839839
# via -r requirements/edx/kernel.in
840840
openedx-django-wiki==3.1.1
841841
# via -r requirements/edx/kernel.in
842-
openedx-events==10.5.0
842+
openedx-events==11.1.0
843843
# via
844844
# -r requirements/edx/kernel.in
845845
# edx-enterprise

requirements/edx/development.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1400,7 +1400,7 @@ openedx-django-wiki==3.1.1
14001400
# via
14011401
# -r requirements/edx/doc.txt
14021402
# -r requirements/edx/testing.txt
1403-
openedx-events==10.5.0
1403+
openedx-events==11.1.0
14041404
# via
14051405
# -r requirements/edx/doc.txt
14061406
# -r requirements/edx/testing.txt

requirements/edx/doc.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ openedx-django-require==3.0.0
10191019
# via -r requirements/edx/base.txt
10201020
openedx-django-wiki==3.1.1
10211021
# via -r requirements/edx/base.txt
1022-
openedx-events==10.5.0
1022+
openedx-events==11.1.0
10231023
# via
10241024
# -r requirements/edx/base.txt
10251025
# edx-enterprise

requirements/edx/testing.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ openedx-django-require==3.0.0
10681068
# via -r requirements/edx/base.txt
10691069
openedx-django-wiki==3.1.1
10701070
# via -r requirements/edx/base.txt
1071-
openedx-events==10.5.0
1071+
openedx-events==11.1.0
10721072
# via
10731073
# -r requirements/edx/base.txt
10741074
# edx-enterprise

0 commit comments

Comments
 (0)