fix: Default to Skip operation instead of Sum operation (#1648)

- Previously we were unable to remove columns from an aggregate
selection
- Default to `Skip` operation unless the user has actually applied one
or more operations for a column
- Requires deephaven/deephaven-core#4780
- Tested using the steps in the description of #1355
- Fixes #1355

packages/iris-grid/src/IrisGrid.tsx

@@ -854,7 +854,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     this.startListening(model);
   }
 
-  componentDidUpdate(prevProps: IrisGridProps): void {
+  componentDidUpdate(prevProps: IrisGridProps, prevState: IrisGridState): void {
     const {
       inputFilters,
       isSelectingColumn,
@@ -924,6 +924,18 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
       });
     }
 
+    const { openOptions } = this.state;
+    if (openOptions !== prevState.openOptions) {
+      const isAggEditType = (option: OptionItem): boolean =>
+        option.type === OptionType.AGGREGATION_EDIT;
+      if (
+        !openOptions.some(isAggEditType) &&
+        prevState.openOptions.some(isAggEditType)
+      ) {
+        this.removeEmptyAggregations();
+      }
+    }
+
     this.sendStateChange();
   }
 
@@ -1275,19 +1287,29 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
       config: UIRollupConfig | undefined,
       aggregationSettings: AggregationSettings
     ): UITotalsTableConfig | null => {
-      const { aggregations, showOnTop } = aggregationSettings;
-      // If we've got rollups, then aggregations are applied as part of that...
-      if (
-        (config?.columns?.length ?? 0) > 0 ||
-        (aggregations?.length ?? 0) === 0
-      ) {
+      if ((config?.columns?.length ?? 0) > 0) {
+        // If we've got rollups, then aggregations are applied as part of that...
+        return null;
+      }
+
+      // Filter out aggregations without any columns actually selected
+      const aggregations = aggregationSettings.aggregations.filter(
+        agg => agg.selected.length > 0 || agg.invert
+      );
+      if (aggregations.length === 0) {
+        // We don't actually have any aggregations set, don't bother
         return null;
       }
 
       const operationMap = this.getOperationMap(columns, aggregations);
       const operationOrder = this.getOperationOrder(aggregations);
 
-      return { operationMap, operationOrder, showOnTop };
+      return {
+        operationMap,
+        operationOrder,
+        showOnTop: aggregationSettings.showOnTop,
+        defaultOperation: AggregationOperation.SKIP,
+      };
     }
   );
 
@@ -3196,6 +3218,7 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     log.debug('handleAggregationChange', aggregation);
 
     this.startLoading(`Aggregating ${aggregation.operation}...`);
+
     this.setState(({ aggregationSettings }) => ({
       selectedAggregation: aggregation,
       aggregationSettings: {
@@ -3334,6 +3357,30 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     }
   }
 
+  /**
+   * Aggregation editing has completed. Need to filter out any aggregations that have no columns selected
+   */
+  removeEmptyAggregations(): void {
+    log.debug('removeEmptyAggregations');
+
+    this.setState(({ aggregationSettings }) => {
+      const { aggregations } = aggregationSettings;
+      const newAggregations = aggregations.filter(
+        a => a.selected.length > 0 || a.invert
+      );
+      if (newAggregations.length !== aggregations.length) {
+        return {
+          selectedAggregation: null,
+          aggregationSettings: {
+            ...aggregationSettings,
+            aggregations: newAggregations,
+          },
+        };
+      }
+      return { selectedAggregation: null, aggregationSettings };
+    });
+  }
+
   async seekRow(inputString: string, isBackwards = false): Promise<void> {
     const {
       gotoValueSelectedColumnName: selectedColumnName,

packages/iris-grid/src/IrisGridTableModel.ts

@@ -13,13 +13,9 @@ import type {
 } from '@deephaven/jsapi-types';
 import Log from '@deephaven/log';
 import { Formatter } from '@deephaven/jsapi-utils';
-import {
-  EventShimCustomEvent,
-  PromiseUtils,
-  assertNotNull,
-} from '@deephaven/utils';
+import { EventShimCustomEvent, assertNotNull } from '@deephaven/utils';
 import IrisGridModel from './IrisGridModel';
-import { ColumnName, UITotalsTableConfig, UIRow } from './CommonTypes';
+import { ColumnName, UIRow } from './CommonTypes';
 import IrisGridTableModelTemplate from './IrisGridTableModelTemplate';
 
 const log = Log.module('IrisGridTableModel');
@@ -185,53 +181,6 @@ class IrisGridTableModel extends IrisGridTableModelTemplate<Table, UIRow> {
     );
   }
 
-  set totalsConfig(totalsConfig: UITotalsTableConfig | null) {
-    log.debug('set totalsConfig', totalsConfig);
-
-    if (totalsConfig === this.totals) {
-      // Totals already set, or it will be set when the next model actually gets set
-      return;
-    }
-
-    this.totals = totalsConfig;
-    this.formattedStringData = [];
-
-    if (this.totalsTablePromise != null) {
-      this.totalsTablePromise.cancel();
-    }
-
-    this.setTotalsTable(null);
-
-    if (totalsConfig == null) {
-      this.dispatchEvent(new EventShimCustomEvent(IrisGridModel.EVENT.UPDATED));
-      return;
-    }
-
-    this.totalsTablePromise = PromiseUtils.makeCancelable(
-      this.table.getTotalsTable(totalsConfig),
-      table => table.close()
-    );
-    this.totalsTablePromise
-      .then(totalsTable => {
-        this.totalsTablePromise = null;
-        this.setTotalsTable(totalsTable);
-      })
-      .catch(err => {
-        if (PromiseUtils.isCanceled(err)) {
-          return;
-        }
-
-        log.error('Unable to set next totalsTable', err);
-        this.totalsTablePromise = null;
-
-        this.dispatchEvent(
-          new EventShimCustomEvent(IrisGridModel.EVENT.REQUEST_FAILED, {
-            detail: err,
-          })
-        );
-      });
-  }
-
   get isFilterRequired(): boolean {
     return this.table.isUncoalesced;
   }