From b2ba83654f5170b4950a88923f4ecce2ff7c01b3 Mon Sep 17 00:00:00 2001 From: Pooja Kulkarni <13742492+pkulkark@users.noreply.github.com> Date: Fri, 11 Dec 2020 20:06:31 +0530 Subject: [PATCH] Fix the theme compilation issue since juniper (#24990) Replaces the deprecated CachedFilesMixin with ManifestFilesMixin. Also sorts xmodules and xblocks js bundles to make theme assets compilation hashes consistent across different machines. (cherry picked from commit e9cba8c2829f6ff9b6823080882c305afc223aaa) --- common/lib/xmodule/xmodule/static_content.py | 33 +++++++++++++------- openedx/core/djangoapps/theming/storage.py | 12 +++---- openedx/core/storage.py | 4 +-- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/common/lib/xmodule/xmodule/static_content.py b/common/lib/xmodule/xmodule/static_content.py index 85855f831cbc..90637f5021ab 100755 --- a/common/lib/xmodule/xmodule/static_content.py +++ b/common/lib/xmodule/xmodule/static_content.py @@ -94,20 +94,24 @@ def write_descriptor_js(output_root): def _list_descriptors(): """Return a list of all registered XModuleDescriptor classes.""" - return [ - desc for desc in [ + return sorted( + [ desc for (_, desc) in XModuleDescriptor.load_classes() - ] - ] + XBLOCK_CLASSES + ] + XBLOCK_CLASSES, + key=str + ) def _list_modules(): """Return a list of all registered XModule classes.""" - return [ - desc.module_class for desc in [ - desc for (_, desc) in XModuleDescriptor.load_classes() - ] - ] + XBLOCK_CLASSES + return sorted( + [ + desc.module_class for desc in [ + desc for (_, desc) in XModuleDescriptor.load_classes() + ] + ] + XBLOCK_CLASSES, + key=str + ) def _ensure_dir(directory): @@ -152,7 +156,8 @@ def _write_styles(selector, output_root, classes, css_attribute): "@import 'bourbon/bourbon';", "@import 'lms/theme/variables';", ] - for class_, fragment_names in css_imports.items(): + for class_, fragment_names in sorted(css_imports.items()): + fragment_names = sorted(fragment_names) module_styles_lines.append("""{selector}.xmodule_{class_} {{""".format( class_=class_, selector=selector )) @@ -268,7 +273,13 @@ def write_webpack(output_file, module_files, descriptor_files): outfile.write( textwrap.dedent(u"""\ module.exports = {config_json}; - """).format(config_json=json.dumps(config, indent=4)) + """).format( + config_json=json.dumps( + config, + indent=4, + sort_keys=True, + ) + ) ) diff --git a/openedx/core/djangoapps/theming/storage.py b/openedx/core/djangoapps/theming/storage.py index 7d560819489a..61254c9d7952 100644 --- a/openedx/core/djangoapps/theming/storage.py +++ b/openedx/core/djangoapps/theming/storage.py @@ -10,7 +10,7 @@ from django.conf import settings from django.contrib.staticfiles.finders import find -from django.contrib.staticfiles.storage import CachedFilesMixin, StaticFilesStorage +from django.contrib.staticfiles.storage import ManifestFilesMixin, StaticFilesStorage from django.utils._os import safe_join from django.utils.six.moves.urllib.parse import ( # pylint: disable=no-name-in-module, import-error unquote, @@ -110,10 +110,10 @@ class ThemeStorage(ThemeMixin, StaticFilesStorage): pass -class ThemeCachedFilesMixin(CachedFilesMixin): +class ThemeManifestFilesMixin(ManifestFilesMixin): """ - Comprehensive theme aware CachedFilesMixin. - Main purpose of subclassing CachedFilesMixin is to override the following methods. + Comprehensive theme aware ManifestFilesMixin. + Main purpose of subclassing ManifestFilesMixin is to override the following methods. 1 - _url 2 - url_converter @@ -177,11 +177,11 @@ def _url(self, hashed_name_func, name, force=False, hashed_files=None): See the class docstring for more info. """ processed_asset_name = self._processed_asset_name(name) - return super(ThemeCachedFilesMixin, self)._url(hashed_name_func, processed_asset_name, force, hashed_files) + return super()._url(hashed_name_func, processed_asset_name, force, hashed_files) def url_converter(self, name, hashed_files, template=None): """ - This is an override of url_converter from CachedFilesMixin. + This is an override of url_converter from ManifestFilesMixin. It changes one line near the end of the method (see the NOTE) in order to return absolute urls instead of relative urls. This behavior is necessary for theme overrides, as we get 404 on assets with relative diff --git a/openedx/core/storage.py b/openedx/core/storage.py index fefad2ee4a31..b5b61698e2ff 100644 --- a/openedx/core/storage.py +++ b/openedx/core/storage.py @@ -12,7 +12,7 @@ from require.storage import OptimizedFilesMixin from storages.backends.s3boto3 import S3Boto3Storage -from openedx.core.djangoapps.theming.storage import ThemeCachedFilesMixin, ThemePipelineMixin, ThemeMixin +from openedx.core.djangoapps.theming.storage import ThemeManifestFilesMixin, ThemePipelineMixin, ThemeMixin class PipelineForgivingMixin(object): @@ -44,7 +44,7 @@ class ProductionMixin( PipelineForgivingMixin, OptimizedFilesMixin, ThemePipelineMixin, - ThemeCachedFilesMixin, + ThemeManifestFilesMixin, ThemeMixin, ): """