refactor(standardized form data): refine interface and improve code smells

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

@@ -344,30 +344,36 @@ export interface ControlPanelSectionConfig {
   controlSetRows: ControlSetRow[];
 }
 
-export interface StandardizedState {
+export interface StandardizedControls {
   metrics: QueryFormMetric[];
   columns: QueryFormColumn[];
 }
 
 export interface StandardizedFormDataInterface {
-  standardizedState: StandardizedState;
+  // Controls not used in the current viz
+  controls: StandardizedControls;
+  // Transformation history
   memorizedFormData: Map<string, QueryFormData>;
 }
 
+export type QueryStandardizedFormData = QueryFormData & {
+  standardizedFormData: StandardizedFormDataInterface;
+};
+
+export const isStandardizedFormData = (
+  formData: QueryFormData,
+): formData is QueryStandardizedFormData =>
+  formData?.standardizedFormData?.controls &&
+  formData?.standardizedFormData?.memorizedFormData &&
+  Array.isArray(formData.standardizedFormData.controls.metrics) &&
+  Array.isArray(formData.standardizedFormData.controls.columns);
+
 export interface ControlPanelConfig {
   controlPanelSections: (ControlPanelSectionConfig | null)[];
   controlOverrides?: ControlOverrides;
   sectionOverrides?: SectionOverrides;
   onInit?: (state: ControlStateMapping) => void;
-  denormalizeFormData?: (
-    formData: QueryFormData & {
-      standardizedFormData: StandardizedFormDataInterface;
-    },
-  ) => QueryFormData;
-  updateStandardizedState?: (
-    prevState: StandardizedState,
-    currState: StandardizedState,
-  ) => StandardizedState;
+  formDataOverrides?: (formData: QueryFormData) => QueryFormData;
 }
 
 export type ControlOverrides = {

superset-frontend/packages/superset-ui-chart-controls/src/utils/getStandardizedControls.ts

@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { makeSingleton, QueryFormData } from '@superset-ui/core';
+import { isStandardizedFormData, StandardizedControls } from '../types';
+
+class StandardizedControlsManager {
+  controls: StandardizedControls;
+
+  constructor() {
+    this.controls = {
+      metrics: [],
+      columns: [],
+    };
+  }
+
+  setStandardizedControls(formData: QueryFormData) {
+    if (isStandardizedFormData(formData)) {
+      const { controls } = formData.standardizedFormData;
+      this.controls = {
+        metrics: controls.metrics,
+        columns: controls.columns,
+      };
+    }
+  }
+
+  shiftMetric() {
+    return this.controls.metrics.shift();
+  }
+
+  popAllMetrics() {
+    return this.controls.metrics.splice(0, this.controls.metrics.length);
+  }
+
+  popAllColumns() {
+    return this.controls.columns.splice(0, this.controls.columns.length);
+  }
+
+  clear() {
+    this.controls = {
+      metrics: [],
+      columns: [],
+    };
+  }
+}
+
+export const getStandardizedControls = makeSingleton(
+  StandardizedControlsManager,
+);

superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts

@@ -23,3 +23,4 @@ export * from './getColorFormatters';
 export { default as mainMetric } from './mainMetric';
 export { default as columnChoices } from './columnChoices';
 export * from './defineSavedMetrics';
+export * from './getStandardizedControls';

superset-frontend/packages/superset-ui-chart-controls/test/utils/getStandardizedControls.test.ts

@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { QueryFormData } from '@superset-ui/core';
+import { getStandardizedControls } from '../../src';
+
+const formData: QueryFormData = {
+  datasource: '30__table',
+  viz_type: 'table',
+  standardizedFormData: {
+    controls: {
+      metrics: ['count(*)', 'sum(sales)'],
+      columns: ['gender', 'gender'],
+    },
+    memorizedFormData: [],
+  },
+};
+
+test('without standardizedFormData', () => {
+  getStandardizedControls().setStandardizedControls({
+    datasource: '30__table',
+    viz_type: 'table',
+  });
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: [],
+    columns: [],
+  });
+});
+
+test('getStandardizedControls', () => {
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: [],
+    columns: [],
+  });
+  getStandardizedControls().setStandardizedControls(formData);
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: ['count(*)', 'sum(sales)'],
+    columns: ['gender', 'gender'],
+  });
+  expect(getStandardizedControls().shiftMetric()).toEqual('count(*)');
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: ['sum(sales)'],
+    columns: ['gender', 'gender'],
+  });
+  expect(getStandardizedControls().popAllMetrics()).toEqual(['sum(sales)']);
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: [],
+    columns: ['gender', 'gender'],
+  });
+  expect(getStandardizedControls().popAllColumns()).toEqual([
+    'gender',
+    'gender',
+  ]);
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: [],
+    columns: [],
+  });
+
+  getStandardizedControls().setStandardizedControls(formData);
+  getStandardizedControls().clear();
+  expect(getStandardizedControls().controls).toEqual({
+    metrics: [],
+    columns: [],
+  });
+});

superset-frontend/packages/superset-ui-core/src/query/types/Column.ts

@@ -19,6 +19,7 @@
  */
 
 import { GenericDataType } from './QueryResponse';
+import { QueryFormColumn } from './QueryFormData';
 
 export interface AdhocColumn {
   hasCustomLabel?: boolean;
@@ -53,12 +54,18 @@ export interface Column {
 
 export default {};
 
-export function isPhysicalColumn(
-  column?: AdhocColumn | PhysicalColumn,
-): column is PhysicalColumn {
+export function isPhysicalColumn(column?: any): column is PhysicalColumn {
   return typeof column === 'string';
 }
 
-export function isAdhocColumn(column?: AdhocColumn | PhysicalColumn) {
-  return (column as AdhocColumn)?.sqlExpression !== undefined;
+export function isAdhocColumn(column?: any): column is AdhocColumn {
+  return (
+    typeof column !== 'string' &&
+    column?.sqlExpression !== undefined &&
+    column?.expressionType === 'SQL'
+  );
+}
+
+export function isQueryFormColumn(column: any): column is QueryFormColumn {
+  return isPhysicalColumn(column) || isAdhocColumn(column);
 }

superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts

@@ -17,7 +17,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { Maybe } from '../../types';
+import { Maybe, QueryFormMetric } from '../../types';
 import { Column } from './Column';
 
 export type Aggregate =
@@ -74,8 +74,22 @@ export interface Metric {
 
 export default {};
 
-export function isAdhocMetricSimple(
-  metric: AdhocMetric,
-): metric is AdhocMetricSimple {
-  return metric.expressionType === 'SIMPLE';
+export function isSavedMetric(metric: any): metric is SavedMetric {
+  return typeof metric === 'string';
+}
+
+export function isAdhocMetricSimple(metric: any): metric is AdhocMetricSimple {
+  return typeof metric !== 'string' && metric?.expressionType === 'SIMPLE';
+}
+
+export function isAdhocMetricSQL(metric: any): metric is AdhocMetricSQL {
+  return typeof metric !== 'string' && metric?.expressionType === 'SQL';
+}
+
+export function isQueryFormMetric(metric: any): metric is QueryFormMetric {
+  return (
+    isSavedMetric(metric) ||
+    isAdhocMetricSimple(metric) ||
+    isAdhocMetricSQL(metric)
+  );
 }

superset-frontend/packages/superset-ui-core/src/query/types/QueryFormData.ts

@@ -220,8 +220,4 @@ export function isDruidFormData(
   return 'granularity' in formData;
 }
 
-export function isSavedMetric(metric: QueryFormMetric): metric is SavedMetric {
-  return typeof metric === 'string';
-}
-
 export default {};

superset-frontend/packages/superset-ui-core/test/query/types/Column.test.ts

@@ -0,0 +1,63 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+  isAdhocColumn,
+  isPhysicalColumn,
+  isQueryFormColumn,
+} from '@superset-ui/core';
+
+const adhocColumn = {
+  expressionType: 'SQL',
+  label: 'country',
+  optionName: 'country',
+  sqlExpression: 'country',
+};
+
+test('isPhysicalColumn returns true', () => {
+  expect(isPhysicalColumn('gender')).toEqual(true);
+});
+
+test('isPhysicalColumn returns false', () => {
+  expect(isPhysicalColumn(adhocColumn)).toEqual(false);
+});
+
+test('isAdhocColumn returns true', () => {
+  expect(isAdhocColumn(adhocColumn)).toEqual(true);
+});
+
+test('isAdhocColumn returns false', () => {
+  expect(isAdhocColumn('hello')).toEqual(false);
+  expect(isAdhocColumn({})).toEqual(false);
+  expect(
+    isAdhocColumn({
+      expressionType: 'SQL',
+      label: 'country',
+      optionName: 'country',
+    }),
+  ).toEqual(false);
+});
+
+test('isQueryFormColumn returns true', () => {
+  expect(isQueryFormColumn('gender')).toEqual(true);
+  expect(isQueryFormColumn(adhocColumn)).toEqual(true);
+});
+
+test('isQueryFormColumn returns false', () => {
+  expect(isQueryFormColumn({})).toEqual(false);
+});