feat: Enable drilling in embedded by Vitor-Avila · Pull Request #34319 · apache/superset

Vitor-Avila · 2025-07-25T15:55:07Z

SUMMARY

This PR exposes drilling capabilities to embedded users. Highlights:

New API endpoint
A new API endpoint was created for drilling: /api/v1/dataset/{{pk}}/drill_info/. Previously, the drilling context (columns to drill, dataset info to display in the useDatasetMetadataBar etc) were loaded from /api/v1/dataset/{{pk}}. Exposing this entire endpoint to embedded users is not ideal, as it exposes a lot of metadata.

Server-side validations

Previously, the backend would return all dataset columns, and then the client would filter out columns that have groupby=False. This would allow embedded users to find out names of other columns that should not be exposed, so the list of columns is now filtered in the backend.
Specifically for embedded users, there's also server-side validation to prevent a user to manipulate the API call and drill by a column that has groupby=False.
Implemented additional validation to make sure that embedded users can only drill datasets that are associated with a dashboard they have access to.

Bonus

This PR also partially fixes drilling for DASHBOARD_RBAC. Full fix might require migrating /datasource/samples to /api/v1/.
Improves performance by reducing repetitive calls. The call to load the drill context was moved up to ChartContextMenu and it's passed to DrillDetail, DrillBy and useDatasetMetadataBar.
The new API endpoint relies on FAB's select_columns, to filter the returned columns at the SQL query level (not at the response level) which also improves performance.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Embedded experience

Screen.Recording.2025-07-28.at.4.49.39.PM.mov

Highlights:

API response just includes the list of columns and dataset ID.
The metadata bar is not exposed to avoid sharing additional context about the underlying data.

Non-embedded experience

Screen.Recording.2025-07-29.at.1.13.04.AM.mov

Highlights:

New endpoint to expose drill-related info.
Less API calls to load data.

TESTING INSTRUCTIONS

Test coverage added. For manual testing:

Make sure the embedded role has the required permissions to perform drilling operations.
Load a dashboard in embedded mode.
Validate the right-click menu on a chart exposes the drilling options.
Confirm the dialogs don't expose additional information or not useful buttons.

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Missing Required Field Specification in Schema ▹ view
	Missing Null Safety Checks in post_dump ▹ view	🧠 Not in scope
	Unmemoized Length Calculation ▹ view	🧠 Not in scope
	Undocumented mock dataset structure ▹ view	🧠 Not in standard
	Unclear embedded context comment ▹ view	🧠 Not in standard
	Missing Column Filtering Logic ▹ view	🧠 Incorrect
	Feature Flag Not Alphabetically Ordered ▹ view	🧠 Not in standard
	Incomplete State Handling in Embedded Mode ▹ view	🧠 Not in scope
	Insufficient Error Context ▹ view	🧠 Not in scope
	Unjustified CSRF Protection Bypass ▹ view	🧠 Not in scope

Files scanned

File Path	Reviewed
superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.skipped-stories.tsx	✅
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts	✅
superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx	✅
superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx	✅
superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx	✅
superset/views/datasource/views.py	✅
superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx	✅
superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx	✅
superset/datasets/schemas.py	✅
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx	✅
superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx	✅
superset/datasets/api.py	✅
superset/config.py	✅
superset/security/manager.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx

-  const showSearch =
-    loadDrillByOptions || columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
+  const columns = dataset ? ensureIsArray(dataset.columns) : [];
+  const showSearch = columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;


superset-frontend/src/features/datasets/metadataBar/DatasetMetadataBar.skipped-stories.tsx

  SupersetClient.configure({ csrfToken: '1234' }).init();
-  const { metadataBar } = useDatasetMetadataBar({ datasetId: 1 });
+
+  const mockDataset = {


superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx

-  }, [datasetId, datasetProps]);

  const metadataBar = useMemo(() => {
+    // Short-circuit for embedded users - they don't need metadata bar


superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx

  const ref = useRef<InputRef>(null);
-  const showSearch =
-    loadDrillByOptions || columns.length > SHOW_COLUMNS_SEARCH_THRESHOLD;
+  const columns = dataset ? ensureIsArray(dataset.columns) : [];


superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts

  EmbeddableCharts = 'EMBEDDABLE_CHARTS',
  EmbeddedSuperset = 'EMBEDDED_SUPERSET',
  EnableAdvancedDataTypes = 'ENABLE_ADVANCED_DATA_TYPES',
+  DrillingInEmbedded = 'DRILLING_IN_EMBEDDED',


korbit-ai · 2025-07-25T16:02:29Z

superset/datasets/schemas.py

+get_drill_info_schema = {
+    "type": "object",
+    "properties": {
+        "dashboard_id": {"type": "integer"},
+    },
+}


Missing Required Field Specification in Schema

Tell me more

What is the issue?

The schema for dashboard_id in get_drill_info_schema lacks the 'required' field specification

Why this matters

Without specifying if dashboard_id is required, the API may accept invalid requests without this crucial parameter, potentially causing drill functionality to fail

Suggested change ∙ Feature Preview

Add the 'required' field to explicitly define whether dashboard_id is mandatory:

get_drill_info_schema = { "type": "object", "properties": { "dashboard_id": {"type": "integer"}, }, "required": ["dashboard_id"] }

Provide feedback to improve future suggestions

_{💬 Looking for more details? Reply to this comment to chat with Korbit.}

The parameter is only required for limited roles -- it works for admins or users with proper dataset acces without this param.

I see - since dashboard_id is only required for limited roles but optional for admins/users with dataset access, keeping it as an optional parameter in the schema makes sense. Thanks for clarifying.

superset/datasets/schemas.py

+    @post_dump(pass_original=True)
+    def post_dump(
+        self, serialized: dict[str, Any], obj: SqlaTable, **kwargs: Any
+    ) -> dict[str, Any]:


superset/config.py

 WTF_CSRF_EXEMPT_LIST = [
-    "superset.views.core.log",
-    "superset.views.core.explore_json",
    "superset.charts.data.api.data",
    "superset.dashboards.api.cache_dashboard_screenshot",
+    "superset.views.core.explore_json",
+    "superset.views.core.log",
+    "superset.views.datasource.views.samples",
 ]


superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx

+    // short circuit if the user is embedded as explore is not available
+    if (isEmbedded()) return;


superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx

+      } catch (error) {
+        logging.error('Failed to load dataset:', error);
+        supersetGetCache.delete(`/api/v1/dataset/${datasetId}/drill_info/`);


michael-s-molina · 2025-07-25T16:57:17Z

@Vitor-Avila Do we really need a new feature flag? Should this be a dashboard property such cross filters?

Vitor-Avila · 2025-07-25T17:19:01Z

@Vitor-Avila Do we really need a new feature flag? Should this be a dashboard property such cross filters?

Hmm open to suggestions here. We could potentially fully remove the FF and rely solely on validating if the role has the required perms to D2D. Would you say that's better?

michael-s-molina · 2025-07-25T17:22:46Z

Hmm open to suggestions here. We could potentially fully remove the FF and rely solely on validating if the role has the required perms to D2D. Would you say that's better?

Definitely. Every time we introduce a new feature flag we increase the complexity and maintainability of the codebase. In this case, permissions seem the natural way. @rusackas pointed out that we could also have readonly embedded dashboards, which in this case a configuration like we have for cross-filtering would be more appropriate. Either way works for me and it's better than adding a new feature flag.

Vitor-Avila · 2025-07-25T17:36:12Z

Hmm open to suggestions here. We could potentially fully remove the FF and rely solely on validating if the role has the required perms to D2D. Would you say that's better?

Definitely. Every time we introduce a new feature flag we increase the complexity and maintainability of the codebase. In this case, permissions seem the natural way. @rusackas pointed out that we could also have readonly embedded dashboards, which in this case a configuration like we have for cross-filtering would be more appropriate. Either way works for me and it's better than adding a new feature flag.

Awesome -- I'll work on moving to perm-based validation only and remove the FF. I would prefer on that because then we can potentially scope it by user, as opposed to by dashboard. Users can decide what's the desired perms for their embedded users, and if they need it dynamically they could have two roles and dynamically change the role assigned. In the future, if drilling can be disabled at the dashboard level, we can then respect that too in the embedded context (like a dashboard-level override) that could potentially be controlled via the embedded SDK too.

Vitor-Avila · 2025-07-29T03:54:36Z

@michael-s-molina I've changed the approach here to remove the FF. Could you please take a look? 🙏

Vitor-Avila · 2025-08-01T15:10:17Z

superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx

-  useEffect(() => {
-    async function loadOptions() {
-      const datasetId = Number(formData.datasource.split('__')[0]);
-      try {
-        setIsLoadingColumns(true);
-        let response: JsonResponse;
-        if (loadDrillByOptions) {
-          response = await loadDrillByOptions(datasetId, formData);
-        } else {
-          response = await cachedSupersetGet({
-            endpoint: `/api/v1/dataset/${datasetId}?q=${queryString}`,
-          });
-        }
-        const { json } = response;
-        const { result } = json;
-        setDataset(result);
-        setColumns(
-          ensureIsArray(result.columns)
-            .filter(column => column.groupby)
-            .filter(
-              column =>
-                !ensureIsArray(
-                  formData[drillByConfig?.groupbyFieldName ?? ''],
-                ).includes(column.column_name) &&
-                column.column_name !== formData.x_axis &&
-                ensureIsArray(excludedColumns)?.every(
-                  excludedCol => excludedCol.column_name !== column.column_name,
-                ),
-            ),
-        );
-      } catch (error) {
-        logging.error(error);
-        supersetGetCache.delete(`/api/v1/dataset/${datasetId}`);
-        addDangerToast(t('Failed to load dimensions for drill by'));
-      } finally {
-        setIsLoadingColumns(false);
-      }
-    }
-    if (handlesDimensionContextMenu && hasDrillBy) {
-      loadOptions();
-    }
-  }, [
-    addDangerToast,
-    drillByConfig?.groupbyFieldName,
-    excludedColumns,
-    formData,
-    handlesDimensionContextMenu,
-    hasDrillBy,
-  ]);
-


Logic moved to ChartContextMenu

Vitor-Avila · 2025-08-01T15:18:45Z

superset-frontend/src/features/datasets/metadataBar/useDatasetMetadataBar.tsx

-  useEffect(() => {
-    if (!datasetProps && datasetId) {
-      cachedSupersetGet({
-        endpoint: `/api/v1/dataset/${datasetId}`,
-      })
-        .then(({ json: { result } }) => {
-          setResult(result);
-          setStatus(ResourceStatus.Complete);
-        })
-        .catch(() => {
-          setStatus(ResourceStatus.Error);
-        });
-    }
-  }, [datasetId, datasetProps]);


Dataset fetched in ChartContextMenu and passed through

Vitor-Avila · 2025-08-01T15:19:26Z

superset/config.py

+    "superset.views.core.explore_json",
+    "superset.views.core.log",
+    "superset.views.datasource.views.samples",


Just really adding "superset.views.datasource.views.samples". Moved the other two to keep the list alphabetically sorted

Vitor-Avila · 2025-08-01T15:24:37Z

tests/integration_tests/base_tests.py

+            if clone_user.roles:
+                for role in clone_user.roles:
+                    pvms.extend(role.permissions)


Moving away from doing temp_user.roles = clone_user.roles because now we support pvms_to_remove and we don't want to remove pvms from the copied role.

Vitor-Avila · 2025-08-01T15:25:04Z

tests/integration_tests/base_tests.py

-        if clone_user:
-            temp_user.roles = clone_user.roles
-


this was redundant here, unsure if there was a purpose but tests still work without it

eschutho · 2025-08-01T17:26:44Z

UPDATING.md

 - [32317](https://github.com/apache/superset/pull/32317) The horizontal filter bar feature is now out of testing/beta development and its feature flag `HORIZONTAL_FILTER_BAR` has been removed.
 - [31590](https://github.com/apache/superset/pull/31590) Marks the begining of intricate work around supporting dynamic Theming, and breaks support for [THEME_OVERRIDES](https://github.com/apache/superset/blob/732de4ac7fae88e29b7f123b6cbb2d7cd411b0e4/superset/config.py#L671) in favor of a new theming system based on AntD V5. Likely this will be in disrepair until settling over the 5.x lifecycle.
 - [32432](https://github.com/apache/superset/pull/31260) Moves the List Roles FAB view to the frontend and requires `FAB_ADD_SECURITY_API` to be enabled in the configuration and `superset init` to be executed.
+- [34319](https://github.com/apache/superset/pull/34319) Drill to Detail and Drill By is now supported in Embedded mode, and also with the `DASHBOARD_RBAC` FF. If you don't want to expose these features in Embedded / `DASHBOARD_RBAC`, make sure the roles used for Embedded / `DASHBOARD_RBAC`don't have the required permissions to perform D2D actions.


Can we highlight what the roles are to turn this on/off?

@eschutho I think these are configureable by deployment, so unsure if we could highlight it. IIRC the embedded role is set in superset_config.py via the GUEST_ROLE_NAME config, then for DASHBOARD_RBAC access users can use any role they had created in the Superset UI.
Let me know if you want me to rephrase this in a better way.

eschutho · 2025-08-01T17:28:23Z

superset/security/manager.py

+        self, dataset: "BaseDatasource", dashboard: "Dashboard"
+    ) -> bool:
+        """
+        Return True if an embedded user or DASHBOARD_RBAC user can drill a dataset.


@Vitor-Avila since drilling exposes more data, is there a way to disable this feature for embedded users if they don't want to turn it on?

@eschutho initially I had a FF to gate this, but was advised against here : #34319 (comment)

This will still be controlled by regular role perm. If the embedded role has can_drill_info on Dataset they'll have access to this endpoint, otherwise they won't have it. Then the UI also checks for the regular permissions to show (or not) the Drill by and Drill to Detail options.

betodealmeida

This is such a great PR, from the summary to the tests, passing through the code! Approved, but left a few minor comments.

betodealmeida · 2025-08-04T19:20:43Z

superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx

+
+    fetchDataset();
+  }, [
+    visible,


Don't we need to add dataset here as well?

I've thought so too and checked with Claude and it advised not to, because this block also does setDataset(result) so if dataset was a dependency it would re-render one more time as the dataset value would change with setDataset(result).

From my manual tests it's working properly.

betodealmeida · 2025-08-04T19:33:02Z

superset/security/manager.py

                            and slc in dashboard_.slices
                            and slc.datasource == datasource
                        )
+                        or (


Oh my, what is this monster expression stating on line 2379?!? 😱

We should break this down into methods that are self-explanatotry, like:

return ( self.is_d2d_allowed(form_data) or self.is_dashbaord_rbac(form_data) or ... )

(Not saying you should do it, but you could start by moving the code you're adding into a method and call that instead here.)

Great idea! Will do with this new block and then open a dedicated follow up later

pull-request-size bot added the size/XXL label Jul 25, 2025

github-actions bot added api Related to the REST API packages labels Jul 25, 2025

dosubot bot added dashboard:drill-by embedded labels Jul 25, 2025

korbit-ai bot reviewed Jul 25, 2025

View reviewed changes

michael-s-molina requested review from michael-s-molina and rusackas July 25, 2025 16:58

michael-s-molina added the review:checkpoint Last PR reviewed during the daily review standup label Jul 25, 2025

Vitor-Avila force-pushed the feat/d2d-in-embedded branch from 5382fd6 to 6edf7e4 Compare July 29, 2025 03:54

github-actions bot removed embedded packages labels Jul 29, 2025

sadpandajoe removed the review:checkpoint Last PR reviewed during the daily review standup label Jul 30, 2025

Vitor-Avila requested review from betodealmeida, eschutho, geido, mistercrunch and sadpandajoe as code owners July 31, 2025 03:16

Vitor-Avila force-pushed the feat/d2d-in-embedded branch 2 times, most recently from 9b09030 to c994fa2 Compare August 1, 2025 00:30

Vitor-Avila added 5 commits August 1, 2025 01:38

feat: Enable drilling in embedded

348bd89

Addressing PR feedback and removing FF

b5c7d93

Fixing tests

8d0d9e6

Fixing backend tests

636f2cd

Try fixing frontend tests

1e3eb37

Fixing more tests

59fbd99

Vitor-Avila force-pushed the feat/d2d-in-embedded branch from 1b09b50 to 59fbd99 Compare August 1, 2025 04:40

Vitor-Avila added 3 commits August 1, 2025 02:02

Fixing another test

1e3bd0a

Fixing another integration test

16323e6

Moving schema property to the top of

77b4881

Vitor-Avila commented Aug 1, 2025

View reviewed changes

Minor improvements

0a6ee76

eschutho reviewed Aug 1, 2025

View reviewed changes

Validating new perm before showing D2D menus

df06ce4

betodealmeida approved these changes Aug 4, 2025

View reviewed changes

Breaking down as suggested

88083ec

Vitor-Avila merged commit 49689ee into master Aug 5, 2025
59 checks passed

Vitor-Avila deleted the feat/d2d-in-embedded branch August 5, 2025 05:23

Vitor-Avila mentioned this pull request Aug 8, 2025

fix: Use labels in Drill to Detail #34620

Merged

9 tasks

LisaHusband pushed a commit to LisaHusband/superset that referenced this pull request Aug 17, 2025

feat: Enable drilling in embedded (apache#34319)

2dd8373

dosubot bot mentioned this pull request Sep 24, 2025

Bug: Drill to Detail / Drill by Not Working Across Charts from Jinja-Based Datasets #35263

Open

3 tasks

github-actions bot added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 6.0.0 First shipped in 6.0.0 labels Dec 18, 2025

		// short circuit if the user is embedded as explore is not available
		if (isEmbedded()) return;

Comments

Conversation

Vitor-Avila commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

korbit-ai bot Jul 25, 2025

Choose a reason for hiding this comment

Missing Required Field Specification in Schema

What is the issue?

Why this matters

Suggested change ∙ Feature Preview

Provide feedback to improve future suggestions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

korbit-ai bot Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

michael-s-molina commented Jul 25, 2025

Uh oh!

Vitor-Avila commented Jul 25, 2025

Uh oh!

michael-s-molina commented Jul 25, 2025

Uh oh!

Vitor-Avila commented Jul 25, 2025

Uh oh!

Vitor-Avila commented Jul 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vitor-Avila Aug 1, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vitor-Avila Aug 1, 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!

Choose a reason for hiding this comment

Uh oh!

betodealmeida left a comment

Choose a reason for hiding this comment

Vitor-Avila commented Jul 25, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

Vitor-Avila Aug 1, 2025 •

edited

Loading

Vitor-Avila Aug 1, 2025 •

edited

Loading