From 60918fb160b5155df4a344c25f4271d8afb53d8b Mon Sep 17 00:00:00 2001 From: Kshitij Sobti Date: Wed, 13 Apr 2022 19:25:27 +0530 Subject: [PATCH] feat: use native learning MFE tab for discussions (#30109) Updates the discussions tab to return the learning MFE tab URL so the discussion content can be shown in the learning MFE itself. --- lms/djangoapps/courseware/tests/test_tabs.py | 31 ++++++++++++++++---- lms/djangoapps/discussion/plugins.py | 14 +++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index b159c20b84..bc4ce6ffdc 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -3,6 +3,8 @@ Test cases for tabs. """ from unittest.mock import MagicMock, Mock, patch + +import ddt import pytest from crum import set_current_request from django.contrib.auth.models import AnonymousUser @@ -22,6 +24,7 @@ from lms.djangoapps.courseware.tabs import ( ) from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase from lms.djangoapps.courseware.views.views import StaticCourseTabView, get_static_tab_fragment +from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.lib.courses import get_course_by_id from openedx.features.course_experience import DISABLE_UNIFIED_COURSE_TAB_FLAG @@ -35,6 +38,7 @@ from common.djangoapps.util.milestones_helpers import ( add_milestone, get_milestone_relationship_types ) +from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url from xmodule import tabs as xmodule_tabs # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order TEST_DATA_MIXED_MODULESTORE, @@ -72,7 +76,7 @@ class TabTestCase(SharedModuleStoreTestCase): """ Returns true if the specified tab is enabled. """ - return tab.is_enabled(course, user=user) + return tab is not None and tab.is_enabled(course, user=user) def set_up_books(self, num_books): """Initializes the textbooks in the course and adds the given number of books to each textbook""" @@ -773,6 +777,7 @@ class CourseInfoTabTestCase(TabTestCase): assert tab.type == 'course_info' +@ddt.ddt class DiscussionLinkTestCase(TabTestCase): """Test cases for discussion link tab.""" @@ -834,11 +839,19 @@ class DiscussionLinkTestCase(TabTestCase): ) @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def test_tabs_with_discussion(self): + @ddt.data(Provider.OPEN_EDX, Provider.LEGACY) + def test_tabs_with_discussion(self, provider): """Test a course with a discussion tab configured""" + config = DiscussionsConfiguration.get(self.course.id) + config.provider_type = provider + config.save() + if provider == Provider.OPEN_EDX: + expected_link = get_learning_mfe_home_url(course_key=self.course.id, url_fragment="discussion") + else: + expected_link = "default_discussion_link" self.check_discussion( tab_list=self.tabs_with_discussion, - expected_discussion_link="default_discussion_link", + expected_discussion_link=expected_link, expected_can_display_value=True, ) @@ -852,11 +865,19 @@ class DiscussionLinkTestCase(TabTestCase): ) @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) - def test_tabs_enrolled_or_staff(self): + @ddt.data(Provider.OPEN_EDX, Provider.LEGACY) + def test_tabs_enrolled_or_staff(self, provider): + config = DiscussionsConfiguration.get(self.course.id) + config.provider_type = provider + config.save() for is_enrolled, is_staff in [(True, False), (False, True)]: + if provider == Provider.OPEN_EDX: + expected_link = get_learning_mfe_home_url(course_key=self.course.id, url_fragment="discussion") + else: + expected_link = "default_discussion_link" self.check_discussion( tab_list=self.tabs_with_discussion, - expected_discussion_link="default_discussion_link", + expected_discussion_link=expected_link, expected_can_display_value=True, is_enrolled=is_enrolled, is_staff=is_staff diff --git a/lms/djangoapps/discussion/plugins.py b/lms/djangoapps/discussion/plugins.py index b97cd49e5d..12ad5061f6 100644 --- a/lms/djangoapps/discussion/plugins.py +++ b/lms/djangoapps/discussion/plugins.py @@ -9,6 +9,8 @@ from xmodule.tabs import TabFragmentViewMixin import lms.djangoapps.discussion.django_comment_client.utils as utils from lms.djangoapps.courseware.tabs import EnrolledTab +from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider +from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url from openedx.features.lti_course_tab.tab import DiscussionLtiCourseTab @@ -35,3 +37,15 @@ class DiscussionTab(TabFragmentViewMixin, EnrolledTab): if DiscussionLtiCourseTab.is_enabled(course, user): return False return utils.is_discussion_enabled(course.id) + + @property + def link_func(self): + legacy_link_func = super().link_func + + def _link_func(course, reverse_func): + config = DiscussionsConfiguration.get(course.id) + if config.provider_type == Provider.OPEN_EDX: + return get_learning_mfe_home_url(course_key=course.id, url_fragment=self.type) + else: + return legacy_link_func(course, reverse_func) + return _link_func