This is an automated email from the ASF dual-hosted git repository.
rusackas 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 c026ae2ce7 fix(dashboard): prevent table chart infinite reload loop
(#36686)
c026ae2ce7 is described below
commit c026ae2ce712126fd009681b36dfd47124567f0b
Author: Joe Li <[email protected]>
AuthorDate: Fri Dec 19 13:45:36 2025 -0800
fix(dashboard): prevent table chart infinite reload loop (#36686)
Co-authored-by: Claude Opus 4.5 <[email protected]>
---
.../src/dashboard/components/Dashboard.test.jsx | 69 ++++++++++
.../util/activeAllDashboardFilters.test.ts | 150 +++++++++++++++++++++
.../dashboard/util/activeAllDashboardFilters.ts | 17 ++-
3 files changed, 235 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx
b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
index dd00834ba2..3c950fe7b5 100644
--- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx
+++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
@@ -332,4 +332,73 @@ describe('Dashboard', () => {
expect(mockTriggerQuery).not.toHaveBeenCalled();
});
});
+
+ test('should NOT call refresh when ownDataCharts content is unchanged', ()
=> {
+ // When only clientView changes, getRelevantDataMask strips it,
+ // so Dashboard receives identical ownDataCharts and should not refresh
+ const initialOwnDataCharts = {
+ 1: { pageSize: 10, currentPage: 0 },
+ };
+
+ const { rerender } = renderDashboard({
+ ownDataCharts: initialOwnDataCharts,
+ dashboardState: {
+ ...dashboardState,
+ editMode: false,
+ },
+ });
+
+ // Rerender with same ownDataCharts (simulates clientView-only change
after stripping)
+ rerender(
+ <PluginContext.Provider value={{ loading: false }}>
+ <Dashboard
+ {...props}
+ ownDataCharts={initialOwnDataCharts}
+ dashboardState={{
+ ...dashboardState,
+ editMode: false,
+ }}
+ >
+ <ChildrenComponent />
+ </Dashboard>
+ </PluginContext.Provider>,
+ );
+
+ expect(mockTriggerQuery).not.toHaveBeenCalled();
+ });
+
+ test('should call refresh when ownDataCharts pageSize changes', () => {
+ const initialOwnDataCharts = {
+ 1: { pageSize: 10, currentPage: 0 },
+ };
+
+ const { rerender } = renderDashboard({
+ ownDataCharts: initialOwnDataCharts,
+ dashboardState: {
+ ...dashboardState,
+ editMode: false,
+ },
+ });
+
+ const updatedOwnDataCharts = {
+ 1: { pageSize: 20, currentPage: 0 },
+ };
+
+ rerender(
+ <PluginContext.Provider value={{ loading: false }}>
+ <Dashboard
+ {...props}
+ ownDataCharts={updatedOwnDataCharts}
+ dashboardState={{
+ ...dashboardState,
+ editMode: false,
+ }}
+ >
+ <ChildrenComponent />
+ </Dashboard>
+ </PluginContext.Provider>,
+ );
+
+ expect(mockTriggerQuery).toHaveBeenCalledWith(true, '1');
+ });
});
diff --git
a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.test.ts
b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.test.ts
index e274c5a081..dd2363f5aa 100644
--- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.test.ts
+++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.test.ts
@@ -165,3 +165,153 @@ test('should preserve the structure of the property
value', () => {
},
});
});
+
+test('should strip clientView from ownState to prevent infinite reload loops',
() => {
+ // TableChart writes clientView to ownState on every filtered-row change
+ // If clientView is not stripped, Dashboard treats it as a chart-state change
+ // and re-triggers queries, causing an infinite reload loop
+ const mockDataMaskWithClientView: DataMaskStateWithId = {
+ chart1: {
+ id: 'chart1',
+ ownState: {
+ pageSize: 10,
+ currentPage: 0,
+ // clientView is added by TableChart for export functionality
+ // but should NOT trigger chart re-queries
+ clientView: {
+ rows: [{ id: 1, name: 'Test' }],
+ columns: [{ key: 'id' }, { key: 'name' }],
+ count: 1,
+ },
+ },
+ },
+ chart2: {
+ id: 'chart2',
+ ownState: {
+ sortBy: [{ id: 'col1', desc: true }],
+ clientView: {
+ rows: [{ id: 2, name: 'Test2' }],
+ columns: [{ key: 'id' }, { key: 'name' }],
+ count: 1,
+ },
+ },
+ },
+ };
+
+ const result = getRelevantDataMask(mockDataMaskWithClientView, 'ownState');
+
+ // clientView should be stripped from ownState
+ // Only pageSize, currentPage, sortBy etc should remain
+ expect(result).toEqual({
+ chart1: {
+ pageSize: 10,
+ currentPage: 0,
+ },
+ chart2: {
+ sortBy: [{ id: 'col1', desc: true }],
+ },
+ });
+});
+
+test('should return equal results when only clientView changes', () => {
+ // When TableChart updates clientView (on every filtered-row change),
+ // the selector output should remain equal, so Dashboard.jsx's
+ // areObjectsEqual comparison returns true and doesn't trigger re-queries
+
+ const dataMaskBefore: DataMaskStateWithId = {
+ chart1: {
+ id: 'chart1',
+ ownState: {
+ pageSize: 10,
+ currentPage: 0,
+ clientView: {
+ rows: [{ id: 1, name: 'Original' }],
+ count: 1,
+ },
+ },
+ },
+ };
+
+ const dataMaskAfter: DataMaskStateWithId = {
+ chart1: {
+ id: 'chart1',
+ ownState: {
+ pageSize: 10,
+ currentPage: 0,
+ // clientView changed (simulating TableChart updating filtered rows)
+ clientView: {
+ rows: [
+ { id: 1, name: 'Updated' },
+ { id: 2, name: 'New Row' },
+ ],
+ count: 2,
+ },
+ },
+ },
+ };
+
+ const resultBefore = getRelevantDataMask(dataMaskBefore, 'ownState');
+ const resultAfter = getRelevantDataMask(dataMaskAfter, 'ownState');
+
+ // Both results should be equal since clientView is stripped
+ // This is what prevents the infinite reload loop in Dashboard
+ expect(resultBefore).toEqual(resultAfter);
+ expect(resultBefore).toEqual({
+ chart1: { pageSize: 10, currentPage: 0 },
+ });
+});
+
+test('should return extraFormData unchanged (clientView stripping only applies
to ownState)', () => {
+ // Verify extraFormData is passed through without modification
+ const mockDataMask: DataMaskStateWithId = {
+ filter1: {
+ id: 'filter1',
+ extraFormData: {
+ filters: [{ col: 'country', op: 'IN', val: ['USA'] }],
+ granularity_sqla: 'ds',
+ },
+ },
+ filter2: {
+ id: 'filter2',
+ extraFormData: {
+ time_range: 'Last 7 days',
+ },
+ },
+ };
+
+ const result = getRelevantDataMask(mockDataMask, 'extraFormData');
+
+ // extraFormData should be returned unchanged
+ expect(result).toEqual({
+ filter1: {
+ filters: [{ col: 'country', op: 'IN', val: ['USA'] }],
+ granularity_sqla: 'ds',
+ },
+ filter2: {
+ time_range: 'Last 7 days',
+ },
+ });
+});
+
+test('should return filterState unchanged (clientView stripping only applies
to ownState)', () => {
+ // Verify filterState is passed through without modification
+ const mockDataMask: DataMaskStateWithId = {
+ filter1: {
+ id: 'filter1',
+ filterState: {
+ value: ['A', 'B'],
+ label: 'Categories A and B',
+ },
+ },
+ };
+
+ const result = getRelevantDataMask(mockDataMask, 'filterState');
+
+ // filterState should be returned unchanged
+ expect(result).toEqual({
+ filter1: {
+ value: ['A', 'B'],
+ label: 'Categories A and B',
+ },
+ });
+});
diff --git a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
index 102a2161ea..c44aa7662a 100644
--- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
+++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
@@ -22,6 +22,7 @@ import {
JsonObject,
PartialFilters,
} from '@superset-ui/core';
+import { omit } from 'lodash';
import { ActiveFilters, ChartConfiguration } from '../types';
export const getRelevantDataMask = (
@@ -31,7 +32,21 @@ export const getRelevantDataMask = (
Object.fromEntries(
Object.values(dataMask)
.filter(item => item[prop])
- .map(item => [item.id, item[prop]]),
+ .map(item => {
+ const value = item[prop];
+ // TableChart writes clientView to ownState on every filtered-row
change for export
+ // but clientView changes should NOT trigger chart re-queries
+ // Only clone when clientView exists to avoid unnecessary allocations
+ if (
+ prop === 'ownState' &&
+ value &&
+ typeof value === 'object' &&
+ 'clientView' in value
+ ) {
+ return [item.id, omit(value, ['clientView'])];
+ }
+ return [item.id, value];
+ }),
);
interface LayerInfo {
