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

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

* Save Dataset Modal:

- Use our Select component as substitute of the Autocomplete one so options are loaded initially without the user having to trigger a search and we are mosre consistent with the rest of the app
- Changing datasetId to lowercase so when custom props get into the DOM we don't get warning related to invalid formatting
- We extracted the dropdown out of the radio because it causes invalid click handling when an option is selected
- Updated tests

* Save Dataset Modal:

- Update missing test for DatasourceControl

* Save Dataset Modal:

- Remove conditional from load options function since only guest users dont have userId, and if that is the case they wont reach this part of the application

* Save Dataset Modal:

- Remove unused comment

* Save Dataset Modal:

- Add getPopupContainer as prop for Select component

* Save Dataset Modal:

- Add tests for our new getPopupContainer prop in Select component. So if passed it gets called.

* Save Dataset Modal:

- use lowercased property when calling post form data

* Save Dataset Modal:

- Update tests so there is no need to define a null returning func

* Save Dataset Modal:

- Including getPopupContainer from PickedSelectProps instead
- Updating definition in SelectFilterPlugin
This commit is contained in:
Antonio Rivero Martinez 2022-07-01 17:40:13 -03:00 committed by GitHub
parent 2389871556
commit 8a57a71bed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 93 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
View File

@ -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');

121
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
View File

@ -17,9 +17,9 @@
* 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';
@ -27,7 +27,6 @@ import {
styled,
t,
SupersetClient,
makeApi,
JsonResponse,
JsonObject,
QueryResponse,
@ -42,7 +41,6 @@ import {
DatasetRadioState,
EXPLORE_CHART_DEFAULT,
DatasetOwner,
DatasetOptionAutocomplete,
SqlLabExploreRootState,
getInitialState,
ExploreDatasource,
@ -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;
@ -70,8 +70,8 @@ const Styles = styled.div`
width: 401px;
}
.sdm-autocomplete {
margin-left: 8px;
width: 401px;
align-self: center;
}
.sdm-radio {
display: block;
@ -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 (
@ -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),
@ -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 }) => ({
@ -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,
@ -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',
@ -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
@ -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);
@ -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 (
@ -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}
/>
</div>
</div>
</Radio.Group>
</div>
)}

22
superset-frontend/src/components/Select/Select.test.tsx
View File

@ -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

6
superset-frontend/src/components/Select/Select.tsx
View File

@ -67,6 +67,7 @@ type PickedSelectProps = Pick<
| 'showSearch'
| 'tokenSeparators'
| 'value'
| 'getPopupContainer'
>;
export type OptionsType = Exclude<AntdSelectAllProps['options'], undefined>;
@ -316,6 +317,7 @@ const Select = (
sortComparator = DEFAULT_SORT_COMPARATOR,
tokenSeparators,
value,
getPopupContainer,
...props
}: SelectProps,
ref: RefObject<SelectRef>,
@ -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}

4
superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
View File

@ -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();
});

5
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
View File

@ -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'}