This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch fix-table-chart-reload in repository https://gitbox.apache.org/repos/asf/superset.git
commit 07707867b51ec104ca4b3ea6daf226c5670271d7 Author: Joe Li <[email protected]> AuthorDate: Tue Dec 16 15:59:11 2025 -0800 fix(dashboard): prevent table chart infinite reload loop Strip clientView from ownState in getRelevantDataMask to prevent infinite reload loops in dashboard context. TableChart writes clientView to ownState on every filtered-row change for export functionality, but clientView changes should NOT trigger chart re-queries. This matches the behavior already present in ExploreViewContainer which strips clientView before comparing ownState changes. Fixes #36595 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --- .../src/dashboard/components/Dashboard.test.jsx | 76 +++++++++++ .../util/activeAllDashboardFilters.test.ts | 151 +++++++++++++++++++++ .../dashboard/util/activeAllDashboardFilters.ts | 9 +- 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..6f664262ce 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx @@ -332,4 +332,80 @@ describe('Dashboard', () => { expect(mockTriggerQuery).not.toHaveBeenCalled(); }); }); + + // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks + describe('ownDataCharts updates', () => { + test('should NOT call refresh when ownDataCharts content is unchanged (clientView stripped upstream)', () => { + // This test verifies Dashboard behavior when ownDataCharts stays the same. + // In the real app, getRelevantDataMask strips clientView from dataMask.ownState, + // so when only clientView changes, Dashboard receives identical ownDataCharts. + const initialOwnDataCharts = { + 1: { pageSize: 10, currentPage: 0 }, + }; + + const { rerender } = renderDashboard({ + ownDataCharts: initialOwnDataCharts, + dashboardState: { + ...dashboardState, + editMode: false, + }, + }); + + // Rerender with same ownDataCharts (this is what happens after clientView is stripped) + rerender( + <PluginContext.Provider value={{ loading: false }}> + <Dashboard + {...props} + ownDataCharts={initialOwnDataCharts} + dashboardState={{ + ...dashboardState, + editMode: false, + }} + > + <ChildrenComponent /> + </Dashboard> + </PluginContext.Provider>, + ); + + // No refresh should occur since ownDataCharts content is unchanged + expect(mockTriggerQuery).not.toHaveBeenCalled(); + }); + + test('should call refresh when real ownDataCharts changes occur', () => { + const initialOwnDataCharts = { + 1: { pageSize: 10, currentPage: 0 }, + }; + + const { rerender } = renderDashboard({ + ownDataCharts: initialOwnDataCharts, + dashboardState: { + ...dashboardState, + editMode: false, + }, + }); + + // Real change: pageSize changed + 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>, + ); + + // pageSize changed, so chart should be refreshed + 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..90bd5ea601 100644 --- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.test.ts +++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.test.ts @@ -165,3 +165,154 @@ test('should preserve the structure of the property value', () => { }, }); }); + +test('should strip clientView from ownState to prevent infinite reload loops', () => { + // This test verifies the fix for issue #36595 + // 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..4b4a2dd394 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,13 @@ export const getRelevantDataMask = ( Object.fromEntries( Object.values(dataMask) .filter(item => item[prop]) - .map(item => [item.id, item[prop]]), + .map(item => [ + item.id, + // Strip clientView from ownState to prevent infinite reload loops (issue #36595) + // TableChart writes clientView to ownState on every filtered-row change for export + // but clientView changes should NOT trigger chart re-queries + prop === 'ownState' ? omit(item[prop], ['clientView']) : item[prop], + ]), ); interface LayerInfo {
