Skip to content

Commit

Permalink
refactor: remove queryFields in QueryObject and update chart control …
Browse files Browse the repository at this point in the history 
…configs (#12091)

* Clean up queryFields

* Clean up unused vars

* Bump chart plugins

* Bringing changes in #12147
  • Loading branch information
@ktmud
ktmud authored Dec 23, 2020
1 parent de61859 commit d2da25a
Show file tree
Hide file tree
Showing 19 changed files with 501 additions and 2,717 deletions.
3 changes: 1 addition & 2 deletions superset-frontend/cypress-base/cypress/integration/dashboard/url_params.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('Dashboard form data', () => {
});
});

it('should apply url params and queryFields to slice requests', () => {
it('should apply url params to slice requests', () => {
const aliases = getChartAliases(dashboard.slices);
// wait and verify one-by-one
cy.wait(aliases).then(requests => {
Expand All @@ -48,7 +48,6 @@ describe('Dashboard form data', () => {
if (isLegacyResponse(responseBody)) {
const requestFormData = xhr.request.body;
const requestParams = JSON.parse(requestFormData.get('form_data'));
expect(requestParams).to.have.property('queryFields');
expect(requestParams.url_params).deep.eq(urlParams);
} else {
xhr.request.body.queries.forEach(query => {
Expand Down
2,778 changes: 320 additions & 2,458 deletions superset-frontend/package-lock.json
View file

Large diffs are not rendered by default.

56 changes: 28 additions & 28 deletions superset-frontend/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,35 @@
"@babel/runtime-corejs3": "^7.8.4",
"@data-ui/sparkline": "^0.0.84",
"@emotion/core": "^10.0.35",
"@superset-ui/chart-controls": "^0.15.18",
"@superset-ui/core": "^0.15.19",
"@superset-ui/legacy-plugin-chart-calendar": "^0.15.18",
"@superset-ui/legacy-plugin-chart-chord": "^0.15.18",
"@superset-ui/legacy-plugin-chart-country-map": "^0.15.18",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.15.18",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.15.18",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.15.18",
"@superset-ui/legacy-plugin-chart-histogram": "^0.15.18",
"@superset-ui/legacy-plugin-chart-horizon": "^0.15.18",
"@superset-ui/legacy-plugin-chart-map-box": "^0.15.18",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.15.18",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.15.18",
"@superset-ui/legacy-plugin-chart-partition": "^0.15.18",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.15.18",
"@superset-ui/legacy-plugin-chart-rose": "^0.15.18",
"@superset-ui/legacy-plugin-chart-sankey": "^0.15.18",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.15.18",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.15.18",
"@superset-ui/legacy-plugin-chart-treemap": "^0.15.18",
"@superset-ui/legacy-plugin-chart-world-map": "^0.15.18",
"@superset-ui/legacy-preset-chart-big-number": "^0.15.18",
"@superset-ui/chart-controls": "^0.16.1",
"@superset-ui/core": "^0.16.1",
"@superset-ui/legacy-plugin-chart-calendar": "^0.16.2",
"@superset-ui/legacy-plugin-chart-chord": "^0.16.2",
"@superset-ui/legacy-plugin-chart-country-map": "^0.16.1",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.16.1",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.16.2",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.16.2",
"@superset-ui/legacy-plugin-chart-histogram": "^0.16.2",
"@superset-ui/legacy-plugin-chart-horizon": "^0.16.2",
"@superset-ui/legacy-plugin-chart-map-box": "^0.16.2",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.16.1",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.16.1",
"@superset-ui/legacy-plugin-chart-partition": "^0.16.2",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.16.2",
"@superset-ui/legacy-plugin-chart-rose": "^0.16.2",
"@superset-ui/legacy-plugin-chart-sankey": "^0.16.1",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.16.2",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.16.1",
"@superset-ui/legacy-plugin-chart-treemap": "^0.16.2",
"@superset-ui/legacy-plugin-chart-world-map": "^0.16.2",
"@superset-ui/legacy-preset-chart-big-number": "^0.16.1",
"@superset-ui/legacy-preset-chart-deckgl": "^0.3.2",
"@superset-ui/legacy-preset-chart-nvd3": "^0.15.18",
"@superset-ui/plugin-chart-echarts": "^0.15.18",
"@superset-ui/plugin-chart-table": "^0.15.18",
"@superset-ui/plugin-chart-word-cloud": "^0.15.18",
"@superset-ui/plugin-filter-antd": "^0.15.18",
"@superset-ui/preset-chart-xy": "^0.15.18",
"@superset-ui/legacy-preset-chart-nvd3": "^0.16.1",
"@superset-ui/plugin-chart-echarts": "^0.16.1",
"@superset-ui/plugin-chart-table": "^0.16.1",
"@superset-ui/plugin-chart-word-cloud": "^0.16.1",
"@superset-ui/plugin-filter-antd": "^0.16.1",
"@superset-ui/preset-chart-xy": "^0.16.1",
"@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9",
"antd": "^4.9.4",
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/spec/javascripts/explore/components/ControlPanelsContainer_spec.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,6 @@ describe('ControlPanelsContainer', () => {

it('renders ControlPanelSections', () => {
wrapper = shallow(<ControlPanelsContainer {...getDefaultProps()} />);
expect(wrapper.find(ControlPanelSection)).toHaveLength(6);
expect(wrapper.find(ControlPanelSection)).toHaveLength(5);
});
});
14 changes: 0 additions & 14 deletions superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import { getChartControlPanelRegistry, t } from '@superset-ui/core';
import {
getControlConfig,
getControlState,
getFormDataFromControls,
applyMapStateToPropsToControl,
getAllControlsState,
findControlItem,
} from 'src/explore/controlUtils';
import {
Expand Down Expand Up @@ -198,18 +196,6 @@ describe('controlUtils', () => {
});
});

describe('queryFields', () => {
it('in formData', () => {
const controlsState = getAllControlsState('table', 'table', {}, {});
const formData = getFormDataFromControls(controlsState);
expect(formData.queryFields).toEqual({
all_columns: 'columns',
metric: 'metrics',
metrics: 'metrics',
});
});
});

describe('findControlItem', () => {
it('find control as a string', () => {
const controlItem = findControlItem(
Expand Down
1 change: 0 additions & 1 deletion superset-frontend/spec/javascripts/explore/fixtures.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export const controlPanelSectionsChartOptionsTable = [
name: 'all_columns',
config: {
type: 'SelectControl',
queryField: 'columns',
multi: true,
label: t('Columns'),
default: [],
Expand Down
20 changes: 11 additions & 9 deletions superset-frontend/src/explore/controlUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@ import { expandControlConfig } from '@superset-ui/chart-controls';
import * as SECTIONS from './controlPanels/sections';

export function getFormDataFromControls(controlsState) {
const formData = { queryFields: {} };
const formData = {};
Object.keys(controlsState).forEach(controlName => {
const control = controlsState[controlName];
formData[controlName] = control.value;
if (control.hasOwnProperty('queryField')) {
formData.queryFields[controlName] = control.queryField;
}
});
return formData;
}
Expand Down Expand Up @@ -193,20 +190,25 @@ const getMemoizedSectionsToRender = memoizeOne(
}
});

const { datasourceAndVizType, sqlaTimeSeries, druidTimeSeries } = sections;
const timeSection =
datasourceType === 'table' ? sqlaTimeSeries : druidTimeSeries;
const { datasourceAndVizType } = sections;
// list of datasource-specific controls that should be removed
const invalidControls =
datasourceType === 'table'
? ['granularity', 'druid_time_origin']
: ['granularity_sqla', 'time_grain_sqla'];

return []
.concat(datasourceAndVizType, timeSection, controlPanelSections)
.concat(datasourceAndVizType, controlPanelSections)
.filter(section => !!section)
.map(section => {
const { controlSetRows } = section;
return {
...section,
controlSetRows:
controlSetRows?.map(row =>
row.map(item => expandControlConfig(item, controlOverrides)),
row
.filter(control => !invalidControls.includes(control))
.map(item => expandControlConfig(item, controlOverrides)),
) || [],
};
});
Expand Down
2 changes: 0 additions & 2 deletions superset-frontend/src/explore/controls.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ const timeColumnOption = {

const groupByControl = {
type: 'SelectControl',
queryField: 'groupby',
multi: true,
freeForm: true,
label: t('Group by'),
Expand Down Expand Up @@ -150,7 +149,6 @@ const groupByControl = {

const metrics = {
type: 'MetricsControl',
queryField: 'metrics',
multi: true,
label: t('Metrics'),
validators: [validateNonEmpty],
Expand Down
2 changes: 0 additions & 2 deletions superset-frontend/src/explore/store.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ export function applyDefaultFormData(inputFormData) {
}
});

// always use dynamically generated queryFields
formData.queryFields = controlFormData.queryFields;
return formData;
}

Expand Down
1 change: 1 addition & 0 deletions superset-frontend/webpack.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ const plugins = [
new ForkTsCheckerWebpackPlugin({
eslint: true,
checkSyntacticErrors: true,
memoryLimit: 4096,
}),

new CopyPlugin({
Expand Down
34 changes: 24 additions & 10 deletions superset/common/query_object.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@
from superset import app, is_feature_enabled
from superset.exceptions import QueryObjectValidationError
from superset.typing import Metric
from superset.utils import core as utils, pandas_postprocessing
from superset.utils import pandas_postprocessing
from superset.utils.core import (
DTTM_ALIAS,
get_since_until,
json_int_dttm_ser,
parse_human_timedelta,
)
from superset.views.utils import get_time_range_endpoints

config = app.config
Expand Down Expand Up @@ -90,7 +96,7 @@ def __init__(
filters: Optional[List[Dict[str, Any]]] = None,
time_range: Optional[str] = None,
time_shift: Optional[str] = None,
is_timeseries: bool = False,
is_timeseries: Optional[bool] = None,
timeseries_limit: int = 0,
row_limit: Optional[int] = None,
row_offset: Optional[int] = None,
Expand All @@ -114,7 +120,7 @@ def __init__(
]
self.applied_time_extras = applied_time_extras or {}
self.granularity = granularity
self.from_dttm, self.to_dttm = utils.get_since_until(
self.from_dttm, self.to_dttm = get_since_until(
relative_start=extras.get(
"relative_start", config["DEFAULT_RELATIVE_START_TIME"]
),
Expand All @@ -124,20 +130,28 @@ def __init__(
time_range=time_range,
time_shift=time_shift,
)
self.is_timeseries = is_timeseries
# is_timeseries is True if time column is in groupby
self.is_timeseries = (
is_timeseries
if is_timeseries is not None
else (DTTM_ALIAS in groupby if groupby else False)
)
self.time_range = time_range
self.time_shift = utils.parse_human_timedelta(time_shift)
self.time_shift = parse_human_timedelta(time_shift)
self.post_processing = [
post_proc for post_proc in post_processing or [] if post_proc
]
if not is_sip_38:
self.groupby = groupby or []

# Temporary solution for backward compatibility issue due the new format of
# non-ad-hoc metric which needs to adhere to superset-ui per
# https://git.io/Jvm7P.
# Support metric reference/definition in the format of
# 1. 'metric_name' - name of predefined metric
# 2. { label: 'label_name' } - legacy format for a predefined metric
# 3. { expressionType: 'SIMPLE' | 'SQL', ... } - adhoc metric
self.metrics = [
metric if "expressionType" in metric else metric["label"] # type: ignore
metric
if isinstance(metric, str) or "expressionType" in metric
else metric["label"] # type: ignore
for metric in metrics
]

Expand Down Expand Up @@ -267,7 +281,7 @@ def cache_key(self, **extra: Any) -> str:
@staticmethod
def json_dumps(obj: Any, sort_keys: bool = False) -> str:
return json.dumps(
obj, default=utils.json_int_dttm_ser, ignore_nan=True, sort_keys=sort_keys
obj, default=json_int_dttm_ser, ignore_nan=True, sort_keys=sort_keys
)

def exec_post_processing(self, df: DataFrame) -> DataFrame:
Expand Down
6 changes: 5 additions & 1 deletion tests/base_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ def post_assert_metric(
return rv


def get_table_by_name(name: str) -> SqlaTable:
return db.session.query(SqlaTable).filter_by(table_name=name).one()


@pytest.fixture
def logged_in_admin():
"""Fixture with app context and logged in admin user."""
Expand Down Expand Up @@ -228,7 +232,7 @@ def get_slice(

@staticmethod
def get_table_by_name(name: str) -> SqlaTable:
return db.session.query(SqlaTable).filter_by(table_name=name).one()
return get_table_by_name(name)

@staticmethod
def get_database_by_id(db_id: int) -> Database:
Expand Down
Loading

0 comments on commit d2da25a

Please sign in to comment.