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.

Sign up for GitHub

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

fix(sqllab): Show warning message when deprecated db is selected #29607

Merged
merged 3 commits into from
Jul 17, 2024
Merged

fix(sqllab): Show warning message when deprecated db is selected #29607

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c62d8b
fix(sqllab): Show warning message when deprecated db is selected
justinpark Jul 16, 2024
a43e448
update warning message
justinpark Jul 16, 2024
1a2889d
more generic desc
justinpark Jul 16, 2024
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
12 changes: 11 additions & 1 deletion superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,23 @@ describe('SqlEditor', () => {
});

it('does not render SqlEditor if no db selected', async () => {
const queryEditor = initialState.sqlLab.queryEditors[1];
const queryEditor = initialState.sqlLab.queryEditors[2];
const { findByText } = setup({ ...mockedProps, queryEditor }, store);
expect(
await findByText('Select a database to write a query'),
).toBeInTheDocument();
});

it('renders db deprecated message', async () => {
const queryEditor = initialState.sqlLab.queryEditors[1];
const { findByText } = setup({ ...mockedProps, queryEditor }, store);
expect(
await findByText(
'The selected database is currently deprecated and cannot be used',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'The selected database is currently deprecated and cannot be used',
'The database that was used to generate this query could not be found',

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option perhaps "could not be reached"?

),
).toBeInTheDocument();
});

it('render a SqlEditorLeftBar', async () => {
const { getByTestId } = setup(mockedProps, store);
await waitFor(() =>
Expand Down
185 changes: 104 additions & 81 deletions superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ import {
setItem,
} from 'src/utils/localStorageHelpers';
import { EmptyStateBig } from 'src/components/EmptyState';
import Alert from 'src/components/Alert';
import getBootstrapData from 'src/utils/getBootstrapData';
import useLogAction from 'src/logger/useLogAction';
import {
Expand Down Expand Up @@ -258,31 +259,38 @@ const SqlEditor: FC<Props> = ({
const theme = useTheme();
const dispatch = useDispatch();

const { database, latestQuery, hideLeftBar, currentQueryEditorId } =
useSelector<
SqlLabRootState,
{
database?: DatabaseObject;
latestQuery?: QueryResponse;
hideLeftBar?: boolean;
currentQueryEditorId: QueryEditor['id'];
}
>(({ sqlLab: { unsavedQueryEditor, databases, queries, tabHistory } }) => {
let { dbId, latestQueryId, hideLeftBar } = queryEditor;
if (unsavedQueryEditor?.id === queryEditor.id) {
dbId = unsavedQueryEditor.dbId || dbId;
latestQueryId = unsavedQueryEditor.latestQueryId || latestQueryId;
hideLeftBar = isBoolean(unsavedQueryEditor.hideLeftBar)
? unsavedQueryEditor.hideLeftBar
: hideLeftBar;
}
return {
database: databases[dbId || ''],
latestQuery: queries[latestQueryId || ''],
hideLeftBar,
currentQueryEditorId: tabHistory.slice(-1)[0],
};
}, shallowEqual);
const {
database,
latestQuery,
hideLeftBar,
currentQueryEditorId,
hasSqlStatement,
} = useSelector<
SqlLabRootState,
{
database?: DatabaseObject;
latestQuery?: QueryResponse;
hideLeftBar?: boolean;
currentQueryEditorId: QueryEditor['id'];
hasSqlStatement: boolean;
}
>(({ sqlLab: { unsavedQueryEditor, databases, queries, tabHistory } }) => {
let { dbId, latestQueryId, hideLeftBar } = queryEditor;
if (unsavedQueryEditor?.id === queryEditor.id) {
dbId = unsavedQueryEditor.dbId || dbId;
latestQueryId = unsavedQueryEditor.latestQueryId || latestQueryId;
hideLeftBar = isBoolean(unsavedQueryEditor.hideLeftBar)
? unsavedQueryEditor.hideLeftBar
: hideLeftBar;
}
return {
hasSqlStatement: Boolean(queryEditor.sql?.trim().length > 0),
database: databases[dbId || ''],
latestQuery: queries[latestQueryId || ''],
hideLeftBar,
currentQueryEditorId: tabHistory.slice(-1)[0],
};
}, shallowEqual);

const logAction = useLogAction({ queryEditorId: queryEditor.id });
const isActive = currentQueryEditorId === queryEditor.id;
Expand Down Expand Up @@ -728,7 +736,7 @@ const SqlEditor: FC<Props> = ({
dispatch(addSavedQueryToTabState(queryEditor, savedQuery));
};

const renderEditorBottomBar = () => {
const renderEditorBottomBar = (hideActions: boolean) => {
const { allow_ctas: allowCTAS, allow_cvas: allowCVAS } = database || {};

const showMenu = allowCTAS || allowCVAS;
Expand Down Expand Up @@ -767,63 +775,78 @@ const SqlEditor: FC<Props> = ({

return (
<StyledToolbar className="sql-toolbar" id="js-sql-toolbar">
<div className="leftItems">
<span>
<RunQueryActionButton
allowAsync={database?.allow_run_async === true}
queryEditorId={queryEditor.id}
queryState={latestQuery?.state}
runQuery={runQuery}
stopQuery={stopQuery}
overlayCreateAsMenu={showMenu ? runMenuBtn : null}
/>
</span>
{isFeatureEnabled(FeatureFlag.EstimateQueryCost) &&
database?.allows_cost_estimate && (
{hideActions ? (
<Alert
type="warning"
message={t(
'The selected database is currently deprecated and cannot be used',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot assume that the database was deprecated. It may be an intermittent error. I suggest a more generic message.

Suggested change
'The selected database is currently deprecated and cannot be used',
'The database that was used to generate this query could not be found',

)}
description={t(
'Choose one of the available databases from the panel on the left.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Choose one of the available databases from the panel on the left.',
'Choose one of the available databases on the left panel.',

)}
closable={false}
/>
) : (
<>
<div className="leftItems">
<span>
<EstimateQueryCostButton
getEstimate={getQueryCostEstimate}
<RunQueryActionButton
allowAsync={database?.allow_run_async === true}
queryEditorId={queryEditor.id}
tooltip={t('Estimate the cost before running a query')}
queryState={latestQuery?.state}
runQuery={runQuery}
stopQuery={stopQuery}
overlayCreateAsMenu={showMenu ? runMenuBtn : null}
/>
</span>
)}
<span>
<QueryLimitSelect
queryEditorId={queryEditor.id}
maxRow={maxRow}
defaultQueryLimit={defaultQueryLimit}
/>
</span>
{latestQuery && (
<Timer
startTime={latestQuery.startDttm}
endTime={latestQuery.endDttm}
status={STATE_TYPE_MAP[latestQuery.state]}
isRunning={latestQuery.state === 'running'}
/>
)}
</div>
<div className="rightItems">
<span>
<SaveQuery
queryEditorId={queryEditor.id}
columns={latestQuery?.results?.columns || []}
onSave={onSaveQuery}
onUpdate={(query, remoteId) =>
dispatch(updateSavedQuery(query, remoteId))
}
saveQueryWarning={saveQueryWarning}
database={database}
/>
</span>
<span>
<ShareSqlLabQuery queryEditorId={queryEditor.id} />
</span>
<AntdDropdown overlay={renderDropdown()} trigger={['click']}>
<Icons.MoreHoriz iconColor={theme.colors.grayscale.base} />
</AntdDropdown>
</div>
{isFeatureEnabled(FeatureFlag.EstimateQueryCost) &&
database?.allows_cost_estimate && (
<span>
<EstimateQueryCostButton
getEstimate={getQueryCostEstimate}
queryEditorId={queryEditor.id}
tooltip={t('Estimate the cost before running a query')}
/>
</span>
)}
<span>
<QueryLimitSelect
queryEditorId={queryEditor.id}
maxRow={maxRow}
defaultQueryLimit={defaultQueryLimit}
/>
</span>
{latestQuery && (
<Timer
startTime={latestQuery.startDttm}
endTime={latestQuery.endDttm}
status={STATE_TYPE_MAP[latestQuery.state]}
isRunning={latestQuery.state === 'running'}
/>
)}
</div>
<div className="rightItems">
<span>
<SaveQuery
queryEditorId={queryEditor.id}
columns={latestQuery?.results?.columns || []}
onSave={onSaveQuery}
onUpdate={(query, remoteId) =>
dispatch(updateSavedQuery(query, remoteId))
}
saveQueryWarning={saveQueryWarning}
database={database}
/>
</span>
<span>
<ShareSqlLabQuery queryEditorId={queryEditor.id} />
</span>
<AntdDropdown overlay={renderDropdown()} trigger={['click']}>
<Icons.MoreHoriz iconColor={theme.colors.grayscale.base} />
</AntdDropdown>
</div>
</>
)}
</StyledToolbar>
);
};
Expand Down Expand Up @@ -866,7 +889,7 @@ const SqlEditor: FC<Props> = ({
height={`${aceEditorHeight}px`}
hotkeys={hotkeys}
/>
{renderEditorBottomBar()}
{renderEditorBottomBar(showEmptyState)}
</div>
<SouthPane
queryEditorId={queryEditor.id}
Expand Down Expand Up @@ -923,7 +946,7 @@ const SqlEditor: FC<Props> = ({
>
<Skeleton active />
</div>
) : showEmptyState ? (
) : showEmptyState && !hasSqlStatement ? (
<EmptyStateBig
image="vector.svg"
title={t('Select a database to write a query')}
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/SqlLab/fixtures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ export const extraQueryEditor1 = {
export const extraQueryEditor2 = {
...defaultQueryEditor,
id: 'owkdi998',
sql: 'SELECT *\nFROM\nWHERE\nGROUP BY',
sql: '',
name: 'Untitled Query 3',
};

Expand Down
Loading