Merge pull request #27 from openedx/jill/refresh-token

feat: adds endpoints for fetchGuestToken
openedx · Apr 16, 2024 · 3927b31 · 3927b31
2 parents 2f295a1 + a77a7f5
commit 3927b31
Show file tree

Hide file tree

Showing 17 changed files with 603 additions and 194 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -16,6 +16,14 @@ Unreleased
 
 *
 
+0.7.0 - 2024-04-12
+******************
+
+Added
+=====
+
+* Add endpoint for fetchGuestToken
+
 0.6.0 - 2024-04-08
 ******************
 

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.6.0"
+__version__ = "0.7.0"
 
 ROOT_DIRECTORY = Path(os.path.dirname(os.path.abspath(__file__)))
diff --git a/platform_plugin_aspects/apps.py b/platform_plugin_aspects/apps.py
@@ -3,7 +3,7 @@
 """
 
 from django.apps import AppConfig
-from edx_django_utils.plugins import PluginSettings, PluginSignals
+from edx_django_utils.plugins import PluginSettings, PluginSignals, PluginURLs
 
 
 class PlatformPluginAspectsConfig(AppConfig):
@@ -14,6 +14,13 @@ class PlatformPluginAspectsConfig(AppConfig):
     name = "platform_plugin_aspects"
 
     plugin_app = {
+        PluginURLs.CONFIG: {
+            "lms.djangoapp": {
+                PluginURLs.NAMESPACE: "",
+                PluginURLs.REGEX: r"^aspects/",
+                PluginURLs.RELATIVE_PATH: "urls",
+            },
+        },
         PluginSettings.CONFIG: {
             "lms.djangoapp": {
                 "production": {PluginSettings.RELATIVE_PATH: "settings.production"},

diff --git a/platform_plugin_aspects/extensions/filters.py b/platform_plugin_aspects/extensions/filters.py
@@ -14,12 +14,6 @@
 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):
     """
@@ -36,9 +30,6 @@ def run_filter(
         """
         course = context["course"]
         dashboards = settings.ASPECTS_INSTRUCTOR_DASHBOARDS
-        extra_filters_format = settings.SUPERSET_EXTRA_FILTERS_FORMAT
-
-        filters = ASPECTS_SECURITY_FILTERS_FORMAT + extra_filters_format
 
         user = get_current_user()
 
@@ -49,11 +40,10 @@ def run_filter(
         if formatted_language not in settings.SUPERSET_DASHBOARD_LOCALES:
             formatted_language = "en_US"
 
+        context["course_id"] = course.id
         context = generate_superset_context(
             context,
-            user,
             dashboards=dashboards,
-            filters=filters,
             language=formatted_language,
         )
 
@@ -66,7 +56,8 @@ def run_filter(
             "fragment": frag,
             "section_key": BLOCK_CATEGORY,
             "section_display_name": _("Analytics"),
-            "course_id": str(course.id),
+            "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")),
             "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
@@ -20,66 +20,62 @@ def setUp(self) -> None:
         """
         self.filter = AddSupersetTab(filter_type=Mock(), running_pipeline=Mock())
         self.template_name = "test-template-name"
-        self.context = {"course": Mock()}
+        self.course_id = "course-v1:org+course+run"
+        self.context = {"course": Mock(id=self.course_id), "sections": []}
 
-    @patch("platform_plugin_aspects.extensions.filters.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
+        self,
+        mock_get_model,
     ):
         """
         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 = "not-a-language"
 
         context = self.filter.run_filter(self.context, self.template_name)
 
         self.assertDictContainsSubset(
             {
-                "course_id": str(self.context["course"].id),
+                "course_id": self.course_id,
                 "section_key": BLOCK_CATEGORY,
                 "section_display_name": "Analytics",
-                "superset_url": "http://superset.testing",
+                "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/",
             },
             context["context"]["sections"][0],
         )
+        mock_get_model.assert_called_once()
 
-    @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
+        self,
+        mock_get_model,
     ):
         """
         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),
+                "course_id": self.course_id,
                 "section_key": BLOCK_CATEGORY,
                 "section_display_name": "Analytics",
-                "superset_url": "http://superset.testing",
+                "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/",
             },
             context["context"]["sections"][0],
         )
+
+        mock_get_model.assert_called_once()
diff --git a/platform_plugin_aspects/signals.py b/platform_plugin_aspects/signals.py
@@ -33,7 +33,6 @@ def receive_course_publish(  # pylint: disable=unused-argument  # pragma: no cov
     dump_course_to_clickhouse.delay(str(course_key))
 
 
-@receiver(post_save, sender=get_model("user_profile"))
 def on_user_profile_updated(  # pylint: disable=unused-argument  # pragma: no cover
     sender, instance, **kwargs
 ):
@@ -53,7 +52,13 @@ def on_user_profile_updated(  # pylint: disable=unused-argument  # pragma: no co
     )
 
 
-@receiver(post_save, sender=get_model("external_id"))
+# Connect the UserProfile.post_save signal handler only if we have a model to attach to.
+# (prevents celery errors during tests)
+_user_profile = get_model("user_profile")
+if _user_profile:
+    post_save.connect(on_user_profile_updated, sender=_user_profile)  # pragma: no cover
+
+
 def on_externalid_saved(  # pylint: disable=unused-argument  # pragma: no cover
     sender, instance, **kwargs
 ):
@@ -73,6 +78,13 @@ def on_externalid_saved(  # pylint: disable=unused-argument  # pragma: no cover
     )
 
 
+# Connect the ExternalId.post_save signal handler only if we have a model to attach to.
+# (prevents celery errors during tests)
+_external_id = get_model("external_id")
+if _external_id:
+    post_save.connect(on_externalid_saved, sender=_external_id)  # pragma: no cover
+
+
 @receiver(USER_RETIRE_LMS_MISC)
 def on_user_retirement(  # pylint: disable=unused-argument  # pragma: no cover
     sender, user, **kwargs

diff --git a/platform_plugin_aspects/static/html/superset.html b/platform_plugin_aspects/static/html/superset.html
@@ -10,7 +10,7 @@ <h2>{{display_name}}</h2>
     <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 %}
+    {% elif superset_url and superset_guest_token_url %} {% if xblock_id %}
     <div class="superset-embedded-container" id="superset-embedded-container-{{xblock_id}}"></div>
     {% else %}
     <div class="aspects-tabs">
@@ -31,7 +31,7 @@ <h2>{{display_name}}</h2>
     <script type="text/javascript">
         window.superset_dashboards = {{superset_dashboards | safe }};
         window.superset_url = "{{superset_url}}";
-        window.superset_token = "{{superset_token}}";
+        window.superset_guest_token_url = "{{superset_guest_token_url}}";
     </script>
     {% endif %}
 </div>
diff --git a/platform_plugin_aspects/static/js/embed_dashboard.js b/platform_plugin_aspects/static/js/embed_dashboard.js
@@ -1,11 +1,46 @@
-function embedDashboard(dashboard_uuid, superset_url, superset_token, xblock_id) {
+function getCookie(name) {
+  let cookieValue = null;
+  if (document.cookie && document.cookie !== "") {
+    const cookies = document.cookie.split(";");
+    for (let i = 0; i < cookies.length; i++) {
+      const cookie = cookies[i].trim();
+      // Does this cookie string begin with the name we want?
+      if (cookie.substring(0, name.length + 1) === name + "=") {
+        cookieValue = decodeURIComponent(cookie.substring(name.length + 1));
+        break;
+      }
+    }
+  }
+  return cookieValue;
+}
+
+async function fetchGuestToken() {
+  const response = await fetch(superset_guest_token_url, {
+    method: 'GET',
+    headers: {
+      "X-CSRFToken": getCookie("csrftoken"),
+    }
+  });
+
+  if (!response.ok) {
+    console.error(await response.json());
+    // TODO: Handle error
+    return null;
+  }
+
+  const data = await response.json();
+  return data.guestToken;
+}
+
+function embedDashboard(dashboard_uuid, superset_url, 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
+      fetchGuestToken: fetchGuestToken,
       dashboardUiConfig: {
         // dashboard UI config: hideTitle, hideTab, hideChartControls, filters.visible, filters.expanded (optional)
         hideTitle: true,
@@ -28,6 +63,6 @@ function embedDashboard(dashboard_uuid, superset_url, superset_token, xblock_id)
 
 if (window.superset_dashboards !== undefined) {
   window.superset_dashboards.forEach(function(dashboard) {
-    embedDashboard(dashboard.uuid, window.superset_url, window.superset_token, dashboard.uuid);
+    embedDashboard(dashboard.uuid, window.superset_url, dashboard.uuid);
   });
 }
diff --git a/platform_plugin_aspects/static/js/superset.js b/platform_plugin_aspects/static/js/superset.js
@@ -2,11 +2,11 @@
 function SupersetXBlock(runtime, element, context) {
   const dashboard_uuid = context.dashboard_uuid;
   const superset_url = context.superset_url;
-  const superset_token = context.superset_token;
+  const superset_guest_token_url = runtime.handlerUrl(element, 'get_superset_guest_token');
   const xblock_id = context.xblock_id
 
   function initSuperset(supersetEmbeddedSdk) {
-    embedDashboard(dashboard_uuid, superset_url, superset_token, xblock_id);
+    embedDashboard(dashboard_uuid, superset_url, superset_guest_token_url, xblock_id);
   }
 
   if (typeof require === "function") {