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.

Author: xitij2000

cms/djangoapps/contentstore/tasks.py

@@ -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)
 

openedx/core/djangoapps/discussions/tasks.py

@@ -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: