This is an automated email from the ASF dual-hosted git repository. amitmiran pushed a commit to branch 1.3 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 241fd945d55e88d51589c83586155395612bc82b Author: Ville Brofeldt <[email protected]> AuthorDate: Thu Jul 1 13:38:56 2021 +0300 chore(native-filters): remove instant filtering option (#15365) * chore(native-filters): remove instant filtering option * fix test (cherry picked from commit f2866471407e0fa2519686750242f1af0878feea) --- .../spec/fixtures/mockNativeFilters.ts | 3 --- .../dashboard/fixtures/mockNativeFilters.ts | 1 - .../nativeFilters/FilterBar/FilterBar.test.tsx | 25 +++++++++++++--------- .../components/nativeFilters/FilterBar/index.tsx | 7 +++--- .../FiltersConfigForm/FiltersConfigForm.tsx | 12 +---------- .../FiltersConfigForm/getControlItemsMap.test.tsx | 1 - .../FiltersConfigModal/FiltersConfigModal.test.tsx | 6 ------ .../nativeFilters/FiltersConfigModal/types.ts | 1 - .../nativeFilters/FiltersConfigModal/utils.ts | 14 ------------ .../dashboard/components/nativeFilters/types.ts | 1 - 10 files changed, 19 insertions(+), 52 deletions(-) diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index afc4959..e087072 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -45,7 +45,6 @@ export const nativeFilters: NativeFiltersState = { rootPath: ['ROOT_ID'], excluded: [], }, - isInstant: true, controlValues: { multiSelect: false, enableEmptyFilter: false, @@ -79,7 +78,6 @@ export const nativeFilters: NativeFiltersState = { enableEmptyFilter: false, inverseSelection: false, }, - isInstant: true, }, }, }; @@ -136,7 +134,6 @@ export const singleNativeFiltersState = { cascadeParentIds: [], scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] }, inverseSelection: false, - isInstant: true, allowsMultipleValues: false, isRequired: false, }, diff --git a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts index 8a106a2..0bf022d 100644 --- a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts @@ -62,7 +62,6 @@ export const nativeFiltersInfo: NativeFiltersState = { rootPath: [], excluded: [], }, - isInstant: true, controlValues: { allowsMultipleValues: true, isRequired: false, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx index 17fc50e..4507064 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx @@ -53,7 +53,7 @@ class MainPreset extends Preset { } } -fetchMock.get(`glob:*/api/v1/dataset/1`, { +fetchMock.get('glob:*/api/v1/dataset/7', { description_columns: {}, id: 1, label_columns: { @@ -156,8 +156,7 @@ describe('FilterBar', () => { "defaultDataMask":{"filterState":{"value":null}}, "controlValues":{}, "cascadeParentIds":[], - "scope":{"rootPath":["ROOT_ID"],"excluded":[]}, - "isInstant":false + "scope":{"rootPath":["ROOT_ID"],"excluded":[]} }], "filter_sets_configuration":[{ "name":"${FILTER_SET_NAME}", @@ -168,17 +167,16 @@ describe('FilterBar', () => { "name":"${FILTER_NAME}", "filterType":"filter_time", "targets":[{}], - "defaultDataMask":{"filterState":{"value":"Last week"},"extraFormData":{"time_range":"Last week"}}, + "defaultDataMask":{"filterState":{},"extraFormData":{}}, "controlValues":{}, "cascadeParentIds":[], - "scope":{"rootPath":["ROOT_ID"],"excluded":[]}, - "isInstant":false + "scope":{"rootPath":["ROOT_ID"],"excluded":[]} } }, "dataMask":{ "${filterId}":{ - "extraFormData":{"time_range":"Last week"}, - "filterState":{"value":"Last week"}, + "extraFormData":{}, + "filterState":{}, "ownState":{}, "id":"${filterId}" } @@ -192,7 +190,14 @@ describe('FilterBar', () => { beforeEach(() => { jest.clearAllMocks(); fetchMock.get( - 'http://localhost/api/v1/time_range/?q=%27Last%20day%27', + 'glob:*/api/v1/time_range/?q=%27No%20filter%27', + { + result: { since: '', until: '', timeRange: 'No filter' }, + }, + { overwriteRoutes: true }, + ); + fetchMock.get( + 'glob:*/api/v1/time_range/?q=%27Last%20day%27', { result: { since: '2021-04-13T00:00:00', @@ -203,7 +208,7 @@ describe('FilterBar', () => { { overwriteRoutes: true }, ); fetchMock.get( - 'http://localhost/api/v1/time_range/?q=%27Last%20week%27', + 'glob:*/api/v1/time_range/?q=%27Last%20week%27', { result: { since: '2021-04-07T00:00:00', diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 84d4b56..e175fc0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -216,11 +216,10 @@ const FilterBar: React.FC<FiltersBarProps> = ({ } // force instant updating on initialization for filters with `requiredFirst` is true or instant filters else if ( - (dataMaskSelected[filter.id] && filter.isInstant) || // filterState.value === undefined - means that value not initialized - (dataMask.filterState?.value !== undefined && - dataMaskSelected[filter.id]?.filterState?.value === undefined && - filter.requiredFirst) + dataMask.filterState?.value !== undefined && + dataMaskSelected[filter.id]?.filterState?.value === undefined && + filter.requiredFirst ) { dispatch(updateDataMask(filter.id, dataMask)); } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index 3f45ded..5e95d16 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -44,7 +44,7 @@ import React, { } from 'react'; import { useSelector } from 'react-redux'; import { FormItem } from 'src/components/Form'; -import { Checkbox, Input } from 'src/common/components'; +import { Input } from 'src/common/components'; import { Select } from 'src/components/Select'; import SupersetResourceSelect, { cachedSupersetGet, @@ -816,16 +816,6 @@ const FiltersConfigForm = ( {Object.keys(controlItems) .filter(key => BASIC_CONTROL_ITEMS.includes(key)) .map(key => controlItems[key].element)} - <StyledRowFormItem - name={['filters', filterId, 'isInstant']} - initialValue={filterToEdit?.isInstant || false} - valuePropName="checked" - colon={false} - > - <Checkbox data-test="apply-changes-instantly-checkbox"> - {t('Apply changes instantly')} - </Checkbox> - </StyledRowFormItem> </Collapse.Panel> {((hasDataset && hasAdditionalFilters) || hasMetrics) && ( <Collapse.Panel diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx index b641a58..7c33f39 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx @@ -51,7 +51,6 @@ const formMock: FormInstance = { const filterMock: Filter = { cascadeParentIds: [], defaultDataMask: {}, - isInstant: false, id: 'mock', name: 'mock', scope: { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index caba35a..c9982a7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -112,7 +112,6 @@ const ADVANCED_REGEX = /^advanced$/i; const DEFAULT_VALUE_REGEX = /^filter has default value$/i; const MULTIPLE_REGEX = /^multiple select$/i; const REQUIRED_REGEX = /^required$/i; -const APPLY_INSTANTLY_REGEX = /^apply changes instantly$/i; const HIERARCHICAL_REGEX = /^filter is hierarchical$/i; const FIRST_ITEM_REGEX = /^default to first item$/i; const INVERSE_SELECTION_REGEX = /^inverse selection$/i; @@ -169,7 +168,6 @@ test('renders a value filter type', () => { expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); expect(getCheckbox(REQUIRED_REGEX)).not.toBeChecked(); - expect(getCheckbox(APPLY_INSTANTLY_REGEX)).not.toBeChecked(); expect(getCheckbox(HIERARCHICAL_REGEX)).not.toBeChecked(); expect(getCheckbox(FIRST_ITEM_REGEX)).not.toBeChecked(); expect(getCheckbox(INVERSE_SELECTION_REGEX)).not.toBeChecked(); @@ -194,7 +192,6 @@ test('renders a numerical range filter type', () => { expect(screen.getByText(REQUIRED_REGEX)).toBeInTheDocument(); expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); - expect(getCheckbox(APPLY_INSTANTLY_REGEX)).not.toBeChecked(); expect(getCheckbox(PRE_FILTER_REGEX)).not.toBeChecked(); expect(queryCheckbox(MULTIPLE_REGEX)).not.toBeInTheDocument(); @@ -217,7 +214,6 @@ test('renders a time range filter type', () => { expect(screen.queryByText(COLUMN_REGEX)).not.toBeInTheDocument(); expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); - expect(getCheckbox(APPLY_INSTANTLY_REGEX)).not.toBeChecked(); expect(screen.queryByText(ADVANCED_REGEX)).not.toBeInTheDocument(); }); @@ -234,7 +230,6 @@ test('renders a time column filter type', () => { expect(screen.queryByText(COLUMN_REGEX)).not.toBeInTheDocument(); expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); - expect(getCheckbox(APPLY_INSTANTLY_REGEX)).not.toBeChecked(); expect(screen.queryByText(ADVANCED_REGEX)).not.toBeInTheDocument(); }); @@ -251,7 +246,6 @@ test('renders a time grain filter type', () => { expect(screen.queryByText(COLUMN_REGEX)).not.toBeInTheDocument(); expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); - expect(getCheckbox(APPLY_INSTANTLY_REGEX)).not.toBeChecked(); expect(screen.queryByText(ADVANCED_REGEX)).not.toBeInTheDocument(); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts index 8d40955..cbb2f14 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts @@ -41,7 +41,6 @@ export interface NativeFiltersFormItem { label: string; }; sortMetric: string | null; - isInstant: boolean; adhoc_filters?: AdhocFilter[]; time_range?: string; granularity_sqla?: string; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts index 4a9b5b4..89de8c9 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts @@ -56,18 +56,6 @@ export const validateForm = async ( throw error; } } - const validateInstant = (filterId: string) => { - const isInstant = formValues.filters[filterId] - ? formValues.filters[filterId].isInstant - : filterConfigMap[filterId]?.isInstant; - if (!isInstant) { - addValidationError( - filterId, - 'isInstant', - 'For hierarchical filters changes must be applied instantly', - ); - } - }; const validateCycles = (filterId: string, trace: string[] = []) => { if (trace.includes(filterId)) { @@ -81,7 +69,6 @@ export const validateForm = async ( ? formValues.filters[filterId].parentFilter?.value : filterConfigMap[filterId]?.cascadeParentIds?.[0]; if (parentId) { - validateInstant(parentId); validateCycles(parentId, [...trace, filterId]); } }; @@ -153,7 +140,6 @@ export const createHandleSave = ( ? [formInputs.parentFilter.value] : [], scope: formInputs.scope, - isInstant: formInputs.isInstant, sortMetric: formInputs.sortMetric, }; }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/types.ts index 324cc49..3792d3e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/types.ts @@ -42,7 +42,6 @@ export interface Target { export interface Filter { cascadeParentIds: string[]; defaultDataMask: DataMask; - isInstant: boolean; id: string; // randomly generated at filter creation name: string; scope: Scope;
