Compare commits

...

4 Commits

Author SHA1 Message Date
Beto Dealmeida
a664294c9f Address comments 2026-07-06 15:21:52 -04:00
Beto Dealmeida
e10d2a7013 Address comments 2026-07-06 13:49:32 -04:00
Beto Dealmeida
046a589c13 Also disable drill-to-detail 2026-07-06 13:49:32 -04:00
Beto Dealmeida
b5e1aefa2c feat(semantic layers): don't show samples tab in explore 2026-07-06 13:49:32 -04:00
19 changed files with 396 additions and 27 deletions

View File

@@ -221,6 +221,12 @@ test('should render the error', async () => {
.spyOn(SupersetClient, 'post')
.mockRejectedValue(new Error('Something went wrong'));
await waitForRender();
// The error is wrapped in an Alert component with a stable headline and the
// raw error text in the description — no more bare ``<pre>`` elements.
expect(await screen.findByRole('alert')).toBeVisible();
expect(
await screen.findByText('Failed to load drill-to-detail rows'),
).toBeVisible();
expect(screen.getByText('Error: Something went wrong')).toBeInTheDocument();
});

View File

@@ -42,6 +42,7 @@ import BooleanCell from '@superset-ui/core/components/Table/cell-renderers/Boole
import NullCell from '@superset-ui/core/components/Table/cell-renderers/NullCell';
import TimeCell from '@superset-ui/core/components/Table/cell-renderers/TimeCell';
import { EmptyState, Loading } from '@superset-ui/core/components';
import { Alert } from '@apache-superset/core/components';
import { getDatasourceSamples } from 'src/components/Chart/chartAction';
import Table, {
ColumnsType,
@@ -362,13 +363,18 @@ export default function DrillDetailPane({
if (responseError) {
// Render error if page download failed
tableContent = (
<pre
<div
css={css`
margin-top: ${theme.sizeUnit * 4}px;
`}
>
{responseError}
</pre>
<Alert
type="error"
showIcon
message={t('Failed to load drill-to-detail rows')}
description={responseError}
/>
</div>
);
} else if (bootstrapping) {
// Render loading if first page hasn't loaded

View File

@@ -50,6 +50,7 @@ const DISABLED_REASONS = {
DATABASE: t(
'Drill to detail is disabled for this database. Change the database settings to enable it.',
),
DATASOURCE: t('Drill to detail is not available for this datasource type.'),
NO_AGGREGATIONS: t(
'Drill to detail is disabled because this chart does not group data by dimension value.',
),
@@ -116,6 +117,17 @@ export const useDrillDetailMenuItems = ({
datasources[formData.datasource]?.database?.disable_drill_to_detail,
);
// Capability flag on the datasource itself. Datasources that don't model
// raw rows (e.g. semantic views) opt out via ``supports_drill_to_detail``
// in the explore data payload.
const datasourceSupportsDrillToDetail = useSelector<
RootState,
boolean | undefined
>(
({ datasources }) =>
datasources[formData.datasource]?.supports_drill_to_detail,
);
const openModal = useCallback(
(filters: BinaryQueryObjectFilterClause[], event: MouseEvent) => {
onClick(event);
@@ -158,7 +170,10 @@ export const useDrillDetailMenuItems = ({
let drillDisabled;
let drillByDisabled;
if (drillToDetailDisabled) {
if (datasourceSupportsDrillToDetail === false) {
drillDisabled = DISABLED_REASONS.DATASOURCE;
drillByDisabled = DISABLED_REASONS.DATASOURCE;
} else if (drillToDetailDisabled) {
drillDisabled = DISABLED_REASONS.DATABASE;
drillByDisabled = DISABLED_REASONS.DATABASE;
} else if (handlesDimensionContextMenu) {

View File

@@ -444,3 +444,45 @@ test('context menu renders <NULL> for null dimension values', async () => {
await expectDrillToDetailByEnabled();
await expectDrillToDetailByDimension(filterNull);
});
const buildStateWithUnsupportedDatasource = () => {
const baseState = getMockStoreWithNativeFilters().getState();
const datasourceKey = defaultFormData.datasource as string;
return {
...baseState,
datasources: {
...baseState.datasources,
[datasourceKey]: {
...baseState.datasources[datasourceKey],
supports_drill_to_detail: false,
},
},
};
};
test('dropdown menu when datasource opts out via supports_drill_to_detail=false', async () => {
cleanup();
render(<MockRenderChart formData={defaultFormData} />, {
useRouter: true,
useRedux: true,
initialState: buildStateWithUnsupportedDatasource(),
});
await expectDrillToDetailDisabled(
'Drill to detail is not available for this datasource type.',
);
await expectNoDrillToDetailBy();
});
test('context menu when datasource opts out via supports_drill_to_detail=false', async () => {
cleanup();
render(<MockRenderChart formData={defaultFormData} isContextMenu />, {
useRouter: true,
useRedux: true,
initialState: buildStateWithUnsupportedDatasource(),
});
const message = 'Drill to detail is not available for this datasource type.';
await expectDrillToDetailDisabled(message);
await expectDrillToDetailByDisabled(message);
});

View File

@@ -231,6 +231,10 @@ export type Datasource = Dataset & {
column_types: GenericDataType[];
table_name: string;
database?: Database;
/** False when the datasource can't return row samples (e.g. semantic views). */
supports_samples?: boolean;
/** False when the datasource can't answer drill-to-detail requests. */
supports_drill_to_detail?: boolean;
};
export type DatasourcesState = {
[key: string]: Datasource;

View File

@@ -234,25 +234,44 @@ export const DataTablesPane = ({
}
}, [resultsTabFallback]);
// Hide the Samples tab for datasources that don't expose raw rows
// (e.g. semantic views). The check is intentionally ``=== false`` so that
// datasources from older backends that don't send the flag still show the
// tab and preserve current behavior.
const showSamplesTab = datasource?.supports_samples !== false;
// If the datasource swaps to one that doesn't support samples while the
// Samples tab is active (e.g. the user picks a semantic view), the tab
// disappears from ``tabItems`` and ``activeTabKey`` is orphaned. Fall back
// to Results so the panel keeps rendering content.
useEffect(() => {
if (!showSamplesTab && activeTabKey === ResultTypes.Samples) {
setActiveTabKey(ResultTypes.Results);
}
}, [showSamplesTab, activeTabKey]);
const tabItems = [
...queryResultsPanes,
{
key: ResultTypes.Samples,
label: t('Samples'),
children: (
<StyledDiv>
<SamplesPane
datasource={datasource}
queryFormData={queryFormData}
queryForce={queryForce}
isRequest={isRequest.samples}
setForceQuery={setForceQuery}
isVisible={ResultTypes.Samples === activeTabKey}
canDownload={canDownload}
/>
</StyledDiv>
),
},
...(showSamplesTab
? [
{
key: ResultTypes.Samples,
label: t('Samples'),
children: (
<StyledDiv>
<SamplesPane
datasource={datasource}
queryFormData={queryFormData}
queryForce={queryForce}
isRequest={isRequest.samples}
setForceQuery={setForceQuery}
isVisible={ResultTypes.Samples === activeTabKey}
canDownload={canDownload}
/>
</StyledDiv>
),
},
]
: []),
];
return (

View File

@@ -21,6 +21,7 @@ import { t } from '@apache-superset/core/translation';
import { ensureIsArray } from '@superset-ui/core';
import { datasetLabelLower } from 'src/features/semanticLayers/label';
import { styled } from '@apache-superset/core/theme';
import { Alert } from '@apache-superset/core/components';
import { EmptyState, Loading } from '@superset-ui/core/components';
import { GenericDataType } from '@apache-superset/core/common';
import { GridTable } from 'src/components/GridTable';
@@ -35,7 +36,7 @@ import {
import { TableControls, ROW_LIMIT_OPTIONS } from './DataTableControls';
import { SamplesPaneProps } from '../types';
const Error = styled.pre`
const ErrorAlertWrapper = styled.div`
margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`};
`;
@@ -155,7 +156,14 @@ export const SamplesPane = ({
rowLimitOptions={ROW_LIMIT_OPTIONS}
onRowLimitChange={handleRowLimitChange}
/>
<Error>{responseError}</Error>
<ErrorAlertWrapper>
<Alert
type="error"
showIcon
message={t('Failed to load samples')}
description={responseError}
/>
</ErrorAlertWrapper>
</>
);
}

View File

@@ -27,13 +27,14 @@ import {
QueryData,
} from '@superset-ui/core';
import { styled } from '@apache-superset/core/theme';
import { Alert } from '@apache-superset/core/components';
import { EmptyState, Loading } from '@superset-ui/core/components';
import { getChartDataRequest } from 'src/components/Chart/chartAction';
import { ResultsPaneProps, QueryResultInterface } from '../types';
import { SingleQueryResultPane } from './SingleQueryResultPane';
import { TableControls, ROW_LIMIT_OPTIONS } from './DataTableControls';
const Error = styled.pre`
const ErrorAlertWrapper = styled.div`
margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`};
`;
@@ -199,7 +200,14 @@ export const useResultsPane = ({
isLoading={false}
canDownload={canDownload}
/>
<Error>{responseError}</Error>
<ErrorAlertWrapper>
<Alert
type="error"
showIcon
message={t('Failed to load results')}
description={responseError}
/>
</ErrorAlertWrapper>
</>
);
return Array(queryCount).fill(err);

View File

@@ -19,7 +19,12 @@
import fetchMock from 'fetch-mock';
import { FeatureFlag } from '@superset-ui/core';
import * as copyUtils from 'src/utils/copy';
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import {
render,
screen,
userEvent,
waitFor,
} from 'spec/helpers/testing-library';
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
import { DataTablesPane } from '..';
@@ -89,6 +94,48 @@ describe('DataTablesPane', () => {
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
});
test('Hides Samples tab when datasource opts out via supports_samples=false', async () => {
const props = createDataTablesPaneProps(0);
const propsWithoutSamples = {
...props,
datasource: { ...props.datasource, supports_samples: false },
};
render(<DataTablesPane {...propsWithoutSamples} />, { useRedux: true });
expect(await screen.findByText('Results')).toBeVisible();
expect(screen.queryByText('Samples')).not.toBeInTheDocument();
});
test('Falls back to Results when active Samples tab disappears mid-session', async () => {
// Regression for codeant Major finding on PR #41509: a datasource swap
// that hides the Samples tab while it was the active tab used to leave
// ``activeTabKey === 'samples'`` orphaned, rendering a blank panel.
const props = createDataTablesPaneProps(0);
const { rerender } = render(<DataTablesPane {...props} />, {
useRedux: true,
});
// Open the panel and pick the Samples tab.
userEvent.click(screen.getByLabelText('Expand data panel'));
userEvent.click(await screen.findByText('Samples'));
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
// Swap to a datasource that doesn't support samples (e.g. a semantic
// view). The Samples tab should disappear and the panel should land on
// Results with content still rendered.
rerender(
<DataTablesPane
{...props}
datasource={{ ...props.datasource, supports_samples: false }}
/>,
);
await waitFor(() => {
expect(screen.queryByText('Samples')).not.toBeInTheDocument();
});
expect(screen.getByText('Results')).toBeVisible();
// Panel stays expanded and renders Results content rather than going blank.
expect(screen.getByLabelText('Collapse data panel')).toBeVisible();
});
test('Should copy data table content correctly', async () => {
fetchMock.post(
'glob:*/api/v1/chart/data?form_data=%7B%22slice_id%22%3A456%7D',

View File

@@ -84,10 +84,14 @@ describe('SamplesPane', () => {
const props = createSamplesPaneProps({
datasourceId: 36,
});
const { findByText } = render(<SamplesPane {...props} />, {
const { findByText, findByRole } = render(<SamplesPane {...props} />, {
useRedux: true,
});
// The error is now rendered inside an Alert component, with a clear
// headline message and the raw error text as the description.
expect(await findByRole('alert')).toBeVisible();
expect(await findByText('Failed to load samples')).toBeVisible();
expect(await findByText('Error: Bad request')).toBeVisible();
});

View File

@@ -78,6 +78,18 @@ export type Datasource = Dataset & {
schema?: string;
is_sqllab_view?: boolean;
extra?: string | object;
/**
* False when the datasource (e.g. a semantic view) doesn't model raw rows
* and therefore can't return a row sample. Defaults to true on the server
* side; missing here means the explore UI keeps current behavior.
*/
supports_samples?: boolean;
/**
* False when the datasource doesn't model raw rows and therefore can't
* answer a drill-to-detail query. Tracked separately from
* ``supports_samples`` so the two capabilities can diverge.
*/
supports_drill_to_detail?: boolean;
};
export interface ExplorePageInitialData {

View File

@@ -230,6 +230,15 @@ def _get_drill_detail(
# todo(yongjie): Remove this function,
# when determining whether samples should be applied to the time filter.
datasource = _get_datasource(query_context, query_obj)
# Refuse for datasource types that don't model raw rows (e.g. semantic
# views). Mirrors the ``supports_samples`` gate on the ``/samples``
# endpoint so drill-detail is hard-blocked on the backend, not just
# hidden in the frontend menu. Defaults to ``True`` for any datasource
# class that doesn't explicitly opt out.
if not getattr(datasource, "supports_drill_to_detail", True):
raise QueryObjectValidationError(
_("Drill to detail is not available for this datasource type.")
)
query_obj = copy.copy(query_obj)
query_obj.is_timeseries = False
query_obj.metrics = None

View File

@@ -194,6 +194,16 @@ class BaseDatasource(
# Only some datasources support Row Level Security
is_rls_supported: bool = False
# Datasources that can return raw row samples (anything backed by a SQL
# table can; semantic-layer abstractions cannot, since they only expose
# pre-defined metrics and dimensions).
supports_samples: bool = True
# Datasources that can answer "drill to detail" requests — i.e. fetch the
# raw rows underlying a chart cell. Conceptually similar to ``samples``
# but kept as a separate capability so the two can diverge.
supports_drill_to_detail: bool = True
@property
def name(self) -> str:
# can be a Column or a property pointing to one
@@ -501,6 +511,8 @@ class BaseDatasource(
"owners": [owner.id for owner in self.owners],
"verbose_map": self.verbose_map,
"select_star": self.select_star,
"supports_samples": self.supports_samples,
"supports_drill_to_detail": self.supports_drill_to_detail,
}
def data_for_slices( # pylint: disable=too-many-locals # noqa: C901

View File

@@ -196,6 +196,12 @@ class SemanticView(AuditMixinNullable, Model):
__tablename__ = "semantic_views"
# Semantic views expose pre-defined metrics and dimensions, not raw rows,
# so neither the "Samples" tab in Explore nor the "Drill to detail"
# affordance from the chart 3-dots menu can return anything meaningful.
supports_samples: bool = False
supports_drill_to_detail: bool = False
# Use integer as the primary key for cross-database auto-increment
# compatibility (sa.Identity() is not supported in MySQL or SQLite).
# The uuid column is a secondary unique identifier used in URLs and perms.
@@ -393,6 +399,8 @@ class SemanticView(AuditMixinNullable, Model):
"filter_select_enabled": True,
"sql": None,
"select_star": None,
"supports_samples": self.supports_samples,
"supports_drill_to_detail": self.supports_drill_to_detail,
"owners": [],
"description": self.description,
"table_name": self.name,

View File

@@ -338,6 +338,11 @@ class ExplorableData(TypedDict, total=False):
extra: str | None
always_filter_main_dttm: bool
normalize_columns: bool
# Set by datasources that cannot return raw row samples (e.g. semantic
# views, which only expose pre-defined metrics and dimensions).
supports_samples: bool
# Set by datasources that cannot answer drill-to-detail requests.
supports_drill_to_detail: bool
VizData: TypeAlias = list[Any] | dict[Any, Any] | None

View File

@@ -208,6 +208,23 @@ class Datasource(BaseSupersetView):
payload = SamplesPayloadSchema().load(request.json)
except ValidationError as err:
return json_error_response(err.messages, status=400)
# Refuse early for datasource types that don't model raw rows
# (e.g. semantic views, which only expose pre-defined metrics and
# dimensions). Without this gate the request would still go through
# the standard query pipeline and fail with an opaque 500.
# ``supports_samples`` defaults to True for any datasource class that
# doesn't explicitly opt out, so SqlaTable/Query/SavedQuery continue
# to work without needing the attribute declared on each class.
ds_class = DatasourceDAO.sources.get(
DatasourceType(params["datasource_type"]),
)
if ds_class is not None and not getattr(ds_class, "supports_samples", True):
return json_error_response(
_("Samples are not available for this datasource type."),
status=400,
)
dashboard_id = None
if security_manager.is_guest_user():
if not params["dashboard_id"]:

View File

@@ -0,0 +1,70 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from unittest.mock import MagicMock, patch
import pytest
from superset.common.query_actions import _get_drill_detail
from superset.exceptions import QueryObjectValidationError
def test_get_drill_detail_refuses_datasource_that_opts_out() -> None:
"""
A datasource with ``supports_drill_to_detail = False`` (e.g. semantic
views) must be hard-blocked on the server. Without this gate the request
would fall through to ``_get_full`` and fail with an opaque error, and
the flag would only be enforced by the frontend menu — leaving the
chart-data API endpoint accepting drill-detail requests it shouldn't.
"""
datasource = MagicMock()
datasource.supports_drill_to_detail = False
query_obj = MagicMock()
query_obj.datasource = datasource
query_context = MagicMock()
with pytest.raises(
QueryObjectValidationError,
match="Drill to detail is not available",
):
_get_drill_detail(query_context, query_obj)
def test_get_drill_detail_allows_datasource_without_flag() -> None:
"""
Datasources that don't declare the flag (e.g. legacy ``SqlaTable``
subclasses via ``getattr`` default) must continue to work — the gate
only fires when the flag is explicitly ``False``.
"""
datasource = MagicMock(spec=["columns"])
column = MagicMock()
column.column_name = "id"
datasource.columns = [column]
query_obj = MagicMock()
query_obj.datasource = datasource
query_obj.columns = []
query_context = MagicMock()
expected_payload: dict[str, list[dict[str, str]]] = {"data": []}
with patch(
"superset.common.query_actions._get_full", return_value=expected_payload
) as mock_get_full:
assert _get_drill_detail(query_context, query_obj) is expected_payload
mock_get_full.assert_called_once()

View File

@@ -653,6 +653,20 @@ def test_semantic_view_data(
assert data["table_name"] == "Orders View"
assert data["datasource_name"] == "Orders View"
assert data["offset"] == 0
# Semantic views don't model raw rows, so neither samples nor
# drill-to-detail are available.
assert data["supports_samples"] is False
assert data["supports_drill_to_detail"] is False
def test_semantic_view_supports_samples_is_false() -> None:
"""The class-level flag opts SemanticView out of the Samples affordance."""
assert SemanticView.supports_samples is False
def test_semantic_view_supports_drill_to_detail_is_false() -> None:
"""The class-level flag opts SemanticView out of Drill to detail."""
assert SemanticView.supports_drill_to_detail is False
def test_semantic_view_get_query_result(

View File

@@ -312,3 +312,66 @@ def test_save_non_owner_with_owners_field_is_rejected(
raw_save(_view_self())
mock_security_manager.raise_for_ownership.assert_called_once_with(mock_orm)
# ---------------------------------------------------------------------------
# Datasource.samples
# ---------------------------------------------------------------------------
@patch("superset.views.datasource.views._", lambda s: s)
@patch("superset.views.datasource.views.get_samples")
@patch("superset.views.datasource.views.json_error_response")
@patch("superset.views.datasource.views.security_manager", new_callable=MagicMock)
def test_samples_returns_400_for_unsupported_datasource_type(
mock_security_manager: MagicMock,
mock_json_error_response: MagicMock,
mock_get_samples: MagicMock,
) -> None:
"""Semantic views can't return raw samples — endpoint should refuse with 400."""
from flask import Flask
mock_security_manager.is_guest_user.return_value = False
mock_json_error_response.return_value = "error-response"
raw_samples = _get_view_func("samples")
app = Flask(__name__)
with app.test_request_context(
"/datasource/samples?datasource_type=semantic_view&datasource_id=1",
method="POST",
json={},
):
result = raw_samples(_view_self())
assert result == "error-response"
mock_json_error_response.assert_called_once()
_, kwargs = mock_json_error_response.call_args
assert kwargs.get("status") == 400
# The bail-out must happen before any sample fetching is attempted.
mock_get_samples.assert_not_called()
@patch("superset.views.datasource.views.get_samples")
@patch("superset.views.datasource.views.security_manager", new_callable=MagicMock)
def test_samples_proceeds_for_supported_datasource_type(
mock_security_manager: MagicMock,
mock_get_samples: MagicMock,
) -> None:
"""A `query` datasource (supports_samples=True) bypasses the 400 short-circuit."""
from flask import Flask
mock_security_manager.is_guest_user.return_value = False
mock_get_samples.return_value = {"rows": []}
view = _view_self()
raw_samples = _get_view_func("samples")
app = Flask(__name__)
with app.test_request_context(
"/datasource/samples?datasource_type=query&datasource_id=1",
method="POST",
json={},
):
raw_samples(view)
mock_get_samples.assert_called_once()
view.json_response.assert_called_once_with({"result": {"rows": []}})