fix merge

Author: dgodinez-dh

packages/iris-grid/src/IrisGrid.tsx

@@ -3397,15 +3397,32 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
   /**
    * User added, removed, or changed the order of aggregations, or position
    * @param aggregationSettings The new aggregation settings
+   * @param added The aggregations that were added
+   * @param removed The aggregations that were removed
    */
-  handleAggregationsChange(aggregationSettings: AggregationSettings): void {
-    log.debug('handleAggregationsChange', aggregationSettings);
-
-    this.startLoading(
-      `Aggregating ${aggregationSettings.aggregations
-        .map(a => a.operation)
-        .join(', ')}...`
+  handleAggregationsChange(
+    aggregationSettings: AggregationSettings,
+    added: AggregationOperation[] = [],
+    removed: AggregationOperation[] = []
+  ): void {
+    log.debug('handleAggregationsChange', aggregationSettings, added, removed);
+    const { rollupConfig } = this.state;
+    const isRollup = (rollupConfig?.columns?.length ?? 0) > 0;
+    // Do not start loading if this is rollup and added / removed aggregations are prohibited for rollups
+    const changes = [...added, ...removed];
+    const shouldStartLoading = !(
+      isRollup &&
+      changes.length > 0 &&
+      changes.every(op => AggregationUtils.isRollupProhibited(op))
     );
+
+    if (shouldStartLoading) {
+      this.startLoading(
+        `Aggregating ${aggregationSettings.aggregations
+          .map(a => a.operation)
+          .join(', ')}...`
+      );
+    }
     this.setState({ aggregationSettings });
   }
 
@@ -3415,8 +3432,16 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
    */
   handleAggregationChange(aggregation: Aggregation): void {
     log.debug('handleAggregationChange', aggregation);
+    const { rollupConfig } = this.state;
+    const isRollup = (rollupConfig?.columns?.length ?? 0) > 0;
+    // Do not start loading if this is rollup and the aggregation is prohibited for rollups
+    const shouldStartLoading = !(
+      isRollup && AggregationUtils.isRollupProhibited(aggregation.operation)
+    );
 
-    this.startLoading(`Aggregating ${aggregation.operation}...`);
+    if (shouldStartLoading) {
+      this.startLoading(`Aggregating ${aggregation.operation}...`);
+    }
 
     this.setState(({ aggregationSettings }) => ({
       selectedAggregation: aggregation,
@@ -4656,6 +4681,7 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
               isRollup={isRollup}
               onChange={this.handleAggregationsChange}
               onEdit={this.handleAggregationEdit}
+              dh={model.dh}
             />
           );
         case OptionType.AGGREGATION_EDIT:

packages/iris-grid/src/IrisGridUtils.ts

@@ -822,7 +822,10 @@ class IrisGridUtils {
       includeDescriptions = true,
     } = config ?? {};
     const { aggregations = [] } = aggregationSettings ?? {};
-    const aggregationColumns = aggregations.map(
+    const filteredAggregations = aggregations.filter(
+      ({ operation }) => !AggregationUtils.isRollupProhibited(operation)
+    );
+    const aggregationColumns = filteredAggregations.map(
       ({ operation, selected, invert }) =>
         AggregationUtils.isRollupOperation(operation)
           ? []
@@ -836,8 +839,8 @@ class IrisGridUtils {
 
     const aggregationMap = {} as Record<AggregationOperation, string[]>;
     // Aggregation columns should show first, add them first
-    for (let i = 0; i < aggregations.length; i += 1) {
-      aggregationMap[aggregations[i].operation] = aggregationColumns[i];
+    for (let i = 0; i < filteredAggregations.length; i += 1) {
+      aggregationMap[filteredAggregations[i].operation] = aggregationColumns[i];
     }
 
     if (showNonAggregatedColumns) {

packages/iris-grid/src/sidebar/aggregations/AggregationOperation.ts

@@ -6,6 +6,7 @@ enum AggregationOperation {
   ABS_SUM = 'AbsSum',
   VAR = 'Var',
   AVG = 'Avg',
+  MEDIAN = 'Median',
   STD = 'Std',
   FIRST = 'First',
   LAST = 'Last',

packages/iris-grid/src/sidebar/aggregations/AggregationUtils.ts

@@ -9,6 +9,7 @@ export const SELECTABLE_OPTIONS = [
   AggregationOperation.MAX,
   AggregationOperation.VAR,
   AggregationOperation.AVG,
+  AggregationOperation.MEDIAN,
   AggregationOperation.STD,
   AggregationOperation.FIRST,
   AggregationOperation.LAST,
@@ -31,6 +32,20 @@ export const isRollupOperation = (type: AggregationOperation): boolean => {
   }
 };
 
+/**
+ * Check if an operation is not supported for a rollup/table grouping
+ * @param type The operation to check
+ * @returns True if this operation is not supported for a rollup/table grouping
+ */
+export const isRollupProhibited = (type: AggregationOperation): boolean => {
+  switch (type) {
+    case AggregationOperation.MEDIAN:
+      return true;
+    default:
+      return false;
+  }
+};
+
 /**
  * Check if an operation is valid against the given column type
  * @param operationType The operation to check
@@ -48,6 +63,7 @@ export const isValidOperation = (
     case AggregationOperation.DISTINCT:
     case AggregationOperation.UNIQUE:
       return true;
+    case AggregationOperation.MEDIAN:
     case AggregationOperation.MIN:
     case AggregationOperation.MAX:
       return (
@@ -87,6 +103,7 @@ export const getOperationColumnNames = (
 
 export default {
   isRollupOperation,
+  isRollupProhibited,
   filterValidColumns,
   getOperationColumnNames,
 };

packages/iris-grid/src/sidebar/aggregations/Aggregations.test.tsx

@@ -1,6 +1,7 @@
 import React from 'react';
 import { render, RenderResult, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
+import type { dh as DhType } from '@deephaven/jsapi-types';
 import Aggregations, { Aggregation } from './Aggregations';
 import AggregationOperation from './AggregationOperation';
 import { SELECTABLE_OPTIONS } from './AggregationUtils';
@@ -13,6 +14,25 @@ function makeAggregation({
   return { operation, selected, invert };
 }
 
+const MOCK_DH = {
+  AggregationOperation: {
+    SUM: 'SUM',
+    ABS_SUM: 'ABS_SUM',
+    MIN: 'MIN',
+    MAX: 'MAX',
+    VAR: 'VAR',
+    AVG: 'AVG',
+    MEDIAN: 'MEDIAN',
+    STD: 'STD',
+    FIRST: 'FIRST',
+    LAST: 'LAST',
+    COUNT_DISTINCT: 'COUNT_DISTINCT',
+    DISTINCT: 'DISTINCT',
+    COUNT: 'COUNT',
+    UNIQUE: 'UNIQUE',
+  },
+} as unknown as typeof DhType;
+
 function mountAggregations({
   settings = { aggregations: [] as Aggregation[], showOnTop: false },
   onChange = jest.fn(),
@@ -25,6 +45,7 @@ function mountAggregations({
       onChange={onChange}
       onEdit={onEdit}
       isRollup={isRollup}
+      dh={MOCK_DH}
     />
   );
 }
@@ -37,6 +58,7 @@ it('shows all operations in select when no aggregations selected yet', () => {
   expect(screen.getByText('Min')).toBeInTheDocument();
   expect(screen.getByText('Max')).toBeInTheDocument();
   expect(screen.getByText('Avg')).toBeInTheDocument();
+  expect(screen.getByText('Median')).toBeInTheDocument();
   expect(screen.getByText('Std')).toBeInTheDocument();
   expect(screen.getByText('First')).toBeInTheDocument();
   expect(screen.getByText('Last')).toBeInTheDocument();
@@ -61,7 +83,9 @@ it('adds an aggregation when clicking the add button', async () => {
       aggregations: expect.arrayContaining([
         expect.objectContaining({ operation: SELECTABLE_OPTIONS[0] }),
       ]),
-    })
+    }),
+    expect.arrayContaining([SELECTABLE_OPTIONS[0]]),
+    expect.arrayContaining([])
   );
 });
 
@@ -80,7 +104,9 @@ it('deletes an aggregation when clicking the trash can', async () => {
   await user.click(buttons[buttons.length - 1]);
 
   expect(onChange).toHaveBeenCalledWith(
-    expect.objectContaining({ aggregations: [], showOnTop: false })
+    expect.objectContaining({ aggregations: [], showOnTop: false }),
+    expect.arrayContaining([]),
+    expect.arrayContaining([AggregationOperation.SUM])
   );
 });
 

packages/iris-grid/src/sidebar/aggregations/Aggregations.tsx

@@ -20,6 +20,7 @@ import {
 } from '@deephaven/components';
 import type { DraggableRenderItemProps, Range } from '@deephaven/components';
 import { ModelIndex } from '@deephaven/grid';
+import { dh as DhType } from '@deephaven/jsapi-types';
 import AggregationOperation from './AggregationOperation';
 import AggregationUtils, { SELECTABLE_OPTIONS } from './AggregationUtils';
 import './Aggregations.scss';
@@ -40,36 +41,48 @@ export type AggregationSettings = {
 export type AggregationsProps = {
   isRollup: boolean;
   settings: AggregationSettings;
-  onChange: (settings: AggregationSettings) => void;
+  onChange: (
+    settings: AggregationSettings,
+    added: AggregationOperation[],
+    removed: AggregationOperation[]
+  ) => void;
   onEdit: (aggregation: Aggregation) => void;
+  dh: typeof DhType;
 };
 
 function Aggregations({
   isRollup,
   settings,
   onChange,
   onEdit,
+  dh,
 }: AggregationsProps): JSX.Element {
   const { aggregations, showOnTop } = settings;
   const options = useMemo(
     () =>
       SELECTABLE_OPTIONS.filter(
         option =>
-          !aggregations.some(aggregation => aggregation.operation === option)
+          !aggregations.some(aggregation => aggregation.operation === option) &&
+          !(
+            option === AggregationOperation.MEDIAN &&
+            // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+            // @ts-ignore - MEDIAN is not defined in older version of Core
+            dh.AggregationOperation.MEDIAN === undefined
+          )
       ),
-    [aggregations]
+    [aggregations, dh]
   );
   const [selectedOperation, setSelectedOperation] = useState(options[0]);
   const [selectedRanges, setSelectedRanges] = useState<Range[]>([]);
   const changeSettings = useCallback(
-    changedSettings => {
-      onChange({ ...settings, ...changedSettings });
+    (changedSettings, added = [], removed = []) => {
+      onChange({ ...settings, ...changedSettings }, added, removed);
     },
     [onChange, settings]
   );
   const changeAggregations = useCallback(
-    newAggregations => {
-      changeSettings({ aggregations: newAggregations });
+    (newAggregations, added = [], removed = []) => {
+      changeSettings({ aggregations: newAggregations }, added, removed);
     },
     [changeSettings]
   );
@@ -130,17 +143,29 @@ function Aggregations({
   );
 
   const handleAdd = useCallback(() => {
-    changeAggregations([
-      ...aggregations,
-      { operation: selectedOperation, selected: [], invert: true },
-    ]);
+    changeAggregations(
+      [
+        ...aggregations,
+        { operation: selectedOperation, selected: [], invert: true },
+      ],
+      [selectedOperation]
+    );
   }, [aggregations, selectedOperation, changeAggregations]);
 
   const handleDeleteClicked = useCallback(
     (itemIndex: ModelIndex) => {
-      changeAggregations(
-        aggregations.filter((aggregation, index) => index !== itemIndex)
+      const [keep, remove] = aggregations.reduce(
+        ([keepSoFar, removeSoFar], aggregation, index) => {
+          if (index === itemIndex) {
+            removeSoFar.push(aggregation.operation);
+          } else {
+            keepSoFar.push(aggregation);
+          }
+          return [keepSoFar, removeSoFar];
+        },
+        [[], []] as [Aggregation[], AggregationOperation[]]
       );
+      changeAggregations(keep, [], remove);
     },
     [aggregations, changeAggregations]
   );
@@ -191,6 +216,9 @@ function Aggregations({
       const isRollupOperation = AggregationUtils.isRollupOperation(
         item.operation
       );
+      const isRollupProhibited = AggregationUtils.isRollupProhibited(
+        item.operation
+      );
       const isEditable = !isClone && !isRollupOperation;
       return (
         <>
@@ -208,6 +236,12 @@ function Aggregations({
                   <FontAwesomeIcon icon={dhWarningFilled} /> Requires rollup
                 </span>
               )}
+              {isRollup && isRollupProhibited && (
+                <span className="small text-warning">
+                  <FontAwesomeIcon icon={dhWarningFilled} /> Not available on
+                  rollups
+                </span>
+              )}
             </span>
             {DraggableItemList.renderBadge({ text: badgeText })}
             {DraggableItemList.renderHandle()}