feat: API endpoint for in-context Supserset dashboards #136

ArturGaspar · 2025-03-20T10:39:45Z

Description

Adds an API endpoint for in-context Supserset dashboards.

This uses the dashboards from openedx/tutor-contrib-aspects#1046.

Supporting information

In-Context Metrics for Studio: Product Requirements

Testing instructions

Add this repository checked out to this branch as a Tutor mountpoint: tutor mounts add platform-plugin-aspects
Install tutor-contrib-aspects from feat: in-context analytics dashboards tutor-contrib-aspects#1046
Enable Aspects as instructed in tutor-contrib-aspects (ignoring the pip install tutor-contrib-aspects, as it should be installed from the PR above)

Add and enable the following Tutor plugin in order to use the local copy instead of the one installed by tutor-contrib-aspects.

from tutor import hooks
  
hooks.Filters.ENV_PATCHES.add_item(
    (
        "openedx-dev-dockerfile-post-python-requirements",
        """RUN pip install -e /mnt/platform-plugin-aspects"""
    )
)

Save the following HTML for testing embedding:

Details

<!DOCTYPE html>
<html lang="">
  <head>
    <meta charset="utf-8">
    <title></title>
  </head>
  <body>
    <style>
    html, body, div, iframe { width: 100%; height: 100%; }
    </style>
    <script src="https://unpkg.com/@superset-ui/embedded-sdk"></script>
    <div id="my-superset-container"></div>

    <script>
      supersetEmbeddedSdk.embedDashboard({
        id: "REPLACE ME",
        supersetDomain: "http://superset.local.openedx.io:8088",
        mountPoint: document.getElementById("my-superset-container"),
        fetchGuestToken: () => "REPLACE ME",
        dashboardUiConfig: {
          hideTitle: true,
          filters: {
            visible: false,
            expanded: false,
          },
          urlParams: {
            native_filters: "REPLACE ME",
          }
        },
        iframeSandboxExtras: ['allow-top-navigation', 'allow-popups-to-escape-sandbox']
      });
    </script>
  </body>
</html>

Obtain the guest token from http://local.openedx.io:8000/aspects/superset_guest_token/course-v1:OpenedX+DemoX+DemoCourse and fill it in the HTML above
Obtain the dashboard UUID and native filters from http://local.openedx.io:8000/aspects/superset_in_context_dashboard/course-v1:OpenedX+DemoX+DemoCourse, fill in the HTML above, see that it embeds successfully and loads data corresponsing to the selected course
Obtain the dashboard UUID and native filters from http://local.openedx.io:8000/aspects/superset_in_context_dashboard/[block id], fill in the HTML above, see that it embeds successfully and loads data corresponsing to the selected subsection or block

Deadline

9 April 2025

Other information

Merge checklist:
Check off if complete or not applicable:

Version bumped
Documentation updated (not only docstrings)
Fixup commits are squashed away
Unit tests added/updated
Manual testing instructions provided
Noted any: Concerns, dependencies, migration issues, deadlines, tickets

Private-ref: https://tasks.opencraft.com/browse/BB-9597

openedx-webhooks · 2025-03-20T10:39:49Z

Thanks for the pull request, @ArturGaspar!

This repository is currently maintained by @bmtcril.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

0x29a · 2025-03-28T08:11:10Z

platform_plugin_aspects/views.py

+            )
+
+        if dashboard.get("allow_translations"):
+            user = get_current_user()


I think you don't need get_current_user from crum here and can use just request.user.

farhaanbukhsh

I have added few comments but overall it looks good.

👍

✅ I tested this on sandbox
✅ I read through the code
❌ I checked for accessibility issues
✅ Includes documentation

farhaanbukhsh · 2025-04-15T11:54:41Z

platform_plugin_aspects/urls.py

 from . import views

-# Copied from openedx.core.constants
+# Copied from edx-platform


Adding the filename or URL should be a good comment.

farhaanbukhsh · 2025-04-15T11:55:19Z

platform_plugin_aspects/urls.py

+        # FIXME: remove this one, keeping it until frontend plugin is updated
+        re_path(
+            rf"superset_embedded_dashboard/{USAGE_ID_PATTERN}/?$",
+            views.SupersetInContextDashboardView.as_view(),
+            name="superset_embedded_dashboard",
+        ),


Do we still need this?

platform_plugin_aspects/utils.py

farhaanbukhsh · 2025-04-15T12:04:57Z

platform_plugin_aspects/views.py

+                )
+
+        course_runs = {}
+        # TODO: obtain course runs.


We should remove this.

farhaanbukhsh · 2025-04-15T12:06:17Z

platform_plugin_aspects/views.py

+
+class SupersetInContextDashboardView(GenericAPIView):
+    """
+    Endpoint for in-context analytics embedded Superset dashboard parameters.


Putting in an example get request and response would make this view look great.

farhaanbukhsh · 2025-04-15T12:09:24Z

platform_plugin_aspects/views.py

+        # TODO: obtain course runs.
+        for course_run in [course_key.run]:
+            course_filter = block_filter.copy()
+            for filter_id in dashboard["course_filter_ids"]:


What iff course_filter_id in not present?

Same as block_filter_ids, I think it makes sense to require it to be specified, especially as it makes less sense to have a dashboard where data is not filtered to the current course.

farhaanbukhsh · 2025-04-15T12:50:13Z

platform_plugin_aspects/views.py

+
+        block_filter = {}
+        if not isinstance(usage_key, CourseKey):
+            for filter_id in dashboard["block_filter_ids"]:


Suggested change

for filter_id in dashboard["block_filter_ids"]:

for filter_id in dashboard.get("block_filter_ids"):

what about this?

@farhaanbukhsh Any block-specific dashboard (including subsection ones) should require this configuration, the course-wide dashboard being the only exception. I think it makes more sense to fail than to silently ignore a misconfiguration.

Embedded dashboard configuration will be operator-extensible in the future e.g. for custom XBlocks.

bmtcril

Looking great overall, just a few comments!

bmtcril · 2025-04-15T14:03:22Z

platform_plugin_aspects/utils.py

+    """
+    try:
+        user_language = (
+            get_model("user_preference").get_value(user, "pref-lang") or "en"


Eventually we would like to make the default language configurable, but in the meantime can we define en as a constant so there are fewer places to change it later?

bmtcril · 2025-04-15T14:06:21Z

platform_plugin_aspects/utils.py

+
+def build_filter(filter_id, col, op, val):
+    """Build a Superset native filter option."""
+    return {


Nit: this comment's format doesn't match the format of the others in the file.

Can you also put in a description of what the filter values mean? Superset's col, op, and val are not very intuitive.

bmtcril · 2025-04-15T14:18:58Z

platform_plugin_aspects/views.py

+    CourseOverview = get_model("course_overviews")
+    if CourseOverview:
+        try:
+            course_overview = CourseOverview.objects.get(id=course_key)


CourseOverview is a pretty big model with a lot of fields, if you only care about display_name you could do (fake code, but should be close):

course_overview = CourseOverview.objects.get(id=course_key).only("display_name") to get the model with only that field populated or

display_name = CourseOverview.objects.get(id=course_key).values("display_name")["display_name"] to skip all of the model code and just get return the name.

This matters more now with likely increase in how often this code will be called.

bmtcril · 2025-04-15T14:22:32Z

platform_plugin_aspects/views.py

        course = self.get_object()

-        dashboards = settings.ASPECTS_INSTRUCTOR_DASHBOARDS
+        dashboards = settings.ASPECTS_INSTRUCTOR_DASHBOARDS[:]


Is the [:] needed here? If it's just for the copy, can you use an explicit settings.ASPECTS_INSTRUCTOR_DASHBOARDS.copy() for readability?

bmtcril · 2025-04-15T14:32:56Z

platform_plugin_aspects/views.py

+                    filter_id, course_key_col, "IN", [str(course_key)]
+                )
+            course_runs[course_run] = {
+                "native_filters": prison.dumps(course_filter),


This usage of prison probably deserves a comment since I think it's the only place in this repo where it's used.

farhaanbukhsh · 2025-04-22T09:49:18Z

@ArturGaspar is this pr complete?

ArturGaspar · 2025-04-22T15:21:56Z

@farhaanbukhsh Yes.

bmtcril

Looks good, thanks for getting this done. There are a couple of outstanding comments, but I don't think any of them are blocking.

farhaanbukhsh · 2025-04-22T19:08:08Z

@bmtcril can we merge this?

bmtcril · 2025-04-22T19:59:17Z

All set, merged and released as v1.1.0!

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 20, 2025

openedx-webhooks added this to Contributions Mar 20, 2025

github-project-automation bot moved this to Needs Triage in Contributions Mar 20, 2025

ArturGaspar marked this pull request as draft March 20, 2025 10:40

ArturGaspar force-pushed the artur/embedded-dashboards branch from 8be5718 to 4abec48 Compare March 20, 2025 10:54

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Mar 24, 2025

0x29a reviewed Mar 28, 2025

View reviewed changes

ArturGaspar force-pushed the artur/embedded-dashboards branch 3 times, most recently from ea229ae to 1bb1aa4 Compare April 2, 2025 13:24

ArturGaspar changed the title ~~feat(wip): API endpoint for embedded Supserset dashboards~~ feat: API endpoint for embedded Supserset dashboards Apr 2, 2025

ArturGaspar changed the title ~~feat: API endpoint for embedded Supserset dashboards~~ feat: API endpoint for in-context Supserset dashboards Apr 2, 2025

ArturGaspar force-pushed the artur/embedded-dashboards branch from 1bb1aa4 to 15e8edd Compare April 3, 2025 11:17

ArturGaspar marked this pull request as ready for review April 3, 2025 11:17

ArturGaspar force-pushed the artur/embedded-dashboards branch 5 times, most recently from df53d40 to 8d8d996 Compare April 3, 2025 12:37

tecoholic mentioned this pull request Apr 14, 2025

feat: implement scrolling to XBlocks when browsing metrics openedx/frontend-plugin-aspects#24

Merged

8 tasks

farhaanbukhsh approved these changes Apr 15, 2025

View reviewed changes

bmtcril reviewed Apr 15, 2025

View reviewed changes

ArturGaspar force-pushed the artur/embedded-dashboards branch from f5f1d9e to ae27d9b Compare April 15, 2025 16:33

ArturGaspar force-pushed the artur/embedded-dashboards branch 3 times, most recently from d3f1ebe to c38633f Compare April 22, 2025 15:18

ArturGaspar force-pushed the artur/embedded-dashboards branch from c38633f to fa4aceb Compare April 22, 2025 15:24

ArturGaspar added 2 commits April 22, 2025 12:24

feat: API endpoint for embedding in-context Supserset dashboards

7eff999

refactor: use constant for default user language

f1bd765

ArturGaspar force-pushed the artur/embedded-dashboards branch from fa4aceb to f1bd765 Compare April 22, 2025 15:24

bmtcril approved these changes Apr 22, 2025

View reviewed changes

bmtcril merged commit 94ffd9a into openedx:main Apr 22, 2025
13 checks passed

github-project-automation bot moved this from Waiting on Author to Done in Contributions Apr 22, 2025

	for filter_id in dashboard["block_filter_ids"]:
	for filter_id in dashboard.get("block_filter_ids"):

feat: API endpoint for in-context Supserset dashboards #136

feat: API endpoint for in-context Supserset dashboards #136

Uh oh!

Conversation

ArturGaspar commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Deadline

Other information

Uh oh!

openedx-webhooks commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

farhaanbukhsh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArturGaspar Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArturGaspar Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmtcril left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

farhaanbukhsh commented Apr 22, 2025

Uh oh!

ArturGaspar commented Apr 22, 2025

Uh oh!

bmtcril left a comment

Choose a reason for hiding this comment

Uh oh!

farhaanbukhsh commented Apr 22, 2025

Uh oh!

Uh oh!

bmtcril commented Apr 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ArturGaspar commented Mar 20, 2025 •

edited

Loading

openedx-webhooks commented Mar 20, 2025 •

edited

Loading

ArturGaspar Apr 15, 2025 •

edited

Loading

ArturGaspar Apr 15, 2025 •

edited

Loading