build: commit builtinblocks Webpack config and stub out xmodule_assets

The Webpack configuration file for built-in XBlock JS used to be generated at build time and git-ignored. It lived at common/static/xmodule/webpack.xmodule.config.js. It was generated because the JS that it referred to was also generated at build-time, and the filenames of those JS modules were not static. Now that its contents have been made entirely static [1], there is no reason we need to continue generating this Webpack configuration file. So, we check it into edx-platform under the name ./webpack.builtinblocks.config.js. We choose to put it in the repo's root directory because the paths contained in the config file are relative to the repo's root. This allows us to behead both the xmodule/static_content.py (`xmodule_assets`) script andthe `process_xmodule_assets` paver task, a major step in removing the need for Python in the edx-platform asset build [2]. It also allows us to delete the `HTMLSnippet` class and all associated attributes, which were exclusively used by xmodule/static_content.py.. We leave `xmodule_assets` and `process_xmodule_assets` in as stubs for now in order to avoid breaking external code (like Tutor) which calls Paver; the entire pavelib/assets.py function will be eventually removed soon anyway [3]. Further, to avoid extraneous refactoring, we keep one method of `HTMLSnippet` around on a few of its former subclasses: `get_html`. This method was originally part of the XModule framework; now, it is left over on a few classes as a simple internal helper method. References: 1. openedx#32480 2. openedx#31800 3. openedx#31895 Part of: openedx#32481
kdmccormick · Jul 27, 2023 · 7eb4836 · 7eb4836
1 parent 54467b4
commit 7eb4836
Show file tree

Hide file tree

Showing 24 changed files with 164 additions and 481 deletions.
diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py
@@ -221,7 +221,7 @@ def test_has_permission(self):
         """
         permission_canary = object()
         with mock.patch(
-            'lms.djangoapps.discussion.django_comment_client.permissions.has_permission',
+            'xmodule.discussion_block.has_permission',
             return_value=permission_canary,
         ) as has_perm:
             actual_permission = self.block.has_permission("test_permission")

diff --git a/pavelib/assets.py b/pavelib/assets.py
@@ -323,7 +323,6 @@ class SassWatcher(PatternMatchingEventHandler):
     """
     ignore_directories = True
     patterns = ['*.scss']
-    ignore_patterns = ['common/static/xmodule/*']
 
     def register(self, observer, directories):
         """
@@ -352,47 +351,6 @@ def on_any_event(self, event):
             traceback.print_exc()
 
 
-class XModuleSassWatcher(SassWatcher):
-    """
-    Watches for sass file changes
-    """
-    ignore_directories = True
-    ignore_patterns = []
-
-    @debounce()
-    def on_any_event(self, event):
-        print('\tCHANGED:', event.src_path)
-        try:
-            process_xmodule_assets()
-        except Exception:  # pylint: disable=broad-except
-            traceback.print_exc()
-
-
-class XModuleAssetsWatcher(PatternMatchingEventHandler):
-    """
-    Watches for css and js file changes
-    """
-    ignore_directories = True
-    patterns = ['*.css', '*.js']
-
-    def register(self, observer):
-        """
-        Register files with observer
-        """
-        observer.schedule(self, 'xmodule/', recursive=True)
-
-    @debounce()
-    def on_any_event(self, event):
-        print('\tCHANGED:', event.src_path)
-        try:
-            process_xmodule_assets()
-        except Exception:  # pylint: disable=broad-except
-            traceback.print_exc()
-
-        # To refresh the hash values of static xmodule content
-        restart_django_servers()
-
-
 @task
 @no_help
 @cmdopts([
@@ -615,16 +573,13 @@ def process_npm_assets():
 
 
 @task
-@needs(
-    'pavelib.prereqs.install_python_prereqs',
-)
 @no_help
 def process_xmodule_assets():
     """
     Process XModule static assets.
     """
-    sh('xmodule_assets common/static/xmodule')
-    print("\t\tFinished processing xmodule assets.")
+    print("\t\tProcessing xmodule assets is no longer needed. This task is now a no-op.")
+    print("\t\tWhen paver is removed from edx-platform, this step will not replaced.")
 
 
 def restart_django_servers():
@@ -822,8 +777,6 @@ def watch_assets(options):
     observer = Observer(timeout=wait)
 
     SassWatcher().register(observer, sass_directories)
-    XModuleSassWatcher().register(observer, ['xmodule/'])
-    XModuleAssetsWatcher().register(observer)
 
     print("Starting asset watcher...")
     observer.start()
@@ -845,6 +798,7 @@ def watch_assets(options):
 @task
 @needs(
     'pavelib.prereqs.install_node_prereqs',
+    'pavelib.prereqs.install_python_prereqs',
 )
 @consume_args
 @timed
@@ -896,8 +850,6 @@ def update_assets(args):
     args = parser.parse_args(args)
     collect_log_args = {}
 
-    process_xmodule_assets()
-
     # Build Webpack
     call_task('pavelib.assets.webpack', options={'settings': args.settings})
 

diff --git a/pavelib/js_test.py b/pavelib/js_test.py
@@ -26,7 +26,6 @@
 @needs(
     'pavelib.prereqs.install_node_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
-    'pavelib.assets.process_xmodule_assets',
 )
 @cmdopts([
     ("suite=", "s", "Test suite to run"),

diff --git a/pavelib/paver_tests/test_servers.py b/pavelib/paver_tests/test_servers.py
@@ -214,7 +214,6 @@ def verify_server_task(self, task_name, options):
             expected_asset_settings = "test_static_optimized"
         expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS
         if not is_fast:
-            expected_messages.append("xmodule_assets common/static/xmodule")
             expected_messages.append("install npm_assets")
             expected_messages.extend(
                 [c.format(settings=expected_asset_settings,
@@ -259,7 +258,6 @@ def verify_run_all_servers_task(self, options):
         expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS
         expected_messages = []
         if not is_fast:
-            expected_messages.append("xmodule_assets common/static/xmodule")
             expected_messages.append("install npm_assets")
             expected_messages.extend(
                 [c.format(settings=expected_asset_settings,

diff --git a/webpack.builtinblocks.config.js b/webpack.builtinblocks.config.js
@@ -0,0 +1,136 @@
+module.exports = {
+    "entry": {
+        "AboutBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/display.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/collapsible.js",
+            "./xmodule/js/src/html/imageModal.js",
+            "./xmodule/js/common_static/js/vendor/draggabilly.js"
+        ],
+        "AboutBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/edit.js"
+        ],
+        "AnnotatableBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/display.js",
+            "./xmodule/js/src/annotatable/display.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/collapsible.js"
+        ],
+        "AnnotatableBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/raw/edit/xml.js"
+        ],
+        "ConditionalBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/conditional/display.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/collapsible.js"
+        ],
+        "ConditionalBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/sequence/edit.js"
+        ],
+        "CourseInfoBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/display.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/collapsible.js",
+            "./xmodule/js/src/html/imageModal.js",
+            "./xmodule/js/common_static/js/vendor/draggabilly.js"
+        ],
+        "CourseInfoBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/edit.js"
+        ],
+        "CustomTagBlockDisplay": "./xmodule/js/src/xmodule.js",
+        "CustomTagBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/raw/edit/xml.js"
+        ],
+        "HtmlBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/display.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/collapsible.js",
+            "./xmodule/js/src/html/imageModal.js",
+            "./xmodule/js/common_static/js/vendor/draggabilly.js"
+        ],
+        "HtmlBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/edit.js"
+        ],
+        "LTIBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/lti/lti.js"
+        ],
+        "LTIBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/raw/edit/metadata-only.js"
+        ],
+        "LibraryContentBlockDisplay": "./xmodule/js/src/xmodule.js",
+        "LibraryContentBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/vertical/edit.js"
+        ],
+        "PollBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/poll/poll.js",
+            "./xmodule/js/src/poll/poll_main.js"
+        ],
+        "PollBlockEditor": "./xmodule/js/src/xmodule.js",
+        "ProblemBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/capa/display.js",
+            "./xmodule/js/src/collapsible.js",
+            "./xmodule/js/src/capa/imageinput.js",
+            "./xmodule/js/src/capa/schematic.js"
+        ],
+        "ProblemBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/problem/edit.js"
+        ],
+        "SequenceBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/sequence/display.js"
+        ],
+        "SequenceBlockEditor": "./xmodule/js/src/xmodule.js",
+        "SplitTestBlockDisplay": "./xmodule/js/src/xmodule.js",
+        "SplitTestBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/sequence/edit.js"
+        ],
+        "StaticTabBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/display.js",
+            "./xmodule/js/src/javascript_loader.js",
+            "./xmodule/js/src/collapsible.js",
+            "./xmodule/js/src/html/imageModal.js",
+            "./xmodule/js/common_static/js/vendor/draggabilly.js"
+        ],
+        "StaticTabBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/html/edit.js"
+        ],
+        "VideoBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/video/10_main.js"
+        ],
+        "VideoBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/tabs/tabs-aggregator.js"
+        ],
+        "WordCloudBlockDisplay": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/assets/word_cloud/src/js/word_cloud.js"
+        ],
+        "WordCloudBlockEditor": [
+            "./xmodule/js/src/xmodule.js",
+            "./xmodule/js/src/raw/edit/metadata-only.js"
+        ]
+    }
+};
diff --git a/webpack.common.config.js b/webpack.common.config.js
@@ -9,7 +9,7 @@ var StringReplace = require('string-replace-webpack-plugin');
 var Merge = require('webpack-merge');
 
 var files = require('./webpack-config/file-lists.js');
-var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js');
+var builtinBlocksJS = require('./webpack.builtinblocks.config.js');
 
 var filesWithRequireJSBlocks = [
     path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'),
@@ -553,4 +553,4 @@ module.exports = Merge.smart({
         }
 
     }
-}, {web: xmoduleJS}, workerConfig());
+}, {web: builtinBlocksJS}, workerConfig());
diff --git a/xmodule/annotatable_block.py b/xmodule/annotatable_block.py
@@ -4,7 +4,6 @@
 import textwrap
 
 from lxml import etree
-from pkg_resources import resource_filename
 from web_fragments.fragment import Fragment
 from xblock.core import XBlock
 from xblock.fields import Scope, String
@@ -15,7 +14,6 @@
 from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment
 from xmodule.xml_block import XmlMixin
 from xmodule.x_module import (
-    HTMLSnippet,
     ResourceTemplates,
     shim_xmodule_js,
     XModuleMixin,
@@ -35,7 +33,6 @@ class AnnotatableBlock(
     XmlMixin,
     EditingMixin,
     XModuleToXBlockMixin,
-    HTMLSnippet,
     ResourceTemplates,
     XModuleMixin,
 ):
@@ -73,22 +70,6 @@ class AnnotatableBlock(
 
     uses_xmodule_styles_setup = True
 
-    preview_view_js = {
-        'js': [
-            resource_filename(__name__, 'js/src/html/display.js'),
-            resource_filename(__name__, 'js/src/annotatable/display.js'),
-            resource_filename(__name__, 'js/src/javascript_loader.js'),
-            resource_filename(__name__, 'js/src/collapsible.js'),
-        ],
-        'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'),
-    }
-
-    studio_view_js = {
-        'js': [
-            resource_filename(__name__, 'js/src/raw/edit/xml.js'),
-        ],
-        'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'),
-    }
     studio_js_module_name = "XMLEditingDescriptor"
     mako_template = "widgets/raw-edit.html"
 

diff --git a/xmodule/assets/README.rst b/xmodule/assets/README.rst
@@ -41,8 +41,8 @@ the corresponding folders in any enabled themes, as part of the edx-platform bui
 It is collected into the static root, and then linked to from XBlock fragments by the
 ``add_sass_to_fragment`` function in `builtin_assets.py`_.
 
-.. _AnnotatableBlockDisplay: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss
-.. _AnnotatableBlockEditor: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss
+.. _AnnotatableBlockDisplay.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss
+.. _AnnotatableBlockEditor.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss
 .. _annotatable/_display.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/annotatable/_display.scss
 .. _simplify things: https://github.com/openedx/edx-platform/issues/32621
 
@@ -59,7 +59,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and
 
 * For many older blocks, their JS is:
 
-  * bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``,
+  * bundled using a `webpack.builtinblocks.config.js`_,
   * which is included into `webpack.common.config.js`_,
   * allowing it to be included into XBlock fragments using ``add_webpack_js_to_fragment`` from `builtin_assets.py`_.
 
@@ -74,11 +74,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and
   * `VerticalBlock`_
   * `LibrarySourcedBlock`_
 
-As part of an `active build refactoring`_:
-
-* 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 delete the ``xmodule_assets`` script.
+As part of an `active build refactoring`_, we will soon consolidate all edx-platform XBlock JS here in `xmodule/assets`_.
 
 .. _xmodule/assets: https://github.com/openedx/edx-platform/tree/master/xmodule/assets
 .. _xmodule/js: https://github.com/openedx/edx-platform/tree/master/xmodule/js
@@ -91,4 +87,5 @@ As part of an `active build refactoring`_:
 .. _builtin_assets.py: https://github.com/openedx/edx-platform/tree/master/xmodule/util/builtin_assets.py
 .. _static_content.py: https://github.com/openedx/edx-platform/blob/master/xmodule/static_content.py
 .. _library_source_block/style.css: https://github.com/openedx/edx-platform/blob/master/xmodule/assets/library_source_block/style.css
+.. _webpack.builtinblocks.config.js: https://github.com/openedx/edx-platform/blob/master/webpack.builtinblocks.config.js
 .. _webpack.common.config.js: https://github.com/openedx/edx-platform/blob/master/webpack.common.config.js