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

chore: Remove actions prop and refactor code in SQL Lab #22231

Merged
merged 6 commits into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ export type Query = {
errorMessage: string | null;
extra: {
progress: string | null;
errors?: SupersetError[];
};
id: string;
isDataPreview: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,8 @@ import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import QueryHistory from 'src/SqlLab/components/QueryHistory';

const NOOP = () => {};
const mockedProps = {
queries: [],
actions: {
queryEditorSetAndSaveSql: NOOP,
cloneQueryToNewTab: NOOP,
fetchQueryResults: NOOP,
clearQueryResults: NOOP,
removeQuery: NOOP,
},
displayLimit: 1000,
latestQueryId: 'yhMUZCGb',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ import QueryTable from 'src/SqlLab/components/QueryTable';

interface QueryHistoryProps {
queries: QueryResponse[];
actions: {
queryEditorSetAndSaveSql: Function;
cloneQueryToNewTab: Function;
fetchQueryResults: Function;
clearQueryResults: Function;
removeQuery: Function;
};
displayLimit: number;
latestQueryId: string | undefined;
}
Expand All @@ -47,7 +40,6 @@ const StyledEmptyStateWrapper = styled.div`

const QueryHistory = ({
queries,
actions,
displayLimit,
latestQueryId,
}: QueryHistoryProps) =>
Expand All @@ -64,7 +56,6 @@ const QueryHistory = ({
'actions',
]}
queries={queries}
actions={actions}
displayLimit={displayLimit}
latestQueryId={latestQueryId}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ fetchMock.get(DATABASE_ENDPOINT, []);

describe('QuerySearch', () => {
const mockedProps = {
actions: { addDangerToast: jest.fn() },
displayLimit: 50,
};

Expand Down
29 changes: 13 additions & 16 deletions superset-frontend/src/SqlLab/components/QuerySearch/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
* under the License.
*/
import React, { useState, useEffect } from 'react';
import { useDispatch } from 'react-redux';

import { setDatabases, addDangerToast } from 'src/SqlLab/actions/sqlLab';
import Button from 'src/components/Button';
import Select from 'src/components/DeprecatedSelect';
import { styled, t, SupersetClient, QueryResponse } from '@superset-ui/core';
Expand All @@ -33,15 +36,6 @@ import { STATUS_OPTIONS, TIME_OPTIONS } from 'src/SqlLab/constants';
import QueryTable from '../QueryTable';

interface QuerySearchProps {
actions: {
addDangerToast: (msg: string) => void;
setDatabases: (data: Record<string, any>) => Record<string, any>;
queryEditorSetAndSaveSql: Function;
cloneQueryToNewTab: Function;
fetchQueryResults: Function;
clearQueryResults: Function;
removeQuery: Function;
};
displayLimit: number;
}

Expand Down Expand Up @@ -77,7 +71,10 @@ const TableStyles = styled.div`
const StyledTableStylesContainer = styled.div`
overflow: auto;
`;
function QuerySearch({ actions, displayLimit }: QuerySearchProps) {

const QuerySearch = ({ displayLimit }: QuerySearchProps) => {
const dispatch = useDispatch();

const [databaseId, setDatabaseId] = useState<string>('');
const [userId, setUserId] = useState<string>('');
const [searchText, setSearchText] = useState<string>('');
Expand Down Expand Up @@ -133,7 +130,7 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) {
const queries = Object.values(response.json);
setQueriesArray(queries);
} catch (err) {
actions.addDangerToast(t('An error occurred when refreshing queries'));
dispatch(addDangerToast(t('An error occurred when refreshing queries')));
} finally {
setQueriesLoading(false);
}
Expand Down Expand Up @@ -178,10 +175,10 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) {
value: id,
label: database_name,
}));
actions.setDatabases(result);
dispatch(setDatabases(result));
if (result.length === 0) {
actions.addDangerToast(
t("It seems you don't have access to any database"),
dispatch(
addDangerToast(t("It seems you don't have access to any database")),
);
}
return options;
Expand Down Expand Up @@ -280,13 +277,13 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) {
onUserClicked={onUserClicked}
onDbClicked={onDbClicked}
queries={queriesArray}
actions={actions}
displayLimit={displayLimit}
/>
</TableStyles>
)}
</StyledTableStylesContainer>
</TableWrapper>
);
}
};

export default QuerySearch;
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ import TableView from 'src/components/TableView';
import TableCollection from 'src/components/TableCollection';
import { Provider } from 'react-redux';
import { queries, user } from 'src/SqlLab/fixtures';
import * as actions from 'src/SqlLab/actions/sqlLab';

describe('QueryTable', () => {
const mockedProps = {
queries,
displayLimit: 100,
actions,
latestQueryId: 'ryhMUZCGb',
};
it('is valid', () => {
Expand Down
51 changes: 16 additions & 35 deletions superset-frontend/src/SqlLab/components/QueryTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@ import Card from 'src/components/Card';
import ProgressBar from 'src/components/ProgressBar';
import Label from 'src/components/Label';
import { t, useTheme, QueryResponse } from '@superset-ui/core';
import { useSelector } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';

import {
queryEditorSetAndSaveSql,
cloneQueryToNewTab,
fetchQueryResults,
clearQueryResults,
removeQuery,
} from 'src/SqlLab/actions/sqlLab';
import TableView from 'src/components/TableView';
import Button from 'src/components/Button';
import { fDuration } from 'src/utils/dates';
Expand All @@ -45,13 +53,6 @@ interface QueryTableQuery

interface QueryTableProps {
columns?: string[];
actions: {
queryEditorSetAndSaveSql: Function;
cloneQueryToNewTab: Function;
fetchQueryResults: Function;
clearQueryResults: Function;
removeQuery: Function;
};
queries?: QueryResponse[];
onUserClicked?: Function;
onDbClicked?: Function;
Expand All @@ -66,14 +67,14 @@ const openQuery = (id: number) => {

const QueryTable = ({
columns = ['started', 'duration', 'rows'],
actions,
queries = [],
onUserClicked = () => undefined,
onDbClicked = () => undefined,
displayLimit,
latestQueryId,
}: QueryTableProps) => {
const theme = useTheme();
const dispatch = useDispatch();

const setHeaders = (column: string) => {
if (column === 'sql') {
Expand All @@ -93,25 +94,17 @@ const QueryTable = ({

const user = useSelector<SqlLabRootState, User>(state => state.sqlLab.user);

const {
queryEditorSetAndSaveSql,
cloneQueryToNewTab,
fetchQueryResults,
clearQueryResults,
removeQuery,
} = actions;

const data = useMemo(() => {
const restoreSql = (query: QueryResponse) => {
queryEditorSetAndSaveSql({ id: query.sqlEditorId }, query.sql);
dispatch(queryEditorSetAndSaveSql({ id: query.sqlEditorId }, query.sql));
};

const openQueryInNewTab = (query: QueryResponse) => {
cloneQueryToNewTab(query, true);
dispatch(cloneQueryToNewTab(query, true));
};

const openAsyncResults = (query: QueryResponse, displayLimit: number) => {
fetchQueryResults(query, displayLimit);
dispatch(fetchQueryResults(query, displayLimit));
};

const statusAttributes = {
Expand Down Expand Up @@ -239,7 +232,7 @@ const QueryTable = ({
}
modalTitle={t('Data preview')}
beforeOpen={() => openAsyncResults(query, displayLimit)}
onExit={() => clearQueryResults(query)}
onExit={() => dispatch(clearQueryResults(query))}
modalBody={
<ResultSet
showSql
Expand Down Expand Up @@ -293,7 +286,7 @@ const QueryTable = ({
{q.id !== latestQueryId && (
<StyledTooltip
tooltip={t('Remove query from log')}
onClick={() => removeQuery(query)}
onClick={() => dispatch(removeQuery(query))}
>
<Icons.Trash iconSize="xl" />
</StyledTooltip>
Expand All @@ -303,19 +296,7 @@ const QueryTable = ({
return q;
})
.reverse();
}, [
queries,
onUserClicked,
onDbClicked,
user,
displayLimit,
actions,
clearQueryResults,
cloneQueryToNewTab,
fetchQueryResults,
queryEditorSetAndSaveSql,
removeQuery,
]);
}, [queries, onUserClicked, onDbClicked, user, displayLimit]);

return (
<div className="QueryTable">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ import { DropdownButton } from 'src/components/DropdownButton';
import Button from 'src/components/Button';
import { DropdownButtonProps } from 'antd/lib/dropdown';

interface Props {
interface SaveDatasetActionButtonProps {
setShowSave: (arg0: boolean) => void;
overlayMenu: JSX.Element | null;
}

export default function SaveDatasetActionButton({
const SaveDatasetActionButton = ({
setShowSave,
overlayMenu,
}: Props) {
}: SaveDatasetActionButtonProps) => {
const theme = useTheme();

const StyledDropdownButton = styled(
Expand Down Expand Up @@ -80,4 +80,6 @@ export default function SaveDatasetActionButton({
{t('Save')}
</StyledDropdownButton>
);
}
};

export default SaveDatasetActionButton;
Loading