Skip to content

Commit

Permalink
stash form_data on invisible
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark committed May 8, 2024
1 parent cb6b989 commit c213433
Show file tree
Hide file tree
Showing 9 changed files with 290 additions and 74 deletions.
21 changes: 21 additions & 0 deletions superset-frontend/src/explore/actions/exploreActions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 13 additions & 0 deletions superset-frontend/src/explore/actions/exploreActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -161,6 +173,7 @@ export const exploreActions = {
saveFaveStar,
setControlValue,
setExploreControls,
setStashFormData,
updateChartTitle,
createNewSlice,
sliceUpdated,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,30 @@
* 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 {
DatasourceType,
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', {
Expand Down Expand Up @@ -166,9 +176,16 @@ describe('ControlPanelsContainer', () => {
},
],
});
render(<ControlPanelsContainer {...getDefaultProps()} />, {
useRedux: true,
});
const { getByTestId } = render(
<>
<ControlPanelsContainer {...getDefaultProps()} />
<FormDataMock />
</>,
{
useRedux: true,
initialState: { explore: { form_data: defaultState.form_data } },
},
);

const disabledSection = screen.queryByRole('button', {
name: /advanced analytics/i,
Expand All @@ -180,5 +197,11 @@ describe('ControlPanelsContainer', () => {
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',
);
});
});
157 changes: 90 additions & 67 deletions superset-frontend/src/explore/components/ControlPanelsContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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>
);
};

Expand Down Expand Up @@ -607,69 +614,85 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
);

return (
isVisible && (
<Collapse.Panel
css={theme => css`
margin-bottom: 0;
box-shadow: none;
&:last-child {
padding-bottom: ${theme.gridUnit * 16}px;
border-bottom: 0;
}
.panel-body {
margin-left: ${theme.gridUnit * 4}px;
padding-bottom: 0;
}
span.label {
display: inline-block;
}
${!section.label &&
`
<>
<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;
}
.panel-body {
margin-left: ${theme.gridUnit * 4}px;
padding-bottom: 0;
}
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;
}
if (React.isValidElement(controlItem)) {
// When the item is a React element
return controlItem;
}
if (
controlItem.name &&
controlItem.config &&
controlItem.name !== 'datasource'
) {
return renderControl(controlItem);
}
})
.filter(x => x !== null);
// don't show the row if it is empty
if (renderedControls.length === 0) {
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>
)}
</>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
);
});
Loading

0 comments on commit c213433

Please sign in to comment.