You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2025/08/14 14:52:58 UTC
(superset) branch master updated: fix: Invalid error tooltip if control label is function (#34698)
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 47874318df fix: Invalid error tooltip if control label is function (#34698)
47874318df is described below
commit 47874318dfde9819fbc19e345d2f2c011f1e47da
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Thu Aug 14 16:52:50 2025 +0200
fix: Invalid error tooltip if control label is function (#34698)
---
.../explore/components/ControlPanelsContainer.tsx | 1 +
.../ExploreViewContainer.test.tsx | 167 +++++++++++++++++++++
.../components/ExploreViewContainer/index.jsx | 6 +-
.../ReportModal/HeaderReportDropdown/index.tsx | 11 --
4 files changed, 173 insertions(+), 12 deletions(-)
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index 83524d102a..d18dd193be 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -700,6 +700,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
title={props.errorMessage}
>
<Icons.CloseCircleOutlined
+ data-test="query-error-tooltip-trigger"
iconColor={theme.colorErrorText}
iconSize="s"
/>
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
index aecb3c775d..8670ac27c4 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx
@@ -299,3 +299,170 @@ test('does omit hiddenFormData when query_mode is not enabled', async () => {
expect(formData[key]).toBeUndefined();
});
});
+
+// Component tests for the errorMessage behavior
+test('does not show error indicator when no controls have validation errors', async () => {
+ getChartControlPanelRegistry().registerValue('table', {
+ controlPanelSections: [],
+ });
+ const customState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ controls: {
+ ...reduxState.explore.controls,
+ metric: { value: 'count', label: 'Metric' },
+ groupby: { value: ['category'], label: 'Group by' },
+ },
+ },
+ };
+
+ renderWithRouter({ initialState: customState });
+
+ await waitFor(() => {
+ const errorIndicator = screen.queryByTestId('query-error-tooltip-trigger');
+ expect(errorIndicator).not.toBeInTheDocument();
+ });
+});
+
+test('shows error indicator when controls have validation errors', async () => {
+ getChartControlPanelRegistry().registerValue('table', {
+ controlPanelSections: [],
+ });
+ const customState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ controls: {
+ ...reduxState.explore.controls,
+ metric: {
+ value: '',
+ label: 'Metric',
+ validationErrors: ['Metric is required'],
+ },
+ },
+ },
+ };
+
+ renderWithRouter({ initialState: customState });
+
+ const errorIndicator = await screen.findByTestId(
+ 'query-error-tooltip-trigger',
+ );
+
+ userEvent.hover(errorIndicator);
+
+ const tooltip = await screen.findByRole('tooltip');
+ expect(tooltip).toBeInTheDocument();
+
+ const errorMessage = await screen.findByText(/Metric is required/);
+ expect(errorMessage).toBeInTheDocument();
+});
+
+test('shows error indicator for multiple controls with validation errors', async () => {
+ getChartControlPanelRegistry().registerValue('table', {
+ controlPanelSections: [],
+ });
+ const customState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ controls: {
+ ...reduxState.explore.controls,
+ metric: {
+ value: '',
+ label: 'Metric',
+ validationErrors: ['Field is required'],
+ },
+ groupby: {
+ value: [],
+ label: 'Group by',
+ validationErrors: ['Field is required'],
+ },
+ },
+ },
+ };
+
+ renderWithRouter({ initialState: customState });
+
+ const errorIndicator = await screen.findByTestId(
+ 'query-error-tooltip-trigger',
+ );
+
+ userEvent.hover(errorIndicator);
+
+ const tooltip = await screen.findByRole('tooltip');
+ expect(tooltip).toBeInTheDocument();
+
+ expect(await screen.findByText(/Field is required/)).toBeInTheDocument();
+});
+
+test('shows error indicator for control with multiple validation errors', async () => {
+ getChartControlPanelRegistry().registerValue('table', {
+ controlPanelSections: [],
+ });
+ const customState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ controls: {
+ ...reduxState.explore.controls,
+ metric: {
+ value: '',
+ label: 'Metric',
+ validationErrors: ['Field is required', 'Invalid format'],
+ },
+ },
+ },
+ };
+
+ renderWithRouter({ initialState: customState });
+
+ const errorIndicator = await screen.findByTestId(
+ 'query-error-tooltip-trigger',
+ );
+
+ userEvent.hover(errorIndicator);
+
+ const tooltip = await screen.findByRole('tooltip');
+ expect(tooltip).toBeInTheDocument();
+
+ expect(await screen.findByText(/Field is required/)).toBeInTheDocument();
+ expect(await screen.findByText(/Invalid format/)).toBeInTheDocument();
+});
+
+test('shows error indicator with function labels', async () => {
+ getChartControlPanelRegistry().registerValue('table', {
+ controlPanelSections: [],
+ });
+ // Ensure the explore state passed to the label function contains the expected property
+ const customState = {
+ ...reduxState,
+ explore: {
+ ...reduxState.explore,
+ someState: 'test',
+ controls: {
+ ...reduxState.explore.controls,
+ metric: {
+ value: '',
+ label: (exploreState: { someState: string }) =>
+ `Dynamic Metric (${exploreState.someState})`,
+ validationErrors: ['Metric is required'],
+ },
+ },
+ },
+ };
+
+ renderWithRouter({ initialState: customState });
+
+ const errorIndicator = await screen.findByTestId(
+ 'query-error-tooltip-trigger',
+ );
+
+ userEvent.hover(errorIndicator);
+
+ const tooltip = await screen.findByRole('tooltip');
+ expect(tooltip).toBeInTheDocument();
+
+ expect(await screen.findByText(/Metric is required/)).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index 13b67c5b99..40caa9decc 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
@@ -541,7 +541,11 @@ function ExploreViewContainer(props) {
.map(message => {
const matchingLabels = controlsWithErrors
.filter(control => control.validationErrors?.includes(message))
- .map(control => control.label);
+ .map(control =>
+ typeof control.label === 'function'
+ ? control.label(props.exploreState)
+ : control.label,
+ );
return [matchingLabels, message];
})
.map(([labels, message]) => (
diff --git a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx
index ad80f98205..b370333d83 100644
--- a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx
+++ b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx
@@ -87,17 +87,6 @@ export const useHeaderReportMenuItems = ({
const resourceTypeReports = reportsState[resourceType] || {};
const reportData = resourceTypeReports[resourceId];
- // Debug logging to understand what's happening
- console.log('Report selector called:', {
- resourceId,
- resourceType,
- reportsState: Object.keys(reportsState),
- resourceTypeReports: Object.keys(resourceTypeReports),
- reportData: reportData
- ? { id: reportData.id, name: reportData.name }
- : null,
- });
-
return reportData || null;
});