betodealmeida commented on a change in pull request #12006:
URL:
https://github.com/apache/incubator-superset/pull/12006#discussion_r541278936
##########
File path: superset-frontend/src/datasource/ChangeDatasourceModal.tsx
##########
@@ -47,86 +68,93 @@ const TABLE_COLUMNS = [
'creator',
].map(col => ({ accessor: col, Header: col }));
-const TABLE_FILTERABLE = ['rawName', 'type', 'schema', 'connection',
'creator'];
const CHANGE_WARNING_MSG = t(
'Changing the dataset may break the chart if the chart relies ' +
'on columns or metadata that does not exist in the target dataset',
);
+const useDebouncedEffect = (effect: any, delay: number) => {
Review comment:
Nit: let's move this to some `utils.ts` module, so it can be used by
other components.
##########
File path: superset-frontend/src/datasource/ChangeDatasourceModal.tsx
##########
@@ -47,86 +68,93 @@ const TABLE_COLUMNS = [
'creator',
].map(col => ({ accessor: col, Header: col }));
-const TABLE_FILTERABLE = ['rawName', 'type', 'schema', 'connection',
'creator'];
const CHANGE_WARNING_MSG = t(
'Changing the dataset may break the chart if the chart relies ' +
'on columns or metadata that does not exist in the target dataset',
);
+const useDebouncedEffect = (effect: any, delay: number) => {
+ const callback = useCallback(effect, [effect]);
+
+ useEffect(() => {
+ const handler = setTimeout(() => {
+ callback();
+ }, delay);
+
+ return () => {
+ clearTimeout(handler);
+ };
+ }, [callback, delay]);
+};
+
const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
addDangerToast,
+ addSuccessToast,
onChange,
onDatasourceSave,
onHide,
show,
}) => {
- const [datasources, setDatasources] = useState<any>(null);
const [filter, setFilter] = useState<any>(undefined);
- const [loading, setLoading] = useState(true);
+ const [confirmChange, setConfirmChange] = useState(false);
+ const [confirmedDataset, setConfirmedDataset] = useState<any>(undefined);
let searchRef = useRef<HTMLInputElement>(null);
- useEffect(() => {
- const selectDatasource = (datasource: any) => {
- SupersetClient.get({
- endpoint: `/datasource/get/${datasource.type}/${datasource.id}`,
- })
- .then(({ json }) => {
- onDatasourceSave(json);
- onChange(datasource.uid);
- })
- .catch(response => {
- getClientErrorObject(response).then(
- ({ error, message }: { error: any; message: string }) => {
- const errorMessage = error
- ? error.error || error.statusText || error
- : message;
- addDangerToast(errorMessage);
- },
- );
- });
- onHide();
- };
+ const {
+ state: { loading, resourceCollection },
+ fetchData,
+ } = useListViewResource<Dataset>('dataset', t('dataset'), addDangerToast);
+
+ const selectDatasource = useCallback(
+ (datasource: { type: string; id: number; uid: string }) => {
+ setConfirmChange(true);
+ setConfirmedDataset(datasource);
+ },
+ [],
+ );
- const onEnterModal = () => {
+ useDebouncedEffect(() => {
+ fetchData({
+ pageIndex: 0,
+ pageSize: 20,
+ filters: [
+ {
+ id: 'table_name',
+ operator: 'ct',
+ value: filter,
+ },
+ ],
+ sortBy: [{ id: 'changed_on_delta_humanized' }],
+ });
+ }, 1000);
+
+ useEffect(() => {
+ const onEnterModal = async () => {
if (searchRef && searchRef.current) {
searchRef.current.focus();
}
- if (!datasources) {
- SupersetClient.get({
- endpoint: '/superset/datasources/',
- })
- .then(({ json }) => {
- const data = json.map((ds: any) => ({
- rawName: ds.name,
- connection: ds.connection,
- schema: ds.schema,
- name: (
- <a
- href="#"
- onClick={() => selectDatasource(ds)}
- className="datasource-link"
- >
- {ds.name}
- </a>
- ),
- type: ds.type,
- }));
- setLoading(false);
- setDatasources(data);
- })
- .catch(response => {
- setLoading(false);
- getClientErrorObject(response).then(({ error }: any) => {
- addDangerToast(error.error || error.statusText || error);
- });
- });
- }
+
+ // Fetch initial datasets for tableview
+ await fetchData({
+ pageIndex: 0,
+ pageSize: 20,
+ filters: [],
+ sortBy: [{ id: 'changed_on_delta_humanized' }],
+ });
Review comment:
Nit: you might want to move this to a const, eg:
```javascript
const emptyRequest = {
pageIndex: 0,
pageSize: 20,
filters: [],
sortBy: [{ id: 'change_on_delta_humanized' }],
};
```
Then in your `useDebounceEffect` you can reuse it too:
```javascript
fetchData({
...emptyRequest,
filters: [{ id: 'table_name', 'operator': 'ct', value: 'filter' }],
});
```
##########
File path: superset-frontend/src/datasource/ChangeDatasourceModal.tsx
##########
@@ -20,25 +20,46 @@ import React, {
FunctionComponent,
useState,
useRef,
- useMemo,
useEffect,
+ useCallback,
} from 'react';
import { Alert, FormControl, FormControlProps } from 'react-bootstrap';
-import { SupersetClient, t } from '@superset-ui/core';
+import { SupersetClient, t, styled } from '@superset-ui/core';
import TableView from 'src/components/TableView';
-import Modal from 'src/common/components/Modal';
+import StyledModal from 'src/common/components/Modal';
+import Button from 'src/components/Button';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import Dataset from 'src/types/Dataset';
import { getClientErrorObject } from '../utils/getClientErrorObject';
import Loading from '../components/Loading';
import withToasts from '../messageToasts/enhancers/withToasts';
+const CONFIRM_WARNING_MESSAGE = t(
+ 'Warning! Changing the dataset may break the chart if the metadata does not
exist in the target dataset',
Review comment:
I'm not sure what this message means, can we clarify what metadata is
necessary?
##########
File path: superset-frontend/src/datasource/ChangeDatasourceModal.tsx
##########
@@ -47,86 +68,93 @@ const TABLE_COLUMNS = [
'creator',
].map(col => ({ accessor: col, Header: col }));
-const TABLE_FILTERABLE = ['rawName', 'type', 'schema', 'connection',
'creator'];
const CHANGE_WARNING_MSG = t(
'Changing the dataset may break the chart if the chart relies ' +
'on columns or metadata that does not exist in the target dataset',
);
+const useDebouncedEffect = (effect: any, delay: number) => {
+ const callback = useCallback(effect, [effect]);
+
+ useEffect(() => {
+ const handler = setTimeout(() => {
+ callback();
+ }, delay);
+
+ return () => {
+ clearTimeout(handler);
+ };
+ }, [callback, delay]);
+};
+
const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({
addDangerToast,
+ addSuccessToast,
onChange,
onDatasourceSave,
onHide,
show,
}) => {
- const [datasources, setDatasources] = useState<any>(null);
const [filter, setFilter] = useState<any>(undefined);
- const [loading, setLoading] = useState(true);
+ const [confirmChange, setConfirmChange] = useState(false);
+ const [confirmedDataset, setConfirmedDataset] = useState<any>(undefined);
let searchRef = useRef<HTMLInputElement>(null);
- useEffect(() => {
- const selectDatasource = (datasource: any) => {
- SupersetClient.get({
- endpoint: `/datasource/get/${datasource.type}/${datasource.id}`,
- })
- .then(({ json }) => {
- onDatasourceSave(json);
- onChange(datasource.uid);
- })
- .catch(response => {
- getClientErrorObject(response).then(
- ({ error, message }: { error: any; message: string }) => {
- const errorMessage = error
- ? error.error || error.statusText || error
- : message;
- addDangerToast(errorMessage);
- },
- );
- });
- onHide();
- };
+ const {
+ state: { loading, resourceCollection },
+ fetchData,
+ } = useListViewResource<Dataset>('dataset', t('dataset'), addDangerToast);
+
+ const selectDatasource = useCallback(
+ (datasource: { type: string; id: number; uid: string }) => {
Review comment:
Nit: let's make this a type, so that you don't have to declare it as
`<any>` when creating the state in line 100.
##########
File path: superset-frontend/src/datasource/ChangeDatasourceModal.tsx
##########
@@ -135,54 +163,112 @@ const ChangeDatasourceModal:
FunctionComponent<ChangeDatasourceModalProps> = ({
const changeSearch = (
event: React.FormEvent<FormControl & FormControlProps>,
) => {
- setFilter((event.currentTarget?.value as string) ?? '');
+ const searchValue = (event.currentTarget?.value as string) ?? '';
+ setFilter(searchValue);
};
- const data = useMemo(
- () =>
- filter && datasources
- ? datasources.filter((datasource: any) =>
- TABLE_FILTERABLE.some(field =>
datasource[field]?.includes(filter)),
- )
- : datasources,
- [datasources, filter],
- );
+ const handleChangeConfirm = () => {
+ SupersetClient.get({
+ endpoint:
`/datasource/get/${confirmedDataset.type}/${confirmedDataset.id}`,
+ })
+ .then(({ json }) => {
+ onDatasourceSave(json);
+ onChange(`${confirmedDataset.id}__table`);
+ })
+ .catch(response => {
+ getClientErrorObject(response).then(
+ ({ error, message }: { error: any; message: string }) => {
+ const errorMessage = error
+ ? error.error || error.statusText || error
+ : message;
+ addDangerToast(errorMessage);
+ },
+ );
+ });
+ onHide();
+ addSuccessToast('Successfully changed datasource!');
+ };
+
+ const handlerCancelConfirm = () => {
+ setConfirmChange(false);
+ };
+
+ const renderTableView = () => {
+ const data = resourceCollection.map((ds: any) => ({
+ rawName: ds.table_name,
+ connection: ds.database.database_name,
+ schema: ds.schema,
+ name: (
+ <a
+ href="#"
+ onClick={() => selectDatasource({ type: 'table', ...ds })}
+ className="datasource-link"
+ >
+ {ds.table_name}
+ </a>
+ ),
+ type: ds.kind,
+ }));
+
+ return data;
+ };
return (
- <Modal
+ <StyledModal
show={show}
onHide={onHide}
responsive
title={t('Select a dataset')}
hideFooter
>
<>
- <Alert bsStyle="warning">
- <strong>{t('Warning!')}</strong> {CHANGE_WARNING_MSG}
- </Alert>
- <div>
- <FormControl
- inputRef={ref => {
- setSearchRef(ref);
- }}
- type="text"
- bsSize="sm"
- value={filter}
- placeholder={t('Search / Filter')}
- onChange={changeSearch}
- />
- </div>
- {loading && <Loading />}
- {datasources && (
- <TableView
- columns={TABLE_COLUMNS}
- data={data}
- pageSize={20}
- className="table-condensed"
- />
+ {!confirmChange && (
+ <>
+ <Alert bsStyle="warning">
Review comment:
I like the warning message here, I should use it in the overwrite
confirmation dialog!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
