From ed6ae31174756e0552a2b9ba2ea6c4a0a733ca88 Mon Sep 17 00:00:00 2001 From: Ali Salman Date: Fri, 1 Dec 2023 12:28:46 +0500 Subject: [PATCH 1/6] feat: add react app --- .../features/google_cdn_uploads/__init__.py | 1 + openedx/features/google_cdn_uploads/admin.py | 17 +++++++++++++ openedx/features/google_cdn_uploads/apps.py | 24 ++++++++++++++++++ .../google_cdn_uploads/migrations/__init__.py | 0 openedx/features/google_cdn_uploads/models.py | 17 +++++++++++++ .../google_cdn_uploads/settings/__init__.py | 0 .../google_cdn_uploads/settings/common.py | 17 +++++++++++++ .../google_cdn_uploads/js/GoogleCDNUpload.js | 11 ++++++++ .../google_cdn_uploads/js/UploadContext.js | 11 ++++++++ .../google_cdn_uploads/templates/uploads.html | 25 +++++++++++++++++++ openedx/features/google_cdn_uploads/urls.py | 21 ++++++++++++++++ openedx/features/google_cdn_uploads/views.py | 23 +++++++++++++++++ setup.py | 3 ++- webpack.common.config.js | 5 +++- 14 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 openedx/features/google_cdn_uploads/__init__.py create mode 100644 openedx/features/google_cdn_uploads/admin.py create mode 100644 openedx/features/google_cdn_uploads/apps.py create mode 100644 openedx/features/google_cdn_uploads/migrations/__init__.py create mode 100644 openedx/features/google_cdn_uploads/models.py create mode 100644 openedx/features/google_cdn_uploads/settings/__init__.py create mode 100644 openedx/features/google_cdn_uploads/settings/common.py create mode 100644 openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/GoogleCDNUpload.js create mode 100644 openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/UploadContext.js create mode 100644 openedx/features/google_cdn_uploads/templates/uploads.html create mode 100644 openedx/features/google_cdn_uploads/urls.py create mode 100644 openedx/features/google_cdn_uploads/views.py diff --git a/openedx/features/google_cdn_uploads/__init__.py b/openedx/features/google_cdn_uploads/__init__.py new file mode 100644 index 000000000000..e36a023259f1 --- /dev/null +++ b/openedx/features/google_cdn_uploads/__init__.py @@ -0,0 +1 @@ +default_app_config = "openedx.features.google_cdn_uploads.apps:GoogleCDNUploadsConfig" \ No newline at end of file diff --git a/openedx/features/google_cdn_uploads/admin.py b/openedx/features/google_cdn_uploads/admin.py new file mode 100644 index 000000000000..477d55442b00 --- /dev/null +++ b/openedx/features/google_cdn_uploads/admin.py @@ -0,0 +1,17 @@ +""" +Admin registration for Messenger. +""" +from django.contrib import admin + +from openedx.features.google_cdn_uploads.models import GoogleCDNUpload + + +class GoogleCDNUploadAdmin(admin.ModelAdmin): + """ + Admin config clearesult credit providers. + """ + list_display = ("file_name", "course_id",) + search_fields = ("file_name", "course_id",) + + +admin.site.register(GoogleCDNUpload, GoogleCDNUploadAdmin) diff --git a/openedx/features/google_cdn_uploads/apps.py b/openedx/features/google_cdn_uploads/apps.py new file mode 100644 index 000000000000..635097343420 --- /dev/null +++ b/openedx/features/google_cdn_uploads/apps.py @@ -0,0 +1,24 @@ +""" +Meta Translations App Config +""" +from django.apps import AppConfig +from edx_django_utils.plugins import PluginURLs, PluginSettings +from openedx.core.djangoapps.plugins.constants import ProjectType, SettingsType + +class GoogleCDNUploadsConfig(AppConfig): + name = 'openedx.features.google_cdn_uploads' + plugin_app = { + PluginURLs.CONFIG: { + ProjectType.CMS: { + PluginURLs.NAMESPACE: 'google_cdn_uploads', + PluginURLs.REGEX: '^google_cdn_uploads/', + PluginURLs.RELATIVE_PATH: 'urls', + }, + }, + PluginSettings.CONFIG: { + ProjectType.CMS: { + SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: 'settings.common'}, + }, + } + } + diff --git a/openedx/features/google_cdn_uploads/migrations/__init__.py b/openedx/features/google_cdn_uploads/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/google_cdn_uploads/models.py b/openedx/features/google_cdn_uploads/models.py new file mode 100644 index 000000000000..61592169bc8a --- /dev/null +++ b/openedx/features/google_cdn_uploads/models.py @@ -0,0 +1,17 @@ +""" +Meta Translations Models +""" +from django.db import models +from opaque_keys.edx.django.models import CourseKeyField + + +class GoogleCDNUpload(models.Model): + """ + Store course id and file name + """ + course_id = CourseKeyField(max_length=255, db_index=True) + file_name = models.CharField(max_length=255) + + class Meta: + app_label = 'google_cdn_uploads' + verbose_name = "Google CDN Uploads" diff --git a/openedx/features/google_cdn_uploads/settings/__init__.py b/openedx/features/google_cdn_uploads/settings/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/google_cdn_uploads/settings/common.py b/openedx/features/google_cdn_uploads/settings/common.py new file mode 100644 index 000000000000..99debf43a8f0 --- /dev/null +++ b/openedx/features/google_cdn_uploads/settings/common.py @@ -0,0 +1,17 @@ +""" +Common settings for Messenger +""" + + +def plugin_settings(settings): + """ + Common settings for uploading mp4 + """ + settings.MAKO_TEMPLATE_DIRS_BASE.append( + settings.OPENEDX_ROOT / 'features' / 'google_cdn_uploads' / 'templates', + ) + + settings.STATICFILES_DIRS.append ( + settings.OPENEDX_ROOT / 'features' / 'google_cdn_uploads' / 'static', + ) + print('CDN App - Setting Updated') diff --git a/openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/GoogleCDNUpload.js b/openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/GoogleCDNUpload.js new file mode 100644 index 000000000000..4a84f8582fa5 --- /dev/null +++ b/openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/GoogleCDNUpload.js @@ -0,0 +1,11 @@ +import React from 'react'; +import ReactDOM from 'react-dom'; +import UploadContext from "./UploadContext"; + +export default class GoogleCDNUpload { + constructor() { + ReactDOM.render(, document.getElementById("root")); + } +} + +export { GoogleCDNUpload } diff --git a/openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/UploadContext.js b/openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/UploadContext.js new file mode 100644 index 000000000000..f28d7e311dfa --- /dev/null +++ b/openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/UploadContext.js @@ -0,0 +1,11 @@ +import React from 'react'; + +function UploadContext({ context }) { + return ( +
+

kjasnxkajsnkxajns

+
+ ) +} + +export default UploadContext; \ No newline at end of file diff --git a/openedx/features/google_cdn_uploads/templates/uploads.html b/openedx/features/google_cdn_uploads/templates/uploads.html new file mode 100644 index 000000000000..fcf153cbadba --- /dev/null +++ b/openedx/features/google_cdn_uploads/templates/uploads.html @@ -0,0 +1,25 @@ +<%inherit file="base.html" /> +<%namespace name='static' file='static_content.html'/> +<%! + from django.urls import reverse + from django.utils.translation import ugettext as _ + from openedx.core.djangolib.js_utils import js_escaped_string, dump_js_escaped_json +%> + +<%def name="online_help_token()"><% return "home" %> +<%block name="title">${_("{studio_name} Home").format(studio_name=settings.STUDIO_SHORT_NAME)} +<%block name="bodyclass">is-signedin index view-dashboard + + + +<%block name="content"> +
+ hello +
+ + <%static:webpack entry="GoogleCDNUpload"> + var context = {} + + new GoogleCDNUpload(context); + + diff --git a/openedx/features/google_cdn_uploads/urls.py b/openedx/features/google_cdn_uploads/urls.py new file mode 100644 index 000000000000..68c1e80aca7d --- /dev/null +++ b/openedx/features/google_cdn_uploads/urls.py @@ -0,0 +1,21 @@ +""" +Urls for Meta Translations +""" +from django.conf.urls import include, url + +from openedx.features.google_cdn_uploads.views import render_google_cdn_uploads_home + +app_name = 'google_cdn_uploads' + +urlpatterns = [ + # url( + # r'^course_blocks_mapping/$', + # course_blocks_mapping, + # name='course_blocks_mapping' + # ), + url( + r'^$', + render_google_cdn_uploads_home, + name='google_cdn_uploads_home' + ), +] diff --git a/openedx/features/google_cdn_uploads/views.py b/openedx/features/google_cdn_uploads/views.py new file mode 100644 index 000000000000..7cefd3e21007 --- /dev/null +++ b/openedx/features/google_cdn_uploads/views.py @@ -0,0 +1,23 @@ +""" +Views for uploading mp4 +""" +import json +from logging import getLogger +from django.contrib.auth.decorators import login_required +from django.views.decorators.http import require_http_methods +from django.http import JsonResponse +from xmodule.modulestore.django import modulestore +from opaque_keys.edx.keys import CourseKey +from django.conf import settings +from common.djangoapps.edxmako.shortcuts import render_to_response + +from lms.djangoapps.courseware.courses import get_course_by_id +log = getLogger(__name__) + + +@login_required +def render_google_cdn_uploads_home(request): + return render_to_response('uploads.html', { + 'uses_bootstrap': True, + 'login_user_username': request.user.username, + }) diff --git a/setup.py b/setup.py index 3a6670c04137..b14525bc2be4 100644 --- a/setup.py +++ b/setup.py @@ -46,7 +46,7 @@ setup( name="Open edX", - version='0.13', + version='0.14', install_requires=["setuptools"], requires=[], # NOTE: These are not the names we should be installing. This tree should @@ -173,6 +173,7 @@ "user_authn = openedx.core.djangoapps.user_authn.apps:UserAuthnConfig", "instructor = lms.djangoapps.instructor.apps:InstructorConfig", "course_apps = openedx.core.djangoapps.course_apps.apps:CourseAppsConfig", + "google_cdn_uploads = openedx.features.google_cdn_uploads.apps:GoogleCDNUploadsConfig" ], 'openedx.learning_context': [ 'lib = openedx.core.djangoapps.content_libraries.library_context:LibraryContextImpl', diff --git a/webpack.common.config.js b/webpack.common.config.js index 042ffa6a8a52..d1dcd4426c5c 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -122,7 +122,10 @@ module.exports = Merge.smart({ ReactRenderer: './common/static/js/src/ReactRenderer.jsx', XModuleShim: './xmodule/js/src/xmodule.js', VerticalStudentView: './xmodule/assets/vertical/public/js/vertical_student_view.js', - commons: 'babel-polyfill' + commons: 'babel-polyfill', + + // SDAIA + GoogleCDNUpload: './openedx/features/google_cdn_uploads/static/google_cdn_uploads/js/GoogleCDNUpload.js', }, output: { From 4e115519181bc874633f204e530b9f963264dcc2 Mon Sep 17 00:00:00 2001 From: Ali Salman Date: Fri, 1 Dec 2023 12:46:36 +0500 Subject: [PATCH 2/6] fix: add styles --- openedx/features/google_cdn_uploads/templates/uploads.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openedx/features/google_cdn_uploads/templates/uploads.html b/openedx/features/google_cdn_uploads/templates/uploads.html index fcf153cbadba..524df681c368 100644 --- a/openedx/features/google_cdn_uploads/templates/uploads.html +++ b/openedx/features/google_cdn_uploads/templates/uploads.html @@ -10,7 +10,9 @@ <%block name="title">${_("{studio_name} Home").format(studio_name=settings.STUDIO_SHORT_NAME)} <%block name="bodyclass">is-signedin index view-dashboard - +<%block name="header_extras"> + + <%block name="content">
From 69aa19bcfe1905c6f38f72c628917a2908afe435 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Thu, 27 Jul 2023 12:33:32 -0400 Subject: [PATCH 3/6] build: `npm run watch` (experimental) (#32866) Implements the `npm run watch` section of the assets ADR [1], plus some modifications since I decided to switch from pywatchman to watchdog (see ADR changes for justification). This will replace `paver watch_assets` (edx-platform) and `openedx-assets watch-themes` (Tutor). Specifically, this PR adds three experimental commands: * `npm run watch-sass` : Watch for Sass changes with watchdog. * `npm run watch-webpack` : Invoke Webpack-watch for JS changes. * `npm run watch` : Invoke both `watch-sass` and `watch-webpack` simultaneously. These commands are only intended to work in development mode. They have been tested both on bare-metal edx-platform and through `tutor dev run` on on Linux. Before removing the "experimental" label, we need to: * Test through Devstack on Linux. * Test through Devstack and `tutor dev run` on macOS. * Test on bare-metal macOS. Might not work, which is OK, but we should document that. * Document the commands in edx-platform's README. * Confirm that this not only works through `tutor dev run`, but also as a suitable replacement in the `watchthemes` service that Tutor runs automatically as part of `tutor dev start`. Tweak if necessary. References: 1. https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst Part of: https://github.com/openedx/edx-platform/issues/31612 --- .../0017-reimplement-asset-processing.rst | 4 +- package.json | 12 ++ requirements/edx/development.in | 1 + scripts/watch_sass.sh | 170 ++++++++++++++++++ xmodule/README.rst | 7 +- 5 files changed, 191 insertions(+), 3 deletions(-) create mode 100755 scripts/watch_sass.sh diff --git a/docs/decisions/0017-reimplement-asset-processing.rst b/docs/decisions/0017-reimplement-asset-processing.rst index 7f16dcc249ff..0a25fb798f13 100644 --- a/docs/decisions/0017-reimplement-asset-processing.rst +++ b/docs/decisions/0017-reimplement-asset-processing.rst @@ -221,7 +221,9 @@ The three top-level edx-platform asset processing actions are *build*, *collect* (where ```` is optionally one of the build stages described above. If provided, only that stage's assets will be watched.) - Bash wrappers around invocation(s) of `watchman `_, a popular file-watching library maintained by Meta. Watchman is already installed into edx-platform (and other services) via the pywatchman pip wrapper package. + Bash wrappers around invocations of the `watchdog library `_ for themable/themed assets, and `webpack --watch `_ for Webpack-managed assets. Both of these tools are available via dependencies that are already installed into edx-platform. + + We considered using `watchman `_, a popular file-watching library maintained by Meta, but found that the Python release of the library is poorly maintained (latest release 2017) and the documentation is difficult to follow. `Django uses pywatchman but is planning to migrate off of it `_ and onto `watchfiles `_. We considered watchfiles, but decided against adding another developer dependency to edx-platform. Future developers could consider migrating to watchfiles if it seemed worthwile. Note: This adds a Python dependency to build-assets.sh. However, we could be clear that watchman is an *optional* dependency of build-assets.sh which enables the optional ``--watch`` feature. This would keep the *build* action Python-free. Alternatively, watchman is also available Python-free via apt and homebrew. diff --git a/package.json b/package.json index f04ea4be6700..cf483823dec2 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,18 @@ "name": "edx", "version": "0.1.0", "repository": "https://github.com/openedx/edx-platform", + "scripts": { + "postinstall": "scripts/copy-node-modules.sh", + "build": "echo 'WARNING: `npm run build` in edx-platform is experimental. Use at your own risk.' && npm run webpack && npm run compile-sass", + "build-dev": "echo 'WARNING: `npm run build-dev` in edx-platform is experimental. Use at your own risk.' && npm run webpack-dev && npm run compile-sass-dev", + "webpack": "NODE_ENV=${NODE_ENV:-production} \"$(npm bin)/webpack\" --config=${WEBPACK_CONFIG_PATH:-webpack.prod.config.js}", + "webpack-dev": "NODE_ENV=development \"$(npm bin)/webpack\" --config=webpack.dev.config.js", + "compile-sass": "scripts/compile_sass.py --env=${NODE_ENV:-production}", + "compile-sass-dev": "scripts/compile_sass.py --env=development", + "watch": "echo 'WARNING: `npm run watch` in edx-platform is experimental. Use at your own risk.' && { npm run watch-webpack& npm run watch-sass& } && sleep infinity", + "watch-webpack": "npm run webpack-dev -- --watch", + "watch-sass": "scripts/watch_sass.sh" + }, "dependencies": { "@babel/core": "7.19.0", "@babel/plugin-proposal-object-rest-spread": "^7.18.9", diff --git a/requirements/edx/development.in b/requirements/edx/development.in index 1a857f291711..be5169de7124 100644 --- a/requirements/edx/development.in +++ b/requirements/edx/development.in @@ -20,3 +20,4 @@ mypy # static type checking pywatchman # More efficient checking for runserver reload trigger events sphinxcontrib-openapi[markdown] # OpenAPI (fka Swagger) spec renderer for Sphinx vulture # Detects possible dead/unused code, used in scripts/find-dead-code.sh +watchdog # Used by `npm run watch` to auto-recompile when assets are changed diff --git a/scripts/watch_sass.sh b/scripts/watch_sass.sh new file mode 100755 index 000000000000..dee0d88f6215 --- /dev/null +++ b/scripts/watch_sass.sh @@ -0,0 +1,170 @@ +#!/usr/bin/env bash + +# Wait for changes to Sass, and recompile. +# Invoke from repo root as `npm run watch-sass`. +# This script tries to recompile the minimal set of Sass for any given change. + +# By default, only watches default Sass. +# To watch themes too, provide colon-separated paths in the EDX_PLATFORM_THEME_DIRS environment variable. +# Each path will be treated as a "theme dir", which means that every immediate child directory is watchable as a theme. +# For example: +# +# EDX_PLATFORM_THEME_DIRS=/openedx/themes:./themes npm run watch-sass +# +# would watch default Sass as well as /openedx/themes/indigo, /openedx/themes/mytheme, ./themes/red-theme, etc. + +set -euo pipefail + +COL_SECTION="\e[1;36m" # Section header color (bold cyan) +COL_LOG="\e[36m" # Log color (cyan) +COL_WARNING="\e[1;33m" # Warning (bold yellow) +COL_ERROR="\e[1;31m" # Error (bold red) +COL_CMD="\e[35m" # Command echoing (magenta) +COL_OFF="\e[0m" # Normal color + +section() { + # Print a header in bold cyan to indicate sections of output. + echo -e "${COL_SECTION}$*${COL_OFF}" +} +log() { + # Info line. Indented by one space so that it appears as nested under section headers. + echo -e " ${COL_LOG}$*${COL_OFF}" +} +warning() { + # Bright yellow warning message. + echo -e "${COL_WARNING}WARNING: $*${COL_OFF}" +} +error() { + # Bright red error message. + echo -e "${COL_ERROR}ERROR: $*${COL_OFF}" +} +echo_quoted_cmd() { + # Echo args, each single-quoted, so that the user could copy-paste and run them as a command. + # Indented by two spaces so it appears as nested under log lines. + echo -e " ${COL_CMD}$(printf "'%s' " "$@")${COL_OFF}" +} + +start_sass_watch() { + # Start a watch for .scss files in a particular dir. Run in the background. + # start_sass_watch NAME_FOR_LOGGING WATCH_ROOT_PATH HANDLER_COMMAND + local name="$1" + local path="$2" + local handler="$3" + log "Starting watcher for $name:" + # Note: --drop means that we should ignore any change events that happen during recompilation. + # This is good because it means that if you edit 3 files, we won't fire off three simultaneous compiles. + # It's not perfect, though, because if you change 3 files, only the first one will trigger a recompile, + # so depending on the timing, your latest changes may or may not be picked up. We accept this as a reasonable + # tradeoff. Some watcher libraries are smarter than watchdog, in that they wait until the filesystem "settles" + # before firing off a the recompilation. This sounds nice but did not seem worth the effort for legacy assets. + local watch_cmd=(watchmedo shell-command -v --patterns '*.scss' --recursive --drop --command "$handler" "$path") + echo_quoted_cmd "${watch_cmd[@]}" + "${watch_cmd[@]}" & +} + +clean_up() { + # Kill all background processes we started. + # Since they're all 'watchmedo' instances, we can just use killall. + log "Stopping all watchers:" + local stop_cmd=(killall watchmedo) + echo_quoted_cmd "${stop_cmd[@]}" + "${stop_cmd[@]}" || true + log "Watchers stopped." +} + +warning "'npm run watch-sass' in edx-platform is experimental. Use at your own risk." + +if [[ ! -d common/static/sass ]] ; then + error 'This command must be run from the root of edx-platform!' + exit 1 +fi +if ! type watchmedo 1>/dev/null 2>&1 ; then + error "command not found: watchmedo" + log "The \`watchdog\` Python package is probably not installed. You can install it with:" + log " pip install -r requirements/edx/development.txt" + exit 1 +fi + +trap clean_up EXIT + +# Start by compiling all watched Sass right away, mirroring the behavior of webpack --watch. +section "COMPILING SASS:" +npm run compile-sass +echo +echo + +section "STARTING DEFAULT SASS WATCHERS:" + +# Changes to LMS Sass require a full recompilation, since LMS Sass can be used in CMS and in themes. +start_sass_watch "default LMS Sass" \ + lms/static/sass \ + 'npm run compile-sass-dev' + +# Changes to default cert Sass only require recompilation of default cert Sass, since cert Sass +# cannot be included into LMS, CMS, or themes. +start_sass_watch "default certificate Sass" \ + lms/static/certificates/sass \ + 'npm run compile-sass-dev -- --skip-cms --skip-themes' + +# Changes to CMS Sass require recompilation of default & themed CMS Sass, but not LMS Sass. +start_sass_watch "default CMS Sass" \ + cms/static/sass \ + 'npm run compile-sass-dev -- --skip-lms' + +# Sass changes in common, node_modules, and xmodule all require full recompilations. +start_sass_watch "default common Sass" \ + common/static \ + 'npm run compile-sass-dev' +start_sass_watch "node_modules Sass" \ + node_modules \ + 'npm run compile-sass-dev' +start_sass_watch "builtin XBlock Sass" \ + xmodule/assets \ + 'npm run compile-sass-dev' + +export IFS=";" +for theme_dir in ${EDX_PLATFORM_THEME_DIRS:-} ; do + for theme_path in "$theme_dir"/* ; do + + theme_name="${theme_path#"$theme_dir/"}" + lms_sass="$theme_path/lms/static/sass" + cert_sass="$theme_path/lms/static/certificates/sass" + cms_sass="$theme_path/cms/static/sass" + + if [[ -d "$lms_sass" ]] || [[ -d "$cert_sass" ]] || [[ -d "$cms_sass" ]] ; then + section "STARTING WATCHERS FOR THEME '$theme_name':" + fi + + # Changes to a theme's LMS Sass require that the full theme is recompiled, since LMS + # Sass is used in certs and CMS. + if [[ -d "$lms_sass" ]] ; then + start_sass_watch "$theme_name LMS Sass" \ + "$lms_sass" \ + "npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default" + fi + + # Changes to a theme's certs Sass only requires that its certs Sass be recompiled. + if [[ -d "$cert_sass" ]] ; then + start_sass_watch "$theme_name certificate Sass" \ + "$cert_sass" \ + "npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default --skip-cms" + fi + + # Changes to a theme's CMS Sass only requires that its CMS Sass be recompiled. + if [[ -d "$cms_sass" ]] ; then + start_sass_watch "$theme_name CMS Sass" \ + "$cms_sass" \ + "npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default --skip-lms" + fi + + done +done + +sleep infinity & +echo +echo "Watching Open edX LMS & CMS Sass for changes." +echo "Use Ctrl+c to exit." +echo +echo +wait $! + diff --git a/xmodule/README.rst b/xmodule/README.rst index e4f9876cc36e..984de176ba81 100644 --- a/xmodule/README.rst +++ b/xmodule/README.rst @@ -6,8 +6,11 @@ The ``xmodule`` folder contains a variety of old-yet-important functionality cor * the ModuleStore, edx-platform's "Version 1" learning content storage backend; * an XBlock Runtime implementation for ModuleStore-backed content; * the "partitions" framework for differentiated XBlock content; -* implementations for the "stuctural" XBlocks: ``course``, ``chapter``, and ``sequential``; and -* the implementations of several different content-level XBlocks, such as ``problem`` and ``html``. +* implementations for the "stuctural" XBlocks: ``course``, ``chapter``, and ``sequential``; +* the implementations of several different built-in content-level XBlocks, such as ``problem`` and ``html``; and +* `assets for those built-in XBlocks`_. + +.. _assets for those built-in XBlocks: https://github.com/openedx/edx-platform/tree/master/xmodule/assets#readme Historical Context ****************** From dd38856e1bf742c5e539b479bb14bfc20b7eee96 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Mon, 17 Jul 2023 13:26:58 -0400 Subject: [PATCH 4/6] build: copy from node_modules using NPM postinstall hook, not Paver (RE-MERGE) Re-merge of 4b64d8342d69bb92afe56a49c61bc7e5663aceff The commit after this one contains the fix for the issue that led to the revert. --- .../0017-reimplement-asset-processing.rst | 27 +++--- package.json | 11 +-- pavelib/assets.py | 90 +------------------ pavelib/utils/test/suites/js_suite.py | 2 - scripts/copy-node-modules.sh | 90 +++++++++++++++++++ 5 files changed, 103 insertions(+), 117 deletions(-) create mode 100755 scripts/copy-node-modules.sh diff --git a/docs/decisions/0017-reimplement-asset-processing.rst b/docs/decisions/0017-reimplement-asset-processing.rst index 0a25fb798f13..b8176e901faa 100644 --- a/docs/decisions/0017-reimplement-asset-processing.rst +++ b/docs/decisions/0017-reimplement-asset-processing.rst @@ -131,17 +131,17 @@ The three top-level edx-platform asset processing actions are *build*, *collect* - ``scripts/build-assets.sh`` A Bash script that contains all build stages, with subcommands available for running each stage separately. Its command-line interface inspired by Tutor's ``openedx-assets`` script. The script will be runnable on any POSIX system, including macOS and Ubuntu and it will linted for common shell scripting mistakes using `shellcheck `_. - + * - + **Build stage 1: Copy npm-installed assets** from node_modules to other folders in edx-platform. They are used by certain especially-old legacy LMS & CMS frontends that are not set up to work with npm directly. - ``paver update_assets --skip-collect`` Implemented in Python within update_assets. There is no standalone command for it. - - ``scripts/build-assets.sh npm`` + - ``npm install`` + + An NPM post-install hook will automatically call scripts/copy-node-modules.sh, a pure Bash reimplementation of the node_modules asset copying, whenever ``npm install`` is invoked. - Pure Bash reimplementation. See *Rejected Alternatives* for a note about this. - * - + **Build stage 2: Copy XModule fragments** from the xmodule source tree over to input directories for Webpack and SCSS compilation. This is required for a hard-coded list of old XModule-style XBlocks. This is not required for new pure XBlocks, which include (or pip-install) their assets into edx-platform as ready-to-serve JS/CSS/etc fragments. - ``paver process_xmodule_assets``, or @@ -156,7 +156,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect* + `Reimplement this step in Bash `_. + `Remove the need for this step entirely `_. - + * - + **Build stage 3: Run Webpack** in order to to shim, minify, otherwise process, and bundle JS modules. This requires a call to the npm-installed ``webpack`` binary. - ``paver webpack`` @@ -168,7 +168,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect* Bash wrapper around a call to webpack. The script will accept parameters rather than looking up Django settings itself. The print_setting command will still be available for distributions to use to extract ``STATIC_ROOT`` from Django settings, but it will only need to be run once. As described in **Build Configuration** below, unnecessary Django settings will be removed. Some distributions may not even need to look up ``STATIC_ROOT``; Tutor, for example, will probably render ``STATIC_ROOT`` directly into the environment variable ``OPENEDX_BUILD_ASSETS_OPTS`` variable, described in the **Build Configuration**. - + * - + **Build stage 4: Compile default SCSS** into CSS for legacy LMS/CMS frontends. - ``paver compile_sass`` @@ -180,7 +180,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect* - ``scripts/build-assets.sh css`` Bash reimplementation, calling ``node-sass`` and ``rtlcss``. - + The initial implementation of build-assets.sh may use ``sassc``, a CLI provided by libsass, instead of node-sass. Then, ``sassc`` can be replaced by ``node-sass`` as part of a subsequent `edx-platform frontend framework upgrade effort `_. * - + **Build stage 5: Compile themes' SCSS** into CSS for legacy LMS/CMS frontends. The default SCSS is used as a base, and theme-provided SCSS files are used as overrides. Themes are searched for from some number of operator-specified theme directories. @@ -198,7 +198,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect* The management command will remain available, but it will need to be updated to point at the Bash script, which will replace the paver task (see build stage 4 for details). The overall asset *build* action will use the Bash script; this means that list of theme directories will need to be provided as arguments, but it ensures that the build can remain Python-free. - + * - **Collect** the built static assets from edx-platform to another location (the ``STATIC_ROOT``) so that they can be efficiently served *without* Django's webserver. This step, by nature, requires Python and Django in order to find and organize the assets, which may come from edx-platform itself or from its many installed Python and NPM packages. This is only needed for **production** environments, where it is usually desirable to serve assets with something efficient like NGINX. - ``paver update_assets`` @@ -210,7 +210,7 @@ The three top-level edx-platform asset processing actions are *build*, *collect* - ``./manage.py lms collectstatic --noinput && ./manage.py cms collectstatic --noinput`` The standard Django interface will be used without a wrapper. The ignore patterns will be added to edx-platform's `staticfiles app configuration `_ so that they do not need to be supplied as part of the command. - + * - **Watch** static assets for changes in the background. When a change occurs, rebuild them automatically, so that the Django webserver picks up the changes. This is only necessary in **development** environments. A few different sets of assets may be watched: XModule fragments, Webpack assets, default SCSS, and theme SCSS. - ``paver watch_assets`` @@ -302,7 +302,7 @@ Either way, the migration path is straightforward: * - ``openedx-assets build`` - ``scripts/build-assets.sh`` * - ``openedx-assets npm`` - - ``scripts/build-assets.sh npm`` + - ``scripts/copy-node-modules.sh # (automatically invoked by 'npm install'!) * - ``openedx-assets xmodule`` - ``scripts/build-assets.sh xmodule`` * - ``openedx-assets common`` @@ -330,13 +330,6 @@ OpenCraft has also performed a discovery on a `modernized system for static asse Rejected Alternatives ********************* -Copy node_modules via npm post-install -====================================== - -It was noted that `npm supports lifecycle scripts `_ in package.json, including ``postinstall``. We could use a post-install script to copy assets out of node_modules; this would occurr automatically after ``npm install``. Arguably, this would be more idiomatic than this ADR's proposal of ``scripts/build-assets.sh npm``. - -For now, we decided against this. While it seems like a good potential future improvement, we are currently unsure how it would interact with `moving node_modules out of edx-platform in Tutor `_, which is a motivation behind this ADR. For example, if node_modules could be located anywhere on the image, then we are not sure how the post-install script could know its target directory without us hard-coding Tutor's directory structure into the script. - Live with the problem ====================== diff --git a/package.json b/package.json index cf483823dec2..8a21fece5eda 100644 --- a/package.json +++ b/package.json @@ -3,16 +3,7 @@ "version": "0.1.0", "repository": "https://github.com/openedx/edx-platform", "scripts": { - "postinstall": "scripts/copy-node-modules.sh", - "build": "echo 'WARNING: `npm run build` in edx-platform is experimental. Use at your own risk.' && npm run webpack && npm run compile-sass", - "build-dev": "echo 'WARNING: `npm run build-dev` in edx-platform is experimental. Use at your own risk.' && npm run webpack-dev && npm run compile-sass-dev", - "webpack": "NODE_ENV=${NODE_ENV:-production} \"$(npm bin)/webpack\" --config=${WEBPACK_CONFIG_PATH:-webpack.prod.config.js}", - "webpack-dev": "NODE_ENV=development \"$(npm bin)/webpack\" --config=webpack.dev.config.js", - "compile-sass": "scripts/compile_sass.py --env=${NODE_ENV:-production}", - "compile-sass-dev": "scripts/compile_sass.py --env=development", - "watch": "echo 'WARNING: `npm run watch` in edx-platform is experimental. Use at your own risk.' && { npm run watch-webpack& npm run watch-sass& } && sleep infinity", - "watch-webpack": "npm run webpack-dev -- --watch", - "watch-sass": "scripts/watch_sass.sh" + "postinstall": "scripts/copy-node-modules.sh" }, "dependencies": { "@babel/core": "7.19.0", diff --git a/pavelib/assets.py b/pavelib/assets.py index ad933de19596..4289f2525247 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -46,39 +46,6 @@ path('node_modules'), ] -# A list of NPM installed libraries that should be copied into the common -# static directory. -# If string ends with '/' then all file in the directory will be copied. -NPM_INSTALLED_LIBRARIES = [ - 'backbone.paginator/lib/backbone.paginator.js', - 'backbone/backbone.js', - 'bootstrap/dist/js/bootstrap.bundle.js', - 'hls.js/dist/hls.js', - 'jquery-migrate/dist/jquery-migrate.js', - 'jquery.scrollto/jquery.scrollTo.js', - 'jquery/dist/jquery.js', - 'moment-timezone/builds/moment-timezone-with-data.js', - 'moment/min/moment-with-locales.js', - 'picturefill/dist/picturefill.js', - 'requirejs/require.js', - 'underscore.string/dist/underscore.string.js', - 'underscore/underscore.js', - '@edx/studio-frontend/dist/', - 'which-country/index.js' -] - -# A list of NPM installed developer libraries that should be copied into the common -# static directory only in development mode. -NPM_INSTALLED_DEVELOPER_LIBRARIES = [ - 'sinon/pkg/sinon.js', - 'squirejs/src/Squire.js', -] - -# Directory to install static vendor files -NPM_JS_VENDOR_DIRECTORY = path('common/static/common/js/vendor') -NPM_CSS_VENDOR_DIRECTORY = path("common/static/common/css/vendor") -NPM_CSS_DIRECTORY = path("common/static/common/css") - # system specific lookup path additions, add sass dirs if one system depends on the sass files for other systems SASS_LOOKUP_DEPENDENCIES = { 'cms': [path('lms') / 'static' / 'sass' / 'partials', ], @@ -604,60 +571,8 @@ def process_npm_assets(): """ Process vendor libraries installed via NPM. """ - def copy_vendor_library(library, skip_if_missing=False): - """ - Copies a vendor library to the shared vendor directory. - """ - if library.startswith('node_modules/'): - library_path = library - else: - library_path = f'node_modules/{library}' - - if library.endswith('.css') or library.endswith('.css.map'): - vendor_dir = NPM_CSS_VENDOR_DIRECTORY - else: - vendor_dir = NPM_JS_VENDOR_DIRECTORY - if os.path.exists(library_path): - sh('/bin/cp -rf {library_path} {vendor_dir}'.format( - library_path=library_path, - vendor_dir=vendor_dir, - )) - elif not skip_if_missing: - raise Exception(f'Missing vendor file {library_path}') - - def copy_vendor_library_dir(library_dir, skip_if_missing=False): - """ - Copies all vendor libraries in directory to the shared vendor directory. - """ - library_dir_path = f'node_modules/{library_dir}' - print(f'Copying vendor library dir: {library_dir_path}') - if os.path.exists(library_dir_path): - for dirpath, _, filenames in os.walk(library_dir_path): - for filename in filenames: - copy_vendor_library(os.path.join(dirpath, filename), skip_if_missing=skip_if_missing) - - # Skip processing of the libraries if this is just a dry run - if tasks.environment.dry_run: - tasks.environment.info("install npm_assets") - return - - # Ensure that the vendor directory exists - NPM_JS_VENDOR_DIRECTORY.mkdir_p() - NPM_CSS_DIRECTORY.mkdir_p() - NPM_CSS_VENDOR_DIRECTORY.mkdir_p() - - # Copy each file to the vendor directory, overwriting any existing file. - print("Copying vendor files into static directory") - for library in NPM_INSTALLED_LIBRARIES: - if library.endswith('/'): - copy_vendor_library_dir(library) - else: - copy_vendor_library(library) - - # Copy over each developer library too if they have been installed - print("Copying developer vendor files into static directory") - for library in NPM_INSTALLED_DEVELOPER_LIBRARIES: - copy_vendor_library(library, skip_if_missing=True) + print("\t\tProcessing NPM assets is now done automatically in an npm post-install hook.") + print("\t\tThis function is now a no-op.") @task @@ -964,7 +879,6 @@ def update_assets(args): collect_log_args = {} process_xmodule_assets() - process_npm_assets() # Build Webpack call_task('pavelib.assets.webpack', options={'settings': args.settings}) diff --git a/pavelib/utils/test/suites/js_suite.py b/pavelib/utils/test/suites/js_suite.py index a6896e285854..65c5feaf843b 100644 --- a/pavelib/utils/test/suites/js_suite.py +++ b/pavelib/utils/test/suites/js_suite.py @@ -39,8 +39,6 @@ def __enter__(self): if self.mode == 'run' and not self.run_under_coverage: test_utils.clean_dir(self.report_dir) - assets.process_npm_assets() - @property def _default_subsuites(self): """ diff --git a/scripts/copy-node-modules.sh b/scripts/copy-node-modules.sh new file mode 100755 index 000000000000..db997da95785 --- /dev/null +++ b/scripts/copy-node-modules.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# Copy certain npm-installed assets from node_modules to other folders in +# edx-platform. These assets are used by certain especially-old legacy LMS & CMS +# frontends that are not set up to import from node_modules directly. +# Many of the destination folders are named "vendor", because they originally +# held vendored-in (directly-committed) libraries; once we moved most frontends +# to use NPM, we decided to keep library versions in-sync by copying to the +# former "vendor" directories. + +# Enable stricter error handling. +set -euo pipefail + +COL_LOG="\e[36m" # Log/step/section color (cyan) +COL_OFF="\e[0m" # Normal color + +# Keep these as variables in case we ever want to parameterize this script's +# input or output dirs, as proposed in: +# https://github.com/openedx/wg-developer-experience/issues/150 +# https://github.com/openedx/wg-developer-experience/issues/151 +node_modules="node_modules" +vendor_js="common/static/common/js/vendor" +vendor_css="common/static/common/css/vendor" + +# Stylized logging. +log ( ) { + echo -e "${COL_LOG}$* $COL_OFF" +} + +log "=====================================================================================" +log "Copying required assets from node_modules..." +log "-------------------------------------------------------------------------------" + +# Start echoing all commands back to user for ease of debugging. +set -x + +log "Ensuring vendor directories exist..." +mkdir -p "$vendor_js" +mkdir -p "$vendor_css" + +log "Copying studio-frontend JS & CSS from node_modules into vendor directores..." +while read -r -d $'\0' src_file ; do + if [[ "$src_file" = *.css ]] || [[ "$src_file" = *.css.map ]] ; then + cp --force "$src_file" "$vendor_css" + else + cp --force "$src_file" "$vendor_js" + fi +done < <(find "$node_modules/@edx/studio-frontend/dist" -type f -print0) + +log "Copying certain JS modules from node_modules into vendor directory..." +cp --force \ + "$node_modules/backbone.paginator/lib/backbone.paginator.js" \ + "$node_modules/backbone/backbone.js" \ + "$node_modules/bootstrap/dist/js/bootstrap.bundle.js" \ + "$node_modules/hls.js/dist/hls.js" \ + "$node_modules/jquery-migrate/dist/jquery-migrate.js" \ + "$node_modules/jquery.scrollto/jquery.scrollTo.js" \ + "$node_modules/jquery/dist/jquery.js" \ + "$node_modules/moment-timezone/builds/moment-timezone-with-data.js" \ + "$node_modules/moment/min/moment-with-locales.js" \ + "$node_modules/picturefill/dist/picturefill.js" \ + "$node_modules/requirejs/require.js" \ + "$node_modules/underscore.string/dist/underscore.string.js" \ + "$node_modules/underscore/underscore.js" \ + "$node_modules/which-country/index.js" \ + "$vendor_js" + +log "Copying certain JS developer modules into vendor directory..." +if [[ "${NODE_ENV:-production}" = development ]] ; then + cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" + cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" +else + # TODO: https://github.com/openedx/edx-platform/issues/31768 + # In the old implementation of this scipt (pavelib/assets.py), these two + # developer libraries were copied into the JS vendor directory whether not + # the build was for prod or dev. In order to exactly match the output of + # the old script, this script will also copy them in for prod builds. + # However, in the future, it would be good to only copy them for dev + # builds. Furthermore, these libraries should not be `npm install`ed + # into prod builds in the first place. + cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true # "|| true" means "tolerate errors"; in this case, + cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true # that's "tolerate if these files don't exist." +fi + +# Done echoing. +set +x + +log "-------------------------------------------------------------------------------" +log " Done copying required assets from node_modules." +log "=====================================================================================" + From 6555adc473cc227b1137ba7e215cc6f4bea28db2 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Wed, 4 Oct 2023 13:34:16 -0400 Subject: [PATCH 5/6] build: log `npm run postinstall` steps to STDOUT so they are visible in CI (#33142) Without this change, `npm run postinstall` (aka scripts/copy-node-modules.sh) uses `set -x`, which echo steps to STDERR. By default, it seems that GitHub Actions doesn't show STDERR. Having the steps visible in CI was very useful while debugging some Tutor build improvements, so I figured it would be good to upstream the change. --- scripts/copy-node-modules.sh | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/scripts/copy-node-modules.sh b/scripts/copy-node-modules.sh index db997da95785..f49514b6def1 100755 --- a/scripts/copy-node-modules.sh +++ b/scripts/copy-node-modules.sh @@ -26,28 +26,33 @@ log ( ) { echo -e "${COL_LOG}$* $COL_OFF" } +# Print a command (prefixed with '+') and then run it, to aid in debugging. +# This is just like `set -x`, except that `set -x` prints to STDERR, which is hidden +# by GitHub Actions logs. This functions prints to STDOUT, which is visible. +log_and_run ( ) { + log "+$*" + "$@" # Joins arguments to form a command (quoting as necessary) and runs the command. +} + log "=====================================================================================" log "Copying required assets from node_modules..." log "-------------------------------------------------------------------------------" -# Start echoing all commands back to user for ease of debugging. -set -x - log "Ensuring vendor directories exist..." -mkdir -p "$vendor_js" -mkdir -p "$vendor_css" +log_and_run mkdir -p "$vendor_js" +log_and_run mkdir -p "$vendor_css" log "Copying studio-frontend JS & CSS from node_modules into vendor directores..." while read -r -d $'\0' src_file ; do if [[ "$src_file" = *.css ]] || [[ "$src_file" = *.css.map ]] ; then - cp --force "$src_file" "$vendor_css" + log_and_run cp --force "$src_file" "$vendor_css" else - cp --force "$src_file" "$vendor_js" + log_and_run cp --force "$src_file" "$vendor_js" fi done < <(find "$node_modules/@edx/studio-frontend/dist" -type f -print0) log "Copying certain JS modules from node_modules into vendor directory..." -cp --force \ +log_and_run cp --force \ "$node_modules/backbone.paginator/lib/backbone.paginator.js" \ "$node_modules/backbone/backbone.js" \ "$node_modules/bootstrap/dist/js/bootstrap.bundle.js" \ @@ -66,8 +71,8 @@ cp --force \ log "Copying certain JS developer modules into vendor directory..." if [[ "${NODE_ENV:-production}" = development ]] ; then - cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" - cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" + log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" + log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" else # TODO: https://github.com/openedx/edx-platform/issues/31768 # In the old implementation of this scipt (pavelib/assets.py), these two @@ -77,13 +82,10 @@ else # However, in the future, it would be good to only copy them for dev # builds. Furthermore, these libraries should not be `npm install`ed # into prod builds in the first place. - cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true # "|| true" means "tolerate errors"; in this case, - cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true # that's "tolerate if these files don't exist." + log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true # "|| true" means "tolerate errors"; in this case, + log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true # that's "tolerate if these files don't exist." fi -# Done echoing. -set +x - log "-------------------------------------------------------------------------------" log " Done copying required assets from node_modules." log "=====================================================================================" From 549278e499f0fe91c23a5c3396394bb88c576cfb Mon Sep 17 00:00:00 2001 From: Ali Salman Date: Mon, 4 Dec 2023 17:31:00 +0500 Subject: [PATCH 6/6] update pacakge.json --- package-lock.json | 1 + package.json | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index fab2ef0b3c44..074c1d44f150 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,6 +7,7 @@ "": { "name": "edx", "version": "0.1.0", + "hasInstallScript": true, "dependencies": { "@babel/core": "7.19.0", "@babel/plugin-proposal-object-rest-spread": "^7.18.9", diff --git a/package.json b/package.json index 8a21fece5eda..cf483823dec2 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,16 @@ "version": "0.1.0", "repository": "https://github.com/openedx/edx-platform", "scripts": { - "postinstall": "scripts/copy-node-modules.sh" + "postinstall": "scripts/copy-node-modules.sh", + "build": "echo 'WARNING: `npm run build` in edx-platform is experimental. Use at your own risk.' && npm run webpack && npm run compile-sass", + "build-dev": "echo 'WARNING: `npm run build-dev` in edx-platform is experimental. Use at your own risk.' && npm run webpack-dev && npm run compile-sass-dev", + "webpack": "NODE_ENV=${NODE_ENV:-production} \"$(npm bin)/webpack\" --config=${WEBPACK_CONFIG_PATH:-webpack.prod.config.js}", + "webpack-dev": "NODE_ENV=development \"$(npm bin)/webpack\" --config=webpack.dev.config.js", + "compile-sass": "scripts/compile_sass.py --env=${NODE_ENV:-production}", + "compile-sass-dev": "scripts/compile_sass.py --env=development", + "watch": "echo 'WARNING: `npm run watch` in edx-platform is experimental. Use at your own risk.' && { npm run watch-webpack& npm run watch-sass& } && sleep infinity", + "watch-webpack": "npm run webpack-dev -- --watch", + "watch-sass": "scripts/watch_sass.sh" }, "dependencies": { "@babel/core": "7.19.0",