diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx index dd00834ba2ee..3c950fe7b5ef 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( + + + + + , + ); + + 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( + + + + + , + ); + + 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 e274c5a08169..dd2363f5aa61 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 102a2161ea7e..c44aa7662a9d 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 {