fix(explore): Adhoc columns don't display correctly (#20802)

* fix(explore): Adhoc columns have empty labels * Add unit test * Address comments
apache · Jul 21, 2022 · af1bddf · af1bddf
1 parent 922b4b8
commit af1bddf
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 5 deletions.
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
@@ -68,7 +68,7 @@ export const getColumnTooltipNode = (
   labelRef?: React.RefObject<any>,
 ): ReactNode => {
   if (
-    !column.verbose_name &&
+    (!column.column_name || !column.verbose_name) &&
     !column.description &&
     !isLabelTruncated(labelRef)
   ) {
@@ -77,7 +77,9 @@ export const getColumnTooltipNode = (
 
   return (
     <>
-      <TooltipSection label={t('Column name')} text={column.column_name} />
+      {column.column_name && (
+        <TooltipSection label={t('Column name')} text={column.column_name} />
+      )}
       {column.verbose_name && (
         <TooltipSection label={t('Label')} text={column.verbose_name} />
       )}

diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Column.ts
@@ -61,7 +61,7 @@ export function isAdhocColumn(column?: any): column is AdhocColumn {
     typeof column !== 'string' &&
     column?.sqlExpression !== undefined &&
     column?.label !== undefined &&
-    column?.expressionType === 'SQL'
+    (column?.expressionType === undefined || column?.expressionType === 'SQL')
   );
 }
 

diff --git a/...t-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx b/...t-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
@@ -121,7 +121,7 @@ const ColumnSelectPopover = ({
 
   const onSqlExpressionChange = useCallback(
     sqlExpression => {
-      setAdhocColumn({ label, sqlExpression } as AdhocColumn);
+      setAdhocColumn({ label, sqlExpression, expressionType: 'SQL' });
       setSelectedSimpleColumn(undefined);
       setSelectedCalculatedColumn(undefined);
     },

diff --git a/...-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx b/...-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx
@@ -27,7 +27,9 @@ const defaultProps: DndColumnSelectProps = {
   type: 'DndColumnSelect',
   name: 'Filter',
   onChange: jest.fn(),
-  options: { string: { column_name: 'Column A' } },
+  options: {
+    string: { column_name: 'Column A' },
+  },
   actions: { setControlValue: jest.fn() },
 };
 
@@ -42,3 +44,19 @@ test('renders with value', () => {
   });
   expect(screen.getByText('Column A')).toBeInTheDocument();
 });
+
+test('renders adhoc column', () => {
+  render(
+    <DndColumnSelect
+      {...defaultProps}
+      value={{
+        sqlExpression: 'Count *',
+        label: 'adhoc column',
+        expressionType: 'SQL',
+      }}
+    />,
+    { useDnd: true },
+  );
+  expect(screen.getByText('adhoc column')).toBeVisible();
+  expect(screen.getByLabelText('calculator')).toBeVisible();
+});