diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFilterConfigsFromFormdata_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getFilterConfigsFromFormdata_spec.js index 4122966a0e..244f2433f6 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getFilterConfigsFromFormdata_spec.js +++ b/superset-frontend/spec/javascripts/dashboard/util/getFilterConfigsFromFormdata_spec.js @@ -56,13 +56,25 @@ describe('getFilterConfigsFromFormdata', () => { }); }); - it('should use default value from form_data', () => { + it('should use default value and treat empty defaults as null', () => { const result = getFilterConfigsFromFormdata({ ...testFormdata, show_sqla_time_column: true, + filter_configs: [ + ...testFormdata.filter_configs, + { + asc: false, + clearable: true, + column: 'country', + defaultValue: '', + key: 'foo', + multiple: true, + }, + ], }); expect(result.columns).toMatchObject({ state: ['CA'], + country: null, }); }); diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts index 5b04e249bd..29a8aae2cd 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts +++ b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts @@ -16,9 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { getChartIdsInFilterScope } from '../../util/activeDashboardFilters'; -import { TIME_FILTER_MAP } from '../../../visualizations/FilterBox/FilterBox'; -import { NativeFiltersState, NativeFilterState } from '../../reducers/types'; +import { TIME_FILTER_MAP } from 'src/explore/constants'; +import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; +import { + NativeFiltersState, + NativeFilterState, +} from 'src/dashboard/reducers/types'; export enum IndicatorStatus { Unset = 'UNSET', diff --git a/superset-frontend/src/dashboard/util/getFilterConfigsFromFormdata.js b/superset-frontend/src/dashboard/util/getFilterConfigsFromFormdata.js index 192f7cbc02..807076cfab 100644 --- a/superset-frontend/src/dashboard/util/getFilterConfigsFromFormdata.js +++ b/superset-frontend/src/dashboard/util/getFilterConfigsFromFormdata.js @@ -17,11 +17,11 @@ * under the License. */ /* eslint-disable camelcase */ -import { TIME_FILTER_MAP } from '../../visualizations/FilterBox/FilterBox'; import { FILTER_CONFIG_ATTRIBUTES, TIME_FILTER_LABELS, -} from '../../explore/constants'; + TIME_FILTER_MAP, +} from 'src/explore/constants'; export default function getFilterConfigsFromFormdata(form_data = {}) { const { @@ -35,14 +35,18 @@ export default function getFilterConfigsFromFormdata(form_data = {}) { let configs = filter_configs.reduce( ({ columns, labels }, config) => { let defaultValues = config[FILTER_CONFIG_ATTRIBUTES.DEFAULT_VALUE]; + + // treat empty string as null (no default value) + if (defaultValues === '') { + defaultValues = null; + } + // defaultValue could be ; separated values, // could be null or '' - if ( - config[FILTER_CONFIG_ATTRIBUTES.DEFAULT_VALUE] && - config[FILTER_CONFIG_ATTRIBUTES.MULTIPLE] - ) { + if (defaultValues && config[FILTER_CONFIG_ATTRIBUTES.MULTIPLE]) { defaultValues = config.defaultValue.split(';'); } + const updatedColumns = { ...columns, [config.column]: config.vals || defaultValues, diff --git a/superset-frontend/src/explore/constants.js b/superset-frontend/src/explore/constants.js index 4a771d04be..d9f083bf92 100644 --- a/superset-frontend/src/explore/constants.js +++ b/superset-frontend/src/explore/constants.js @@ -92,3 +92,14 @@ export const FILTER_CONFIG_ATTRIBUTES = { }; export const FILTER_OPTIONS_LIMIT = 1000; + +/** + * Map control names to their key in extra_filters + */ +export const TIME_FILTER_MAP = { + time_range: '__time_range', + granularity_sqla: '__time_col', + time_grain_sqla: '__time_grain', + druid_time_origin: '__time_origin', + granularity: '__granularity', +}; diff --git a/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx b/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx index e87ebb944b..e2dce1e6b5 100644 --- a/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx +++ b/superset-frontend/src/visualizations/FilterBox/FilterBox.jsx @@ -36,19 +36,11 @@ import { FILTER_CONFIG_ATTRIBUTES, FILTER_OPTIONS_LIMIT, TIME_FILTER_LABELS, + TIME_FILTER_MAP, } from 'src/explore/constants'; import './FilterBox.less'; -// maps control names to their key in extra_filters -export const TIME_FILTER_MAP = { - time_range: '__time_range', - granularity_sqla: '__time_col', - time_grain_sqla: '__time_grain', - druid_time_origin: '__time_origin', - granularity: '__granularity', -}; - // a shortcut to a map key, used by many components export const TIME_RANGE = TIME_FILTER_MAP.time_range; @@ -336,10 +328,12 @@ class FilterBox extends React.PureComponent { // Add created options to filtersChoices, even though it doesn't exist, // or these options will exist in query sql but invisible to end user. Object.keys(selectedValues) - .filter( - key => selectedValues.hasOwnProperty(key) && key in filtersChoices, - ) + .filter(key => key in filtersChoices) .forEach(key => { + // empty values are ignored + if (!selectedValues[key]) { + return; + } const choices = filtersChoices[key] || (filtersChoices[key] = []); const choiceIds = new Set(choices.map(f => f.id)); const selectedValuesForKey = Array.isArray(selectedValues[key]) @@ -356,21 +350,21 @@ class FilterBox extends React.PureComponent { }); }); }); - const { key, label } = filterConfig; + const { + key, + label, + [FILTER_CONFIG_ATTRIBUTES.MULTIPLE]: isMultiple, + [FILTER_CONFIG_ATTRIBUTES.DEFAULT_VALUE]: defaultValue, + [FILTER_CONFIG_ATTRIBUTES.CLEARABLE]: isClearable, + [FILTER_CONFIG_ATTRIBUTES.SEARCH_ALL_OPTIONS]: searchAllOptions, + } = filterConfig; const data = filtersChoices[key] || []; let value = selectedValues[key] || null; // Assign default value if required - if ( - value === undefined && - filterConfig[FILTER_CONFIG_ATTRIBUTES.DEFAULT_VALUE] - ) { - if (filterConfig[FILTER_CONFIG_ATTRIBUTES.MULTIPLE]) { - // Support for semicolon-delimited multiple values - value = filterConfig[FILTER_CONFIG_ATTRIBUTES.DEFAULT_VALUE].split(';'); - } else { - value = filterConfig[FILTER_CONFIG_ATTRIBUTES.DEFAULT_VALUE]; - } + if (value === undefined && defaultValue) { + // multiple values are separated by semicolons + value = isMultiple ? defaultValue.split(';') : defaultValue; } return ( @@ -380,8 +374,8 @@ class FilterBox extends React.PureComponent { defaultOptions={this.transformOptions(data)} key={key} placeholder={t('Type or Select [%s]', label)} - isMulti={filterConfig[FILTER_CONFIG_ATTRIBUTES.MULTIPLE]} - isClearable={filterConfig[FILTER_CONFIG_ATTRIBUTES.CLEARABLE]} + isMulti={isMultiple} + isClearable={isClearable} value={value} options={this.transformOptions(data)} onChange={newValue => { @@ -396,8 +390,7 @@ class FilterBox extends React.PureComponent { onBlur={() => this.onFilterMenuClose(key)} onMenuClose={() => this.onFilterMenuClose(key)} selectWrap={ - filterConfig[FILTER_CONFIG_ATTRIBUTES.SEARCH_ALL_OPTIONS] && - data.length >= FILTER_OPTIONS_LIMIT + searchAllOptions && data.length >= FILTER_OPTIONS_LIMIT ? AsyncCreatableSelect : CreatableSelect } diff --git a/superset-frontend/src/visualizations/constants.js b/superset-frontend/src/visualizations/constants.js deleted file mode 100644 index 539e2678bb..0000000000 --- a/superset-frontend/src/visualizations/constants.js +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -export const TIME_CHOICES = [ - '1 hour ago', - '12 hours ago', - '1 day ago', - '7 days ago', - '28 days ago', - '90 days ago', - '1 year ago', -];