refactor(plugin-chart-table): migrate to API v1 (#889)

package.json

@@ -18,7 +18,7 @@
     "commit": "superset-commit",
     "changelog": "conventional-changelog -p angular -i CHANGELOG.md -s -r 10",
     "jest": "NODE_ENV=test jest --coverage --verbose",
-    "lint": "esprint check --workers=3",
+    "lint": "esprint check",
     "lint:fix": "yarn lint --fix",
     "format": "yarn prettier",
     "prettier": "prettier --write .",

packages/superset-ui-core/src/chart/index.ts

@@ -15,6 +15,6 @@ export { default as getChartTransformPropsRegistry } from './registries/ChartTra
 
 export { default as ChartDataProvider } from './components/ChartDataProvider';
 
-export { SetExtraFormDataHook } from './types/Base';
+export * from './types/Base';
 export * from './types/TransformFunction';
 export * from './types/QueryResponse';

packages/superset-ui-core/src/chart/models/ChartPlugin.ts

@@ -8,6 +8,7 @@ import getChartControlPanelRegistry from '../registries/ChartControlPanelRegistr
 import getChartTransformPropsRegistry from '../registries/ChartTransformPropsRegistrySingleton';
 import { BuildQueryFunction, TransformProps } from '../types/TransformFunction';
 import { ChartControlPanel } from './ChartControlPanel';
+import { ChartProps } from '..';
 
 function IDENTITY<T>(x: T) {
   return x;
@@ -20,16 +21,19 @@ export type PromiseOrValueLoader<T> = () => PromiseOrValue<T>;
 export type ChartType = ComponentType<any>;
 type ValueOrModuleWithValue<T> = T | { default: T };
 
-interface ChartPluginConfig<T extends QueryFormData> {
+interface ChartPluginConfig<
+  FormData extends QueryFormData = QueryFormData,
+  Props extends ChartProps = ChartProps
+> {
   metadata: ChartMetadata;
   /** Use buildQuery for immediate value. For lazy-loading, use loadBuildQuery. */
-  buildQuery?: BuildQueryFunction<T>;
+  buildQuery?: BuildQueryFunction<FormData>;
   /** Use loadBuildQuery for dynamic import (lazy-loading) */
-  loadBuildQuery?: PromiseOrValueLoader<ValueOrModuleWithValue<BuildQueryFunction<T>>>;
+  loadBuildQuery?: PromiseOrValueLoader<ValueOrModuleWithValue<BuildQueryFunction<FormData>>>;
   /** Use transformProps for immediate value. For lazy-loading, use loadTransformProps.  */
-  transformProps?: TransformProps;
+  transformProps?: TransformProps<Props>;
   /** Use loadTransformProps for dynamic import (lazy-loading) */
-  loadTransformProps?: PromiseOrValueLoader<ValueOrModuleWithValue<TransformProps>>;
+  loadTransformProps?: PromiseOrValueLoader<ValueOrModuleWithValue<TransformProps<Props>>>;
   /** Use Chart for immediate value. For lazy-loading, use loadChart. */
   Chart?: ChartType;
   /** Use loadChart for dynamic import (lazy-loading) */
@@ -54,18 +58,21 @@ function sanitizeLoader<T>(
   };
 }
 
-export default class ChartPlugin<T extends QueryFormData = QueryFormData> extends Plugin {
+export default class ChartPlugin<
+  FormData extends QueryFormData = QueryFormData,
+  Props extends ChartProps = ChartProps
+> extends Plugin {
   controlPanel: ChartControlPanel;
 
   metadata: ChartMetadata;
 
-  loadBuildQuery?: PromiseOrValueLoader<BuildQueryFunction<T>>;
+  loadBuildQuery?: PromiseOrValueLoader<BuildQueryFunction<FormData>>;
 
-  loadTransformProps: PromiseOrValueLoader<TransformProps>;
+  loadTransformProps: PromiseOrValueLoader<TransformProps<Props>>;
 
   loadChart: PromiseOrValueLoader<ChartType>;
 
-  constructor(config: ChartPluginConfig<T>) {
+  constructor(config: ChartPluginConfig<FormData, Props>) {
     super();
     const {
       metadata,
@@ -95,26 +102,24 @@ export default class ChartPlugin<T extends QueryFormData = QueryFormData> extend
   }
 
   register() {
-    const { key = isRequired('config.key') } = this.config;
-    getChartMetadataRegistry().registerValue(key as string, this.metadata);
-    getChartComponentRegistry().registerLoader(key as string, this.loadChart);
-    getChartControlPanelRegistry().registerValue(key as string, this.controlPanel);
-    getChartTransformPropsRegistry().registerLoader(key as string, this.loadTransformProps);
+    const key: string = this.config.key || isRequired('config.key');
+    getChartMetadataRegistry().registerValue(key, this.metadata);
+    getChartComponentRegistry().registerLoader(key, this.loadChart);
+    getChartControlPanelRegistry().registerValue(key, this.controlPanel);
+    getChartTransformPropsRegistry().registerLoader(key, this.loadTransformProps);
     if (this.loadBuildQuery) {
-      getChartBuildQueryRegistry().registerLoader(key as string, this.loadBuildQuery);
+      getChartBuildQueryRegistry().registerLoader(key, this.loadBuildQuery);
     }
-
     return this;
   }
 
   unregister() {
-    const { key = isRequired('config.key') } = this.config;
-    getChartMetadataRegistry().remove(key as string);
-    getChartComponentRegistry().remove(key as string);
-    getChartControlPanelRegistry().remove(key as string);
-    getChartTransformPropsRegistry().remove(key as string);
-    getChartBuildQueryRegistry().remove(key as string);
-
+    const key: string = this.config.key || isRequired('config.key');
+    getChartMetadataRegistry().remove(key);
+    getChartComponentRegistry().remove(key);
+    getChartControlPanelRegistry().remove(key);
+    getChartTransformPropsRegistry().remove(key);
+    getChartBuildQueryRegistry().remove(key);
     return this;
   }
 

packages/superset-ui-core/src/chart/models/ChartProps.ts

@@ -56,7 +56,7 @@ export interface ChartPropsConfig {
 const DEFAULT_WIDTH = 800;
 const DEFAULT_HEIGHT = 600;
 
-export default class ChartProps {
+export default class ChartProps<FormData extends RawFormData = RawFormData> {
   static createSelector: () => ChartPropsSelector;
 
   annotationData: AnnotationData;
@@ -69,7 +69,7 @@ export default class ChartProps {
 
   formData: CamelCaseFormData;
 
-  rawFormData: RawFormData;
+  rawFormData: FormData;
 
   height: number;
 
@@ -79,11 +79,11 @@ export default class ChartProps {
 
   width: number;
 
-  constructor(config: ChartPropsConfig = {}) {
+  constructor(config: ChartPropsConfig & { formData?: FormData } = {}) {
     const {
       annotationData = {},
       datasource = {},
-      formData = {},
+      formData = {} as FormData,
       hooks = {},
       initialValues = {},
       queriesData = [],

packages/superset-ui-core/src/chart/registries/ChartTransformPropsRegistrySingleton.ts

@@ -1,7 +1,7 @@
 import { Registry, makeSingleton, OverwritePolicy } from '../..';
 import { TransformProps } from '../types/TransformFunction';
 
-class ChartTransformPropsRegistry extends Registry<TransformProps> {
+class ChartTransformPropsRegistry extends Registry<TransformProps<any>> {
   constructor() {
     super({ name: 'ChartTransformProps', overwritePolicy: OverwritePolicy.WARN });
   }

packages/superset-ui-core/src/chart/types/QueryResponse.ts

@@ -1,7 +1,7 @@
 /**
  * Types for query response
  */
-import { DataRecordValue, DataRecord } from '../../types';
+import { DataRecordValue, DataRecord, ChartDataResponseResult } from '../../types';
 import { PlainObject } from './Base';
 
 export interface TimeseriesDataRecord extends DataRecord {
@@ -13,5 +13,15 @@ export interface DataRecordFilters {
   [key: string]: DataRecordValue[];
 }
 
-// the response json from query API
-export type QueryData = PlainObject;
+/**
+ * Legacy queried data for charts. List of arbitrary dictionaries generated
+ * by `viz.py`.
+ * TODO: clean this up when all charts have been migrated to v1 API.
+ */
+export type LegacyQueryData = PlainObject;
+
+/**
+ * Ambiguous query data type. Reserved for the generic QueryFormData.
+ * Don't use this for a specific chart (since you know which API it uses already).
+ */
+export type QueryData = LegacyQueryData | ChartDataResponseResult;

packages/superset-ui-core/src/chart/types/TransformFunction.ts

@@ -1,15 +1,13 @@
 import { QueryFormData, QueryContext } from '../..';
 import ChartProps from '../models/ChartProps';
+import { PlainObject } from './Base';
 
-export interface PlainProps {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  [key: string]: any;
-}
+export type PlainProps = PlainObject;
 
 type TransformFunction<Input = PlainProps, Output = PlainProps> = (x: Input) => Output;
 
 export type PreTransformProps = TransformFunction<ChartProps, ChartProps>;
-export type TransformProps = TransformFunction<ChartProps>;
+export type TransformProps<Props extends ChartProps = ChartProps> = TransformFunction<Props>;
 export type PostTransformProps = TransformFunction;
 
 export type BuildQueryFunction<T extends QueryFormData> = (formData: T) => QueryContext;

packages/superset-ui-core/src/query/buildQueryObject.ts

@@ -38,7 +38,7 @@ export default function buildQueryObject<T extends QueryFormData>(
 
   const numericRowLimit = Number(row_limit);
   const numericRowOffset = Number(row_offset);
-  const { metrics, groupby, columns } = extractQueryFields(residualFormData, queryFields);
+  const { metrics, columns } = extractQueryFields(residualFormData, queryFields);
 
   const extras = extractExtras(formData);
   const extrasAndfilters = processFilters({
@@ -54,7 +54,6 @@ export default function buildQueryObject<T extends QueryFormData>(
     ...extras,
     ...extrasAndfilters,
     annotation_layers,
-    groupby,
     columns,
     metrics,
     order_desc: typeof order_desc === 'undefined' ? true : order_desc,

packages/superset-ui-core/src/query/extractQueryFields.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 import { DTTM_ALIAS } from './buildQueryObject';
-import { QueryFields, QueryFieldAliases, FormDataResidual } from './types/QueryFormData';
+import { QueryFields, QueryFieldAliases, FormDataResidual, QueryMode } from './types/QueryFormData';
 
 /**
  * Extra SQL query related fields from chart form data.
@@ -34,7 +34,6 @@ export default function extractQueryFields(
   const queryFieldAliases: QueryFieldAliases = {
     /** These are predefined for backward compatibility */
     metric: 'metrics',
-    percent_metrics: 'metrics',
     metric_2: 'metrics',
     secondary_metric: 'metrics',
     x: 'metrics',
@@ -46,26 +45,45 @@ export default function extractQueryFields(
   };
   const finalQueryFields: QueryFields = {
     columns: [],
-    groupby: [],
     metrics: [],
   };
+  const { query_mode: queryMode, include_time: includeTime, ...restFormData } = formData;
 
-  Object.entries(formData).forEach(([key, value]) => {
-    const normalizedKey = queryFieldAliases[key] || key;
-    if (normalizedKey in finalQueryFields) {
-      if (normalizedKey === 'metrics') {
-        finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey].concat(value);
-      } else {
-        // currently the groupby and columns field only accept pre-defined columns (string shortcut)
-        finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey]
-          .concat(value)
-          .filter(x => typeof x === 'string' && x);
-      }
+  Object.entries(restFormData).forEach(([key, value]) => {
+    // ignore `null` or `undefined` value
+    if (value == null) {
+      return;
+    }
+
+    let normalizedKey: string = queryFieldAliases[key] || key;
+
+    // ignore groupby and metrics when in raw records mode
+    if (
+      queryMode === QueryMode.raw &&
+      (normalizedKey === 'groupby' || normalizedKey === 'metrics')
+    ) {
+      return;
+    }
+    // ignore columns when (specifically) in aggregate mode.
+    if (queryMode === QueryMode.aggregate && normalizedKey === 'columns') {
+      return;
+    }
+    // groupby has been deprecated: https://github.com/apache/superset/pull/9366
+    if (normalizedKey === 'groupby') {
+      normalizedKey = 'columns';
+    }
+    if (normalizedKey === 'metrics') {
+      finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey].concat(value);
+    } else if (normalizedKey === 'columns') {
+      // currently the columns field only accept pre-defined columns (string shortcut)
+      finalQueryFields[normalizedKey] = finalQueryFields[normalizedKey]
+        .concat(value)
+        .filter(x => typeof x === 'string' && x);
     }
   });
 
-  if (formData.include_time && !finalQueryFields.groupby.includes(DTTM_ALIAS)) {
-    finalQueryFields.groupby.unshift(DTTM_ALIAS);
+  if (includeTime && !finalQueryFields.columns.includes(DTTM_ALIAS)) {
+    finalQueryFields.columns.unshift(DTTM_ALIAS);
   }
 
   return finalQueryFields;

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

@@ -43,7 +43,7 @@ export type AdhocMetric = AdhocMetricSimple | AdhocMetricSQL;
 /**
  * Select a predefined metric by its `metric_name`.
  */
-export type PredefinedMetric = string;
+export type SavedMetric = string;
 
 /**
  * Metric definition stored in datasource metadata.