Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: preparation for time section migration #21766

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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