From ccfeede8e6d8ac249a66cd553c4eb7f89f4d9034 Mon Sep 17 00:00:00 2001 From: Paulo Viadanna Date: Mon, 2 Sep 2024 14:36:16 -0300 Subject: [PATCH 1/4] fix: unhide discussion tab when enabling it --- openedx/core/djangoapps/discussions/utils.py | 13 +++++++++++++ openedx/core/djangoapps/discussions/views.py | 4 +++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/discussions/utils.py b/openedx/core/djangoapps/discussions/utils.py index 7c26a4e482e6..fe5c88fff92f 100644 --- a/openedx/core/djangoapps/discussions/utils.py +++ b/openedx/core/djangoapps/discussions/utils.py @@ -191,3 +191,16 @@ def get_course_division_scheme(course_discussion_settings: CourseDiscussionSetti ): division_scheme = CourseDiscussionSettings.NONE return division_scheme + + +def show_discussion_tab(course_key, user_id: int) -> None: + """ + Toggle the visibility of the discussion tab in the course. + """ + store = modulestore() + course = store.get_course(course_key) + for tab in course.tabs: + if tab.tab_id == 'discussion': + tab.is_hidden = False + store.update_item(course, user_id) + break diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index 9d27d8f5f8e1..adbb5867050f 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -21,6 +21,7 @@ DiscussionsConfigurationSerializer, DiscussionsProvidersSerializer, ) +from .utils import show_discussion_tab class DiscussionsConfigurationSettingsView(APIView): @@ -120,8 +121,9 @@ def update_configuration_data(request, course_key_string): new_provider_type = serializer.validated_data.get('provider_type', None) if new_provider_type is not None and new_provider_type != configuration.provider_type: check_course_permissions(course, request.user, 'change_provider') - serializer.save() + if configuration.is_enabled(course_key): + show_discussion_tab(course_key, request.user.id) return serializer.data From d5774e092442419a43a0061cc8343f43380555ec Mon Sep 17 00:00:00 2001 From: Paulo Viadanna Date: Tue, 3 Sep 2024 08:06:51 -0300 Subject: [PATCH 2/4] refactor: move modulestore code to the serializer --- openedx/core/djangoapps/discussions/serializers.py | 10 ++++++++++ openedx/core/djangoapps/discussions/utils.py | 13 ------------- openedx/core/djangoapps/discussions/views.py | 4 +--- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/discussions/serializers.py b/openedx/core/djangoapps/discussions/serializers.py index 88648a499598..1514a696f195 100644 --- a/openedx/core/djangoapps/discussions/serializers.py +++ b/openedx/core/djangoapps/discussions/serializers.py @@ -7,6 +7,8 @@ from rest_framework import serializers from xmodule.modulestore.django import modulestore +from opaque_keys.edx.keys import CourseKey + from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings from openedx.core.lib.courses import get_course_by_id @@ -265,6 +267,14 @@ def update(self, instance: DiscussionsConfiguration, validated_data: dict) -> Di course_overview_id=instance.context_key, type='discussion' ).update(is_hidden=not instance.enabled) + # do the same for the modulestore representation + store = modulestore() + course = store.get_course(instance.context_key) + for tab in course.tabs: + if tab.tab_id == 'discussion': + tab.is_hidden = not instance.enabled + store.update_item(course, self.context['user_id']) + break update_discussions_settings_from_course_task.delay(str(instance.context_key)) return instance diff --git a/openedx/core/djangoapps/discussions/utils.py b/openedx/core/djangoapps/discussions/utils.py index fe5c88fff92f..7c26a4e482e6 100644 --- a/openedx/core/djangoapps/discussions/utils.py +++ b/openedx/core/djangoapps/discussions/utils.py @@ -191,16 +191,3 @@ def get_course_division_scheme(course_discussion_settings: CourseDiscussionSetti ): division_scheme = CourseDiscussionSettings.NONE return division_scheme - - -def show_discussion_tab(course_key, user_id: int) -> None: - """ - Toggle the visibility of the discussion tab in the course. - """ - store = modulestore() - course = store.get_course(course_key) - for tab in course.tabs: - if tab.tab_id == 'discussion': - tab.is_hidden = False - store.update_item(course, user_id) - break diff --git a/openedx/core/djangoapps/discussions/views.py b/openedx/core/djangoapps/discussions/views.py index adbb5867050f..9d27d8f5f8e1 100644 --- a/openedx/core/djangoapps/discussions/views.py +++ b/openedx/core/djangoapps/discussions/views.py @@ -21,7 +21,6 @@ DiscussionsConfigurationSerializer, DiscussionsProvidersSerializer, ) -from .utils import show_discussion_tab class DiscussionsConfigurationSettingsView(APIView): @@ -121,9 +120,8 @@ def update_configuration_data(request, course_key_string): new_provider_type = serializer.validated_data.get('provider_type', None) if new_provider_type is not None and new_provider_type != configuration.provider_type: check_course_permissions(course, request.user, 'change_provider') + serializer.save() - if configuration.is_enabled(course_key): - show_discussion_tab(course_key, request.user.id) return serializer.data From d7357b9e67142d8dff2e221c51cdde8038f78986 Mon Sep 17 00:00:00 2001 From: Paulo Viadanna Date: Tue, 3 Sep 2024 08:52:39 -0300 Subject: [PATCH 3/4] fix: linting issue --- openedx/core/djangoapps/discussions/serializers.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/openedx/core/djangoapps/discussions/serializers.py b/openedx/core/djangoapps/discussions/serializers.py index 1514a696f195..29403ebde00d 100644 --- a/openedx/core/djangoapps/discussions/serializers.py +++ b/openedx/core/djangoapps/discussions/serializers.py @@ -7,8 +7,6 @@ from rest_framework import serializers from xmodule.modulestore.django import modulestore -from opaque_keys.edx.keys import CourseKey - from openedx.core.djangoapps.discussions.tasks import update_discussions_settings_from_course_task from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings from openedx.core.lib.courses import get_course_by_id From 20ff3920ff775a316b8f64b75c4d5de4b06e9a82 Mon Sep 17 00:00:00 2001 From: Paulo Viadanna Date: Wed, 4 Sep 2024 09:24:34 -0300 Subject: [PATCH 4/4] refactor: improve code performance --- openedx/core/djangoapps/discussions/serializers.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/discussions/serializers.py b/openedx/core/djangoapps/discussions/serializers.py index 29403ebde00d..8eeb7e10278a 100644 --- a/openedx/core/djangoapps/discussions/serializers.py +++ b/openedx/core/djangoapps/discussions/serializers.py @@ -265,14 +265,6 @@ def update(self, instance: DiscussionsConfiguration, validated_data: dict) -> Di course_overview_id=instance.context_key, type='discussion' ).update(is_hidden=not instance.enabled) - # do the same for the modulestore representation - store = modulestore() - course = store.get_course(instance.context_key) - for tab in course.tabs: - if tab.tab_id == 'discussion': - tab.is_hidden = not instance.enabled - store.update_item(course, self.context['user_id']) - break update_discussions_settings_from_course_task.delay(str(instance.context_key)) return instance @@ -362,6 +354,12 @@ def _update_course_configuration( key not in LegacySettingsSerializer.Meta.fields_cohorts ) } + # toogle discussion tab is_hidden + for tab in course.tabs: + if tab.tab_id == 'discussion' and tab.is_hidden != instance.enabled: + tab.is_hidden = not instance.enabled + save = True + break if save: modulestore().update_item(course, self.context['user_id']) return instance