From 0ebcfdfbddce32e3d34c8611d69f5c9e90843e31 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 8 Oct 2025 18:35:37 +0200 Subject: [PATCH 1/2] Revert "fix: use a single 'provider_type' key for storing discussion provider type in course (#722)" This reverts commit 8612f2acfaca1a312830a3bf5755b6868ec0c92a. --- cms/djangoapps/contentstore/tasks.py | 2 +- openedx/core/djangoapps/discussions/tasks.py | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 863863238036..faaf9dc7e1ca 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -456,11 +456,11 @@ def sync_discussion_settings(course_key, user): if ( ENABLE_NEW_STRUCTURE_DISCUSSIONS.is_enabled() and not course.discussions_settings['provider_type'] == Provider.OPEN_EDX - and not course.discussions_settings['provider'] == Provider.OPEN_EDX ): LOGGER.info(f"New structure is enabled, also updating {course_key} to use new provider") course.discussions_settings['enable_graded_units'] = False course.discussions_settings['unit_level_visibility'] = True + course.discussions_settings['provider'] = Provider.OPEN_EDX course.discussions_settings['provider_type'] = Provider.OPEN_EDX modulestore().update_item(course, user.id) diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index 27682246a017..fea20dc59bd4 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -196,10 +196,7 @@ def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, u """ store = modulestore() course = store.get_course(course_key) - provider = course.discussions_settings.get( - 'provider_type', - course.discussions_settings.get('provider', None), - ) + provider = course.discussions_settings.get('provider', None) # Only migrate to the new discussion provider if the current provider is the legacy provider. log.info(f"Current provider for {course_key} is {provider}") if provider is not None and provider != Provider.LEGACY and not force: From 18e2d56ce3754f7c8a3a4038d7be6d07cc93bd41 Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Tue, 17 Dec 2024 14:41:35 +0530 Subject: [PATCH 2/2] fix: use a single 'provider_type' key for storing discussion provider type in course Both 'provider' and 'provider_type' have been used for storing the discussion provider type in course 'discussions_settings' field, there are some places in the code checking for 'provider' and others checking for 'provider_type', in some cases this can cause a bug where it doesn't detect the correct provider which causes discussion settings not being copied correctly when a course is cloned. This change prioritises the `provider_type` setting over `provider` and reads `provider` only as a fallback. The `provider` setting is now made read-only just for backwards-compatibility, to avoid confusion. (cherry picked from commit 9ee4afaaf199dddcf0444224e775b46f0aad3847) --- cms/djangoapps/contentstore/tasks.py | 4 ++-- openedx/core/djangoapps/discussions/tasks.py | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index faaf9dc7e1ca..3c5f60ca7c82 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -455,12 +455,12 @@ def sync_discussion_settings(course_key, user): if ( ENABLE_NEW_STRUCTURE_DISCUSSIONS.is_enabled() - and not course.discussions_settings['provider_type'] == Provider.OPEN_EDX + and not course.discussions_settings.get('provider_type', None) == Provider.OPEN_EDX + and not course.discussions_settings.get('provider', None) == Provider.OPEN_EDX ): LOGGER.info(f"New structure is enabled, also updating {course_key} to use new provider") course.discussions_settings['enable_graded_units'] = False course.discussions_settings['unit_level_visibility'] = True - course.discussions_settings['provider'] = Provider.OPEN_EDX course.discussions_settings['provider_type'] = Provider.OPEN_EDX modulestore().update_item(course, user.id) diff --git a/openedx/core/djangoapps/discussions/tasks.py b/openedx/core/djangoapps/discussions/tasks.py index fea20dc59bd4..4b08112f8266 100644 --- a/openedx/core/djangoapps/discussions/tasks.py +++ b/openedx/core/djangoapps/discussions/tasks.py @@ -196,7 +196,14 @@ def update_unit_discussion_state_from_discussion_blocks(course_key: CourseKey, u """ store = modulestore() course = store.get_course(course_key) - provider = course.discussions_settings.get('provider', None) + # The provider information has been written to both `provider_type` and `provider`. + # Both of these serve the same purpose and this is an accident of early development. + # The `provider_type` key is now treated as read-only to allow existing values + # to be respected while moving to the `provider` key in the future. + provider = course.discussions_settings.get( + 'provider_type', + course.discussions_settings.get('provider', None), + ) # Only migrate to the new discussion provider if the current provider is the legacy provider. log.info(f"Current provider for {course_key} is {provider}") if provider is not None and provider != Provider.LEGACY and not force: