iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(explore): hide advanced analytics for non temporal xaxis (#28312)

This commit is contained in:
JUST.in DO IT 2024-05-08 13:40:28 -07:00 committed by GitHub
parent c8185694be
commit 07cd1d89d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 492 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx
View File

@ -21,7 +21,7 @@ import { t, RollingType, ComparisonType } from '@superset-ui/core';
import { ControlSubSectionHeader } from '../components/ControlSubSectionHeader';
import { ControlPanelSectionConfig } from '../types';
import { formatSelectOptions } from '../utils';
import { formatSelectOptions, displayTimeRelatedControls } from '../utils';
export const advancedAnalyticsControls: ControlPanelSectionConfig = {
label: t('Advanced analytics'),
@ -31,6 +31,7 @@ export const advancedAnalyticsControls: ControlPanelSectionConfig = {
'that allow for advanced analytical post processing ' +
'of query results',
),
visibility: displayTimeRelatedControls,
controlSetRows: [
[<ControlSubSectionHeader>{t('Rolling window')}</ControlSubSectionHeader>],
[

2
superset-frontend/packages/superset-ui-chart-controls/src/sections/forecastInterval.tsx
View File

@ -22,6 +22,7 @@ import {
t,
} from '@superset-ui/core';
import { ControlPanelSectionConfig } from '../types';
import { displayTimeRelatedControls } from '../utils';
export const FORECAST_DEFAULT_DATA = {
forecastEnabled: false,
@ -35,6 +36,7 @@ export const FORECAST_DEFAULT_DATA = {
export const forecastIntervalControls: ControlPanelSectionConfig = {
label: t('Predictive Analytics'),
expanded: false,
visibility: displayTimeRelatedControls,
controlSetRows: [
[
{

22
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
View File

@ -41,8 +41,6 @@ import {
SequentialScheme,
legacyValidateInteger,
ComparisonType,
isAdhocColumn,
isPhysicalColumn,
ensureIsArray,
isDefined,
NO_TIME_RANGE,
@ -51,6 +49,7 @@ import {
import {
formatSelectOptions,
displayTimeRelatedControls,
D3_FORMAT_OPTIONS,
D3_FORMAT_DOCS,
D3_TIME_FORMAT_OPTIONS,
@ -62,7 +61,6 @@ import { DEFAULT_MAX_ROW, TIME_FILTER_LABELS } from '../constants';
import {
SharedControlConfig,
Dataset,
ColumnMeta,
ControlState,
ControlPanelState,
} from '../types';
@ -203,23 +201,7 @@ const time_grain_sqla: SharedControlConfig<'SelectControl'> = {
mapStateToProps: ({ datasource }) => ({
choices: (datasource as Dataset)?.time_grain_sqla || [],
}),
visibility: ({ controls }) => {
if (!controls?.x_axis) {
return true;
}
const xAxis = controls?.x_axis;
const xAxisValue = xAxis?.value;
if (isAdhocColumn(xAxisValue)) {
return true;
}
if (isPhysicalColumn(xAxisValue)) {
return !!(xAxis?.options ?? []).find(
(col: ColumnMeta) => col?.column_name === xAxisValue,
)?.is_dttm;
}
return false;
},
visibility: displayTimeRelatedControls,
};
const time_range: SharedControlConfig<'DateFilterControl'> = {

4
superset-frontend/packages/superset-ui-chart-controls/src/types.ts
View File

@ -376,6 +376,10 @@ export interface ControlPanelSectionConfig {
expanded?: boolean;
tabOverride?: TabOverride;
controlSetRows: ControlSetRow[];
visibility?: (
props: ControlPanelsContainerProps,
controlData: AnyDict,
) => boolean;
}
export interface StandardizedControls {

40
superset-frontend/packages/superset-ui-chart-controls/src/utils/displayTimeRelatedControls.ts Normal file
View File

@ -0,0 +1,40 @@
/**
* 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.
*/
import { isAdhocColumn, isPhysicalColumn } from '@superset-ui/core';
import type { ColumnMeta, ControlPanelsContainerProps } from '../types';
export default function displayTimeRelatedControls({
controls,
}: ControlPanelsContainerProps) {
if (!controls?.x_axis) {
return true;
}
const xAxis = controls?.x_axis;
const xAxisValue = xAxis?.value;
if (isAdhocColumn(xAxisValue)) {
return true;
}
if (isPhysicalColumn(xAxisValue)) {
return !!(xAxis?.options ?? []).find(
(col: ColumnMeta) => col?.column_name === xAxisValue,
)?.is_dttm;
}
return false;
}

1
superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts
View File

@ -26,3 +26,4 @@ export { default as columnChoices } from './columnChoices';
export * from './defineSavedMetrics';
export * from './getStandardizedControls';
export * from './getTemporalColumns';
export { default as displayTimeRelatedControls } from './displayTimeRelatedControls';

118
superset-frontend/packages/superset-ui-chart-controls/test/utils/displayTimeRelatedControls.test.ts Normal file
View File

@ -0,0 +1,118 @@
/**
* 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.
*/
import { displayTimeRelatedControls } from '../../src';
const mockData = {
actions: {
setDatasource: jest.fn(),
},
controls: {
x_axis: {
type: 'SelectControl' as const,
value: 'not_temporal',
options: [
{ column_name: 'not_temporal', is_dttm: false },
{ column_name: 'ds', is_dttm: true },
],
},
},
exportState: {},
form_data: {
datasource: '22__table',
viz_type: 'table',
},
};
test('returns true when no x-axis exists', () => {
expect(
displayTimeRelatedControls({
...mockData,
controls: {
control_options: {
type: 'SelectControl',
value: 'not_temporal',
options: [],
},
},
}),
).toBeTruthy();
});
test('returns false when x-axis value is not temporal', () => {
expect(displayTimeRelatedControls(mockData)).toBeFalsy();
});
test('returns true when x-axis value is temporal', () => {
expect(
displayTimeRelatedControls({
...mockData,
controls: {
x_axis: {
...mockData.controls.x_axis,
value: 'ds',
},
},
}),
).toBeTruthy();
});
test('returns false when x-axis value without options', () => {
expect(
displayTimeRelatedControls({
...mockData,
controls: {
x_axis: {
type: 'SelectControl' as const,
value: 'not_temporal',
},
},
}),
).toBeFalsy();
});
test('returns true when x-axis is ad-hoc column', () => {
expect(
displayTimeRelatedControls({
...mockData,
controls: {
x_axis: {
...mockData.controls.x_axis,
value: {
sqlExpression: 'ds',
label: 'ds',
expressionType: 'SQL',
},
},
},
}),
).toBeTruthy();
});
test('returns false when the x-axis is neither an ad-hoc column nor a physical column', () => {
expect(
displayTimeRelatedControls({
...mockData,
controls: {
x_axis: {
...mockData.controls.x_axis,
value: {},
},
},
}),
).toBeFalsy();
});

21
superset-frontend/src/explore/actions/exploreActions.test.js
View File

@ -217,4 +217,25 @@ describe('reducers', () => {
expectedColumnConfig,
);
});
test('setStashFormData works as expected with fieldNames', () => {
const newState = exploreReducer(
defaultState,
actions.setStashFormData(true, ['y_axis_format']),
);
expect(newState.hiddenFormData).toEqual({
y_axis_format: defaultState.form_data.y_axis_format,
});
expect(newState.form_data.y_axis_format).toBeFalsy();
const updatedState = exploreReducer(
newState,
actions.setStashFormData(false, ['y_axis_format']),
);
expect(updatedState.hiddenFormData).toEqual({
y_axis_format: defaultState.form_data.y_axis_format,
});
expect(updatedState.form_data.y_axis_format).toEqual(
defaultState.form_data.y_axis_format,
);
});
});

13
superset-frontend/src/explore/actions/exploreActions.ts
View File

@ -152,6 +152,18 @@ export function setForceQuery(force: boolean) {
};
}
export const SET_STASH_FORM_DATA = 'SET_STASH_FORM_DATA';
export function setStashFormData(
isHidden: boolean,
fieldNames: ReadonlyArray<string>,
) {
return {
type: SET_STASH_FORM_DATA,
isHidden,
fieldNames,
};
}
export const exploreActions = {
...toastActions,
fetchDatasourcesStarted,
@ -161,6 +173,7 @@ export const exploreActions = {
saveFaveStar,
setControlValue,
setExploreControls,
setStashFormData,
updateChartTitle,
createNewSlice,
sliceUpdated,

62
superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx
View File

@ -17,6 +17,7 @@
* under the License.
*/
import React from 'react';
import { useSelector } from 'react-redux';
import userEvent from '@testing-library/user-event';
import { render, screen } from 'spec/helpers/testing-library';
import {
@ -24,13 +25,22 @@ import {
getChartControlPanelRegistry,
t,
} from '@superset-ui/core';
import { defaultControls } from 'src/explore/store';
import { defaultControls, defaultState } from 'src/explore/store';
import { ExplorePageState } from 'src/explore/types';
import { getFormDataFromControls } from 'src/explore/controlUtils';
import {
ControlPanelsContainer,
ControlPanelsContainerProps,
} from 'src/explore/components/ControlPanelsContainer';
const FormDataMock = () => {
const formData = useSelector(
(state: ExplorePageState) => state.explore.form_data,
);
return <div data-test="mock-formdata">{Object.keys(formData).join(':')}</div>;
};
describe('ControlPanelsContainer', () => {
beforeAll(() => {
getChartControlPanelRegistry().registerValue('table', {
@ -144,4 +154,54 @@ describe('ControlPanelsContainer', () => {
await screen.findAllByTestId('collapsible-control-panel-header'),
).toHaveLength(2);
});
test('visibility of panels is correctly applied', async () => {
getChartControlPanelRegistry().registerValue('table', {
controlPanelSections: [
{
label: t('Advanced analytics'),
description: t('Advanced analytics post processing'),
expanded: true,
controlSetRows: [['groupby'], ['metrics'], ['percent_metrics']],
visibility: () => false,
},
{
label: t('Chart Title'),
visibility: () => true,
controlSetRows: [['timeseries_limit_metric', 'row_limit']],
},
{
label: t('Chart Options'),
controlSetRows: [['include_time', 'order_desc']],
},
],
});
const { getByTestId } = render(
<>
<ControlPanelsContainer {...getDefaultProps()} />
<FormDataMock />
</>,
{
useRedux: true,
initialState: { explore: { form_data: defaultState.form_data } },
},
);
const disabledSection = screen.queryByRole('button', {
name: /advanced analytics/i,
});
expect(disabledSection).not.toBeInTheDocument();
expect(
screen.getByRole('button', { name: /chart title/i }),
).toBeInTheDocument();
expect(
screen.queryByRole('button', { name: /chart options/i }),
).toBeInTheDocument();
expect(getByTestId('mock-formdata')).not.toHaveTextContent('groupby');
expect(getByTestId('mock-formdata')).not.toHaveTextContent('metrics');
expect(getByTestId('mock-formdata')).not.toHaveTextContent(
'percent_metrics',
);
});
});

153
superset-frontend/src/explore/components/ControlPanelsContainer.tsx
View File

@ -71,6 +71,7 @@ import { ExploreAlert } from './ExploreAlert';
import { RunQueryButton } from './RunQueryButton';
import { Operators } from '../constants';
import { Clauses } from './controls/FilterControl/types';
import StashFormDataContainer from './StashFormDataContainer';
const { confirm } = Modal;
@ -521,16 +522,22 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
}
return (
<Control
key={`control-${name}`}
name={name}
label={label}
description={description}
validationErrors={validationErrors}
actions={props.actions}
isVisible={isVisible}
{...restProps}
/>
<StashFormDataContainer
shouldStash={isVisible === false}
fieldNames={[name]}
key={`control-container-${name}`}
>
<Control
key={`control-${name}`}
name={name}
label={label}
description={description}
validationErrors={validationErrors}
actions={props.actions}
isVisible={isVisible}
{...restProps}
/>
</StashFormDataContainer>
);
};
@ -543,13 +550,13 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
section: ExpandedControlPanelSectionConfig,
) => {
const { controls } = props;
const { label, description } = section;
const { label, description, visibility } = section;
// Section label can be a ReactNode but in some places we want to
// have a string ID. Using forced type conversion for now,
// should probably add a `id` field to sections in the future.
const sectionId = String(label);
const isVisible = visibility?.call(this, props, controls) !== false;
const hasErrors = section.controlSetRows.some(rows =>
rows.some(item => {
const controlName =
@ -607,67 +614,85 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
);
return (
<Collapse.Panel
css={theme => css`
margin-bottom: 0;
box-shadow: none;
<>
<StashFormDataContainer
key={`sectionId-${sectionId}`}
shouldStash={!isVisible}
fieldNames={section.controlSetRows
.flat()
.map(item =>
item && typeof item === 'object'
? 'name' in item
? item.name
: ''
: String(item || ''),
)
.filter(Boolean)}
/>
{isVisible && (
<Collapse.Panel
css={theme => css`
margin-bottom: 0;
box-shadow: none;
&:last-child {
padding-bottom: ${theme.gridUnit * 16}px;
border-bottom: 0;
}
&:last-child {
padding-bottom: ${theme.gridUnit * 16}px;
border-bottom: 0;
}
.panel-body {
margin-left: ${theme.gridUnit * 4}px;
padding-bottom: 0;
}
.panel-body {
margin-left: ${theme.gridUnit * 4}px;
padding-bottom: 0;
}
span.label {
display: inline-block;
}
${!section.label &&
`
span.label {
display: inline-block;
}
${!section.label &&
`
.ant-collapse-header {
display: none;
}
`}
`}
header={<PanelHeader />}
key={sectionId}
>
{section.controlSetRows.map((controlSets, i) => {
const renderedControls = controlSets
.map(controlItem => {
if (!controlItem) {
// When the item is invalid
`}
header={<PanelHeader />}
key={sectionId}
>
{section.controlSetRows.map((controlSets, i) => {
const renderedControls = controlSets
.map(controlItem => {
if (!controlItem) {
// When the item is invalid
return null;
}
if (React.isValidElement(controlItem)) {
// When the item is a React element
return controlItem;
}
if (
controlItem.name &&
controlItem.config &&
controlItem.name !== 'datasource'
) {
return renderControl(controlItem);
}
return null;
})
.filter(x => x !== null);
// don't show the row if it is empty
if (renderedControls.length === 0) {
return null;
}
if (React.isValidElement(controlItem)) {
// When the item is a React element
return controlItem;
}
if (
controlItem.name &&
controlItem.config &&
controlItem.name !== 'datasource'
) {
return renderControl(controlItem);
}
return null;
})
.filter(x => x !== null);
// don't show the row if it is empty
if (renderedControls.length === 0) {
return null;
}
return (
<ControlRow
key={`controlsetrow-${i}`}
controls={renderedControls}
/>
);
})}
</Collapse.Panel>
return (
<ControlRow
key={`controlsetrow-${i}`}
controls={renderedControls}
/>
);
})}
</Collapse.Panel>
)}
</>
);
};

9
superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
View File

@ -31,7 +31,7 @@ import {
useComponentDidMount,
usePrevious,
} from '@superset-ui/core';
import { debounce, pick } from 'lodash';
import { debounce, omit, pick } from 'lodash';
import { Resizable } from 're-resizable';
import { usePluginContext } from 'src/components/DynamicPlugins';
import { Global } from '@emotion/react';
@ -715,8 +715,11 @@ function mapStateToProps(state) {
user,
saveModal,
} = state;
const { controls, slice, datasource, metadata } = explore;
const form_data = getFormDataFromControls(controls);
const { controls, slice, datasource, metadata, hiddenFormData } = explore;
const form_data = omit(
getFormDataFromControls(controls),
Object.keys(hiddenFormData ?? {}),
);
const slice_id = form_data.slice_id ?? slice?.slice_id ?? 0; // 0 - unsaved chart
form_data.extra_form_data = mergeExtraFormData(
{ ...form_data.extra_form_data },

57
superset-frontend/src/explore/components/StashFormDataContainer/StashFormDataContainer.test.tsx Normal file
View File

@ -0,0 +1,57 @@
/**
* 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.
*/
import React from 'react';
import { defaultState } from 'src/explore/store';
import { render } from 'spec/helpers/testing-library';
import { useSelector } from 'react-redux';
import { ExplorePageState } from 'src/explore/types';
import StashFormDataContainer from '.';
const FormDataMock = () => {
const formData = useSelector(
(state: ExplorePageState) => state.explore.form_data,
);
return <div>{Object.keys(formData).join(':')}</div>;
};
test('should stash form data from fieldNames', () => {
const { rerender, container } = render(
<StashFormDataContainer
shouldStash={false}
fieldNames={['granularity_sqla']}
>
<FormDataMock />
</StashFormDataContainer>,
{
useRedux: true,
initialState: { explore: { form_data: defaultState.form_data } },
},
);
expect(container.querySelector('div')).toHaveTextContent('granularity_sqla');
rerender(
<StashFormDataContainer shouldStash fieldNames={['granularity_sqla']}>
<FormDataMock />
</StashFormDataContainer>,
);
expect(container.querySelector('div')).not.toHaveTextContent(
'granularity_sqla',
);
});

50
superset-frontend/src/explore/components/StashFormDataContainer/index.tsx Normal file
View File

@ -0,0 +1,50 @@
/**
* 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.
*/
import React, { useEffect, useRef } from 'react';
import { useDispatch } from 'react-redux';
import { setStashFormData } from 'src/explore/actions/exploreActions';
import useEffectEvent from 'src/hooks/useEffectEvent';
type Props = {
shouldStash: boolean;
fieldNames: ReadonlyArray<string>;
};
const StashFormDataContainer: React.FC<Props> = ({
shouldStash,
fieldNames,
children,
}) => {
const dispatch = useDispatch();
const isMounted = useRef(false);
const onVisibleUpdate = useEffectEvent((shouldStash: boolean) =>
dispatch(setStashFormData(shouldStash, fieldNames)),
);
useEffect(() => {
if (!isMounted.current && !shouldStash) {
isMounted.current = true;
} else {
onVisibleUpdate(shouldStash);
}
}, [shouldStash, onVisibleUpdate]);
return <>{children}</>;
};
export default StashFormDataContainer;

25
superset-frontend/src/explore/reducers/exploreReducer.js
View File

@ -18,6 +18,7 @@
*/
/* eslint camelcase: 0 */
import { ensureIsArray } from '@superset-ui/core';
import { omit, pick } from 'lodash';
import { DYNAMIC_PLUGIN_CONTROLS_READY } from 'src/components/Chart/chartAction';
import { getControlsState } from 'src/explore/store';
import {
@ -245,6 +246,30 @@ export default function exploreReducer(state = {}, action) {
can_overwrite: action.can_overwrite,
};
},
[actions.SET_STASH_FORM_DATA]() {
const { form_data, hiddenFormData } = state;
const { fieldNames, isHidden } = action;
if (isHidden) {
return {
...state,
hiddenFormData: {
...hiddenFormData,
...pick(form_data, fieldNames),
},
form_data: omit(form_data, fieldNames),
};
}
const restoredField = pick(hiddenFormData, fieldNames);
return {
...state,
form_data: {
...form_data,
...restoredField,
},
hiddenFormData,
};
},
[actions.SLICE_UPDATED]() {
return {
...state,

1
superset-frontend/src/explore/types.ts
View File

@ -109,6 +109,7 @@ export interface ExplorePageState {
datasource: Dataset;
controls: ControlStateMapping;
form_data: QueryFormData;
hiddenFormData?: Partial<QueryFormData>;
slice: Slice;
controlsTransferred: string[];
standalone: boolean;