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.
This commit is contained in:
committed by
Farhaan Bukhsh
parent
2e9f000fe0
commit
9ee4afaaf1
@@ -474,11 +474,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)
|
||||
|
||||
|
||||
@@ -201,7 +201,10 @@ def update_unit_discussion_state_from_discussion_blocks(
|
||||
"""
|
||||
store = modulestore()
|
||||
course = store.get_course(course_key)
|
||||
provider = course.discussions_settings.get('provider', None)
|
||||
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:
|
||||
|
||||
Reference in New Issue
Block a user