rebase and fix commnent -- patch 2

apache · Nov 5, 2021 · 3a27565 · 3a27565
1 parent 254a4ef
commit 3a27565
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 89 deletions.
diff --git a/superset-frontend/src/common/hooks/apiResources/apiResources.ts b/superset-frontend/src/common/hooks/apiResources/apiResources.ts
@@ -153,10 +153,18 @@ export function useTransformedResource<IN, OUT>(
       // While incomplete, there is no result - no need to transform.
       return resource;
     }
-    return {
-      ...resource,
-      result: transformFn(resource.result),
-    };
+    try {
+      return {
+        ...resource,
+        result: transformFn(resource.result),
+      };
+    } catch (e) {
+      return {
+        status: ResourceStatus.ERROR,
+        result: null,
+        error: e,
+      };
+    }
   }, [resource, transformFn]);
 }
 

diff --git a/superset-frontend/src/common/hooks/apiResources/dashboards.ts b/superset-frontend/src/common/hooks/apiResources/dashboards.ts
@@ -17,33 +17,20 @@
  * under the License.
  */
 
-import { Dashboard, DashboardMetadata, Datasource } from 'src/dashboard/types';
+import { Dashboard, Datasource } from 'src/dashboard/types';
 import { Chart } from 'src/types/Chart';
 import { useApiV1Resource, useTransformedResource } from './apiResources';
 
 export const useDashboard = (idOrSlug: string | number) =>
   useTransformedResource(
     useApiV1Resource<Dashboard>(`/api/v1/dashboard/${idOrSlug}`),
-    dashboard => {
-      let metadata: DashboardMetadata = {};
-      let position_data = {};
-      try {
-        // need handle data parsing error
-        if (dashboard.json_metadata) {
-          metadata = JSON.parse(dashboard.json_metadata);
-        }
-        if (dashboard.position_json) {
-          position_data = JSON.parse(dashboard.position_json);
-        }
-      } catch (e) {
-        console.error('can not parse dashboard metadata.');
-      }
-      return {
-        ...dashboard,
-        metadata,
-        position_data,
-      };
-    },
+    dashboard => ({
+      ...dashboard,
+      metadata:
+        (dashboard.json_metadata && JSON.parse(dashboard.json_metadata)) || {},
+      position_data:
+        dashboard.position_json && JSON.parse(dashboard.position_json),
+    }),
   );
 
 // gets the chart definitions for a dashboard

diff --git a/superset-frontend/src/components/Modal/Modal.tsx b/superset-frontend/src/components/Modal/Modal.tsx
@@ -51,7 +51,6 @@ export interface ModalProps {
   centered?: boolean;
   footer?: React.ReactNode;
   wrapProps?: object;
-  maskStyle?: object;
   height?: string;
   closable?: boolean;
   resizable?: boolean;
@@ -208,7 +207,6 @@ const CustomModal = ({
   footer,
   hideFooter,
   wrapProps,
-  maskStyle,
   draggable = false,
   resizable = false,
   resizableConfig = {
@@ -325,7 +323,6 @@ const CustomModal = ({
         )
       }
       mask={shouldShowMask}
-      maskStyle={maskStyle}
       draggable={draggable}
       resizable={resizable}
       destroyOnClose={destroyOnClose || resizable || draggable}

diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js
@@ -69,7 +69,7 @@ export const hydrateDashboard = (
 ) => (dispatch, getState) => {
   const { user, common } = getState();
 
-  let { metadata } = dashboardData;
+  const { metadata } = dashboardData;
   const regularUrlParams = extractUrlParams('regular');
   const reservedUrlParams = extractUrlParams('reserved');
   const editMode = reservedUrlParams.edit === 'true';
@@ -296,17 +296,12 @@ export const hydrateDashboard = (
       filterScopes,
       preselectFilters,
     );
-    metadata = metadata || {};
     metadata.native_filter_configuration = filterConfig;
+    metadata.show_native_filters = true;
   }
   const nativeFilters = getInitialNativeFilterState({
     filterConfig,
   });
-
-  if (!metadata) {
-    metadata = {};
-  }
-
   metadata.show_native_filters =
     dashboardData?.metadata?.show_native_filters ??
     (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) &&

diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts
@@ -76,10 +76,12 @@ export const useNativeFilters = () => {
   useEffect(() => {
     if (
       filterValues.length === 0 &&
-      dashboardFiltersOpen &&
-      nativeFiltersEnabled
+      nativeFiltersEnabled &&
+      ['CONVERTED', 'REVIEWING', 'NOOP'].includes(filterboxMigrationState)
     ) {
       toggleDashboardFiltersOpen(false);
+    } else {
+      toggleDashboardFiltersOpen(true);
     }
   }, [filterValues.length, filterboxMigrationState]);
 

diff --git a/superset-frontend/src/dashboard/components/FilterBoxMigrationModal.tsx b/superset-frontend/src/dashboard/components/FilterBoxMigrationModal.tsx
@@ -44,7 +44,7 @@ const StyledFilterBoxMigrationModal = styled(Modal)`
   }
 
   .ant-modal-body {
-    overflow: visible;
+    overflow: auto;
   }
 `;
 
@@ -64,7 +64,6 @@ const FilterBoxMigrationModal: FunctionComponent<FilterBoxMigrationModalProps> =
   hideFooter = false,
 }) => (
   <StyledFilterBoxMigrationModal
-    maskStyle={{ top: '50px' }}
     show={show}
     onHide={onHide}
     title={t('Ready to review filters in this dashboard?')}

diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
@@ -19,7 +19,7 @@
 import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
-import { styled, supersetTheme } from '@superset-ui/core';
+import { styled } from '@superset-ui/core';
 import { isEqual } from 'lodash';
 
 import {
@@ -97,7 +97,6 @@ const SHOULD_UPDATE_ON_PROP_CHANGES = Object.keys(propTypes).filter(
 );
 const OVERFLOWABLE_VIZ_TYPES = new Set(['filter_box']);
 const DEFAULT_HEADER_HEIGHT = 22;
-const { colors } = supersetTheme;
 
 const ChartOverlay = styled.div`
   position: absolute;
@@ -107,7 +106,7 @@ const ChartOverlay = styled.div`
 
   &.is-deactivated {
     opacity: 0.5;
-    background-color: ${colors.grayscale.light1};
+    background-color: ${({ theme }) => theme.colors.grayscale.light1};
   }
 `;
 

diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts
@@ -65,11 +65,6 @@ export type DashboardState = {
   isRefreshing: boolean;
   hasUnsavedChanges: boolean;
 };
-export type DashboardMetadata = {
-  native_filter_configuration?: { [key: string]: any };
-  show_native_filters?: boolean;
-  chart_configuration?: JsonObject;
-};
 export type DashboardInfo = {
   id: number;
   common: {

diff --git a/superset-frontend/src/dashboard/util/filterboxMigrationHelper.test.ts b/superset-frontend/src/dashboard/util/filterboxMigrationHelper.test.ts
@@ -100,51 +100,45 @@ const preselectedFilters = {
   },
 };
 
-describe('getNativeFilterConfig', () => {
-  it('should convert filter_box config to dashboard native filter config', () => {
-    const filterConfig = getNativeFilterConfig(chartData, {}, {});
-    // convert to 2 components
-    expect(filterConfig.length).toEqual(2);
+test('should convert filter_box config to dashboard native filter config', () => {
+  const filterConfig = getNativeFilterConfig(chartData, {}, {});
+  // convert to 2 components
+  expect(filterConfig.length).toEqual(2);
 
-    expect(filterConfig[0].id).toBeDefined();
-    expect(filterConfig[0].filterType).toBe('filter_select');
-    expect(filterConfig[0].name).toBe('region');
-    expect(filterConfig[0].targets).toEqual([
-      { column: { name: 'region' }, datasetId: 1 },
-    ]);
-    expect(filterConfig[0].scope).toEqual({
-      excluded: [],
-      rootPath: ['ROOT_ID'],
-    });
+  expect(filterConfig[0].id).toBeDefined();
+  expect(filterConfig[0].filterType).toBe('filter_select');
+  expect(filterConfig[0].name).toBe('region');
+  expect(filterConfig[0].targets).toEqual([
+    { column: { name: 'region' }, datasetId: 1 },
+  ]);
+  expect(filterConfig[0].scope).toEqual({
+    excluded: [],
+    rootPath: ['ROOT_ID'],
+  });
 
-    expect(filterConfig[1].id).toBeDefined();
-    expect(filterConfig[1].filterType).toBe('filter_select');
-    expect(filterConfig[1].name).toBe('country_name');
-    expect(filterConfig[1].targets).toEqual([
-      { column: { name: 'country_name' }, datasetId: 1 },
-    ]);
-    expect(filterConfig[1].scope).toEqual({
-      excluded: [],
-      rootPath: ['ROOT_ID'],
-    });
+  expect(filterConfig[1].id).toBeDefined();
+  expect(filterConfig[1].filterType).toBe('filter_select');
+  expect(filterConfig[1].name).toBe('country_name');
+  expect(filterConfig[1].targets).toEqual([
+    { column: { name: 'country_name' }, datasetId: 1 },
+  ]);
+  expect(filterConfig[1].scope).toEqual({
+    excluded: [],
+    rootPath: ['ROOT_ID'],
   });
+});
 
-  it('should convert preselected filters', () => {
-    const filterConfig = getNativeFilterConfig(
-      chartData,
-      {},
-      preselectedFilters,
-    );
-    const { defaultDataMask } = filterConfig[0];
-    expect(defaultDataMask.filterState).toEqual({
-      value: ['East Asia & Pacific'],
-    });
-    expect(defaultDataMask.extraFormData?.filters).toEqual([
-      {
-        col: 'region',
-        op: 'IN',
-        val: ['East Asia & Pacific'],
-      },
-    ]);
+test('should convert preselected filters', () => {
+  const filterConfig = getNativeFilterConfig(chartData, {}, preselectedFilters);
+  const { defaultDataMask } = filterConfig[0];
+  expect(defaultDataMask.filterState).toEqual({
+    value: ['East Asia & Pacific'],
   });
+  expect(defaultDataMask.extraFormData?.filters).toEqual([
+    {
+      col: 'region',
+      op: 'IN',
+      val: ['East Asia & Pacific'],
+    },
+  ]);
 });
diff --git a/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts b/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts
@@ -228,6 +228,7 @@ export default function getNativeFilterConfig(
           DASHBOARD_FILTER_SCOPE_GLOBAL;
         const timeRangeFilter: Filter = {
           id: `NATIVE_FILTER-${shortid.generate()}`,
+          description: 'time range filter',
           controlValues: {},
           name: TIME_FILTER_LABELS.time_range,
           filterType: FILTER_COMPONENT_FILTER_TYPES.FILTER_TIME,
@@ -260,6 +261,7 @@ export default function getNativeFilterConfig(
           const timeGrainFilter: Filter = {
             id: `NATIVE_FILTER-${shortid.generate()}`,
             controlValues: {},
+            description: 'time grain filter',
             name: TIME_FILTER_LABELS.time_grain_sqla,
             filterType: FILTER_COMPONENT_FILTER_TYPES.FILTER_TIMEGRAIN,
             targets: [
@@ -304,6 +306,7 @@ export default function getNativeFilterConfig(
             DASHBOARD_FILTER_SCOPE_GLOBAL;
           const timeColumnFilter: Filter = {
             id: `NATIVE_FILTER-${shortid.generate()}`,
+            description: 'time column filter',
             controlValues: {},
             name: TIME_FILTER_LABELS.granularity_sqla,
             filterType: FILTER_COMPONENT_FILTER_TYPES.FILTER_TIMECOLUMN,
@@ -349,6 +352,7 @@ export default function getNativeFilterConfig(
             DASHBOARD_FILTER_SCOPE_GLOBAL;
           const druidGranularityFilter: Filter = {
             id: `NATIVE_FILTER-${shortid.generate()}`,
+            description: 'time grain filter',
             controlValues: {},
             name: TIME_FILTER_LABELS.granularity,
             filterType: FILTER_COMPONENT_FILTER_TYPES.FILTER_TIMEGRAIN,
@@ -391,6 +395,7 @@ export default function getNativeFilterConfig(
             DASHBOARD_FILTER_SCOPE_GLOBAL;
           const druidOriginFilter: Filter = {
             id: `NATIVE_FILTER-${shortid.generate()}`,
+            description: 'time column filter',
             controlValues: {},
             name: TIME_FILTER_LABELS.druid_time_origin,
             filterType: FILTER_COMPONENT_FILTER_TYPES.FILTER_TIMECOLUMN,
@@ -437,6 +442,7 @@ export default function getNativeFilterConfig(
           scopesByChartId[config.column] || DASHBOARD_FILTER_SCOPE_GLOBAL;
         const entry: Filter = {
           id: `NATIVE_FILTER-${shortid.generate()}`,
+          description: '',
           controlValues: {
             enableEmptyFilter: !config[FILTER_CONFIG_ATTRIBUTES.CLEARABLE],
             defaultToFirstItem: false,