Skip to content

Commit

Permalink
fix: discussion tab should be None if discussion tab is disabled (ope…
Browse files Browse the repository at this point in the history
  • Loading branch information
muhammadadeeltajamul authored and OmarIthawi committed May 31, 2024
1 parent 8a43fac commit fb0f16a
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 6 deletions.
3 changes: 1 addition & 2 deletions lms/djangoapps/mobile_api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def to_representation(self, course_overview): # lint-amnesty, pylint: disable=a
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,
Expand Down Expand Up @@ -74,7 +73,7 @@ def to_representation(self, course_overview): # lint-amnesty, pylint: disable=a
'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
Expand Down
49 changes: 49 additions & 0 deletions lms/djangoapps/mobile_api/users/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
)
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
Expand Down Expand Up @@ -713,3 +714,51 @@ def test_with_display_overrides(self, api_version):
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
9 changes: 5 additions & 4 deletions openedx/core/djangoapps/content/course_overviews/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField
from simple_history.models import HistoricalRecords

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
Expand Down Expand Up @@ -702,15 +701,17 @@ def get_all_course_keys(cls):
"""
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
Expand Down

0 comments on commit fb0f16a

Please sign in to comment.