datacube: bug fixes (#3554)

* datacube: fix problem with grid event handler

* datacube: use ag-grid auto-group column

* datacube: suppress data fetch on column rearrange

* datacube: minor fix for column name extraction in grid context menu

* datacube: apply grid styling to group column

* datacube: fix issue with coldefs affecting column visibility post hpivot

* datacube: partially make use of ag-grid pivot support

* datacube: fix sorting in grid controller

* fix lint

.changeset/blue-knives-allow.md

@@ -0,0 +1,3 @@
+---
+'@finos/legend-application-repl': patch
+---

packages/legend-application-repl/src/components/dataCube/editor/DataCubeEditorColumnPropertiesPanel.tsx

@@ -44,6 +44,7 @@ import {
 } from '../../../stores/dataCube/core/DataCubeQueryEngine.js';
 import { DocumentationKey } from '../../../application/LegendREPLDocumentation.js';
 import type { DataCubeState } from '../../../stores/dataCube/DataCubeState.js';
+import { _sortByColName } from '../../../stores/dataCube/core/DataCubeQuerySnapshot.js';
 
 export const DataCubeEditorColumnPropertiesPanel = observer(
  (props: { dataCube: DataCubeState }) => {
@@ -169,7 +170,7 @@ export const DataCubeEditorColumnPropertiesPanel = observer(
  <FormDropdownMenu className="w-80" {...columnsDropdownProps}>
  {panel.columns
  .slice()
- .sort((a, b) => a.name.localeCompare(b.name))
+ .sort(_sortByColName)
  .map((column) => (
  <FormDropdownMenuItem
  key={column.name}

packages/legend-application-repl/src/stores/dataCube/core/DataCubeQueryEngine.ts

@@ -304,7 +304,7 @@ export const DEFAULT_ROW_BUFFER = 20;
 export const DEFAULT_URL_LABEL_QUERY_PARAM = 'dataCube.linkLabel';
 export const DEFAULT_MISSING_VALUE_DISPLAY_TEXT = '';
 
-export const DEFAULT_GRID_LINE_COLOR = TailwindCSSPalette.neutral[200];
+export const DEFAULT_GRID_LINE_COLOR = TailwindCSSPalette.neutral[300];
 export const DEFAULT_ROW_HIGHLIGHT_BACKGROUND_COLOR = '#d7e0eb';
 
 export const DEFAULT_COLUMN_WIDTH = 300;

packages/legend-application-repl/src/stores/dataCube/core/DataCubeQuerySnapshot.ts

@@ -186,3 +186,6 @@ export function _toCol(col: {
 }): DataCubeQuerySnapshotColumn {
  return { name: col.name, type: col.type };
 }
+
+export const _sortByColName = (a: { name: string }, b: { name: string }) =>
+ a.name.localeCompare(b.name);

packages/legend-application-repl/src/stores/dataCube/core/DataCubeQuerySnapshotBuilder.ts

@@ -298,7 +298,7 @@ export function validateAndBuildQuerySnapshot(
 
  // --------------------------------- SOURCE ---------------------------------
 
- data.sourceColumns = baseQuery.source.columns.map((col) => _toCol(col));
+ data.sourceColumns = baseQuery.source.columns.map(_toCol);
  data.sourceColumns.map((col) => colsMap.set(col.name, col));
 
  // --------------------------------- LEAF-LEVEL EXTEND ---------------------------------
@@ -309,11 +309,6 @@ export function validateAndBuildQuerySnapshot(
  if (funcMap.filter) {
  /** TODO: @datacube roundtrip */
  data.filter = undefined;
- // data.selectColumns = _colSpecArrayParam(funcMap.select, 0).colSpecs.map(
- // (colSpec) => ({
- // _col(colSpec),
- // }),
- // );
  }
 
  // --------------------------------- SELECT ---------------------------------

packages/legend-application-repl/src/stores/dataCube/core/DataCubeQuerySnapshotManager.ts

@@ -67,6 +67,8 @@ export abstract class DataCubeQuerySnapshotController
  `New Snapshot`,
  '\nSnapshot',
  snapshot,
+ '\nPrevious Snapshot',
+ this.latestSnapshot ?? {},
  '\nDiff',
  deepDiff(snapshot, this.latestSnapshot ?? {}),
  );

packages/legend-application-repl/src/stores/dataCube/editor/DataCubeEditorColumnsPanelState.ts

@@ -193,6 +193,6 @@ export class DataCubeEditorColumnsPanelState
  (column) => column.name === col.name,
  ),
  )
- .map((col) => _toCol(col));
+ .map(_toCol);
  }
 }

packages/legend-application-repl/src/stores/dataCube/editor/DataCubeEditorColumnsSelectorState.ts

@@ -18,6 +18,7 @@ import { makeObservable, observable, action, computed } from 'mobx';
 import type { DataCubeState } from '../DataCubeState.js';
 import type { DataCubeEditorState } from './DataCubeEditorState.js';
 import { guaranteeNonNullable } from '@finos/legend-shared';
+import { _sortByColName } from '../core/DataCubeQuerySnapshot.js';
 
 export class DataCubeEditorColumnsSelectorColumnState {
  readonly name: string;
@@ -80,7 +81,7 @@ export abstract class DataCubeEditorColumnsSelectorState<
  (column) =>
  !this.selectedColumns.find((col) => column.name === col.name),
  )
- .sort((a, b) => a.name.localeCompare(b.name));
+ .sort(_sortByColName);
  }
 
  get selectedColumnsForDisplay(): T[] {

packages/legend-application-repl/src/stores/dataCube/editor/DataCubeEditorHorizontalPivotsPanelState.ts

@@ -73,10 +73,8 @@ export class DataCubeEditorHorizontalPivotsPanelState
  constructor(editor: DataCubeEditorState) {
  makeObservable(this, {
  castColumns: observable.ref,
- setCastColumns: action,
-
  pivotResultColumns: computed,
- columnsConsumedByPivot: computed,
+ setCastColumns: action,
 
  applySnaphot: action,
  });
@@ -91,30 +89,7 @@ export class DataCubeEditorHorizontalPivotsPanelState
  get pivotResultColumns(): DataCubeQuerySnapshotColumn[] {
  return this.castColumns
  .filter((col) => isPivotResultColumnName(col.name))
- .map((col) => _toCol(col));
- }
-
- /**
- * Due to the nature of pivot() operation, some base columns (i.e. source columns and leaf-level columns)
- * will be "consumed" and not available for subsequent operations (e.g. sort, groupBy, etc.).
- */
- get columnsConsumedByPivot(): DataCubeQuerySnapshotColumn[] {
- if (!this.selector.selectedColumns.length) {
- return [];
- }
- return uniqBy(
- [
- ...this.selector.selectedColumns,
- ...this.editor.columnProperties.columns.filter(
- (col) =>
- col.isSelected &&
- col.kind === DataCubeColumnKind.MEASURE &&
- !col.excludedFromHorizontalPivot,
- ),
- /** TODO: @datacube pivot - need to include columns used in complex aggregates (such as weighted-average) */
- ],
- (col) => col.name,
- ).map((col) => _toCol(col));
+ .map(_toCol);
  }
 
  setCastColumns(value: DataCubeQuerySnapshotColumn[]) {
@@ -145,13 +120,13 @@ export class DataCubeEditorHorizontalPivotsPanelState
  ) {
  newSnapshot.data.pivot = this.selector.selectedColumns.length
  ? {
- columns: this.selector.selectedColumns.map((col) => _toCol(col)),
- castColumns: this.castColumns.map((col) => _toCol(col)),
+ columns: this.selector.selectedColumns.map(_toCol),
+ castColumns: this.castColumns.map(_toCol),
  }
  : undefined;
  newSnapshot.data.selectColumns = uniqBy(
  [...newSnapshot.data.selectColumns, ...this.selector.selectedColumns],
  (col) => col.name,
- ).map((col) => _toCol(col));
+ ).map(_toCol);
  }
 }

packages/legend-application-repl/src/stores/dataCube/editor/DataCubeEditorSortsPanelState.ts

@@ -17,7 +17,10 @@
 import { action, makeObservable, observable } from 'mobx';
 import type { DataCubeState } from '../DataCubeState.js';
 import { type DataCubeQuerySnapshot } from '../core/DataCubeQuerySnapshot.js';
-import { DataCubeQuerySortOperator } from '../core/DataCubeQueryEngine.js';
+import {
+ DataCubeColumnKind,
+ DataCubeQuerySortOperator,
+} from '../core/DataCubeQueryEngine.js';
 import type { DataCubeQueryEditorPanelState } from './DataCubeEditorPanelState.js';
 import {
  DataCubeEditorColumnsSelectorColumnState,
@@ -58,17 +61,28 @@ export class DataCubeEditorSortColumnsSelectorState extends DataCubeEditorColumn
  override get availableColumns() {
  return uniqBy(
  [
- ...this.editor.horizontalPivots.pivotResultColumns,
- ...[
- ...this.editor.columns.selector.selectedColumns,
- ...this.editor.horizontalPivots.selector.selectedColumns,
- ...this.editor.verticalPivots.selector.selectedColumns,
- ].filter(
- (column) =>
- !this.editor.horizontalPivots.columnsConsumedByPivot.find(
- (col) => col.name === column.name,
- ),
- ),
+ // if pivot is active, take the pivot result columns and include
+ // selected dimension columns which are not part of pivot columns
+ ...(this.editor.horizontalPivots.selector.selectedColumns.length
+ ? [
+ ...this.editor.horizontalPivots.pivotResultColumns,
+ ...[
+ ...this.editor.columns.selector.selectedColumns,
+ ...this.editor.verticalPivots.selector.selectedColumns,
+ ].filter(
+ (column) =>
+ this.editor.columnProperties.getColumnConfiguration(
+ column.name,
+ ).kind === DataCubeColumnKind.DIMENSION &&
+ !this.editor.horizontalPivots.selector.selectedColumns.find(
+ (col) => col.name === column.name,
+ ),
+ ),
+ ]
+ : [
+ ...this.editor.columns.selector.selectedColumns,
+ ...this.editor.verticalPivots.selector.selectedColumns,
+ ]),
  ...this.editor.groupExtendColumns,
  ],
  (col) => col.name,

packages/legend-application-repl/src/stores/dataCube/editor/DataCubeEditorState.tsx

@@ -170,7 +170,7 @@ export class DataCubeEditorState extends DataCubeQuerySnapshotController {
  (column) => column.name === col.name,
  ),
  )
- .map((col) => _toCol(col));
+ .map(_toCol);
  }
  await this.dataCube.engine.getQueryRelationType(
  _lambda(

packages/legend-application-repl/src/stores/dataCube/editor/DataCubeEditorVerticalPivotsPanelState.ts

@@ -99,12 +99,12 @@ export class DataCubeEditorVerticalPivotsPanelState
  ) {
  newSnapshot.data.groupBy = this.selector.selectedColumns.length
  ? {
- columns: this.selector.selectedColumns.map((col) => _toCol(col)),
+ columns: this.selector.selectedColumns.map(_toCol),
  }
  : undefined;
  newSnapshot.data.selectColumns = uniqBy(
  [...newSnapshot.data.selectColumns, ...this.selector.selectedColumns],
  (col) => col.name,
- ).map((col) => _toCol(col));
+ ).map(_toCol);
  }
 }

packages/legend-application-repl/src/stores/dataCube/extend/DataCubeExtendManagerState.tsx

@@ -138,9 +138,7 @@ export class DataCubeExtendManagerState extends DataCubeQuerySnapshotController
  this.groupExtendedColumns = snapshot.data.groupExtendedColumns.map(
  (col) => new DataCubeQueryExtendedColumnState(col),
  );
- this.selectedColumns = snapshot.data.selectColumns.map((col) =>
- _toCol(col),
- );
+ this.selectedColumns = snapshot.data.selectColumns.map(_toCol);
 
  // trigger re-compile in each existing column editor as the base query has changed
  this.newColumnEditors.forEach((editor) => {

packages/legend-application-repl/src/stores/dataCube/grid/DataCubeGridClientEngine.ts

@@ -38,7 +38,6 @@ import {
  V1_Lambda,
 } from '@finos/legend-graph';
 import { APPLICATION_EVENT } from '@finos/legend-application';
-import { buildQuerySnapshot } from './DataCubeGridQuerySnapshotBuilder.js';
 import { generateRowGroupingDrilldownExecutableQueryPostProcessor } from './DataCubeGridQueryBuilder.js';
 import { makeObservable, observable, runInAction } from 'mobx';
 import type {
@@ -47,12 +46,17 @@ import type {
 } from '../core/DataCubeConfiguration.js';
 import { AlertType } from '../../../components/repl/Alert.js';
 import { DEFAULT_LARGE_ALERT_WINDOW_CONFIG } from '../../LayoutManagerState.js';
-import type { DataCubeQuerySnapshot } from '../core/DataCubeQuerySnapshot.js';
+import {
+ _sortByColName,
+ type DataCubeQuerySnapshot,
+ type DataCubeQuerySnapshotData,
+} from '../core/DataCubeQuerySnapshot.js';
 import type { DataCubeEngine } from '../DataCubeEngine.js';
 import { generateColumnDefs } from './DataCubeGridConfigurationBuilder.js';
-import type { DataCubeQueryFunctionMap } from '../core/DataCubeQueryEngine.js';
+import { type DataCubeQueryFunctionMap } from '../core/DataCubeQueryEngine.js';
 import type { DataCubeQueryFilterOperation } from '../core/filter/DataCubeQueryFilterOperation.js';
 import type { DataCubeQueryAggregateOperation } from '../core/aggregation/DataCubeQueryAggregateOperation.js';
+import { buildQuerySnapshot } from './DataCubeGridQuerySnapshotBuilder.js';
 
 type GridClientCellValue = string | number | boolean | null | undefined;
 type GridClientRowData = {
@@ -127,10 +131,10 @@ export const INTERNAL__GRID_CLIENT_SIDE_BAR_WIDTH = 200;
 export const INTERNAL__GRID_CLIENT_COLUMN_MIN_WIDTH = 50;
 export const INTERNAL__GRID_CLIENT_HEADER_HEIGHT = 24;
 export const INTERNAL__GRID_CLIENT_ROW_HEIGHT = 20;
-export const INTERNAL__GRID_CLIENT_TOOLTIP_SHOW_DELAY = 1000;
+export const INTERNAL__GRID_CLIENT_TOOLTIP_SHOW_DELAY = 1500;
 export const INTERNAL__GRID_CLIENT_AUTO_RESIZE_PADDING = 10;
 export const INTERNAL__GRID_CLIENT_MISSING_VALUE = '__MISSING';
-export const INTERNAL__GRID_CLIENT_TREE_COLUMN_ID = 'INTERNAL__tree';
+export const INTERNAL__GRID_CLIENT_TREE_COLUMN_ID = 'ag-Grid-AutoColumn';
 export const INTERNAL__GRID_CLIENT_DATA_FETCH_MANUAL_TRIGGER_COLUMN_ID =
  'INTERNAL__dataFetchManualTrigger';
 export const INTERNAL__GRID_CLIENT_ROW_GROUPING_COUNT_AGG_COLUMN_ID =
@@ -181,23 +185,29 @@ export function computeHashCodeForDataFetchManualTrigger(
  return hashObject(
  pruneObject({
  ...snapshot.data,
- name: undefined,
+ name: '', // name change should not trigger data fetching
  configuration: {
  initialExpandLevel: configuration.initialExpandLevel,
  showRootAggregation: configuration.showRootAggregation,
- showLeafCount: configuration.showLeafCount,
  pivotTotalColumnPlacement: configuration.pivotTotalColumnPlacement,
  treeGroupSortFunction: configuration.treeGroupSortFunction,
- columns: configuration.columns.map((column) => ({
- name: column.name,
- type: column.type,
- aggregateOperator: column.aggregateOperator,
- aggregationParameters: column.aggregationParameters,
- excludedFromHorizontalPivot: column.excludedFromHorizontalPivot,
- horizontalPivotSortFunction: column.horizontalPivotSortFunction,
- })),
+ columns: configuration.columns
+ .map((column) => ({
+ name: column.name,
+ type: column.type,
+ kind: column.kind,
+ aggregateOperator: column.aggregateOperator,
+ aggregationParameters: column.aggregationParameters,
+ excludedFromHorizontalPivot: column.excludedFromHorizontalPivot,
+ horizontalPivotSortFunction: column.horizontalPivotSortFunction,
+ }))
+ .sort(_sortByColName), // sort to make sure column reordering does not trigger data fetching
  },
- }),
+ selectColumns: snapshot.data.selectColumns.slice().sort(_sortByColName), // sort to make sure column reordering does not trigger data fetching
+ pivot: undefined,
+ groupBy: undefined,
+ sortColumns: [],
+ } satisfies DataCubeQuerySnapshotData),
  );
 }