From e855b63d2a4a60adb91fc81b0c462a8665b1c19b Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 23 Jul 2024 14:19:03 -0700 Subject: [PATCH 1/2] fix(explore): missing column autocomplete in custom SQL --- .../AsyncAceEditor/Tooltip.test.tsx | 47 ++++++++++++ .../src/components/AsyncAceEditor/Tooltip.tsx | 57 ++++++++++++++ .../src/components/AsyncAceEditor/index.tsx | 74 ++++++++++++++++--- .../ColumnSelectPopover.tsx | 7 ++ .../index.jsx | 15 +--- .../AdhocMetricEditPopover/index.jsx | 10 +-- .../controlUtils/getColumnKeywords.test.tsx | 38 ++++++++++ .../controlUtils/getColumnKeywords.tsx | 49 ++++++++++++ 8 files changed, 267 insertions(+), 30 deletions(-) create mode 100644 superset-frontend/src/components/AsyncAceEditor/Tooltip.test.tsx create mode 100644 superset-frontend/src/components/AsyncAceEditor/Tooltip.tsx create mode 100644 superset-frontend/src/explore/controlUtils/getColumnKeywords.test.tsx create mode 100644 superset-frontend/src/explore/controlUtils/getColumnKeywords.tsx diff --git a/superset-frontend/src/components/AsyncAceEditor/Tooltip.test.tsx b/superset-frontend/src/components/AsyncAceEditor/Tooltip.test.tsx new file mode 100644 index 0000000000000..8365bd6b59914 --- /dev/null +++ b/superset-frontend/src/components/AsyncAceEditor/Tooltip.test.tsx @@ -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:
icon
, + body:
body
, + meta: 'meta', + footer:
footer
, + }; + render(); + 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:
icon
, + body:
body
, + meta: 'meta', + footer:
footer
, + }); + expect(html).toContain('tooltip title'); +}); diff --git a/superset-frontend/src/components/AsyncAceEditor/Tooltip.tsx b/superset-frontend/src/components/AsyncAceEditor/Tooltip.tsx new file mode 100644 index 0000000000000..bc504587a635f --- /dev/null +++ b/superset-frontend/src/components/AsyncAceEditor/Tooltip.tsx @@ -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 = ({ + title, + icon, + body, + meta, + footer, +}) => ( +
+
+
+ {icon} + {title} +
+ {meta && ( + + {meta} + + )} +
+ {body &&
{body ?? title}
} + {footer &&
{footer}
} +
+); + +export const getTooltipHTML = (props: Props) => + `${renderToStaticMarkup()}`; + +export default Tooltip; diff --git a/superset-frontend/src/components/AsyncAceEditor/index.tsx b/superset-frontend/src/components/AsyncAceEditor/index.tsx index 32e5a687fdf2d..f599438c7daf7 100644 --- a/superset-frontend/src/components/AsyncAceEditor/index.tsx +++ b/superset-frontend/src/components/AsyncAceEditor/index.tsx @@ -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); @@ -135,6 +139,7 @@ export default function AsyncAceEditor( }, ref, ) { + const supersetTheme = useTheme(); const langTools = acequire('ace/ext/language_tools'); const setCompleters = useEffectEvent( (keywords: AceCompleterKeyword[]) => { @@ -167,15 +172,66 @@ export default function AsyncAceEditor( }, [keywords, setCompleters]); return ( - + <> + .ant-tag { + margin-right: 0px; + } + } + } + `} + /> + + ); }, ); diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx index ed274d0899d87..fd50f038a6f04 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx @@ -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, @@ -287,6 +289,10 @@ const ColumnSelectPopover = ({ const savedExpressionsLabel = t('Saved expressions'); const simpleColumnsLabel = t('Column'); + const keywords = useMemo( + () => sqlKeywords.concat(getColumnKeywords(columns)), + [columns], + ); return (
@@ -451,6 +457,7 @@ const ColumnSelectPopover = ({ className="filter-sql-editor" wrapEnabled ref={sqlEditorRef} + keywords={keywords} /> diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx index f24dff5aabc8a..dfbc19db29188 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx @@ -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'; @@ -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, diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx index af1ddc4489c9c..c527aa5825438 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/index.jsx @@ -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, @@ -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) || diff --git a/superset-frontend/src/explore/controlUtils/getColumnKeywords.test.tsx b/superset-frontend/src/explore/controlUtils/getColumnKeywords.test.tsx new file mode 100644 index 0000000000000..59782ba94dfae --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getColumnKeywords.test.tsx @@ -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', + }); +}); diff --git a/superset-frontend/src/explore/controlUtils/getColumnKeywords.tsx b/superset-frontend/src/explore/controlUtils/getColumnKeywords.tsx new file mode 100644 index 0000000000000..0ef134b1a2e86 --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getColumnKeywords.tsx @@ -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', + }), + ); +} From da1f147515bc6ea9a0d6b14d982cf38ec34d04e8 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 25 Jul 2024 13:59:53 -0700 Subject: [PATCH 2/2] fix missing actual --- .../controls/DndColumnSelectControl/DndFilterSelect.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx index 733b20e864936..80c60ca859d09 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx @@ -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) => (
{props.value}
),