From 811eb5848c7fc88ac7482d367fa5014958ad77b7 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 13:38:43 -0500 Subject: [PATCH 1/9] feat: allow to embed superset dashboard in instructor dashboard --- aspects/__init__.py | 4 + aspects/apps.py | 21 +++++ aspects/extensions/__init__.py | 0 aspects/extensions/filters.py | 70 +++++++++++++++ aspects/settings/__init__.py | 0 aspects/settings/common.py | 49 +++++++++++ aspects/settings/production.py | 19 ++++ aspects/settings/test.py | 42 +++++++++ aspects/static/css/superset.css | 5 ++ aspects/static/html/superset.html | 25 ++++++ aspects/static/js/embed_dashboard.js | 30 +++++++ aspects/static/js/superset.js | 36 ++++++++ aspects/templates/aspects/base.html | 26 ------ aspects/utils.py | 124 +++++++++++++++++++++++++++ requirements/base.in | 5 ++ requirements/base.txt | 28 ++++++ requirements/dev.txt | 40 +++++++++ requirements/doc.txt | 35 +++++++- requirements/quality.txt | 40 +++++++++ requirements/test.txt | 44 +++++++++- 20 files changed, 613 insertions(+), 30 deletions(-) create mode 100644 aspects/extensions/__init__.py create mode 100644 aspects/extensions/filters.py create mode 100644 aspects/settings/__init__.py create mode 100644 aspects/settings/common.py create mode 100644 aspects/settings/production.py create mode 100644 aspects/settings/test.py create mode 100644 aspects/static/css/superset.css create mode 100644 aspects/static/html/superset.html create mode 100644 aspects/static/js/embed_dashboard.js create mode 100644 aspects/static/js/superset.js delete mode 100644 aspects/templates/aspects/base.html create mode 100644 aspects/utils.py diff --git a/aspects/__init__.py b/aspects/__init__.py index 1ba0e25..06ceb10 100644 --- a/aspects/__init__.py +++ b/aspects/__init__.py @@ -1,5 +1,9 @@ """ One-line description for README and other doc files. """ +import os +from pathlib import Path __version__ = '0.1.0' + +ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__))) diff --git a/aspects/apps.py b/aspects/apps.py index 051e25b..ac21f85 100644 --- a/aspects/apps.py +++ b/aspects/apps.py @@ -11,3 +11,24 @@ class AspectsConfig(AppConfig): """ name = 'aspects' + + plugin_app = { + "settings_config": { + "lms.djangoapp": { + "common": {"relative_path": "settings.common"}, + "test": {"relative_path": "settings.test"}, + "production": {"relative_path": "settings.production"}, + }, + "cms.djangoapp": { + "common": {"relative_path": "settings.common"}, + "test": {"relative_path": "settings.test"}, + "production": {"relative_path": "settings.production"}, + }, + }, + } + + def ready(self): + """Load modules of Aspects.""" + from aspects.extensions import ( # pylint: disable=unused-import, import-outside-toplevel + filters, + ) diff --git a/aspects/extensions/__init__.py b/aspects/extensions/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/aspects/extensions/filters.py b/aspects/extensions/filters.py new file mode 100644 index 0000000..d0a422c --- /dev/null +++ b/aspects/extensions/filters.py @@ -0,0 +1,70 @@ +""" +Open edX Filters needed for Aspects integration. +""" +import pkg_resources +from django.conf import settings +from django.template import Context, Template +from openedx_filters import PipelineStep +from web_fragments.fragment import Fragment + +from aspects.utils import update_context + +TEMPLATE_ABSOLUTE_PATH = "/instructor_dashboard/" +BLOCK_CATEGORY = "superset" + + +class AddSupersetTab(PipelineStep): + """Add superset tab to instructor dashboard.""" + + def run_filter( + self, context, template_name + ): # pylint: disable=arguments-differ, unused-argument + """Execute filter that modifies the instructor dashboard context. + Args: + context (dict): the context for the instructor dashboard. + _ (str): instructor dashboard template name. + """ + course = context["course"] + + instructor_dashboard_config = getattr( + settings, "SUPERSET_INSTRUCTOR_DASHBOARD", {} + ) + dashboard_uuid = instructor_dashboard_config.get("dashboard_uuid") + + extra_filters_format = getattr(settings, "SUPERSET_EXTRA_FILTERS_FORMAT", []) + + default_filters = [ + "org = '{course.org}'", + "course_name = '{course.display_name}'", + "course_run = '{course.id.run}'", + ] + + filters = default_filters + extra_filters_format + + context = update_context( + context, dashboard_uuid=dashboard_uuid, filters=filters + ) + template = Template(self.resource_string("static/html/superset.html")) + + html = template.render(Context(context)) + frag = Fragment(html) + + frag.add_css(self.resource_string("static/css/superset.css")) + frag.add_javascript(self.resource_string("static/js/embed_dashboard.js")) + + section_data = { + "fragment": frag, + "section_key": BLOCK_CATEGORY, + "section_display_name": "Aspects", + "course_id": str(course.id), + "template_path_prefix": TEMPLATE_ABSOLUTE_PATH, + } + context["sections"].append(section_data) + return { + "context": context, + } + + def resource_string(self, path): + """Handy helper for getting resources from our kit.""" + data = pkg_resources.resource_string("aspects", path) + return data.decode("utf8") diff --git a/aspects/settings/__init__.py b/aspects/settings/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/aspects/settings/common.py b/aspects/settings/common.py new file mode 100644 index 0000000..20b0ea1 --- /dev/null +++ b/aspects/settings/common.py @@ -0,0 +1,49 @@ +""" +Common Django settings for eox_hooks project. +For more information on this file, see +https://docs.djangoproject.com/en/2.22/topics/settings/ +For the full list of settings and their values, see +https://docs.djangoproject.com/en/2.22/ref/settings/ +""" +from aspects import ROOT_DIRECTORY + +# Quick-start development settings - unsuitable for production +# See https://docs.djangoproject.com/en/2.22/howto/deployment/checklist/ + +# SECURITY WARNING: keep the secret key used in production secret! +SECRET_KEY = "secret-key" + + +# Application definition + +INSTALLED_APPS = [ + "aspects", +] + + +# Internationalization +# https://docs.djangoproject.com/en/2.22/topics/i18n/ + +LANGUAGE_CODE = "en-us" + +TIME_ZONE = "UTC" + +USE_TZ = True + + +def plugin_settings(settings): + """ + Set of plugin settings used by the Open Edx platform. + More info: https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/plugins/README.rst + """ + settings.MAKO_TEMPLATE_DIRS_BASE.append(ROOT_DIRECTORY / "templates") + settings.SUPERSET_CONFIG = { + "url": "http://superset.local.overhang.io:8088", + "username": "superset", + "password": "superset", + } + settings.SUPERSET_INSTRUCTOR_DASHBOARD = { + "dashboard_slug": "instructor-dashboard", + "dashboard_uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", + } + settings.SUPERSET_EXTRA_FILTERS_FORMAT = [] diff --git a/aspects/settings/production.py b/aspects/settings/production.py new file mode 100644 index 0000000..7fa33a6 --- /dev/null +++ b/aspects/settings/production.py @@ -0,0 +1,19 @@ +""" +Production Django settings for Aspects project. +""" + + +def plugin_settings(settings): + """ + Set of plugin settings used by the Open Edx platform. + More info: https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/plugins/README.rst + """ + settings.SUPERSET_CONFIG = getattr(settings, "ENV_TOKENS", {}).get( + "SUPERSET_CONFIG", settings.SUPERSET_CONFIG + ) + settings.SUPERSET_INSTRUCTOR_DASHBOARD = getattr(settings, "ENV_TOKENS", {}).get( + "SUPERSET_INSTRUCTOR_DASHBOARD", settings.SUPERSET_INSTRUCTOR_DASHBOARD + ) + settings.SUPERSET_EXTRA_FILTERS_FORMAT = getattr(settings, "ENV_TOKENS", {}).get( + "SUPERSET_EXTRA_FILTERS_FORMAT", settings.SUPERSET_EXTRA_FILTERS_FORMAT + ) diff --git a/aspects/settings/test.py b/aspects/settings/test.py new file mode 100644 index 0000000..87c9866 --- /dev/null +++ b/aspects/settings/test.py @@ -0,0 +1,42 @@ +""" +Common Django settings for eox_hooks project. +For more information on this file, see +https://docs.djangoproject.com/en/2.22/topics/settings/ +For the full list of settings and their values, see +https://docs.djangoproject.com/en/2.22/ref/settings/ +""" + + +# Quick-start development settings - unsuitable for production +# See https://docs.djangoproject.com/en/2.22/howto/deployment/checklist/ + +# SECURITY WARNING: keep the secret key used in production secret! +DATABASES = { + "default": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": "default.db", + "USER": "", + "PASSWORD": "", + "HOST": "", + "PORT": "", + }, + "read_replica": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": "read_replica.db", + "USER": "", + "PASSWORD": "", + "HOST": "", + "PORT": "", + }, +} + +SECRET_KEY = "not-so-secret-key" + +# Internationalization +# https://docs.djangoproject.com/en/2.22/topics/i18n/ + +LANGUAGE_CODE = "en-us" + +TIME_ZONE = "UTC" + +USE_TZ = True diff --git a/aspects/static/css/superset.css b/aspects/static/css/superset.css new file mode 100644 index 0000000..e549ea4 --- /dev/null +++ b/aspects/static/css/superset.css @@ -0,0 +1,5 @@ +.superset-embedded-container > iframe { + height: 720px; + width: 100%; + display: block; +} diff --git a/aspects/static/html/superset.html b/aspects/static/html/superset.html new file mode 100644 index 0000000..6b4837f --- /dev/null +++ b/aspects/static/html/superset.html @@ -0,0 +1,25 @@ +{% load i18n %} + + + +
+

{{display_name}}

+ + {% if exception %} +

{% trans 'Superset is not configured properly. Please contact your system administrator.'%}

+

+ {{exception}} +

+ {% elif not dashboard_uuid %} +

+ Dashboard UUID is not set. Please set the dashboard UUID in the Studio. {{dashboard_uuid}} +

+ {% elif superset_url and superset_token %} +
+ + {% endif %} +
diff --git a/aspects/static/js/embed_dashboard.js b/aspects/static/js/embed_dashboard.js new file mode 100644 index 0000000..08c42f7 --- /dev/null +++ b/aspects/static/js/embed_dashboard.js @@ -0,0 +1,30 @@ +function embedDashboard(dashboard_uuid, superset_url, superset_token, xblock_id) { + xblock_id = xblock_id || ""; + window.supersetEmbeddedSdk + .embedDashboard({ + id: dashboard_uuid, // given by the Superset embedding UI + supersetDomain: superset_url, // your Superset instance + mountPoint: document.getElementById(`superset-embedded-container-${xblock_id}`), // any html element that can contain an iframe + fetchGuestToken: () => superset_token, // function that returns a Promise with the guest token + dashboardUiConfig: { + // dashboard UI config: hideTitle, hideTab, hideChartControls, filters.visible, filters.expanded (optional) + hideTitle: true, + filters: { + expanded: false, + }, + hideTab: true, + hideChartControls: false, + hideFilters: true, + }, + }) + .then((dashboard) => { + mountPoint = document.getElementById("superset-embedded-container"); + /* + Perform extra operations on the dashboard object or the container + when the dashboard is loaded + */ + }); +} +if (window.dashboard_uuid !== undefined) { + embedDashboard(window.dashboard_uuid, window.superset_url, window.superset_token, window.xblock_id); +} diff --git a/aspects/static/js/superset.js b/aspects/static/js/superset.js new file mode 100644 index 0000000..14ae644 --- /dev/null +++ b/aspects/static/js/superset.js @@ -0,0 +1,36 @@ +/* Javascript for SupersetXBlock. */ +function SupersetXBlock(runtime, element, context) { + const dashboard_uuid = context.dashboard_uuid; + const superset_url = context.superset_url; + const superset_token = context.superset_token; + const xblock_id = context.xblock_id + + function initSuperset(supersetEmbeddedSdk) { + embedDashboard(dashboard_uuid, superset_url, superset_token, xblock_id); + } + + if (typeof require === "function") { + require(["supersetEmbeddedSdk"], function (supersetEmbeddedSdk) { + window.supersetEmbeddedSdk = supersetEmbeddedSdk; + initSuperset(); + }); + } else { + loadJS(function () { + initSuperset(); + }); + } +} + +function loadJS(callback) { + if (window.supersetEmbeddedSdk) { + callback(); + } else { + $.getScript("https://cdn.jsdelivr.net/npm/@superset-ui/embedded-sdk@0.1.0-alpha.10/bundle/index.min.js") + .done(function () { + callback(); + }) + .fail(function () { + console.error("Error loading supersetEmbeddedSdk."); + }); + } +} diff --git a/aspects/templates/aspects/base.html b/aspects/templates/aspects/base.html deleted file mode 100644 index daf311a..0000000 --- a/aspects/templates/aspects/base.html +++ /dev/null @@ -1,26 +0,0 @@ - - -{% load i18n %} -{% trans "Dummy text to generate a translation (.po) source file. It is safe to delete this line. It is also safe to delete (load i18n) above if there are no other (trans) tags in the file" %} - -{% comment %} -As the developer of this package, don't place anything here if you can help it -since this allows developers to have interoperability between your template -structure and their own. - -Example: Developer melding the 2SoD pattern to fit inside with another pattern:: - - {% extends "base.html" %} - {% load static %} - - - {% block extra_js %} - - - {% block javascript %} - - {% endblock javascript %} - - {% endblock extra_js %} -{% endcomment %} - diff --git a/aspects/utils.py b/aspects/utils.py new file mode 100644 index 0000000..12d2b12 --- /dev/null +++ b/aspects/utils.py @@ -0,0 +1,124 @@ +""" +Utilities for the Aspects app. +""" + +import logging +import os + +from crum import get_current_user +from django.conf import settings +from supersetapiclient.client import SupersetClient + +if settings.DEBUG: + os.environ["OAUTHLIB_INSECURE_TRANSPORT"] = "1" + + +logger = logging.getLogger(__name__) + + +def update_context( # pylint: disable=dangerous-default-value + context, + superset_config={}, + dashboard_uuid="", + filters=[], + user=None, +): + """ + Update context with superset token and dashboard id. + + Args: + context (dict): the context for the instructor dashboard. It must include a course object + superset_config (dict): superset config. + dashboard_uuid (str): superset dashboard uuid. + filters (list): list of filters to apply to the dashboard. + user (User): user object. + """ + course = context["course"] + + if user is None: + user = get_current_user() + superset_token, dashboard_uuid = generate_guest_token( + user=user, + course=course, + superset_config=superset_config, + dashboard_uuid=dashboard_uuid, + filters=filters, + ) + + if superset_token: + context.update( + { + "superset_token": superset_token, + "dashboard_uuid": dashboard_uuid, + "superset_url": settings.SUPERSET_CONFIG.get("host"), + } + ) + else: + context.update( + { + "exception": dashboard_uuid, + } + ) + + return context + + +def generate_guest_token(user, course, superset_config, dashboard_uuid, filters): + """ + Generate a Superset guest token for the user. + + Args: + user: User object. + course: Course object. + + Returns: + tuple: Superset guest token and dashboard id. + or None, exception if Superset is missconfigured or cannot generate guest token. + """ + if not superset_config: + superset_config = getattr(settings, "SUPERSET_CONFIG", {}) + + superset_internal_host = superset_config.get("service_url", "http://superset:8088/") + superset_username = superset_config.get("username") + superset_password = superset_config.get("password") + + try: + client = SupersetClient( + host=superset_internal_host, + username=superset_username, + password=superset_password, + ) + except Exception as exc: # pylint: disable=broad-except + logger.error(exc) + return None, exc + + formatted_filters = [filter.format(course=course, user=user) for filter in filters] + + data = { + "user": { + "username": user.username, + # We can send more info about the user to superset + # but Open edX only provides the full name. For now is not needed + # and doesn't add any value so we don't send it. + # { + # "first_name": "John", + # "last_name": "Doe", + # } + }, + "resources": [{"type": "dashboard", "id": dashboard_uuid}], + "rls": [{"clause": filter} for filter in formatted_filters], + } + + try: + response = client.session.post( + url=f"{superset_internal_host}api/v1/security/guest_token/", + json=data, + headers={"Content-Type": "application/json"}, + ) + response.raise_for_status() + token = response.json()["token"] + + return token, dashboard_uuid + except Exception as exc: # pylint: disable=broad-except + logger.error(exc) + return None, exc diff --git a/requirements/base.in b/requirements/base.in index 31859da..da5221d 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -2,3 +2,8 @@ -c constraints.txt Django # Web application framework +openedx-filters +web_fragments +superset-api-client +web_fragments +django_crum diff --git a/requirements/base.txt b/requirements/base.txt index 7a694e7..691da2d 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,13 +6,41 @@ # asgiref==3.7.2 # via django +certifi==2024.2.2 + # via requests +charset-normalizer==3.3.2 + # via requests django==3.2.24 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.in + # django-crum + # openedx-filters +django-crum==0.7.9 + # via -r requirements/base.in +idna==3.6 + # via requests +oauthlib==3.2.2 + # via requests-oauthlib +openedx-filters==1.6.0 + # via -r requirements/base.in pytz==2024.1 # via django +pyyaml==6.0.1 + # via superset-api-client +requests==2.31.0 + # via + # requests-oauthlib + # superset-api-client +requests-oauthlib==1.3.1 + # via superset-api-client sqlparse==0.4.4 # via django +superset-api-client==0.6.0 + # via -r requirements/base.in typing-extensions==4.9.0 # via asgiref +urllib3==2.2.1 + # via requests +web-fragments==2.1.0 + # via -r requirements/base.in diff --git a/requirements/dev.txt b/requirements/dev.txt index 022fc3c..96c7bbf 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -21,11 +21,19 @@ cachetools==5.3.2 # via # -r requirements/ci.txt # tox +certifi==2024.2.2 + # via + # -r requirements/quality.txt + # requests chardet==5.2.0 # via # -r requirements/ci.txt # diff-cover # tox +charset-normalizer==3.3.2 + # via + # -r requirements/quality.txt + # requests click==8.1.7 # via # -r requirements/pip-tools.txt @@ -64,7 +72,11 @@ django==3.2.24 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/quality.txt + # django-crum # edx-i18n-tools + # openedx-filters +django-crum==0.7.9 + # via -r requirements/quality.txt edx-i18n-tools==1.3.0 # via -r requirements/dev.in edx-lint==5.3.6 @@ -78,6 +90,10 @@ filelock==3.13.1 # -r requirements/ci.txt # tox # virtualenv +idna==3.6 + # via + # -r requirements/quality.txt + # requests importlib-metadata==7.0.1 # via # -r requirements/pip-tools.txt @@ -105,6 +121,12 @@ mccabe==0.7.0 # via # -r requirements/quality.txt # pylint +oauthlib==3.2.2 + # via + # -r requirements/quality.txt + # requests-oauthlib +openedx-filters==1.6.0 + # via -r requirements/quality.txt packaging==23.2 # via # -r requirements/ci.txt @@ -195,6 +217,16 @@ pyyaml==6.0.1 # -r requirements/quality.txt # code-annotations # edx-i18n-tools + # superset-api-client +requests==2.31.0 + # via + # -r requirements/quality.txt + # requests-oauthlib + # superset-api-client +requests-oauthlib==1.3.1 + # via + # -r requirements/quality.txt + # superset-api-client six==1.16.0 # via # -r requirements/quality.txt @@ -211,6 +243,8 @@ stevedore==5.1.0 # via # -r requirements/quality.txt # code-annotations +superset-api-client==0.6.0 + # via -r requirements/quality.txt text-unidecode==1.3 # via # -r requirements/quality.txt @@ -240,10 +274,16 @@ typing-extensions==4.9.0 # asgiref # astroid # pylint +urllib3==2.2.1 + # via + # -r requirements/quality.txt + # requests virtualenv==20.25.0 # via # -r requirements/ci.txt # tox +web-fragments==2.1.0 + # via -r requirements/quality.txt wheel==0.42.0 # via # -r requirements/pip-tools.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 34dc482..ccf30a9 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -21,11 +21,15 @@ beautifulsoup4==4.12.3 build==1.0.3 # via -r requirements/doc.in certifi==2024.2.2 - # via requests + # via + # -r requirements/test.txt + # requests cffi==1.16.0 # via cryptography charset-normalizer==3.3.2 - # via requests + # via + # -r requirements/test.txt + # requests click==8.1.7 # via # -r requirements/test.txt @@ -44,6 +48,10 @@ django==3.2.24 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-crum + # openedx-filters +django-crum==0.7.9 + # via -r requirements/test.txt doc8==1.1.1 # via -r requirements/doc.in docutils==0.19 @@ -59,7 +67,9 @@ exceptiongroup==1.2.0 # -r requirements/test.txt # pytest idna==3.6 - # via requests + # via + # -r requirements/test.txt + # requests imagesize==1.4.1 # via sphinx importlib-metadata==7.0.1 @@ -101,6 +111,12 @@ more-itertools==10.2.0 # via jaraco-classes nh3==0.2.15 # via readme-renderer +oauthlib==3.2.2 + # via + # -r requirements/test.txt + # requests-oauthlib +openedx-filters==1.6.0 + # via -r requirements/test.txt packaging==23.2 # via # -r requirements/test.txt @@ -154,14 +170,22 @@ pyyaml==6.0.1 # via # -r requirements/test.txt # code-annotations + # superset-api-client readme-renderer==42.0 # via twine requests==2.31.0 # via + # -r requirements/test.txt + # requests-oauthlib # requests-toolbelt # sphinx # sphinxcontrib-images + # superset-api-client # twine +requests-oauthlib==1.3.1 + # via + # -r requirements/test.txt + # superset-api-client requests-toolbelt==1.0.0 # via twine restructuredtext-lint==1.4.0 @@ -226,6 +250,8 @@ stevedore==5.1.0 # -r requirements/test.txt # code-annotations # doc8 +superset-api-client==0.6.0 + # via -r requirements/test.txt text-unidecode==1.3 # via # -r requirements/test.txt @@ -250,8 +276,11 @@ typing-extensions==4.9.0 # rich urllib3==2.2.1 # via + # -r requirements/test.txt # requests # twine +web-fragments==2.1.0 + # via -r requirements/test.txt zipp==3.17.0 # via # importlib-metadata diff --git a/requirements/quality.txt b/requirements/quality.txt index ece826a..3b17427 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -12,6 +12,14 @@ astroid==3.0.3 # via # pylint # pylint-celery +certifi==2024.2.2 + # via + # -r requirements/test.txt + # requests +charset-normalizer==3.3.2 + # via + # -r requirements/test.txt + # requests click==8.1.7 # via # -r requirements/test.txt @@ -34,12 +42,20 @@ django==3.2.24 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-crum + # openedx-filters +django-crum==0.7.9 + # via -r requirements/test.txt edx-lint==5.3.6 # via -r requirements/quality.in exceptiongroup==1.2.0 # via # -r requirements/test.txt # pytest +idna==3.6 + # via + # -r requirements/test.txt + # requests iniconfig==2.0.0 # via # -r requirements/test.txt @@ -58,6 +74,12 @@ markupsafe==2.1.5 # jinja2 mccabe==0.7.0 # via pylint +oauthlib==3.2.2 + # via + # -r requirements/test.txt + # requests-oauthlib +openedx-filters==1.6.0 + # via -r requirements/test.txt packaging==23.2 # via # -r requirements/test.txt @@ -111,6 +133,16 @@ pyyaml==6.0.1 # via # -r requirements/test.txt # code-annotations + # superset-api-client +requests==2.31.0 + # via + # -r requirements/test.txt + # requests-oauthlib + # superset-api-client +requests-oauthlib==1.3.1 + # via + # -r requirements/test.txt + # superset-api-client six==1.16.0 # via edx-lint snowballstemmer==2.2.0 @@ -123,6 +155,8 @@ stevedore==5.1.0 # via # -r requirements/test.txt # code-annotations +superset-api-client==0.6.0 + # via -r requirements/test.txt text-unidecode==1.3 # via # -r requirements/test.txt @@ -141,3 +175,9 @@ typing-extensions==4.9.0 # asgiref # astroid # pylint +urllib3==2.2.1 + # via + # -r requirements/test.txt + # requests +web-fragments==2.1.0 + # via -r requirements/test.txt diff --git a/requirements/test.txt b/requirements/test.txt index 638da99..2e01c16 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -8,6 +8,14 @@ asgiref==3.7.2 # via # -r requirements/base.txt # django +certifi==2024.2.2 + # via + # -r requirements/base.txt + # requests +charset-normalizer==3.3.2 + # via + # -r requirements/base.txt + # requests click==8.1.7 # via code-annotations code-annotations==1.6.0 @@ -17,14 +25,28 @@ coverage[toml]==7.4.1 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt + # django-crum + # openedx-filters +django-crum==0.7.9 + # via -r requirements/base.txt exceptiongroup==1.2.0 # via pytest +idna==3.6 + # via + # -r requirements/base.txt + # requests iniconfig==2.0.0 # via pytest jinja2==3.1.3 # via code-annotations markupsafe==2.1.5 # via jinja2 +oauthlib==3.2.2 + # via + # -r requirements/base.txt + # requests-oauthlib +openedx-filters==1.6.0 + # via -r requirements/base.txt packaging==23.2 # via pytest pbr==6.0.0 @@ -46,13 +68,27 @@ pytz==2024.1 # -r requirements/base.txt # django pyyaml==6.0.1 - # via code-annotations + # via + # -r requirements/base.txt + # code-annotations + # superset-api-client +requests==2.31.0 + # via + # -r requirements/base.txt + # requests-oauthlib + # superset-api-client +requests-oauthlib==1.3.1 + # via + # -r requirements/base.txt + # superset-api-client sqlparse==0.4.4 # via # -r requirements/base.txt # django stevedore==5.1.0 # via code-annotations +superset-api-client==0.6.0 + # via -r requirements/base.txt text-unidecode==1.3 # via python-slugify tomli==2.0.1 @@ -63,3 +99,9 @@ typing-extensions==4.9.0 # via # -r requirements/base.txt # asgiref +urllib3==2.2.1 + # via + # -r requirements/base.txt + # requests +web-fragments==2.1.0 + # via -r requirements/base.txt From f4242212265bc4a8a8ad9b8fadf659ce45793e4e Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 13:54:16 -0500 Subject: [PATCH 2/9] chore: quality fixes --- aspects/__init__.py | 3 ++- aspects/apps.py | 6 ++---- aspects/extensions/filters.py | 1 + aspects/settings/common.py | 1 + aspects/settings/test.py | 1 - 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/aspects/__init__.py b/aspects/__init__.py index 06ceb10..ca6a6cb 100644 --- a/aspects/__init__.py +++ b/aspects/__init__.py @@ -1,9 +1,10 @@ """ One-line description for README and other doc files. """ + import os from pathlib import Path -__version__ = '0.1.0' +__version__ = "0.1.0" ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__))) diff --git a/aspects/apps.py b/aspects/apps.py index ac21f85..13a72fa 100644 --- a/aspects/apps.py +++ b/aspects/apps.py @@ -10,7 +10,7 @@ class AspectsConfig(AppConfig): Configuration for the aspects Django application. """ - name = 'aspects' + name = "aspects" plugin_app = { "settings_config": { @@ -29,6 +29,4 @@ class AspectsConfig(AppConfig): def ready(self): """Load modules of Aspects.""" - from aspects.extensions import ( # pylint: disable=unused-import, import-outside-toplevel - filters, - ) + from aspects.extensions import filters # pylint: disable=unused-import, import-outside-toplevel diff --git a/aspects/extensions/filters.py b/aspects/extensions/filters.py index d0a422c..b1583a9 100644 --- a/aspects/extensions/filters.py +++ b/aspects/extensions/filters.py @@ -1,6 +1,7 @@ """ Open edX Filters needed for Aspects integration. """ + import pkg_resources from django.conf import settings from django.template import Context, Template diff --git a/aspects/settings/common.py b/aspects/settings/common.py index 20b0ea1..91bd07d 100644 --- a/aspects/settings/common.py +++ b/aspects/settings/common.py @@ -5,6 +5,7 @@ For the full list of settings and their values, see https://docs.djangoproject.com/en/2.22/ref/settings/ """ + from aspects import ROOT_DIRECTORY # Quick-start development settings - unsuitable for production diff --git a/aspects/settings/test.py b/aspects/settings/test.py index 87c9866..dd00b5d 100644 --- a/aspects/settings/test.py +++ b/aspects/settings/test.py @@ -6,7 +6,6 @@ https://docs.djangoproject.com/en/2.22/ref/settings/ """ - # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/2.22/howto/deployment/checklist/ From 9dd263a0b9763a49e202f97ce747e354ab44e595 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 15:48:09 -0500 Subject: [PATCH 3/9] test: implement test suite for superset integration --- aspects/extensions/filters.py | 17 ++---- aspects/settings/common.py | 24 -------- aspects/settings/test.py | 41 -------------- aspects/utils.py | 18 ++---- test_settings.py | 14 +++++ tests/__init__.py | 0 tests/test_dummy.py | 11 ---- tests/test_filters.py | 46 ++++++++++++++++ tests/test_settings.py | 58 ++++++++++++++++++++ tests/test_utils.py | 100 ++++++++++++++++++++++++++++++++++ 10 files changed, 230 insertions(+), 99 deletions(-) delete mode 100644 aspects/settings/test.py create mode 100644 tests/__init__.py delete mode 100644 tests/test_dummy.py create mode 100644 tests/test_filters.py create mode 100644 tests/test_settings.py create mode 100644 tests/test_utils.py diff --git a/aspects/extensions/filters.py b/aspects/extensions/filters.py index b1583a9..6b2f010 100644 --- a/aspects/extensions/filters.py +++ b/aspects/extensions/filters.py @@ -8,10 +8,10 @@ from openedx_filters import PipelineStep from web_fragments.fragment import Fragment -from aspects.utils import update_context +from aspects.utils import generate_superset_context TEMPLATE_ABSOLUTE_PATH = "/instructor_dashboard/" -BLOCK_CATEGORY = "superset" +BLOCK_CATEGORY = "aspects" class AddSupersetTab(PipelineStep): @@ -31,32 +31,27 @@ def run_filter( settings, "SUPERSET_INSTRUCTOR_DASHBOARD", {} ) dashboard_uuid = instructor_dashboard_config.get("dashboard_uuid") - extra_filters_format = getattr(settings, "SUPERSET_EXTRA_FILTERS_FORMAT", []) - default_filters = [ "org = '{course.org}'", "course_name = '{course.display_name}'", "course_run = '{course.id.run}'", ] - filters = default_filters + extra_filters_format - context = update_context( - context, dashboard_uuid=dashboard_uuid, filters=filters + context = generate_superset_context( + context, dashboard_uuid, filters ) - template = Template(self.resource_string("static/html/superset.html")) + template = Template(self.resource_string("static/html/superset.html")) html = template.render(Context(context)) frag = Fragment(html) - frag.add_css(self.resource_string("static/css/superset.css")) frag.add_javascript(self.resource_string("static/js/embed_dashboard.js")) - section_data = { "fragment": frag, "section_key": BLOCK_CATEGORY, - "section_display_name": "Aspects", + "section_display_name": BLOCK_CATEGORY.title(), "course_id": str(course.id), "template_path_prefix": TEMPLATE_ABSOLUTE_PATH, } diff --git a/aspects/settings/common.py b/aspects/settings/common.py index 91bd07d..3896381 100644 --- a/aspects/settings/common.py +++ b/aspects/settings/common.py @@ -5,32 +5,8 @@ For the full list of settings and their values, see https://docs.djangoproject.com/en/2.22/ref/settings/ """ - from aspects import ROOT_DIRECTORY -# Quick-start development settings - unsuitable for production -# See https://docs.djangoproject.com/en/2.22/howto/deployment/checklist/ - -# SECURITY WARNING: keep the secret key used in production secret! -SECRET_KEY = "secret-key" - - -# Application definition - -INSTALLED_APPS = [ - "aspects", -] - - -# Internationalization -# https://docs.djangoproject.com/en/2.22/topics/i18n/ - -LANGUAGE_CODE = "en-us" - -TIME_ZONE = "UTC" - -USE_TZ = True - def plugin_settings(settings): """ diff --git a/aspects/settings/test.py b/aspects/settings/test.py deleted file mode 100644 index dd00b5d..0000000 --- a/aspects/settings/test.py +++ /dev/null @@ -1,41 +0,0 @@ -""" -Common Django settings for eox_hooks project. -For more information on this file, see -https://docs.djangoproject.com/en/2.22/topics/settings/ -For the full list of settings and their values, see -https://docs.djangoproject.com/en/2.22/ref/settings/ -""" - -# Quick-start development settings - unsuitable for production -# See https://docs.djangoproject.com/en/2.22/howto/deployment/checklist/ - -# SECURITY WARNING: keep the secret key used in production secret! -DATABASES = { - "default": { - "ENGINE": "django.db.backends.sqlite3", - "NAME": "default.db", - "USER": "", - "PASSWORD": "", - "HOST": "", - "PORT": "", - }, - "read_replica": { - "ENGINE": "django.db.backends.sqlite3", - "NAME": "read_replica.db", - "USER": "", - "PASSWORD": "", - "HOST": "", - "PORT": "", - }, -} - -SECRET_KEY = "not-so-secret-key" - -# Internationalization -# https://docs.djangoproject.com/en/2.22/topics/i18n/ - -LANGUAGE_CODE = "en-us" - -TIME_ZONE = "UTC" - -USE_TZ = True diff --git a/aspects/utils.py b/aspects/utils.py index 12d2b12..f502283 100644 --- a/aspects/utils.py +++ b/aspects/utils.py @@ -16,12 +16,10 @@ logger = logging.getLogger(__name__) -def update_context( # pylint: disable=dangerous-default-value +def generate_superset_context( # pylint: disable=dangerous-default-value context, - superset_config={}, dashboard_uuid="", - filters=[], - user=None, + filters=[] ): """ Update context with superset token and dashboard id. @@ -31,16 +29,13 @@ def update_context( # pylint: disable=dangerous-default-value superset_config (dict): superset config. dashboard_uuid (str): superset dashboard uuid. filters (list): list of filters to apply to the dashboard. - user (User): user object. """ course = context["course"] + user = get_current_user() - if user is None: - user = get_current_user() superset_token, dashboard_uuid = generate_guest_token( user=user, course=course, - superset_config=superset_config, dashboard_uuid=dashboard_uuid, filters=filters, ) @@ -63,7 +58,7 @@ def update_context( # pylint: disable=dangerous-default-value return context -def generate_guest_token(user, course, superset_config, dashboard_uuid, filters): +def generate_guest_token(user, course, dashboard_uuid, filters): """ Generate a Superset guest token for the user. @@ -75,10 +70,9 @@ def generate_guest_token(user, course, superset_config, dashboard_uuid, filters) tuple: Superset guest token and dashboard id. or None, exception if Superset is missconfigured or cannot generate guest token. """ - if not superset_config: - superset_config = getattr(settings, "SUPERSET_CONFIG", {}) + superset_config = getattr(settings, "SUPERSET_CONFIG", {}) - superset_internal_host = superset_config.get("service_url", "http://superset:8088/") + superset_internal_host = superset_config.get("service_url") superset_username = superset_config.get("username") superset_password = superset_config.get("password") diff --git a/test_settings.py b/test_settings.py index c51c483..4559fc7 100644 --- a/test_settings.py +++ b/test_settings.py @@ -14,6 +14,7 @@ def root(*args): """ return join(abspath(dirname(__file__)), *args) +DEBUG = True DATABASES = { 'default': { @@ -57,3 +58,16 @@ def root(*args): ], }, }] + +SUPERSET_INSTRUCTOR_DASHBOARD = { + "dashboard_slug": "test-dashboard-slug", + "dashboard_uuid": "test-dashboard-uuid", +} + +SUPERSET_CONFIG = { + "url": "http://dummy-superset-url:8088", + "username": "superset", + "password": "superset", +} + +SUPERSET_EXTRA_FILTERS_FORMAT = [] \ No newline at end of file diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_dummy.py b/tests/test_dummy.py deleted file mode 100644 index 3a92518..0000000 --- a/tests/test_dummy.py +++ /dev/null @@ -1,11 +0,0 @@ -""" -Dummy test to check if the test suite is working. -""" -import pytest - - -@pytest.mark.skip(reason="Placeholder to allow pytest to succeed before real tests are in place.") -def test_placeholder(): - """ - TODO: Delete this test once there are real tests. - """ diff --git a/tests/test_filters.py b/tests/test_filters.py new file mode 100644 index 0000000..4bdee4a --- /dev/null +++ b/tests/test_filters.py @@ -0,0 +1,46 @@ +""" +Tests for the filters module. +""" + +from unittest import TestCase +from unittest.mock import Mock, patch + +from aspects.extensions.filters import BLOCK_CATEGORY, AddSupersetTab + + +class TestFilters(TestCase): + """ + Test suite for the LimeSurveyXBlock filters. + """ + + def setUp(self) -> None: + """ + Set up the test suite. + """ + self.filter = AddSupersetTab(filter_type=Mock(), running_pipeline=Mock()) + self.template_name = "test-template-name" + self.context = {"course": Mock()} + + @patch("aspects.extensions.filters.generate_superset_context") + def test_run_filter(self, mock_generate_superset_context): + """ + Check the filter is not executed when there are no LimeSurvey blocks in the course. + + Expected result: + - The context is returned without modifications. + """ + mock_generate_superset_context.return_value = { + "sections": [], + } + + context = self.filter.run_filter(self.context, self.template_name) + + self.assertDictContainsSubset( + { + "course_id": str(self.context["course"].id), + "section_key": BLOCK_CATEGORY, + "section_display_name": BLOCK_CATEGORY.title(), + "template_path_prefix": "/instructor_dashboard/", + }, + context["context"]["sections"][0], + ) diff --git a/tests/test_settings.py b/tests/test_settings.py new file mode 100644 index 0000000..4d7417e --- /dev/null +++ b/tests/test_settings.py @@ -0,0 +1,58 @@ +""" +Test plugin settings for commond, devstack and production environments +""" + +from django.conf import settings +from django.test import TestCase + +from aspects.settings import common as common_settings +from aspects.settings import production as production_setttings + + +class TestPluginSettings(TestCase): + """ + Tests plugin settings + """ + + def test_common_settings(self): + """ + Test common settings + """ + settings.MAKO_TEMPLATE_DIRS_BASE = [] + common_settings.plugin_settings(settings) + self.assertIn("MAKO_TEMPLATE_DIRS_BASE", settings.__dict__) + self.assertIn("url", settings.SUPERSET_CONFIG) + self.assertIn("username", settings.SUPERSET_CONFIG) + self.assertIn("password", settings.SUPERSET_CONFIG) + self.assertIn("dashboard_slug", settings.SUPERSET_INSTRUCTOR_DASHBOARD) + self.assertIn("dashboard_uuid", settings.SUPERSET_INSTRUCTOR_DASHBOARD) + self.assertIsNotNone(settings.SUPERSET_EXTRA_FILTERS_FORMAT) + + def test_production_settings(self): + """ + Test production settings + """ + settings.ENV_TOKENS = { + "SUPERSET_CONFIG": { + "url": "http://superset.local.overhang.io:8088", + "username": "superset", + "password": "superset", + }, + "SUPERSET_INSTRUCTOR_DASHBOARD": { + "dashboard_slug": "instructor-dashboard", + "dashboard_uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", + }, + "SUPERSET_EXTRA_FILTERS_FORMAT": [], + } + production_setttings.plugin_settings(settings) + self.assertEqual( + settings.SUPERSET_CONFIG, settings.ENV_TOKENS["SUPERSET_CONFIG"] + ) + self.assertEqual( + settings.SUPERSET_INSTRUCTOR_DASHBOARD, + settings.ENV_TOKENS["SUPERSET_INSTRUCTOR_DASHBOARD"], + ) + self.assertEqual( + settings.SUPERSET_EXTRA_FILTERS_FORMAT, + settings.ENV_TOKENS["SUPERSET_EXTRA_FILTERS_FORMAT"], + ) diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000..7966aa9 --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,100 @@ +""" +Tests for the utils module. +""" + +from collections import namedtuple +from unittest import TestCase +from unittest.mock import Mock, patch + +from django.conf import settings +from django.test.utils import override_settings + +from aspects.utils import generate_superset_context + +User = namedtuple("User", ["username"]) + + +class TestContext(TestCase): + + @patch("aspects.utils.generate_guest_token") + def test_generate_superset_context(self, mock_generate_guest_token): + """ + Test generate_superset_context + """ + course_mock = Mock() + filter_mock = Mock() + context = {"course": course_mock} + mock_generate_guest_token.return_value = ("test-token", "test-dashboard-uuid") + + context = generate_superset_context( + context, + dashboard_uuid="test-dashboard-uuid", + filters=[filter_mock], + ) + + self.assertEqual(context["superset_token"], "test-token") + self.assertEqual(context["dashboard_uuid"], "test-dashboard-uuid") + self.assertEqual(context["superset_url"], settings.SUPERSET_CONFIG.get("host")) + self.assertNotIn("exception", context) + + @patch("aspects.utils.SupersetClient") + def test_generate_superset_context_with_superset_client_exception(self, mock_superset_client): + """ + Test generate_superset_context + """ + course_mock = Mock() + filter_mock = Mock() + context = {"course": course_mock} + mock_superset_client.side_effect = Exception("test-exception") + + context = generate_superset_context( + context, + dashboard_uuid="test-dashboard-uuid", + filters=[filter_mock], + ) + + self.assertIn("exception", context) + + @patch("aspects.utils.SupersetClient") + @patch("aspects.utils.get_current_user") + def test_generate_superset_context_succesful(self, mock_get_current_user, mock_superset_client): + """ + Test generate_superset_context + """ + course_mock = Mock() + filter_mock = Mock() + context = {"course": course_mock} + response_mock = Mock(status_code=200) + mock_superset_client.return_value.session.post.return_value = response_mock + response_mock.json.return_value = { + "token": "test-token", + } + mock_get_current_user.return_value = User(username="test-user") + + context = generate_superset_context( + context, + dashboard_uuid="test-dashboard-uuid", + filters=[filter_mock], + ) + + self.assertEqual(context["superset_token"], "test-token") + self.assertEqual(context["dashboard_uuid"], "test-dashboard-uuid") + self.assertEqual(context["superset_url"], settings.SUPERSET_CONFIG.get("host")) + + @patch("aspects.utils.get_current_user") + def test_generate_superset_context_with_exception(self, mock_get_current_user): + """ + Test generate_superset_context + """ + course_mock = Mock() + filter_mock = Mock() + mock_get_current_user.return_value = User(username="test-user") + context = {"course": course_mock} + + context = generate_superset_context( + context, + dashboard_uuid="test-dashboard-uuid", + filters=[filter_mock], + ) + + self.assertIn("exception", context) From 8f56754b491c1aee09488fe6388c889582577132 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 15:50:09 -0500 Subject: [PATCH 4/9] chore: quality fixes --- tests/test_utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 7966aa9..c93974e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -7,14 +7,16 @@ from unittest.mock import Mock, patch from django.conf import settings -from django.test.utils import override_settings from aspects.utils import generate_superset_context User = namedtuple("User", ["username"]) -class TestContext(TestCase): +class TestUtils(TestCase): + """ + Test utils module + """ @patch("aspects.utils.generate_guest_token") def test_generate_superset_context(self, mock_generate_guest_token): From 184698d24880105261ebc44502d938272f47ab35 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 17:23:57 -0500 Subject: [PATCH 5/9] fix: use only dashboard uuid --- aspects/extensions/filters.py | 11 +++++++---- aspects/settings/common.py | 5 +---- aspects/settings/production.py | 4 ++-- aspects/templates/instructor_dashboard/aspects.html | 8 ++++++++ test_settings.py | 5 +---- tests/test_settings.py | 9 ++++----- 6 files changed, 23 insertions(+), 19 deletions(-) create mode 100644 aspects/templates/instructor_dashboard/aspects.html diff --git a/aspects/extensions/filters.py b/aspects/extensions/filters.py index 6b2f010..fe7b129 100644 --- a/aspects/extensions/filters.py +++ b/aspects/extensions/filters.py @@ -27,10 +27,13 @@ def run_filter( """ course = context["course"] - instructor_dashboard_config = getattr( - settings, "SUPERSET_INSTRUCTOR_DASHBOARD", {} - ) - dashboard_uuid = instructor_dashboard_config.get("dashboard_uuid") + if not hasattr(settings, "ASPECTS_INSTRUCTOR_DASHBOARD_UUID"): + return { + "context": context, + } + + dashboard_uuid = settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID + extra_filters_format = getattr(settings, "SUPERSET_EXTRA_FILTERS_FORMAT", []) default_filters = [ "org = '{course.org}'", diff --git a/aspects/settings/common.py b/aspects/settings/common.py index 3896381..5cbb44c 100644 --- a/aspects/settings/common.py +++ b/aspects/settings/common.py @@ -19,8 +19,5 @@ def plugin_settings(settings): "username": "superset", "password": "superset", } - settings.SUPERSET_INSTRUCTOR_DASHBOARD = { - "dashboard_slug": "instructor-dashboard", - "dashboard_uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", - } + settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID = "1d6bf904-f53f-47fd-b1c9-6cd7e284d286" settings.SUPERSET_EXTRA_FILTERS_FORMAT = [] diff --git a/aspects/settings/production.py b/aspects/settings/production.py index 7fa33a6..25a72a9 100644 --- a/aspects/settings/production.py +++ b/aspects/settings/production.py @@ -11,8 +11,8 @@ def plugin_settings(settings): settings.SUPERSET_CONFIG = getattr(settings, "ENV_TOKENS", {}).get( "SUPERSET_CONFIG", settings.SUPERSET_CONFIG ) - settings.SUPERSET_INSTRUCTOR_DASHBOARD = getattr(settings, "ENV_TOKENS", {}).get( - "SUPERSET_INSTRUCTOR_DASHBOARD", settings.SUPERSET_INSTRUCTOR_DASHBOARD + settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID = getattr(settings, "ENV_TOKENS", {}).get( + "ASPECTS_INSTRUCTOR_DASHBOARD_UUID", settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID ) settings.SUPERSET_EXTRA_FILTERS_FORMAT = getattr(settings, "ENV_TOKENS", {}).get( "SUPERSET_EXTRA_FILTERS_FORMAT", settings.SUPERSET_EXTRA_FILTERS_FORMAT diff --git a/aspects/templates/instructor_dashboard/aspects.html b/aspects/templates/instructor_dashboard/aspects.html new file mode 100644 index 0000000..1e8020e --- /dev/null +++ b/aspects/templates/instructor_dashboard/aspects.html @@ -0,0 +1,8 @@ +<%page args="section_data" expression_filter="h"/> +<%! from openedx.core.djangolib.markup import HTML %> + +<%include file="/courseware/xqa_interface.html/"/> + +
+ ${HTML(section_data['fragment'].body_html())} +
diff --git a/test_settings.py b/test_settings.py index 4559fc7..fe4db2e 100644 --- a/test_settings.py +++ b/test_settings.py @@ -59,10 +59,7 @@ def root(*args): }, }] -SUPERSET_INSTRUCTOR_DASHBOARD = { - "dashboard_slug": "test-dashboard-slug", - "dashboard_uuid": "test-dashboard-uuid", -} +ASPECTS_INSTRUCTOR_DASHBOARD_UUID = "test-dashboard-uuid" SUPERSET_CONFIG = { "url": "http://dummy-superset-url:8088", diff --git a/tests/test_settings.py b/tests/test_settings.py index 4d7417e..728ab99 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -24,8 +24,7 @@ def test_common_settings(self): self.assertIn("url", settings.SUPERSET_CONFIG) self.assertIn("username", settings.SUPERSET_CONFIG) self.assertIn("password", settings.SUPERSET_CONFIG) - self.assertIn("dashboard_slug", settings.SUPERSET_INSTRUCTOR_DASHBOARD) - self.assertIn("dashboard_uuid", settings.SUPERSET_INSTRUCTOR_DASHBOARD) + self.assertIsNotNone(settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID) self.assertIsNotNone(settings.SUPERSET_EXTRA_FILTERS_FORMAT) def test_production_settings(self): @@ -38,7 +37,7 @@ def test_production_settings(self): "username": "superset", "password": "superset", }, - "SUPERSET_INSTRUCTOR_DASHBOARD": { + "ASPECTS_INSTRUCTOR_DASHBOARD_UUID": { "dashboard_slug": "instructor-dashboard", "dashboard_uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", }, @@ -49,8 +48,8 @@ def test_production_settings(self): settings.SUPERSET_CONFIG, settings.ENV_TOKENS["SUPERSET_CONFIG"] ) self.assertEqual( - settings.SUPERSET_INSTRUCTOR_DASHBOARD, - settings.ENV_TOKENS["SUPERSET_INSTRUCTOR_DASHBOARD"], + settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID, + settings.ENV_TOKENS["ASPECTS_INSTRUCTOR_DASHBOARD_UUID"], ) self.assertEqual( settings.SUPERSET_EXTRA_FILTERS_FORMAT, From ba4be6be495cf57b6966d2ca7b45a26b2d680a26 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 17:24:10 -0500 Subject: [PATCH 6/9] chore: add lms/cms entry points --- aspects/apps.py | 2 -- setup.py | 8 ++++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/aspects/apps.py b/aspects/apps.py index 13a72fa..327a8f0 100644 --- a/aspects/apps.py +++ b/aspects/apps.py @@ -16,12 +16,10 @@ class AspectsConfig(AppConfig): "settings_config": { "lms.djangoapp": { "common": {"relative_path": "settings.common"}, - "test": {"relative_path": "settings.test"}, "production": {"relative_path": "settings.production"}, }, "cms.djangoapp": { "common": {"relative_path": "settings.common"}, - "test": {"relative_path": "settings.test"}, "production": {"relative_path": "settings.production"}, }, }, diff --git a/setup.py b/setup.py index 8048220..06b7394 100755 --- a/setup.py +++ b/setup.py @@ -157,4 +157,12 @@ def is_requirement(line): 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.8', ], + entry_points={ + 'lms.djangoapp': [ + 'aspects = aspects.apps:AspectsConfig', + ], + 'cms.djangoapp': [ + 'aspects = aspects.apps:AspectsConfig', + ], + }, ) From fd61aebfb4c74475ea804c2eb05bb407b19b006a Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 20 Feb 2024 17:31:45 -0500 Subject: [PATCH 7/9] fix: use settings object directly --- aspects/extensions/filters.py | 20 +++++++------------- aspects/utils.py | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/aspects/extensions/filters.py b/aspects/extensions/filters.py index fe7b129..506023e 100644 --- a/aspects/extensions/filters.py +++ b/aspects/extensions/filters.py @@ -13,6 +13,11 @@ TEMPLATE_ABSOLUTE_PATH = "/instructor_dashboard/" BLOCK_CATEGORY = "aspects" +ASPECTS_SECURITY_FILTERS_FORMAT = [ + "org = '{course.org}'", + "course_name = '{course.display_name}'", + "course_run = '{course.id.run}'", +] class AddSupersetTab(PipelineStep): """Add superset tab to instructor dashboard.""" @@ -26,21 +31,10 @@ def run_filter( _ (str): instructor dashboard template name. """ course = context["course"] - - if not hasattr(settings, "ASPECTS_INSTRUCTOR_DASHBOARD_UUID"): - return { - "context": context, - } - dashboard_uuid = settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID + extra_filters_format = settings.SUPERSET_EXTRA_FILTERS_FORMAT - extra_filters_format = getattr(settings, "SUPERSET_EXTRA_FILTERS_FORMAT", []) - default_filters = [ - "org = '{course.org}'", - "course_name = '{course.display_name}'", - "course_run = '{course.id.run}'", - ] - filters = default_filters + extra_filters_format + filters = ASPECTS_SECURITY_FILTERS_FORMAT + extra_filters_format context = generate_superset_context( context, dashboard_uuid, filters diff --git a/aspects/utils.py b/aspects/utils.py index f502283..d3608ca 100644 --- a/aspects/utils.py +++ b/aspects/utils.py @@ -70,7 +70,7 @@ def generate_guest_token(user, course, dashboard_uuid, filters): tuple: Superset guest token and dashboard id. or None, exception if Superset is missconfigured or cannot generate guest token. """ - superset_config = getattr(settings, "SUPERSET_CONFIG", {}) + superset_config = settings.SUPERSET_CONFIG superset_internal_host = superset_config.get("service_url") superset_username = superset_config.get("username") From b8e08ff87bf3f288956fff5c1503ad5cf3dfdab8 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Wed, 21 Feb 2024 08:35:39 -0500 Subject: [PATCH 8/9] test: move tests to aspects folder --- aspects/extensions/filters.py | 1 + {tests => aspects/tests}/__init__.py | 0 {tests => aspects/tests}/test_filters.py | 0 {tests => aspects/tests}/test_settings.py | 0 {tests => aspects/tests}/test_utils.py | 0 aspects/utils.py | 5 ++--- 6 files changed, 3 insertions(+), 3 deletions(-) rename {tests => aspects/tests}/__init__.py (100%) rename {tests => aspects/tests}/test_filters.py (100%) rename {tests => aspects/tests}/test_settings.py (100%) rename {tests => aspects/tests}/test_utils.py (100%) diff --git a/aspects/extensions/filters.py b/aspects/extensions/filters.py index 506023e..aa85afe 100644 --- a/aspects/extensions/filters.py +++ b/aspects/extensions/filters.py @@ -19,6 +19,7 @@ "course_run = '{course.id.run}'", ] + class AddSupersetTab(PipelineStep): """Add superset tab to instructor dashboard.""" diff --git a/tests/__init__.py b/aspects/tests/__init__.py similarity index 100% rename from tests/__init__.py rename to aspects/tests/__init__.py diff --git a/tests/test_filters.py b/aspects/tests/test_filters.py similarity index 100% rename from tests/test_filters.py rename to aspects/tests/test_filters.py diff --git a/tests/test_settings.py b/aspects/tests/test_settings.py similarity index 100% rename from tests/test_settings.py rename to aspects/tests/test_settings.py diff --git a/tests/test_utils.py b/aspects/tests/test_utils.py similarity index 100% rename from tests/test_utils.py rename to aspects/tests/test_utils.py diff --git a/aspects/utils.py b/aspects/utils.py index d3608ca..ce09db1 100644 --- a/aspects/utils.py +++ b/aspects/utils.py @@ -9,13 +9,12 @@ from django.conf import settings from supersetapiclient.client import SupersetClient +logger = logging.getLogger(__name__) + if settings.DEBUG: os.environ["OAUTHLIB_INSECURE_TRANSPORT"] = "1" -logger = logging.getLogger(__name__) - - def generate_superset_context( # pylint: disable=dangerous-default-value context, dashboard_uuid="", From d1bad7746a11f958fc6aa222831d5aecb98e49e0 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Wed, 21 Feb 2024 09:47:37 -0500 Subject: [PATCH 9/9] test: remove base tests folder --- tox.ini | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tox.ini b/tox.ini index 93cf810..e6e2d5e 100644 --- a/tox.ini +++ b/tox.ini @@ -31,7 +31,7 @@ match-dir = (?!migrations) [pytest] DJANGO_SETTINGS_MODULE = test_settings -addopts = --cov aspects --cov tests --cov-report term-missing --cov-report xml +addopts = --cov aspects --cov-report term-missing --cov-report xml norecursedirs = .* docs requirements site-packages [testenv] @@ -71,12 +71,10 @@ allowlist_externals = deps = -r{toxinidir}/requirements/quality.txt commands = - touch tests/__init__.py - pylint aspects tests test_utils manage.py setup.py - rm tests/__init__.py - pycodestyle aspects tests manage.py setup.py - pydocstyle aspects tests manage.py setup.py - isort --check-only --diff tests test_utils aspects manage.py setup.py test_settings.py + pylint aspects test_utils manage.py setup.py + pycodestyle aspects manage.py setup.py + pydocstyle aspects manage.py setup.py + isort --check-only --diff test_utils aspects manage.py setup.py test_settings.py make selfcheck [testenv:pii_check]