fix(plugin-chart-ag-grid-table): persist Value Aggregation changes without a sort (#97551)

Changing a column's Value Aggregation only persisted if another piece of grid
state (such as a sort) changed at the same time, because the grid-state change
detector hashed only column order, sorts and filters. This matches the
reported workaround (trigger a sort, then Update chart).

Include per-column aggFunc in the capture signature so the change is detected
and saved on its own. Same root cause as #107166.

Add unit tests for the Explore 'change aggregation then Update chart' scenario.

superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx

@@ -57,6 +57,7 @@ import { SearchOption, SortByItem } from '../types';
 import getInitialSortState, { shouldSort } from '../utils/getInitialSortState';
 import getInitialFilterModel from '../utils/getInitialFilterModel';
 import reconcileColumnState from '../utils/reconcileColumnState';
+import getColumnStateSignature from '../utils/getColumnStateSignature';
 import { PAGE_SIZE_OPTIONS } from '../consts';
 import { getCompleteFilterState } from '../utils/filterStateManager';
 
@@ -337,11 +338,11 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
               timestamp: Date.now(),
             };
 
-            const stateHash = JSON.stringify({
-              columnOrder: columnState.map(c => c.colId),
-              sorts: sortModel,
-              filters: filterModel,
-            });
+            const stateHash = getColumnStateSignature(
+              columnState,
+              sortModel,
+              filterModel,
+            );
 
             if (stateHash !== lastCapturedStateRef.current) {
               lastCapturedStateRef.current = stateHash;

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getColumnStateSignature.ts

@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { ColumnState } from '@superset-ui/core/components/ThemedAgGridReact';
+
+type SortModelEntry = {
+  colId: string;
+  sort: 'asc' | 'desc';
+  sortIndex: number;
+};
+
+/**
+ * Builds a stable signature of the parts of AG Grid column state that must be
+ * persisted (and therefore must trigger an `onColumnStateChange` when they
+ * change): column order, per-column value aggregation, sorts and filters.
+ *
+ * The value aggregation (`aggFunc`) is included so that changing a column's
+ * Value Aggregation is captured and saved. Without it, the change was only
+ * persisted if some *other* state (e.g. a sort) changed at the same time — so
+ * "Update chart"/save reverted the aggregation back to its default.
+ */
+export default function getColumnStateSignature(
+  columnState: ColumnState[],
+  sortModel: SortModelEntry[],
+  filterModel: Record<string, unknown>,
+): string {
+  return JSON.stringify({
+    columnOrder: columnState.map(col => col.colId),
+    aggregations: columnState.map(col => ({
+      colId: col.colId,
+      // Normalize falsy "None" (null/undefined) to an explicit sentinel so it
+      // is preserved and distinguishable from a real aggregation function.
+      aggFunc: col.aggFunc ?? null,
+    })),
+    sorts: sortModel,
+    filters: filterModel,
+  });
+}

superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getColumnStateSignature.test.ts

@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import getColumnStateSignature from '../../src/utils/getColumnStateSignature';
+
+const noSorts: any[] = [];
+const noFilters = {};
+
+test('changing only the value aggregation changes the signature (no sort needed)', () => {
+  // Regression test for #97551: in Explore, changing a metric's Value
+  // Aggregation and clicking "Update chart" must persist without first having
+  // to trigger an unrelated sort change. The capture signature must therefore
+  // react to aggFunc on its own.
+  const before = getColumnStateSignature(
+    [{ colId: 'SUM(sales)', aggFunc: 'sum' }] as any,
+    noSorts,
+    noFilters,
+  );
+  const after = getColumnStateSignature(
+    [{ colId: 'SUM(sales)', aggFunc: 'avg' }] as any,
+    noSorts,
+    noFilters,
+  );
+  expect(after).not.toEqual(before);
+});
+
+test('aggregation change is detected even when order/sort/filter are identical', () => {
+  const columnsA = [
+    { colId: 'state', aggFunc: undefined },
+    { colId: 'SUM(sales)', aggFunc: 'sum' },
+  ];
+  const columnsB = [
+    { colId: 'state', aggFunc: undefined },
+    { colId: 'SUM(sales)', aggFunc: 'max' },
+  ];
+  expect(
+    getColumnStateSignature(columnsA as any, noSorts, noFilters),
+  ).not.toEqual(getColumnStateSignature(columnsB as any, noSorts, noFilters));
+});
+
+test('identical aggregation produces a stable signature', () => {
+  const cols = [{ colId: 'SUM(sales)', aggFunc: 'sum' }];
+  expect(getColumnStateSignature(cols as any, noSorts, noFilters)).toEqual(
+    getColumnStateSignature(cols as any, noSorts, noFilters),
+  );
+});