Skip to content

Commit 1638532

Browse files
justinparksadpandajoe
authored andcommitted
fix(explore): missing column autocomplete in custom SQL (#29672)
(cherry picked from commit 3c97145)
1 parent 9677fa9 commit 1638532

File tree

9 files changed

+268
-30
lines changed

9 files changed

+268
-30
lines changed
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { render, screen } from 'spec/helpers/testing-library';
21+
import Tooltip, { getTooltipHTML } from './Tooltip';
22+
23+
test('should render a tooltip', () => {
24+
const expected = {
25+
title: 'tooltip title',
26+
icon: <div>icon</div>,
27+
body: <div>body</div>,
28+
meta: 'meta',
29+
footer: <div>footer</div>,
30+
};
31+
render(<Tooltip {...expected} />);
32+
expect(screen.getByText(expected.title)).toBeInTheDocument();
33+
expect(screen.getByText(expected.meta)).toBeInTheDocument();
34+
expect(screen.getByText('icon')).toBeInTheDocument();
35+
expect(screen.getByText('body')).toBeInTheDocument();
36+
});
37+
38+
test('returns the tooltip HTML', () => {
39+
const html = getTooltipHTML({
40+
title: 'tooltip title',
41+
icon: <div>icon</div>,
42+
body: <div>body</div>,
43+
meta: 'meta',
44+
footer: <div>footer</div>,
45+
});
46+
expect(html).toContain('tooltip title');
47+
});
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import { renderToStaticMarkup } from 'react-dom/server';
20+
import { Tag } from 'src/components';
21+
22+
type Props = {
23+
title: string;
24+
icon?: React.ReactNode;
25+
body?: React.ReactNode;
26+
meta?: string;
27+
footer?: React.ReactNode;
28+
};
29+
30+
export const Tooltip: React.FC<Props> = ({
31+
title,
32+
icon,
33+
body,
34+
meta,
35+
footer,
36+
}) => (
37+
<div className="tooltip-detail">
38+
<div className="tooltip-detail-head">
39+
<div className="tooltip-detail-title">
40+
{icon}
41+
{title}
42+
</div>
43+
{meta && (
44+
<span className="tooltip-detail-meta">
45+
<Tag color="default">{meta}</Tag>
46+
</span>
47+
)}
48+
</div>
49+
{body && <div className="tooltip-detail-body">{body ?? title}</div>}
50+
{footer && <div className="tooltip-detail-footer">{footer}</div>}
51+
</div>
52+
);
53+
54+
export const getTooltipHTML = (props: Props) =>
55+
`${renderToStaticMarkup(<Tooltip {...props} />)}`;
56+
57+
export default Tooltip;

superset-frontend/src/components/AsyncAceEditor/index.tsx

Lines changed: 65 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ import AsyncEsmComponent, {
3232
} from 'src/components/AsyncEsmComponent';
3333
import useEffectEvent from 'src/hooks/useEffectEvent';
3434
import cssWorkerUrl from 'ace-builds/src-noconflict/worker-css';
35+
import { useTheme, css } from '@superset-ui/core';
36+
import { Global } from '@emotion/react';
37+
38+
export { getTooltipHTML } from './Tooltip';
3539

3640
config.setModuleUrl('ace/mode/css_worker', cssWorkerUrl);
3741

@@ -135,6 +139,7 @@ export default function AsyncAceEditor(
135139
},
136140
ref,
137141
) {
142+
const supersetTheme = useTheme();
138143
const langTools = acequire('ace/ext/language_tools');
139144
const setCompleters = useEffectEvent(
140145
(keywords: AceCompleterKeyword[]) => {
@@ -167,15 +172,66 @@ export default function AsyncAceEditor(
167172
}, [keywords, setCompleters]);
168173

169174
return (
170-
<ReactAceEditor
171-
ref={ref}
172-
mode={mode}
173-
theme={theme}
174-
tabSize={tabSize}
175-
defaultValue={defaultValue}
176-
setOptions={{ fontFamily }}
177-
{...props}
178-
/>
175+
<>
176+
<Global
177+
styles={css`
178+
.ace_tooltip {
179+
margin-left: ${supersetTheme.gridUnit * 2}px;
180+
padding: 0px;
181+
border: 1px solid ${supersetTheme.colors.grayscale.light1};
182+
}
183+
184+
& .tooltip-detail {
185+
background-color: ${supersetTheme.colors.grayscale.light5};
186+
white-space: pre-wrap;
187+
word-break: break-all;
188+
min-width: ${supersetTheme.gridUnit * 50}px;
189+
max-width: ${supersetTheme.gridUnit * 100}px;
190+
& .tooltip-detail-head {
191+
background-color: ${supersetTheme.colors.grayscale.light4};
192+
color: ${supersetTheme.colors.grayscale.dark1};
193+
display: flex;
194+
column-gap: ${supersetTheme.gridUnit}px;
195+
align-items: baseline;
196+
justify-content: space-between;
197+
}
198+
& .tooltip-detail-title {
199+
display: flex;
200+
column-gap: ${supersetTheme.gridUnit}px;
201+
}
202+
& .tooltip-detail-body {
203+
word-break: break-word;
204+
}
205+
& .tooltip-detail-head,
206+
& .tooltip-detail-body {
207+
padding: ${supersetTheme.gridUnit}px
208+
${supersetTheme.gridUnit * 2}px;
209+
}
210+
& .tooltip-detail-footer {
211+
border-top: 1px ${supersetTheme.colors.grayscale.light2}
212+
solid;
213+
padding: 0 ${supersetTheme.gridUnit * 2}px;
214+
color: ${supersetTheme.colors.grayscale.dark1};
215+
font-size: ${supersetTheme.typography.sizes.xs}px;
216+
}
217+
& .tooltip-detail-meta {
218+
& > .ant-tag {
219+
margin-right: 0px;
220+
}
221+
}
222+
}
223+
`}
224+
/>
225+
<ReactAceEditor
226+
ref={ref}
227+
mode={mode}
228+
theme={theme}
229+
tabSize={tabSize}
230+
defaultValue={defaultValue}
231+
setOptions={{ fontFamily }}
232+
{...props}
233+
/>
234+
</>
179235
);
180236
},
181237
);

superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ import Button from 'src/components/Button';
4141
import { Select } from 'src/components';
4242

4343
import { Form, FormItem } from 'src/components/Form';
44+
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';
4445
import { SQLEditor } from 'src/components/AsyncAceEditor';
4546
import { EmptyStateSmall } from 'src/components/EmptyState';
47+
import { getColumnKeywords } from 'src/explore/controlUtils/getColumnKeywords';
4648
import { StyledColumnOption } from 'src/explore/components/optionRenderers';
4749
import {
4850
POPOVER_INITIAL_HEIGHT,
@@ -287,6 +289,10 @@ const ColumnSelectPopover = ({
287289

288290
const savedExpressionsLabel = t('Saved expressions');
289291
const simpleColumnsLabel = t('Column');
292+
const keywords = useMemo(
293+
() => sqlKeywords.concat(getColumnKeywords(columns)),
294+
[columns],
295+
);
290296

291297
return (
292298
<Form layout="vertical" id="metrics-edit-popover">
@@ -451,6 +457,7 @@ const ColumnSelectPopover = ({
451457
className="filter-sql-editor"
452458
wrapEnabled
453459
ref={sqlEditorRef}
460+
keywords={keywords}
454461
/>
455462
</Tabs.TabPane>
456463
</Tabs>

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import { DndItemType } from '../../DndItemType';
4848
import DatasourcePanelDragOption from '../../DatasourcePanel/DatasourcePanelDragOption';
4949

5050
jest.mock('src/components/AsyncAceEditor', () => ({
51+
...jest.requireActual('src/components/AsyncAceEditor'),
5152
SQLEditor: (props: AsyncAceEditorProps) => (
5253
<div data-test="react-ace">{props.value}</div>
5354
),

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { styled, t } from '@superset-ui/core';
2323
import { SQLEditor } from 'src/components/AsyncAceEditor';
2424
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';
2525

26+
import { getColumnKeywords } from 'src/explore/controlUtils/getColumnKeywords';
2627
import adhocMetricType from 'src/explore/components/controls/MetricControl/adhocMetricType';
2728
import columnType from 'src/explore/components/controls/FilterControl/columnType';
2829
import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter';
@@ -91,19 +92,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends Component {
9192
const { adhocFilter, height, options } = this.props;
9293

9394
const keywords = sqlKeywords.concat(
94-
options
95-
.map(option => {
96-
if (option.column_name) {
97-
return {
98-
name: option.column_name,
99-
value: option.column_name,
100-
score: 50,
101-
meta: 'option',
102-
};
103-
}
104-
return null;
105-
})
106-
.filter(Boolean),
95+
getColumnKeywords(options.filter(option => option.column_name)),
10796
);
10897
const selectOptions = Object.values(Clauses).map(clause => ({
10998
label: clause,

superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
StyledMetricOption,
5050
StyledColumnOption,
5151
} from 'src/explore/components/optionRenderers';
52+
import { getColumnKeywords } from 'src/explore/controlUtils/getColumnKeywords';
5253

5354
const propTypes = {
5455
onChange: PropTypes.func.isRequired,
@@ -304,14 +305,7 @@ export default class AdhocMetricEditPopover extends PureComponent {
304305
...popoverProps
305306
} = this.props;
306307
const { adhocMetric, savedMetric } = this.state;
307-
const keywords = sqlKeywords.concat(
308-
columns.map(column => ({
309-
name: column.column_name,
310-
value: column.column_name,
311-
score: 50,
312-
meta: 'column',
313-
})),
314-
);
308+
const keywords = sqlKeywords.concat(getColumnKeywords(columns));
315309

316310
const columnValue =
317311
(adhocMetric.column && adhocMetric.column.column_name) ||
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { getColumnKeywords } from './getColumnKeywords';
21+
22+
test('returns HTML for a column tooltip', () => {
23+
const expected = {
24+
column_name: 'test column1',
25+
verbose_name: null,
26+
is_certified: false,
27+
certified_by: null,
28+
description: 'test description',
29+
type: 'VARCHAR',
30+
};
31+
expect(getColumnKeywords([expected])).toContainEqual({
32+
name: expected.column_name,
33+
value: expected.column_name,
34+
docHTML: expect.stringContaining(expected.description),
35+
score: 50,
36+
meta: 'column',
37+
});
38+
});
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { ColumnMeta } from '@superset-ui/chart-controls';
21+
import { t } from '@superset-ui/core';
22+
import { getTooltipHTML } from 'src/components/AsyncAceEditor';
23+
import { COLUMN_AUTOCOMPLETE_SCORE } from 'src/SqlLab/constants';
24+
25+
export function getColumnKeywords(columns: ColumnMeta[]) {
26+
return columns.map(
27+
({
28+
column_name,
29+
verbose_name,
30+
is_certified,
31+
certified_by,
32+
description,
33+
type,
34+
}) => ({
35+
name: verbose_name || column_name,
36+
value: column_name,
37+
docHTML: getTooltipHTML({
38+
title: column_name,
39+
meta: type ? `column: ${type}` : 'column',
40+
body: `${description ?? ''}`,
41+
footer: is_certified ? (
42+
<>{t('Certified by %s', certified_by)}</>
43+
) : undefined,
44+
}),
45+
score: COLUMN_AUTOCOMPLETE_SCORE,
46+
meta: 'column',
47+
}),
48+
);
49+
}

0 commit comments

Comments
 (0)