Merge pull request #25910 from open-craft/kshitij/lti-tab

[BD-03] [TNL-7805] [BB-3371] LTI Discussion Tab

lms/djangoapps/ccx/tests/test_field_override_performance.py

@@ -245,7 +245,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
     __test__ = True
 
     # TODO: decrease query count as part of REVO-28
-    QUERY_COUNT = 31
+    QUERY_COUNT = 33
     TEST_DATA = {
         # (providers, course_width, enable_ccx, view_as_ccx): (
         #     # of sql queries to default,
@@ -274,7 +274,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
     __test__ = True
 
     # TODO: decrease query count as part of REVO-28
-    QUERY_COUNT = 31
+    QUERY_COUNT = 33
 
     TEST_DATA = {
         ('no_overrides', 1, True, False): (QUERY_COUNT, 3),

lms/djangoapps/courseware/tests/test_views.py

@@ -269,8 +269,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
     NUM_PROBLEMS = 20
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 10, 173),
-        (ModuleStoreEnum.Type.split, 4, 169),
+        (ModuleStoreEnum.Type.mongo, 10, 175),
+        (ModuleStoreEnum.Type.split, 4, 171),
     )
     @ddt.unpack
     def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1425,8 +1425,8 @@ class ProgressPageTests(ProgressPageBaseTests):
             self.assertContains(resp, u"Download Your Certificate")
 
     @ddt.data(
-        (True, 53),
-        (False, 52),
+        (True, 55),
+        (False, 54),
     )
     @ddt.unpack
     def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1439,8 +1439,8 @@ class ProgressPageTests(ProgressPageBaseTests):
 
     @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
     @ddt.data(
-        (False, 61, 42),
-        (True, 52, 37)
+        (False, 63, 44),
+        (True, 54, 39)
     )
     @ddt.unpack
     def test_progress_queries(self, enable_waffle, initial, subsequent):

lms/djangoapps/courseware/views/views.py

@@ -134,6 +134,7 @@ from xmodule.x_module import STUDENT_VIEW
 from ..context_processor import user_timezone_locale_prefs
 from ..entrance_exams import user_can_skip_entrance_exam
 from ..module_render import get_module, get_module_by_usage_id, get_module_for_descriptor
+from ..tabs import _get_dynamic_tabs
 
 log = logging.getLogger("edx.courseware")
 
@@ -602,7 +603,8 @@ class CourseTabView(EdxFragmentView):
             course = get_course_with_access(request.user, 'load', course_key)
             try:
                 # Render the page
-                tab = CourseTabList.get_tab_by_type(course.tabs, tab_type)
+                course_tabs = course.tabs + _get_dynamic_tabs(course, request.user)
+                tab = CourseTabList.get_tab_by_type(course_tabs, tab_type)
                 page_context = self.create_page_context(request, course=course, tab=tab, **kwargs)
 
                 # Show warnings if the user has limited access

lms/djangoapps/discussion/plugins.py

@@ -8,6 +8,7 @@ from django.utils.translation import ugettext_noop
 
 import lms.djangoapps.discussion.django_comment_client.utils as utils
 from lms.djangoapps.courseware.tabs import EnrolledTab
+from openedx.features.lti_course_tab.tab import DiscussionLtiCourseTab
 from xmodule.tabs import TabFragmentViewMixin
 
 
@@ -30,4 +31,7 @@ class DiscussionTab(TabFragmentViewMixin, EnrolledTab):
     def is_enabled(cls, course, user=None):
         if not super(DiscussionTab, cls).is_enabled(course, user):
             return False
+        # Disable the regular discussion tab if LTI-based external Discussion forum is enabled
+        if DiscussionLtiCourseTab.is_enabled(course, user):
+            return False
         return utils.is_discussion_enabled(course.id)

lms/urls.py

@@ -746,6 +746,17 @@ urlpatterns += [
     ),
 ]
 
+urlpatterns += [
+    url(
+        r'^courses/{}/lti_tab/(?P<provider_uuid>[^/]+)/$'.format(
+            settings.COURSE_ID_PATTERN,
+        ),
+        CourseTabView.as_view(),
+        name='lti_course_tab',
+        kwargs={'tab_type': 'lti_tab'},
+    ),
+]
+
 urlpatterns += [
     # This MUST be the last view in the courseware--it's a catch-all for custom tabs.
     url(

openedx/features/course_experience/tests/views/test_course_home.py

@@ -208,7 +208,7 @@ class TestCourseHomePage(CourseHomePageTestCase):
 
         # Fetch the view and verify the query counts
         # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(75, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(78, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
             with check_mongo_calls(4):
                 url = course_home_url(self.course)
                 self.client.get(url)

openedx/features/course_experience/tests/views/test_course_updates.py

@@ -49,7 +49,7 @@ class TestCourseUpdatesPage(BaseCourseUpdatesTestCase):
 
         # Fetch the view and verify that the query counts haven't changed
         # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(49, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(52, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
             with check_mongo_calls(4):
                 url = course_updates_url(self.course)
                 self.client.get(url)

openedx/features/lti_course_tab/tab.py

@@ -0,0 +1,247 @@
+"""
+Code related to LTI course tab functionality.
+"""
+from typing import Dict
+from urllib.parse import quote
+
+from django.contrib.auth.models import AbstractBaseUser
+from django.contrib.sites.shortcuts import get_current_site
+from django.http import HttpRequest
+from django.utils.translation import get_language, to_locale, ugettext_lazy
+from lti_consumer.lti_1p1.contrib.django import lti_embed
+from lti_consumer.models import LtiConfiguration
+from opaque_keys.edx.keys import CourseKey
+from web_fragments.fragment import Fragment
+
+from lms.djangoapps.courseware.access import get_user_role
+from lms.djangoapps.courseware.tabs import EnrolledTab
+from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
+from openedx.core.djangolib.markup import HTML
+from common.djangoapps.student.models import anonymous_id_for_user
+from xmodule.course_module import CourseDescriptor
+from xmodule.tabs import TabFragmentViewMixin, key_checker
+
+
+class LtiCourseLaunchMixin:
+    """
+    Mixin that encapsulates all LTI-related functionality from the View
+    """
+
+    ROLE_MAP = {
+        'student': 'Student',
+        'staff': 'Administrator',
+        'instructor': 'Instructor',
+    }
+    DEFAULT_ROLE = 'Student'
+
+    def _get_additional_lti_parameters(self, course: CourseDescriptor, request: HttpRequest) -> Dict[str, str]:
+        lti_config = self._get_lti_config(course)
+        additional_config = lti_config.lti_config.get('additional_parameters', {})
+        return additional_config
+
+    @staticmethod
+    def _get_user_id(user: AbstractBaseUser, course_key: CourseKey):
+        return anonymous_id_for_user(user, course_key)
+
+    def _get_lti_roles(self, user: AbstractBaseUser, course_key: CourseKey) -> str:
+        return self.ROLE_MAP.get(
+            get_user_role(user, course_key),
+            self.DEFAULT_ROLE,
+        )
+
+    @staticmethod
+    def _get_context_id(course_key: CourseKey) -> str:
+        return quote(str(course_key))
+
+    @staticmethod
+    def _get_resource_link_id(course_key: CourseKey, request: HttpRequest) -> str:
+        site = get_current_site(request)
+        return '{}-{}'.format(
+            site.domain,
+            str(course_key.make_usage_key('course', course_key.run)),
+        )
+
+    @staticmethod
+    def _get_result_sourcedid(context_id: str, resource_link_id: str, user_id: str) -> str:
+        return "{context}:{resource_link}:{user_id}".format(
+            context=context_id,
+            resource_link=resource_link_id,
+            user_id=user_id,
+        )
+
+    @staticmethod
+    def _get_context_title(course: CourseDescriptor) -> str:
+        return "{} - {}".format(
+            course.display_name_with_default,
+            course.display_org_with_default,
+        )
+
+    def _get_lti_config(self, course: CourseDescriptor) -> LtiConfiguration:
+        raise NotImplementedError
+
+    def _get_lti_embed_code(self, course: CourseDescriptor, request: HttpRequest) -> str:
+        """
+        Returns the LTI embed code for embedding in the current course context.
+        Args:
+            course (CourseDescriptor): CourseDescriptor object.
+            request (HttpRequest): Request object for view in which LTI will be embedded.
+        Returns:
+            HTML code to embed LTI in course page.
+        """
+        course_key = course.id
+        lti_config = self._get_lti_config(course)
+        lti_consumer = lti_config.get_lti_consumer()
+        user_id = quote(self._get_user_id(request.user, course_key))
+        context_id = quote(self._get_context_id(course_key))
+        resource_link_id = quote(self._get_resource_link_id(course_key, request))
+        roles = self._get_lti_roles(request.user, course_key)
+        context_title = self._get_context_title(course)
+        result_sourcedid = quote(self._get_result_sourcedid(context_id, resource_link_id, user_id))
+        additional_params = self._get_additional_lti_parameters(course, request)
+        locale = to_locale(get_language())
+
+        return lti_embed(
+            html_element_id='lti-tab-launcher',
+            lti_consumer=lti_consumer,
+            resource_link_id=resource_link_id,
+            user_id=user_id,
+            roles=roles,
+            context_id=context_id,
+            context_title=context_title,
+            context_label=context_id,
+            result_sourcedid=result_sourcedid,
+            launch_presentation_locale=locale,
+            **additional_params,
+        )
+
+    # pylint: disable=unused-argument
+    def render_to_fragment(self, request: HttpRequest, course: CourseDescriptor, **kwargs) -> Fragment:
+        """
+        Returns a fragment view for the LTI launch.
+        Args:
+            request (HttpRequest): request object
+            course (CourseDescriptor): A course object
+        Returns:
+            A Fragment that embeds LTI in a course page.
+        """
+        lti_embed_html = self._get_lti_embed_code(course, request)
+
+        fragment = Fragment(
+            HTML(
+                """
+                <iframe
+                    id='lti-tab-embed'
+                    srcdoc='{srcdoc}'
+                 >
+                </iframe>
+                """
+            ).format(
+                srcdoc=lti_embed_html
+            )
+        )
+        fragment.add_css(
+            """
+            #lti-tab-embed {
+                width: 100%;
+                min-height: 400px;
+                border: none;
+            }
+            """
+        )
+        return fragment
+
+
+class LtiCourseTab(LtiCourseLaunchMixin, EnrolledTab):
+    """
+    A tab to add custom LTI components to a course in a tab.
+    """
+    type = 'lti_tab'
+    is_default = False
+    allow_multiple = True
+
+    def _get_lti_config(self, course: CourseDescriptor) -> LtiConfiguration:
+        return LtiConfiguration.objects.get(config_id=self.lti_config_id)
+
+    def __init__(self, tab_dict=None, name=None, lti_config_id=None):
+        def link_func(course, reverse_func):
+            """ Returns a function that returns the lti tab's URL. """
+            return reverse_func('lti_course_tab', args=[str(course.id), self.lti_config_id])
+
+        self.lti_config_id = tab_dict.get('lti_config_id') if tab_dict else lti_config_id
+
+        if tab_dict is None:
+            tab_dict = dict()
+
+        if name is not None:
+            tab_dict['name'] = name
+
+        tab_dict['link_func'] = link_func
+        tab_dict['tab_id'] = 'lti_tab_{0}'.format(self.lti_config_id)
+
+        super().__init__(tab_dict)
+
+    @classmethod
+    def validate(cls, tab_dict, raise_error=True):
+        """
+        Ensures that the specified tab_dict is valid.
+        """
+        return (
+            super().validate(tab_dict, raise_error)
+            and key_checker(['name', 'lti_config_id'])(tab_dict, raise_error)
+        )
+
+    def __getitem__(self, key):
+        if key == 'lti_config_id':
+            return self.lti_config_id
+        else:
+            return super().__getitem__(key)
+
+    def __setitem__(self, key, value):
+        if key == 'lti_config_id':
+            self.lti_config_id = value
+        else:
+            super().__setitem__(key, value)
+
+    def to_json(self):
+        """
+        Return a dictionary representation of this tab.
+        """
+        to_json_val = super().to_json()
+        to_json_val.update({'lti_config_id': self.lti_config_id})
+        return to_json_val
+
+    def __eq__(self, other):
+        if not super().__eq__(other):
+            return False
+        return self.lti_config_id == other.get('lti_config_id')
+
+    def __hash__(self):
+        """
+        Return a hash representation of Tab Object.
+        """
+        return hash(repr(self))
+
+
+class DiscussionLtiCourseTab(LtiCourseLaunchMixin, TabFragmentViewMixin, EnrolledTab):
+    """
+    Course tab that loads the associated LTI-based discussion provider in a tab.
+    """
+    type = 'lti_discussion'
+    allow_multiple = False
+    is_dynamic = True
+    title = ugettext_lazy("Discussion")
+
+    def _get_lti_config(self, course: CourseDescriptor) -> LtiConfiguration:
+        config = DiscussionsConfiguration.get(course.id)
+        return config.lti_configuration
+
+    @classmethod
+    def is_enabled(cls, course, user=None):
+        if super().is_enabled(course, user):
+            config = DiscussionsConfiguration.get(course.id)
+            return (
+                config.enabled and
+                config.lti_configuration is not None
+            )
+        else:
+            return False

openedx/features/lti_course_tab/tests.py

@@ -0,0 +1,39 @@
+"""
+Tests for LTI Course tabs.
+"""
+from unittest.mock import Mock, patch
+
+from lms.djangoapps.courseware.tests.test_tabs import TabTestCase
+from openedx.features.lti_course_tab.tab import DiscussionLtiCourseTab
+
+
+class DiscussionLtiCourseTabTestCase(TabTestCase):
+    """Test cases for LTI Discussion Tab."""
+
+    def check_discussion_tab(self):
+        """Helper function for verifying the LTI discussion tab."""
+        return self.check_tab(
+            tab_class=DiscussionLtiCourseTab,
+            dict_tab={'type': DiscussionLtiCourseTab.type, 'name': 'same'},
+            expected_link=self.reverse('course_tab_view', args=[str(self.course.id), DiscussionLtiCourseTab.type]),
+            expected_tab_id=DiscussionLtiCourseTab.type,
+            invalid_dict_tab=None,
+        )
+
+    @patch('openedx.features.lti_course_tab.tab.DiscussionsConfiguration.get')
+    @patch('common.djangoapps.student.models.CourseEnrollment.is_enrolled')
+    def test_discussion_lti_tab(self, is_enrolled, discussion_config_get):
+        is_enrolled.return_value = True
+        mock_config = Mock()
+        mock_config.lti_configuration = {}
+        mock_config.enabled = False
+        discussion_config_get.return_value = mock_config
+        tab = self.check_discussion_tab()
+        self.check_can_display_results(
+            tab, for_staff_only=True, for_enrolled_users_only=True, expected_value=False
+        )
+        mock_config.enabled = True
+        self.check_discussion_tab()
+        self.check_can_display_results(
+            tab, for_staff_only=True, for_enrolled_users_only=True
+        )

requirements/edx-sandbox/py35.txt

@@ -20,7 +20,7 @@ matplotlib==2.2.4         # via -c requirements/edx-sandbox/../constraints.txt,
 mpmath==1.1.0             # via sympy
 networkx==2.2             # via -r requirements/edx-sandbox/py35.in
 nltk==3.5                 # via -r requirements/edx-sandbox/shared.txt, chem
-numpy==1.16.5             # via -c requirements/edx-sandbox/../constraints.txt, -r requirements/edx-sandbox/py35.in, chem, matplotlib, openedx-calc, scipy
+numpy==1.16.5             # via -c requirements/edx-sandbox/../constraints.txt, -r requirements/edx-sandbox/py35.in, chem, matplotlib, openedx-calc
 openedx-calc==1.0.9       # via -r requirements/edx-sandbox/py35.in
 pycparser==2.20           # via -r requirements/edx-sandbox/shared.txt, cffi
 pyparsing==2.2.0          # via -r requirements/edx-sandbox/py35.in, chem, matplotlib, openedx-calc

requirements/edx/base.txt

@@ -67,7 +67,7 @@ django-mysql==3.10.0      # via -r requirements/edx/base.in
 django-oauth-toolkit==1.3.2  # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.in
 django-object-actions==3.0.1  # via edx-enterprise
 django-pipeline==1.7.0    # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.in
-django-pyfs==2.2          # via -r requirements/edx/base.in
+django-pyfs==3.0          # via -r requirements/edx/base.in
 git+https://github.com/edx/django-ratelimit-backend.git@v2.0.1a5#egg=django-ratelimit-backend==2.0.1a5  # via -r requirements/edx/github.in
 django-ratelimit==3.0.1   # via -r requirements/edx/base.in
 django-require==1.0.11    # via -r requirements/edx/base.in
@@ -126,7 +126,7 @@ future==0.18.2            # via django-ses, edx-celeryutils, edx-enterprise, pyc
 geoip2==3.0.0             # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.in
 glob2==0.7                # via -r requirements/edx/base.in
 gunicorn==20.0.4          # via -r requirements/edx/base.in
-help-tokens==1.1.3        # via -r requirements/edx/base.in
+help-tokens==2.0.0        # via -r requirements/edx/base.in
 html5lib==1.1             # via -r requirements/edx/base.in, ora2
 icalendar==4.0.7          # via -r requirements/edx/base.in
 idna==2.10                # via -r requirements/edx/paver.txt, requests

requirements/edx/development.txt

@@ -78,7 +78,7 @@ django-mysql==3.10.0      # via -r requirements/edx/testing.txt
 django-oauth-toolkit==1.3.2  # via -c requirements/edx/../constraints.txt, -r requirements/edx/testing.txt
 django-object-actions==3.0.1  # via -r requirements/edx/testing.txt, edx-enterprise
 django-pipeline==1.7.0    # via -c requirements/edx/../constraints.txt, -r requirements/edx/testing.txt
-django-pyfs==2.2          # via -r requirements/edx/testing.txt
+django-pyfs==3.0          # via -r requirements/edx/testing.txt
 git+https://github.com/edx/django-ratelimit-backend.git@v2.0.1a5#egg=django-ratelimit-backend==2.0.1a5  # via -r requirements/edx/testing.txt
 django-ratelimit==3.0.1   # via -r requirements/edx/testing.txt
 django-require==1.0.11    # via -r requirements/edx/testing.txt
@@ -146,7 +146,7 @@ gitdb==4.0.5              # via -r requirements/edx/testing.txt, gitpython
 gitpython==3.1.12         # via -r requirements/edx/testing.txt, transifex-client
 glob2==0.7                # via -r requirements/edx/testing.txt
 gunicorn==20.0.4          # via -r requirements/edx/testing.txt
-help-tokens==1.1.3        # via -r requirements/edx/testing.txt
+help-tokens==2.0.0        # via -r requirements/edx/testing.txt
 html5lib==1.1             # via -r requirements/edx/testing.txt, ora2
 httpretty==0.9.7          # via -c requirements/edx/../constraints.txt, -r requirements/edx/testing.txt
 icalendar==4.0.7          # via -r requirements/edx/testing.txt

requirements/edx/testing.txt

@@ -76,7 +76,7 @@ django-mysql==3.10.0      # via -r requirements/edx/base.txt
 django-oauth-toolkit==1.3.2  # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.txt
 django-object-actions==3.0.1  # via -r requirements/edx/base.txt, edx-enterprise
 django-pipeline==1.7.0    # via -c requirements/edx/../constraints.txt, -r requirements/edx/base.txt
-django-pyfs==2.2          # via -r requirements/edx/base.txt
+django-pyfs==3.0          # via -r requirements/edx/base.txt
 git+https://github.com/edx/django-ratelimit-backend.git@v2.0.1a5#egg=django-ratelimit-backend==2.0.1a5  # via -r requirements/edx/base.txt
 django-ratelimit==3.0.1   # via -r requirements/edx/base.txt
 django-require==1.0.11    # via -r requirements/edx/base.txt
@@ -142,7 +142,7 @@ gitdb==4.0.5              # via gitpython
 gitpython==3.1.12         # via transifex-client
 glob2==0.7                # via -r requirements/edx/base.txt
 gunicorn==20.0.4          # via -r requirements/edx/base.txt
-help-tokens==1.1.3        # via -r requirements/edx/base.txt
+help-tokens==2.0.0        # via -r requirements/edx/base.txt
 html5lib==1.1             # via -r requirements/edx/base.txt, ora2
 httpretty==0.9.7          # via -c requirements/edx/../constraints.txt, -r requirements/edx/testing.in
 icalendar==4.0.7          # via -r requirements/edx/base.txt

setup.py

@@ -28,6 +28,8 @@ setup(
             "external_link = lms.djangoapps.courseware.tabs:ExternalLinkCourseTab",
             "html_textbooks = lms.djangoapps.courseware.tabs:HtmlTextbookTabs",
             "instructor = lms.djangoapps.instructor.views.instructor_dashboard:InstructorDashboardTab",
+            "lti_discussion = openedx.features.lti_course_tab.tab:DiscussionLtiCourseTab",
+            "lti_tab = openedx.features.lti_course_tab.tab:LtiCourseTab",
             "pdf_textbooks = lms.djangoapps.courseware.tabs:PDFTextbookTabs",
             "progress = lms.djangoapps.courseware.tabs:ProgressTab",
             "static_tab = xmodule.tabs:StaticTab",