Skip to content

Commit

Permalink
fix(explore): missing column autocomplete in custom SQL (#29672)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored Jul 26, 2024
1 parent 5ed1931 commit 3c97145
Show file tree
Hide file tree
Showing 9 changed files with 268 additions and 30 deletions.
47 changes: 47 additions & 0 deletions superset-frontend/src/components/AsyncAceEditor/Tooltip.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { render, screen } from 'spec/helpers/testing-library';
import Tooltip, { getTooltipHTML } from './Tooltip';

test('should render a tooltip', () => {
const expected = {
title: 'tooltip title',
icon: <div>icon</div>,
body: <div>body</div>,
meta: 'meta',
footer: <div>footer</div>,
};
render(<Tooltip {...expected} />);
expect(screen.getByText(expected.title)).toBeInTheDocument();
expect(screen.getByText(expected.meta)).toBeInTheDocument();
expect(screen.getByText('icon')).toBeInTheDocument();
expect(screen.getByText('body')).toBeInTheDocument();
});

test('returns the tooltip HTML', () => {
const html = getTooltipHTML({
title: 'tooltip title',
icon: <div>icon</div>,
body: <div>body</div>,
meta: 'meta',
footer: <div>footer</div>,
});
expect(html).toContain('tooltip title');
});
57 changes: 57 additions & 0 deletions superset-frontend/src/components/AsyncAceEditor/Tooltip.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { renderToStaticMarkup } from 'react-dom/server';
import { Tag } from 'src/components';

type Props = {
title: string;
icon?: React.ReactNode;
body?: React.ReactNode;
meta?: string;
footer?: React.ReactNode;
};

export const Tooltip: React.FC<Props> = ({
title,
icon,
body,
meta,
footer,
}) => (
<div className="tooltip-detail">
<div className="tooltip-detail-head">
<div className="tooltip-detail-title">
{icon}
{title}
</div>
{meta && (
<span className="tooltip-detail-meta">
<Tag color="default">{meta}</Tag>
</span>
)}
</div>
{body && <div className="tooltip-detail-body">{body ?? title}</div>}
{footer && <div className="tooltip-detail-footer">{footer}</div>}
</div>
);

export const getTooltipHTML = (props: Props) =>
`${renderToStaticMarkup(<Tooltip {...props} />)}`;

export default Tooltip;
74 changes: 65 additions & 9 deletions superset-frontend/src/components/AsyncAceEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ import AsyncEsmComponent, {
} from 'src/components/AsyncEsmComponent';
import useEffectEvent from 'src/hooks/useEffectEvent';
import cssWorkerUrl from 'ace-builds/src-noconflict/worker-css';
import { useTheme, css } from '@superset-ui/core';
import { Global } from '@emotion/react';

export { getTooltipHTML } from './Tooltip';

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

Expand Down Expand Up @@ -135,6 +139,7 @@ export default function AsyncAceEditor(
},
ref,
) {
const supersetTheme = useTheme();
const langTools = acequire('ace/ext/language_tools');
const setCompleters = useEffectEvent(
(keywords: AceCompleterKeyword[]) => {
Expand Down Expand Up @@ -167,15 +172,66 @@ export default function AsyncAceEditor(
}, [keywords, setCompleters]);

return (
<ReactAceEditor
ref={ref}
mode={mode}
theme={theme}
tabSize={tabSize}
defaultValue={defaultValue}
setOptions={{ fontFamily }}
{...props}
/>
<>
<Global
styles={css`
.ace_tooltip {
margin-left: ${supersetTheme.gridUnit * 2}px;
padding: 0px;
border: 1px solid ${supersetTheme.colors.grayscale.light1};
}
& .tooltip-detail {
background-color: ${supersetTheme.colors.grayscale.light5};
white-space: pre-wrap;
word-break: break-all;
min-width: ${supersetTheme.gridUnit * 50}px;
max-width: ${supersetTheme.gridUnit * 100}px;
& .tooltip-detail-head {
background-color: ${supersetTheme.colors.grayscale.light4};
color: ${supersetTheme.colors.grayscale.dark1};
display: flex;
column-gap: ${supersetTheme.gridUnit}px;
align-items: baseline;
justify-content: space-between;
}
& .tooltip-detail-title {
display: flex;
column-gap: ${supersetTheme.gridUnit}px;
}
& .tooltip-detail-body {
word-break: break-word;
}
& .tooltip-detail-head,
& .tooltip-detail-body {
padding: ${supersetTheme.gridUnit}px
${supersetTheme.gridUnit * 2}px;
}
& .tooltip-detail-footer {
border-top: 1px ${supersetTheme.colors.grayscale.light2}
solid;
padding: 0 ${supersetTheme.gridUnit * 2}px;
color: ${supersetTheme.colors.grayscale.dark1};
font-size: ${supersetTheme.typography.sizes.xs}px;
}
& .tooltip-detail-meta {
& > .ant-tag {
margin-right: 0px;
}
}
}
`}
/>
<ReactAceEditor
ref={ref}
mode={mode}
theme={theme}
tabSize={tabSize}
defaultValue={defaultValue}
setOptions={{ fontFamily }}
{...props}
/>
</>
);
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ import Button from 'src/components/Button';
import { Select } from 'src/components';

import { Form, FormItem } from 'src/components/Form';
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';
import { SQLEditor } from 'src/components/AsyncAceEditor';
import { EmptyStateSmall } from 'src/components/EmptyState';
import { getColumnKeywords } from 'src/explore/controlUtils/getColumnKeywords';
import { StyledColumnOption } from 'src/explore/components/optionRenderers';
import {
POPOVER_INITIAL_HEIGHT,
Expand Down Expand Up @@ -287,6 +289,10 @@ const ColumnSelectPopover = ({

const savedExpressionsLabel = t('Saved expressions');
const simpleColumnsLabel = t('Column');
const keywords = useMemo(
() => sqlKeywords.concat(getColumnKeywords(columns)),
[columns],
);

return (
<Form layout="vertical" id="metrics-edit-popover">
Expand Down Expand Up @@ -451,6 +457,7 @@ const ColumnSelectPopover = ({
className="filter-sql-editor"
wrapEnabled
ref={sqlEditorRef}
keywords={keywords}
/>
</Tabs.TabPane>
</Tabs>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { DndItemType } from '../../DndItemType';
import DatasourcePanelDragOption from '../../DatasourcePanel/DatasourcePanelDragOption';

jest.mock('src/components/AsyncAceEditor', () => ({
...jest.requireActual('src/components/AsyncAceEditor'),
SQLEditor: (props: AsyncAceEditorProps) => (
<div data-test="react-ace">{props.value}</div>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { styled, t } from '@superset-ui/core';
import { SQLEditor } from 'src/components/AsyncAceEditor';
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';

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

const keywords = sqlKeywords.concat(
options
.map(option => {
if (option.column_name) {
return {
name: option.column_name,
value: option.column_name,
score: 50,
meta: 'option',
};
}
return null;
})
.filter(Boolean),
getColumnKeywords(options.filter(option => option.column_name)),
);
const selectOptions = Object.values(Clauses).map(clause => ({
label: clause,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
StyledMetricOption,
StyledColumnOption,
} from 'src/explore/components/optionRenderers';
import { getColumnKeywords } from 'src/explore/controlUtils/getColumnKeywords';

const propTypes = {
onChange: PropTypes.func.isRequired,
Expand Down Expand Up @@ -304,14 +305,7 @@ export default class AdhocMetricEditPopover extends PureComponent {
...popoverProps
} = this.props;
const { adhocMetric, savedMetric } = this.state;
const keywords = sqlKeywords.concat(
columns.map(column => ({
name: column.column_name,
value: column.column_name,
score: 50,
meta: 'column',
})),
);
const keywords = sqlKeywords.concat(getColumnKeywords(columns));

const columnValue =
(adhocMetric.column && adhocMetric.column.column_name) ||
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { getColumnKeywords } from './getColumnKeywords';

test('returns HTML for a column tooltip', () => {
const expected = {
column_name: 'test column1',
verbose_name: null,
is_certified: false,
certified_by: null,
description: 'test description',
type: 'VARCHAR',
};
expect(getColumnKeywords([expected])).toContainEqual({
name: expected.column_name,
value: expected.column_name,
docHTML: expect.stringContaining(expected.description),
score: 50,
meta: 'column',
});
});
49 changes: 49 additions & 0 deletions superset-frontend/src/explore/controlUtils/getColumnKeywords.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { ColumnMeta } from '@superset-ui/chart-controls';
import { t } from '@superset-ui/core';
import { getTooltipHTML } from 'src/components/AsyncAceEditor';
import { COLUMN_AUTOCOMPLETE_SCORE } from 'src/SqlLab/constants';

export function getColumnKeywords(columns: ColumnMeta[]) {
return columns.map(
({
column_name,
verbose_name,
is_certified,
certified_by,
description,
type,
}) => ({
name: verbose_name || column_name,
value: column_name,
docHTML: getTooltipHTML({
title: column_name,
meta: type ? `column: ${type}` : 'column',
body: `${description ?? ''}`,
footer: is_certified ? (
<>{t('Certified by %s', certified_by)}</>
) : undefined,
}),
score: COLUMN_AUTOCOMPLETE_SCORE,
meta: 'column',
}),
);
}

0 comments on commit 3c97145

Please sign in to comment.