Merge pull request #13713 from open-craft/fix-reverted-itsjeyd/inline-discussion-assets

Fix Reverted: Have DiscussionXBlock take care of loading JS and CSS files it depends on

common/static/common/js/discussion/mathjax_include.js

+// See common/templates/mathjax_include.html for info on Fast Preview mode.
+var disableFastPreview = true,
+    vendorScript;
+if (typeof MathJax === 'undefined') {
+    if (disableFastPreview) {
+        window.MathJax = {
+            menuSettings: {CHTMLpreview: false}
+        };
+    }
+
+    vendorScript = document.createElement('script');
+    vendorScript.onload = function() {
+        'use strict';
+        var MathJax = window.MathJax,
+            setMathJaxDisplayDivSettings;
+        MathJax.Hub.Config({
+            tex2jax: {
+                inlineMath: [
+                    ['\\(', '\\)'],
+                    ['[mathjaxinline]', '[/mathjaxinline]']
+                ],
+                displayMath: [
+                    ['\\[', '\\]'],
+                    ['[mathjax]', '[/mathjax]']
+                ]
+            }
+        });
+        if (disableFastPreview) {
+            MathJax.Hub.processSectionDelay = 0;
+        }
+        MathJax.Hub.signal.Interest(function(message) {
+            if (message[0] === 'End Math') {
+                setMathJaxDisplayDivSettings();
+            }
+        });
+        setMathJaxDisplayDivSettings = function() {
+            $('.MathJax_Display').each(function() {
+                this.setAttribute('tabindex', '0');
+                this.setAttribute('aria-live', 'off');
+                this.removeAttribute('role');
+                this.removeAttribute('aria-readonly');
+            });
+        };
+    };
+    vendorScript.src = 'https://cdn.mathjax.org/mathjax/2.6-latest/MathJax.js?config=TeX-MML-AM_SVG';
+    document.body.appendChild(vendorScript);
+}

common/static/common/js/spec/discussion/view/discussion_thread_profile_view_spec.js

@@ -18,12 +18,6 @@
                 created_at: '2014-09-09T20:11:08Z'
             };
             this.imageTag = '<img src="https://www.google.com.pk/images/srpr/logo11w.png">';
-            window.MathJax = {
-                Hub: {
-                    Queue: function() {
-                    }
-                }
-            };
         });
         makeView = function(thread) {
             var view;

lms/djangoapps/teams/templates/teams/teams.html

@@ -15,6 +15,7 @@ from openedx.core.djangolib.js_utils import (
 <%block name="headextra">
 <%static:css group='style-course-vendor'/>
 <%static:css group='style-course'/>
+<%static:css group='style-inline-discussion'/>
 <%include file="../discussion/_js_head_dependencies.html" />
 </%block>
 

lms/envs/common.py

@@ -1290,6 +1290,7 @@ dashboard_js = (
     sorted(rooted_glob(PROJECT_ROOT / 'static', 'js/dashboard/**/*.js'))
 )
 discussion_js = (
+    rooted_glob(COMMON_ROOT / 'static', 'common/js/discussion/mathjax_include.js') +
     rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/customwmd.js') +
     rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/mathjax_accessible.js') +
     rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/mathjax_delay_renderer.js') +
@@ -1454,6 +1455,18 @@ PIPELINE_CSS = {
         ],
         'output_filename': 'css/discussion/lms-discussion-main-rtl.css',
     },
+    'style-inline-discussion': {
+        'source_filenames': [
+            'css/discussion/inline-discussion.css',
+        ],
+        'output_filename': 'css/discussion/inline-discussion.css',
+    },
+    'style-inline-discussion-rtl': {
+        'source_filenames': [
+            'css/discussion/inline-discussion-rtl.css',
+        ],
+        'output_filename': 'css/discussion/inline-discussion-rtl.css',
+    },
     'style-xmodule-annotations': {
         'source_filenames': [
             'css/vendor/ova/annotator.css',

lms/static/sass/_build-base-v1-rtl.scss

+// libs and resets *do not edit*
+@import 'bourbon/bourbon'; // lib - bourbon
+@import 'vendor/bi-app/bi-app-rtl'; // set the layout for right to left languages
+@import 'base/variables-rtl';
+
+// base - utilities
+@import 'base/reset';
+@import 'base/variables';
+@import 'base/mixins';

lms/static/sass/_build-base-v1.scss

+// libs and resets *do not edit*
+@import 'bourbon/bourbon'; // lib - bourbon
+@import 'vendor/bi-app/bi-app-ltr'; // set the layout for left to right languages
+
+// base - utilities
+@import 'base/reset';
+@import 'base/variables';
+@import 'base/mixins';

lms/static/sass/_build-course.scss

@@ -59,10 +59,6 @@
 // course - ccx_coach
 @import "course/ccx_coach/dashboard";
 
-// discussion
-@import "course/discussion/form-wmd-toolbar";
-@import "course/discussion/form";
-
 // search
 @import 'search/_search';
 

lms/static/sass/_build-lms-v1.scss

@@ -61,22 +61,6 @@
 @import 'course/auto-cert';
 @import 'views/api-access';
 
-// app - discussion
-@import "discussion/utilities/variables-v1";
-@import "discussion/mixins";
-@import 'discussion/discussion'; // Process old file after definitions but before everything else, partial is deprecated.
-@import 'discussion/discussion-v1'; // Non-Pattern Library styling
-@import "discussion/elements/actions";
-@import "discussion/elements/editor";
-@import "discussion/elements/labels";
-@import "discussion/elements/navigation";
-@import "discussion/views/home";
-@import "discussion/views/thread";
-@import "discussion/views/create-edit-post";
-@import "discussion/views/response";
-@import 'discussion/utilities/developer';
-@import 'discussion/utilities/shame';
-
 // search
 @import 'search/search';
 

lms/static/sass/discussion/_build-discussion.scss

+// ------------------------------
+// Discussions: Shared Build Compile
+
+// About: Sass compile for discussion elements that are shared between LTR and RTL UI. Configuration and vendor specific imports happen before this shared set of imports is compiled in the inline-discussion-*.scss files.
+
+// Set the relative path to the static root
+$static-path: '../..';
+
+// Styles for discussions
+@import "utilities/variables-v1";
+@import "mixins";
+@import 'discussion'; // Process old file after definitions but before everything else, partial is deprecated.
+@import 'discussion-v1'; // Non-Pattern Library styling
+@import "elements/actions";
+@import "elements/editor";
+@import "elements/labels";
+@import "elements/navigation";
+@import "views/home";
+@import "views/thread";
+@import "views/create-edit-post";
+@import "views/response";
+@import 'utilities/developer';
+@import 'utilities/shame';
+
+// Styles for WYSIWYG Markdown editor
+@import "../course/discussion/form-wmd-toolbar";
+@import "../course/discussion/form";

lms/static/sass/discussion/inline-discussion-rtl.scss

+// Discussions - CSS application architecture
+// Version 1 styling (pre-Pattern Library)
+// ====================
+
+@import '../build-base-v1-rtl';
+
+// base - elements
+@import '../elements/typography';
+
+// app - discussion
+@import 'build-discussion';

lms/static/sass/discussion/inline-discussion.scss

+// Discussions - CSS application architecture
+// Version 1 styling (pre-Pattern Library)
+// ====================
+
+@import '../build-base-v1';
+
+// base - elements
+@import '../elements/typography';
+
+// app - discussion
+@import 'build-discussion';

lms/static/sass/lms-main-v1-rtl.scss

@@ -2,17 +2,5 @@
 // Version 1 styling (pre-Pattern Library)
 // ====================
 
-// libs and resets *do not edit*
-@import 'bourbon/bourbon'; // lib - bourbon
-@import 'vendor/bi-app/bi-app-rtl'; // set the layout for right to left languages
-@import 'base/variables-rtl';
-
-// BASE  *default edX offerings*
-// ====================
-
-// base - utilities
-@import 'base/reset';
-@import 'base/variables';
-@import 'base/mixins';
-
+@import 'build-base-v1-rtl';
 @import 'build-lms-v1'; // shared app style assets/rendering

lms/static/sass/lms-main-v1.scss

@@ -2,16 +2,5 @@
 // Version 1 styling (pre-Pattern Library)
 // ====================
 
-// libs and resets *do not edit*
-@import 'bourbon/bourbon'; // lib - bourbon
-@import 'vendor/bi-app/bi-app-ltr'; // set the layout for left to right languages
-
-// BASE  *default edX offerings*
-// ====================
-
-// base - utilities
-@import 'base/reset';
-@import 'base/variables';
-@import 'base/mixins';
-
+@import 'build-base-v1';
 @import 'build-lms-v1'; // shared app style assets/rendering

lms/templates/courseware/courseware-chromeless.html

@@ -40,7 +40,10 @@ ${static.get_page_title_breadcrumbs(course_name())}
 <%static:css group='style-student-notes'/>
 % endif
 
-<%include file="../discussion/_js_head_dependencies.html" />
+<script type="text/javascript" src="${static.url('js/jquery.autocomplete.js')}"></script>
+<script type="text/javascript" src="${static.url('js/src/tooltip_manager.js')}"></script>
+
+<link href="${static.url('css/vendor/jquery.autocomplete.css')}" rel="stylesheet" type="text/css">
   ${HTML(fragment.head_html())}
 </%block>
 
@@ -53,7 +56,7 @@ ${static.get_page_title_breadcrumbs(course_name())}
 
   <%static:js group='courseware'/>
 
-  <%include file="../discussion/_js_body_dependencies.html" />
+  <%include file="/mathjax_include.html" args="disable_fast_preview=True"/>
   % if staff_access:
   	<%include file="xqa_interface.html"/>
   % endif

lms/templates/courseware/courseware.html

@@ -61,7 +61,10 @@ ${static.get_page_title_breadcrumbs(course_name())}
 <%static:css group='style-student-notes'/>
 % endif
 
-<%include file="../discussion/_js_head_dependencies.html" />
+<script type="text/javascript" src="${static.url('js/jquery.autocomplete.js')}"></script>
+<script type="text/javascript" src="${static.url('js/src/tooltip_manager.js')}"></script>
+
+<link href="${static.url('css/vendor/jquery.autocomplete.css')}" rel="stylesheet" type="text/css">
   ${HTML(fragment.head_html())}
 </%block>
 
@@ -73,7 +76,7 @@ ${static.get_page_title_breadcrumbs(course_name())}
   <script type="text/javascript" src="${static.url('js/vendor/codemirror-compressed.js')}"></script>
 
   <%static:js group='courseware'/>
-  <%include file="../discussion/_js_body_dependencies.html" />
+  <%include file="/mathjax_include.html" args="disable_fast_preview=True"/>
 
   % if settings.FEATURES.get('ENABLE_COURSEWARE_SEARCH'):
     <%static:require_module module_name="js/search/course/course_search_factory" class_name="CourseSearchFactory">

lms/templates/discussion/_discussion_inline.html

 <%page expression_filter="h"/>
+
 <%include file="_underscore_templates.html" />
+
 <%!
 from django.utils.translation import ugettext as _
 from json import dumps as json_dumps

openedx/core/lib/tests/test_xblock_utils.py

@@ -22,7 +22,11 @@ from openedx.core.lib.xblock_utils import (
     replace_jump_to_id_urls,
     replace_course_urls,
     replace_static_urls,
-    sanitize_html_id
+    sanitize_html_id,
+)
+from openedx.core.lib.xblock_builtin import (
+    get_css_dependencies,
+    get_js_dependencies,
 )
 
 
@@ -181,3 +185,41 @@ class TestXblockUtils(SharedModuleStoreTestCase):
         clean_string = sanitize_html_id(dirty_string)
 
         self.assertEqual(clean_string, 'I_have_un_allowed_characters')
+
+    @ddt.data(
+        (True, ["combined.css"]),
+        (False, ["a.css", "b.css", "c.css"]),
+    )
+    @ddt.unpack
+    def test_get_css_dependencies(self, pipeline_enabled, expected_css_dependencies):
+        """
+        Verify that `get_css_dependencies` returns correct list of files.
+        """
+        pipeline_css = {
+            'style-group': {
+                'source_filenames': ["a.css", "b.css", "c.css"],
+                'output_filename': "combined.css"
+            }
+        }
+        with self.settings(PIPELINE_ENABLED=pipeline_enabled, PIPELINE_CSS=pipeline_css):
+            css_dependencies = get_css_dependencies("style-group")
+            self.assertEqual(css_dependencies, expected_css_dependencies)
+
+    @ddt.data(
+        (True, ["combined.js"]),
+        (False, ["a.js", "b.js", "c.js"]),
+    )
+    @ddt.unpack
+    def test_get_js_dependencies(self, pipeline_enabled, expected_js_dependencies):
+        """
+        Verify that `get_js_dependencies` returns correct list of files.
+        """
+        pipeline_js = {
+            'js-group': {
+                'source_filenames': ["a.js", "b.js", "c.js"],
+                'output_filename': "combined.js"
+            }
+        }
+        with self.settings(PIPELINE_ENABLED=pipeline_enabled, PIPELINE_JS=pipeline_js):
+            js_dependencies = get_js_dependencies("js-group")
+            self.assertEqual(js_dependencies, expected_js_dependencies)

openedx/core/lib/xblock_builtin/__init__.py

+"""
+Helper functions shared by built-in XBlocks.
+
+"""
+
+
+from django.conf import settings
+
+
+def get_css_dependencies(group):
+    """
+    Returns list of CSS dependencies belonging to `group` in settings.PIPELINE_JS.
+
+    Respects `PIPELINE_ENABLED` setting.
+    """
+    if settings.PIPELINE_ENABLED:
+        return [settings.PIPELINE_CSS[group]['output_filename']]
+    else:
+        return settings.PIPELINE_CSS[group]['source_filenames']
+
+
+def get_js_dependencies(group):
+    """
+    Returns list of JS dependencies belonging to `group` in settings.PIPELINE_JS.
+
+    Respects `PIPELINE_ENABLED` setting.
+    """
+    if settings.PIPELINE_ENABLED:
+        return [settings.PIPELINE_JS[group]['output_filename']]
+    else:
+        return settings.PIPELINE_JS[group]['source_filenames']

openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion.py

@@ -4,6 +4,9 @@ Discussion XBlock
 """
 import logging
 
+from django.templatetags.static import static
+from django.utils.translation import get_language_bidi
+
 from xblockutils.resources import ResourceLoader
 from xblockutils.studio_editable import StudioEditableXBlockMixin
 from xmodule.raw_module import RawDescriptor
@@ -13,6 +16,9 @@ from xblock.fields import Scope, String, UNIQUE_ID
 from xblock.fragment import Fragment
 from xmodule.xml_module import XmlParserMixin
 
+from openedx.core.lib.xblock_builtin import get_css_dependencies, get_js_dependencies
+
+
 log = logging.getLogger(__name__)
 loader = ResourceLoader(__name__)  # pylint: disable=invalid-name
 
@@ -88,6 +94,57 @@ class DiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlParserMixin):
             return None
         return user_service._django_user  # pylint: disable=protected-access
 
+    @staticmethod
+    def vendor_js_dependencies():
+        """
+        Returns list of vendor JS files that this XBlock depends on.
+
+        The helper function that it uses to obtain the list of vendor JS files
+        works in conjunction with the Django pipeline to ensure that in development mode
+        the files are loaded individually, but in production just the single bundle is loaded.
+        """
+        return get_js_dependencies('discussion_vendor')
+
+    @staticmethod
+    def js_dependencies():
+        """
+        Returns list of JS files that this XBlock depends on.
+
+        The helper function that it uses to obtain the list of JS files
+        works in conjunction with the Django pipeline to ensure that in development mode
+        the files are loaded individually, but in production just the single bundle is loaded.
+        """
+        return get_js_dependencies('discussion')
+
+    @staticmethod
+    def css_dependencies():
+        """
+        Returns list of CSS files that this XBlock depends on.
+
+        The helper function that it uses to obtain the list of CSS files
+        works in conjunction with the Django pipeline to ensure that in development mode
+        the files are loaded individually, but in production just the single bundle is loaded.
+        """
+        if get_language_bidi():
+            return get_css_dependencies('style-inline-discussion-rtl')
+        else:
+            return get_css_dependencies('style-inline-discussion')
+
+    def add_resource_urls(self, fragment):
+        """
+        Adds URLs for JS and CSS resources that this XBlock depends on to `fragment`.
+        """
+        # Head dependencies
+        for vendor_js_file in self.vendor_js_dependencies():
+            fragment.add_resource_url(static(vendor_js_file), "application/javascript", "head")
+
+        for css_file in self.css_dependencies():
+            fragment.add_css_url(static(css_file))
+
+        # Body dependencies
+        for js_file in self.js_dependencies():
+            fragment.add_javascript_url(static(js_file))
+
     def has_permission(self, permission):
         """
         Encapsulates lms specific functionality, as `has_permission` is not
@@ -108,12 +165,11 @@ class DiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlParserMixin):
         """
         fragment = Fragment()
 
-        course = self.runtime.modulestore.get_course(self.course_key)
+        self.add_resource_urls(fragment)
 
         context = {
             'discussion_id': self.discussion_id,
             'user': self.django_user,
-            'course': course,
             'course_id': self.course_key,
             'can_create_thread': self.has_permission("create_thread"),
             'can_create_comment': self.has_permission("create_comment"),