Skip to content

Commit

Permalink
refactor: Removes the deprecated GENERIC_CHART_AXES feature flag (apa…
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina authored and sfirke committed Mar 22, 2024
1 parent 858926b commit 6a53665
Show file tree
Hide file tree
Showing 99 changed files with 363 additions and 1,115 deletions.
2 changes: 0 additions & 2 deletions RESOURCES/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ These features are **finished** but currently being tested. They are usable, but
- DRILL_TO_DETAIL
- DYNAMIC_PLUGINS: [(docs)](https://superset.apache.org/docs/installation/running-on-kubernetes)
- ESTIMATE_QUERY_COST
- GENERIC_CHART_AXES
- GLOBAL_ASYNC_QUERIES [(docs)](https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries)
- HORIZONTAL_FILTER_BAR
- PLAYWRIGHT_REPORTS_AND_THUMBNAILS
Expand Down Expand Up @@ -85,5 +84,4 @@ These features flags currently default to True and **will be removed in a future

- DASHBOARD_CROSS_FILTERS
- ENABLE_JAVASCRIPT_CONTROLS
- GENERIC_CHART_AXES
- KV_STORE
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ assists people when migrating to a new version.
- [26377](https://github.com/apache/superset/pull/26377): Removes the deprecated Redirect API that supported short URLs used before the permalink feature.
- [26329](https://github.com/apache/superset/issues/26329): Removes the deprecated `DASHBOARD_NATIVE_FILTERS` feature flag. The previous value of the feature flag was `True` and now the feature is permanently enabled.
- [25510](https://github.com/apache/superset/pull/25510): Reenforces that any newly defined Python data format (other than epoch) must adhere to the ISO 8601 standard (enforced by way of validation at the API and database level) after a previous relaxation to include slashes in addition to dashes. From now on when specifying new columns, dataset owners will need to use a SQL expression instead to convert their string columns of the form %Y/%m/%d etc. to a `DATE`, `DATETIME`, etc. type.
- [26372](https://github.com/apache/superset/issues/26372): Removes the deprecated `GENERIC_CHART_AXES` feature flag. The previous value of the feature flag was `True` and now the feature is permanently enabled.

### Potential Downtime

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,28 +503,28 @@ describe('Drill by modal', () => {
});

it('Line chart', () => {
testEchart('echarts_timeseries_line', 'Time-Series Line Chart', [
testEchart('echarts_timeseries_line', 'Line Chart', [
[70, 93],
[70, 93],
]);
});

it('Area Chart', () => {
testEchart('echarts_area', 'Time-Series Area Chart', [
testEchart('echarts_area', 'Area Chart', [
[70, 93],
[70, 93],
]);
});

it('Time-Series Scatter Chart', () => {
testEchart('echarts_timeseries_scatter', 'Time-Series Scatter Chart', [
it('Scatter Chart', () => {
testEchart('echarts_timeseries_scatter', 'Scatter Chart', [
[70, 93],
[70, 93],
]);
});

it('Time-Series Bar Chart V2', () => {
testEchart('echarts_timeseries_bar', 'Time-Series Bar Chart V2', [
it('Bar Chart V2', () => {
testEchart('echarts_timeseries_bar', 'Bar Chart V2', [
[70, 94],
[362, 68],
]);
Expand Down Expand Up @@ -557,22 +557,22 @@ describe('Drill by modal', () => {
);
});

it('Time-Series Generic Chart', () => {
testEchart('echarts_timeseries', 'Time-Series Generic Chart', [
it('Generic Chart', () => {
testEchart('echarts_timeseries', 'Generic Chart', [
[70, 93],
[70, 93],
]);
});

it('Time-Series Smooth Line Chart', () => {
testEchart('echarts_timeseries_smooth', 'Time-Series Smooth Line Chart', [
it('Smooth Line Chart', () => {
testEchart('echarts_timeseries_smooth', 'Smooth Line Chart', [
[70, 93],
[70, 93],
]);
});

it('Time-Series Step Line Chart', () => {
testEchart('echarts_timeseries_step', 'Time-Series Step Line Chart', [
it('Step Line Chart', () => {
testEchart('echarts_timeseries_step', 'Step Line Chart', [
[70, 93],
[70, 93],
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,13 @@ describe('Drill to detail modal', () => {
});
});

describe('Time-Series Line Chart', () => {
describe('Line Chart', () => {
it('opens the modal with the correct filters', () => {
testTimeChart('echarts_timeseries_line');
});
});

describe('Time-series Bar Chart', () => {
describe('Bar Chart', () => {
it('opens the modal with the correct filters', () => {
interceptSamples();

Expand Down Expand Up @@ -372,13 +372,13 @@ describe('Drill to detail modal', () => {
});
});

describe('Time-Series Area Chart', () => {
describe('Area Chart', () => {
it('opens the modal with the correct filters', () => {
testTimeChart('echarts_area');
});
});

describe('Time-Series Scatter Chart', () => {
describe('Scatter Chart', () => {
it('opens the modal with the correct filters', () => {
testTimeChart('echarts_timeseries_scatter');
});
Expand Down Expand Up @@ -509,19 +509,19 @@ describe('Drill to detail modal', () => {
});
});

describe('Time-Series Generic Chart', () => {
describe('Generic Chart', () => {
it('opens the modal with the correct filters', () => {
testTimeChart('echarts_timeseries');
});
});

describe('Time-Series Smooth Chart', () => {
describe('Smooth Chart', () => {
it('opens the modal with the correct filters', () => {
testTimeChart('echarts_timeseries_smooth');
});
});

describe('Time-Series Step Line Chart', () => {
describe('Step Line Chart', () => {
it('opens the modal with the correct filters', () => {
testTimeChart('echarts_timeseries_step');
});
Expand Down
14 changes: 7 additions & 7 deletions superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,17 @@ export const SUPPORTED_TIER1_CHARTS = [
{ name: 'Pie Chart', viz: 'pie' },
{ name: 'Table', viz: 'table' },
{ name: 'Pivot Table', viz: 'pivot_table_v2' },
{ name: 'Time-Series Line Chart', viz: 'echarts_timeseries_line' },
{ name: 'Time-Series Area Chart', viz: 'echarts_area' },
{ name: 'Time-Series Scatter Chart', viz: 'echarts_timeseries_scatter' },
{ name: 'Time-Series Bar Chart V2', viz: 'echarts_timeseries_bar' },
{ name: 'Line Chart', viz: 'echarts_timeseries_line' },
{ name: 'Area Chart', viz: 'echarts_area' },
{ name: 'Scatter Chart', viz: 'echarts_timeseries_scatter' },
{ name: 'Bar Chart V2', viz: 'echarts_timeseries_bar' },
] as ChartSpec[];

export const SUPPORTED_TIER2_CHARTS = [
{ name: 'Box Plot Chart', viz: 'box_plot' },
{ name: 'Time-Series Generic Chart', viz: 'echarts_timeseries' },
{ name: 'Time-Series Smooth Line Chart', viz: 'echarts_timeseries_smooth' },
{ name: 'Time-Series Step Line Chart', viz: 'echarts_timeseries_step' },
{ name: 'Generic Chart', viz: 'echarts_timeseries' },
{ name: 'Smooth Line Chart', viz: 'echarts_timeseries_smooth' },
{ name: 'Step Line Chart', viz: 'echarts_timeseries_step' },
{ name: 'Funnel Chart', viz: 'funnel' },
{ name: 'Gauge Chart', viz: 'gauge_chart' },
{ name: 'Radar Chart', viz: 'radar' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const config: ControlPanelConfig = {

// For control input types, see: superset-frontend/src/explore/components/controls/index.js
controlPanelSections: [
<%if (chartType === 'timeseries') { %>sections.legacyTimeseriesTime,<% } else { %>sections.legacyRegularTime,<% } %>
<%if (chartType === 'timeseries') { %>sections.legacyTimeseriesTime,<% } %>
{
label: t('Query'),
expanded: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
ensureIsArray,
getMetricLabel,
getXAxisLabel,
hasGenericChartAxes,
isDefined,
PostProcessingSort,
} from '@superset-ui/core';
Expand All @@ -40,7 +39,6 @@ export const sortOperator: PostProcessingFactory<PostProcessingSort> = (
].filter(Boolean);

if (
hasGenericChartAxes &&
isDefined(formData?.x_axis_sort) &&
isDefined(formData?.x_axis_sort_asc) &&
sortableLabels.includes(formData.x_axis_sort) &&
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 { hasGenericChartAxes, t } from '@superset-ui/core';
import { t } from '@superset-ui/core';
import { ControlPanelSectionConfig, ControlSetRow } from '../types';
import {
contributionModeControl,
Expand All @@ -43,24 +43,20 @@ const controlsWithoutXAxis: ControlSetRow[] = [
export const echartsTimeSeriesQuery: ControlPanelSectionConfig = {
label: t('Query'),
expanded: true,
controlSetRows: [
[hasGenericChartAxes ? 'x_axis' : null],
[hasGenericChartAxes ? 'time_grain_sqla' : null],
...controlsWithoutXAxis,
],
controlSetRows: [['x_axis'], ['time_grain_sqla'], ...controlsWithoutXAxis],
};

export const echartsTimeSeriesQueryWithXAxisSort: ControlPanelSectionConfig = {
label: t('Query'),
expanded: true,
controlSetRows: [
[hasGenericChartAxes ? 'x_axis' : null],
[hasGenericChartAxes ? 'time_grain_sqla' : null],
[hasGenericChartAxes ? xAxisForceCategoricalControl : null],
[hasGenericChartAxes ? xAxisSortControl : null],
[hasGenericChartAxes ? xAxisSortAscControl : null],
[hasGenericChartAxes ? xAxisSortSeriesControl : null],
[hasGenericChartAxes ? xAxisSortSeriesAscendingControl : null],
['x_axis'],
['time_grain_sqla'],
[xAxisForceCategoricalControl],
[xAxisSortControl],
[xAxisSortAscControl],
[xAxisSortSeriesControl],
[xAxisSortSeriesAscendingControl],
...controlsWithoutXAxis,
],
};
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 { hasGenericChartAxes, t } from '@superset-ui/core';
import { t } from '@superset-ui/core';
import { ControlPanelSectionConfig } from '../types';

// A few standard controls sections that are used internally.
Expand All @@ -38,24 +38,6 @@ export const legacyTimeseriesTime: ControlPanelSectionConfig = {
],
};

export const genericTime: ControlPanelSectionConfig = hasGenericChartAxes
? { controlSetRows: [] }
: {
...baseTimeSection,
controlSetRows: [
['granularity_sqla'],
['time_grain_sqla'],
['time_range'],
],
};

export const legacyRegularTime: ControlPanelSectionConfig = hasGenericChartAxes
? { controlSetRows: [] }
: {
...baseTimeSection,
controlSetRows: [['granularity_sqla'], ['time_range']],
};

export const datasourceAndVizType: ControlPanelSectionConfig = {
label: t('Datasource & Chart Type'),
expanded: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
import {
ensureIsArray,
hasGenericChartAxes,
NO_TIME_RANGE,
QueryFormData,
t,
Expand All @@ -42,7 +41,6 @@ export const xAxisMixin = {
validators: [validateNonEmpty],
initialValue: (control: ControlState, state: ControlPanelState | null) => {
if (
hasGenericChartAxes &&
state?.form_data?.granularity_sqla &&
!state.form_data?.x_axis &&
!control?.value
Expand Down Expand Up @@ -72,11 +70,9 @@ export const datePickerInAdhocFilterMixin: Pick<
> = {
initialValue: (control: ControlState, state: ControlPanelState | null) => {
// skip initialValue if
// 1) GENERIC_CHART_AXES is disabled
// 2) the time_range control is present (this is the case for legacy charts)
// 3) there was a time filter in adhoc filters
// 1) the time_range control is present (this is the case for legacy charts)
// 2) there was a time filter in adhoc filters
if (
!hasGenericChartAxes ||
state?.controls?.time_range?.value ||
ensureIsArray(control.value).findIndex(
(flt: any) => flt?.operator === 'TEMPORAL_RANGE',
Expand Down Expand Up @@ -105,7 +101,7 @@ export const datePickerInAdhocFilterMixin: Pick<
const temporalColumn =
state?.datasource &&
getTemporalColumns(state.datasource).defaultTemporalColumn;
if (hasGenericChartAxes && temporalColumn) {
if (temporalColumn) {
return [
...ensureIsArray(control.value),
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import {
isPhysicalColumn,
ensureIsArray,
isDefined,
hasGenericChartAxes,
NO_TIME_RANGE,
validateMaxValue,
} from '@superset-ui/core';
Expand Down Expand Up @@ -205,7 +204,7 @@ const time_grain_sqla: SharedControlConfig<'SelectControl'> = {
choices: (datasource as Dataset)?.time_grain_sqla || [],
}),
visibility: ({ controls }) => {
if (!hasGenericChartAxes || !controls?.x_axis) {
if (!controls?.x_axis) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
import { QueryObject, SqlaFormData } from '@superset-ui/core';
import { sortOperator } from '@superset-ui/chart-controls';
import * as supersetCoreModule from '@superset-ui/core';

const formData: SqlaFormData = {
metrics: [
Expand Down Expand Up @@ -54,16 +53,6 @@ const queryObject: QueryObject = {
};

test('should ignore the sortOperator', () => {
// FF is disabled
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: false,
});
expect(sortOperator(formData, queryObject)).toEqual(undefined);

// FF is enabled
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
expect(
sortOperator(
{
Expand All @@ -78,9 +67,6 @@ test('should ignore the sortOperator', () => {
).toEqual(undefined);

// sortOperator doesn't support multiple series
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
expect(
sortOperator(
{
Expand All @@ -98,9 +84,6 @@ test('should ignore the sortOperator', () => {
});

test('should sort by metric', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
expect(
sortOperator(
{
Expand All @@ -123,9 +106,6 @@ test('should sort by metric', () => {
});

test('should sort by axis', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
expect(
sortOperator(
{
Expand All @@ -148,9 +128,6 @@ test('should sort by axis', () => {
});

test('should sort by extra metric', () => {
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
value: true,
});
expect(
sortOperator(
{
Expand Down
Loading

0 comments on commit 6a53665

Please sign in to comment.