From ed81774569fd0f1eb8eea5b46d40c60222ceaf1b Mon Sep 17 00:00:00 2001 From: Keith Grootboom Date: Wed, 23 Mar 2022 14:16:02 +0200 Subject: [PATCH] feat: added new setting CUSTOM_RESOURCE_TEMPLATES_DIRECTORY This setting allows loading of Resource Templates from outside the edx-platform codebase. Operators will be able to add their own custom resource templates without needing to fork the codebase. --- cms/envs/common.py | 10 ++ lms/envs/common.py | 10 ++ xmodule/tests/test_resource_templates.py | 31 +++++- xmodule/x_module.py | 120 +++++++++++++++++------ 4 files changed, 139 insertions(+), 32 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 1dcb068487..7fe7f7ea5c 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2125,6 +2125,16 @@ DEFAULT_SITE_THEME = None # .. toggle_creation_date: 2016-06-30 ENABLE_COMPREHENSIVE_THEMING = False +# .. setting_name: CUSTOM_RESOURCE_TEMPLATES_DIRECTORY +# .. setting_default: None +# .. setting_description: Path to an existing directory of YAML files containing +# html content to be used with the subclasses of xmodule.x_module.ResourceTemplates. +# Default example templates can be found in xmodule/templates/html. +# Note that the extension used is ".yaml" and not ".yml". +# See xmodule.x_module.ResourceTemplates for usage. +# "CUSTOM_RESOURCE_TEMPLATES_DIRECTORY" : null +CUSTOM_RESOURCE_TEMPLATES_DIRECTORY = None + ############################ Global Database Configuration ##################### DATABASE_ROUTERS = [ diff --git a/lms/envs/common.py b/lms/envs/common.py index 4cd68a3f29..7f7baa99b9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4443,6 +4443,16 @@ DEFAULT_SITE_THEME = None # .. toggle_creation_date: 2016-06-30 ENABLE_COMPREHENSIVE_THEMING = False +# .. setting_name: CUSTOM_RESOURCE_TEMPLATES_DIRECTORY +# .. setting_default: None +# .. setting_description: Path to an existing directory of YAML files containing +# html content to be used with the subclasses of xmodule.x_module.ResourceTemplates. +# Default example templates can be found in xmodule/templates/html. +# Note that the extension used is ".yaml" and not ".yml". +# See xmodule.x_module.ResourceTemplates for usage. +# "CUSTOM_RESOURCE_TEMPLATES_DIRECTORY" : null +CUSTOM_RESOURCE_TEMPLATES_DIRECTORY = None + # API access management API_ACCESS_MANAGER_EMAIL = 'api-access@example.com' API_ACCESS_FROM_EMAIL = 'api-requests@example.com' diff --git a/xmodule/tests/test_resource_templates.py b/xmodule/tests/test_resource_templates.py index e51f69ed30..742a7e9da1 100644 --- a/xmodule/tests/test_resource_templates.py +++ b/xmodule/tests/test_resource_templates.py @@ -1,12 +1,14 @@ """ Tests for xmodule.x_module.ResourceTemplates """ - - +import pathlib import unittest +from django.test import override_settings from xmodule.x_module import ResourceTemplates +CUSTOM_RESOURCE_TEMPLATES_DIRECTORY = pathlib.Path(__file__).parent.parent / "templates/" + class ResourceTemplatesTests(unittest.TestCase): """ @@ -28,6 +30,20 @@ class ResourceTemplatesTests(unittest.TestCase): def test_get_template(self): assert TestClass.get_template('latex_html.yaml')['template_id'] == 'latex_html.yaml' + @override_settings(CUSTOM_RESOURCE_TEMPLATES_DIRECTORY=CUSTOM_RESOURCE_TEMPLATES_DIRECTORY) + def test_get_custom_template(self): + assert TestClassResourceTemplate.get_template('latex_html.yaml')['template_id'] == 'latex_html.yaml' + + @override_settings(CUSTOM_RESOURCE_TEMPLATES_DIRECTORY=CUSTOM_RESOURCE_TEMPLATES_DIRECTORY) + def test_custom_templates(self): + expected = { + 'latex_html.yaml', + 'zooming_image.yaml', + 'announcement.yaml', + 'anon_user_id.yaml'} + got = {t['template_id'] for t in TestClassResourceTemplate.templates()} + assert expected == got + class TestClass(ResourceTemplates): """ @@ -55,3 +71,14 @@ class TestClass2(TestClass): @classmethod def get_template_dir(cls): return 'foo' + + +class TestClassResourceTemplate(ResourceTemplates): + """ + Like TestClass, but `template_packages` contains a module that doesn't + have any templates. + + See `TestClass`. + """ + template_packages = ['capa.checker'] + template_dir_name = 'test' diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 1f6e4ea270..61bea727f0 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -9,11 +9,12 @@ from functools import partial import yaml +from django.conf import settings from lazy import lazy from lxml import etree from opaque_keys.edx.asides import AsideDefinitionKeyV2, AsideUsageKeyV2 from opaque_keys.edx.keys import UsageKey -from pkg_resources import resource_exists, resource_isdir, resource_listdir, resource_string +from pkg_resources import resource_isdir, resource_string, resource_filename from web_fragments.fragment import Fragment from webob import Response from webob.multidict import MultiDict @@ -876,11 +877,48 @@ Template = namedtuple("Template", "metadata data children") class ResourceTemplates: """ - Gets the templates associated w/ a containing cls. The cls must have a 'template_dir_name' attribute. - It finds the templates as directly in this directory under 'templates'. + Gets the yaml templates associated with a containing cls for display in the Studio. + + The cls must have a 'template_dir_name' attribute. It finds the templates as directly + in this directory under 'templates'. + + Additional templates can be loaded by setting the + CUSTOM_RESOURCE_TEMPLATES_DIRECTORY configuration setting. + + Note that a template must end with ".yaml" extension otherwise it will not be + loaded. """ template_packages = [__name__] + @classmethod + def _load_template(cls, template_path, template_id): + """ + Reads an loads the yaml content provided in the template_path and + return the content as a dictionary. + """ + if not os.path.exists(template_path): + return None + + with open(template_path) as file_object: + template = yaml.safe_load(file_object) + template['template_id'] = template_id + return template + + @classmethod + def _load_templates_in_dir(cls, dirpath): + """ + Lists every resource template found in the provided dirpath. + """ + templates = [] + for template_file in os.listdir(dirpath): + if not template_file.endswith('.yaml'): + log.warning("Skipping unknown template file %s", template_file) + continue + + template = cls._load_template(os.path.join(dirpath, template_file), template_file) + templates.append(template) + return templates + @classmethod def templates(cls): """ @@ -888,23 +926,15 @@ class ResourceTemplates: to seed a module of this type. Expects a class attribute template_dir_name that defines the directory - inside the 'templates' resource directory to pull templates from + inside the 'templates' resource directory to pull templates from. """ - templates = [] - dirname = cls.get_template_dir() - if dirname is not None: - for pkg in cls.template_packages: - if not resource_isdir(pkg, dirname): - continue - for template_file in resource_listdir(pkg, dirname): - if not template_file.endswith('.yaml'): - log.warning("Skipping unknown template file %s", template_file) - continue - template_content = resource_string(pkg, os.path.join(dirname, template_file)) - template = yaml.safe_load(template_content) - template['template_id'] = template_file - templates.append(template) - return templates + templates = {} + + for dirpath in cls.get_template_dirpaths(): + for template in cls._load_templates_in_dir(dirpath): + templates[template['template_id']] = template + + return list(templates.values()) @classmethod def get_template_dir(cls): # lint-amnesty, pylint: disable=missing-function-docstring @@ -921,22 +951,53 @@ class ResourceTemplates: else: return None + @classmethod + def get_template_dirpaths(cls): + """ + Returns of list of directories containing resource templates. + """ + template_dirpaths = [] + template_dirname = cls.get_template_dir() + if template_dirname and resource_isdir(__name__, template_dirname): + template_dirpaths.append(resource_filename(__name__, template_dirname)) + + custom_template_dir = cls.get_custom_template_dir() + if custom_template_dir: + template_dirpaths.append(custom_template_dir) + return template_dirpaths + + @classmethod + def get_custom_template_dir(cls): + """ + If settings.CUSTOM_RESOURCE_TEMPLATES_DIRECTORY is defined, check if it has a + subdirectory named as the class's template_dir_name and return the full path. + """ + template_dir_name = getattr(cls, 'template_dir_name', None) + + if template_dir_name is None: + return + + resource_dir = settings.CUSTOM_RESOURCE_TEMPLATES_DIRECTORY + + if not resource_dir: + return None + + template_dir_path = os.path.join(resource_dir, template_dir_name) + + if os.path.exists(template_dir_path): + return template_dir_path + return None + @classmethod def get_template(cls, template_id): """ Get a single template by the given id (which is the file name identifying it w/in the class's template_dir_name) - """ - dirname = cls.get_template_dir() - if dirname is not None: - path = os.path.join(dirname, template_id) - for pkg in cls.template_packages: - if resource_exists(pkg, path): - template_content = resource_string(pkg, path) - template = yaml.safe_load(template_content) - template['template_id'] = template_id - return template + for directory in sorted(cls.get_template_dirpaths(), reverse=True): + abs_path = os.path.join(directory, template_id) + if os.path.exists(abs_path): + return cls._load_template(abs_path, template_id) class ConfigurableFragmentWrapper: @@ -1626,7 +1687,6 @@ class ModuleSystemShim: 'runtime.hostname is deprecated. Please use `LMS_BASE` from `django.conf.settings`.', DeprecationWarning, stacklevel=3, ) - from django.conf import settings return settings.LMS_BASE @property