From 47a0e3bb637c346fd1fa4906739c3f696ed48b80 Mon Sep 17 00:00:00 2001 From: Cristhian Garcia Date: Fri, 5 Apr 2024 16:12:03 -0500 Subject: [PATCH] feat: allow to embed translated dashboard fix: allow to translate Analytics tab title test: fix tests --- .coveragerc | 1 + platform_plugin_aspects/extensions/filters.py | 10 ++- .../extensions/tests/test_filters.py | 44 +++++++++- 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, 115 insertions(+), 99 deletions(-) delete mode 100644 platform_plugin_aspects/settings/tests/test_settings.py diff --git a/.coveragerc b/.coveragerc index a7e7bad..c26143e 100644 --- a/.coveragerc +++ b/.coveragerc @@ -9,3 +9,4 @@ omit = */static/* */templates/* **/tests/* + */settings/* diff --git a/platform_plugin_aspects/extensions/filters.py b/platform_plugin_aspects/extensions/filters.py index 6b6342f..2ba8141 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,17 @@ def run_filter( user = get_current_user() + language = get_model("user_preference").get_value(user, "pref-lang") or "en_US" + formatted_language = language.replace("-", "_") + if formatted_language not in settings.SUPERSET_DASHBOARD_LOCALES: + formatted_language = None + context = generate_superset_context( context, user, dashboards=dashboards, filters=filters, + language=formatted_language, ) template = Template(self.resource_string("static/html/superset.html")) @@ -57,7 +63,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..d6b0816 100644 --- a/platform_plugin_aspects/extensions/tests/test_filters.py +++ b/platform_plugin_aspects/extensions/tests/test_filters.py @@ -2,9 +2,11 @@ Tests for the filters module. """ -from unittest import TestCase from unittest.mock import Mock, patch +from django.conf import settings +from django.test import TestCase + from platform_plugin_aspects.extensions.filters import BLOCK_CATEGORY, AddSupersetTab @@ -22,7 +24,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 +39,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"]