fix(explore): Prevent shared controls from checking feature flags outside React render (#21315)

This commit is contained in:
Cody Leff
2022-09-14 14:41:47 -04:00
committed by GitHub
parent 59ca7861c0
commit 2285ebe72e
21 changed files with 285 additions and 435 deletions

View File

@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FeatureFlag } from '@superset-ui/core';
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import {
@@ -27,12 +28,18 @@ const defaultProps: DndColumnSelectProps = {
type: 'DndColumnSelect',
name: 'Filter',
onChange: jest.fn(),
options: {
string: { column_name: 'Column A' },
},
options: [{ column_name: 'Column A' }],
actions: { setControlValue: jest.fn() },
};
beforeAll(() => {
window.featureFlags = { [FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP]: true };
});
afterAll(() => {
window.featureFlags = {};
});
test('renders with default props', async () => {
render(<DndColumnSelect {...defaultProps} />, {
useDnd: true,
@@ -42,7 +49,7 @@ test('renders with default props', async () => {
});
test('renders with value', async () => {
render(<DndColumnSelect {...defaultProps} value="string" />, {
render(<DndColumnSelect {...defaultProps} value="Column A" />, {
useDnd: true,
useRedux: true,
});

View File

@@ -24,7 +24,11 @@ import {
tn,
QueryFormColumn,
} from '@superset-ui/core';
import { ColumnMeta, isColumnMeta } from '@superset-ui/chart-controls';
import {
ColumnMeta,
isColumnMeta,
withDndFallback,
} from '@superset-ui/chart-controls';
import { isEmpty } from 'lodash';
import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
import OptionWrapper from 'src/explore/components/controls/DndColumnSelectControl/OptionWrapper';
@@ -34,13 +38,14 @@ import { DndItemType } from 'src/explore/components/DndItemType';
import { useComponentDidUpdate } from 'src/hooks/useComponentDidUpdate';
import ColumnSelectPopoverTrigger from './ColumnSelectPopoverTrigger';
import { DndControlProps } from './types';
import SelectControl from '../SelectControl';
export type DndColumnSelectProps = DndControlProps<QueryFormColumn> & {
options: Record<string, ColumnMeta>;
options: ColumnMeta[];
isTemporal?: boolean;
};
export function DndColumnSelect(props: DndColumnSelectProps) {
function DndColumnSelect(props: DndColumnSelectProps) {
const {
value,
options,
@@ -48,16 +53,20 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
onChange,
canDelete = true,
ghostButtonText,
clickEnabledGhostButtonText,
name,
label,
isTemporal,
} = props;
const [newColumnPopoverVisible, setNewColumnPopoverVisible] = useState(false);
const optionSelector = useMemo(
() => new OptionSelector(options, multi, value),
[multi, options, value],
);
const optionSelector = useMemo(() => {
const optionsMap = Object.fromEntries(
options.map(option => [option.column_name, option]),
);
return new OptionSelector(optionsMap, multi, value);
}, [multi, options, value]);
// synchronize values in case of dataset changes
const handleOptionsChange = useCallback(() => {
@@ -126,15 +135,18 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
[onChange, optionSelector],
);
const popoverOptions = useMemo(() => Object.values(options), [options]);
const clickEnabled = useMemo(
() => isFeatureEnabled(FeatureFlag.ENABLE_DND_WITH_CLICK_UX),
[],
);
const valuesRenderer = useCallback(
() =>
optionSelector.values.map((column, idx) =>
isFeatureEnabled(FeatureFlag.ENABLE_DND_WITH_CLICK_UX) ? (
clickEnabled ? (
<ColumnSelectPopoverTrigger
key={idx}
columns={popoverOptions}
columns={options}
onColumnEdit={newColumn => {
if (isColumnMeta(newColumn)) {
optionSelector.replace(idx, newColumn.column_name);
@@ -171,13 +183,15 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
),
[
canDelete,
clickEnabled,
isTemporal,
label,
name,
onChange,
onClickClose,
onShiftOptions,
optionSelector,
popoverOptions,
options,
],
);
@@ -205,15 +219,24 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
togglePopover(true);
}, [togglePopover]);
const defaultGhostButtonText = isFeatureEnabled(
FeatureFlag.ENABLE_DND_WITH_CLICK_UX,
)
? tn(
'Drop a column here or click',
'Drop columns here or click',
multi ? 2 : 1,
)
: tn('Drop column here', 'Drop columns here', multi ? 2 : 1);
const labelGhostButtonText = useMemo(() => {
if (clickEnabled) {
return (
clickEnabledGhostButtonText ??
ghostButtonText ??
tn(
'Drop a column here or click',
'Drop columns here or click',
multi ? 2 : 1,
)
);
}
return (
ghostButtonText ??
tn('Drop column here', 'Drop columns here', multi ? 2 : 1)
);
}, [clickEnabled, clickEnabledGhostButtonText, ghostButtonText, multi]);
return (
<div>
@@ -223,16 +246,12 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
valuesRenderer={valuesRenderer}
accept={DndItemType.Column}
displayGhostButton={multi || optionSelector.values.length === 0}
ghostButtonText={ghostButtonText || defaultGhostButtonText}
onClickGhostButton={
isFeatureEnabled(FeatureFlag.ENABLE_DND_WITH_CLICK_UX)
? openPopover
: undefined
}
ghostButtonText={labelGhostButtonText}
onClickGhostButton={clickEnabled ? openPopover : undefined}
{...props}
/>
<ColumnSelectPopoverTrigger
columns={popoverOptions}
columns={options}
onColumnEdit={addNewColumnWithPopover}
isControlledComponent
togglePopover={togglePopover}
@@ -245,3 +264,10 @@ export function DndColumnSelect(props: DndColumnSelectProps) {
</div>
);
}
const DndColumnSelectWithFallback = withDndFallback(
DndColumnSelect,
SelectControl,
);
export { DndColumnSelectWithFallback as DndColumnSelect };

View File

@@ -17,7 +17,7 @@
* under the License.
*/
import React from 'react';
import { GenericDataType } from '@superset-ui/core';
import { FeatureFlag, GenericDataType } from '@superset-ui/core';
import { render, screen } from 'spec/helpers/testing-library';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import AdhocFilter, {
@@ -48,6 +48,14 @@ const baseFormData = {
datasource: 'table__1',
};
beforeAll(() => {
window.featureFlags = { [FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP]: true };
});
afterAll(() => {
window.featureFlags = {};
});
test('renders with default props', async () => {
render(<DndFilterSelect {...defaultProps} />, { useDnd: true });
expect(

View File

@@ -27,7 +27,7 @@ import {
SupersetClient,
t,
} from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { ColumnMeta, withDndFallback } from '@superset-ui/chart-controls';
import {
OPERATOR_ENUM_TO_OPERATOR_TYPE,
Operators,
@@ -49,6 +49,7 @@ import {
} from 'src/explore/components/DatasourcePanel/types';
import { DndItemType } from 'src/explore/components/DndItemType';
import { ControlComponentProps } from 'src/explore/components/Control';
import AdhocFilterControl from '../FilterControl/AdhocFilterControl';
const EMPTY_OBJECT = {};
const DND_ACCEPTED_TYPES = [
@@ -69,7 +70,7 @@ export interface DndFilterSelectProps
datasource: Datasource;
}
export const DndFilterSelect = (props: DndFilterSelectProps) => {
const DndFilterSelect = (props: DndFilterSelectProps) => {
const { datasource, onChange = () => {}, name: controlName } = props;
const propsValues = Array.from(props.value ?? []);
@@ -407,3 +408,10 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
</>
);
};
const DndFilterSelectWithFallback = withDndFallback(
DndFilterSelect,
AdhocFilterControl,
);
export { DndFilterSelectWithFallback as DndFilterSelect };

View File

@@ -18,6 +18,7 @@
*/
import React from 'react';
import userEvent from '@testing-library/user-event';
import { FeatureFlag } from '@superset-ui/core';
import {
render,
screen,
@@ -67,6 +68,14 @@ const adhocMetricB = {
optionName: 'def',
};
beforeAll(() => {
window.featureFlags = { [FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP]: true };
});
afterAll(() => {
window.featureFlags = {};
});
test('renders with default props', () => {
render(<DndMetricSelect {...defaultProps} />, { useDnd: true });
expect(screen.getByText('Drop column or metric here')).toBeInTheDocument();

View File

@@ -27,7 +27,7 @@ import {
QueryFormMetric,
tn,
} from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { ColumnMeta, withDndFallback } from '@superset-ui/chart-controls';
import { isEqual } from 'lodash';
import { usePrevious } from 'src/hooks/usePrevious';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
@@ -41,6 +41,7 @@ import { DndItemType } from 'src/explore/components/DndItemType';
import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
import { savedMetricType } from 'src/explore/components/controls/MetricControl/types';
import { AGGREGATES } from 'src/explore/constants';
import MetricsControl from '../MetricControl/MetricsControl';
const EMPTY_OBJECT = {};
const DND_ACCEPTED_TYPES = [DndItemType.Column, DndItemType.Metric];
@@ -125,7 +126,7 @@ const getMetricsMatchingCurrentDataset = (
}, []);
};
export const DndMetricSelect = (props: any) => {
const DndMetricSelect = (props: any) => {
const { onChange, multi, columns, savedMetrics } = props;
const handleChange = useCallback(
@@ -408,3 +409,10 @@ export const DndMetricSelect = (props: any) => {
</div>
);
};
const DndMetricSelectWithFallback = withDndFallback(
DndMetricSelect,
MetricsControl,
);
export { DndMetricSelectWithFallback as DndMetricSelect };

View File

@@ -46,6 +46,7 @@ export type DndControlProps<ValueType extends JsonValue> =
multi?: boolean;
canDelete?: boolean;
ghostButtonText?: string;
clickEnabledGhostButtonText?: string;
onChange: (value: ValueType | ValueType[] | null | undefined) => void;
};