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

feat(native-filters): add timegrain and column filter #13484

Merged
merged 10 commits into from
Mar 9, 2021
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
676 changes: 296 additions & 380 deletions superset-frontend/package-lock.json

Large diffs are not rendered by default.

54 changes: 27 additions & 27 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,34 +65,34 @@
"@babel/runtime-corejs3": "^7.12.5",
"@data-ui/sparkline": "^0.0.84",
"@emotion/core": "^10.0.35",
"@superset-ui/chart-controls": "^0.17.14",
"@superset-ui/core": "^0.17.14",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.14",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.14",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.14",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.14",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.14",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.14",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.14",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.14",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.14",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.14",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.14",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.14",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.14",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.14",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.14",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.14",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.14",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.14",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.14",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.14",
"@superset-ui/chart-controls": "^0.17.15",
"@superset-ui/core": "^0.17.15",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.15",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.15",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.15",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.15",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.15",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.15",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.15",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.15",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.15",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.15",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.15",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.15",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.15",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.15",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.15",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.15",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.15",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.15",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.15",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.15",
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.6",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.14",
"@superset-ui/plugin-chart-echarts": "^0.17.14",
"@superset-ui/plugin-chart-table": "^0.17.14",
"@superset-ui/plugin-chart-word-cloud": "^0.17.14",
"@superset-ui/preset-chart-xy": "^0.17.14",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.15",
"@superset-ui/plugin-chart-echarts": "^0.17.15",
"@superset-ui/plugin-chart-table": "^0.17.15",
"@superset-ui/plugin-chart-word-cloud": "^0.17.15",
"@superset-ui/preset-chart-xy": "^0.17.15",
"@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9",
"antd": "^4.9.4",
Expand Down
10 changes: 7 additions & 3 deletions superset-frontend/spec/javascripts/filters/utils_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,13 @@ describe('Filter utils', () => {
expect(getSelectExtraFormData('testCol', ['value'], true, false)).toEqual(
{
append_form_data: {
extras: {
where: '1 = 0',
},
adhoc_filters: [
{
clause: 'WHERE',
expressionType: 'SQL',
sqlExpression: '1 = 0',
},
],
},
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { NativeFiltersState } from 'src/dashboard/reducers/types';
import { DataMaskStateWithId } from 'src/dataMask/types';
import { Layout } from '../../types';
import { getTreeCheckedItems } from '../nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils';
import { FilterValue } from '../nativeFilters/types';

export enum IndicatorStatus {
Unset = 'UNSET',
Expand Down Expand Up @@ -52,7 +53,7 @@ const selectIndicatorValue = (
columnKey: string,
filter: Filter,
datasource: Datasource,
): string[] => {
): FilterValue => {
const values = filter.columns[columnKey];
const arrValues = Array.isArray(values) ? values : [values];

Expand Down Expand Up @@ -132,7 +133,7 @@ const getRejectedColumns = (chart: any): Set<string> =>
export type Indicator = {
column?: string;
name: string;
value: string[];
value: FilterValue;
status: IndicatorStatus;
path: string[];
};
Expand Down Expand Up @@ -185,20 +186,22 @@ export const selectNativeIndicatorsForChart = (
const rejectedColumns = getRejectedColumns(chart);

const getStatus = (
value: string[],
value: FilterValue,
isAffectedByScope: boolean,
column?: string,
): IndicatorStatus => {
// a filter is only considered unset if it's value is null
const hasValue = value !== null;
if (!isAffectedByScope) {
return IndicatorStatus.Unset;
}
if (!column) {
if (!column && hasValue) {
// Filter without datasource
return IndicatorStatus.Applied;
}
if (column && rejectedColumns.has(column))
return IndicatorStatus.Incompatible;
if (column && appliedColumns.has(column) && value.length > 0) {
if (column && appliedColumns.has(column) && hasValue) {
return IndicatorStatus.Applied;
}
return IndicatorStatus.Unset;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const FilterValue: React.FC<FilterProps> = ({
column = {},
}: Partial<{ datasetId: number; column: { name?: string } }> = target;
const { name: groupby } = column;
const hasDataSource = !!(datasetId && groupby);
const hasDataSource = !!datasetId;
const [loading, setLoading] = useState<boolean>(hasDataSource);
useEffect(() => {
const newFormData = getFormData({
Expand Down Expand Up @@ -137,7 +137,7 @@ const FilterValue: React.FC<FilterProps> = ({
width={220}
formData={formData}
// For charts that don't have datasource we need workaround for empty placeholder
queriesData={hasDataSource ? state : [{ data: [null] }]}
queriesData={hasDataSource ? state : [{ data: [{}] }]}
chartType={filterType}
behaviors={[Behavior.NATIVE_FILTER]}
hooks={{ setDataMask }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ const ControlItems: FC<ControlItemsProps> = ({
const controlPanelRegistry = getChartControlPanelRegistry();
const controlItems =
getControlItems(controlPanelRegistry.get(filterType)) ?? [];

return (
<>
{controlItems
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export interface FiltersConfigFormProps {
parentFilters: { id: string; title: string }[];
}

const FILTERS_WITH_ONLY_DATASOURCE = ['filter_timegrain', 'filter_timecolumn'];

/**
* The configuration form for a specific filter.
* Assigns field values to `filters[filterId]` in the form.
Expand All @@ -104,17 +106,21 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
// @ts-ignore
const hasDatasource = !!nativeFilterItems[formFilter?.filterType]?.value
?.datasourceCount;
const hasColumn =
hasDatasource &&
!FILTERS_WITH_ONLY_DATASOURCE.includes(formFilter?.filterType);

const hasFilledDatasource =
(formFilter?.dataset && formFilter?.column) || !hasDatasource;
!hasDatasource ||
(formFilter?.dataset?.value && (formFilter?.column || !hasColumn));

useBackendFormUpdate(form, filterId, filterToEdit, hasDatasource);
useBackendFormUpdate(form, filterId, filterToEdit, hasDatasource, hasColumn);

const initDatasetId = filterToEdit?.targets[0]?.datasetId;
const initColumn = filterToEdit?.targets[0]?.column?.name;
const newFormData = getFormData({
datasetId: formFilter?.dataset?.value,
groupby: formFilter?.column,
groupby: hasColumn ? formFilter?.column : undefined,
defaultValue: formFilter?.defaultValue,
...formFilter,
});
Expand Down Expand Up @@ -204,22 +210,24 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
}}
/>
</StyledFormItem>
<StyledFormItem
// don't show the column select unless we have a dataset
// style={{ display: datasetId == null ? undefined : 'none' }}
name={['filters', filterId, 'column']}
initialValue={initColumn}
label={<StyledLabel>{t('Column')}</StyledLabel>}
rules={[{ required: !removed, message: t('Field is required') }]}
data-test="field-input"
>
<ColumnSelect
form={form}
filterId={filterId}
datasetId={formFilter?.dataset?.value}
onChange={forceUpdate}
/>
</StyledFormItem>
{hasColumn && (
<StyledFormItem
// don't show the column select unless we have a dataset
// style={{ display: datasetId == null ? undefined : 'none' }}
name={['filters', filterId, 'column']}
initialValue={initColumn}
label={<StyledLabel>{t('Column')}</StyledLabel>}
rules={[{ required: !removed, message: t('Field is required') }]}
data-test="field-input"
>
<ColumnSelect
form={form}
filterId={filterId}
datasetId={formFilter?.dataset?.value}
onChange={forceUpdate}
/>
</StyledFormItem>
)}
</>
)}
{hasFilledDatasource && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const useBackendFormUpdate = (
filterId: string,
filterToEdit?: Filter,
hasDatasource?: boolean,
hasColumn?: boolean,
) => {
const forceUpdate = useForceUpdate();
const formFilter = (form.getFieldValue('filters') || {})[filterId];
Expand All @@ -42,13 +43,17 @@ export const useBackendFormUpdate = (
}
// No need to check data set change because it cascading update column
// So check that column exists is enough
if (!formFilter?.column) {
if (hasColumn && !formFilter?.column) {
setFilterFieldValues(form, filterId, {
defaultValueQueriesData: [],
defaultValue: resolvedDefaultValue,
});
return;
}
if (!formFilter?.dataset?.value) {
// no need to make chart data request if no dataset is defined
return;
}
const formData = getFormData({
datasetId: formFilter?.dataset?.value,
groupby: formFilter?.column,
Expand All @@ -63,7 +68,8 @@ export const useBackendFormUpdate = (
if (
filterToEdit?.filterType === formFilter?.filterType &&
filterToEdit?.targets[0].datasetId === formFilter?.dataset?.value &&
formFilter?.column === filterToEdit?.targets[0].column?.name
(!hasColumn ||
formFilter?.column === filterToEdit?.targets[0].column?.name)
) {
resolvedDefaultValue = filterToEdit?.defaultValue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { FormInstance } from 'antd/lib/form';
import shortid from 'shortid';
import { FilterRemoval, NativeFiltersForm } from './types';
import { Filter, FilterConfiguration } from '../types';
import { Filter, FilterConfiguration, Target } from '../types';

export const REMOVAL_DELAY_SECS = 5;

Expand Down Expand Up @@ -132,14 +132,12 @@ export const createHandleSave = (
const formInputs = values.filters[id];
// if user didn't open a filter, return the original config
if (!formInputs) return filterConfigMap[id];
let target = {};
const target: Partial<Target> = {};
if (formInputs.dataset) {
target.datasetId = formInputs.dataset.value;
}
if (formInputs.dataset && formInputs.column) {
target = {
datasetId: formInputs.dataset.value,
column: {
name: formInputs.column,
},
};
target.column = { name: formInputs.column };
}
return {
id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ export interface Target {
// clarityColumns?: Column[];
}

export type FilterValue = string | number | (string | number)[] | null;

export interface Filter {
cascadeParentIds: string[];
defaultValue: any;
currentValue?: any;
defaultValue: FilterValue;
currentValue?: FilterValue;
isInstant: boolean;
id: string; // randomly generated at filter creation
name: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ export const getFormData = ({
cascadingFilters?: object;
groupby?: string;
}): Partial<QueryFormData> => {
let otherProps: { datasource?: string; groupby?: string[] } = {};
if (datasetId && groupby) {
otherProps = {
datasource: `${datasetId}__table`,
groupby: [groupby],
};
const otherProps: { datasource?: string; groupby?: string[] } = {};
if (datasetId) {
otherProps.datasource = `${datasetId}__table`;
}
if (groupby) {
otherProps.groupby = [groupby];
}
return {
...controlValues,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import { styled, t, DataMask, Behavior } from '@superset-ui/core';
import { t, DataMask, Behavior } from '@superset-ui/core';
import React, { useEffect, useState } from 'react';
import { Slider } from 'src/common/components';
import { PluginFilterRangeProps } from './types';
import { PluginFilterStylesProps } from '../types';
import { Styles } from '../common';
import { getRangeExtraFormData } from '../../utils';

const Styles = styled.div<PluginFilterStylesProps>`
height: ${({ height }) => height};
width: ${({ width }) => width};
`;

export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
const {
data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default function buildQuery(formData: QueryFormData) {
return buildQueryContext(formData, baseQueryObject => [
{
...baseQueryObject,
apply_fetch_values_predicate: true,
columns: [],
groupby: [],
metrics: [
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/filters/components/Range/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default class RangeFilterPlugin extends ChartPlugin {
constructor() {
const metadata = new ChartMetadata({
name: t('Range filter'),
description: 'Range filter plugin using AntD',
description: t('Range filter plugin using AntD'),
behaviors: [Behavior.CROSS_FILTER, Behavior.NATIVE_FILTER],
thumbnail,
});
Expand Down
Loading