mirror of
https://github.com/apache/superset.git
synced 2026-07-27 17:12:36 +00:00
fix(ag-grid): persist "None" value aggregation selection (#41386)
Co-authored-by: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com>
Co-authored-by: Enzo Martellucci <enzomartellucci@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit fb496e158a)
This commit is contained in:
@@ -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';
|
||||
|
||||
@@ -335,11 +336,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;
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
@@ -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 },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user