Skip to content

Commit

Permalink
refactor: remove XModule JS from Django Pipeline
Browse files Browse the repository at this point in the history
`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. openedx@3fc59b3
2. openedx#10324
3. Before fix: https://github.com/openedx/edx-platform/assets/3628148/25fc41b2-403d-4339-8c49-0b04664dfa02
4. openedx#32018
5. After fix: https://github.com/openedx/edx-platform/assets/3628148/9a5d74f1-cc83-4ebe-8f0c-ee270f7721b8

Part of: openedx#32481
  • Loading branch information
kdmccormick committed Jul 17, 2023
1 parent 4b64d83 commit 41e7ae4
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 41 deletions.
19 changes: 0 additions & 19 deletions cms/djangoapps/pipeline_js/utils.py

This file was deleted.

9 changes: 0 additions & 9 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -1383,15 +1383,6 @@
'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 = (
Expand Down
8 changes: 0 additions & 8 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2586,14 +2586,6 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
'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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js')}"></script>
<script type="text/javascript" src="${static.url('js/vendor/jQuery-File-Upload/js/jquery.fileupload.js')}"></script>
<script type="text/javascript" src="${static.url('js/vendor/jquery.qubit.js')}"></script>
<%static:js group='module-descriptor-js'/>
<%static:webpack entry='HtmlBlockEditor'/>
<link rel="stylesheet" href="${static.url('css/HtmlBlockEditor.css')}">
<%static:js group='instructor_dash'/>
<%static:js group='application'/>

Expand Down
1 change: 0 additions & 1 deletion lms/templates/wiki/preview_inline.html
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ <h1 class="page-header">{{ title }}</h1>
</section>

{% javascript 'application' %}
{% javascript 'module-js' %}
{% with mathjax_mode='wiki' %}
{% include "mathjax_include.html" %}
{% endwith %}
Expand Down
3 changes: 0 additions & 3 deletions xmodule/assets/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 41e7ae4

Please sign in to comment.