From 47255197e130e0c5aa17f815ffbb7a1ab6b0b4c9 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Fri, 6 Mar 2020 10:47:52 -0500 Subject: [PATCH 1/6] Allow plugins to update contexts in specific views Instead of requiring views like the dashboard to know about plugins so they can include their data in the context, this allows plugins to define a mapping between a view and a function where the function returns a dictionary of new context for the view. Each view would have to purposefully enable this additional context before it could be used. This will allow new content to be added to the pages without updating the core with a combination of a plugin to add new context, and a theme override of that page to use the new context. --- common/djangoapps/student/views/dashboard.py | 9 +++++ openedx/core/djangoapps/plugins/README.rst | 20 ++++++++++- openedx/core/djangoapps/plugins/constants.py | 16 +++++++++ .../djangoapps/plugins/plugin_contexts.py | 35 +++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 openedx/core/djangoapps/plugins/plugin_contexts.py diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 38d4ef1fda..96a57c6934 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -47,6 +47,8 @@ from openedx.core.djangoapps.catalog.utils import ( get_visible_sessions_for_entitlement ) from openedx.core.djangoapps.credit.email_utils import get_credit_provider_attribute_values, make_providers_strings +from openedx.core.djangoapps.plugins.plugin_contexts import get_plugins_view_context +from openedx.core.djangoapps.plugins import constants as plugin_constants from openedx.core.djangoapps.programs.models import ProgramsApiConfig from openedx.core.djangoapps.programs.utils import ProgramDataExtender, ProgramProgressMeter from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -880,6 +882,13 @@ def student_dashboard(request): # TODO START: clean up as part of REVEM-199 (END) } + context_from_plugins = get_plugins_view_context( + plugin_constants.ProjectType.LMS, + plugin_constants.PluginContexts.VIEWS.STUDENT_DASHBOARD, + context + ) + context.update(context_from_plugins) + if ecommerce_service.is_enabled(request.user): context.update({ 'use_ecommerce_payment_flow': True, diff --git a/openedx/core/djangoapps/plugins/README.rst b/openedx/core/djangoapps/plugins/README.rst index 2013200231..dfab6e0d26 100644 --- a/openedx/core/djangoapps/plugins/README.rst +++ b/openedx/core/djangoapps/plugins/README.rst @@ -106,7 +106,7 @@ class:: from django.apps import AppConfig from openedx.core.djangoapps.plugins.constants import ( - ProjectType, SettingsType, PluginURLs, PluginSettings + ProjectType, SettingsType, PluginURLs, PluginSettings, PluginContexts ) class MyAppConfig(AppConfig): name = u'full_python_path.my_app' @@ -184,6 +184,19 @@ class:: PluginSignals.SENDER_PATH: u'full_path_to_sender_app.ModelZ', }], } + }, + + # Configuration setting for Plugin Contexts for this app. + PluginContexts.CONFIG: { + + # Configure the Plugin Signals for each Project Type, as needed. + ProjectType.LMS: { + + # Key is the view that the app wishes to add context to and the value + # is the function within the app that will return additional context + # when called with the original context + PluginContexts.VIEWS.STUDENT_DASHBOARD: u'my_app.context_api.get_dashboard_context' + } } } @@ -217,6 +230,11 @@ OR use string constants when they cannot import from djangoapps.plugins:: u'sender_path': u'full_path_to_sender_app.ModelZ', }], } + }, + u'context_config': { + u'lms.djangoapp': { + 'student_dashboard': u'my_app.context_api.get_dashboard_context' + } } } diff --git a/openedx/core/djangoapps/plugins/constants.py b/openedx/core/djangoapps/plugins/constants.py index 56949535df..8683429ce8 100644 --- a/openedx/core/djangoapps/plugins/constants.py +++ b/openedx/core/djangoapps/plugins/constants.py @@ -76,3 +76,19 @@ class PluginSignals(object): RELATIVE_PATH = u'relative_path' DEFAULT_RELATIVE_PATH = u'signals' + + +class PluginContextsViews(object): + STUDENT_DASHBOARD = u'student_dashboard' + + +class PluginContexts(object): + """ + The PluginContexts enum defines dictionary field names (and defaults) + that can be specified by a Plugin App in order to configure the + additional views it would like to add context into. + """ + CONFIG = u"context_config" + + VIEWS = PluginContextsViews + diff --git a/openedx/core/djangoapps/plugins/plugin_contexts.py b/openedx/core/djangoapps/plugins/plugin_contexts.py new file mode 100644 index 0000000000..abf482c67c --- /dev/null +++ b/openedx/core/djangoapps/plugins/plugin_contexts.py @@ -0,0 +1,35 @@ +from importlib import import_module + +from logging import getLogger +from . import constants, registry + +log = getLogger(__name__) + + +def get_plugins_view_context(project_type, view_name, existing_context={}): + """ + Returns a dict of additonal view context. Will check if any plugin apps + have that view in their context_config, and if so will call their + selected function to get their context dicts. + """ + aggregate_context = {} + + for app_config in registry.get_app_configs(project_type): + context_function_path = _get_context_function(app_config, project_type, view_name) + if context_function_path: + module_path, _, name = context_function_path.rpartition('.') + context_function = getattr(import_module(module_path), name) + plugin_context = context_function(existing_context) + + # NOTE: If two plugins have try to set the same context keys, the last one + # called will overwrite the others. + aggregate_context.update(plugin_context) + + return aggregate_context + + +def _get_context_function(app_config, project_type, view_name): + plugin_config = getattr(app_config, constants.PLUGIN_APP_CLASS_ATTRIBUTE_NAME, {}) + context_config = plugin_config.get(constants.PluginContexts.CONFIG, {}) + project_type_settings = context_config.get(project_type, {}) + return project_type_settings.get(view_name) From e5b8f2877840a944a321b23f37076a928e0deb41 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Mon, 9 Mar 2020 14:23:17 -0400 Subject: [PATCH 2/6] Update with some preferences --- openedx/core/djangoapps/plugins/constants.py | 2 +- .../djangoapps/plugins/plugin_contexts.py | 29 +++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/plugins/constants.py b/openedx/core/djangoapps/plugins/constants.py index 8683429ce8..73139795f7 100644 --- a/openedx/core/djangoapps/plugins/constants.py +++ b/openedx/core/djangoapps/plugins/constants.py @@ -79,7 +79,7 @@ class PluginSignals(object): class PluginContextsViews(object): - STUDENT_DASHBOARD = u'student_dashboard' + COURSE_DASHBOARD = u'course_dashboard' class PluginContexts(object): diff --git a/openedx/core/djangoapps/plugins/plugin_contexts.py b/openedx/core/djangoapps/plugins/plugin_contexts.py index abf482c67c..8e0b46507b 100644 --- a/openedx/core/djangoapps/plugins/plugin_contexts.py +++ b/openedx/core/djangoapps/plugins/plugin_contexts.py @@ -1,10 +1,11 @@ from importlib import import_module from logging import getLogger + from . import constants, registry -log = getLogger(__name__) +log = getLogger(__name__) def get_plugins_view_context(project_type, view_name, existing_context={}): """ @@ -18,11 +19,33 @@ def get_plugins_view_context(project_type, view_name, existing_context={}): context_function_path = _get_context_function(app_config, project_type, view_name) if context_function_path: module_path, _, name = context_function_path.rpartition('.') - context_function = getattr(import_module(module_path), name) - plugin_context = context_function(existing_context) + try: + module = import_module(module_path) + except ImportError, ModuleNotFoundError: + log.exception("Failed to import %s plugin when creating %s context", module_path, view_name) + continue + context_function = getattr(module, name, None) + if context_function: + plugin_context = context_function(existing_context) + else: + log.exception( + "Failed to call %s function from %s plugin when creating %s context", + name, + module_path, + view_name + ) + continue # NOTE: If two plugins have try to set the same context keys, the last one # called will overwrite the others. + for key in plugin_context: + if key in aggregate_context: + log.warning( + "Plugin %s is overwriting the value of %s for view %s", + app_config.__module__, + key, + view_name + ) aggregate_context.update(plugin_context) return aggregate_context From bd9dbdf7de090c10388252b5cdce6f3e7676b0bb Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Wed, 11 Mar 2020 15:17:59 -0400 Subject: [PATCH 3/6] Syntax fix --- openedx/core/djangoapps/plugins/plugin_contexts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/plugins/plugin_contexts.py b/openedx/core/djangoapps/plugins/plugin_contexts.py index 8e0b46507b..707406c369 100644 --- a/openedx/core/djangoapps/plugins/plugin_contexts.py +++ b/openedx/core/djangoapps/plugins/plugin_contexts.py @@ -21,7 +21,7 @@ def get_plugins_view_context(project_type, view_name, existing_context={}): module_path, _, name = context_function_path.rpartition('.') try: module = import_module(module_path) - except ImportError, ModuleNotFoundError: + except (ImportError, ModuleNotFoundError): log.exception("Failed to import %s plugin when creating %s context", module_path, view_name) continue context_function = getattr(module, name, None) From fc3bc032b981230ee61c5572e21fb23b7c892763 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Fri, 13 Mar 2020 15:14:37 -0400 Subject: [PATCH 4/6] Update with suggestions: - Add ADR describing Plugin Contexts - Remove app-specific constants from framework-level code - Add dashboard constants to student app with README --- common/djangoapps/student/README.rst | 5 ++ common/djangoapps/student/api.py | 1 + common/djangoapps/student/views/dashboard.py | 3 +- docs/decisions/0003-plugin-contexts.rst | 72 ++++++++++++++++++++ openedx/core/djangoapps/plugins/constants.py | 8 --- 5 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 docs/decisions/0003-plugin-contexts.rst diff --git a/common/djangoapps/student/README.rst b/common/djangoapps/student/README.rst index e0af6f15f3..c62ff706ca 100644 --- a/common/djangoapps/student/README.rst +++ b/common/djangoapps/student/README.rst @@ -14,3 +14,8 @@ Glossary More Documentation ================== + +Plugins +------- +Plugin Context view names (see ADR 0003-plugin-contexts.rst): +* "course_dashboard" -> student.views.dashboard.student_dashboard diff --git a/common/djangoapps/student/api.py b/common/djangoapps/student/api.py index 2d372ed514..f1c7d486f3 100644 --- a/common/djangoapps/student/api.py +++ b/common/djangoapps/student/api.py @@ -42,6 +42,7 @@ MANUAL_ENROLLMENT_ROLE_CHOICES = configuration_helpers.get_value( settings.MANUAL_ENROLLMENT_ROLE_CHOICES ) +COURSE_DASHBOARD_PLUGIN_VIEW_NAME = "course_dashboard" def create_manual_enrollment_audit( enrolled_by, diff --git a/common/djangoapps/student/views/dashboard.py b/common/djangoapps/student/views/dashboard.py index 96a57c6934..b85782452d 100644 --- a/common/djangoapps/student/views/dashboard.py +++ b/common/djangoapps/student/views/dashboard.py @@ -25,6 +25,7 @@ from opaque_keys.edx.keys import CourseKey from pytz import UTC from shoppingcart.models import CourseRegistrationCode, DonationConfiguration from six import iteritems, text_type +from student.api import COURSE_DASHBOARD_PLUGIN_VIEW_NAME from student.helpers import cert_info, check_verify_status_by_course, get_resume_urls_for_enrollments from student.models import ( AccountRecovery, @@ -884,7 +885,7 @@ def student_dashboard(request): context_from_plugins = get_plugins_view_context( plugin_constants.ProjectType.LMS, - plugin_constants.PluginContexts.VIEWS.STUDENT_DASHBOARD, + COURSE_DASHBOARD_PLUGIN_VIEW_NAME, context ) context.update(context_from_plugins) diff --git a/docs/decisions/0003-plugin-contexts.rst b/docs/decisions/0003-plugin-contexts.rst new file mode 100644 index 0000000000..386a0f1f0a --- /dev/null +++ b/docs/decisions/0003-plugin-contexts.rst @@ -0,0 +1,72 @@ +Plugin Contexts +--------------- + +Status +====== +Draft + +Context +======= +edx-platform contains a plugin system which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, we wasn't a any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered. + +Decisions +========= +We have added the ability to add page context additions to the plugin system. This means that a plugin will be able to add context any view where it is enabled. To support this we have decided: +# Plugins will define a callable function that the LMS and/or studio can import and call, which will return additional context to be added. +# Every page that a plugin wants to add context to, must add a line to add the plugin contexts directly before the render. +# All view + plugin data will exist in the same dictionary. To better protect against dictionary key collisions, it is suggested that you prefix your new context items with your app name (e.g. return {"myapp__some_variable": True} vs {"some_variable": True}) +# Each view will have a constant name that will be defined within it's app's API.py which will be used by plugins. These must be globally unique. These will also be recorded in the rendering app's README.rst file. +# Plugin app's may import the view name from the rendering app's api.py or just use it's string. +# For now, in order to use these new context data items, we must use theming alongside this to keep the new context out of the core. This may be iterated on in the future. + +Implementation +-------------- + +In the plugin app +~~~~~~~~~~~~~~~~~ +Config +++++++ +Inside of your AppConfig your new plugin app, add a "context_config" item like below. +* The format will be {"globally_unique_view_name": "function_inside_plugin_app"} +* The function name & path don't need to be named anything specific, so long as they work +* These functions will be called on **every** render of that view, so keep them efficient or memoize them if they aren't user specific. + +:: + class MyAppConfig(AppConfig): + name = "my_app" + + plugin_app = { + "context_config": { + "lms.djangoapp": { + "course_dashboard": "my_app.context_api.get_dashboard_context" + } + } + } + +Function +++++++++ +The function that will be called by the plugin system should accept a single parameter which will be the previously existing context. It should then return a dictionary which consists of items which will be added to the context + +Example: +:: + def my_context_function(existing_context): + additional_context = {"myapp__some_variable": 10} + if existing_context.get("some_value"): + additional_context.append({"myapp__some_other_variable": True}) + return additional_context + + +In the core (LMS / Studio) +~~~~~~~~~~~~~~~~~~~~~~~~~~ +The view you wish to add context to should have the following pieces enabled: + +* A constant defined inside the app's for the globally unique view name. +* The view must call lines similar to the below right before the render so that the plugin has the full context. +:: + context_from_plugins = get_plugins_view_context( + plugin_constants.ProjectType.LMS, + current_app.api.THIS_VIEW_NAME, + context + ) + context.update(context_from_plugins) + diff --git a/openedx/core/djangoapps/plugins/constants.py b/openedx/core/djangoapps/plugins/constants.py index 73139795f7..3a2b4d434e 100644 --- a/openedx/core/djangoapps/plugins/constants.py +++ b/openedx/core/djangoapps/plugins/constants.py @@ -77,11 +77,6 @@ class PluginSignals(object): RELATIVE_PATH = u'relative_path' DEFAULT_RELATIVE_PATH = u'signals' - -class PluginContextsViews(object): - COURSE_DASHBOARD = u'course_dashboard' - - class PluginContexts(object): """ The PluginContexts enum defines dictionary field names (and defaults) @@ -89,6 +84,3 @@ class PluginContexts(object): additional views it would like to add context into. """ CONFIG = u"context_config" - - VIEWS = PluginContextsViews - From 8a4821862843d2a27a3224469dede0d40369c7f4 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Mon, 16 Mar 2020 17:39:24 -0400 Subject: [PATCH 5/6] Typos, constant names, add context nesting. --- docs/decisions/0003-plugin-contexts.rst | 26 ++++-- openedx/core/djangoapps/plugins/README.rst | 2 +- openedx/core/djangoapps/plugins/constants.py | 2 +- .../djangoapps/plugins/plugin_contexts.py | 89 ++++++++++++------- 4 files changed, 75 insertions(+), 44 deletions(-) diff --git a/docs/decisions/0003-plugin-contexts.rst b/docs/decisions/0003-plugin-contexts.rst index 386a0f1f0a..8f65dd2e26 100644 --- a/docs/decisions/0003-plugin-contexts.rst +++ b/docs/decisions/0003-plugin-contexts.rst @@ -7,16 +7,24 @@ Draft Context ======= -edx-platform contains a plugin system which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, we wasn't a any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered. +edx-platform contains a plugin system which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, there wasn't any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered. Decisions ========= -We have added the ability to add page context additions to the plugin system. This means that a plugin will be able to add context any view where it is enabled. To support this we have decided: +We have added the ability to add page context additions to the plugin system. This means that a plugin will be able to add context to any view where it is enabled. To support this we have decided: # Plugins will define a callable function that the LMS and/or studio can import and call, which will return additional context to be added. # Every page that a plugin wants to add context to, must add a line to add the plugin contexts directly before the render. -# All view + plugin data will exist in the same dictionary. To better protect against dictionary key collisions, it is suggested that you prefix your new context items with your app name (e.g. return {"myapp__some_variable": True} vs {"some_variable": True}) +# Plugin context will live in a dictionary called "plugins" that will be passed into the context the templates receive. The structure will look like: +:: + { + ..existing context values + "plugins": { + "my_new_plugin": {... my_new_plugins's values ...}, + "my_other_plugin": {... my_other_plugin's values ...}, + } + } # Each view will have a constant name that will be defined within it's app's API.py which will be used by plugins. These must be globally unique. These will also be recorded in the rendering app's README.rst file. -# Plugin app's may import the view name from the rendering app's api.py or just use it's string. +# Plugin apps have the option to either use the view name strings directly or import the constants from the rendering app's api.py if the plugin is part of the edx-platform repo. # For now, in order to use these new context data items, we must use theming alongside this to keep the new context out of the core. This may be iterated on in the future. Implementation @@ -26,7 +34,7 @@ In the plugin app ~~~~~~~~~~~~~~~~~ Config ++++++ -Inside of your AppConfig your new plugin app, add a "context_config" item like below. +Inside of your AppConfig your new plugin app, add a "view_context_config" item like below. * The format will be {"globally_unique_view_name": "function_inside_plugin_app"} * The function name & path don't need to be named anything specific, so long as they work * These functions will be called on **every** render of that view, so keep them efficient or memoize them if they aren't user specific. @@ -36,7 +44,7 @@ Inside of your AppConfig your new plugin app, add a "context_config" item like b name = "my_app" plugin_app = { - "context_config": { + "view_context_config": { "lms.djangoapp": { "course_dashboard": "my_app.context_api.get_dashboard_context" } @@ -50,9 +58,9 @@ The function that will be called by the plugin system should accept a single par Example: :: def my_context_function(existing_context): - additional_context = {"myapp__some_variable": 10} - if existing_context.get("some_value"): - additional_context.append({"myapp__some_other_variable": True}) + additional_context = {"some_plugin_value": 10} + if existing_context.get("some_core_value"): + additional_context.append({"some_other_plugin_value": True}) return additional_context diff --git a/openedx/core/djangoapps/plugins/README.rst b/openedx/core/djangoapps/plugins/README.rst index dfab6e0d26..cd0bd69337 100644 --- a/openedx/core/djangoapps/plugins/README.rst +++ b/openedx/core/djangoapps/plugins/README.rst @@ -231,7 +231,7 @@ OR use string constants when they cannot import from djangoapps.plugins:: }], } }, - u'context_config': { + u'view_context_config': { u'lms.djangoapp': { 'student_dashboard': u'my_app.context_api.get_dashboard_context' } diff --git a/openedx/core/djangoapps/plugins/constants.py b/openedx/core/djangoapps/plugins/constants.py index 3a2b4d434e..2ebb30eb71 100644 --- a/openedx/core/djangoapps/plugins/constants.py +++ b/openedx/core/djangoapps/plugins/constants.py @@ -83,4 +83,4 @@ class PluginContexts(object): that can be specified by a Plugin App in order to configure the additional views it would like to add context into. """ - CONFIG = u"context_config" + CONFIG = u"view_context_config" diff --git a/openedx/core/djangoapps/plugins/plugin_contexts.py b/openedx/core/djangoapps/plugins/plugin_contexts.py index 707406c369..9f966ea880 100644 --- a/openedx/core/djangoapps/plugins/plugin_contexts.py +++ b/openedx/core/djangoapps/plugins/plugin_contexts.py @@ -2,6 +2,8 @@ from importlib import import_module from logging import getLogger +from openedx.core.lib.cache_utils import process_cached + from . import constants, registry @@ -10,43 +12,27 @@ log = getLogger(__name__) def get_plugins_view_context(project_type, view_name, existing_context={}): """ Returns a dict of additonal view context. Will check if any plugin apps - have that view in their context_config, and if so will call their + have that view in their view_context_config, and if so will call their selected function to get their context dicts. """ - aggregate_context = {} + aggregate_context = {"plugins": {}} - for app_config in registry.get_app_configs(project_type): - context_function_path = _get_context_function(app_config, project_type, view_name) - if context_function_path: - module_path, _, name = context_function_path.rpartition('.') - try: - module = import_module(module_path) - except (ImportError, ModuleNotFoundError): - log.exception("Failed to import %s plugin when creating %s context", module_path, view_name) - continue - context_function = getattr(module, name, None) - if context_function: - plugin_context = context_function(existing_context) - else: - log.exception( - "Failed to call %s function from %s plugin when creating %s context", - name, - module_path, - view_name - ) - continue + # This functionality is cached + context_functions = _get_context_functions_for_view(project_type, view_name) - # NOTE: If two plugins have try to set the same context keys, the last one - # called will overwrite the others. - for key in plugin_context: - if key in aggregate_context: - log.warning( - "Plugin %s is overwriting the value of %s for view %s", - app_config.__module__, - key, - view_name - ) - aggregate_context.update(plugin_context) + for (context_function, plugin_name) in context_functions: + plugin_context = context_function(existing_context) + try: + plugin_context = context_function(existing_context) + except Exception as exc: + # We're catching this because we don't want the core to blow up when a + # plugin is broken. This exception will probably need some sort of + # monitoring hooked up to it to make sure that these errors don't go + # unseen. + log.exception("Failed to call plugin context function. Error: %s", exc) + continue + + aggregate_context["plugins"][plugin_name] = plugin_context return aggregate_context @@ -56,3 +42,40 @@ def _get_context_function(app_config, project_type, view_name): context_config = plugin_config.get(constants.PluginContexts.CONFIG, {}) project_type_settings = context_config.get(project_type, {}) return project_type_settings.get(view_name) + +@process_cached +def _get_context_functions_for_view(project_type, view_name): + """ + Returns a list of tuples where the first item is the context function + and the second item is the name of the plugin it's being called from. + + NOTE: These will be functions will be cached (in RAM not memcache) on this unique + combination. If we enable many new views to use this system, we may notice an + increase in memory usage as the entirety of these functions will be held in memory. + """ + context_functions = [] + for app_config in registry.get_app_configs(project_type): + context_function_path = _get_context_function(app_config, project_type, view_name) + if context_function_path: + module_path, _, name = context_function_path.rpartition('.') + try: + module = import_module(module_path) + except (ImportError, ModuleNotFoundError): + log.exception( + "Failed to import %s plugin when creating %s context", + module_path, + view_name + ) + continue + context_function = getattr(module, name, None) + if context_function: + plugin_name, _, _ = module_path.partition('.') + context_functions.append((context_function, plugin_name)) + else: + log.warning( + "Failed to retrieve %s function from %s plugin when creating %s context", + name, + module_path, + view_name + ) + return context_functions From 0a8b06a122a2b81e6fe85284ac68f005da335fa4 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Tue, 17 Mar 2020 22:19:22 -0400 Subject: [PATCH 6/6] Lints and final tweaks --- common/djangoapps/student/api.py | 1 + openedx/core/djangoapps/plugins/README.rst | 4 +- openedx/core/djangoapps/plugins/constants.py | 1 + .../docs}/decisions/0003-plugin-contexts.rst | 6 +-- .../djangoapps/plugins/plugin_contexts.py | 38 ++++++++++++------- 5 files changed, 32 insertions(+), 18 deletions(-) rename {docs => openedx/core/djangoapps/plugins/docs}/decisions/0003-plugin-contexts.rst (80%) diff --git a/common/djangoapps/student/api.py b/common/djangoapps/student/api.py index f1c7d486f3..318f5a9143 100644 --- a/common/djangoapps/student/api.py +++ b/common/djangoapps/student/api.py @@ -44,6 +44,7 @@ MANUAL_ENROLLMENT_ROLE_CHOICES = configuration_helpers.get_value( COURSE_DASHBOARD_PLUGIN_VIEW_NAME = "course_dashboard" + def create_manual_enrollment_audit( enrolled_by, user_email, diff --git a/openedx/core/djangoapps/plugins/README.rst b/openedx/core/djangoapps/plugins/README.rst index cd0bd69337..c2b76d4be9 100644 --- a/openedx/core/djangoapps/plugins/README.rst +++ b/openedx/core/djangoapps/plugins/README.rst @@ -195,7 +195,7 @@ class:: # Key is the view that the app wishes to add context to and the value # is the function within the app that will return additional context # when called with the original context - PluginContexts.VIEWS.STUDENT_DASHBOARD: u'my_app.context_api.get_dashboard_context' + u'course_dashboard': u'my_app.context_api.get_dashboard_context' } } } @@ -233,7 +233,7 @@ OR use string constants when they cannot import from djangoapps.plugins:: }, u'view_context_config': { u'lms.djangoapp': { - 'student_dashboard': u'my_app.context_api.get_dashboard_context' + 'course_dashboard': u'my_app.context_api.get_dashboard_context' } } } diff --git a/openedx/core/djangoapps/plugins/constants.py b/openedx/core/djangoapps/plugins/constants.py index 2ebb30eb71..a887162c3a 100644 --- a/openedx/core/djangoapps/plugins/constants.py +++ b/openedx/core/djangoapps/plugins/constants.py @@ -77,6 +77,7 @@ class PluginSignals(object): RELATIVE_PATH = u'relative_path' DEFAULT_RELATIVE_PATH = u'signals' + class PluginContexts(object): """ The PluginContexts enum defines dictionary field names (and defaults) diff --git a/docs/decisions/0003-plugin-contexts.rst b/openedx/core/djangoapps/plugins/docs/decisions/0003-plugin-contexts.rst similarity index 80% rename from docs/decisions/0003-plugin-contexts.rst rename to openedx/core/djangoapps/plugins/docs/decisions/0003-plugin-contexts.rst index 8f65dd2e26..7967f371b0 100644 --- a/docs/decisions/0003-plugin-contexts.rst +++ b/openedx/core/djangoapps/plugins/docs/decisions/0003-plugin-contexts.rst @@ -7,7 +7,7 @@ Draft Context ======= -edx-platform contains a plugin system which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, there wasn't any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered. +edx-platform contains a plugin system (https://github.com/edx/edx-platform/tree/master/openedx/core/djangoapps/plugins) which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, there wasn't any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered. Decisions ========= @@ -34,7 +34,7 @@ In the plugin app ~~~~~~~~~~~~~~~~~ Config ++++++ -Inside of your AppConfig your new plugin app, add a "view_context_config" item like below. +Inside of the AppConfig of your new plugin app, add a "view_context_config" item like below. * The format will be {"globally_unique_view_name": "function_inside_plugin_app"} * The function name & path don't need to be named anything specific, so long as they work * These functions will be called on **every** render of that view, so keep them efficient or memoize them if they aren't user specific. @@ -57,7 +57,7 @@ The function that will be called by the plugin system should accept a single par Example: :: - def my_context_function(existing_context): + def my_context_function(existing_context, *args, **kwargs): additional_context = {"some_plugin_value": 10} if existing_context.get("some_core_value"): additional_context.append({"some_other_plugin_value": True}) diff --git a/openedx/core/djangoapps/plugins/plugin_contexts.py b/openedx/core/djangoapps/plugins/plugin_contexts.py index 9f966ea880..8152ac96d4 100644 --- a/openedx/core/djangoapps/plugins/plugin_contexts.py +++ b/openedx/core/djangoapps/plugins/plugin_contexts.py @@ -9,19 +9,30 @@ from . import constants, registry log = getLogger(__name__) -def get_plugins_view_context(project_type, view_name, existing_context={}): + +def get_plugins_view_context(project_type, view_name, existing_context=None): """ - Returns a dict of additonal view context. Will check if any plugin apps + Returns a dict of additional view context. Will check if any plugin apps have that view in their view_context_config, and if so will call their selected function to get their context dicts. + + Params: + project_type: a string that determines which project (lms or studio) the view is being called in. See the + ProjectType enum in plugins/constants.py for valid options + view_name: a string that determines which view needs the additional context. These are globally unique and + noted in the api.py in the view's app. + existing_context: a dictionary which includes all of the data that the page was going to render with prior + to the addition of each plugin's context. This is what will be passed to plugins so they may choose + what data to add to the view. """ aggregate_context = {"plugins": {}} - # This functionality is cached - context_functions = _get_context_functions_for_view(project_type, view_name) + if existing_context is None: + existing_context = {} + + context_functions = _get_cached_context_functions_for_view(project_type, view_name) for (context_function, plugin_name) in context_functions: - plugin_context = context_function(existing_context) try: plugin_context = context_function(existing_context) except Exception as exc: @@ -37,14 +48,8 @@ def get_plugins_view_context(project_type, view_name, existing_context={}): return aggregate_context -def _get_context_function(app_config, project_type, view_name): - plugin_config = getattr(app_config, constants.PLUGIN_APP_CLASS_ATTRIBUTE_NAME, {}) - context_config = plugin_config.get(constants.PluginContexts.CONFIG, {}) - project_type_settings = context_config.get(project_type, {}) - return project_type_settings.get(view_name) - @process_cached -def _get_context_functions_for_view(project_type, view_name): +def _get_cached_context_functions_for_view(project_type, view_name): """ Returns a list of tuples where the first item is the context function and the second item is the name of the plugin it's being called from. @@ -55,7 +60,7 @@ def _get_context_functions_for_view(project_type, view_name): """ context_functions = [] for app_config in registry.get_app_configs(project_type): - context_function_path = _get_context_function(app_config, project_type, view_name) + context_function_path = _get_context_function_path(app_config, project_type, view_name) if context_function_path: module_path, _, name = context_function_path.rpartition('.') try: @@ -79,3 +84,10 @@ def _get_context_functions_for_view(project_type, view_name): view_name ) return context_functions + + +def _get_context_function_path(app_config, project_type, view_name): + plugin_config = getattr(app_config, constants.PLUGIN_APP_CLASS_ATTRIBUTE_NAME, {}) + context_config = plugin_config.get(constants.PluginContexts.CONFIG, {}) + project_type_settings = context_config.get(project_type, {}) + return project_type_settings.get(view_name)