You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2021/05/07 10:07:46 UTC
[superset] branch master updated: feat(native-filters): Auto apply
changes in FiltersConfigModal (#14461)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new e8e838e feat(native-filters): Auto apply changes in FiltersConfigModal (#14461)
e8e838e is described below
commit e8e838e279fe3565bc7a24545d33c808537db6f4
Author: simcha90 <56...@users.noreply.github.com>
AuthorDate: Fri May 7 13:06:59 2021 +0300
feat(native-filters): Auto apply changes in FiltersConfigModal (#14461)
* fix:fix get permission function
* feat: auto apply changes after save filters config modal
* fix: repopulate default values
* test: fix tests
* test: fix tests
* fix: fix CR notes
* fix: fix CR notes
---
.../src/dashboard/actions/nativeFilters.ts | 3 +-
.../nativeFilters/FilterBar/FilterBar.test.tsx | 43 +++++--------
.../FiltersConfigForm/ControlItems.tsx | 73 +++++++++++++---------
.../FiltersConfigForm/FiltersConfigForm.tsx | 9 ++-
.../nativeFilters/FiltersConfigModal/utils.ts | 15 +++--
superset-frontend/src/dataMask/actions.ts | 4 ++
superset-frontend/src/dataMask/reducer.ts | 41 +++++++++---
.../src/filters/components/GroupBy/controlPanel.ts | 1 +
.../src/filters/components/Select/controlPanel.ts | 3 +
9 files changed, 119 insertions(+), 73 deletions(-)
diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts
index 660f422..2afb9fe 100644
--- a/superset-frontend/src/dashboard/actions/nativeFilters.ts
+++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts
@@ -74,6 +74,7 @@ export const setFilterConfiguration = (
filterConfig,
});
const { id, metadata } = getState().dashboardInfo;
+ const oldFilters = getState().nativeFilters?.filters;
// TODO extract this out when makeApi supports url parameters
const updateDashboard = makeApi<
@@ -100,7 +101,7 @@ export const setFilterConfiguration = (
type: SET_FILTER_CONFIG_COMPLETE,
filterConfig,
});
- dispatch(setDataMaskForFilterConfigComplete(filterConfig));
+ dispatch(setDataMaskForFilterConfigComplete(filterConfig, oldFilters));
} catch (err) {
dispatch({ type: SET_FILTER_CONFIG_FAIL, filterConfig });
dispatch({ type: SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, filterConfig });
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 afe0c0a..4507e38 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
@@ -18,7 +18,7 @@
*/
import React from 'react';
-import { render, screen, cleanup } from 'spec/helpers/testing-library';
+import { render, screen } from 'spec/helpers/testing-library';
import { Provider } from 'react-redux';
import userEvent from '@testing-library/user-event';
import {
@@ -71,7 +71,7 @@ const getDateControlTestId = testWithId<string>(
const FILTER_NAME = 'Time filter 1';
const FILTER_SET_NAME = 'New filter set';
-const addFilterFlow = () => {
+const addFilterFlow = async () => {
// open filter config modal
userEvent.click(screen.getByTestId(getTestId('collapsable')));
userEvent.click(screen.getByTestId(getTestId('create-filter')));
@@ -80,6 +80,7 @@ const addFilterFlow = () => {
userEvent.click(screen.getByText('Time filter'));
userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
userEvent.click(screen.getByText('Save'));
+ await screen.findByText('All Filters (1)');
};
const addFilterSetFlow = async () => {
@@ -175,7 +176,7 @@ describe('FilterBar', () => {
});
beforeEach(() => {
- toggleFiltersBar.mockClear();
+ jest.clearAllMocks();
fetchMock.get(
'http://localhost/api/v1/time_range/?q=%27Last%20day%27',
{
@@ -203,11 +204,6 @@ describe('FilterBar', () => {
mockCore.makeApi = jest.fn(() => mockApi);
});
- afterEach(() => {
- cleanup();
- jest.clearAllMocks();
- });
-
const renderWrapper = (props = closedBarProps, state?: object) =>
render(
<Provider store={state ? getMockStore(state) : mockStore}>
@@ -306,14 +302,12 @@ describe('FilterBar', () => {
renderWrapper(openedBarProps, stateWithoutNativeFilters);
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
- addFilterFlow();
+ await addFilterFlow();
- await screen.findByText('All Filters (1)');
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- // TODO: fix flakiness and re-enable
- it.skip('add and apply filter set', async () => {
+ it('add and apply filter set', async () => {
// @ts-ignore
global.featureFlags = {
[FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true,
@@ -321,22 +315,17 @@ describe('FilterBar', () => {
};
renderWrapper(openedBarProps, stateWithoutNativeFilters);
- addFilterFlow();
-
- await screen.findByText('All Filters (1)');
+ await addFilterFlow();
await addFilterSetFlow();
// change filter
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-
+ userEvent.click(await screen.findByText('All Filters (1)'));
await changeFilterValue();
await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(2));
// apply new filter value
- await waitFor(() =>
- expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(),
- );
userEvent.click(screen.getByTestId(getTestId('apply-button')));
await waitFor(() =>
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(),
@@ -352,12 +341,11 @@ describe('FilterBar', () => {
).not.toHaveAttribute('data-selected', 'true');
userEvent.click(screen.getByTestId(getTestId('filter-set-wrapper')));
expect(await screen.findByText('Last week')).toBeInTheDocument();
- expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled();
userEvent.click(screen.getByTestId(getTestId('apply-button')));
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
});
- it.skip('add and edit filter set', async () => {
+ it('add and edit filter set', async () => {
// @ts-ignore
global.featureFlags = {
[FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET]: true,
@@ -365,9 +353,7 @@ describe('FilterBar', () => {
};
renderWrapper(openedBarProps, stateWithoutNativeFilters);
- addFilterFlow();
-
- await screen.findByText('All Filters (1)');
+ await addFilterFlow();
await addFilterSetFlow();
@@ -375,12 +361,13 @@ describe('FilterBar', () => {
userEvent.click(screen.getByText('Edit'));
await changeFilterValue();
- await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(1));
// apply new changes and save them
- expect(
- screen.getByTestId(getTestId('filter-set-edit-save')),
- ).toBeDisabled();
+ await waitFor(() =>
+ expect(
+ screen.getByTestId(getTestId('filter-set-edit-save')),
+ ).toBeDisabled(),
+ );
expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled();
userEvent.click(screen.getByTestId(getTestId('apply-button')));
expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
index 82a46bf..ae20a88 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx
@@ -23,13 +23,15 @@ import {
import React, { FC } from 'react';
import { Checkbox } from 'src/common/components';
import { FormInstance } from 'antd/lib/form';
-import { getChartControlPanelRegistry } from '@superset-ui/core';
+import { getChartControlPanelRegistry, t } from '@superset-ui/core';
+import { Tooltip } from 'src/components/Tooltip';
import { getControlItems, setNativeFilterFieldValues } from './utils';
import { NativeFiltersForm, NativeFiltersFormItem } from '../types';
import { StyledCheckboxFormItem } from './FiltersConfigForm';
import { Filter } from '../../types';
type ControlItemsProps = {
+ disabled: boolean;
filterId: string;
forceUpdate: Function;
filterToEdit?: Filter;
@@ -38,6 +40,7 @@ type ControlItemsProps = {
};
const ControlItems: FC<ControlItemsProps> = ({
+ disabled,
forceUpdate,
form,
filterId,
@@ -59,38 +62,48 @@ const ControlItems: FC<ControlItemsProps> = ({
controlItem?.config?.renderTrigger,
)
.map(controlItem => (
- <StyledCheckboxFormItem
- key={controlItem.name}
- name={['filters', filterId, 'controlValues', controlItem.name]}
- initialValue={
- filterToEdit?.controlValues?.[controlItem.name] ??
- controlItem?.config?.default
+ <Tooltip
+ placement="left"
+ title={
+ controlItem.config.affectsDataMask &&
+ disabled &&
+ t('Populate "Default value" to enable this control')
}
- valuePropName="checked"
- colon={false}
>
- <Checkbox
- onChange={() => {
- if (!controlItem.config.resetConfig) {
- forceUpdate();
- return;
- }
- setNativeFilterFieldValues(form, filterId, {
- defaultDataMask: null,
- });
- forceUpdate();
- }}
+ <StyledCheckboxFormItem
+ key={controlItem.name}
+ name={['filters', filterId, 'controlValues', controlItem.name]}
+ initialValue={
+ filterToEdit?.controlValues?.[controlItem.name] ??
+ controlItem?.config?.default
+ }
+ valuePropName="checked"
+ colon={false}
>
- {controlItem.config.label}{' '}
- {controlItem.config.description && (
- <InfoTooltipWithTrigger
- placement="top"
- label={controlItem.config.name}
- tooltip={controlItem.config.description}
- />
- )}
- </Checkbox>
- </StyledCheckboxFormItem>
+ <Checkbox
+ disabled={controlItem.config.affectsDataMask && disabled}
+ onChange={() => {
+ if (!controlItem.config.resetConfig) {
+ forceUpdate();
+ return;
+ }
+ setNativeFilterFieldValues(form, filterId, {
+ defaultDataMask: null,
+ });
+ forceUpdate();
+ }}
+ >
+ {controlItem.config.label}{' '}
+ {controlItem.config.description && (
+ <InfoTooltipWithTrigger
+ placement="top"
+ label={controlItem.config.name}
+ tooltip={controlItem.config.description}
+ />
+ )}
+ </Checkbox>
+ </StyledCheckboxFormItem>
+ </Tooltip>
))}
</>
);
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 d3c3298..fe5d048 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -260,6 +260,8 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
label: filter.title,
}));
+ const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset);
+
return (
<>
<Typography.Title level={5}>{t('Settings')}</Typography.Title>
@@ -434,7 +436,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
data-test="default-input"
label={<StyledLabel>{t('Default Value')}</StyledLabel>}
>
- {(!hasDataset || (!isDataDirty && hasFilledDataset)) && (
+ {showDefaultValue ? (
<DefaultValue
setDataMask={dataMask => {
setNativeFilterFieldValues(form, filterId, {
@@ -447,6 +449,10 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
form={form}
formData={newFormData}
/>
+ ) : hasFilledDataset ? (
+ t('Click "Populate" to get "Default Value" ->')
+ ) : (
+ t('Fill all required fields to enable "Default Value"')
)}
</StyledFormItem>
</StyledContainer>
@@ -461,6 +467,7 @@ export const FiltersConfigForm: React.FC<FiltersConfigFormProps> = ({
</Checkbox>
</StyledCheckboxFormItem>
<ControlItems
+ disabled={!showDefaultValue}
filterToEdit={filterToEdit}
formFilter={formFilter}
filterId={filterId}
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
index d9ae39d..968eb17 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
@@ -42,13 +42,20 @@ export const validateForm = async (
errors: [error],
};
form.setFields([fieldError]);
- // eslint-disable-next-line no-throw-literal
- throw { errorFields: [fieldError] };
};
try {
- const formValues = (await form.validateFields()) as NativeFiltersForm;
-
+ let formValues: NativeFiltersForm;
+ try {
+ formValues = (await form.validateFields()) as NativeFiltersForm;
+ } catch (error) {
+ // In Jest tests in chain of tests, Ant generate `outOfDate` error so need to catch it here
+ if (!error?.errorFields?.length && error?.outOfDate) {
+ formValues = error.values;
+ } else {
+ throw error;
+ }
+ }
const validateInstant = (filterId: string) => {
const isInstant = formValues.filters[filterId]
? formValues.filters[filterId].isInstant
diff --git a/superset-frontend/src/dataMask/actions.ts b/superset-frontend/src/dataMask/actions.ts
index 7a9bb3f..76a286c 100644
--- a/superset-frontend/src/dataMask/actions.ts
+++ b/superset-frontend/src/dataMask/actions.ts
@@ -19,6 +19,7 @@
import { DataMask } from '@superset-ui/core';
import { FilterConfiguration } from '../dashboard/components/nativeFilters/types';
import { FeatureFlag, isFeatureEnabled } from '../featureFlags';
+import { Filters } from '../dashboard/reducers/types';
export const UPDATE_DATA_MASK = 'UPDATE_DATA_MASK';
export interface UpdateDataMask {
@@ -33,6 +34,7 @@ export const SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE =
export interface SetDataMaskForFilterConfigComplete {
type: typeof SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE;
filterConfig: FilterConfiguration;
+ filters?: Filters;
}
export const SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL =
@@ -44,10 +46,12 @@ export interface SetDataMaskForFilterConfigFail {
}
export function setDataMaskForFilterConfigComplete(
filterConfig: FilterConfiguration,
+ filters?: Filters,
): SetDataMaskForFilterConfigComplete {
return {
type: SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE,
filterConfig,
+ filters,
};
}
export function updateDataMask(
diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts
index c4ee71f..a0b0e38 100644
--- a/superset-frontend/src/dataMask/reducer.ts
+++ b/superset-frontend/src/dataMask/reducer.ts
@@ -34,6 +34,8 @@ import {
Filter,
FilterConfiguration,
} from '../dashboard/components/nativeFilters/types';
+import { areObjectsEqual } from '../reduxUtils';
+import { Filters } from '../dashboard/reducers/types';
export function getInitialDataMask(id?: string): DataMask;
export function getInitialDataMask(id: string): DataMaskWithId {
@@ -54,21 +56,37 @@ export function getInitialDataMask(id: string): DataMaskWithId {
}
function fillNativeFilters(
- data: FilterConfiguration,
- cleanState: DataMaskStateWithId,
- draft: DataMaskStateWithId,
+ filterConfig: FilterConfiguration,
+ mergedDataMask: DataMaskStateWithId,
+ draftDataMask: DataMaskStateWithId,
+ currentFilters?: Filters,
) {
- data.forEach((filter: Filter) => {
- cleanState[filter.id] = {
+ filterConfig.forEach((filter: Filter) => {
+ mergedDataMask[filter.id] = {
...getInitialDataMask(filter.id), // take initial data
...filter.defaultDataMask, // if something new came from BE - take it
- ...draft[filter.id], // keep local filter data
+ ...draftDataMask[filter.id], // keep local filter data
};
+ // if we came from filters config modal and particular filters changed take it's dataMask
+ if (
+ currentFilters &&
+ !areObjectsEqual(
+ filter.defaultDataMask,
+ currentFilters[filter.id]?.defaultDataMask,
+ { ignoreUndefined: true },
+ )
+ ) {
+ mergedDataMask[filter.id] = {
+ ...mergedDataMask[filter.id],
+ ...filter.defaultDataMask,
+ };
+ }
});
+
// Get back all other non-native filters
- Object.values(draft).forEach(filter => {
+ Object.values(draftDataMask).forEach(filter => {
if (!String(filter?.id).startsWith(NATIVE_FILTER_PREFIX)) {
- cleanState[filter?.id] = filter;
+ mergedDataMask[filter?.id] = filter;
}
});
}
@@ -106,7 +124,12 @@ const dataMaskReducer = produce(
);
return cleanState;
case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE:
- fillNativeFilters(action.filterConfig ?? [], cleanState, draft);
+ fillNativeFilters(
+ action.filterConfig ?? [],
+ cleanState,
+ draft,
+ action.filters,
+ );
return cleanState;
default:
diff --git a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts
index 33922d0..5b9b898 100644
--- a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts
+++ b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts
@@ -37,6 +37,7 @@ const config: ControlPanelConfig = {
type: 'CheckboxControl',
label: t('Multiple select'),
default: multiSelect,
+ affectsDataMask: true,
resetConfig: true,
renderTrigger: true,
description: t('Allow selecting multiple values'),
diff --git a/superset-frontend/src/filters/components/Select/controlPanel.ts b/superset-frontend/src/filters/components/Select/controlPanel.ts
index a866c98..1fad930 100644
--- a/superset-frontend/src/filters/components/Select/controlPanel.ts
+++ b/superset-frontend/src/filters/components/Select/controlPanel.ts
@@ -61,6 +61,7 @@ const config: ControlPanelConfig = {
label: t('Multiple select'),
default: multiSelect,
resetConfig: true,
+ affectsDataMask: true,
renderTrigger: true,
description: t('Allow selecting multiple values'),
},
@@ -89,6 +90,7 @@ const config: ControlPanelConfig = {
label: t('Default to first item'),
default: defaultToFirstItem,
resetConfig: true,
+ affectsDataMask: true,
renderTrigger: true,
description: t('Select first item by default'),
},
@@ -100,6 +102,7 @@ const config: ControlPanelConfig = {
config: {
type: 'CheckboxControl',
renderTrigger: true,
+ affectsDataMask: true,
label: t('Inverse selection'),
default: inverseSelection,
description: t('Exclude selected values'),