Skip to content

Commit

Permalink
refactor: preparation for time section migration (#21766)
Browse files Browse the repository at this point in the history
  • Loading branch information
zhaoyongjie authored Oct 12, 2022
1 parent bd3166b commit 8f61e3c
Show file tree
Hide file tree
Showing 43 changed files with 416 additions and 377 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ export { legacySortBy } from './shared-controls/legacySortBy';
export * from './shared-controls/emitFilterControl';
export * from './shared-controls/components';
export * from './types';
export { xAxisMixin, temporalColumnMixin } from './shared-controls/mixins';
export * from './shared-controls/mixins';
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
ContributionType,
FeatureFlag,
isFeatureEnabled,
t,
} from '@superset-ui/core';
import { ContributionType, hasGenericChartAxes, t } from '@superset-ui/core';
import { ControlPanelSectionConfig } from '../types';
import { emitFilterControl } from '../shared-controls/emitFilterControl';

export const echartsTimeSeriesQuery: ControlPanelSectionConfig = {
label: t('Query'),
expanded: true,
controlSetRows: [
[isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES) ? 'x_axis' : null],
[
isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
? 'time_grain_sqla'
: null,
],
[hasGenericChartAxes ? 'x_axis' : null],
[hasGenericChartAxes ? 'time_grain_sqla' : null],
['metrics'],
['groupby'],
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core';
import { hasGenericChartAxes, t } from '@superset-ui/core';
import { ControlPanelSectionConfig } from '../types';

// A few standard controls sections that are used internally.
Expand All @@ -42,11 +42,7 @@ export const genericTime: ControlPanelSectionConfig = {
...baseTimeSection,
controlSetRows: [
['granularity_sqla'],
[
isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
? null
: 'time_grain_sqla',
],
[hasGenericChartAxes ? null : 'time_grain_sqla'],
['time_range'],
],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
SharedControlConfig,
Dataset,
Metric,
isDataset,
} from '../types';
import { DATASET_TIME_COLUMN_OPTION, TIME_FILTER_LABELS } from '../constants';
import {
Expand Down Expand Up @@ -138,8 +139,8 @@ export const dndAdhocFilterControl: SharedControlConfig<
default: [],
description: '',
mapStateToProps: ({ datasource, form_data }) => ({
columns: datasource?.columns[0]?.hasOwnProperty('filterable')
? (datasource as Dataset)?.columns.filter(c => c.filterable)
columns: isDataset(datasource)
? datasource.columns.filter(c => c.filterable)
: datasource?.columns || [],
savedMetrics: defineSavedMetrics(datasource),
// current active adhoc metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,18 @@
*/
import { isEmpty } from 'lodash';
import {
FeatureFlag,
t,
getCategoricalSchemeRegistry,
getSequentialSchemeRegistry,
isFeatureEnabled,
SequentialScheme,
legacyValidateInteger,
ComparisionType,
isAdhocColumn,
isPhysicalColumn,
ensureIsArray,
isDefined,
hasGenericChartAxes,
NO_TIME_RANGE,
} from '@superset-ui/core';

import {
Expand Down Expand Up @@ -205,7 +205,7 @@ const time_grain_sqla: SharedControlConfig<'SelectControl'> = {
choices: (datasource as Dataset)?.time_grain_sqla || [],
}),
visibility: ({ controls }) => {
if (!isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)) {
if (!hasGenericChartAxes) {
return true;
}

Expand All @@ -227,7 +227,7 @@ const time_range: SharedControlConfig<'DateFilterControl'> = {
type: 'DateFilterControl',
freeForm: true,
label: TIME_FILTER_LABELS.time_range,
default: t('No filter'), // this value is translated, but the backend wouldn't understand a translated value?
default: NO_TIME_RANGE, // this value is an empty filter constant so shouldn't translate it.
description: t(
'The time range for the visualization. All relative times, e.g. "Last month", ' +
'"Last 7 days", "now", etc. are evaluated on the server using the server\'s ' +
Expand All @@ -236,9 +236,6 @@ const time_range: SharedControlConfig<'DateFilterControl'> = {
"using the engine's local timezone. Note one can explicitly set the timezone " +
'per the ISO 8601 format if specifying either the start and/or end time.',
),
mapStateToProps: ({ datasource }) => ({
datasource,
}),
};

const row_limit: SharedControlConfig<'SelectControl'> = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
* under the License.
*/
import {
FeatureFlag,
isFeatureEnabled,
hasGenericChartAxes,
QueryFormData,
t,
validateNonEmpty,
Expand All @@ -41,7 +40,7 @@ export const xAxisMixin = {
validators: [validateNonEmpty],
initialValue: (control: ControlState, state: ControlPanelState | null) => {
if (
isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES) &&
hasGenericChartAxes &&
state?.form_data?.granularity_sqla &&
!state.form_data?.x_axis &&
!control?.value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const TestDataset: Dataset = {
column_format: {},
columns: [
{
advanced_data_type: null,
advanced_data_type: undefined,
certification_details: null,
certified_by: null,
column_name: 'num',
Expand All @@ -41,7 +41,7 @@ export const TestDataset: Dataset = {
warning_markdown: null,
},
{
advanced_data_type: null,
advanced_data_type: undefined,
certification_details: null,
certified_by: null,
column_name: 'gender',
Expand All @@ -59,7 +59,7 @@ export const TestDataset: Dataset = {
warning_markdown: null,
},
{
advanced_data_type: null,
advanced_data_type: undefined,
certification_details: null,
certified_by: null,
column_name: 'state',
Expand All @@ -77,7 +77,7 @@ export const TestDataset: Dataset = {
warning_markdown: null,
},
{
advanced_data_type: null,
advanced_data_type: undefined,
certification_details: null,
certified_by: null,
column_name: 'ds',
Expand All @@ -95,7 +95,7 @@ export const TestDataset: Dataset = {
warning_markdown: null,
},
{
advanced_data_type: null,
advanced_data_type: undefined,
certification_details: null,
certified_by: null,
column_name: 'name',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ test('get temporal columns from a Dataset', () => {
expect(getTemporalColumns(TestDataset)).toEqual({
temporalColumns: [
{
advanced_data_type: null,
advanced_data_type: undefined,
certification_details: null,
certified_by: null,
column_name: 'ds',
Expand Down
18 changes: 10 additions & 8 deletions superset-frontend/packages/superset-ui-core/src/query/constants.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
import {
ExtraFormDataAppend,
ExtraFormDataOverrideExtras,
ExtraFormDataOverrideRegular,
ExtraFormDataOverride,
QueryObject,
} from './types';

/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
Expand All @@ -24,7 +16,17 @@ import {
* specific language governing permissions and limitations
* under the License.
*/
import {
ExtraFormDataAppend,
ExtraFormDataOverrideExtras,
ExtraFormDataOverrideRegular,
ExtraFormDataOverride,
QueryObject,
} from './types';

export const DTTM_ALIAS = '__timestamp';
export const DEFAULT_TIME_RANGE = 'No filter'; // TODO: make this configurable per Superset installation
export const NO_TIME_RANGE = 'No filter';

export const EXTRA_FORM_DATA_OVERRIDE_EXTRA_KEYS: (keyof ExtraFormDataOverrideExtras)[] =
['relative_start', 'relative_end', 'time_grain_sqla'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
import {
DTTM_ALIAS,
FeatureFlag,
isFeatureEnabled,
getColumnLabel,
isQueryFormColumn,
QueryFormData,
Expand All @@ -26,6 +28,10 @@ import {
export const isXAxisSet = (formData: QueryFormData) =>
isQueryFormColumn(formData.x_axis);

export const hasGenericChartAxes = isFeatureEnabled(
FeatureFlag.GENERIC_CHART_AXES,
);

export const getXAxis = (formData: QueryFormData): string | undefined => {
// The formData should be "raw form_data" -- the snake_case version of formData rather than camelCase.
if (!(formData.granularity_sqla || formData.x_axis)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export { default as getMetricLabel } from './getMetricLabel';
export { default as DatasourceKey } from './DatasourceKey';
export { default as normalizeOrderBy } from './normalizeOrderBy';
export { normalizeTimeColumn } from './normalizeTimeColumn';
export { getXAxis, isXAxisSet } from './getXAxis';
export { getXAxis, isXAxisSet, hasGenericChartAxes } from './getXAxis';

export * from './types/AnnotationLayer';
export * from './types/QueryFormData';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ export interface Column {
expression?: string | null;
database_expression?: string | null;
python_date_format?: string | null;

// used for advanced_data_type
optionName?: string;
filterBy?: string;
value?: string;
advanced_data_type?: string;
}

export function isPhysicalColumn(column?: any): column is PhysicalColumn {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
FeatureFlag,
isFeatureEnabled,
smartDateFormatter,
t,
} from '@superset-ui/core';
import { hasGenericChartAxes, smartDateFormatter, t } from '@superset-ui/core';
import {
ControlPanelConfig,
D3_FORMAT_DOCS,
Expand All @@ -41,12 +36,8 @@ const config: ControlPanelConfig = {
label: t('Query'),
expanded: true,
controlSetRows: [
[isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES) ? 'x_axis' : null],
[
isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
? 'time_grain_sqla'
: null,
],
[hasGenericChartAxes ? 'x_axis' : null],
[hasGenericChartAxes ? 'time_grain_sqla' : null],
['metric'],
['adhoc_filters'],
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@
* under the License.
*/
import React from 'react';
import {
ensureIsArray,
FeatureFlag,
isFeatureEnabled,
t,
} from '@superset-ui/core';
import { ensureIsArray, hasGenericChartAxes, t } from '@superset-ui/core';
import { cloneDeep } from 'lodash';
import {
ControlPanelConfig,
Expand Down Expand Up @@ -292,7 +287,7 @@ function createAdvancedAnalyticsSection(
const config: ControlPanelConfig = {
controlPanelSections: [
sections.genericTime,
isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
hasGenericChartAxes
? {
label: t('Shared query fields'),
expanded: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ import {
Behavior,
ChartMetadata,
ChartPlugin,
FeatureFlag,
isFeatureEnabled,
hasGenericChartAxes,
t,
} from '@superset-ui/core';
import buildQuery from './buildQuery';
Expand Down Expand Up @@ -57,7 +56,7 @@ export default class EchartsTimeseriesChartPlugin extends ChartPlugin<
behaviors: [Behavior.INTERACTIVE_CHART],
category: t('Evolution'),
credits: ['https://echarts.apache.org'],
description: isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
description: hasGenericChartAxes
? 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).',
)
Expand All @@ -70,9 +69,7 @@ export default class EchartsTimeseriesChartPlugin extends ChartPlugin<
AnnotationType.Interval,
AnnotationType.Timeseries,
],
name: isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
? t('Mixed Chart')
: t('Mixed Time-Series'),
name: hasGenericChartAxes ? t('Mixed Chart') : t('Mixed Time-Series'),
thumbnail,
tags: [
t('Advanced-Analytics'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import {
ChartPlugin,
AnnotationType,
Behavior,
isFeatureEnabled,
FeatureFlag,
hasGenericChartAxes,
} from '@superset-ui/core';
import buildQuery from '../buildQuery';
import controlPanel from './controlPanel';
Expand Down Expand Up @@ -54,7 +53,7 @@ export default class EchartsAreaChartPlugin extends ChartPlugin<
behaviors: [Behavior.INTERACTIVE_CHART],
category: t('Evolution'),
credits: ['https://echarts.apache.org'],
description: isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
description: hasGenericChartAxes
? t(
'Area charts are similar to line charts in that they represent variables with the same scale, but area charts stack the metrics on top of each other.',
)
Expand All @@ -68,7 +67,7 @@ export default class EchartsAreaChartPlugin extends ChartPlugin<
AnnotationType.Interval,
AnnotationType.Timeseries,
],
name: isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)
name: hasGenericChartAxes
? t('Area Chart v2')
: t('Time-series Area Chart'),
tags: [
Expand Down
Loading

0 comments on commit 8f61e3c

Please sign in to comment.