You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by am...@apache.org on 2026/06/24 11:40:43 UTC
(superset) branch fix/107166-value-aggregation-none-persist created (now 70843d668af)
This is an automated email from the ASF dual-hosted git repository.
amaannawab923 pushed a change to branch fix/107166-value-aggregation-none-persist
in repository https://gitbox.apache.org/repos/asf/superset.git
at 70843d668af fix(plugin-chart-ag-grid-table): persist Value Aggregation "None" selection (#107166)
This branch includes the following new commits:
new 70843d668af fix(plugin-chart-ag-grid-table): persist Value Aggregation "None" selection (#107166)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(plugin-chart-ag-grid-table): persist Value Aggregation "None" selection (#107166)
Posted by am...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
amaannawab923 pushed a commit to branch fix/107166-value-aggregation-none-persist
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 70843d668afb4eaad004bbb4bd1b58aaac3e8d16
Author: amaannawab923 <am...@gmail.com>
AuthorDate: Wed Jun 24 17:10:13 2026 +0530
fix(plugin-chart-ag-grid-table): persist Value Aggregation "None" selection (#107166)
The grid-state change detector hashed only column order, sorts and filters,
so changing a column's Value Aggregation (aggFunc) never triggered
onColumnStateChange. On reload the column fell back to its default
initialAggFunc (sum-like), which is why selecting "None" reverted to Sum
while other values appeared to stick.
Extract the signature into getColumnStateSignature() and include per-column
aggFunc, normalizing the "None" case (null/undefined) to a stable sentinel so
the change is captured and persisted. Add unit tests.
---
.../src/AgGridTable/index.tsx | 11 ++--
.../src/utils/getColumnStateSignature.ts | 54 ++++++++++++++++
.../test/utils/getColumnStateSignature.test.ts | 75 ++++++++++++++++++++++
3 files changed, 135 insertions(+), 5 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 ebb66b0db28..606ad0d685d 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..8598cecd52e
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getColumnStateSignature.ts
@@ -0,0 +1,54 @@
+/**
+ * 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 normalized so that "no aggregation"
+ * (the AG Grid "None" option, represented as `null`/`undefined`) produces a
+ * distinct, stable signature. Without this, switching a column's Value
+ * Aggregation to "None" did not change the signature, so the change was never
+ * captured and the column reverted to its default aggregation 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,
+ // 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,
+ });
+}
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..45ef8df3292
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getColumnStateSignature.test.ts
@@ -0,0 +1,75 @@
+/**
+ * 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 sortModel: any[] = [];
+const filterModel = {};
+
+test('signature changes when a column value aggregation changes', () => {
+ const before = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'sum' }] as any,
+ sortModel,
+ filterModel,
+ );
+ const after = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'avg' }] as any,
+ sortModel,
+ filterModel,
+ );
+ expect(after).not.toEqual(before);
+});
+
+test('switching value aggregation to "None" (null/undefined) changes the signature', () => {
+ // Regression test for #107166: selecting "None" must be captured so it
+ // persists instead of reverting to the default Sum aggregation on reload.
+ const sumState = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'sum' }] as any,
+ sortModel,
+ filterModel,
+ );
+ const noneNull = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: null }] as any,
+ sortModel,
+ filterModel,
+ );
+ const noneUndefined = getColumnStateSignature(
+ [{ colId: 'sales' }] as any,
+ sortModel,
+ 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 a = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'sum' }] as any,
+ sortModel,
+ filterModel,
+ );
+ const b = getColumnStateSignature(
+ [{ colId: 'sales', aggFunc: 'sum' }] as any,
+ sortModel,
+ filterModel,
+ );
+ expect(a).toEqual(b);
+});