feat(plugin-chart-echarts): add support for generic axis to mixed cha…

…rt (apache#20097)

* feat(plugin-chart-echarts): add support for generic axis to mixed chart

* fix tests + add new tests

* address review comments

* simplify control panel

* fix types and tests

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

@@ -341,7 +341,7 @@ export interface ControlPanelSectionConfig {
 }
 
 export interface ControlPanelConfig {
-  controlPanelSections: ControlPanelSectionConfig[];
+  controlPanelSections: (ControlPanelSectionConfig | null)[];
   controlOverrides?: ControlOverrides;
   sectionOverrides?: SectionOverrides;
   onInit?: (state: ControlStateMapping) => void;
@@ -413,3 +413,9 @@ export function isAdhocColumn(
 ): column is AdhocColumn {
   return 'label' in column && 'sqlExpression' in column;
 }
+
+export function isControlPanelSectionConfig(
+  section: ControlPanelSectionConfig | null,
+): section is ControlPanelSectionConfig {
+  return section !== null;
+}

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

@@ -18,10 +18,12 @@
  */
 import { AdhocColumn } from '@superset-ui/core';
 import {
+  ColumnMeta,
+  ControlPanelSectionConfig,
   isAdhocColumn,
   isColumnMeta,
+  isControlPanelSectionConfig,
   isSavedExpression,
-  ColumnMeta,
 } from '../src';
 
 const ADHOC_COLUMN: AdhocColumn = {
@@ -37,37 +39,46 @@ const SAVED_EXPRESSION: ColumnMeta = {
   column_name: 'Saved expression',
   expression: 'case when 1 = 1 then 1 else 2 end',
 };
+const CONTROL_PANEL_SECTION_CONFIG: ControlPanelSectionConfig = {
+  label: 'My Section',
+  description: 'My Description',
+  controlSetRows: [],
+};
 
-describe('isColumnMeta', () => {
-  it('returns false for AdhocColumn', () => {
-    expect(isColumnMeta(ADHOC_COLUMN)).toEqual(false);
-  });
+test('isColumnMeta returns false for AdhocColumn', () => {
+  expect(isColumnMeta(ADHOC_COLUMN)).toEqual(false);
+});
 
-  it('returns true for ColumnMeta', () => {
-    expect(isColumnMeta(COLUMN_META)).toEqual(true);
-  });
+test('isColumnMeta returns true for ColumnMeta', () => {
+  expect(isColumnMeta(COLUMN_META)).toEqual(true);
 });
 
-describe('isAdhocColumn', () => {
-  it('returns true for AdhocColumn', () => {
-    expect(isAdhocColumn(ADHOC_COLUMN)).toEqual(true);
-  });
+test('isAdhocColumn returns true for AdhocColumn', () => {
+  expect(isAdhocColumn(ADHOC_COLUMN)).toEqual(true);
+});
 
-  it('returns false for ColumnMeta', () => {
-    expect(isAdhocColumn(COLUMN_META)).toEqual(false);
-  });
+test('isAdhocColumn returns false for ColumnMeta', () => {
+  expect(isAdhocColumn(COLUMN_META)).toEqual(false);
 });
 
-describe('isSavedExpression', () => {
-  it('returns false for AdhocColumn', () => {
-    expect(isSavedExpression(ADHOC_COLUMN)).toEqual(false);
-  });
+test('isSavedExpression returns false for AdhocColumn', () => {
+  expect(isSavedExpression(ADHOC_COLUMN)).toEqual(false);
+});
 
-  it('returns false for ColumnMeta without expression', () => {
-    expect(isSavedExpression(COLUMN_META)).toEqual(false);
-  });
+test('isSavedExpression returns false for ColumnMeta without expression', () => {
+  expect(isSavedExpression(COLUMN_META)).toEqual(false);
+});
+
+test('isSavedExpression returns true for ColumnMeta with expression', () => {
+  expect(isSavedExpression(SAVED_EXPRESSION)).toEqual(true);
+});
+
+test('isControlPanelSectionConfig returns true for section', () => {
+  expect(isControlPanelSectionConfig(CONTROL_PANEL_SECTION_CONFIG)).toEqual(
+    true,
+  );
+});
 
-  it('returns true for ColumnMeta with expression', () => {
-    expect(isSavedExpression(SAVED_EXPRESSION)).toEqual(true);
-  });
+test('isControlPanelSectionConfig returns true for null value', () => {
+  expect(isControlPanelSectionConfig(null)).toEqual(false);
 });

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts

@@ -18,10 +18,12 @@
  */
 import {
   buildQueryContext,
-  QueryFormData,
-  QueryObject,
+  DTTM_ALIAS,
+  ensureIsArray,
   normalizeOrderBy,
   PostProcessingPivot,
+  QueryFormData,
+  QueryObject,
 } from '@superset-ui/core';
 import {
   pivotOperator,
@@ -39,28 +41,36 @@ import {
 } from '../utils/formDataSuffix';
 
 export default function buildQuery(formData: QueryFormData) {
+  const { x_axis: index } = formData;
+  const is_timeseries = index === DTTM_ALIAS || !index;
   const baseFormData = {
     ...formData,
-    is_timeseries: true,
-    columns: formData.groupby,
-    columns_b: formData.groupby_b,
+    is_timeseries,
   };
+
   const formData1 = removeFormDataSuffix(baseFormData, '_b');
   const formData2 = retainFormDataSuffix(baseFormData, '_b');
 
   const queryContexts = [formData1, formData2].map(fd =>
     buildQueryContext(fd, baseQueryObject => {
       const queryObject = {
         ...baseQueryObject,
-        is_timeseries: true,
+        columns: [...ensureIsArray(index), ...ensureIsArray(fd.groupby)],
+        series_columns: fd.groupby,
+        is_timeseries,
       };
 
       const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison(
         fd,
         queryObject,
       )
         ? timeComparePivotOperator(fd, queryObject)
-        : pivotOperator(fd, queryObject);
+        : pivotOperator(fd, {
+            ...queryObject,
+            columns: fd.groupby,
+            index,
+            is_timeseries,
+          });
 
       const tmpQueryObject = {
         ...queryObject,
@@ -70,9 +80,13 @@ export default function buildQuery(formData: QueryFormData) {
           rollingWindowOperator(fd, queryObject),
           timeCompareOperator(fd, queryObject),
           resampleOperator(fd, queryObject),
-          renameOperator(fd, queryObject),
+          renameOperator(fd, {
+            ...queryObject,
+            columns: fd.groupby,
+            is_timeseries,
+          }),
           flattenOperator(fd, queryObject),
-        ],
+        ].filter(Boolean),
       } as QueryObject;
       return [normalizeOrderBy(tmpQueryObject)];
     }),

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx

@@ -17,7 +17,7 @@
  * under the License.
  */
 import React from 'react';
-import { t } from '@superset-ui/core';
+import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core';
 import { cloneDeep } from 'lodash';
 import {
   ControlPanelConfig,
@@ -31,7 +31,7 @@ import {
 
 import { DEFAULT_FORM_DATA } from './types';
 import { EchartsTimeseriesSeriesType } from '../Timeseries/types';
-import { legendSection, richTooltipSection } from '../controls';
+import { legendSection, richTooltipSection, xAxisControl } from '../controls';
 
 const {
   area,
@@ -278,6 +278,13 @@ function createAdvancedAnalyticsSection(
 const config: ControlPanelConfig = {
   controlPanelSections: [
     sections.legacyTimeseriesTime,
+    isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
+      ? {
+          label: t('Shared query fields'),
+          expanded: true,
+          controlSetRows: [[xAxisControl]],
+        }
+      : null,
     createQuerySection(t('Query A'), ''),
     createAdvancedAnalyticsSection(t('Advanced analytics Query A'), ''),
     createQuerySection(t('Query B'), '_b'),

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/index.ts

@@ -17,19 +17,21 @@
  * under the License.
  */
 import {
-  t,
-  ChartMetadata,
-  ChartPlugin,
   AnnotationType,
   Behavior,
+  ChartMetadata,
+  ChartPlugin,
+  FeatureFlag,
+  isFeatureEnabled,
+  t,
 } from '@superset-ui/core';
 import buildQuery from './buildQuery';
 import controlPanel from './controlPanel';
 import transformProps from './transformProps';
 import thumbnail from './images/thumbnail.png';
 import {
-  EchartsMixedTimeseriesProps,
   EchartsMixedTimeseriesFormData,
+  EchartsMixedTimeseriesProps,
 } from './types';
 
 export default class EchartsTimeseriesChartPlugin extends ChartPlugin<
@@ -55,16 +57,22 @@ export default class EchartsTimeseriesChartPlugin extends ChartPlugin<
         behaviors: [Behavior.INTERACTIVE_CHART],
         category: t('Evolution'),
         credits: ['https://echarts.apache.org'],
-        description: t(
-          'Visualize two different time series using the same x-axis time range. Note that each time series can be visualized differently (e.g. 1 using bars and 1 using a line).',
-        ),
+        description: isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
+          ? t(
+              'Visualize two different series using the same x-axis. Note that both series can be visualized with a different chart type (e.g. 1 using bars and 1 using a line).',
+            )
+          : t(
+              'Visualize two different time series using the same x-axis. Note that each time series can be visualized differently (e.g. 1 using bars and 1 using a line).',
+            ),
         supportedAnnotationTypes: [
           AnnotationType.Event,
           AnnotationType.Formula,
           AnnotationType.Interval,
           AnnotationType.Timeseries,
         ],
-        name: t('Mixed Time-Series'),
+        name: isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
+          ? t('Mixed Chart')
+          : t('Mixed Time-Series'),
         thumbnail,
         tags: [
           t('Advanced-Analytics'),
@@ -73,7 +81,6 @@ export default class EchartsTimeseriesChartPlugin extends ChartPlugin<
           t('Experimental'),
           t('Line'),
           t('Multi-Variables'),
-          t('Predictive'),
           t('Time'),
           t('Transformable'),
         ],

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts

@@ -21,12 +21,15 @@ import {
   AnnotationLayer,
   CategoricalColorNamespace,
   DataRecordValue,
-  TimeseriesDataRecord,
+  DTTM_ALIAS,
+  GenericDataType,
+  getColumnLabel,
   getNumberFormatter,
   isEventAnnotationLayer,
   isFormulaAnnotationLayer,
   isIntervalAnnotationLayer,
   isTimeseriesAnnotationLayer,
+  TimeseriesDataRecord,
 } from '@superset-ui/core';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
 import {
@@ -41,6 +44,8 @@ import {
   currentSeries,
   dedupSeries,
   extractSeries,
+  getAxisType,
+  getColtypesMapping,
   getLegendProps,
 } from '../utils/series';
 import { extractAnnotationLabels } from '../utils/annotation';
@@ -62,7 +67,7 @@ import {
   transformSeries,
   transformTimeseriesAnnotation,
 } from '../Timeseries/transformers';
-import { TIMESERIES_CONSTANTS } from '../constants';
+import { TIMESERIES_CONSTANTS, TIMEGRAIN_TO_TIMESTAMP } from '../constants';
 
 export default function transformProps(
   chartProps: EchartsMixedTimeseriesProps,
@@ -124,24 +129,35 @@ export default function transformProps(
     groupbyB,
     emitFilter,
     emitFilterB,
+    xAxis: xAxisOrig,
     xAxisTitle,
     yAxisTitle,
     xAxisTitleMargin,
     yAxisTitleMargin,
     yAxisTitlePosition,
     sliceId,
+    timeGrainSqla,
   }: EchartsMixedTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
 
   const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
+
+  const xAxisCol =
+    verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS);
+
   const rebasedDataA = rebaseForecastDatum(data1, verboseMap);
   const rawSeriesA = extractSeries(rebasedDataA, {
     fillNeighborValue: stack ? 0 : undefined,
+    xAxis: xAxisCol,
   });
   const rebasedDataB = rebaseForecastDatum(data2, verboseMap);
   const rawSeriesB = extractSeries(rebasedDataB, {
     fillNeighborValue: stackB ? 0 : undefined,
+    xAxis: xAxisCol,
   });
 
+  const dataTypes = getColtypesMapping(queriesData[0]);
+  const xAxisDataType = dataTypes?.[xAxisCol];
+  const xAxisType = getAxisType(xAxisDataType);
   const series: SeriesOption[] = [];
   const formatter = getNumberFormatter(contributionMode ? ',.0%' : yAxisFormat);
   const formatterSecondary = getNumberFormatter(
@@ -255,8 +271,14 @@ export default function transformProps(
     if (max === undefined) max = 1;
   }
 
-  const tooltipTimeFormatter = getTooltipTimeFormatter(tooltipTimeFormat);
-  const xAxisFormatter = getXAxisFormatter(xAxisTimeFormat);
+  const tooltipFormatter =
+    xAxisDataType === GenericDataType.TEMPORAL
+      ? getTooltipTimeFormatter(tooltipTimeFormat)
+      : String;
+  const xAxisFormatter =
+    xAxisDataType === GenericDataType.TEMPORAL
+      ? getXAxisFormatter(xAxisTimeFormat)
+      : String;
 
   const addYAxisTitleOffset = !!(yAxisTitle || yAxisTitleSecondary);
   const addXAxisTitleOffset = !!xAxisTitle;
@@ -298,14 +320,18 @@ export default function transformProps(
       ...chartPadding,
     },
     xAxis: {
-      type: 'time',
+      type: xAxisType,
       name: xAxisTitle,
       nameGap: convertInteger(xAxisTitleMargin),
       nameLocation: 'middle',
       axisLabel: {
         formatter: xAxisFormatter,
         rotate: xAxisLabelRotation,
       },
+      minInterval:
+        xAxisType === 'time' && timeGrainSqla
+          ? TIMEGRAIN_TO_TIMESTAMP[timeGrainSqla]
+          : 0,
     },
     yAxis: [
       {
@@ -350,7 +376,7 @@ export default function transformProps(
           forecastValue.sort((a, b) => b.data[1] - a.data[1]);
         }
 
-        const rows: Array<string> = [`${tooltipTimeFormatter(xValue)}`];
+        const rows: Array<string> = [`${tooltipFormatter(xValue)}`];
         const forecastValues =
           extractForecastValuesFromTooltipParams(forecastValue);