fix: discussion tab should be None if discussion tab is disabled (#33861)

lms/djangoapps/mobile_api/users/serializers.py

@@ -23,7 +23,6 @@ class CourseOverviewField(serializers.RelatedField):  # lint-amnesty, pylint: di
         request = self.context.get('request')
         api_version = self.context.get('api_version')
         enrollment = CourseEnrollment.get_enrollment(user=self.context.get('request').user, course_key=course_id)
-
         return {
             # identifiers
             'id': course_id,
@@ -74,7 +73,7 @@ class CourseOverviewField(serializers.RelatedField):  # lint-amnesty, pylint: di
                 'discussion_course',
                 kwargs={'course_id': course_id},
                 request=request,
-            ) if course_overview.is_discussion_tab_enabled() else None,
+            ) if course_overview.is_discussion_tab_enabled(request.user) else None,
 
             # This is an old API that was removed as part of DEPR-4. We keep the
             # field present in case API parsers expect it, but this API is now

lms/djangoapps/mobile_api/users/tests.py

@@ -36,6 +36,7 @@ from lms.djangoapps.mobile_api.testutils import (
 )
 from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3
 from openedx.core.lib.courses import course_image_url
+from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
 from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
 from openedx.features.course_experience.tests.views.helpers import add_course_mode
 from xmodule.course_block import DEFAULT_START_DATE  # lint-amnesty, pylint: disable=wrong-import-order
@@ -713,3 +714,51 @@ class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin)
         assert serialized['course']['number'] == self.course.display_coursenumber
         assert serialized['course']['org'] == self.course.display_organization
         self._expiration_in_response(serialized, api_version)
+
+
+@ddt.ddt
+class TestDiscussionCourseEnrollmentSerializer(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin):
+    """
+    Tests discussion data in course enrollment serializer
+    """
+
+    def setUp(self):
+        """
+        Setup data for test
+        """
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_DISCUSSION_SERVICE': True}):
+            super().setUp()
+        self.login_and_enroll()
+        self.request = RequestFactory().get('/')
+        self.request.user = self.user
+
+    def get_serialized_data(self, api_version):
+        """
+        Return data from CourseEnrollmentSerializer
+        """
+        if api_version == API_V05:
+            serializer = CourseEnrollmentSerializerv05
+        else:
+            serializer = CourseEnrollmentSerializer
+
+        return serializer(
+            CourseEnrollment.enrollments_for_user(self.user)[0],
+            context={'request': self.request, 'api_version': api_version},
+        ).data
+
+    @ddt.data(True, False)
+    def test_discussion_tab_url(self, discussion_tab_enabled):
+        """
+        Tests discussion tab url is None if tab is disabled
+        """
+        config, _ = DiscussionsConfiguration.objects.get_or_create(context_key=self.course.id)
+        config.enabled = discussion_tab_enabled
+        config.save()
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_DISCUSSION_SERVICE': True}):
+            serialized = self.get_serialized_data(API_V2)
+        discussion_url = serialized["course"]["discussion_url"]
+        if discussion_tab_enabled:
+            assert discussion_url is not None
+            assert isinstance(discussion_url, str)
+        else:
+            assert discussion_url is None

openedx/core/djangoapps/content/course_overviews/models.py

@@ -24,7 +24,6 @@ from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField
 from simple_history.models import HistoricalRecords
 
 from lms.djangoapps.courseware.model_data import FieldDataCache
-from lms.djangoapps.discussion import django_comment_client
 from openedx.core.djangoapps.catalog.models import CatalogIntegration
 from openedx.core.djangoapps.lang_pref.api import get_closest_released_language
 from openedx.core.djangoapps.models.course_details import CourseDetails
@@ -708,15 +707,17 @@ class CourseOverview(TimeStampedModel):
         """
         return CourseOverview.objects.values_list('id', flat=True)
 
-    def is_discussion_tab_enabled(self):
+    def is_discussion_tab_enabled(self, user=None):
         """
         Returns True if course has discussion tab and is enabled
         """
+        # Importing here to avoid circular import
+        from lms.djangoapps.discussion.plugins import DiscussionTab
         tabs = self.tab_set.all()
         # creates circular import; hence explicitly referenced is_discussion_enabled
         for tab in tabs:
-            if tab.tab_id == "discussion" and django_comment_client.utils.is_discussion_enabled(self.id):
-                return True
+            if tab.tab_id == "discussion":
+                return DiscussionTab.is_enabled(self, user)
         return False
 
     @property