Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sql lab): Save Dataset Modal Autocomplete should display list when overwritting #20512

Merged
merged 9 commits into from
Jul 1, 2022
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 select or type dataset name/i,
name: /overwrite existing/i,
});
const fieldLabel = screen.getByText(/overwrite existing/i);
const inputField = screen.getByRole('combobox');
Expand Down
121 changes: 59 additions & 62 deletions superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@
* under the License.
*/

import React, { FunctionComponent, useState } from 'react';
import React, { FunctionComponent, useCallback, useState } from 'react';
import { Radio } from 'src/components/Radio';
import { AutoComplete, RadioChangeEvent } from 'src/components';
import { RadioChangeEvent, Select } 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 @@ -42,7 +41,6 @@ import {
DatasetRadioState,
EXPLORE_CHART_DEFAULT,
DatasetOwner,
DatasetOptionAutocomplete,
SqlLabExploreRootState,
getInitialState,
ExploreDatasource,
Expand All @@ -51,6 +49,8 @@ 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,6 +82,10 @@ 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 @@ -129,13 +133,12 @@ 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 [autocompleteValue, setAutocompleteValue] = useState('');
const [selectedDatasetToOverwrite, setSelectedDatasetToOverwrite] = useState<
SelectValue | undefined
>(undefined);

const user = useSelector<SqlLabExploreRootState, User>(user =>
getInitialState(user),
Expand All @@ -146,7 +149,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 @@ -158,9 +161,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 @@ -179,17 +182,15 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
setDatasetName(getDefaultDatasetName());
};

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 loadDatasetOverwriteOptions = useCallback(
async (input = '') => {
const { userId } = user;
const queryParams = rison.encode({
filters: [
{
col: 'table_name',
opr: 'ct',
value: searchText,
value: input,
},
{
col: 'owners',
Expand All @@ -201,22 +202,22 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
order_direction: 'desc',
});

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

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

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 @@ -298,12 +294,11 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
(newOrOverwrite === DatasetRadioState.SAVE_NEW &&
datasetName.length === 0) ||
(newOrOverwrite === DatasetRadioState.OVERWRITE_DATASET &&
Object.keys(datasetToOverwrite).length === 0 &&
autocompleteValue.length === 0);
isEmpty(selectedDatasetToOverwrite));

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 @@ -359,23 +354,25 @@ export const SaveDatasetModal: FunctionComponent<SaveDatasetModalProps> = ({
disabled={newOrOverwrite !== 1}
/>
</Radio>
<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>
<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}
Antonio-RiveroMartnez marked this conversation as resolved.
Show resolved Hide resolved
/>
</div>
</div>
</Radio.Group>
</div>
)}
Expand Down
22 changes: 22 additions & 0 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,28 @@ 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: 5 additions & 1 deletion superset-frontend/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type PickedSelectProps = Pick<
| 'showSearch'
| 'tokenSeparators'
| 'value'
| 'getPopupContainer'
>;

export type OptionsType = Exclude<AntdSelectAllProps['options'], undefined>;
Expand Down Expand Up @@ -316,6 +317,7 @@ const Select = (
sortComparator = DEFAULT_SORT_COMPARATOR,
tokenSeparators,
value,
getPopupContainer,
...props
}: SelectProps,
ref: RefObject<SelectRef>,
Expand Down Expand Up @@ -701,7 +703,9 @@ const Select = (
dropdownRender={dropdownRender}
filterOption={handleFilterOption}
filterSort={sortComparatorWithSearch}
getPopupContainer={triggerNode => triggerNode.parentNode}
getPopupContainer={
getPopupContainer || (triggerNode => triggerNode.parentNode)
}
labelInValue={isAsync || labelInValue}
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,12 @@ test('Click on Save as dataset', () => {
name: /save as new undefined/i,
});
const overwriteRadioBtn = screen.getByRole('radio', {
name: /overwrite existing select or type dataset name/i,
name: /overwrite existing/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();
});
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,9 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
disabled={isDisabled}
getPopupContainer={
showOverflow
? () => parentRef?.current
: (trigger: HTMLElement) => trigger?.parentNode
? () => (parentRef?.current as HTMLElement) || document.body
: (trigger: HTMLElement) =>
(trigger?.parentNode as HTMLElement) || document.body
}
showSearch={showSearch}
mode={multiSelect ? 'multiple' : 'single'}
Expand Down