From 4aedeb8988c2b7838e9ee210a7f3b704f2a8954d Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Tue, 18 Jul 2023 09:32:12 -0400 Subject: [PATCH] refactor: remove XModule JS from Django Pipeline (#32530) `module-js` and `module-descriptor-js` are old JavaScript group indicators, left over from when we managed XModule assets via Django Pipeline. We would like to get rid of them in order to make it easier to build XModule JS without using Python. There is one single usage of `module-js` in the entire platform (the rest have been replaced with Webpack references, which is the less-outdated way of managing XModule assets :). The lone `module-js` reference was added in 2013 [1] so that circuit diagrams would display in the course wiki. However, the ability to render circuits in the wiki was removed in 2015 [2], so it is safe to remove the reference. There is also one single usage of `module-descriptor-js`. It's in the legacy bulk email editor, which hackily cribs from the old HtmlBlock editor. Fortunately, we are able to simply replace the Django Pipeline reference with the equivalent XModule JS Webpack bundle. (Note: The old email editor is currently still supported, but is currently being replaced by frontend-app-communications, so this hack will be gone eventually). Finally, this commit also sneaks in one styling fix: it adds the HtmlBlockEditor CSS back to the aforementioned legacy bulk email page. The missing CSS was causing a read-only 1-line codemirror editor to appear below the HTML editor [3]. This bug was introduced during the original XModule SCSS decoupling [4], which removed builtin block CSS from the LMS-wide bundle, thus removing the HTML editor CSS from the bulk email page. We imagine that nobody noticed because the bug only exists in master (not Palm) and frontend-app-communications seems to be globally enabled on edx.org. As a simple fix, we add the new CSS link to the legacy bulk email page, and it renders fine again [5]. References: 1. https://github.com/openedx/edx-platform/commit/3fc59b3da5901370ceb2129a1cb8bcada09c3a99 2. https://github.com/openedx/edx-platform/pull/10324 3. Before fix: https://github.com/openedx/edx-platform/assets/3628148/25fc41b2-403d-4339-8c49-0b04664dfa02 4. https://github.com/openedx/edx-platform/pull/32018 5. After fix: https://github.com/openedx/edx-platform/assets/3628148/9a5d74f1-cc83-4ebe-8f0c-ee270f7721b8 Part of: https://github.com/openedx/edx-platform/issues/32481 --- cms/djangoapps/pipeline_js/utils.py | 19 ------------------- cms/envs/common.py | 9 --------- lms/envs/common.py | 8 -------- .../instructor_dashboard_2.html | 3 ++- lms/templates/wiki/preview_inline.html | 1 - xmodule/assets/README.rst | 3 --- 6 files changed, 2 insertions(+), 41 deletions(-) delete mode 100644 cms/djangoapps/pipeline_js/utils.py diff --git a/cms/djangoapps/pipeline_js/utils.py b/cms/djangoapps/pipeline_js/utils.py deleted file mode 100644 index 2584b023ac..0000000000 --- a/cms/djangoapps/pipeline_js/utils.py +++ /dev/null @@ -1,19 +0,0 @@ -""" -Utilities for returning XModule JS (used by requirejs) -""" - - -from django.conf import settings -from django.contrib.staticfiles.storage import staticfiles_storage - - -def get_xmodule_urls(): - """ - Returns a list of the URLs to hit to grab all the XModule JS - """ - pipeline_js_settings = settings.PIPELINE['JAVASCRIPT']["module-js"] - if settings.DEBUG: - paths = [path.replace(".coffee", ".js") for path in pipeline_js_settings["source_filenames"]] - else: - paths = [pipeline_js_settings["output_filename"]] - return [staticfiles_storage.url(path) for path in paths] diff --git a/cms/envs/common.py b/cms/envs/common.py index e3e321c568..bed1862fab 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1383,15 +1383,6 @@ PIPELINE['JAVASCRIPT'] = { 'source_filenames': base_vendor_js, 'output_filename': 'js/cms-base-vendor.js', }, - 'module-js': { - 'source_filenames': ( - rooted_glob(COMMON_ROOT / 'static/', 'xmodule/descriptors/js/*.js') + - rooted_glob(COMMON_ROOT / 'static/', 'xmodule/modules/js/*.js') + - rooted_glob(COMMON_ROOT / 'static/', 'common/js/discussion/*.js') - ), - 'output_filename': 'js/cms-modules.js', - 'test_order': 1 - }, } STATICFILES_IGNORE_PATTERNS = ( diff --git a/lms/envs/common.py b/lms/envs/common.py index cc953d355e..a9b5f0203f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2580,14 +2580,6 @@ PIPELINE['JAVASCRIPT'] = { 'source_filenames': main_vendor_js, 'output_filename': 'js/lms-main_vendor.js', }, - 'module-descriptor-js': { - 'source_filenames': rooted_glob(COMMON_ROOT / 'static/', 'xmodule/descriptors/js/*.js'), - 'output_filename': 'js/lms-module-descriptors.js', - }, - 'module-js': { - 'source_filenames': rooted_glob(COMMON_ROOT / 'static', 'xmodule/modules/js/*.js'), - 'output_filename': 'js/lms-modules.js', - }, 'discussion': { 'source_filenames': discussion_js, 'output_filename': 'js/discussion.js', diff --git a/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html b/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html index 7f0046a79d..c0392f3293 100644 --- a/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html +++ b/lms/templates/instructor/instructor_dashboard_2/instructor_dashboard_2.html @@ -54,7 +54,8 @@ from openedx.core.djangolib.markup import HTML - <%static:js group='module-descriptor-js'/> + <%static:webpack entry='HtmlBlockEditor'/> + <%static:js group='instructor_dash'/> <%static:js group='application'/> diff --git a/lms/templates/wiki/preview_inline.html b/lms/templates/wiki/preview_inline.html index 1dd3ede920..4fdab7a575 100644 --- a/lms/templates/wiki/preview_inline.html +++ b/lms/templates/wiki/preview_inline.html @@ -40,7 +40,6 @@ {% javascript 'application' %} - {% javascript 'module-js' %} {% with mathjax_mode='wiki' %} {% include "mathjax_include.html" %} {% endwith %} diff --git a/xmodule/assets/README.rst b/xmodule/assets/README.rst index c87642384f..618ba02924 100644 --- a/xmodule/assets/README.rst +++ b/xmodule/assets/README.rst @@ -75,14 +75,11 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and * `VerticalBlock`_ * `LibrarySourcedBlock`_ -* Some XBlock JS is also processed through Django Pipeline and used in a couple specific legacy places. - As part of an `active build refactoring`_: * We update the older builtin XBlocks to reference their JS directly rather than using copies of it. * We will move ``webpack.xmodule.config.js`` here instead of generating it. * We will consolidate all edx-platform XBlock JS here in `xmodule/assets`_. -* We will remove XBlock JS from Django Pipeline. * We will delete the ``xmodule_assets`` script. .. _xmodule/assets: https://github.com/openedx/edx-platform/tree/master/xmodule/assets