From 66f0dd45713757e1321fa46e5a9cb236c83ffb61 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Mon, 8 Apr 2024 09:01:45 -0500 Subject: [PATCH 1/3] build: replace covecov with python-coverage-comment-action --- .coveragerc | 1 + .github/workflows/ci.yml | 26 +++++++++++++++++++++--- .github/workflows/coverage.yml | 36 ++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/coverage.yml diff --git a/.coveragerc b/.coveragerc index a7e7bad..31ab62f 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,5 +1,6 @@ [run] branch = True +relative_files = True data_file = .coverage source=platform_plugin_aspects omit = diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b6e266d..ec228f0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,14 @@ jobs: os: [ubuntu-20.04] python-version: ['3.8'] toxenv: [quality, docs, pii_check, django32, django40] + permissions: + # Gives the action the necessary permissions for publishing new + # comments in pull requests. + pull-requests: write + # Gives the action the necessary permissions for pushing data to the + # python-coverage-comment-action branch, and for editing existing + # comments (to avoid publishing multiple comments in the same PR) + contents: write steps: - uses: actions/checkout@v4 @@ -38,7 +46,19 @@ jobs: - name: Run coverage if: matrix.python-version == '3.8' && matrix.toxenv == 'django32' - uses: codecov/codecov-action@v3 + uses: py-cov-action/python-coverage-comment-action@v3 with: - flags: unittests - fail_ci_if_error: true + GITHUB_TOKEN: ${{ github.token }} + MINIMUM_GREEN: 90 + MINIMUM_ORANGE: 85 + ANNOTATE_MISSING_LINES: true + ANNOTATION_TYPE: error + + - name: Store Pull Request comment to be posted + uses: actions/upload-artifact@v4 + if: steps.coverage_comment.outputs.COMMENT_FILE_WRITTEN == 'true' + with: + # If you use a different name, update COMMENT_ARTIFACT_NAME accordingly + name: python-coverage-comment-action + # If you use a different name, update COMMENT_FILENAME accordingly + path: python-coverage-comment-action.txt diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml new file mode 100644 index 0000000..a36cf24 --- /dev/null +++ b/.github/workflows/coverage.yml @@ -0,0 +1,36 @@ +# .github/workflows/coverage.yml +name: Post coverage comment + +on: + workflow_run: + workflows: ["Python CI"] + types: + - completed + +jobs: + test: + name: Run tests & display coverage + runs-on: ubuntu-latest + if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' + permissions: + # Gives the action the necessary permissions for publishing new + # comments in pull requests. + pull-requests: write + # Gives the action the necessary permissions for editing existing + # comments (to avoid publishing multiple comments in the same PR) + contents: write + # Gives the action the necessary permissions for looking up the + # workflow that launched this workflow, and download the related + # artifact that contains the comment to be published + actions: read + steps: + # DO NOT run actions/checkout here, for security reasons + # For details, refer to https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ + - name: Post comment + uses: py-cov-action/python-coverage-comment-action@v3 + with: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_PR_RUN_ID: ${{ github.event.workflow_run.id }} + # Update those if you changed the default values: + # COMMENT_ARTIFACT_NAME: python-coverage-comment-action + # COMMENT_FILENAME: python-coverage-comment-action.txt From 3331cadf9101a7ba8b4b68bf74960ef9261b8a4a Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Fri, 5 Apr 2024 16:12:03 -0500 Subject: [PATCH 2/3] feat: allow to embed translated dashboard fix: allow to translate Analytics tab title test: fix tests --- .coveragerc | 1 + platform_plugin_aspects/extensions/filters.py | 12 ++- .../extensions/tests/test_filters.py | 43 +++++++++- platform_plugin_aspects/settings/common.py | 7 ++ .../settings/production.py | 3 + .../settings/tests/test_settings.py | 85 ------------------- platform_plugin_aspects/tests/test_utils.py | 14 ++- platform_plugin_aspects/utils.py | 19 +++++ test_settings.py | 31 +++++-- 9 files changed, 116 insertions(+), 99 deletions(-) delete mode 100644 platform_plugin_aspects/settings/tests/test_settings.py diff --git a/.coveragerc b/.coveragerc index 31ab62f..d754fc3 100644 --- a/.coveragerc +++ b/.coveragerc @@ -10,3 +10,4 @@ omit = */static/* */templates/* **/tests/* + */settings/* diff --git a/platform_plugin_aspects/extensions/filters.py b/platform_plugin_aspects/extensions/filters.py index 6b6342f..428d53c 100644 --- a/platform_plugin_aspects/extensions/filters.py +++ b/platform_plugin_aspects/extensions/filters.py @@ -9,7 +9,7 @@ from openedx_filters import PipelineStep from web_fragments.fragment import Fragment -from platform_plugin_aspects.utils import generate_superset_context +from platform_plugin_aspects.utils import _, generate_superset_context, get_model TEMPLATE_ABSOLUTE_PATH = "/instructor_dashboard/" BLOCK_CATEGORY = "aspects" @@ -42,11 +42,19 @@ def run_filter( user = get_current_user() + user_language = ( + get_model("user_preference").get_value(user, "pref-lang") or "en_US" + ) + formatted_language = user_language.replace("-", "_") + if formatted_language not in settings.SUPERSET_DASHBOARD_LOCALES: + formatted_language = "en_US" + context = generate_superset_context( context, user, dashboards=dashboards, filters=filters, + language=formatted_language, ) template = Template(self.resource_string("static/html/superset.html")) @@ -57,7 +65,7 @@ def run_filter( section_data = { "fragment": frag, "section_key": BLOCK_CATEGORY, - "section_display_name": BLOCK_CATEGORY.title(), + "section_display_name": _("Analytics"), "course_id": str(course.id), "superset_url": str(context.get("superset_url")), "template_path_prefix": TEMPLATE_ABSOLUTE_PATH, diff --git a/platform_plugin_aspects/extensions/tests/test_filters.py b/platform_plugin_aspects/extensions/tests/test_filters.py index 10fef06..eb64abf 100644 --- a/platform_plugin_aspects/extensions/tests/test_filters.py +++ b/platform_plugin_aspects/extensions/tests/test_filters.py @@ -2,9 +2,10 @@ Tests for the filters module. """ -from unittest import TestCase from unittest.mock import Mock, patch +from django.test import TestCase + from platform_plugin_aspects.extensions.filters import BLOCK_CATEGORY, AddSupersetTab @@ -22,7 +23,10 @@ def setUp(self) -> None: self.context = {"course": Mock()} @patch("platform_plugin_aspects.extensions.filters.generate_superset_context") - def test_run_filter(self, mock_generate_superset_context): + @patch("platform_plugin_aspects.extensions.filters.get_model") + def test_run_filter_with_language( + self, mock_get_model, mock_generate_superset_context + ): """ Check the filter is not executed when there are no LimeSurvey blocks in the course. @@ -34,13 +38,46 @@ def test_run_filter(self, mock_generate_superset_context): "superset_url": "http://superset.testing", } + mock_get_model.return_value.get_value.return_value = "not-a-language" + + 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": "Analytics", + "superset_url": "http://superset.testing", + "template_path_prefix": "/instructor_dashboard/", + }, + context["context"]["sections"][0], + ) + + @patch("platform_plugin_aspects.extensions.filters.generate_superset_context") + @patch("platform_plugin_aspects.extensions.filters.get_model") + def test_run_filter_without_language( + self, mock_get_model, 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": [], + "superset_url": "http://superset.testing", + } + + mock_get_model.return_value.get_value.return_value = None + 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(), + "section_display_name": "Analytics", "superset_url": "http://superset.testing", "template_path_prefix": "/instructor_dashboard/", }, diff --git a/platform_plugin_aspects/settings/common.py b/platform_plugin_aspects/settings/common.py index e430d59..9584994 100644 --- a/platform_plugin_aspects/settings/common.py +++ b/platform_plugin_aspects/settings/common.py @@ -23,10 +23,13 @@ def plugin_settings(settings): settings.ASPECTS_INSTRUCTOR_DASHBOARDS = [ { "name": "Instructor Dashboard", + "slug": "instructor-dashboard", "uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", + "allow_translations": True, }, ] settings.SUPERSET_EXTRA_FILTERS_FORMAT = [] + settings.SUPERSET_DASHBOARD_LOCALES = ["en_US"] settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG = { # URL to a running ClickHouse server's HTTP interface. ex: https://foo.openedx.org:8443/ or # http://foo.openedx.org:8123/ . Note that we only support the ClickHouse HTTP interface @@ -64,4 +67,8 @@ def plugin_settings(settings): "module": "lms.djangoapps.ccx.models", "model": "CustomCourseForEdX", }, + "user_preference": { + "module": "openedx.core.djangoapps.user_api.models", + "model": "UserPreference", + }, } diff --git a/platform_plugin_aspects/settings/production.py b/platform_plugin_aspects/settings/production.py index 0b37432..e355ff5 100644 --- a/platform_plugin_aspects/settings/production.py +++ b/platform_plugin_aspects/settings/production.py @@ -17,6 +17,9 @@ def plugin_settings(settings): settings.SUPERSET_EXTRA_FILTERS_FORMAT = getattr(settings, "ENV_TOKENS", {}).get( "SUPERSET_EXTRA_FILTERS_FORMAT", settings.SUPERSET_EXTRA_FILTERS_FORMAT ) + settings.SUPERSET_DASHBOARD_LOCALES = getattr(settings, "ENV_TOKENS", {}).get( + "SUPERSET_DASHBOARD_LOCALES", settings.SUPERSET_DASHBOARD_LOCALES + ) settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG = settings.ENV_TOKENS.get( "EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG", settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG, diff --git a/platform_plugin_aspects/settings/tests/test_settings.py b/platform_plugin_aspects/settings/tests/test_settings.py deleted file mode 100644 index ffc88c7..0000000 --- a/platform_plugin_aspects/settings/tests/test_settings.py +++ /dev/null @@ -1,85 +0,0 @@ -""" -Test plugin settings for commond, devstack and production environments -""" - -from django.conf import settings -from django.test import TestCase - -from platform_plugin_aspects.settings import common as common_settings -from platform_plugin_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("internal_service_url", settings.SUPERSET_CONFIG) - self.assertNotIn("service_url", settings.SUPERSET_CONFIG) - self.assertIn("username", settings.SUPERSET_CONFIG) - self.assertIn("password", settings.SUPERSET_CONFIG) - self.assertIsNotNone(settings.ASPECTS_INSTRUCTOR_DASHBOARDS) - self.assertIsNotNone(settings.SUPERSET_EXTRA_FILTERS_FORMAT) - for key in ("url", "username", "password", "database", "timeout_secs"): - assert key in settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG - - def test_production_settings(self): - """ - Test production settings - """ - test_url = "https://foo.bar" - test_username = "bob" - test_password = "secret" - test_database = "cool_data" - test_timeout = 1 - settings.ENV_TOKENS = { - "SUPERSET_CONFIG": { - "internal_service_url": "http://superset:8088", - "service_url": "http://superset.local.overhang.io", - "username": "superset", - "password": "superset", - }, - "ASPECTS_INSTRUCTOR_DASHBOARDS": [ - { - "name": "Instructor Dashboard", - "uuid": "test-settings-dashboard-uuid", - } - ], - "SUPERSET_EXTRA_FILTERS_FORMAT": [], - "EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG": { - "url": test_url, - "username": test_username, - "password": test_password, - "database": test_database, - "timeout_secs": test_timeout, - }, - } - production_setttings.plugin_settings(settings) - self.assertEqual( - settings.SUPERSET_CONFIG, settings.ENV_TOKENS["SUPERSET_CONFIG"] - ) - self.assertEqual( - settings.ASPECTS_INSTRUCTOR_DASHBOARDS, - settings.ENV_TOKENS["ASPECTS_INSTRUCTOR_DASHBOARDS"], - ) - self.assertEqual( - settings.SUPERSET_EXTRA_FILTERS_FORMAT, - settings.ENV_TOKENS["SUPERSET_EXTRA_FILTERS_FORMAT"], - ) - - for key, val in ( - ("url", test_url), - ("username", test_username), - ("password", test_password), - ("database", test_database), - ("timeout_secs", test_timeout), - ): - assert key in settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG - assert settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG[key] == val diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py index d14ee05..3839373 100644 --- a/platform_plugin_aspects/tests/test_utils.py +++ b/platform_plugin_aspects/tests/test_utils.py @@ -3,10 +3,10 @@ """ from collections import namedtuple -from unittest import TestCase from unittest.mock import Mock, patch from django.conf import settings +from django.test import TestCase from platform_plugin_aspects.utils import ( generate_superset_context, @@ -113,7 +113,16 @@ def test_generate_superset_context(self, mock_generate_guest_token): filter_mock = Mock() user_mock = Mock() context = {"course": course_mock} - dashboards = [{"name": "test", "uuid": "test-dashboard-uuid"}] + dashboards = settings.ASPECTS_INSTRUCTOR_DASHBOARDS + + dashboards.append( + { + "slug": "test-slug", + "uuid": "3ea6e738-989d-4325-8f93-82bb684dab5c", + "allow_translations": False, + } + ) + mock_generate_guest_token.return_value = ("test-token", dashboards) context = generate_superset_context( @@ -121,6 +130,7 @@ def test_generate_superset_context(self, mock_generate_guest_token): user_mock, dashboards=dashboards, filters=[filter_mock], + language="en_US", ) self.assertEqual(context["superset_token"], "test-token") diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py index d44aeb6..849252f 100644 --- a/platform_plugin_aspects/utils.py +++ b/platform_plugin_aspects/utils.py @@ -6,6 +6,7 @@ import logging import os +import uuid from importlib import import_module from django.conf import settings @@ -30,6 +31,7 @@ def generate_superset_context( # pylint: disable=dangerous-default-value user, dashboards, filters=[], + language=None, ): """ Update context with superset token and dashboard id. @@ -40,10 +42,18 @@ def generate_superset_context( # pylint: disable=dangerous-default-value superset_config (dict): superset config. dashboards (list): list of superset dashboard uuid. filters (list): list of filters to apply to the dashboard. + language (str): the language code of the end user. """ course = context["course"] superset_config = settings.SUPERSET_CONFIG + if language: + for dashboard in dashboards: + if not dashboard["allow_translations"]: + continue + dashboard["slug"] = f"{dashboard['slug']}-{language}" + dashboard["uuid"] = str(get_uuid5(dashboard["uuid"], language)) + superset_token, dashboards = _generate_guest_token( user=user, course=course, @@ -220,3 +230,12 @@ def get_ccx_courses(course_id): if settings.FEATURES.get("CUSTOM_COURSES_EDX"): return get_model("custom_course_edx").objects.filter(course_id=course_id) return [] + + +def get_uuid5(base_uuid, language): + """ + Generate an idempotent uuid. + """ + base_uuid = uuid.UUID(base_uuid) + base_namespace = uuid.uuid5(base_uuid, "superset") + return uuid.uuid5(base_namespace, language) diff --git a/test_settings.py b/test_settings.py index f3a099c..987b008 100644 --- a/test_settings.py +++ b/test_settings.py @@ -58,13 +58,6 @@ } ] -ASPECTS_INSTRUCTOR_DASHBOARDS = [ - { - "name": "Instructor Dashboard", - "uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", - }, -] - SUPERSET_EXTRA_FILTERS_FORMAT = [] SUPERSET_CONFIG = { @@ -73,3 +66,27 @@ "username": "superset", "password": "superset", } + +SUPERSET_CONFIG = { + "internal_service_url": "http://superset:8088", + "service_url": "http://superset.local.overhang.io", + "username": "superset", + "password": "superset", +} +ASPECTS_INSTRUCTOR_DASHBOARDS = [ + { + "name": "Instructor Dashboard", + "slug": "instructor-dashboard", + "uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286", + "allow_translations": True, + } +] +EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG = { + "url": "https://foo.bar", + "username": "bob", + "password": "secret", + "database": "cool_data", + "timeout_secs": 1, +} + +SUPERSET_DASHBOARD_LOCALES = ["en_US", "es_419"] From 0ab1bc91e5b468216ece546970df845be1419388 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Tue, 9 Apr 2024 10:42:07 -0500 Subject: [PATCH 3/3] chore: bump version to v0.6.0 --- CHANGELOG.rst | 11 ++++++++++- platform_plugin_aspects/__init__.py | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 501b5bc..66314ec 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,14 @@ Unreleased * +0.6.0 - 2024-04-08 +****************** + +Added +===== + +* Allow to embed translated Superset Dashboards. + 0.5.0 - 2024-04-01 ****************** @@ -24,7 +32,6 @@ Added * Load testing and test monitoring scripts. - 0.4.0 - 2024-03-18 ****************** @@ -43,6 +50,7 @@ Added 0.3.0 – 2024-03-10 ****************** + Added ===== @@ -50,6 +58,7 @@ Added 0.2.0 – 2024-03-05 ****************** + Added ===== diff --git a/platform_plugin_aspects/__init__.py b/platform_plugin_aspects/__init__.py index 6b6ca5f..8927290 100644 --- a/platform_plugin_aspects/__init__.py +++ b/platform_plugin_aspects/__init__.py @@ -5,6 +5,6 @@ import os from pathlib import Path -__version__ = "0.5.0" +__version__ = "0.6.0" ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__)))