Skip to content

Commit

Permalink
Revert "build: commit builtinblocks Webpack config and stub out `xmod…
Browse files Browse the repository at this point in the history
…ule_assets` (#32685)"

This reverts commit 3557799.
  • Loading branch information
kdmccormick authored Jul 27, 2023
1 parent 3557799 commit ac93829
Show file tree
Hide file tree
Showing 24 changed files with 481 additions and 164 deletions.
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/tests/test_discussion_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def test_has_permission(self):
"""
permission_canary = object()
with mock.patch(
'xmodule.discussion_block.has_permission',
'lms.djangoapps.discussion.django_comment_client.permissions.has_permission',
return_value=permission_canary,
) as has_perm:
actual_permission = self.block.has_permission("test_permission")
Expand Down
54 changes: 51 additions & 3 deletions pavelib/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ class SassWatcher(PatternMatchingEventHandler):
"""
ignore_directories = True
patterns = ['*.scss']
ignore_patterns = ['common/static/xmodule/*']

def register(self, observer, directories):
"""
Expand Down Expand Up @@ -351,6 +352,47 @@ 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([
Expand Down Expand Up @@ -573,13 +615,16 @@ def process_npm_assets():


@task
@needs(
'pavelib.prereqs.install_python_prereqs',
)
@no_help
def process_xmodule_assets():
"""
Process XModule static 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.")
sh('xmodule_assets common/static/xmodule')
print("\t\tFinished processing xmodule assets.")


def restart_django_servers():
Expand Down Expand Up @@ -777,6 +822,8 @@ 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()
Expand All @@ -798,7 +845,6 @@ def watch_assets(options):
@task
@needs(
'pavelib.prereqs.install_node_prereqs',
'pavelib.prereqs.install_python_prereqs',
)
@consume_args
@timed
Expand Down Expand Up @@ -850,6 +896,8 @@ 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})

Expand Down
1 change: 1 addition & 0 deletions pavelib/js_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
@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"),
Expand Down
2 changes: 2 additions & 0 deletions pavelib/paver_tests/test_servers.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ 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,
Expand Down Expand Up @@ -258,6 +259,7 @@ 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,
Expand Down
136 changes: 0 additions & 136 deletions webpack.builtinblocks.config.js

This file was deleted.

4 changes: 2 additions & 2 deletions webpack.common.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 builtinBlocksJS = require('./webpack.builtinblocks.config.js');
var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js');

var filesWithRequireJSBlocks = [
path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'),
Expand Down Expand Up @@ -553,4 +553,4 @@ module.exports = Merge.smart({
}

}
}, {web: builtinBlocksJS}, workerConfig());
}, {web: xmoduleJS}, workerConfig());
19 changes: 19 additions & 0 deletions xmodule/annotatable_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
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
Expand All @@ -14,6 +15,7 @@
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,
Expand All @@ -33,6 +35,7 @@ class AnnotatableBlock(
XmlMixin,
EditingMixin,
XModuleToXBlockMixin,
HTMLSnippet,
ResourceTemplates,
XModuleMixin,
):
Expand Down Expand Up @@ -70,6 +73,22 @@ 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"

Expand Down
13 changes: 8 additions & 5 deletions xmodule/assets/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.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
.. _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
.. _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

Expand All @@ -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 `webpack.builtinblocks.config.js`_,
* bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.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`_.

Expand All @@ -74,7 +74,11 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and
* `VerticalBlock`_
* `LibrarySourcedBlock`_

As part of an `active build refactoring`_, we will soon consolidate all edx-platform XBlock JS here in `xmodule/assets`_.
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.

.. _xmodule/assets: https://github.com/openedx/edx-platform/tree/master/xmodule/assets
.. _xmodule/js: https://github.com/openedx/edx-platform/tree/master/xmodule/js
Expand All @@ -87,5 +91,4 @@ As part of an `active build refactoring`_, we will soon consolidate all edx-plat
.. _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
Loading

0 comments on commit ac93829

Please sign in to comment.