From b866f3562063d90ef6e213c684f3ba29d2dc8280 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 30 Oct 2017 14:36:06 -0400 Subject: [PATCH] Remove support for COMPREHENSIVE_THEME_DIR - all dirs must now go into COMPREHENSIVE_THEME_DIRS. Move comprehensive theming setup section out of startup.py and into settings files using new 'derived' functionality. Add 'derive_settings' at the end of all top-level Django settings files. Move validation of comprehensive theming settings into new apps.py theming file. Split theming code into code safe to run before settings are initialized -and- after settings are initialized. --- cms/envs/aws.py | 9 +- cms/envs/common.py | 32 ++- cms/envs/dev.py | 5 + cms/envs/test.py | 5 + cms/envs/test_static_optimized.py | 5 + cms/envs/yaml_config.py | 5 + cms/startup.py | 14 +- lms/envs/aws.py | 5 + lms/envs/common.py | 38 ++- lms/envs/dev.py | 5 + lms/envs/static.py | 5 + lms/envs/test.py | 7 +- lms/envs/test_static_optimized.py | 5 + lms/envs/yaml_config.py | 5 + lms/startup.py | 7 - openedx/core/djangoapps/theming/apps.py | 83 ++++++ openedx/core/djangoapps/theming/core.py | 38 --- openedx/core/djangoapps/theming/helpers.py | 264 +++++------------- .../core/djangoapps/theming/helpers_dirs.py | 165 +++++++++++ .../core/djangoapps/theming/helpers_static.py | 19 ++ .../management/commands/compile_sass.py | 2 +- .../theming/templatetags/theme_pipeline.py | 2 +- .../djangoapps/theming/tests/test_helpers.py | 16 +- 23 files changed, 455 insertions(+), 286 deletions(-) create mode 100644 openedx/core/djangoapps/theming/apps.py delete mode 100644 openedx/core/djangoapps/theming/core.py create mode 100644 openedx/core/djangoapps/theming/helpers_dirs.py create mode 100644 openedx/core/djangoapps/theming/helpers_static.py diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 991e2212e6..855543d463 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -15,6 +15,7 @@ import json from .common import * +from openedx.core.lib.derived import derive_settings from openedx.core.lib.logsettings import get_logger_config import os @@ -202,10 +203,6 @@ COURSES_WITH_UNSAFE_CODE = ENV_TOKENS.get("COURSES_WITH_UNSAFE_CODE", []) ASSET_IGNORE_REGEX = ENV_TOKENS.get('ASSET_IGNORE_REGEX', ASSET_IGNORE_REGEX) -# following setting is for backward compatibility -if ENV_TOKENS.get('COMPREHENSIVE_THEME_DIR', None): - COMPREHENSIVE_THEME_DIR = ENV_TOKENS.get('COMPREHENSIVE_THEME_DIR') - COMPREHENSIVE_THEME_DIRS = ENV_TOKENS.get('COMPREHENSIVE_THEME_DIRS', COMPREHENSIVE_THEME_DIRS) or [] # COMPREHENSIVE_THEME_LOCALE_PATHS contain the paths to themes locale directories e.g. @@ -534,3 +531,7 @@ PARENTAL_CONSENT_AGE_LIMIT = ENV_TOKENS.get( # Allow extra middleware classes to be added to the app through configuration. MIDDLEWARE_CLASSES.extend(ENV_TOKENS.get('EXTRA_MIDDLEWARE_CLASSES', [])) + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/cms/envs/common.py b/cms/envs/common.py index 2f6e91d02b..a0d36d2f8f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -50,7 +50,7 @@ import lms.envs.common from lms.envs.common import ( USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, PLATFORM_DESCRIPTION, BUGS_EMAIL, DOC_STORE_CONFIG, DATA_DIR, ALL_LANGUAGES, WIKI_ENABLED, update_module_store_settings, ASSET_IGNORE_REGEX, - PARENTAL_CONSENT_AGE_LIMIT, COMPREHENSIVE_THEME_DIRS, REGISTRATION_EMAIL_PATTERNS_ALLOWED, + PARENTAL_CONSENT_AGE_LIMIT, REGISTRATION_EMAIL_PATTERNS_ALLOWED, # The following PROFILE_IMAGE_* settings are included as they are # indirectly accessed through the email opt-in API, which is # technically accessible through the CMS via legacy URLs. @@ -81,6 +81,8 @@ from lms.envs.common import ( # Enable or disable theming ENABLE_COMPREHENSIVE_THEMING, + COMPREHENSIVE_THEME_LOCALE_PATHS, + COMPREHENSIVE_THEME_DIRS, # constants for redirects app REDIRECT_CACHE_TIMEOUT, @@ -113,6 +115,10 @@ from lms.envs.common import ( # Video Image settings VIDEO_IMAGE_SETTINGS, VIDEO_TRANSCRIPTS_SETTINGS, + + # Methods to derive settings + _make_main_mako_templates, + _make_locale_paths, ) from path import Path as path from warnings import simplefilter @@ -121,7 +127,12 @@ from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin from cms.lib.xblock.authoring_mixin import AuthoringMixin import dealer.git from xmodule.modulestore.edit_info import EditInfoMixin +from openedx.core.djangoapps.theming.helpers_dirs import ( + get_themes_unchecked, + get_theme_base_dirs_from_settings +) from openedx.core.lib.license import LicenseMixin +from openedx.core.lib.derived import derived, derived_dict_entry ############################ FEATURE CONFIGURATION ############################# @@ -300,7 +311,7 @@ GEOIPV6_PATH = REPO_ROOT / "common/static/data/geoip/GeoIPv6.dat" import tempfile MAKO_MODULE_DIR = os.path.join(tempfile.gettempdir(), 'mako_cms') MAKO_TEMPLATES = {} -MAKO_TEMPLATES['main'] = [ +MAIN_MAKO_TEMPLATES_BASE = [ PROJECT_ROOT / 'templates', COMMON_ROOT / 'templates', COMMON_ROOT / 'djangoapps' / 'pipeline_mako' / 'templates', @@ -310,9 +321,10 @@ MAKO_TEMPLATES['main'] = [ OPENEDX_ROOT / 'core' / 'lib' / 'license' / 'templates', CMS_ROOT / 'djangoapps' / 'pipeline_js' / 'templates', ] +MAKO_TEMPLATES['lms.main'] = lms.envs.common.MAIN_MAKO_TEMPLATES_BASE -for namespace, template_dirs in lms.envs.common.MAKO_TEMPLATES.iteritems(): - MAKO_TEMPLATES['lms.' + namespace] = template_dirs +MAKO_TEMPLATES['main'] = _make_main_mako_templates +derived_dict_entry('MAKO_TEMPLATES', 'main') # Django templating TEMPLATES = [ @@ -321,7 +333,7 @@ TEMPLATES = [ # Don't look for template source files inside installed applications. 'APP_DIRS': False, # Instead, look for template source files in these dirs. - 'DIRS': MAKO_TEMPLATES['main'], + 'DIRS': MAIN_MAKO_TEMPLATES_BASE, # Options specific to this backend. 'OPTIONS': { 'loaders': ( @@ -601,8 +613,9 @@ USE_L10N = True STATICI18N_ROOT = PROJECT_ROOT / "static" -# Localization strings (e.g. django.po) are under this directory -LOCALE_PATHS = (REPO_ROOT + '/conf/locale',) # edx-platform/conf/locale/ +# Localization strings (e.g. django.po) are under these directories +LOCALE_PATHS = _make_locale_paths +derived('LOCALE_PATHS') # Messages MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' @@ -959,7 +972,7 @@ INSTALLED_APPS = [ 'webpack_loader', # Theming - 'openedx.core.djangoapps.theming', + 'openedx.core.djangoapps.theming.apps.ThemingConfig', # Site configuration for theming and behavioral modification 'openedx.core.djangoapps.site_configuration', @@ -1370,9 +1383,6 @@ AFFILIATE_COOKIE_NAME = 'affiliate_id' HELP_TOKENS_INI_FILE = REPO_ROOT / "cms" / "envs" / "help_tokens.ini" -# Theme directory locale paths -COMPREHENSIVE_THEME_LOCALE_PATHS = [] - # This is required for the migrations in oauth_dispatch.models # otherwise it fails saying this attribute is not present in Settings # Although Studio does not exable OAuth2 Provider capability, the new approach diff --git a/cms/envs/dev.py b/cms/envs/dev.py index ae4efe5ec4..20dd68d6a5 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -6,6 +6,7 @@ This config file runs the simplest dev environment""" # pylint: disable=wildcard-import, unused-wildcard-import from .common import * +from openedx.core.lib.derived import derive_settings from openedx.core.lib.logsettings import get_logger_config # import settings from LMS for consistent behavior with CMS @@ -179,3 +180,7 @@ try: from .private import * # pylint: disable=import-error except ImportError: pass + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/cms/envs/test.py b/cms/envs/test.py index 5657ecd1b8..1bcd33372b 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -24,6 +24,7 @@ from path import Path as path from warnings import filterwarnings, simplefilter from uuid import uuid4 from util.db import NoOpMigrationModules +from openedx.core.lib.derived import derive_settings # import settings from LMS for consistent behavior with CMS # pylint: disable=unused-import @@ -360,3 +361,7 @@ VIDEO_TRANSCRIPTS_SETTINGS = dict( ), DIRECTORY_PREFIX='video-transcripts/', ) + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/cms/envs/test_static_optimized.py b/cms/envs/test_static_optimized.py index ddff762b57..2874ee494d 100644 --- a/cms/envs/test_static_optimized.py +++ b/cms/envs/test_static_optimized.py @@ -12,6 +12,7 @@ from the same directory. # Start with the common settings from .common import * # pylint: disable=wildcard-import, unused-wildcard-import +from openedx.core.lib.derived import derive_settings # Use an in-memory database since this settings file is only used for updating assets DATABASES = { @@ -46,3 +47,7 @@ WEBPACK_LOADER['DEFAULT']['STATS_FILE'] = STATIC_ROOT / "webpack-stats.json" # 1. Uglify is by far the slowest part of the build process # 2. Having full source code makes debugging tests easier for developers os.environ['REQUIRE_BUILD_PROFILE_OPTIMIZE'] = 'none' + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/cms/envs/yaml_config.py b/cms/envs/yaml_config.py index f8944b3e8b..1def36ef06 100644 --- a/cms/envs/yaml_config.py +++ b/cms/envs/yaml_config.py @@ -17,6 +17,7 @@ defined in the environment: import yaml from .common import * +from openedx.core.lib.derived import derive_settings from openedx.core.lib.logsettings import get_logger_config from util.config_parse import convert_tokens import os @@ -264,3 +265,7 @@ if FEATURES.get('CUSTOM_COURSES_EDX'): # Allow extra middleware classes to be added to the app through configuration. MIDDLEWARE_CLASSES.extend(ENV_TOKENS.get('EXTRA_MIDDLEWARE_CLASSES', [])) + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/cms/startup.py b/cms/startup.py index 29b11e1716..4ed7cf4490 100644 --- a/cms/startup.py +++ b/cms/startup.py @@ -8,17 +8,16 @@ from django.conf import settings import cms.lib.xblock.runtime import xmodule.x_module from openedx.core.djangoapps.monkey_patch import django_db_models_options -from openedx.core.djangoapps.theming.core import enable_theming -from openedx.core.djangoapps.theming.helpers import is_comprehensive_theming_enabled from openedx.core.lib.django_startup import autostartup -from openedx.core.lib.xblock_utils import xblock_local_resource_url -from openedx.core.release import doc_version -from startup_configurations.validate_config import validate_cms_config # Force settings to run so that the python path is modified settings.INSTALLED_APPS # pylint: disable=pointless-statement +from openedx.core.lib.xblock_utils import xblock_local_resource_url +from openedx.core.release import doc_version +from startup_configurations.validate_config import validate_cms_config + def run(): """ @@ -29,11 +28,6 @@ def run(): """ django_db_models_options.patch() - # Comprehensive theming needs to be set up before django startup, - # because modifying django template paths after startup has no effect. - if is_comprehensive_theming_enabled(): - enable_theming() - django.setup() autostartup() diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 8b61c3d70c..7be1753a7c 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -25,6 +25,7 @@ import warnings import dateutil from .common import * +from openedx.core.lib.derived import derive_settings from openedx.core.lib.logsettings import get_logger_config import os @@ -1068,3 +1069,7 @@ ACE_ROUTING_KEY = ENV_TOKENS.get('ACE_ROUTING_KEY', ACE_ROUTING_KEY) # Allow extra middleware classes to be added to the app through configuration. MIDDLEWARE_CLASSES.extend(ENV_TOKENS.get('EXTRA_MIDDLEWARE_CLASSES', [])) + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/lms/envs/common.py b/lms/envs/common.py index bbbf537e19..1517e8be6d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -39,9 +39,13 @@ from warnings import simplefilter from django.utils.translation import ugettext_lazy as _ from .discussionsettings import * +from openedx.core.djangoapps.theming.helpers_dirs import ( + get_themes_unchecked, + get_theme_base_dirs_from_settings +) +from openedx.core.lib.derived import derived, derived_dict_entry from xmodule.modulestore.modulestore_settings import update_module_store_settings from xmodule.modulestore.edit_info import EditInfoMixin -from openedx.core.lib.license import LicenseMixin from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin ################################### FEATURES ################################### @@ -530,7 +534,7 @@ OAUTH2_PROVIDER_APPLICATION_MODEL = 'oauth2_provider.Application' import tempfile MAKO_MODULE_DIR = os.path.join(tempfile.gettempdir(), 'mako_lms') MAKO_TEMPLATES = {} -MAKO_TEMPLATES['main'] = [ +MAIN_MAKO_TEMPLATES_BASE = [ PROJECT_ROOT / 'templates', COMMON_ROOT / 'templates', COMMON_ROOT / 'lib' / 'capa' / 'capa' / 'templates', @@ -540,6 +544,20 @@ MAKO_TEMPLATES['main'] = [ OPENEDX_ROOT / 'core' / 'lib' / 'license' / 'templates', ] + +def _make_main_mako_templates(settings): + """ + Derives the final MAKO_TEMPLATES['main'] setting from other settings. + """ + if settings.ENABLE_COMPREHENSIVE_THEMING: + themes_dirs = get_theme_base_dirs_from_settings(settings.COMPREHENSIVE_THEME_DIRS) + for theme in get_themes_unchecked(themes_dirs, PROJECT_ROOT): + if theme.themes_base_dir not in settings.MAIN_MAKO_TEMPLATES_BASE: + settings.MAIN_MAKO_TEMPLATES_BASE.insert(0, theme.themes_base_dir) + return settings.MAIN_MAKO_TEMPLATES_BASE +MAKO_TEMPLATES['main'] = _make_main_mako_templates +derived_dict_entry('MAKO_TEMPLATES', 'main') + # Django templating TEMPLATES = [ { @@ -1015,8 +1033,18 @@ USE_L10N = True STATICI18N_ROOT = PROJECT_ROOT / "static" STATICI18N_OUTPUT_DIR = "js/i18n" -# Localization strings (e.g. django.po) are under this directory -LOCALE_PATHS = (REPO_ROOT + '/conf/locale',) # edx-platform/conf/locale/ + +# Localization strings (e.g. django.po) are under these directories +def _make_locale_paths(settings): + locale_paths = [settings.REPO_ROOT + '/conf/locale'] # edx-platform/conf/locale/ + if settings.ENABLE_COMPREHENSIVE_THEMING: + # Add locale paths to settings for comprehensive theming. + for locale_path in settings.COMPREHENSIVE_THEME_LOCALE_PATHS: + locale_paths += (path(locale_path), ) + return locale_paths +LOCALE_PATHS = _make_locale_paths +derived('LOCALE_PATHS') + # Messages MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' @@ -2018,7 +2046,7 @@ INSTALLED_APPS = [ 'openedx.core.djangoapps.contentserver', # Theming - 'openedx.core.djangoapps.theming', + 'openedx.core.djangoapps.theming.apps.ThemingConfig', # Site configuration for theming and behavioral modification 'openedx.core.djangoapps.site_configuration', diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 6dd5e729e4..fad2d4652f 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -13,6 +13,7 @@ sessions. Assumes structure: # pylint: disable=wildcard-import, unused-wildcard-import from .common import * +from openedx.core.lib.derived import derive_settings DEBUG = True TEMPLATE_DEBUG = True @@ -270,3 +271,7 @@ try: from .private import * # pylint: disable=import-error except ImportError: pass + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/lms/envs/static.py b/lms/envs/static.py index 9f9749a953..3a02a8140d 100644 --- a/lms/envs/static.py +++ b/lms/envs/static.py @@ -13,6 +13,7 @@ sessions. Assumes structure: # pylint: disable=wildcard-import, unused-wildcard-import from .common import * +from openedx.core.lib.derived import derive_settings from openedx.core.lib.logsettings import get_logger_config STATIC_GRAB = True @@ -69,3 +70,7 @@ FILE_UPLOAD_HANDLERS = [ 'django.core.files.uploadhandler.MemoryFileUploadHandler', 'django.core.files.uploadhandler.TemporaryFileUploadHandler', ] + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/lms/envs/test.py b/lms/envs/test.py index 83eb021ad4..af913e7533 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -25,6 +25,7 @@ from uuid import uuid4 from warnings import filterwarnings, simplefilter from util.db import NoOpMigrationModules +from openedx.core.lib.derived import derive_settings from openedx.core.lib.tempdir import mkdtemp_clean # This patch disables the commit_on_success decorator during tests @@ -506,7 +507,7 @@ MICROSITE_LOGISTRATION_HOSTNAME = 'logistration.testserver' TEST_THEME = COMMON_ROOT / "test" / "test-theme" # add extra template directory for test-only templates -MAKO_TEMPLATES['main'].extend([ +MAIN_MAKO_TEMPLATES_BASE.extend([ COMMON_ROOT / 'test' / 'templates', COMMON_ROOT / 'test' / 'test_sites', REPO_ROOT / 'openedx' / 'core' / 'djangolib' / 'tests' / 'templates', @@ -605,3 +606,7 @@ ENTERPRISE_CONSENT_API_URL = 'http://enterprise.example.com/consent/api/v1/' ACTIVATION_EMAIL_FROM_ADDRESS = 'test_activate@edx.org' TEMPLATES[0]['OPTIONS']['debug'] = True + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/lms/envs/test_static_optimized.py b/lms/envs/test_static_optimized.py index 618e39a438..25a44eb9e4 100644 --- a/lms/envs/test_static_optimized.py +++ b/lms/envs/test_static_optimized.py @@ -12,6 +12,7 @@ from the same directory. # Start with the common settings from .common import * # pylint: disable=wildcard-import, unused-wildcard-import +from openedx.core.lib.derived import derive_settings # Use an in-memory database since this settings file is only used for updating assets DATABASES = { @@ -59,3 +60,7 @@ WEBPACK_LOADER['DEFAULT']['STATS_FILE'] = STATIC_ROOT / "webpack-stats.json" # 1. Uglify is by far the slowest part of the build process # 2. Having full source code makes debugging tests easier for developers os.environ['REQUIRE_BUILD_PROFILE_OPTIMIZE'] = 'none' + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/lms/envs/yaml_config.py b/lms/envs/yaml_config.py index 50c633764d..83ed8b56b7 100644 --- a/lms/envs/yaml_config.py +++ b/lms/envs/yaml_config.py @@ -16,6 +16,7 @@ defined in the environment: import yaml from .common import * +from openedx.core.lib.derived import derive_settings from openedx.core.lib.logsettings import get_logger_config from util.config_parse import convert_tokens import os @@ -329,3 +330,7 @@ CREDENTIALS_GENERATION_ROUTING_KEY = HIGH_PRIORITY_QUEUE # Allow extra middleware classes to be added to the app through configuration. MIDDLEWARE_CLASSES.extend(ENV_TOKENS.get('EXTRA_MIDDLEWARE_CLASSES', [])) + +########################## Derive Any Derived Settings ####################### + +derive_settings(__name__) diff --git a/lms/startup.py b/lms/startup.py index 05b4ff18f9..d5836dad34 100644 --- a/lms/startup.py +++ b/lms/startup.py @@ -21,8 +21,6 @@ import xmodule.x_module import lms_xblock.runtime from startup_configurations.validate_config import validate_lms_config -from openedx.core.djangoapps.theming.core import enable_theming -from openedx.core.djangoapps.theming.helpers import is_comprehensive_theming_enabled from microsite_configuration import microsite @@ -38,11 +36,6 @@ def run(): """ django_db_models_options.patch() - # Comprehensive theming needs to be set up before django startup, - # because modifying django template paths after startup has no effect. - if is_comprehensive_theming_enabled(): - enable_theming() - # We currently use 2 template rendering engines, mako and django_templates, # and one of them (django templates), requires the directories be added # before the django.setup(). diff --git a/openedx/core/djangoapps/theming/apps.py b/openedx/core/djangoapps/theming/apps.py new file mode 100644 index 0000000000..a02cc6971b --- /dev/null +++ b/openedx/core/djangoapps/theming/apps.py @@ -0,0 +1,83 @@ + +import os +import six +from django.apps import AppConfig +from django.conf import settings +from django.core.checks import Error, Tags, register + + +class ThemingConfig(AppConfig): + name = 'openedx.core.djangoapps.theming' + verbose_name = "Theming" + + +@register(Tags.compatibility) +def check_comprehensive_theme_settings(app_configs, **kwargs): + """ + Checks the comprehensive theming theme directory settings. + + Raises compatibility Errors upon: + - COMPREHENSIVE_THEME_DIRS is not a list + - theme dir path is not a string + - theme dir path is not an absolute path + - path specified in COMPREHENSIVE_THEME_DIRS does not exist + + Returns: + List of any Errors. + """ + if not getattr(settings, "ENABLE_COMPREHENSIVE_THEMING"): + # Only perform checks when comprehensive theming is enabled. + return [] + + errors = [] + + # COMPREHENSIVE_THEME_DIR is no longer supported - support has been removed. + if hasattr(settings, "COMPREHENSIVE_THEME_DIR"): + theme_dir = settings.COMPREHENSIVE_THEME_DIR + + errors.append( + Error( + "COMPREHENSIVE_THEME_DIR setting has been removed in favor of COMPREHENSIVE_THEME_DIRS.", + hint='Transfer the COMPREHENSIVE_THEME_DIR value to COMPREHENSIVE_THEME_DIRS.', + obj=theme_dir, + id='openedx.core.djangoapps.theming.E001', + ) + ) + + if hasattr(settings, "COMPREHENSIVE_THEME_DIRS"): + theme_dirs = settings.COMPREHENSIVE_THEME_DIRS + + if not isinstance(theme_dirs, list): + errors.append( + Error( + "COMPREHENSIVE_THEME_DIRS must be a list.", + obj=theme_dirs, + id='openedx.core.djangoapps.theming.E004', + ) + ) + if not all([isinstance(theme_dir, six.string_types) for theme_dir in theme_dirs]): + errors.append( + Error( + "COMPREHENSIVE_THEME_DIRS must contain only strings.", + obj=theme_dirs, + id='openedx.core.djangoapps.theming.E005', + ) + ) + if not all([theme_dir.startswith("/") for theme_dir in theme_dirs]): + errors.append( + Error( + "COMPREHENSIVE_THEME_DIRS must contain only absolute paths to themes dirs.", + obj=theme_dirs, + id='openedx.core.djangoapps.theming.E006', + ) + ) + if not all([os.path.isdir(theme_dir) for theme_dir in theme_dirs]): + errors.append( + Error( + "COMPREHENSIVE_THEME_DIRS must contain valid paths.", + obj=theme_dirs, + id='openedx.core.djangoapps.theming.E007', + ) + ) + + return errors diff --git a/openedx/core/djangoapps/theming/core.py b/openedx/core/djangoapps/theming/core.py deleted file mode 100644 index d2e68c7dd1..0000000000 --- a/openedx/core/djangoapps/theming/core.py +++ /dev/null @@ -1,38 +0,0 @@ -""" -Core logic for Comprehensive Theming. -""" -from logging import getLogger - -from django.conf import settings -from path import Path as path - -from .helpers import get_themes - -logger = getLogger(__name__) # pylint: disable=invalid-name - - -def enable_theming(): - """ - Add directories and relevant paths to settings for comprehensive theming. - """ - # Deprecated Warnings - if hasattr(settings, "COMPREHENSIVE_THEME_DIR"): - logger.warning( - "\033[93m \nDeprecated: " - "\n\tCOMPREHENSIVE_THEME_DIR setting has been deprecated in favor of COMPREHENSIVE_THEME_DIRS.\033[00m" - ) - - for theme in get_themes(): - if theme.themes_base_dir not in settings.MAKO_TEMPLATES['main']: - settings.MAKO_TEMPLATES['main'].insert(0, theme.themes_base_dir) - - _add_theming_locales() - - -def _add_theming_locales(): - """ - Add locale paths to settings for comprehensive theming. - """ - theme_locale_paths = settings.COMPREHENSIVE_THEME_LOCALE_PATHS - for locale_path in theme_locale_paths: - settings.LOCALE_PATHS += (path(locale_path), ) # pylint: disable=no-member diff --git a/openedx/core/djangoapps/theming/helpers.py b/openedx/core/djangoapps/theming/helpers.py index d178c81111..69140b9e6d 100644 --- a/openedx/core/djangoapps/theming/helpers.py +++ b/openedx/core/djangoapps/theming/helpers.py @@ -5,12 +5,18 @@ import os import re from logging import getLogger -from django.conf import ImproperlyConfigured, settings -from django.contrib.staticfiles.storage import staticfiles_storage +from django.conf import settings from path import Path from microsite_configuration import microsite from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangoapps.theming.helpers_dirs import ( + get_theme_base_dirs_from_settings, + get_themes_unchecked, + get_theme_dirs, + get_project_root_name_from_settings, + Theme +) from request_cache.middleware import RequestCache logger = getLogger(__name__) # pylint: disable=invalid-name @@ -101,6 +107,23 @@ def get_all_theme_template_dirs(): return template_paths +def get_project_root_name(): + """ + Return root name for the current project + + Example: + >> get_project_root_name() + 'lms' + # from studio + >> get_project_root_name() + 'cms' + + Returns: + (str): component name of platform e.g lms, cms + """ + return get_project_root_name_from_settings(settings.PROJECT_ROOT) + + def strip_site_theme_templates_path(uri): """ Remove site template theme path from the uri. @@ -189,6 +212,7 @@ def get_current_theme(): name=site_theme.theme_dir_name, theme_dir_name=site_theme.theme_dir_name, themes_base_dir=get_theme_base_dir(site_theme.theme_dir_name), + project_root=get_project_root_name() ) except ValueError as error: # Log exception message and return None, so that open source theme is used instead @@ -232,78 +256,64 @@ def get_theme_base_dir(theme_dir_name, suppress_error=False): )) -def get_project_root_name(): +def theme_exists(theme_name, themes_dir=None): """ - Return root name for the current project + Returns True if a theme exists with the specified name. + """ + for theme in get_themes(themes_dir=themes_dir): + if theme.theme_dir_name == theme_name: + return True + return False + + +def get_themes(themes_dir=None): + """ + get a list of all themes known to the system. + + Args: + themes_dir (str): (Optional) Path to themes base directory + Returns: + list of themes known to the system. + """ + if not is_comprehensive_theming_enabled(): + return [] + if themes_dir is None: + themes_dir = get_theme_base_dirs_unchecked() + return get_themes_unchecked(themes_dir, settings.PROJECT_ROOT) + + +def get_theme_base_dirs_unchecked(): + """ + Return base directories that contains all the themes. Example: - >> get_project_root_name() - 'lms' - # from studio - >> get_project_root_name() - 'cms' + >> get_theme_base_dirs_unchecked() + ['/edx/app/ecommerce/ecommerce/themes'] Returns: - (str): component name of platform e.g lms, cms + (List of Paths): Base theme directory paths """ - root = Path(settings.PROJECT_ROOT) - if root.name == "": - root = root.parent - return root.name + theme_dirs = getattr(settings, "COMPREHENSIVE_THEME_DIRS", None) + + return get_theme_base_dirs_from_settings(theme_dirs) def get_theme_base_dirs(): """ - Return base directory that contains all the themes. - - Raises: - ImproperlyConfigured - exception is raised if - 1 - COMPREHENSIVE_THEME_DIRS is not a list - 1 - theme dir path is not a string - 2 - theme dir path is not an absolute path - 3 - path specified in COMPREHENSIVE_THEME_DIRS does not exist + Return base directories that contains all the themes. + Ensures comprehensive theming is enabled. Example: >> get_theme_base_dirs() ['/edx/app/ecommerce/ecommerce/themes'] Returns: - (Path): Base theme directory path + (List of Paths): Base theme directory paths """ # Return an empty list if theming is disabled if not is_comprehensive_theming_enabled(): return [] - - theme_base_dirs = [] - - # Legacy code for COMPREHENSIVE_THEME_DIR backward compatibility - if hasattr(settings, "COMPREHENSIVE_THEME_DIR"): - theme_dir = settings.COMPREHENSIVE_THEME_DIR - - if not isinstance(theme_dir, basestring): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIR must be a string.") - if not theme_dir.startswith("/"): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIR must be an absolute paths to themes dir.") - if not os.path.isdir(theme_dir): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIR must be a valid path.") - - theme_base_dirs.append(Path(theme_dir)) - - if hasattr(settings, "COMPREHENSIVE_THEME_DIRS"): - theme_dirs = settings.COMPREHENSIVE_THEME_DIRS - - if not isinstance(theme_dirs, list): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIRS must be a list.") - if not all([isinstance(theme_dir, basestring) for theme_dir in theme_dirs]): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIRS must contain only strings.") - if not all([theme_dir.startswith("/") for theme_dir in theme_dirs]): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIRS must contain only absolute paths to themes dirs.") - if not all([os.path.isdir(theme_dir) for theme_dir in theme_dirs]): - raise ImproperlyConfigured("COMPREHENSIVE_THEME_DIRS must contain valid paths.") - - theme_base_dirs.extend([Path(theme_dir) for theme_dir in theme_dirs]) - - return theme_base_dirs + return get_theme_base_dirs_unchecked() def is_comprehensive_theming_enabled(): @@ -326,149 +336,3 @@ def is_comprehensive_theming_enabled(): return False return settings.ENABLE_COMPREHENSIVE_THEMING - - -def get_static_file_url(asset): - """ - Returns url of the themed asset if asset is not themed than returns the default asset url. - - Example: - >> get_static_file_url('css/lms-main-v1.css') - '/static/red-theme/css/lms-main-v1.css' - - Parameters: - asset (str): asset's path relative to the static files directory - - Returns: - (str): static asset's url - """ - return staticfiles_storage.url(asset) - - -def get_themes(themes_dir=None): - """ - get a list of all themes known to the system. - - Args: - themes_dir (str): (Optional) Path to themes base directory - Returns: - list of themes known to the system. - """ - if not is_comprehensive_theming_enabled(): - return [] - - themes_dirs = [Path(themes_dir)] if themes_dir else get_theme_base_dirs() - # pick only directories and discard files in themes directory - themes = [] - for themes_dir in themes_dirs: - themes.extend([Theme(name, name, themes_dir) for name in get_theme_dirs(themes_dir)]) - - return themes - - -def theme_exists(theme_name, themes_dir=None): - """ - Returns True if a theme exists with the specified name. - """ - for theme in get_themes(themes_dir=themes_dir): - if theme.theme_dir_name == theme_name: - return True - return False - - -def get_theme_dirs(themes_dir=None): - """ - Returns theme dirs in given dirs - Args: - themes_dir (Path): base dir that contains themes. - """ - return [_dir for _dir in os.listdir(themes_dir) if is_theme_dir(themes_dir / _dir)] - - -def is_theme_dir(_dir): - """ - Returns true if given dir contains theme overrides. - A theme dir must have subdirectory 'lms' or 'cms' or both. - - Args: - _dir: directory path to check for a theme - - Returns: - Returns true if given dir is a theme directory. - """ - theme_sub_directories = {'lms', 'cms'} - return bool(os.path.isdir(_dir) and theme_sub_directories.intersection(os.listdir(_dir))) - - -class Theme(object): - """ - class to encapsulate theme related information. - """ - name = '' - theme_dir_name = '' - themes_base_dir = None - - def __init__(self, name='', theme_dir_name='', themes_base_dir=None): - """ - init method for Theme - - Args: - name: name if the theme - theme_dir_name: directory name of the theme - themes_base_dir: directory path of the folder that contains the theme - """ - self.name = name - self.theme_dir_name = theme_dir_name - self.themes_base_dir = themes_base_dir - - def __eq__(self, other): - """ - Returns True if given theme is same as the self - Args: - other: Theme object to compare with self - - Returns: - (bool) True if two themes are the same else False - """ - return (self.theme_dir_name, self.path) == (other.theme_dir_name, other.path) - - def __hash__(self): - return hash((self.theme_dir_name, self.path)) - - def __unicode__(self): - return u"".format(name=self.name, path=self.path) - - def __repr__(self): - return self.__unicode__() - - @property - def path(self): - """ - Get absolute path of the directory that contains current theme's templates, static assets etc. - - Returns: - Path: absolute path to current theme's contents - """ - return Path(self.themes_base_dir) / self.theme_dir_name / get_project_root_name() - - @property - def template_path(self): - """ - Get absolute path of current theme's template directory. - - Returns: - Path: absolute path to current theme's template directory - """ - return Path(self.theme_dir_name) / get_project_root_name() / 'templates' - - @property - def template_dirs(self): - """ - Get a list of all template directories for current theme. - - Returns: - list: list of all template directories for current theme. - """ - return [ - self.path / 'templates', - ] diff --git a/openedx/core/djangoapps/theming/helpers_dirs.py b/openedx/core/djangoapps/theming/helpers_dirs.py new file mode 100644 index 0000000000..7439aed5e9 --- /dev/null +++ b/openedx/core/djangoapps/theming/helpers_dirs.py @@ -0,0 +1,165 @@ +""" +Code which dynamically discovers comprehensive themes. Deliberately uses no Django settings, +as the discovery happens during the initial setup of Django settings. +""" +import os +from path import Path + + +def get_theme_base_dirs_from_settings(theme_dirs=None): + """ + Return base directories that contains all the themes. + + Example: + >> get_theme_base_dirs_from_settings('/edx/app/ecommerce/ecommerce/themes') + ['/edx/app/ecommerce/ecommerce/themes'] + + Returns: + (List of Paths): Base theme directory paths + """ + theme_base_dirs = [] + if theme_dirs: + theme_base_dirs.extend([Path(theme_dir) for theme_dir in theme_dirs]) + return theme_base_dirs + + +def get_themes_unchecked(themes_dirs, project_root=None): + """ + Returns a list of all themes known to the system. + + Args: + themes_dirs (list): Paths to themes base directory + project_root (str): (optional) Path to project root + Returns: + List of themes known to the system. + """ + themes_base_dirs = [Path(themes_dir) for themes_dir in themes_dirs] + # pick only directories and discard files in themes directory + themes = [] + for themes_dir in themes_base_dirs: + themes.extend([Theme(name, name, themes_dir, project_root) for name in get_theme_dirs(themes_dir)]) + + return themes + + +def get_theme_dirs(themes_dir=None): + """ + Returns theme dirs in given dirs + Args: + themes_dir (Path): base dir that contains themes. + """ + return [_dir for _dir in os.listdir(themes_dir) if is_theme_dir(themes_dir / _dir)] + + +def is_theme_dir(_dir): + """ + Returns true if given dir contains theme overrides. + A theme dir must have subdirectory 'lms' or 'cms' or both. + + Args: + _dir: directory path to check for a theme + + Returns: + Returns true if given dir is a theme directory. + """ + theme_sub_directories = {'lms', 'cms'} + return bool(os.path.isdir(_dir) and theme_sub_directories.intersection(os.listdir(_dir))) + + +def get_project_root_name_from_settings(project_root): + """ + Return root name for the current project + + Example: + >> get_project_root_name() + 'lms' + # from studio + >> get_project_root_name() + 'cms' + + Args: + project_root (str): Root directory of the project. + + Returns: + (str): component name of platform e.g lms, cms + """ + root = Path(project_root) + if root.name == "": + root = root.parent + return root.name + + +class Theme(object): + """ + class to encapsulate theme related information. + """ + name = '' + theme_dir_name = '' + themes_base_dir = None + project_root = None + + def __init__(self, name='', theme_dir_name='', themes_base_dir=None, project_root=None): + """ + init method for Theme + + Args: + name: name if the theme + theme_dir_name: directory name of the theme + themes_base_dir: directory path of the folder that contains the theme + """ + self.name = name + self.theme_dir_name = theme_dir_name + self.themes_base_dir = themes_base_dir + self.project_root = project_root + + def __eq__(self, other): + """ + Returns True if given theme is same as the self + Args: + other: Theme object to compare with self + + Returns: + (bool) True if two themes are the same else False + """ + return (self.theme_dir_name, self.path) == (other.theme_dir_name, other.path) + + def __hash__(self): + return hash((self.theme_dir_name, self.path)) + + def __unicode__(self): + return u"".format(name=self.name, path=self.path) + + def __repr__(self): + return self.__unicode__() + + @property + def path(self): + """ + Get absolute path of the directory that contains current theme's templates, static assets etc. + + Returns: + Path: absolute path to current theme's contents + """ + return Path(self.themes_base_dir) / self.theme_dir_name / get_project_root_name_from_settings(self.project_root) + + @property + def template_path(self): + """ + Get absolute path of current theme's template directory. + + Returns: + Path: absolute path to current theme's template directory + """ + return Path(self.theme_dir_name) / get_project_root_name_from_settings(self.project_root) / 'templates' + + @property + def template_dirs(self): + """ + Get a list of all template directories for current theme. + + Returns: + list: list of all template directories for current theme. + """ + return [ + self.path / 'templates', + ] diff --git a/openedx/core/djangoapps/theming/helpers_static.py b/openedx/core/djangoapps/theming/helpers_static.py new file mode 100644 index 0000000000..9fc54c9e03 --- /dev/null +++ b/openedx/core/djangoapps/theming/helpers_static.py @@ -0,0 +1,19 @@ + +from django.contrib.staticfiles.storage import staticfiles_storage + + +def get_static_file_url(asset): + """ + Returns url of the themed asset if asset is not themed than returns the default asset url. + + Example: + >> get_static_file_url('css/lms-main-v1.css') + '/static/red-theme/css/lms-main-v1.css' + + Parameters: + asset (str): asset's path relative to the static files directory + + Returns: + (str): static asset's url + """ + return staticfiles_storage.url(asset) diff --git a/openedx/core/djangoapps/theming/management/commands/compile_sass.py b/openedx/core/djangoapps/theming/management/commands/compile_sass.py index 9d3a33cab2..5b3f9640fa 100644 --- a/openedx/core/djangoapps/theming/management/commands/compile_sass.py +++ b/openedx/core/djangoapps/theming/management/commands/compile_sass.py @@ -92,7 +92,7 @@ class Command(BaseCommand): if theme_dirs: available_themes = {} for theme_dir in theme_dirs: - available_themes.update({t.theme_dir_name: t for t in get_themes(theme_dir)}) + available_themes.update({t.theme_dir_name: t for t in get_themes([theme_dir])}) else: theme_dirs = get_theme_base_dirs() available_themes = {t.theme_dir_name: t for t in get_themes()} diff --git a/openedx/core/djangoapps/theming/templatetags/theme_pipeline.py b/openedx/core/djangoapps/theming/templatetags/theme_pipeline.py index 7beb99ca55..3a79e7fd96 100644 --- a/openedx/core/djangoapps/theming/templatetags/theme_pipeline.py +++ b/openedx/core/djangoapps/theming/templatetags/theme_pipeline.py @@ -9,7 +9,7 @@ from django.utils.safestring import mark_safe from pipeline.templatetags.pipeline import StylesheetNode, JavascriptNode from pipeline.utils import guess_type -from openedx.core.djangoapps.theming.helpers import get_static_file_url +from openedx.core.djangoapps.theming.helpers_static import get_static_file_url register = template.Library() # pylint: disable=invalid-name diff --git a/openedx/core/djangoapps/theming/tests/test_helpers.py b/openedx/core/djangoapps/theming/tests/test_helpers.py index 862d5c95f3..96e441004c 100644 --- a/openedx/core/djangoapps/theming/tests/test_helpers.py +++ b/openedx/core/djangoapps/theming/tests/test_helpers.py @@ -22,13 +22,13 @@ class TestHelpers(TestCase): Tests template paths are returned from enabled theme. """ expected_themes = [ - Theme('dark-theme', 'dark-theme', get_theme_base_dir('dark-theme')), - Theme('edge.edx.org', 'edge.edx.org', get_theme_base_dir('edge.edx.org')), - Theme('edx.org', 'edx.org', get_theme_base_dir('edx.org')), - Theme('open-edx', 'open-edx', get_theme_base_dir('open-edx')), - Theme('red-theme', 'red-theme', get_theme_base_dir('red-theme')), - Theme('stanford-style', 'stanford-style', get_theme_base_dir('stanford-style')), - Theme('test-theme', 'test-theme', get_theme_base_dir('test-theme')), + Theme('dark-theme', 'dark-theme', get_theme_base_dir('dark-theme'), settings.PROJECT_ROOT), + Theme('edge.edx.org', 'edge.edx.org', get_theme_base_dir('edge.edx.org'), settings.PROJECT_ROOT), + Theme('edx.org', 'edx.org', get_theme_base_dir('edx.org'), settings.PROJECT_ROOT), + Theme('open-edx', 'open-edx', get_theme_base_dir('open-edx'), settings.PROJECT_ROOT), + Theme('red-theme', 'red-theme', get_theme_base_dir('red-theme'), settings.PROJECT_ROOT), + Theme('stanford-style', 'stanford-style', get_theme_base_dir('stanford-style'), settings.PROJECT_ROOT), + Theme('test-theme', 'test-theme', get_theme_base_dir('test-theme'), settings.PROJECT_ROOT), ] actual_themes = get_themes() self.assertItemsEqual(expected_themes, actual_themes) @@ -39,7 +39,7 @@ class TestHelpers(TestCase): Tests template paths are returned from enabled theme. """ expected_themes = [ - Theme('test-theme', 'test-theme', get_theme_base_dir('test-theme')), + Theme('test-theme', 'test-theme', get_theme_base_dir('test-theme'), settings.PROJECT_ROOT), ] actual_themes = get_themes() self.assertItemsEqual(expected_themes, actual_themes)