diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx index 96cb6ab13d106..db449b3450ef6 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/ColumnOption.tsx @@ -17,7 +17,7 @@ * under the License. */ import { useState, ReactNode, useLayoutEffect, RefObject } from 'react'; -import { css, styled, SupersetTheme } from '@superset-ui/core'; +import { css, SafeMarkdown, styled, SupersetTheme } from '@superset-ui/core'; import { Tooltip } from './Tooltip'; import { ColumnTypeLabel } from './ColumnTypeLabel/ColumnTypeLabel'; import CertifiedIconWithTooltip from './CertifiedIconWithTooltip'; @@ -28,6 +28,7 @@ import { getColumnTypeTooltipNode, } from './labelUtils'; import { SQLPopover } from './SQLPopover'; +import InfoTooltipWithTrigger from './InfoTooltipWithTrigger'; export type ColumnOptionProps = { column: ColumnMeta; @@ -50,6 +51,8 @@ export function ColumnOption({ }: ColumnOptionProps) { const { expression, column_name, type_generic } = column; const hasExpression = expression && expression !== column_name; + const warningMarkdown = + column.warning_markdown || column.warning_text || column.error_text; const type = hasExpression ? 'expression' : type_generic; const [tooltipText, setTooltipText] = useState(column.column_name); const [columnTypeTooltipText, setcolumnTypeTooltipText] = useState( @@ -94,6 +97,19 @@ export function ColumnOption({ details={column.certification_details} /> )} + {warningMarkdown && ( + } + label={`warn-${column.column_name}`} + iconsStyle={{ marginLeft: 0 }} + {...(column.error_text && { + className: 'text-danger', + icon: 'exclamation-circle', + })} + /> + )} ); } diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx index b558a0987ab0c..c424cde518e26 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/MetricOption.tsx @@ -81,7 +81,8 @@ export function MetricOption({ ); - const warningMarkdown = metric.warning_markdown || metric.warning_text; + const warningMarkdown = + metric.warning_markdown || metric.warning_text || metric.error_text; const [tooltipText, setTooltipText] = useState(metric.metric_name); @@ -116,6 +117,10 @@ export function MetricOption({ tooltip={} label={`warn-${metric.metric_name}`} iconsStyle={{ marginLeft: 0 }} + {...(metric.error_text && { + className: 'text-danger', + icon: 'exclamation-circle', + })} /> )} diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx index 24cb7743b04e5..c5b369aa2dbe9 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/ColumnOption.test.tsx @@ -34,6 +34,9 @@ jest.mock('../../src/components/ColumnTypeLabel/ColumnTypeLabel', () => ({
{type}
), })); +jest.mock('../../src/components/InfoTooltipWithTrigger', () => () => ( +
+)); const defaultProps: ColumnOptionProps = { column: { @@ -111,3 +114,17 @@ test('dttm column has correct column label if showType is true', () => { String(GenericDataType.Temporal), ); }); +test('doesnt show InfoTooltipWithTrigger when no warning', () => { + const { queryByText } = setup(); + expect(queryByText('mock-info-tooltip-with-trigger')).not.toBeInTheDocument(); +}); +test('shows a warning with InfoTooltipWithTrigger when it contains warning', () => { + const { getByTestId } = setup({ + ...defaultProps, + column: { + ...defaultProps.column, + warning_text: 'This is a warning', + }, + }); + expect(getByTestId('mock-info-tooltip-with-trigger')).toBeInTheDocument(); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts index ac6523bedb35d..227ca6e71d562 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts @@ -71,6 +71,7 @@ export interface Metric { verbose_name?: string; warning_markdown?: Maybe; warning_text?: Maybe; + error_text?: string; } export function isSavedMetric(metric: any): metric is SavedMetric { diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx index b19803931d909..a1ce875c01d8d 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx @@ -16,7 +16,8 @@ * specific language governing permissions and limitations * under the License. */ -import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { render, screen, within } from 'spec/helpers/testing-library'; import { DndColumnSelect, DndColumnSelectProps, @@ -63,3 +64,52 @@ test('renders adhoc column', async () => { expect(await screen.findByText('adhoc column')).toBeVisible(); expect(screen.getByLabelText('calculator')).toBeVisible(); }); + +test('warn selected custom metric when metric gets removed from dataset', async () => { + const columnValues = ['column1', 'column2']; + + const { rerender, container } = render( + , + { + useDnd: true, + useRedux: true, + }, + ); + + rerender( + , + ); + expect(screen.getByText('column2')).toBeVisible(); + expect(screen.queryByText('column1')).toBeInTheDocument(); + const warningIcon = within( + screen.getByText('column1').parentElement ?? container, + ).getByRole('button'); + expect(warningIcon).toBeInTheDocument(); + userEvent.hover(warningIcon); + const warningTooltip = await screen.findByText( + 'This column might be incompatible with current dataset', + ); + expect(warningTooltip).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx index 455e1bf1b2e1b..faceb68336e1f 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx @@ -109,6 +109,8 @@ function DndColumnSelect(props: DndColumnSelectProps) { isAdhocColumn(column) && column.datasourceWarning ? t('This column might be incompatible with current dataset') : undefined; + const withCaret = isAdhocColumn(column) || !column.error_text; + return ( ); diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx index 5a0264665a392..c00b50397dd53 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx @@ -92,13 +92,13 @@ test('render selected metrics correctly', () => { expect(screen.getByText('SUM(Column B)')).toBeVisible(); }); -test('remove selected custom metric when metric gets removed from dataset', () => { +test('warn selected custom metric when metric gets removed from dataset', async () => { let metricValues = ['metric_a', 'metric_b', adhocMetricA, adhocMetricB]; const onChange = (val: any[]) => { metricValues = val; }; - const { rerender } = render( + const { rerender, container } = render( { +test('warn selected custom metric when metric gets removed from dataset for single-select metric control', async () => { let metricValue = 'metric_b'; const onChange = (val: any) => { metricValue = val; }; - const { rerender } = render( + const { rerender, container } = render( { diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx index 214334a5ac27f..c6586bb97e534 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx @@ -60,11 +60,6 @@ const coerceMetrics = ( } const metricsCompatibleWithDataset = ensureIsArray(addedMetrics).filter( metric => { - if (isSavedMetric(metric)) { - return savedMetrics.some( - savedMetric => savedMetric.metric_name === metric, - ); - } if (isAdhocMetricSimple(metric)) { return columns.some( column => column.column_name === metric.column.column_name, @@ -75,6 +70,15 @@ const coerceMetrics = ( ); return metricsCompatibleWithDataset.map(metric => { + if ( + isSavedMetric(metric) && + !savedMetrics.some(savedMetric => savedMetric.metric_name === metric) + ) { + return { + metric_name: metric, + error_text: t('This metric might be incompatible with current dataset'), + }; + } if (!isDictionaryForAdhocMetric(metric)) { return metric; } diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts index 433b449be2125..70e22b31ed20b 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts @@ -22,6 +22,7 @@ import { ensureIsArray, QueryFormColumn, isPhysicalColumn, + t, } from '@superset-ui/core'; const getColumnNameOrAdhocColumn = ( @@ -55,7 +56,13 @@ export class OptionSelector { if (!isPhysicalColumn(value)) { return value; } - return null; + return { + type_generic: 'UNKNOWN', + column_name: value, + error_text: t( + 'This column might be incompatible with current dataset', + ), + }; }) .filter(Boolean) as ColumnMeta[]; } diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx index 700d31d7d57ea..7ca0f7065fb84 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx @@ -67,6 +67,7 @@ class AdhocMetricOption extends PureComponent { multi, datasourceWarningMessage, } = this.props; + const withCaret = !savedMetric.error_text; return (