Compare commits

...

3 Commits

Author SHA1 Message Date
Beto Dealmeida
48958cc8ce Address comments 2026-06-29 15:59:14 -04:00
Beto Dealmeida
7dac6c1d4f Also disable drill-to-detail 2026-06-29 12:11:14 -04:00
Beto Dealmeida
a1297d10ac feat(semantic layers): don't show samples tab in explore 2026-06-26 17:12:37 -04:00
18 changed files with 317 additions and 51 deletions

View File

@@ -8438,9 +8438,6 @@
"arm64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8458,9 +8455,6 @@
"arm64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8478,9 +8472,6 @@
"ppc64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8498,9 +8489,6 @@
"riscv64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8518,9 +8506,6 @@
"riscv64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8538,9 +8523,6 @@
"s390x"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8558,9 +8540,6 @@
"x64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -8578,9 +8557,6 @@
"x64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [

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

@@ -49,6 +49,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.',
),
@@ -115,6 +116,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);
@@ -157,7 +169,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

@@ -426,3 +426,45 @@ test.skip('context menu for supported chart, dimensions, all filters', async ()
await setupMenu(filters);
await expectDrillToDetailByAll(filters);
});
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

@@ -197,25 +197,44 @@ export const DataTablesPane = ({
children: pane,
}));
// 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

@@ -25,13 +25,14 @@ import {
getClientErrorObject,
} 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`};
`;
@@ -157,7 +158,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

@@ -193,6 +193,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
@@ -500,6 +510,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

@@ -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": []}})