Merge pull request #33 from openedx/bmtcril/lms_dash_fixes

fix: Fix slug repeating language and change tab name

CHANGELOG.rst

@@ -16,6 +16,15 @@ Unreleased
 
 *
 
+0.7.1 - 2024-04-17
+******************
+
+Fixed
+=====
+
+* Fixed issue with embedded dashboards throwing javascript errors
+* Fixed issues with translated embedded dashboards erroring in Superset
+
 0.7.0 - 2024-04-12
 ******************
 

platform_plugin_aspects/__init__.py

@@ -5,6 +5,6 @@
 import os
 from pathlib import Path
 
-__version__ = "0.7.0"
+__version__ = "0.7.1"
 
 ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__)))

platform_plugin_aspects/extensions/filters.py

@@ -55,7 +55,7 @@ def run_filter(
         section_data = {
             "fragment": frag,
             "section_key": BLOCK_CATEGORY,
-            "section_display_name": _("Analytics"),
+            "section_display_name": _("Reports"),
             "course_id": str(context.get("course_id")),
             "superset_guest_token_url": str(context.get("superset_guest_token_url")),
             "superset_url": str(context.get("superset_url")),

platform_plugin_aspects/extensions/tests/test_filters.py

@@ -42,7 +42,7 @@ def test_run_filter_with_language(
             {
                 "course_id": self.course_id,
                 "section_key": BLOCK_CATEGORY,
-                "section_display_name": "Analytics",
+                "section_display_name": "Reports",
                 "superset_url": "http://superset-dummy-url/",
                 "superset_guest_token_url": f"https://lms.url/superset_guest_token/{self.course_id}",
                 "template_path_prefix": "/instructor_dashboard/",
@@ -70,7 +70,7 @@ def test_run_filter_without_language(
             {
                 "course_id": self.course_id,
                 "section_key": BLOCK_CATEGORY,
-                "section_display_name": "Analytics",
+                "section_display_name": "Reports",
                 "superset_url": "http://superset-dummy-url/",
                 "superset_guest_token_url": f"https://lms.url/superset_guest_token/{self.course_id}",
                 "template_path_prefix": "/instructor_dashboard/",

platform_plugin_aspects/static/html/superset.html

@@ -28,8 +28,10 @@ <h2>{{display_name}}</h2>
     </div>
     {% endif %}
 
+    {{superset_dashboards|json_script:"superset_dashboards"}}
+
     <script type="text/javascript">
-        window.superset_dashboards = {{superset_dashboards | safe }};
+        window.superset_dashboards = JSON.parse(document.getElementById('superset_dashboards').textContent);
         window.superset_url = "{{superset_url}}";
         window.superset_guest_token_url = "{{superset_guest_token_url}}";
     </script>

platform_plugin_aspects/tests/test_utils.py

@@ -121,7 +121,8 @@ def test_generate_superset_context(self):
             context["superset_guest_token_url"],
             f"https://lms.url/superset_guest_token/{COURSE_ID}",
         )
-        self.assertEqual(context["superset_dashboards"], dashboards)
+
+        self.assertEqual(len(context["superset_dashboards"]), len(dashboards))
         self.assertEqual(context["superset_url"], "http://superset-dummy-url/")
         self.assertNotIn("superset_token", context)
         self.assertNotIn("exception", context)
@@ -195,3 +196,38 @@ def test_generate_guest_token_succesful(self, mock_superset_client):
 
         mock_superset_client.assert_called_once()
         self.assertEqual(token, "test-token")
+
+    @patch("platform_plugin_aspects.utils.SupersetClient")
+    def test_generate_guest_token_loc(self, mock_superset_client):
+        """
+        Test generate_guest_token works.
+        """
+        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",
+        }
+
+        filter_mock = Mock()
+        user_mock = Mock()
+        dashboards = [
+            {
+                "name": "test",
+                "uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286",
+                "allow_translations": True,
+            }
+        ]
+
+        token = generate_guest_token(
+            user=user_mock,
+            course=COURSE_ID,
+            dashboards=dashboards,
+            filters=[filter_mock],
+        )
+
+        mock_superset_client.assert_called_once()
+        self.assertEqual(token, "test-token")
+
+        # We should have one resource for en_US, one for es_419, and one untranslated
+        calls = mock_superset_client.return_value.session.post.call_args
+        self.assertEqual(len(calls[1]["json"]["resources"]), 3)

platform_plugin_aspects/utils.py

@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 
+import copy
 import logging
 import os
 import uuid
@@ -49,12 +50,15 @@ def generate_superset_context(
     course_id = context["course_id"]
     superset_config = settings.SUPERSET_CONFIG
 
+    # We're modifying this, keep a local copy
+    rtn_dashboards = copy.deepcopy(dashboards)
+
     if language:
-        for dashboard in dashboards:
+        for dashboard in rtn_dashboards:
             if not dashboard.get("allow_translations"):
                 continue
             dashboard["slug"] = f"{dashboard['slug']}-{language}"
-            dashboard["uuid"] = str(get_uuid5(dashboard["uuid"], language))
+            dashboard["uuid"] = get_localized_uuid(dashboard["uuid"], language)
 
     superset_url = _fix_service_url(superset_config.get("service_url"))
 
@@ -69,7 +73,7 @@ def generate_superset_context(
 
     context.update(
         {
-            "superset_dashboards": dashboards,
+            "superset_dashboards": rtn_dashboards,
             "superset_url": superset_url,
             "superset_guest_token_url": guest_token_url,
         }
@@ -104,11 +108,24 @@ def generate_guest_token(user, course, dashboards, filters) -> str:
 
     formatted_filters = [filter.format(course=course, user=user) for filter in filters]
 
+    resources = []
+
+    # Get permissions for all localized versions of the dashboards
+    for dashboard in dashboards:
+        resources.append({"type": "dashboard", "id": dashboard["uuid"]})
+
+        if dashboard.get("allow_translations"):
+            for locale in settings.SUPERSET_DASHBOARD_LOCALES:
+                resources.append(
+                    {
+                        "type": "dashboard",
+                        "id": get_localized_uuid(dashboard["uuid"], locale),
+                    }
+                )
+
     data = {
         "user": _superset_user_data(user),
-        "resources": [
-            {"type": "dashboard", "id": dashboard["uuid"]} for dashboard in dashboards
-        ],
+        "resources": resources,
         "rls": [{"clause": filter} for filter in formatted_filters],
     }
 
@@ -245,10 +262,10 @@ def get_ccx_courses(course_id):
     return []
 
 
-def get_uuid5(base_uuid, language):
+def get_localized_uuid(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)
+    return str(uuid.uuid5(base_namespace, language))

test_settings.py

@@ -33,8 +33,8 @@
 
 # Disable caching in tests.
 CACHES = {
-    'default': {
-        'BACKEND': 'django.core.cache.backends.dummy.DummyCache',
+    "default": {
+        "BACKEND": "django.core.cache.backends.dummy.DummyCache",
     }
 }