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: remove queryFields in QueryObject and update chart control configs #12091

Merged
merged 4 commits into from
Dec 23, 2020
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 @@ -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

Large diffs are not rendered by default.

56 changes: 28 additions & 28 deletions superset-frontend/package.json
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
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
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
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
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
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
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
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
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
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