fix(explore): Display missing dataset for denied access (#34129)

Author: justinpark

superset-frontend/packages/superset-ui-chart-controls/src/types.ts

@@ -90,6 +90,7 @@ export interface Dataset {
   database?: Record<string, unknown>;
   normalize_columns?: boolean;
   always_filter_main_dttm?: boolean;
+  extra?: object | string;
 }
 
 export interface ControlPanelState {

superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx

@@ -479,3 +479,30 @@ test('should show missing dataset state', () => {
     ),
   ).toBeVisible();
 });
+
+test('should show forbidden dataset state', () => {
+  // @ts-ignore
+  delete window.location;
+  // @ts-ignore
+  window.location = { search: '?slice_id=152' };
+  const error = {
+    error_type: 'TABLE_SECURITY_ACCESS_ERROR',
+    statusText: 'FORBIDDEN',
+    message: 'You do not have access to the following tables: blocked_table',
+    extra: {
+      datasource: 152,
+      datasource_name: 'forbidden dataset',
+    },
+  };
+  const props = createProps({
+    datasource: {
+      ...fallbackExploreInitialData.dataset,
+      extra: {
+        error,
+      },
+    },
+  });
+  render(<DatasourceControl {...props} />, { useRedux: true, useRouter: true });
+  expect(screen.getByText(error.message)).toBeInTheDocument();
+  expect(screen.getByText(error.statusText)).toBeVisible();
+});

superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx

@@ -50,6 +50,7 @@ import {
   userHasPermission,
   isUserAdmin,
 } from 'src/dashboard/util/permissionUtils';
+import { ErrorMessageWithStackTrace } from 'src/components/ErrorMessage/ErrorMessageWithStackTrace';
 import ViewQueryModalFooter from 'src/explore/components/controls/ViewQueryModalFooter';
 import ViewQuery from 'src/explore/components/controls/ViewQuery';
 import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
@@ -281,7 +282,17 @@ class DatasourceControl extends PureComponent {
       showSaveDatasetModal,
     } = this.state;
     const { datasource, onChange, theme } = this.props;
-    const isMissingDatasource = !datasource?.id;
+    let extra;
+    if (datasource?.extra) {
+      if (typeof datasource.extra === 'string') {
+        try {
+          extra = JSON.parse(datasource.extra);
+        } catch {} // eslint-disable-line no-empty
+      } else {
+        extra = datasource.extra; // eslint-disable-line prefer-destructuring
+      }
+    }
+    const isMissingDatasource = !datasource?.id || Boolean(extra?.error);
     let isMissingParams = false;
     if (isMissingDatasource) {
       const datasourceId = getUrlParam(URL_PARAMS.datasourceId);
@@ -386,20 +397,10 @@ class DatasourceControl extends PureComponent {
 
     const { health_check_message: healthCheckMessage } = datasource;
 
-    let extra;
-    if (datasource?.extra) {
-      if (typeof datasource.extra === 'string') {
-        try {
-          extra = JSON.parse(datasource.extra);
-        } catch {} // eslint-disable-line no-empty
-      } else {
-        extra = datasource.extra; // eslint-disable-line prefer-destructuring
-      }
-    }
-
-    const titleText = isMissingDatasource
-      ? t('Missing dataset')
-      : getDatasourceTitle(datasource);
+    const titleText =
+      isMissingDatasource && !datasource.name
+        ? t('Missing dataset')
+        : getDatasourceTitle(datasource);
 
     const tooltip = titleText;
 
@@ -452,31 +453,44 @@ class DatasourceControl extends PureComponent {
         )}
         {isMissingDatasource && !isMissingParams && (
           <div className="error-alert">
-            <ErrorAlert
-              type="warning"
-              errorType={t('Missing dataset')}
-              descriptionPre={false}
-              descriptionDetailsCollapsed={false}
-              descriptionDetails={
-                <>
-                  <p>
-                    {t(
-                      'The dataset linked to this chart may have been deleted.',
-                    )}
-                  </p>
-                  <p>
-                    <Button
-                      buttonStyle="warning"
-                      onClick={() =>
-                        this.handleMenuItemClick({ key: CHANGE_DATASET })
-                      }
-                    >
-                      {t('Swap dataset')}
-                    </Button>
-                  </p>
-                </>
-              }
-            />
+            {extra?.error ? (
+              <div className="error-alert">
+                <ErrorMessageWithStackTrace
+                  title={extra.error.statusText || extra.error.message}
+                  subtitle={
+                    extra.error.statusText ? extra.error.message : undefined
+                  }
+                  error={extra.error}
+                  source="explore"
+                />
+              </div>
+            ) : (
+              <ErrorAlert
+                type="warning"
+                errorType={t('Missing dataset')}
+                descriptionPre={false}
+                descriptionDetailsCollapsed={false}
+                descriptionDetails={
+                  <>
+                    <p>
+                      {t(
+                        'The dataset linked to this chart may have been deleted.',
+                      )}
+                    </p>
+                    <p>
+                      <Button
+                        buttonStyle="warning"
+                        onClick={() =>
+                          this.handleMenuItemClick({ key: CHANGE_DATASET })
+                        }
+                      >
+                        {t('Swap dataset')}
+                      </Button>
+                    </p>
+                  </>
+                }
+              />
+            )}
           </div>
         )}
         {showEditDatasourceModal && (

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx

@@ -90,7 +90,10 @@ const DndFilterSelect = (props: DndFilterSelectProps) => {
     let extra = {};
     if (datasource?.extra) {
       try {
-        extra = JSON.parse(datasource.extra);
+        extra =
+          typeof datasource.extra === 'string'
+            ? JSON.parse(datasource.extra)
+            : datasource.extra;
       } catch {} // eslint-disable-line no-empty
     }
     return extra;

superset-frontend/src/explore/fixtures.tsx

@@ -159,8 +159,8 @@ export const fallbackExploreInitialData: ExplorePageInitialData = {
     verbose_map: {},
     main_dttm_col: '',
     owners: [],
-    datasource_name: 'missing_datasource',
-    name: 'missing_datasource',
+    datasource_name: '',
+    name: '',
     description: null,
   },
   slice: null,

superset-frontend/src/explore/types.ts

@@ -69,7 +69,7 @@ export type Datasource = Dataset & {
   catalog?: string | null;
   schema?: string;
   is_sqllab_view?: boolean;
-  extra?: string;
+  extra?: string | object;
 };
 
 export interface ExplorePageInitialData {

superset-frontend/src/pages/Chart/Chart.test.tsx

@@ -31,6 +31,7 @@ import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWi
 import { URL_PARAMS } from 'src/constants';
 import { JsonObject, VizType } from '@superset-ui/core';
 import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
+import { getParsedExploreURLParams } from 'src/explore/exploreUtils/getParsedExploreURLParams';
 import ChartPage from '.';
 
 jest.mock('src/hooks/useUnsavedChangesPrompt', () => ({
@@ -49,6 +50,9 @@ jest.mock(
     ),
 );
 jest.mock('src/dashboard/util/charts/getFormDataWithExtraFilters');
+jest.mock('src/explore/exploreUtils/getParsedExploreURLParams', () => ({
+  getParsedExploreURLParams: jest.fn(),
+}));
 
 describe('ChartPage', () => {
   beforeEach(() => {
@@ -89,6 +93,91 @@ describe('ChartPage', () => {
     );
   });
 
+  test('displays the dataset name and error when it is prohibited', async () => {
+    const chartApiRoute = `glob:*/api/v1/chart/*`;
+    const exploreApiRoute = 'glob:*/api/v1/explore/*';
+    const expectedDatasourceName = 'failed datasource name';
+    (getParsedExploreURLParams as jest.Mock).mockReturnValue(
+      new Map([['datasource_id', 1]]),
+    );
+    fetchMock.get(exploreApiRoute, () => {
+      class Extra {
+        datasource = 123;
+
+        datasource_name = expectedDatasourceName;
+      }
+      class SupersetSecurityError {
+        message = 'You do not have a permission to the table';
+
+        extra = new Extra();
+      }
+      throw new SupersetSecurityError();
+    });
+    fetchMock.get(chartApiRoute, 200);
+    const { getByTestId } = render(<ChartPage />, {
+      useRouter: true,
+      useRedux: true,
+      useDnd: true,
+    });
+    await waitFor(
+      () =>
+        expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent(
+          JSON.stringify({ datasource_name: expectedDatasourceName }).slice(
+            1,
+            -1,
+          ),
+        ),
+      {
+        timeout: 5000,
+      },
+    );
+    expect(fetchMock.calls(chartApiRoute).length).toEqual(0);
+    expect(fetchMock.calls(exploreApiRoute).length).toBeGreaterThanOrEqual(1);
+  });
+
+  test('fetches the chart api when explore metadata is prohibited and access from the chart link', async () => {
+    const expectedChartId = 7;
+    const expectedChartName = 'Unauthorized dataset owned chart name';
+    (getParsedExploreURLParams as jest.Mock).mockReturnValue(
+      new Map([['slice_id', expectedChartId]]),
+    );
+    const chartApiRoute = `glob:*/api/v1/chart/${expectedChartId}`;
+    const exploreApiRoute = 'glob:*/api/v1/explore/*';
+
+    fetchMock.get(exploreApiRoute, () => {
+      class Extra {
+        datasource = 123;
+      }
+      class SupersetSecurityError {
+        message = 'You do not have a permission to the table';
+
+        extra = new Extra();
+      }
+      throw new SupersetSecurityError();
+    });
+    fetchMock.get(chartApiRoute, {
+      result: {
+        id: expectedChartId,
+        slice_name: expectedChartName,
+        url: 'chartid',
+      },
+    });
+    const { getByTestId, getByText } = render(<ChartPage />, {
+      useRouter: true,
+      useRedux: true,
+      useDnd: true,
+    });
+    await waitFor(() => expect(fetchMock.calls(chartApiRoute).length).toBe(1), {
+      timeout: 5000,
+    });
+    expect(fetchMock.calls(exploreApiRoute).length).toBeGreaterThanOrEqual(1);
+    expect(getByTestId('mock-explore-chart-panel')).toBeInTheDocument();
+    expect(getByTestId('mock-explore-chart-panel')).toHaveTextContent(
+      JSON.stringify({ datasource: 123 }).slice(1, -1),
+    );
+    expect(getByText(expectedChartName)).toBeInTheDocument();
+  });
+
   describe('with dashboardContextFormData', () => {
     const dashboardPageId = 'mockPageId';