fix(chart): make chart error banners non-dismissible (#38014)

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(<ChartErrorMessage {...defaultProps} />);
+
+    expect(
+      screen.queryByRole('button', { name: /close/i }),
+    ).not.toBeInTheDocument();
+  });
 });

superset-frontend/src/components/Chart/ChartErrorMessage.tsx

@@ -49,6 +49,7 @@ export const ChartErrorMessage: FC<Props> = ({ chartId, error, ...props }) => {
       {...props}
       error={ownedError}
       title={DEFAULT_CHART_ERROR}
+      closable={false}
     />
   );
 };

superset-frontend/src/components/ErrorMessage/DatabaseErrorMessage.tsx

@@ -39,6 +39,7 @@ interface DatabaseErrorExtra {
 export function DatabaseErrorMessage({
   error,
   source,
+  closable,
 }: ErrorMessageComponentProps<DatabaseErrorExtra | null>) {
   const { extra, level, message } = error;
 
@@ -101,6 +102,7 @@ export function DatabaseErrorMessage({
       description={alertDescription}
       type={level}
       descriptionDetails={body}
+      closable={closable}
     />
   );
 }

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}
     />
   );
 }

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(<ErrorMessageWithStackTrace {...mockedProps} />);
+  expect(screen.getByRole('button', { name: /close/i })).toBeInTheDocument();
+});
+
+test('should not render a close button when closable is false', () => {
+  render(<ErrorMessageWithStackTrace {...mockedProps} closable={false} />);
+  expect(
+    screen.queryByRole('button', { name: /close/i }),
+  ).not.toBeInTheDocument();
+});
+
 test('should render the fallback', () => {
   const body = 'Blahblah';
   render(

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 (
         <ErrorMessageComponent
           compact={compact}
+          closable={closable}
           error={error}
           source={source}
           subtitle={subtitle}
@@ -99,6 +102,7 @@ export function ErrorMessageWithStackTrace({
       description={description}
       descriptionDetails={computedDescriptionDetails}
       compact={compact}
+      closable={closable}
     />
   );
 }

superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx

@@ -25,11 +25,13 @@ export function FrontendNetworkErrorMessage({
   error,
   subtitle,
   compact,
+  closable,
 }: ErrorMessageComponentProps) {
   const { level, message } = error;
   return (
     <ErrorAlert
       compact={compact}
+      closable={closable}
       errorType={t('Network Error')}
       message={message}
       type={level}

superset-frontend/src/components/ErrorMessage/InvalidSQLErrorMessage.tsx

@@ -34,6 +34,7 @@ interface SupersetParseErrorExtra {
 export function InvalidSQLErrorMessage({
   error,
   subtitle,
+  closable,
 }: ErrorMessageComponentProps<SupersetParseErrorExtra>) {
   const { extra, level, message } = error;
 
@@ -58,6 +59,7 @@ export function InvalidSQLErrorMessage({
       message={subtitle}
       type={level}
       description={body}
+      closable={closable}
     />
   );
 }

superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx

@@ -63,6 +63,7 @@ interface OAuth2RedirectExtra {
 export function OAuth2RedirectMessage({
   error,
   source,
+  closable,
 }: ErrorMessageComponentProps<OAuth2RedirectExtra>) {
   const oAuthTab = useRef<Window | null>(null);
   const { extra, level } = error;
@@ -172,6 +173,7 @@ export function OAuth2RedirectMessage({
       message={subtitle}
       type={level}
       description={body}
+      closable={closable}
     />
   );
 }

superset-frontend/src/components/ErrorMessage/ParameterErrorMessage.tsx

@@ -56,6 +56,7 @@ const findMatches = (undefinedParameters: string[], templateKeys: string[]) => {
 export function ParameterErrorMessage({
   error,
   subtitle,
+  closable,
 }: ErrorMessageComponentProps<ParameterErrorExtra>) {
   const { extra = { issue_codes: [] }, level, message } = error;
 
@@ -118,6 +119,7 @@ export function ParameterErrorMessage({
       message={message}
       description={subtitle}
       descriptionDetails={body}
+      closable={closable}
     />
   );
 }

superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx

@@ -36,6 +36,7 @@ interface TimeoutErrorExtra {
 export function TimeoutErrorMessage({
   error,
   source,
+  closable,
 }: ErrorMessageComponentProps<TimeoutErrorExtra>) {
   const { extra, level } = error;
 
@@ -95,6 +96,7 @@ export function TimeoutErrorMessage({
       message={subtitle}
       type={level}
       descriptionDetails={body}
+      closable={closable}
     />
   );
 }

superset-frontend/src/components/ErrorMessage/types.ts

@@ -26,6 +26,7 @@ export type ErrorMessageComponentProps<ExtraType = Record<string, any> | null> =
     source?: ErrorSource;
     subtitle?: ReactNode;
     compact?: boolean;
+    closable?: boolean;
   };
 
 export type ErrorMessageComponent = ComponentType<ErrorMessageComponentProps>;

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')