Merge pull request #15 from openedx/cag/multiple-dashboards

feat: allow embedding multiple superset dashboards
openedx · Mar 20, 2024 · 1e04acd · 1e04acd
2 parents 3430bff + 6bb0b19
commit 1e04acd
Show file tree

Hide file tree

Showing 15 changed files with 150 additions and 50 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -16,6 +16,14 @@ Unreleased
 
 *
 
+0.4.0 - 2024-03-18
+******************
+
+Added
+=====
+
+* Embed multiple Superset Dashboards.
+
 0.3.1 - 2024-03-14
 ******************
 

diff --git a/platform_plugin_aspects/__init__.py b/platform_plugin_aspects/__init__.py
@@ -5,6 +5,6 @@
 import os
 from pathlib import Path
 
-__version__ = "0.3.1"
+__version__ = "0.4.0"
 
 ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__)))
diff --git a/platform_plugin_aspects/extensions/filters.py b/platform_plugin_aspects/extensions/filters.py
@@ -35,7 +35,7 @@ def run_filter(
             _ (str): instructor dashboard template name.
         """
         course = context["course"]
-        dashboard_uuid = settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID
+        dashboards = settings.ASPECTS_INSTRUCTOR_DASHBOARDS
         extra_filters_format = settings.SUPERSET_EXTRA_FILTERS_FORMAT
 
         filters = ASPECTS_SECURITY_FILTERS_FORMAT + extra_filters_format
@@ -45,7 +45,7 @@ def run_filter(
         context = generate_superset_context(
             context,
             user,
-            dashboard_uuid=dashboard_uuid,
+            dashboards=dashboards,
             filters=filters,
         )
 

diff --git a/platform_plugin_aspects/settings/common.py b/platform_plugin_aspects/settings/common.py
@@ -20,7 +20,12 @@ def plugin_settings(settings):
         "username": "superset",
         "password": "superset",
     }
-    settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID = "1d6bf904-f53f-47fd-b1c9-6cd7e284d286"
+    settings.ASPECTS_INSTRUCTOR_DASHBOARDS = [
+        {
+            "name": "Instructor Dashboard",
+            "uuid": "1d6bf904-f53f-47fd-b1c9-6cd7e284d286",
+        },
+    ]
     settings.SUPERSET_EXTRA_FILTERS_FORMAT = []
     settings.EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG = {
         # URL to a running ClickHouse server's HTTP interface. ex: https://foo.openedx.org:8443/ or

diff --git a/platform_plugin_aspects/settings/production.py b/platform_plugin_aspects/settings/production.py
@@ -11,10 +11,8 @@ def plugin_settings(settings):
     settings.SUPERSET_CONFIG = getattr(settings, "ENV_TOKENS", {}).get(
         "SUPERSET_CONFIG", settings.SUPERSET_CONFIG
     )
-    settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID = getattr(
-        settings, "ENV_TOKENS", {}
-    ).get(
-        "ASPECTS_INSTRUCTOR_DASHBOARD_UUID", settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID
+    settings.ASPECTS_INSTRUCTOR_DASHBOARDS = getattr(settings, "ENV_TOKENS", {}).get(
+        "ASPECTS_INSTRUCTOR_DASHBOARDS", settings.ASPECTS_INSTRUCTOR_DASHBOARDS
     )
     settings.SUPERSET_EXTRA_FILTERS_FORMAT = getattr(settings, "ENV_TOKENS", {}).get(
         "SUPERSET_EXTRA_FILTERS_FORMAT", settings.SUPERSET_EXTRA_FILTERS_FORMAT

diff --git a/platform_plugin_aspects/settings/tests/test_settings.py b/platform_plugin_aspects/settings/tests/test_settings.py
@@ -25,7 +25,7 @@ def test_common_settings(self):
         self.assertNotIn("service_url", settings.SUPERSET_CONFIG)
         self.assertIn("username", settings.SUPERSET_CONFIG)
         self.assertIn("password", settings.SUPERSET_CONFIG)
-        self.assertIsNotNone(settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID)
+        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
@@ -46,7 +46,12 @@ def test_production_settings(self):
                 "username": "superset",
                 "password": "superset",
             },
-            "ASPECTS_INSTRUCTOR_DASHBOARD_UUID": "test-settings-dashboard-uuid",
+            "ASPECTS_INSTRUCTOR_DASHBOARDS": [
+                {
+                    "name": "Instructor Dashboard",
+                    "uuid": "test-settings-dashboard-uuid",
+                }
+            ],
             "SUPERSET_EXTRA_FILTERS_FORMAT": [],
             "EVENT_SINK_CLICKHOUSE_BACKEND_CONFIG": {
                 "url": test_url,
@@ -61,8 +66,8 @@ def test_production_settings(self):
             settings.SUPERSET_CONFIG, settings.ENV_TOKENS["SUPERSET_CONFIG"]
         )
         self.assertEqual(
-            settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID,
-            settings.ENV_TOKENS["ASPECTS_INSTRUCTOR_DASHBOARD_UUID"],
+            settings.ASPECTS_INSTRUCTOR_DASHBOARDS,
+            settings.ENV_TOKENS["ASPECTS_INSTRUCTOR_DASHBOARDS"],
         )
         self.assertEqual(
             settings.SUPERSET_EXTRA_FILTERS_FORMAT,

diff --git a/platform_plugin_aspects/static/css/superset.css b/platform_plugin_aspects/static/css/superset.css
@@ -1,5 +1,53 @@
-.superset-embedded-container > iframe {
-    height: 720px;
+.superset-embedded-container>iframe {
+    height: 920px;
     width: 100%;
     display: block;
+    border: none;
 }
+
+.aspects-tabs {
+    position: relative;
+    min-height: 920px;
+    /* This part sucks */
+    clear: both;
+    margin: 25px 0;
+}
+
+.aspects-tab {
+    float: left;
+}
+
+.aspects-tab label {
+    background: #eee;
+    padding: 10px;
+    border: 1px solid #ccc;
+    margin-left: -1px;
+    position: relative;
+    left: 1px;
+    bottom: 5px;
+}
+
+.aspects-tab [type=radio] {
+    display: none;
+}
+
+.aspects-content {
+    position: absolute;
+    top: 28px;
+    left: 0;
+    background: white;
+    right: 0;
+    bottom: 0;
+    padding: 0;
+    border: 1px solid #ccc;
+}
+
+[type=radio]:checked~label {
+    background: white;
+    border-bottom: 1px solid white;
+    z-index: 2;
+}
+
+[type=radio]:checked~label~.aspects-content {
+    z-index: 1;
+}
diff --git a/platform_plugin_aspects/static/html/superset.html b/platform_plugin_aspects/static/html/superset.html
@@ -2,22 +2,34 @@
 
 <script src="https://unpkg.com/@superset-ui/embedded-sdk"></script>
 
-<div class="email-notifier-instructor-wrapper" width="parent">
+<div class="aspects-wrapper" width="fit-content">
     <h2>{{display_name}}</h2>
 
     {% if exception %}
     <p>{% trans 'Superset is not configured properly. Please contact your system administrator.'%}</p>
-    <p>
-        {{exception}}
-    </p>
-    {% elif not dashboard_uuid %}
-    <p>
-        Dashboard UUID is not set. Please set the dashboard UUID in the Studio. {{dashboard_uuid}}
-    </p>
-    {% elif superset_url and superset_token %}
+    <p>{{exception}}</p>
+    {% elif not superset_dashboards %}
+    <p>Dashboard UUID is not set. Please set the dashboard UUID in the Studio.</p>
+    {% elif superset_url and superset_token %} {% if xblock_id %}
     <div class="superset-embedded-container" id="superset-embedded-container-{{xblock_id}}"></div>
+    {% else %}
+    <div class="aspects-tabs">
+        {% for dashboard in superset_dashboards %}
+        <div class="aspects-tab">
+            {% if forloop.counter == 1 %}
+            <input type="radio" id="tab-{{forloop.counter}}" name="tab-group-1" checked="checked" />
+            {% else %}
+            <input type="radio" id="tab-{{forloop.counter}}" name="tab-group-1" />
+            {% endif %}
+            <label for="tab-{{forloop.counter}}">{{dashboard.name}}</label>
+            <div class="aspects-content superset-embedded-container" id="superset-embedded-container-{{dashboard.uuid}}"></div>
+        </div>
+        {% endfor %}
+    </div>
+    {% endif %}
+
     <script type="text/javascript">
-        window.dashboard_uuid ="{{dashboard_uuid}}";
+        window.superset_dashboards = {{superset_dashboards | safe }};
         window.superset_url = "{{superset_url}}";
         window.superset_token = "{{superset_token}}";
     </script>

diff --git a/platform_plugin_aspects/static/js/embed_dashboard.js b/platform_plugin_aspects/static/js/embed_dashboard.js
@@ -24,7 +24,10 @@ function embedDashboard(dashboard_uuid, superset_url, superset_token, xblock_id)
       when the dashboard is loaded
       */
     });
-}
-if (window.dashboard_uuid !== undefined) {
-  embedDashboard(window.dashboard_uuid, window.superset_url, window.superset_token, window.xblock_id);
+};
+
+if (window.superset_dashboards !== undefined) {
+  window.superset_dashboards.forEach(function(dashboard) {
+    embedDashboard(dashboard.uuid, window.superset_url, window.superset_token, dashboard.uuid);
+  });
 }
diff --git a/platform_plugin_aspects/templates/instructor_dashboard/aspects.html b/platform_plugin_aspects/templates/instructor_dashboard/aspects.html
@@ -14,7 +14,7 @@
 <section class="superset">
     % if superset_url:
     <div class="superset-link">
-      <a href="${superset_url}">${_("Go to Superset")}</a>
+      <a href="${superset_url}login/openedxsso">${_("Go to Superset to create new or further customize the reports below.")}</a>
     </div>
     % endif
     ${HTML(section_data['fragment'].body_html())}

diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py
@@ -113,17 +113,18 @@ def test_generate_superset_context(self, mock_generate_guest_token):
         filter_mock = Mock()
         user_mock = Mock()
         context = {"course": course_mock}
-        mock_generate_guest_token.return_value = ("test-token", "test-dashboard-uuid")
+        dashboards = [{"name": "test", "uuid": "test-dashboard-uuid"}]
+        mock_generate_guest_token.return_value = ("test-token", dashboards)
 
         context = generate_superset_context(
             context,
             user_mock,
-            dashboard_uuid="test-dashboard-uuid",
+            dashboards=dashboards,
             filters=[filter_mock],
         )
 
         self.assertEqual(context["superset_token"], "test-token")
-        self.assertEqual(context["dashboard_uuid"], "test-dashboard-uuid")
+        self.assertEqual(context["superset_dashboards"], dashboards)
         self.assertEqual(context["superset_url"], "http://superset-dummy-url/")
         self.assertNotIn("exception", context)
 
@@ -143,7 +144,7 @@ def test_generate_superset_context_with_superset_client_exception(
         context = generate_superset_context(
             context,
             user_mock,
-            dashboard_uuid="test-dashboard-uuid",
+            dashboards=[{"name": "test", "uuid": "test-dashboard-uuid"}],
             filters=[filter_mock],
         )
 
@@ -175,14 +176,17 @@ def test_generate_superset_context_succesful(self, mock_superset_client):
             "token": "test-token",
         }
 
+        dashboards = [{"name": "test", "uuid": "test-dashboard-uuid"}]
+
         context = generate_superset_context(
             context,
             user_mock,
+            dashboards=dashboards,
             filters=[filter_mock],
         )
 
         self.assertEqual(context["superset_token"], "test-token")
-        self.assertEqual(context["dashboard_uuid"], "test-settings-dashboard-uuid")
+        self.assertEqual(context["superset_dashboards"], dashboards)
         self.assertEqual(context["superset_url"], "http://dummy-superset-url/")
 
     def test_generate_superset_context_with_exception(self):
@@ -198,7 +202,7 @@ def test_generate_superset_context_with_exception(self):
         context = generate_superset_context(
             context,
             user_mock,
-            dashboard_uuid="test-dashboard-uuid",
+            dashboards=[{"name": "test", "uuid": "test-dashboard-uuid"}],
             filters=[filter_mock],
         )
 

diff --git a/platform_plugin_aspects/tests/test_xblock.py b/platform_plugin_aspects/tests/test_xblock.py
@@ -69,7 +69,10 @@ def test_render_instructor(self, mock_superset_client):
         mock_superset_client.assert_called_once()
         html = student_view.content
         self.assertIsNotNone(html)
-        self.assertIn("superset-embedded-container", html)
+        self.assertIn(
+            "Dashboard UUID is not set. Please set the dashboard UUID in the Studio.",
+            html,
+        )
 
     def test_render_student(self):
         """

diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py
@@ -28,7 +28,7 @@ def _(text):
 def generate_superset_context(  # pylint: disable=dangerous-default-value
     context,
     user,
-    dashboard_uuid=None,
+    dashboards,
     filters=[],
 ):
     """
@@ -38,17 +38,17 @@ def generate_superset_context(  # pylint: disable=dangerous-default-value
         context (dict): the context for the instructor dashboard. It must include a course object
         user (XBlockUser or User): the current user.
         superset_config (dict): superset config.
-        dashboard_uuid (str): superset dashboard uuid.
+        dashboards (list): list of superset dashboard uuid.
         filters (list): list of filters to apply to the dashboard.
     """
     course = context["course"]
     superset_config = settings.SUPERSET_CONFIG
 
-    superset_token, dashboard_uuid = _generate_guest_token(
+    superset_token, dashboards = _generate_guest_token(
         user=user,
         course=course,
         superset_config=superset_config,
-        dashboard_uuid=dashboard_uuid,
+        dashboards=dashboards,
         filters=filters,
     )
 
@@ -57,21 +57,21 @@ def generate_superset_context(  # pylint: disable=dangerous-default-value
         context.update(
             {
                 "superset_token": superset_token,
-                "dashboard_uuid": dashboard_uuid,
+                "superset_dashboards": dashboards,
                 "superset_url": superset_url,
             }
         )
     else:
         context.update(
             {
-                "exception": str(dashboard_uuid),
+                "exception": str(dashboards),
             }
         )
 
     return context
 
 
-def _generate_guest_token(user, course, superset_config, dashboard_uuid, filters):
+def _generate_guest_token(user, course, superset_config, dashboards, filters):
     """
     Generate a Superset guest token for the user.
 
@@ -102,16 +102,16 @@ def _generate_guest_token(user, course, superset_config, dashboard_uuid, filters
 
     formatted_filters = [filter.format(course=course, user=user) for filter in filters]
 
-    if not dashboard_uuid:
-        dashboard_uuid = settings.ASPECTS_INSTRUCTOR_DASHBOARD_UUID
-
     data = {
         "user": _superset_user_data(user),
-        "resources": [{"type": "dashboard", "id": dashboard_uuid}],
+        "resources": [
+            {"type": "dashboard", "id": dashboard["uuid"]} for dashboard in dashboards
+        ],
         "rls": [{"clause": filter} for filter in formatted_filters],
     }
 
     try:
+        logger.info(f"Requesting guest token from Superset, {data}")
         response = client.session.post(
             url=f"{superset_internal_host}api/v1/security/guest_token/",
             json=data,
@@ -120,7 +120,7 @@ def _generate_guest_token(user, course, superset_config, dashboard_uuid, filters
         response.raise_for_status()
         token = response.json()["token"]
 
-        return token, dashboard_uuid
+        return token, dashboards
     except Exception as exc:  # pylint: disable=broad-except
         logger.error(exc)
         return None, exc