This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new fb496e158a2 fix(ag-grid): persist "None" value aggregation selection
(#41386)
fb496e158a2 is described below
commit fb496e158a2a9ab84387239286293b58b1af7f6b
Author: amaannawab923 <[email protected]>
AuthorDate: Tue Jul 7 01:21:26 2026 +0530
fix(ag-grid): persist "None" value aggregation selection (#41386)
Co-authored-by: Enzo Martellucci <[email protected]>
Co-authored-by: Enzo Martellucci <[email protected]>
Co-authored-by: Claude Fable 5 <[email protected]>
---
.../src/AgGridTable/index.tsx | 11 +--
.../src/utils/getColumnStateSignature.ts | 51 ++++++++++++++
.../src/utils/reconcileColumnState.test.ts | 17 +++++
.../test/AgGridTableChart.test.tsx | 48 ++++++++++++-
.../test/utils/getColumnStateSignature.test.ts | 82 ++++++++++++++++++++++
5 files changed, 203 insertions(+), 6 deletions(-)
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
index 0dc74c8ac58..c363cf88556 100644
---
a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
+++
b/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;
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getColumnStateSignature.ts
b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getColumnStateSignature.ts
new file mode 100644
index 00000000000..239c7eba190
--- /dev/null
+++
b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getColumnStateSignature.ts
@@ -0,0 +1,51 @@
+/**
+ * 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;
+};
+
+/**
+ * Stable signature of the persisted parts of AG Grid column state (order,
+ * value aggregation, sorts, filters), used to detect changes worth saving.
+ * `aggFunc` is normalized so the "None" option (`null`/`undefined`) produces
+ * a distinct signature instead of reverting to the default on reload.
+ */
+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,
+ // "None" (null/undefined) maps to an explicit sentinel; functions map
+ // to a fixed marker so the signature never depends on JSON.stringify
+ // dropping them (defensive: persisted aggFuncs are always strings).
+ aggFunc:
+ typeof col.aggFunc === 'function' ? 'custom' : (col.aggFunc ?? null),
+ })),
+ sorts: sortModel,
+ filters: filterModel,
+ });
+}
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/reconcileColumnState.test.ts
b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/reconcileColumnState.test.ts
index 78e0e79b2da..814b85b2a4c 100644
---
a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/reconcileColumnState.test.ts
+++
b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/reconcileColumnState.test.ts
@@ -84,3 +84,20 @@ test('drops stale order when a dynamic group by swaps the
dimension column', ()
],
});
});
+
+test('preserves per-column aggFunc, including explicit "None" (null), through
reconciliation', () => {
+ // Regression #107166: saved aggFunc must reach applyColumnState untouched.
+ const colDefs: ColDef[] = [{ field: 'name' }, { field: 'SUM(Sales_Amount)'
}];
+ const savedColumnState: ColumnState[] = [
+ { colId: 'name' },
+ { colId: 'SUM(Sales_Amount)', aggFunc: null },
+ ];
+
+ expect(reconcileColumnState(savedColumnState, colDefs)).toEqual({
+ applyOrder: true,
+ columnState: [
+ { colId: 'name' },
+ { colId: 'SUM(Sales_Amount)', aggFunc: null },
+ ],
+ });
+});
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/AgGridTableChart.test.tsx
b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/AgGridTableChart.test.tsx
index 8c27f3b3498..9e233e53543 100644
---
a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/AgGridTableChart.test.tsx
+++
b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/AgGridTableChart.test.tsx
@@ -20,7 +20,10 @@ import '@testing-library/jest-dom';
import { render, screen, waitFor } from '@superset-ui/core/spec';
import { QueryMode, TimeGranularity, SMART_DATE_ID } from '@superset-ui/core';
import { GenericDataType } from '@apache-superset/core/common';
-import { setupAGGridModules } from
'@superset-ui/core/components/ThemedAgGridReact';
+import {
+ setupAGGridModules,
+ type ColumnState,
+} from '@superset-ui/core/components/ThemedAgGridReact';
import AgGridTableChart from '../src/AgGridTableChart';
import transformProps from '../src/transformProps';
import { ProviderWrapper } from '../../plugin-chart-table/test/testHelpers';
@@ -726,3 +729,46 @@ test('AgGridTableChart pins no summary row when totals are
absent', async () =>
const pinnedRows = document.querySelectorAll('.ag-floating-bottom .ag-row');
expect(pinnedRows.length).toBe(0);
});
+
+test('AgGridTableChart emits column state with aggFunc through the debounced
save path', async () => {
+ const props = transformProps(testData.basic);
+ const onChartStateChange = jest.fn();
+
+ render(
+ ProviderWrapper({
+ children: (
+ <AgGridTableChart
+ {...props}
+ setDataMask={mockSetDataMask}
+ slice_id={1}
+ onChartStateChange={onChartStateChange}
+ chartState={{
+ columnState: [{ colId: 'sum__num', aggFunc: null }],
+ sortModel: [],
+ filterModel: {},
+ }}
+ />
+ ),
+ }),
+ );
+
+ await waitFor(() => {
+ expect(document.querySelector('.ag-container')).toBeInTheDocument();
+ });
+
+ // The save path is debounced (SLOW_DEBOUNCE = 500ms); wait for a capture.
+ await waitFor(() => expect(onChartStateChange).toHaveBeenCalled(), {
+ timeout: 3000,
+ });
+
+ const savedState =
+ onChartStateChange.mock.calls[onChartStateChange.mock.calls.length - 1][0];
+ const savedColumn = (savedState.columnState as ColumnState[]).find(
+ col => col.colId === 'sum__num',
+ );
+
+ // Entries must carry an aggFunc key so "None" survives reload. The value
+ // itself is not assertable here: aggregation state needs an enterprise
+ // (SharedAggregation) module; the community modules always report null.
+ expect(savedColumn).toMatchObject({ aggFunc: null });
+});
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getColumnStateSignature.test.ts
b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getColumnStateSignature.test.ts
new file mode 100644
index 00000000000..3c9e894463f
--- /dev/null
+++
b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getColumnStateSignature.test.ts
@@ -0,0 +1,82 @@
+/**
+ * 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';
+import getColumnStateSignature from '../../src/utils/getColumnStateSignature';
+
+const filterModel = {};
+
+test('signature changes when a column value aggregation changes', () => {
+ const before = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'sum' }],
+ [],
+ filterModel,
+ );
+ const after = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'avg' }],
+ [],
+ filterModel,
+ );
+ expect(after).not.toEqual(before);
+});
+
+test('switching value aggregation to "None" (null/undefined) changes the
signature', () => {
+ // Regression #107166: "None" must be captured so it persists on reload.
+ const sumState = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'sum' }],
+ [],
+ filterModel,
+ );
+ const noneNull = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: null }],
+ [],
+ filterModel,
+ );
+ const noneUndefined = getColumnStateSignature(
+ [{ colId: 'sales' }],
+ [],
+ filterModel,
+ );
+
+ expect(noneNull).not.toEqual(sumState);
+ expect(noneUndefined).not.toEqual(sumState);
+ // null and undefined both represent "None" and must be treated identically.
+ expect(noneNull).toEqual(noneUndefined);
+});
+
+test('signature is stable when nothing changes', () => {
+ const columnState: ColumnState[] = [{ colId: 'sales', aggFunc: 'sum' }];
+ const a = getColumnStateSignature(columnState, [], filterModel);
+ const b = getColumnStateSignature([...columnState], [], filterModel);
+ expect(a).toEqual(b);
+});
+
+test('a function aggFunc is distinguishable from "None"', () => {
+ // Pins the defensive marker: function aggFuncs stay distinct from "None".
+ const customAgg = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: () => 42 }],
+ [],
+ filterModel,
+ );
+ const none = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: null }],
+ [],
+ filterModel,
+ );
+ expect(customAgg).not.toEqual(none);
+});
