From d1bc369f0644562433f5d41c9a0199a0186653e4 Mon Sep 17 00:00:00 2001
From: Vadim Ogievetsky <vadim@ogievetsky.com>
Date: Thu, 3 Oct 2024 19:52:17 -0700
Subject: [PATCH] Web console: Final explore QA pass (#17240)

* cleanup

* remove redundancy

* aggregate works for multiple queries
---
 .../components/filter-pane/filter-pane.scss     |  2 ++
 .../components/module-pane/module-pane.scss     | 16 ----------------
 .../components/module-picker/module-picker.tsx  | 17 ++++++++++-------
 .../src/views/explore-view/explore-view.tsx     |  8 --------
 .../module-repository/module-repository.ts      |  6 ++++--
 .../explore-view/modules/bar-chart-module.tsx   |  2 ++
 .../modules/grouping-table-module.tsx           |  2 ++
 .../src/views/explore-view/modules/index.ts     |  2 +-
 .../modules/multi-axis-chart-module.tsx         |  2 ++
 .../explore-view/modules/pie-chart-module.tsx   |  2 ++
 .../modules/record-table-module.tsx             |  2 ++
 .../explore-view/modules/time-chart-module.tsx  |  2 ++
 .../explore-view/query-macros/aggregate.ts      | 16 ++++++++--------
 13 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/web-console/src/views/explore-view/components/filter-pane/filter-pane.scss b/web-console/src/views/explore-view/components/filter-pane/filter-pane.scss
index 9be736495f00..56dd2cff5c71 100644
--- a/web-console/src/views/explore-view/components/filter-pane/filter-pane.scss
+++ b/web-console/src/views/explore-view/components/filter-pane/filter-pane.scss
@@ -22,6 +22,8 @@
   display: flex;
   flex-wrap: wrap;
   gap: 5px;
+  align-items: flex-start;
+  align-content: flex-start;
 
   .filter-icon-button {
     pointer-events: none;
diff --git a/web-console/src/views/explore-view/components/module-pane/module-pane.scss b/web-console/src/views/explore-view/components/module-pane/module-pane.scss
index 9d180cb8da2b..6955ab1b5865 100644
--- a/web-console/src/views/explore-view/components/module-pane/module-pane.scss
+++ b/web-console/src/views/explore-view/components/module-pane/module-pane.scss
@@ -47,20 +47,4 @@
       left: 0;
     }
   }
-
-  .tile-content {
-    width: 100%;
-    height: 100%;
-
-    &.issue {
-      display: flex;
-      justify-content: center;
-      align-items: center;
-    }
-
-    & > * {
-      width: 100%;
-      height: 100%;
-    }
-  }
 }
diff --git a/web-console/src/views/explore-view/components/module-picker/module-picker.tsx b/web-console/src/views/explore-view/components/module-picker/module-picker.tsx
index d4561e1431fa..6ca05de9647e 100644
--- a/web-console/src/views/explore-view/components/module-picker/module-picker.tsx
+++ b/web-console/src/views/explore-view/components/module-picker/module-picker.tsx
@@ -17,24 +17,27 @@
  */
 
 import { Button, ButtonGroup, Menu, MenuItem, Popover, Position } from '@blueprintjs/core';
-import type { IconName } from '@blueprintjs/icons';
 import { IconNames } from '@blueprintjs/icons';
 import type { JSX } from 'react';
 import React from 'react';
 
+import { ModuleRepository } from '../../module-repository/module-repository';
+
 import './module-picker.scss';
 
 export interface ModulePickerProps {
-  modules: readonly { id: string; icon: IconName; label: string }[];
   selectedModuleId: string | undefined;
   onSelectedModuleIdChange(newSelectedModuleId: string): void;
   moreMenu?: JSX.Element;
 }
 
 export const ModulePicker = React.memo(function ModulePicker(props: ModulePickerProps) {
-  const { modules, selectedModuleId, onSelectedModuleIdChange, moreMenu } = props;
+  const { selectedModuleId, onSelectedModuleIdChange, moreMenu } = props;
 
-  const selectedTileManifest = modules.find(module => module.id === selectedModuleId);
+  const modules = ModuleRepository.getAllModuleEntries();
+  const selectedModule = selectedModuleId
+    ? ModuleRepository.getModule(selectedModuleId)
+    : undefined;
   return (
     <ButtonGroup className="module-picker" fill>
       <Popover
@@ -48,7 +51,7 @@ export const ModulePicker = React.memo(function ModulePicker(props: ModulePicker
               <MenuItem
                 key={i}
                 icon={module.icon}
-                text={module.label}
+                text={module.title}
                 onClick={() => onSelectedModuleIdChange(module.id)}
               />
             ))}
@@ -56,8 +59,8 @@ export const ModulePicker = React.memo(function ModulePicker(props: ModulePicker
         }
       >
         <Button
-          icon={selectedTileManifest ? selectedTileManifest.icon : IconNames.BOX}
-          text={selectedTileManifest ? selectedTileManifest.label : 'Select module'}
+          icon={selectedModule ? selectedModule.icon : IconNames.BOX}
+          text={selectedModule ? selectedModule.title : 'Select module'}
           fill
           minimal
           rightIcon={IconNames.CARET_DOWN}
diff --git a/web-console/src/views/explore-view/explore-view.tsx b/web-console/src/views/explore-view/explore-view.tsx
index a8022fd50169..05f6ee693257 100644
--- a/web-console/src/views/explore-view/explore-view.tsx
+++ b/web-console/src/views/explore-view/explore-view.tsx
@@ -323,14 +323,6 @@ export const ExploreView = React.memo(function ExploreView() {
             }}
           />
           <ModulePicker
-            modules={[
-              { id: 'grouping-table', icon: IconNames.PANEL_TABLE, label: 'Grouping table' },
-              { id: 'record-table', icon: IconNames.TH, label: 'Record table' },
-              { id: 'time-chart', icon: IconNames.TIMELINE_LINE_CHART, label: 'Time chart' },
-              { id: 'bar-chart', icon: IconNames.VERTICAL_BAR_CHART_DESC, label: 'Bar chart' },
-              { id: 'pie-chart', icon: IconNames.PIE_CHART, label: 'Pie chart' },
-              { id: 'multi-axis-chart', icon: IconNames.SERIES_ADD, label: 'Multi-axis chart' },
-            ]}
             selectedModuleId={moduleId}
             onSelectedModuleIdChange={newModuleId => {
               const newParameterValues = getStickyParameterValuesForModule(newModuleId);
diff --git a/web-console/src/views/explore-view/module-repository/module-repository.ts b/web-console/src/views/explore-view/module-repository/module-repository.ts
index 0d90a8b69336..d050a2779c51 100644
--- a/web-console/src/views/explore-view/module-repository/module-repository.ts
+++ b/web-console/src/views/explore-view/module-repository/module-repository.ts
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+import type { IconName } from '@blueprintjs/icons';
 import type { QueryResult, SqlExpression, SqlQuery } from '@druid-toolkit/query';
 import type { CancelToken } from 'axios';
 
@@ -23,6 +24,7 @@ import type { ParameterDefinition, QuerySource, Stage } from '../models';
 
 interface ModuleDefinition<P> {
   id: string;
+  icon: IconName;
   title: string;
   parameters: Record<keyof P, ParameterDefinition>;
   component: (props: ModuleComponentProps<P>) => any;
@@ -52,7 +54,7 @@ export class ModuleRepository {
     return ModuleRepository.repo.get(id);
   }
 
-  static getAllModuleIds(): string[] {
-    return [...ModuleRepository.repo.keys()];
+  static getAllModuleEntries(): ModuleDefinition<any>[] {
+    return [...ModuleRepository.repo.values()];
   }
 }
diff --git a/web-console/src/views/explore-view/modules/bar-chart-module.tsx b/web-console/src/views/explore-view/modules/bar-chart-module.tsx
index 8d597db47dca..21003e43108f 100644
--- a/web-console/src/views/explore-view/modules/bar-chart-module.tsx
+++ b/web-console/src/views/explore-view/modules/bar-chart-module.tsx
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+import { IconNames } from '@blueprintjs/icons';
 import { L } from '@druid-toolkit/query';
 import type { ECharts } from 'echarts';
 import * as echarts from 'echarts';
@@ -43,6 +44,7 @@ interface BarChartParameterValues {
 ModuleRepository.registerModule<BarChartParameterValues>({
   id: 'bar-chart',
   title: 'Bar chart',
+  icon: IconNames.VERTICAL_BAR_CHART_DESC,
   parameters: {
     splitColumn: {
       type: 'expression',
diff --git a/web-console/src/views/explore-view/modules/grouping-table-module.tsx b/web-console/src/views/explore-view/modules/grouping-table-module.tsx
index 06c5408702d0..ab19fbcf0cee 100644
--- a/web-console/src/views/explore-view/modules/grouping-table-module.tsx
+++ b/web-console/src/views/explore-view/modules/grouping-table-module.tsx
@@ -17,6 +17,7 @@
  */
 
 import { Button } from '@blueprintjs/core';
+import { IconNames } from '@blueprintjs/icons';
 import type { SqlExpression, SqlOrderByDirection, SqlQuery } from '@druid-toolkit/query';
 import { C, F } from '@druid-toolkit/query';
 import React, { useMemo } from 'react';
@@ -70,6 +71,7 @@ interface GroupingTableParameterValues {
 ModuleRepository.registerModule<GroupingTableParameterValues>({
   id: 'grouping-table',
   title: 'Grouping table',
+  icon: IconNames.PANEL_TABLE,
   parameters: {
     splitColumns: {
       type: 'expressions',
diff --git a/web-console/src/views/explore-view/modules/index.ts b/web-console/src/views/explore-view/modules/index.ts
index 14f1d2e86e69..3d8a063b104e 100644
--- a/web-console/src/views/explore-view/modules/index.ts
+++ b/web-console/src/views/explore-view/modules/index.ts
@@ -18,7 +18,7 @@
 
 import './grouping-table-module';
 import './record-table-module';
+import './time-chart-module';
 import './bar-chart-module';
 import './pie-chart-module';
-import './time-chart-module';
 import './multi-axis-chart-module';
diff --git a/web-console/src/views/explore-view/modules/multi-axis-chart-module.tsx b/web-console/src/views/explore-view/modules/multi-axis-chart-module.tsx
index 4aed56d09470..ac0d47f1b18e 100644
--- a/web-console/src/views/explore-view/modules/multi-axis-chart-module.tsx
+++ b/web-console/src/views/explore-view/modules/multi-axis-chart-module.tsx
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+import { IconNames } from '@blueprintjs/icons';
 import type { SqlQuery } from '@druid-toolkit/query';
 import { C, F, L } from '@druid-toolkit/query';
 import type { ECharts } from 'echarts';
@@ -46,6 +47,7 @@ interface MultiAxisChartParameterValues {
 ModuleRepository.registerModule<MultiAxisChartParameterValues>({
   id: 'multi-axis-chart',
   title: 'Multi-axis chart',
+  icon: IconNames.SERIES_ADD,
   parameters: {
     timeGranularity: {
       type: 'option',
diff --git a/web-console/src/views/explore-view/modules/pie-chart-module.tsx b/web-console/src/views/explore-view/modules/pie-chart-module.tsx
index e63c95e4a12d..69d49ff9b5e1 100644
--- a/web-console/src/views/explore-view/modules/pie-chart-module.tsx
+++ b/web-console/src/views/explore-view/modules/pie-chart-module.tsx
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+import { IconNames } from '@blueprintjs/icons';
 import { C, L } from '@druid-toolkit/query';
 import type { ECharts } from 'echarts';
 import * as echarts from 'echarts';
@@ -63,6 +64,7 @@ interface PieChartParameterValues {
 ModuleRepository.registerModule<PieChartParameterValues>({
   id: 'pie-chart',
   title: 'Pie chart',
+  icon: IconNames.PIE_CHART,
   parameters: {
     splitColumn: {
       type: 'expression',
diff --git a/web-console/src/views/explore-view/modules/record-table-module.tsx b/web-console/src/views/explore-view/modules/record-table-module.tsx
index 43e84e60de95..7d2d86ae9baf 100644
--- a/web-console/src/views/explore-view/modules/record-table-module.tsx
+++ b/web-console/src/views/explore-view/modules/record-table-module.tsx
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+import { IconNames } from '@blueprintjs/icons';
 import { C, SqlQuery } from '@druid-toolkit/query';
 import React, { useMemo } from 'react';
 
@@ -38,6 +39,7 @@ interface RecordTableParameterValues {
 ModuleRepository.registerModule<RecordTableParameterValues>({
   id: 'record-table',
   title: 'Record table',
+  icon: IconNames.TH,
   parameters: {
     maxRows: {
       type: 'number',
diff --git a/web-console/src/views/explore-view/modules/time-chart-module.tsx b/web-console/src/views/explore-view/modules/time-chart-module.tsx
index c074d89222cd..6cc26e681bf4 100644
--- a/web-console/src/views/explore-view/modules/time-chart-module.tsx
+++ b/web-console/src/views/explore-view/modules/time-chart-module.tsx
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+import { IconNames } from '@blueprintjs/icons';
 import { C, F, L, SqlCase } from '@druid-toolkit/query';
 import type { ECharts } from 'echarts';
 import * as echarts from 'echarts';
@@ -73,6 +74,7 @@ interface TimeChartParameterValues {
 ModuleRepository.registerModule<TimeChartParameterValues>({
   id: 'time-chart',
   title: 'Time chart',
+  icon: IconNames.TIMELINE_LINE_CHART,
   parameters: {
     timeGranularity: {
       type: 'option',
diff --git a/web-console/src/views/explore-view/query-macros/aggregate.ts b/web-console/src/views/explore-view/query-macros/aggregate.ts
index fd9418416e80..475810519ba3 100644
--- a/web-console/src/views/explore-view/query-macros/aggregate.ts
+++ b/web-console/src/views/explore-view/query-macros/aggregate.ts
@@ -24,7 +24,7 @@ import { KNOWN_AGGREGATIONS } from '../utils';
 
 export function rewriteAggregate(query: SqlQuery, measures: Measure[]): SqlQuery {
   const usedMeasures: Map<string, boolean> = new Map();
-  let queryToRewrite: SqlQuery | undefined;
+  const queriesToRewrite: SqlQuery[] = [];
   const newQuery = query.walk(ex => {
     if (ex instanceof SqlFunction && ex.getEffectiveFunctionName() === Measure.AGGREGATE) {
       if (ex.numArgs() !== 1)
@@ -51,27 +51,27 @@ export function rewriteAggregate(query: SqlQuery, measures: Measure[]): SqlQuery
     if (ex instanceof SqlQuery) {
       const queryMeasures = Measure.extractQueryMeasures(ex);
       if (queryMeasures.length) {
-        queryToRewrite = ex;
+        queriesToRewrite.push(ex);
       }
     }
 
     return ex;
   }) as SqlQuery;
 
-  if (!queryToRewrite) return newQuery;
+  if (!queriesToRewrite.length) return newQuery;
 
-  return newQuery.walk(ex => {
-    if (ex === queryToRewrite) {
-      return queryToRewrite.applyForEach(
+  return newQuery.walk(subQuery => {
+    if (subQuery instanceof SqlQuery && queriesToRewrite.includes(subQuery)) {
+      return subQuery.applyForEach(
         uniq(
           filterMap(measures, queryMeasure =>
             usedMeasures.get(queryMeasure.name) ? queryMeasure.expression : undefined,
           ).flatMap(ex => ex.getUsedColumnNames()),
-        ).filter(columnName => queryToRewrite!.getSelectIndexForOutputColumn(columnName) === -1),
+        ).filter(columnName => subQuery.getSelectIndexForOutputColumn(columnName) === -1),
         (q, columnName) => q.addSelect(C(columnName)),
       );
     }
 
-    return ex;
+    return subQuery;
   }) as SqlQuery;
 }