This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 8a57a71bed fix(sql lab): Save Dataset Modal Autocomplete should
display list when overwritting (#20512)
8a57a71bed is described below
commit 8a57a71bed30a781a1d5e5b2ce42ccd08045b3e9
Author: Antonio Rivero Martinez
<[email protected]>
AuthorDate: Fri Jul 1 17:40:13 2022 -0300
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
---
.../SaveDatasetModal/SaveDatasetModal.test.tsx | 2 +-
.../SqlLab/components/SaveDatasetModal/index.tsx | 121 ++++++++++-----------
.../src/components/Select/Select.test.tsx | 22 ++++
superset-frontend/src/components/Select/Select.tsx | 6 +-
.../DatasourceControl/DatasourceControl.test.tsx | 4 +-
.../components/Select/SelectFilterPlugin.tsx | 5 +-
6 files changed, 93 insertions(+), 67 deletions(-)
diff --git
a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
index c35b5eb2b6..d0698e3edf 100644
---
a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
+++
b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
@@ -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');
diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
index 4a87d52c18..4fbfe11adb 100644
--- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
@@ -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>
)}
diff --git a/superset-frontend/src/components/Select/Select.test.tsx
b/superset-frontend/src/components/Select/Select.test.tsx
index 97aec1c081..1515a7c548 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -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
diff --git a/superset-frontend/src/components/Select/Select.tsx
b/superset-frontend/src/components/Select/Select.tsx
index 591fe350ce..9cea5a726d 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -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}
diff --git
a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
index 8a65a1139d..46ecbeae58 100644
---
a/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
+++
b/superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
@@ -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();
});
diff --git
a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
index 9803e4a7e6..936c6d548a 100644
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@@ -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'}
