Skip to content

Commit

Permalink
Revert "fix(sql lab): Save Dataset Modal Autocomplete should display …
Browse files Browse the repository at this point in the history 
…list when overwritting (apache#20512)"

This reverts commit 8a57a71.
  • Loading branch information
@jinghua-qa
jinghua-qa committed Jul 8, 2022
1 parent 4c00cb9 commit 8b0d28d
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 93 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('SaveDatasetModal RTL', () => {
render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });

const overwriteRadioBtn = screen.getByRole('radio', {
name: /overwrite existing/i,
name: /overwrite existing select or type dataset name/i,
});
const fieldLabel = screen.getByText(/overwrite existing/i);
const inputField = screen.getByRole('combobox');
Expand Down
121 changes: 62 additions & 59 deletions superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@
* under the License.
*/

import React, { FunctionComponent, useCallback, useState } from 'react';
import React, { FunctionComponent, useState } from 'react';
import { Radio } from 'src/components/Radio';
import { RadioChangeEvent, Select } from 'src/components';
import { AutoComplete, RadioChangeEvent } from 'src/components';
import { Input } from 'src/components/Input';
import StyledModal from 'src/components/Modal';
import Button from 'src/components/Button';
import {
styled,
t,
SupersetClient,
makeApi,
JsonResponse,
JsonObject,
QueryResponse,
Expand All @@ -41,6 +42,7 @@ import {
DatasetRadioState,
EXPLORE_CHART_DEFAULT,
DatasetOwner,
DatasetOptionAutocomplete,
SqlLabExploreRootState,
getInitialState,
ExploreDatasource,
Expand All @@ -49,8 +51,6 @@ import {
import { mountExploreUrl } from 'src/explore/exploreUtils';
import { postFormData } from 'src/explore/exploreUtils/formData';
import { URL_PARAMS } from 'src/constants';
import { SelectValue } from 'antd/lib/select';
import { isEmpty } from 'lodash';

interface SaveDatasetModalProps {
visible: boolean;
Expand All @@ -70,8 +70,8 @@ const Styles = styled.div`
width: 401px;
}
.sdm-autocomplete {
margin-left: 8px;
width: 401px;
align-self: center;
}
.sdm-radio {
display: block;
Expand All @@ -82,10 +82,6 @@ const Styles = styled.div`
.sdm-overwrite-msg {
margin: 7px;
}
.sdm-overwrite-container {
flex: 1 1 auto;
display: flex;
}
`;

const updateDataset = async (
Expand Down Expand Up @@ -133,12 +129,13 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
DatasetRadioState.SAVE_NEW,
);
const [shouldOverwriteDataset, setShouldOverwriteDataset] = useState(false);
const [userDatasetOptions, setUserDatasetOptions] = useState<
DatasetOptionAutocomplete[]
>([]);
const [datasetToOverwrite, setDatasetToOverwrite] = useState<
Record<string, any>
>({});
const [selectedDatasetToOverwrite, setSelectedDatasetToOverwrite] = useState<
SelectValue | undefined
>(undefined);
const [autocompleteValue, setAutocompleteValue] = useState('');

const user = useSelector<SqlLabExploreRootState, User>(user =>
getInitialState(user),
Expand All @@ -149,7 +146,7 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
const [, key] = await Promise.all([
updateDataset(
query.dbId,
datasetToOverwrite.datasetid,
datasetToOverwrite.datasetId,
query.sql,
query.results.selected_columns.map(
(d: { name: string; type: string; is_dttm: boolean }) => ({
Expand All @@ -161,9 +158,9 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
datasetToOverwrite.owners.map((o: DatasetOwner) => o.id),
true,
),
postFormData(datasetToOverwrite.datasetid, 'table', {
postFormData(datasetToOverwrite.datasetId, 'table', {
...EXPLORE_CHART_DEFAULT,
datasource: `${datasetToOverwrite.datasetid}__table`,
datasource: `${datasetToOverwrite.datasetId}__table`,
...(defaultVizType === 'table' && {
all_columns: query.results.selected_columns.map(
column => column.name,
Expand All @@ -182,15 +179,17 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
setDatasetName(getDefaultDatasetName());
};

const loadDatasetOverwriteOptions = useCallback(
async (input = '') => {
const { userId } = user;
const getUserDatasets = async (searchText = '') => {
// Making sure that autocomplete input has a value before rendering the dropdown
// Transforming the userDatasetsOwned data for SaveModalComponent)
const { userId } = user;
if (userId) {
const queryParams = rison.encode({
filters: [
{
col: 'table_name',
opr: 'ct',
value: input,
value: searchText,
},
{
col: 'owners',
Expand All @@ -202,22 +201,22 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
order_direction: 'desc',
});

return SupersetClient.get({
endpoint: `/api/v1/dataset?q=${queryParams}`,
}).then(response => ({
data: response.json.result.map(
(r: { table_name: string; id: number; owners: [DatasetOwner] }) => ({
value: r.table_name,
label: r.table_name,
datasetid: r.id,
owners: r.owners,
}),
),
totalCount: response.json.count,
}));
},
[user],
);
const response = await makeApi({
method: 'GET',
endpoint: '/api/v1/dataset',
})(`q=${queryParams}`);

return response.result.map(
(r: { table_name: string; id: number; owners: [DatasetOwner] }) => ({
value: r.table_name,
datasetId: r.id,
owners: r.owners,
}),
);
}

return null;
};

const handleSaveInDataset = () => {
// if user wants to overwrite a dataset we need to prompt them
Expand Down Expand Up @@ -275,11 +274,16 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
onHide();
};

const handleOverwriteDatasetOption = (value: SelectValue, option: any) => {
setDatasetToOverwrite(option);
setSelectedDatasetToOverwrite(value);
const handleSaveDatasetModalSearch = async (searchText: string) => {
const userDatasetsOwned = await getUserDatasets(searchText);
setUserDatasetOptions(userDatasetsOwned);
};

const handleOverwriteDatasetOption = (
_data: string,
option: Record<string, any>,
) => setDatasetToOverwrite(option);

const handleDatasetNameChange = (e: React.FormEvent<HTMLInputElement>) => {
// @ts-expect-error
setDatasetName(e.target.value);
Expand All @@ -294,11 +298,12 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
(newOrOverwrite === DatasetRadioState.SAVE_NEW &&
datasetName.length === 0) ||
(newOrOverwrite === DatasetRadioState.OVERWRITE_DATASET &&
isEmpty(selectedDatasetToOverwrite));
Object.keys(datasetToOverwrite).length === 0 &&
autocompleteValue.length === 0);

const filterAutocompleteOption = (
inputValue: string,
option: { value: string; datasetid: number },
option: { value: string; datasetId: number },
) => option.value.toLowerCase().includes(inputValue.toLowerCase());

return (
Expand Down Expand Up @@ -354,25 +359,23 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
disabled={newOrOverwrite !== 1}
/>
</Radio>
<div className="sdm-overwrite-container">
<Radio className="sdm-radio" value={2}>
{t('Overwrite existing')}
</Radio>
<div className="sdm-autocomplete">
<Select
allowClear
showSearch
placeholder={t('Select or type dataset name')}
ariaLabel={t('Existing dataset')}
onChange={handleOverwriteDatasetOption}
options={input => loadDatasetOverwriteOptions(input)}
value={selectedDatasetToOverwrite}
filterOption={filterAutocompleteOption}
disabled={newOrOverwrite !== 2}
getPopupContainer={() => document.body}
/>
</div>
</div>
<Radio className="sdm-radio" value={2}>
{t('Overwrite existing')}
<AutoComplete
className="sdm-autocomplete"
options={userDatasetOptions}
onSelect={handleOverwriteDatasetOption}
onSearch={handleSaveDatasetModalSearch}
onChange={value => {
setDatasetToOverwrite({});
setAutocompleteValue(value);
}}
placeholder={t('Select or type dataset name')}
filterOption={filterAutocompleteOption}
disabled={newOrOverwrite !== 2}
value={autocompleteValue}
/>
</Radio>
</Radio.Group>
</div>
)}
Expand Down
22 changes: 0 additions & 22 deletions superset-frontend/src/components/Select/Select.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -829,28 +829,6 @@ test('async - requests the options again after clearing the cache', async () =>
expect(mock).toHaveBeenCalledTimes(2);
});

test('async - triggers getPopupContainer if passed', async () => {
const getPopupContainer = jest.fn();
render(
<div>
<Select
{...defaultProps}
options={loadOptions}
getPopupContainer={getPopupContainer}
/>
</div>,
);
await open();
expect(getPopupContainer).toHaveBeenCalled();
});

test('static - triggers getPopupContainer if passed', async () => {
const getPopupContainer = jest.fn();
render(<Select {...defaultProps} getPopupContainer={getPopupContainer} />);
await open();
expect(getPopupContainer).toHaveBeenCalled();
});

/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
Expand Down
6 changes: 1 addition & 5 deletions superset-frontend/src/components/Select/Select.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ type PickedSelectProps = Pick<
| 'showSearch'
| 'tokenSeparators'
| 'value'
| 'getPopupContainer'
>;

export type OptionsType = Exclude<AntdSelectAllProps['options'], undefined>;
Expand Down Expand Up @@ -317,7 +316,6 @@ const Select = (
sortComparator = DEFAULT_SORT_COMPARATOR,
tokenSeparators,
value,
getPopupContainer,
...props
}: SelectProps,
ref: RefObject<SelectRef>,
Expand Down Expand Up @@ -703,9 +701,7 @@ const Select = (
dropdownRender={dropdownRender}
filterOption={handleFilterOption}
filterSort={sortComparatorWithSearch}
getPopupContainer={
getPopupContainer || (triggerNode => triggerNode.parentNode)
}
getPopupContainer={triggerNode => triggerNode.parentNode}
labelInValue={isAsync || labelInValue}
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
Expand Down
4 changes: 1 addition & 3 deletions ...set-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,10 @@ test('Click on Save as dataset', () => {
name: /save as new undefined/i,
});
const overwriteRadioBtn = screen.getByRole('radio', {
name: /overwrite existing/i,
name: /overwrite existing select or type dataset name/i,
});
const dropdownField = screen.getByText(/select or type dataset name/i);
expect(saveRadioBtn).toBeVisible();
expect(overwriteRadioBtn).toBeVisible();
expect(screen.getByRole('button', { name: /save/i })).toBeVisible();
expect(screen.getByRole('button', { name: /close/i })).toBeVisible();
expect(dropdownField).toBeVisible();
});
5 changes: 2 additions & 3 deletions superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,8 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
disabled={isDisabled}
getPopupContainer={
showOverflow
? () => (parentRef?.current as HTMLElement) || document.body
: (trigger: HTMLElement) =>
(trigger?.parentNode as HTMLElement) || document.body
? () => parentRef?.current
: (trigger: HTMLElement) => trigger?.parentNode
}
showSearch={showSearch}
mode={multiSelect ? 'multiple' : 'single'}
Expand Down

0 comments on commit 8b0d28d

Please sign in to comment.