diff --git a/superset-frontend/src/components/Chart/ChartErrorMessage.test.tsx b/superset-frontend/src/components/Chart/ChartErrorMessage.test.tsx index 1680b0e7602..5a96d6c108c 100644 --- a/superset-frontend/src/components/Chart/ChartErrorMessage.test.tsx +++ b/superset-frontend/src/components/Chart/ChartErrorMessage.test.tsx @@ -82,4 +82,17 @@ describe('ChartErrorMessage', () => { expect(screen.getByText('Test error')).toBeInTheDocument(); expect(screen.getByText('Test subtitle')).toBeInTheDocument(); }); + + test('chart error banner is not dismissible', () => { + mockUseChartOwnerNames.mockReturnValue({ + result: null, + status: ResourceStatus.Loading, + error: null, + }); + render(); + + expect( + screen.queryByRole('button', { name: /close/i }), + ).not.toBeInTheDocument(); + }); }); diff --git a/superset-frontend/src/components/Chart/ChartErrorMessage.tsx b/superset-frontend/src/components/Chart/ChartErrorMessage.tsx index 4c3f79965b3..cd21275153f 100644 --- a/superset-frontend/src/components/Chart/ChartErrorMessage.tsx +++ b/superset-frontend/src/components/Chart/ChartErrorMessage.tsx @@ -49,6 +49,7 @@ export const ChartErrorMessage: FC = ({ chartId, error, ...props }) => { {...props} error={ownedError} title={DEFAULT_CHART_ERROR} + closable={false} /> ); }; diff --git a/superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx index 3125ca92162..fad71169c58 100644 --- a/superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx @@ -39,6 +39,7 @@ interface DatabaseErrorExtra { export function DatabaseErrorMessage({ error, source, + closable, }: ErrorMessageComponentProps) { const { extra, level, message } = error; @@ -101,6 +102,7 @@ export function DatabaseErrorMessage({ description={alertDescription} type={level} descriptionDetails={body} + closable={closable} /> ); } diff --git a/superset-frontend/src/components/ErrorMessage/DatasetNotFoundErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/DatasetNotFoundErrorMessage.tsx index f0f279e4a22..6623715ee0b 100644 --- a/superset-frontend/src/components/ErrorMessage/DatasetNotFoundErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/DatasetNotFoundErrorMessage.tsx @@ -24,6 +24,7 @@ import { ErrorAlert } from './ErrorAlert'; export function DatasetNotFoundErrorMessage({ error, subtitle, + closable, }: ErrorMessageComponentProps) { const { level, message } = error; return ( @@ -32,6 +33,7 @@ export function DatasetNotFoundErrorMessage({ message={subtitle} description={message} type={level} + closable={closable} /> ); } diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.test.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.test.tsx index b27ff32d00a..4b9c5946e76 100644 --- a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.test.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.test.tsx @@ -58,6 +58,18 @@ test('should render the link', () => { expect(link).toHaveAttribute('href', mockedProps.link); }); +test('should render a close button by default', () => { + render(); + expect(screen.getByRole('button', { name: /close/i })).toBeInTheDocument(); +}); + +test('should not render a close button when closable is false', () => { + render(); + expect( + screen.queryByRole('button', { name: /close/i }), + ).not.toBeInTheDocument(); +}); + test('should render the fallback', () => { const body = 'Blahblah'; render( diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx index dc562e473ba..2dc7f793bbe 100644 --- a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx @@ -38,6 +38,7 @@ type Props = { errorMitigationFunction?: () => void; fallback?: ReactNode; compact?: boolean; + closable?: boolean; }; export function ErrorMessageWithStackTrace({ @@ -51,6 +52,7 @@ export function ErrorMessageWithStackTrace({ descriptionDetails, fallback, compact, + closable = true, }: Props) { // Check if a custom error message component was registered for this message if (error) { @@ -62,6 +64,7 @@ export function ErrorMessageWithStackTrace({ return ( ); } diff --git a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx index 98338af13c7..e52b96b8fee 100644 --- a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx @@ -25,11 +25,13 @@ export function FrontendNetworkErrorMessage({ error, subtitle, compact, + closable, }: ErrorMessageComponentProps) { const { level, message } = error; return ( ) { const { extra, level, message } = error; @@ -58,6 +59,7 @@ export function InvalidSQLErrorMessage({ message={subtitle} type={level} description={body} + closable={closable} /> ); } diff --git a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx index f62a5e076c6..1c6ede178b7 100644 --- a/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx @@ -63,6 +63,7 @@ interface OAuth2RedirectExtra { export function OAuth2RedirectMessage({ error, source, + closable, }: ErrorMessageComponentProps) { const oAuthTab = useRef(null); const { extra, level } = error; @@ -172,6 +173,7 @@ export function OAuth2RedirectMessage({ message={subtitle} type={level} description={body} + closable={closable} /> ); } diff --git a/superset-frontend/src/components/ErrorMessage/ParameterErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/ParameterErrorMessage.tsx index 2e4dbf23d9e..73d8b264c7f 100644 --- a/superset-frontend/src/components/ErrorMessage/ParameterErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/ParameterErrorMessage.tsx @@ -56,6 +56,7 @@ const findMatches = (undefinedParameters: string[], templateKeys: string[]) => { export function ParameterErrorMessage({ error, subtitle, + closable, }: ErrorMessageComponentProps) { const { extra = { issue_codes: [] }, level, message } = error; @@ -118,6 +119,7 @@ export function ParameterErrorMessage({ message={message} description={subtitle} descriptionDetails={body} + closable={closable} /> ); } diff --git a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx index d9e5a26093d..298d1859832 100644 --- a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx @@ -36,6 +36,7 @@ interface TimeoutErrorExtra { export function TimeoutErrorMessage({ error, source, + closable, }: ErrorMessageComponentProps) { const { extra, level } = error; @@ -95,6 +96,7 @@ export function TimeoutErrorMessage({ message={subtitle} type={level} descriptionDetails={body} + closable={closable} /> ); } diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index de38332fa6b..de597d19347 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -26,6 +26,7 @@ export type ErrorMessageComponentProps | null> = source?: ErrorSource; subtitle?: ReactNode; compact?: boolean; + closable?: boolean; }; export type ErrorMessageComponent = ComponentType; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx index c4ba8646538..7ef19dce5fb 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx @@ -370,6 +370,31 @@ test('should fallback to formData state when runtime state not available', () => expect(getByTestId('chart-container')).toBeInTheDocument(); }); +test('should not show a close button on chart error banners', () => { + const { queryByRole } = setup( + {}, + { + ...defaultState, + charts: { + ...defaultState.charts, + [queryId]: { + ...defaultState.charts[queryId], + chartStatus: 'failed', + chartAlert: 'Something went wrong', + queriesResponse: [ + { + message: 'Something went wrong', + errors: [], + }, + ], + }, + }, + }, + ); + + expect(queryByRole('button', { name: /close/i })).not.toBeInTheDocument(); +}); + test('should handle chart state when no converter exists', () => { jest .spyOn(chartStateConverter, 'hasChartStateConverter')