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(SqlLab): improved database, schema, and table selection UI #31591

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
40 changes: 18 additions & 22 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,15 @@ export function clearInactiveQueries(interval) {
return { type: CLEAR_INACTIVE_QUERIES, interval };
}

export function startQuery(query) {
export function startQuery(query, runPreviewOnly) {
Object.assign(query, {
id: query.id ? query.id : nanoid(11),
progress: 0,
startDttm: now(),
state: query.runAsync ? 'pending' : 'running',
cached: false,
});
return { type: START_QUERY, query };
return { type: START_QUERY, query, runPreviewOnly };
}

export function querySuccess(query, results) {
Expand Down Expand Up @@ -327,9 +327,9 @@ export function fetchQueryResults(query, displayLimit, timeoutInMs) {
};
}

export function runQuery(query) {
export function runQuery(query, runPreviewOnly) {
return function (dispatch) {
dispatch(startQuery(query));
dispatch(startQuery(query, runPreviewOnly));
const postPayload = {
client_id: query.id,
database_id: query.dbId,
Expand Down Expand Up @@ -949,29 +949,25 @@ export function mergeTable(table, query, prepend) {

export function addTable(queryEditor, tableName, catalogName, schemaName) {
return function (dispatch, getState) {
const query = getUpToDateQuery(getState(), queryEditor, queryEditor.id);
const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id);
const table = {
dbId: query.dbId,
queryEditorId: query.id,
dbId,
queryEditorId: queryEditor.id,
catalog: catalogName,
schema: schemaName,
name: tableName,
};
dispatch(
mergeTable(
{
...table,
id: nanoid(11),
expanded: true,
},
null,
true,
),
mergeTable({
...table,
id: nanoid(11),
expanded: true,
}),
);
};
}

export function runTablePreviewQuery(newTable) {
export function runTablePreviewQuery(newTable, runPreviewOnly) {
return function (dispatch, getState) {
const {
sqlLab: { databases },
Expand All @@ -981,7 +977,7 @@ export function runTablePreviewQuery(newTable) {

if (database && !database.disable_data_preview) {
const dataPreviewQuery = {
id: nanoid(11),
id: newTable.previewQueryId ?? nanoid(11),
dbId,
catalog,
schema,
Expand All @@ -993,6 +989,9 @@ export function runTablePreviewQuery(newTable) {
ctas: false,
isDataPreview: true,
};
if (runPreviewOnly) {
return dispatch(runQuery(dataPreviewQuery, runPreviewOnly));
}
return Promise.all([
dispatch(
mergeTable(
Expand Down Expand Up @@ -1026,17 +1025,14 @@ export function syncTable(table, tableMetadata) {

return sync
.then(({ json: resultJson }) => {
const newTable = { ...table, id: resultJson.id };
const newTable = { ...table, id: `${resultJson.id}` };
dispatch(
mergeTable({
...newTable,
expanded: true,
initialized: true,
}),
);
if (!table.dataPreviewQueryId) {
dispatch(runTablePreviewQuery({ ...tableMetadata, ...newTable }));
}
})
.catch(() =>
dispatch(
Expand Down
70 changes: 51 additions & 19 deletions superset-frontend/src/SqlLab/actions/sqlLab.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -924,30 +924,38 @@ describe('async actions', () => {
});

describe('runTablePreviewQuery', () => {
it('updates and runs data preview query when configured', () => {
expect.assertions(3);
const results = {
data: mockBigNumber,
query: { sqlEditorId: 'null', dbId: 1 },
query_id: 'efgh',
};
const tableName = 'table';
const catalogName = null;
const schemaName = 'schema';
const store = mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
databases: {
1: { disable_data_preview: false },
},
},
});

const results = {
data: mockBigNumber,
query: { sqlEditorId: 'null', dbId: 1 },
query_id: 'efgh',
};
beforeEach(() => {
fetchMock.post(runQueryEndpoint, JSON.stringify(results), {
overwriteRoutes: true,
});
});

afterEach(() => {
store.clearActions();
fetchMock.resetHistory();
});

it('updates and runs data preview query when configured', () => {
expect.assertions(3);

const tableName = 'table';
const catalogName = null;
const schemaName = 'schema';
const store = mockStore({
...initialState,
sqlLab: {
...initialState.sqlLab,
databases: {
1: { disable_data_preview: false },
},
},
});
const expectedActionTypes = [
actions.MERGE_TABLE, // addTable (data preview)
actions.START_QUERY, // runQuery (data preview)
Expand All @@ -968,6 +976,30 @@ describe('async actions', () => {
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0);
});
});

it('runs data preview query only', () => {
const expectedActionTypes = [
actions.START_QUERY, // runQuery (data preview)
actions.QUERY_SUCCESS, // querySuccess
];
const request = actions.runTablePreviewQuery(
{
dbId: 1,
name: tableName,
catalog: catalogName,
schema: schemaName,
},
true,
);
return request(store.dispatch, store.getState).then(() => {
expect(store.getActions().map(a => a.type)).toEqual(
expectedActionTypes,
);
expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1);
// tab state is not updated, since the query is a data preview
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0);
});
});
});

describe('expandTable', () => {
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/components/App/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const SqlLabStyles = styled.div`
left: 0;
padding: 0 ${theme.gridUnit * 2}px;

pre {
pre:not(.code) {
padding: 0 !important;
margin: 0;
border: none;
Expand Down
30 changes: 26 additions & 4 deletions superset-frontend/src/SqlLab/components/ColumnElement/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,40 @@ interface ColumnElementProps {
name: string;
keys?: { type: ColumnKeyTypeType }[];
type: string;
longType: string;
};
searchText?: string;
}

const NowrapDiv = styled.div`
white-space: nowrap;
`;

const ColumnElement = ({ column }: ColumnElementProps) => {
let columnName: ReactNode = column.name;
const HighlightText = styled.span`
background-color: ${({ theme }) => theme.colors.alert.light1};
`;

const ColumnElement = ({ column, searchText }: ColumnElementProps) => {
const columnName = column.name;
const index = searchText ? columnName.indexOf(searchText) : -1;
const beforeStr = columnName.substring(0, index);
const afterStr = columnName.slice(index + (searchText?.length ?? 0));
let icons;
let columnDisplayName = (
<>
{index > -1 ? (
<span>
{beforeStr}
<HighlightText>{searchText}</HighlightText>
{afterStr}
</span>
) : (
columnName
)}
</>
);
if (column.keys && column.keys.length > 0) {
columnName = <strong>{column.name}</strong>;
columnDisplayName = <strong>{columnDisplayName}</strong>;
icons = column.keys.map((key, i) => (
<span key={i} className="ColumnElement">
<StyledTooltip
Expand All @@ -106,7 +128,7 @@ const ColumnElement = ({ column }: ColumnElementProps) => {
return (
<div className="clearfix table-column">
<div className="pull-left m-l-10 col-name">
{columnName}
{columnDisplayName}
{icons}
</div>
<NowrapDiv className="pull-right text-muted">
Expand Down
12 changes: 8 additions & 4 deletions superset-frontend/src/SqlLab/components/ShowSQL/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,25 @@ interface ShowSQLProps {
sql: string;
title: string;
tooltipText: string;
triggerNode?: React.ReactNode;
}

export default function ShowSQL({
tooltipText,
title,
sql: sqlString,
triggerNode,
}: ShowSQLProps) {
return (
<ModalTrigger
modalTitle={title}
triggerNode={
<IconTooltip
className="fa fa-eye pull-left m-l-2"
tooltip={tooltipText}
/>
triggerNode || (
<IconTooltip
className="fa fa-eye pull-left m-l-2"
tooltip={tooltipText}
/>
)
}
modalBody={
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ test('should render empty result state when latestQuery is empty', () => {
expect(resultPanel).toHaveTextContent('Run a query to display results');
});

test('should render tabs for table preview queries', () => {
test('should render tabs for table metadata view', () => {
const { getAllByRole } = render(<SouthPane {...mockedProps} />, {
useRedux: true,
initialState: mockState,
Expand All @@ -145,7 +145,7 @@ test('should render tabs for table preview queries', () => {
expect(tabs).toHaveLength(mockState.sqlLab.tables.length + 2);
expect(tabs[0]).toHaveTextContent('Results');
expect(tabs[1]).toHaveTextContent('Query history');
mockState.sqlLab.tables.forEach(({ name }, index) => {
expect(tabs[index + 2]).toHaveTextContent(`Preview: \`${name}\``);
mockState.sqlLab.tables.forEach(({ name, schema }, index) => {
expect(tabs[index + 2]).toHaveTextContent(`${schema}.${name}`);
});
});
Loading
Loading