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: Removes the deprecated GENERIC_CHART_AXES feature flag #26372

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
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
Loading