From db7398096bc13f74432dd14fd804ef10981a8da0 Mon Sep 17 00:00:00 2001 From: Puja Gupta Date: Tue, 21 Apr 2026 23:55:20 +0530 Subject: [PATCH 01/12] fix(home): refresh recent activity after dashboard/chart delete - Added refreshKey state to trigger re-fetch of activity data - Added onActivityRefresh callback prop to DashboardTable and ChartTable - Activity section now updates automatically after delete operations Fixes #39435 --- superset-frontend/src/features/home/ChartTable.tsx | 13 ++++++++++--- .../src/features/home/DashboardTable.tsx | 2 ++ superset-frontend/src/pages/Home/index.tsx | 9 ++++++++- superset-frontend/src/views/CRUD/types.ts | 1 + 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx index 2829634c362d..452ff3187a54 100644 --- a/superset-frontend/src/features/home/ChartTable.tsx +++ b/superset-frontend/src/features/home/ChartTable.tsx @@ -59,6 +59,7 @@ interface ChartTableProps { otherTabData?: Array; otherTabFilters: Filter[]; otherTabTitle: string; + onActivityRefresh?: () => void; } function ChartTable({ @@ -70,6 +71,7 @@ function ChartTable({ otherTabData, otherTabFilters, otherTabTitle, + onActivityRefresh, }: ChartTableProps) { const history = useHistory(); const initialTab = getItem( @@ -79,6 +81,7 @@ function ChartTable({ const filteredOtherTabData = otherTabData?.filter(obj => 'viz_type' in obj); + const { state: { loading, resourceCollection: charts, bulkSelectEnabled }, setResourceCollection: setCharts, @@ -94,6 +97,10 @@ function ChartTable({ [], false, ); + const handleRefreshData = () => { + refreshData(); + onActivityRefresh?.(); + }; const chartIds = useMemo(() => charts.map(c => c.id), [charts]); const [saveFavoriteStatus, favoriteStatus] = useFavoriteStatus( @@ -212,8 +219,8 @@ function ChartTable({ const target = activeTab === TableTab.Favorite ? `/chart/list/?filters=(favorite:(label:${t( - 'Yes', - )},value:!t))` + 'Yes', + )},value:!t))` : '/chart/list/'; history.push(target); }, @@ -232,7 +239,7 @@ function ChartTable({ hasPerm={hasPerm} showThumbnails={showThumbnails} bulkSelectEnabled={bulkSelectEnabled} - refreshData={refreshData} + refreshData={handleRefreshData} addDangerToast={addDangerToast} addSuccessToast={addSuccessToast} getData={getData} diff --git a/superset-frontend/src/features/home/DashboardTable.tsx b/superset-frontend/src/features/home/DashboardTable.tsx index 5289ab687d98..7c1d5f43840d 100644 --- a/superset-frontend/src/features/home/DashboardTable.tsx +++ b/superset-frontend/src/features/home/DashboardTable.tsx @@ -60,6 +60,7 @@ function DashboardTable({ otherTabData, otherTabFilters, otherTabTitle, + onActivityRefresh, }: DashboardTableProps) { const history = useHistory(); const defaultTab = getItem( @@ -268,6 +269,7 @@ function DashboardTable({ getData, ); setDashboardToDelete(null); + onActivityRefresh?.(); }} onHide={() => setDashboardToDelete(null)} open={!!dashboardToDelete} diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index 82ff403b81e2..2604a0afc60a 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -172,6 +172,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { null, ); const [isFetchingActivityData, setIsFetchingActivityData] = useState(true); + const [refreshKey, setRefreshKey] = useState(0); const collapseState = getItem(LocalStorageKeys.HomepageCollapseState, []); const [activeState, setActiveState] = useState>(collapseState); @@ -181,6 +182,10 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { setItem(LocalStorageKeys.HomepageCollapseState, state); }; + const handleRefreshActivity = () => { + setRefreshKey(prev => prev + 1); +}; + const SubmenuExtension = extensionsRegistry.get('home.submenu'); const WelcomeMessageExtension = extensionsRegistry.get('welcome.message'); const WelcomeTopExtension = extensionsRegistry.get('welcome.banner'); @@ -294,7 +299,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { ]).then(() => { setIsFetchingActivityData(false); }); - }, [otherTabFilters]); + }, [otherTabFilters, refreshKey]); const handleToggle = () => { setChecked(!checked); @@ -398,6 +403,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { otherTabData={activityData?.[TableTab.Other]} otherTabFilters={otherTabFilters} otherTabTitle={otherTabTitle} + onActivityRefresh={handleRefreshActivity} /> ), }, @@ -418,6 +424,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { otherTabData={activityData?.[TableTab.Other]} otherTabFilters={otherTabFilters} otherTabTitle={otherTabTitle} + onActivityRefresh={handleRefreshActivity} /> ), }, diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts index 08206bb03828..034d3fa9b321 100644 --- a/superset-frontend/src/views/CRUD/types.ts +++ b/superset-frontend/src/views/CRUD/types.ts @@ -50,6 +50,7 @@ export interface DashboardTableProps { otherTabData?: Array; otherTabFilters: Filter[]; otherTabTitle: string; + onActivityRefresh?: () => void; } export interface Dashboard { From 096656670b8efdaf9834a11aaa0c2549ecb4f670 Mon Sep 17 00:00:00 2001 From: Puja Gupta Date: Wed, 22 Apr 2026 00:27:57 +0530 Subject: [PATCH 02/12] fix(home): forward config param in handleRefreshData --- superset-frontend/src/features/home/ChartTable.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx index 452ff3187a54..4ac4b310446c 100644 --- a/superset-frontend/src/features/home/ChartTable.tsx +++ b/superset-frontend/src/features/home/ChartTable.tsx @@ -97,8 +97,8 @@ function ChartTable({ [], false, ); - const handleRefreshData = () => { - refreshData(); + const handleRefreshData = (config?: Parameters[0]) => { + refreshData(config); onActivityRefresh?.(); }; From ccc3f3165201e0ec703ba524b59c0b88d2f1b7d4 Mon Sep 17 00:00:00 2001 From: Evan Date: Tue, 30 Jun 2026 23:17:51 -0700 Subject: [PATCH 03/12] style: apply pre-commit autofixes (prettier, trailing whitespace) Co-Authored-By: Claude Opus 4.8 --- superset-frontend/src/features/home/ChartTable.tsx | 5 ++--- superset-frontend/src/pages/Home/index.tsx | 4 ++-- superset-frontend/src/views/CRUD/types.ts | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx index 4ac4b310446c..d8d911d4edfa 100644 --- a/superset-frontend/src/features/home/ChartTable.tsx +++ b/superset-frontend/src/features/home/ChartTable.tsx @@ -81,7 +81,6 @@ function ChartTable({ const filteredOtherTabData = otherTabData?.filter(obj => 'viz_type' in obj); - const { state: { loading, resourceCollection: charts, bulkSelectEnabled }, setResourceCollection: setCharts, @@ -219,8 +218,8 @@ function ChartTable({ const target = activeTab === TableTab.Favorite ? `/chart/list/?filters=(favorite:(label:${t( - 'Yes', - )},value:!t))` + 'Yes', + )},value:!t))` : '/chart/list/'; history.push(target); }, diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index 2604a0afc60a..dacc07a3887c 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -183,8 +183,8 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { }; const handleRefreshActivity = () => { - setRefreshKey(prev => prev + 1); -}; + setRefreshKey(prev => prev + 1); + }; const SubmenuExtension = extensionsRegistry.get('home.submenu'); const WelcomeMessageExtension = extensionsRegistry.get('welcome.message'); diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts index 034d3fa9b321..c06bbdefdcec 100644 --- a/superset-frontend/src/views/CRUD/types.ts +++ b/superset-frontend/src/views/CRUD/types.ts @@ -50,7 +50,7 @@ export interface DashboardTableProps { otherTabData?: Array; otherTabFilters: Filter[]; otherTabTitle: string; - onActivityRefresh?: () => void; + onActivityRefresh?: () => void; } export interface Dashboard { From 5622722e2ddfaccb8e9e07d05dd317c08c61b87a Mon Sep 17 00:00:00 2001 From: rusackas Date: Tue, 8 Sep 2026 10:56:54 -0700 Subject: [PATCH 04/12] fix(home): guard delete-triggered activity refresh against stale data - DashboardTable: only fire onActivityRefresh once the delete actually succeeds and triggers a real refetch, instead of immediately alongside the fire-and-forget delete call (which could race ahead of the backend delete and refresh with stale data). - Home/Welcome: guard the recent-activity effect against out-of-order responses when refreshKey changes again before an in-flight refresh resolves, so an older response can no longer overwrite newer state. Addresses CodeAnt review feedback on PR #39528. --- .../src/features/home/DashboardTable.tsx | 18 +++++-- superset-frontend/src/pages/Home/index.tsx | 48 +++++++++++++------ 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/superset-frontend/src/features/home/DashboardTable.tsx b/superset-frontend/src/features/home/DashboardTable.tsx index 7c1d5f43840d..44e9e82c34e2 100644 --- a/superset-frontend/src/features/home/DashboardTable.tsx +++ b/superset-frontend/src/features/home/DashboardTable.tsx @@ -135,6 +135,19 @@ function DashboardTable({ setLoaded(true); }, [activeTab]); + // Only notify the Home page's activity feed once the delete-triggered + // refresh actually runs (i.e. after the delete call succeeds), instead of + // firing immediately alongside the fire-and-forget delete request. + const handleRefreshData = (config?: Parameters[0]) => { + refreshData(config); + onActivityRefresh?.(); + }; + + const handleGetData = (tab: TableTab) => { + getData(tab); + onActivityRefresh?.(); + }; + const handleBulkDashboardExport = async (dashboardsToExport: Dashboard[]) => { const ids = dashboardsToExport.map(({ id }) => id); setPreparingExport(true); @@ -261,15 +274,14 @@ function DashboardTable({ onConfirm={() => { handleDashboardDelete( dashboardToDelete, - refreshData, + handleRefreshData, addSuccessToast, addDangerToast, activeTab, user?.userId, - getData, + handleGetData, ); setDashboardToDelete(null); - onActivityRefresh?.(); }} onHide={() => setDashboardToDelete(null)} open={!!dashboardToDelete} diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index dacc07a3887c..a86bcd9ee71b 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -219,12 +219,17 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { useEffect(() => { if (!otherTabFilters || WelcomeMainExtension) { - return; + return undefined; } + // Guards against out-of-order responses: if `refreshKey` changes again + // (e.g. a rapid follow-up delete) before this run's requests resolve, + // its results are discarded instead of overwriting newer state. + let isStale = false; const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); setActiveState(collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR); getRecentActivityObjs(user.userId!, recent, addDangerToast, otherTabFilters) .then(res => { + if (isStale) return; const data: ActivityData | null = {}; data[TableTab.Other] = res.other; if (res.viewed) { @@ -241,6 +246,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { }) .catch( createErrorHandler((errMsg: unknown) => { + if (isStale) return; setActivityData(activityData => ({ ...activityData, [TableTab.Viewed]: [], @@ -262,43 +268,55 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { Promise.all([ getUserEditableObjects(id, 'dashboard') .then(r => { - setDashboardData(r); + if (!isStale) setDashboardData(r); return Promise.resolve(); }) .catch((err: unknown) => { - setDashboardData([]); - addDangerToast( - t('There was an issue fetching your dashboards: %s', err), - ); + if (!isStale) { + setDashboardData([]); + addDangerToast( + t('There was an issue fetching your dashboards: %s', err), + ); + } return Promise.resolve(); }), getUserEditableObjects(id, 'chart') .then(r => { - setChartData(r); + if (!isStale) setChartData(r); return Promise.resolve(); }) .catch((err: unknown) => { - setChartData([]); - addDangerToast(t('There was an issue fetching your chart: %s', err)); + if (!isStale) { + setChartData([]); + addDangerToast( + t('There was an issue fetching your chart: %s', err), + ); + } return Promise.resolve(); }), canReadSavedQueries ? getUserEditableObjects(id, 'saved_query', ownSavedQueryFilters) .then(r => { - setQueryData(r); + if (!isStale) setQueryData(r); return Promise.resolve(); }) .catch((err: unknown) => { - setQueryData([]); - addDangerToast( - t('There was an issue fetching your saved queries: %s', err), - ); + if (!isStale) { + setQueryData([]); + addDangerToast( + t('There was an issue fetching your saved queries: %s', err), + ); + } return Promise.resolve(); }) : Promise.resolve(), ]).then(() => { - setIsFetchingActivityData(false); + if (!isStale) setIsFetchingActivityData(false); }); + + return () => { + isStale = true; + }; }, [otherTabFilters, refreshKey]); const handleToggle = () => { From 450b3dffdbee32148a4cccefbac651b61ee66595 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Wed, 21 Oct 2026 13:35:14 -0700 Subject: [PATCH 05/12] fix(home): wrap getData in ChartTable so delete refresh covers all tabs handleChartDelete calls getData(chartFilter) instead of refreshData whenever chartFilter is set and isn't 'Mine' (i.e. for the default Other/All tab), but ChartTable was passing the raw, unwrapped getData to that path - so deleting a chart from any tab other than Mine never fired onActivityRefresh at all. DashboardTable already wraps its equivalent getData call (handleGetData); mirrors that same fix here. Also adds the test coverage for the onActivityRefresh wiring that was asked for across review rounds - this exact gap only showed up once a test exercised the non-Mine delete path. Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5 --- superset-frontend/src/features/home/ChartTable.test.tsx | 3 +++ superset-frontend/src/features/home/ChartTable.tsx | 6 +++++- .../src/features/home/DashboardTable.test.tsx | 8 ++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/features/home/ChartTable.test.tsx b/superset-frontend/src/features/home/ChartTable.test.tsx index 997e5514fd49..c93360c252ee 100644 --- a/superset-frontend/src/features/home/ChartTable.test.tsx +++ b/superset-frontend/src/features/home/ChartTable.test.tsx @@ -208,9 +208,11 @@ test('refreshes other tab data after deleting a chart', async () => { message: 'Chart deleted', }); + const onActivityRefresh = jest.fn(); await renderChartTable({ ...otherTabProps, otherTabTitle: 'All', + onActivityRefresh, }); expect(screen.getByText('cool chart 0')).toBeInTheDocument(); @@ -247,4 +249,5 @@ test('refreshes other tab data after deleting a chart', async () => { expect(screen.getByText('cool chart 1')).toBeInTheDocument(); expect(screen.getByText('cool chart 2')).toBeInTheDocument(); + expect(onActivityRefresh).toHaveBeenCalledTimes(1); }); diff --git a/superset-frontend/src/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx index d8d911d4edfa..1207b9c9f333 100644 --- a/superset-frontend/src/features/home/ChartTable.tsx +++ b/superset-frontend/src/features/home/ChartTable.tsx @@ -131,6 +131,10 @@ function ChartTable({ }); const getData = (tab: TableTab) => fetchData(getChartFetchDataConfig(tab)); + const handleGetData = (tab: TableTab) => { + getData(tab); + onActivityRefresh?.(); + }; useEffect(() => { if (loaded || activeTab === TableTab.Favorite) { @@ -241,7 +245,7 @@ function ChartTable({ refreshData={handleRefreshData} addDangerToast={addDangerToast} addSuccessToast={addSuccessToast} - getData={getData} + getData={handleGetData} favoriteStatus={favoriteStatus[e.id]} saveFavoriteStatus={saveFavoriteStatus} handleBulkChartExport={handleBulkChartExport} diff --git a/superset-frontend/src/features/home/DashboardTable.test.tsx b/superset-frontend/src/features/home/DashboardTable.test.tsx index 24e41f6ef3a3..ff20102c817e 100644 --- a/superset-frontend/src/features/home/DashboardTable.test.tsx +++ b/superset-frontend/src/features/home/DashboardTable.test.tsx @@ -356,9 +356,11 @@ test('handles bulk dashboard export with correct ID and shows spinner', async () }); test('handles dashboard deletion confirmation', async () => { + const onActivityRefresh = jest.fn(); const props = { ...defaultProps, mine: mockDashboards, + onActivityRefresh, }; const refreshDataMock = jest.fn(); @@ -377,6 +379,11 @@ test('handles dashboard deletion confirmation', async () => { toggleBulkSelect: jest.fn(), })); + // onActivityRefresh must not fire before the delete-triggered refresh + // actually runs, since it's wired to follow refreshData, not the + // fire-and-forget delete call itself. + expect(onActivityRefresh).not.toHaveBeenCalled(); + render( @@ -418,6 +425,7 @@ test('handles dashboard deletion confirmation', async () => { ); expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(onActivityRefresh).toHaveBeenCalledTimes(1); }); test('passes correct parameters to handleDashboardDelete for Other tab', async () => { From 8c1c703119eedc1ece869079fa0cfb578be0e273 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Wed, 21 Oct 2026 13:36:22 -0700 Subject: [PATCH 06/12] chore: credit original author for home recent-activity refresh fix Adopting this fix as a fresh PR since the OAuth token used here lacks the `workflow` scope needed to push directly to the contributor's fork. The preceding commits are unchanged aside from a rebase onto current master, plus one follow-up fix (wrap getData in ChartTable so the activity refresh covers all tabs, not just Mine) and test coverage for the onActivityRefresh wiring that review rounds had asked for. Co-Authored-By: Puja Gupta Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5 From 7e1fb0789692f1569e5fc9d2733708430573cc54 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 22 Oct 2026 23:00:28 -0700 Subject: [PATCH 07/12] fix(home): keep collapse/tab state on refresh, refresh Edited list, tighten delete test Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5.5 --- .../src/features/home/ActivityTable.tsx | 4 ++- .../src/features/home/DashboardTable.test.tsx | 31 ++++++++++++++++--- superset-frontend/src/pages/Home/index.tsx | 25 ++++++++++----- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/features/home/ActivityTable.tsx b/superset-frontend/src/features/home/ActivityTable.tsx index 57167225bbc7..47f09fa322ec 100644 --- a/superset-frontend/src/features/home/ActivityTable.tsx +++ b/superset-frontend/src/features/home/ActivityTable.tsx @@ -63,6 +63,7 @@ interface ActivityProps { setActiveChild: (arg0: string) => void; activityData: ActivityData; isFetchingActivityData: boolean; + refreshKey?: number; } const Styles = styled.div` @@ -129,6 +130,7 @@ export default function ActivityTable({ activityData, user, isFetchingActivityData, + refreshKey = 0, }: ActivityProps) { const [editedCards, setEditedCards] = useState(); const [isFetchingEditedCards, setIsFetchingEditedCards] = useState(false); @@ -155,7 +157,7 @@ export default function ActivityTable({ isMounted = false; setIsFetchingEditedCards(false); }; - }, [activeChild, user.userId]); + }, [activeChild, user.userId, refreshKey]); const tabs = [ { diff --git a/superset-frontend/src/features/home/DashboardTable.test.tsx b/superset-frontend/src/features/home/DashboardTable.test.tsx index ff20102c817e..59e22e4e8fce 100644 --- a/superset-frontend/src/features/home/DashboardTable.test.tsx +++ b/superset-frontend/src/features/home/DashboardTable.test.tsx @@ -379,10 +379,19 @@ test('handles dashboard deletion confirmation', async () => { toggleBulkSelect: jest.fn(), })); - // onActivityRefresh must not fire before the delete-triggered refresh - // actually runs, since it's wired to follow refreshData, not the - // fire-and-forget delete call itself. - expect(onActivityRefresh).not.toHaveBeenCalled(); + // Defer the delete so the test can observe the state before it resolves. + let resolveDelete: () => void = () => {}; + const mockHandleDashboardDelete = + require('src/views/CRUD/utils').handleDashboardDelete; + mockHandleDashboardDelete.mockImplementationOnce( + (_dashboard: unknown, refreshData: () => void) => + new Promise(resolve => { + resolveDelete = () => { + refreshData(); + resolve(); + }; + }), + ); render( @@ -417,6 +426,15 @@ test('handles dashboard deletion confirmation', async () => { // Click the now-enabled Delete button await userEvent.click(confirmDeleteButton); + await waitFor(() => { + expect(mockHandleDashboardDelete).toHaveBeenCalled(); + }); + // The delete has not succeeded yet, so nothing may refresh. + expect(refreshDataMock).not.toHaveBeenCalled(); + expect(onActivityRefresh).not.toHaveBeenCalled(); + + resolveDelete(); + await waitFor( () => { expect(refreshDataMock).toHaveBeenCalled(); @@ -451,10 +469,12 @@ test('passes correct parameters to handleDashboardDelete for Other tab', async ( toggleBulkSelect: jest.fn(), })); + const onActivityRefresh = jest.fn(); const props = { ...defaultProps, otherTabData: mockDashboards, otherTabTitle: 'All', + onActivityRefresh, }; render( @@ -523,6 +543,8 @@ test('passes correct parameters to handleDashboardDelete for Other tab', async ( const lastCall = mockHandleDashboardDelete.mock.calls[0]; const getDataParam = lastCall[6]; + // The default mock also invokes refreshData; isolate the getData path. + onActivityRefresh.mockClear(); getDataParam('Other'); expect(fetchDataMock).toHaveBeenCalledWith({ filters: [], @@ -530,6 +552,7 @@ test('passes correct parameters to handleDashboardDelete for Other tab', async ( pageSize: 5, sortBy: [{ desc: true, id: 'changed_on_delta_humanized' }], }); + expect(onActivityRefresh).toHaveBeenCalledTimes(1); }); test('with soft delete on, the card delete flow shows the archive dialog', async () => { diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index a86bcd9ee71b..f45fbf35fafe 100644 --- a/superset-frontend/src/pages/Home/index.tsx +++ b/superset-frontend/src/pages/Home/index.tsx @@ -226,7 +226,14 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { // its results are discarded instead of overwriting newer state. let isStale = false; const activeTab = getItem(LocalStorageKeys.HomepageActivityFilter, null); - setActiveState(collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR); + // Collapse state and the initial tab are only restored on first load, so + // later refreshes update the data without overriding user choices. + const isInitialLoad = refreshKey === 0; + if (isInitialLoad) { + setActiveState( + collapseState.length > 0 ? collapseState : DEFAULT_TAB_ARR, + ); + } getRecentActivityObjs(user.userId!, recent, addDangerToast, otherTabFilters) .then(res => { if (isStale) return; @@ -235,13 +242,14 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { if (res.viewed) { const filtered = reject(res.viewed, ['item_url', null]).map(r => r); data[TableTab.Viewed] = filtered; - if (!activeTab && data[TableTab.Viewed]) { - setActiveChild(TableTab.Viewed); - } else if (!activeTab && !data[TableTab.Viewed]) { - setActiveChild(TableTab.Created); - } else setActiveChild(activeTab || TableTab.Created); - } else if (!activeTab) setActiveChild(TableTab.Created); - else setActiveChild(activeTab); + } + if (isInitialLoad) { + if (res.viewed) { + if (!activeTab) setActiveChild(TableTab.Viewed); + else setActiveChild(activeTab); + } else if (!activeTab) setActiveChild(TableTab.Created); + else setActiveChild(activeTab); + } setActivityData(activityData => ({ ...activityData, ...data })); }) .catch( @@ -402,6 +410,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { setActiveChild={setActiveChild} activityData={activityData} isFetchingActivityData={isFetchingActivityData} + refreshKey={refreshKey} /> ) : ( From 9eb39c27d9987117dc31c0844103390e4aa9b647 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Fri, 23 Oct 2026 14:47:59 -0700 Subject: [PATCH 08/12] test(home): cover delete-triggered recents refresh and stale-response guard Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5.5 --- .../src/pages/Home/Home.refresh.test.tsx | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 superset-frontend/src/pages/Home/Home.refresh.test.tsx diff --git a/superset-frontend/src/pages/Home/Home.refresh.test.tsx b/superset-frontend/src/pages/Home/Home.refresh.test.tsx new file mode 100644 index 000000000000..b1b5700ee5cf --- /dev/null +++ b/superset-frontend/src/pages/Home/Home.refresh.test.tsx @@ -0,0 +1,178 @@ +/** + * 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. + */ +// Imported first so the antd mock is defined before anything requires 'antd'. +import { mockAntdWithDesktopBreakpoint } from 'spec/helpers/mobileTestUtils'; +import fetchMock from 'fetch-mock'; +import { SupersetClient } from '@superset-ui/core'; +import { + render, + screen, + userEvent, + waitFor, +} from 'spec/helpers/testing-library'; +import Welcome from 'src/pages/Home'; +import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers'; + +const chartsEndpoint = 'glob:*/api/v1/chart/?*'; +const dashboardsEndpoint = 'glob:*/api/v1/dashboard/?*'; +const savedQueryEndpoint = 'glob:*/api/v1/saved_query/?*'; +const recentActivityEndpoint = 'glob:*/api/v1/log/recent_activity/*'; + +jest.mock('antd', () => mockAntdWithDesktopBreakpoint()); + +// The delete flow lives in the tables; Home only receives `onActivityRefresh`. +// Stub the tables with a button that simulates a completed delete. +jest.mock('src/features/home/DashboardTable', () => ({ + __esModule: true, + default: ({ onActivityRefresh }: { onActivityRefresh?: () => void }) => ( + + ), +})); +jest.mock('src/features/home/ChartTable', () => ({ + __esModule: true, + default: () =>
, +})); + +const user = { + username: 'alpha', + firstName: 'alpha', + lastName: 'alpha', + createdOn: '2016-11-11T12:34:17', + userId: 5, + email: 'alpha@alpha.com', + isActive: true, + isAnonymous: false, + permissions: {}, + roles: { sql_lab: [['can_read', 'SavedQuery']] }, +}; + +const recentItem = (title: string, id: number) => ({ + action: 'dashboard', + item_title: title, + item_type: 'dashboard', + item_url: `/superset/dashboard/${id}/`, + time: 1741644942130 + id, + time_delta_humanized: 'a day ago', +}); + +let recentResponder: () => unknown; + +beforeAll(() => { + SupersetClient.configure().init(); +}); + +beforeEach(() => { + localStorage.clear(); + // Start with the Recents and Dashboards panels expanded. + setItem(LocalStorageKeys.HomepageCollapseState, ['recents', 'dashboards']); + fetchMock.get(chartsEndpoint, { result: [] }); + fetchMock.get(dashboardsEndpoint, { result: [] }); + fetchMock.get(savedQueryEndpoint, { result: [] }); + fetchMock.get(recentActivityEndpoint, () => recentResponder()); +}); + +afterEach(() => { + fetchMock.removeRoutes(); + fetchMock.clearHistory(); +}); + +const renderHome = async () => { + render(, { useRedux: true, useRouter: true }); + return screen.findByRole('button', { name: 'simulate delete' }); +}; + +test('delete refetches recents without resetting the selected tab', async () => { + recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); + const deleteButton = await renderHome(); + expect(await screen.findByText('Viewed dash')).toBeInTheDocument(); + expect(fetchMock.callHistory.calls(recentActivityEndpoint)).toHaveLength(1); + const dashboardCallsBefore = + fetchMock.callHistory.calls(dashboardsEndpoint).length; + + // Switch away from the default (Viewed) tab; a refresh must not undo this. + await userEvent.click(screen.getByText('Created')); + await waitFor(() => + expect(screen.queryByText('Viewed dash')).not.toBeInTheDocument(), + ); + + // Drop the persisted tab so only the in-memory selection survives; a + // refresh that re-ran the first-load tab restore would flip back to Viewed. + localStorage.removeItem(LocalStorageKeys.HomepageActivityFilter); + + recentResponder = () => ({ result: [recentItem('Fresh dash', 2)] }); + await userEvent.click(deleteButton); + + await waitFor(() => + expect(fetchMock.callHistory.calls(recentActivityEndpoint)).toHaveLength(2), + ); + await waitFor(() => + expect( + fetchMock.callHistory.calls(dashboardsEndpoint).length, + ).toBeGreaterThan(dashboardCallsBefore), + ); + // Still on the Created tab: neither the old nor the refreshed Viewed items + // are shown, and the collapse panels were not reset. + expect(screen.queryByText('Viewed dash')).not.toBeInTheDocument(); + expect(screen.queryByText('Fresh dash')).not.toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'simulate delete' }), + ).toBeInTheDocument(); + + await userEvent.click(screen.getByText('Viewed')); + expect(await screen.findByText('Fresh dash')).toBeInTheDocument(); +}); + +test('an older in-flight refresh cannot repopulate a deleted item', async () => { + recentResponder = () => ({ + result: [recentItem('Doomed dash', 1), recentItem('Kept dash', 2)], + }); + const deleteButton = await renderHome(); + expect(await screen.findByText('Doomed dash')).toBeInTheDocument(); + + // Defer the next two recents responses so they can resolve out of order. + const resolvers: Array<(value: unknown) => void> = []; + recentResponder = () => + new Promise(resolve => { + resolvers.push(resolve); + }); + + await userEvent.click(deleteButton); + await waitFor(() => expect(resolvers).toHaveLength(1)); + await userEvent.click(deleteButton); + await waitFor(() => expect(resolvers).toHaveLength(2)); + + // The newer refresh (post-delete data) resolves first... + resolvers[1]({ result: [recentItem('Kept dash', 2)] }); + await waitFor(() => + expect(screen.queryByText('Doomed dash')).not.toBeInTheDocument(), + ); + expect(screen.getByText('Kept dash')).toBeInTheDocument(); + + // ...then the older, stale response arrives and must be discarded. + resolvers[0]({ + result: [recentItem('Doomed dash', 1), recentItem('Kept dash', 2)], + }); + await new Promise(resolve => { + setTimeout(resolve, 50); + }); + expect(screen.queryByText('Doomed dash')).not.toBeInTheDocument(); + expect(screen.getByText('Kept dash')).toBeInTheDocument(); +}); From b63264c229a11299d42379ba88cdef68a14919cf Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Fri, 23 Oct 2026 22:35:39 -0700 Subject: [PATCH 09/12] test(home): cover Mine-tab delete refresh, Edited refetch and collapse state on refresh Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5.5 --- .../src/features/home/ChartTable.test.tsx | 47 ++++++++++++ .../src/pages/Home/Home.refresh.test.tsx | 76 ++++++++++++++++++- 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/features/home/ChartTable.test.tsx b/superset-frontend/src/features/home/ChartTable.test.tsx index c93360c252ee..2d9aaff37a68 100644 --- a/superset-frontend/src/features/home/ChartTable.test.tsx +++ b/superset-frontend/src/features/home/ChartTable.test.tsx @@ -251,3 +251,50 @@ test('refreshes other tab data after deleting a chart', async () => { expect(screen.getByText('cool chart 2')).toBeInTheDocument(); expect(onActivityRefresh).toHaveBeenCalledTimes(1); }); + +test('refreshes recent activity only after a Mine tab chart delete completes', async () => { + // Restore the full chart list the previous test replaced. + fetchMock.removeRoute(chartsEndpoint); + fetchMock.get( + chartsEndpoint, + { result: mockCharts }, + { name: chartsEndpoint }, + ); + let resolveDelete: (value: unknown) => void = () => {}; + fetchMock.delete( + 'glob:*/api/v1/chart/1', + () => + new Promise(resolve => { + resolveDelete = resolve; + }), + ); + + const onActivityRefresh = jest.fn(); + await renderChartTable({ ...mineTabProps, onActivityRefresh }); + await userEvent.click(screen.getByText(/mine/i)); + await waitFor(() => { + expect(screen.getAllByText(/cool chart/i)).toHaveLength(3); + }); + onActivityRefresh.mockClear(); + + const moreButtons = screen.getAllByRole('img', { name: /more/i }); + await userEvent.click(moreButtons[1]); + await userEvent.click(await screen.findByText('Delete')); + await userEvent.type(screen.getByTestId('delete-modal-input'), 'DELETE'); + await userEvent.click(screen.getByTestId('modal-confirm-button')); + + await waitFor(() => { + expect( + fetchMock.callHistory.calls(/api\/v1\/chart\/1/, { method: 'DELETE' }), + ).toHaveLength(1); + }); + expect(onActivityRefresh).not.toHaveBeenCalled(); + + await act(async () => { + resolveDelete({ message: 'Chart deleted' }); + }); + + await waitFor(() => { + expect(onActivityRefresh).toHaveBeenCalledTimes(1); + }); +}); diff --git a/superset-frontend/src/pages/Home/Home.refresh.test.tsx b/superset-frontend/src/pages/Home/Home.refresh.test.tsx index b1b5700ee5cf..5b7e0f54cc70 100644 --- a/superset-frontend/src/pages/Home/Home.refresh.test.tsx +++ b/superset-frontend/src/pages/Home/Home.refresh.test.tsx @@ -73,7 +73,15 @@ const recentItem = (title: string, id: number) => ({ time_delta_humanized: 'a day ago', }); +const editedDashboard = (title: string, id: number) => ({ + id, + dashboard_title: title, + url: `/superset/dashboard/${id}/`, + changed_on_delta_humanized: 'a day ago', +}); + let recentResponder: () => unknown; +let editedDashboards: ReturnType[] = []; beforeAll(() => { SupersetClient.configure().init(); @@ -84,7 +92,10 @@ beforeEach(() => { // Start with the Recents and Dashboards panels expanded. setItem(LocalStorageKeys.HomepageCollapseState, ['recents', 'dashboards']); fetchMock.get(chartsEndpoint, { result: [] }); - fetchMock.get(dashboardsEndpoint, { result: [] }); + editedDashboards = []; + fetchMock.get(dashboardsEndpoint, ({ url }) => + url.includes('changed_by') ? { result: editedDashboards } : { result: [] }, + ); fetchMock.get(savedQueryEndpoint, { result: [] }); fetchMock.get(recentActivityEndpoint, () => recentResponder()); }); @@ -176,3 +187,66 @@ test('an older in-flight refresh cannot repopulate a deleted item', async () => expect(screen.queryByText('Doomed dash')).not.toBeInTheDocument(); expect(screen.getByText('Kept dash')).toBeInTheDocument(); }); + +test('delete refetches the Edited tab so a removed item disappears', async () => { + recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); + editedDashboards = [ + editedDashboard('Doomed edited dash', 1), + editedDashboard('Kept edited dash', 2), + ]; + const deleteButton = await renderHome(); + expect(await screen.findByText('Viewed dash')).toBeInTheDocument(); + + await userEvent.click(screen.getByText('Edited')); + expect(await screen.findByText('Doomed edited dash')).toBeInTheDocument(); + expect(screen.getByText('Kept edited dash')).toBeInTheDocument(); + + // The deleted dashboard no longer comes back from the edited-objects query. + editedDashboards = [editedDashboard('Kept edited dash', 2)]; + await userEvent.click(deleteButton); + + await waitFor(() => + expect(screen.queryByText('Doomed edited dash')).not.toBeInTheDocument(), + ); + expect(screen.getByText('Kept edited dash')).toBeInTheDocument(); +}); + +test('delete refresh keeps a collapsed Dashboards panel collapsed', async () => { + recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); + const deleteButton = await renderHome(); + expect(await screen.findByText('Viewed dash')).toBeInTheDocument(); + + // Collapse Dashboards while keeping Recents open. + const dashboardsHeader = screen.getByRole('button', { name: /Dashboards/ }); + expect(dashboardsHeader).toHaveAttribute('aria-expanded', 'true'); + await userEvent.click(dashboardsHeader); + await waitFor(() => + expect(screen.getByRole('button', { name: /Dashboards/ })).toHaveAttribute( + 'aria-expanded', + 'false', + ), + ); + + // Drop the persisted collapse state so only the in-memory selection + // survives; re-running the first-load restore would fall back to the + // defaults and re-expand Dashboards. + localStorage.removeItem(LocalStorageKeys.HomepageCollapseState); + + await userEvent.click(deleteButton); + await waitFor(() => + expect(fetchMock.callHistory.calls(recentActivityEndpoint)).toHaveLength(2), + ); + await new Promise(resolve => { + setTimeout(resolve, 50); + }); + + expect(screen.getByRole('button', { name: /Dashboards/ })).toHaveAttribute( + 'aria-expanded', + 'false', + ); + expect(screen.getByRole('button', { name: /Recents/ })).toHaveAttribute( + 'aria-expanded', + 'true', + ); + expect(screen.getByText('Viewed dash')).toBeInTheDocument(); +}); From c5204cd64824735e1c204f138e8fda076dd7b003 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 29 Oct 2026 10:51:38 -0700 Subject: [PATCH 10/12] test(home): cover stale-refresh rejection and editable-object ordering, drain async work instead of sleeping Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5.5 --- .../src/pages/Home/Home.refresh.test.tsx | 167 +++++++++++++++--- 1 file changed, 141 insertions(+), 26 deletions(-) diff --git a/superset-frontend/src/pages/Home/Home.refresh.test.tsx b/superset-frontend/src/pages/Home/Home.refresh.test.tsx index 5b7e0f54cc70..e99c8b81600b 100644 --- a/superset-frontend/src/pages/Home/Home.refresh.test.tsx +++ b/superset-frontend/src/pages/Home/Home.refresh.test.tsx @@ -21,11 +21,14 @@ import { mockAntdWithDesktopBreakpoint } from 'spec/helpers/mobileTestUtils'; import fetchMock from 'fetch-mock'; import { SupersetClient } from '@superset-ui/core'; import { + act, + createStore, render, screen, userEvent, waitFor, } from 'spec/helpers/testing-library'; +import reducerIndex from 'spec/helpers/reducerIndex'; import Welcome from 'src/pages/Home'; import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers'; @@ -40,10 +43,21 @@ jest.mock('antd', () => mockAntdWithDesktopBreakpoint()); // Stub the tables with a button that simulates a completed delete. jest.mock('src/features/home/DashboardTable', () => ({ __esModule: true, - default: ({ onActivityRefresh }: { onActivityRefresh?: () => void }) => ( - + default: ({ + onActivityRefresh, + mine, + }: { + onActivityRefresh?: () => void; + mine?: { id: number; dashboard_title: string }[]; + }) => ( + <> + + {mine?.map(d => ( + {`mine: ${d.dashboard_title}`} + ))} + ), })); jest.mock('src/features/home/ChartTable', () => ({ @@ -80,7 +94,31 @@ const editedDashboard = (title: string, id: number) => ({ changed_on_delta_humanized: 'a day ago', }); +type Deferred = { + resolve: (value: unknown) => void; + reject: (reason: Error) => void; +}; + +// Returns a response promise that the test settles by hand, so requests can +// resolve (or fail) out of order. +const deferResponse = (pending: Deferred[]) => + new Promise((resolve, reject) => { + pending.push({ resolve, reject }); + }); + +// Waits for the requests a just-settled response kicked off (the chained +// chart/dashboard fetches) and for the resulting React updates to finish. +const drainPendingWork = async () => { + await act(async () => { + await fetchMock.callHistory.flush(true); + }); + await act(async () => { + await fetchMock.callHistory.flush(true); + }); +}; + let recentResponder: () => unknown; +let editableDashboardsResponder: () => unknown; let editedDashboards: ReturnType[] = []; beforeAll(() => { @@ -93,9 +131,15 @@ beforeEach(() => { setItem(LocalStorageKeys.HomepageCollapseState, ['recents', 'dashboards']); fetchMock.get(chartsEndpoint, { result: [] }); editedDashboards = []; - fetchMock.get(dashboardsEndpoint, ({ url }) => - url.includes('changed_by') ? { result: editedDashboards } : { result: [] }, - ); + editableDashboardsResponder = () => ({ result: [] }); + fetchMock.get(dashboardsEndpoint, ({ url }) => { + if (url.includes('is_editable')) { + return editableDashboardsResponder(); + } + return url.includes('changed_by') + ? { result: editedDashboards } + : { result: [] }; + }); fetchMock.get(savedQueryEndpoint, { result: [] }); fetchMock.get(recentActivityEndpoint, () => recentResponder()); }); @@ -106,13 +150,17 @@ afterEach(() => { }); const renderHome = async () => { - render(, { useRedux: true, useRouter: true }); - return screen.findByRole('button', { name: 'simulate delete' }); + const store = createStore({}, reducerIndex); + render(, { store, useRouter: true }); + const deleteButton = await screen.findByRole('button', { + name: 'simulate delete', + }); + return { deleteButton, store }; }; test('delete refetches recents without resetting the selected tab', async () => { recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); - const deleteButton = await renderHome(); + const { deleteButton } = await renderHome(); expect(await screen.findByText('Viewed dash')).toBeInTheDocument(); expect(fetchMock.callHistory.calls(recentActivityEndpoint)).toHaveLength(1); const dashboardCallsBefore = @@ -155,46 +203,113 @@ test('an older in-flight refresh cannot repopulate a deleted item', async () => recentResponder = () => ({ result: [recentItem('Doomed dash', 1), recentItem('Kept dash', 2)], }); - const deleteButton = await renderHome(); + const { deleteButton } = await renderHome(); expect(await screen.findByText('Doomed dash')).toBeInTheDocument(); // Defer the next two recents responses so they can resolve out of order. - const resolvers: Array<(value: unknown) => void> = []; - recentResponder = () => - new Promise(resolve => { - resolvers.push(resolve); - }); + const pending: Deferred[] = []; + recentResponder = () => deferResponse(pending); await userEvent.click(deleteButton); - await waitFor(() => expect(resolvers).toHaveLength(1)); + await waitFor(() => expect(pending).toHaveLength(1)); await userEvent.click(deleteButton); - await waitFor(() => expect(resolvers).toHaveLength(2)); + await waitFor(() => expect(pending).toHaveLength(2)); // The newer refresh (post-delete data) resolves first... - resolvers[1]({ result: [recentItem('Kept dash', 2)] }); + pending[1].resolve({ result: [recentItem('Kept dash', 2)] }); await waitFor(() => expect(screen.queryByText('Doomed dash')).not.toBeInTheDocument(), ); expect(screen.getByText('Kept dash')).toBeInTheDocument(); + await drainPendingWork(); - // ...then the older, stale response arrives and must be discarded. - resolvers[0]({ + // ...then the older, stale response arrives and must be discarded. Wait for + // the chart/dashboard requests it chains, then let React settle. + const chainedCallsBefore = + fetchMock.callHistory.calls(chartsEndpoint).length; + pending[0].resolve({ result: [recentItem('Doomed dash', 1), recentItem('Kept dash', 2)], }); - await new Promise(resolve => { - setTimeout(resolve, 50); - }); + await waitFor(() => + expect(fetchMock.callHistory.calls(chartsEndpoint).length).toBeGreaterThan( + chainedCallsBefore, + ), + ); + await drainPendingWork(); expect(screen.queryByText('Doomed dash')).not.toBeInTheDocument(); expect(screen.getByText('Kept dash')).toBeInTheDocument(); }); +test('a stale refresh that fails cannot blank current items or toast', async () => { + recentResponder = () => ({ result: [recentItem('Kept dash', 2)] }); + const { deleteButton, store } = await renderHome(); + expect(await screen.findByText('Kept dash')).toBeInTheDocument(); + + const recents: Deferred[] = []; + const editable: Deferred[] = []; + recentResponder = () => deferResponse(recents); + editableDashboardsResponder = () => deferResponse(editable); + + // Refresh A starts, then refresh B supersedes it. + await userEvent.click(deleteButton); + await waitFor(() => expect(recents).toHaveLength(1)); + await userEvent.click(deleteButton); + await waitFor(() => expect(recents).toHaveLength(2)); + await waitFor(() => expect(editable).toHaveLength(2)); + + // B succeeds and renders the current items. + recents[1].resolve({ result: [recentItem('Kept dash', 2)] }); + editable[1].resolve({ result: [editedDashboard('Current dash', 2)] }); + expect(await screen.findByText('mine: Current dash')).toBeInTheDocument(); + await drainPendingWork(); + + // A then fails: its guards must keep B's data and suppress the toast. + recents[0].reject(new Error('stale recents failure')); + editable[0].reject(new Error('stale editable failure')); + await drainPendingWork(); + + expect(screen.getByText('Kept dash')).toBeInTheDocument(); + expect(screen.getByText('mine: Current dash')).toBeInTheDocument(); + expect( + (store.getState() as { messageToasts: unknown[] }).messageToasts, + ).toEqual([]); +}); + +test('out-of-order editable-object responses keep the newest data', async () => { + recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); + editableDashboardsResponder = () => ({ + result: [editedDashboard('Doomed dash', 1)], + }); + const { deleteButton } = await renderHome(); + expect(await screen.findByText('mine: Doomed dash')).toBeInTheDocument(); + + const editable: Deferred[] = []; + editableDashboardsResponder = () => deferResponse(editable); + + await userEvent.click(deleteButton); + await waitFor(() => expect(editable).toHaveLength(1)); + await userEvent.click(deleteButton); + await waitFor(() => expect(editable).toHaveLength(2)); + + // The newer refresh resolves first with post-delete data... + editable[1].resolve({ result: [editedDashboard('Kept dash', 2)] }); + expect(await screen.findByText('mine: Kept dash')).toBeInTheDocument(); + await drainPendingWork(); + + // ...then the older one resolves with pre-delete data and must be dropped. + editable[0].resolve({ result: [editedDashboard('Doomed dash', 1)] }); + await drainPendingWork(); + expect(screen.queryByText('mine: Doomed dash')).not.toBeInTheDocument(); + expect(screen.getByText('mine: Kept dash')).toBeInTheDocument(); +}); + test('delete refetches the Edited tab so a removed item disappears', async () => { recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); editedDashboards = [ editedDashboard('Doomed edited dash', 1), editedDashboard('Kept edited dash', 2), ]; - const deleteButton = await renderHome(); + const { deleteButton } = await renderHome(); expect(await screen.findByText('Viewed dash')).toBeInTheDocument(); await userEvent.click(screen.getByText('Edited')); @@ -213,7 +328,7 @@ test('delete refetches the Edited tab so a removed item disappears', async () => test('delete refresh keeps a collapsed Dashboards panel collapsed', async () => { recentResponder = () => ({ result: [recentItem('Viewed dash', 1)] }); - const deleteButton = await renderHome(); + const { deleteButton } = await renderHome(); expect(await screen.findByText('Viewed dash')).toBeInTheDocument(); // Collapse Dashboards while keeping Recents open. From eaa66c24b8e36878f5d22c30482d76d739a5aa6e Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Thu, 29 Oct 2026 17:48:36 -0700 Subject: [PATCH 11/12] test(home): drain refresh work by event-loop turns, fix store cast and formatting Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5.5 --- .../src/pages/Home/Home.refresh.test.tsx | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/pages/Home/Home.refresh.test.tsx b/superset-frontend/src/pages/Home/Home.refresh.test.tsx index e99c8b81600b..67b0cf35c2ed 100644 --- a/superset-frontend/src/pages/Home/Home.refresh.test.tsx +++ b/superset-frontend/src/pages/Home/Home.refresh.test.tsx @@ -106,15 +106,19 @@ const deferResponse = (pending: Deferred[]) => pending.push({ resolve, reject }); }); -// Waits for the requests a just-settled response kicked off (the chained -// chart/dashboard fetches) and for the resulting React updates to finish. +// Lets the work a just-settled response kicked off (chained chart/dashboard +// fetches and the resulting React updates) run to completion. It yields whole +// event-loop turns rather than awaiting `fetchMock.callHistory.flush`, which +// would never return while another deferred response is still unsettled. const drainPendingWork = async () => { - await act(async () => { - await fetchMock.callHistory.flush(true); - }); - await act(async () => { - await fetchMock.callHistory.flush(true); - }); + for (let turn = 0; turn < 5; turn += 1) { + // eslint-disable-next-line no-await-in-loop + await act(async () => { + await new Promise(resolve => { + setTimeout(resolve, 0); + }); + }); + } }; let recentResponder: () => unknown; @@ -225,8 +229,7 @@ test('an older in-flight refresh cannot repopulate a deleted item', async () => // ...then the older, stale response arrives and must be discarded. Wait for // the chart/dashboard requests it chains, then let React settle. - const chainedCallsBefore = - fetchMock.callHistory.calls(chartsEndpoint).length; + const chainedCallsBefore = fetchMock.callHistory.calls(chartsEndpoint).length; pending[0].resolve({ result: [recentItem('Doomed dash', 1), recentItem('Kept dash', 2)], }); @@ -271,7 +274,7 @@ test('a stale refresh that fails cannot blank current items or toast', async () expect(screen.getByText('Kept dash')).toBeInTheDocument(); expect(screen.getByText('mine: Current dash')).toBeInTheDocument(); expect( - (store.getState() as { messageToasts: unknown[] }).messageToasts, + (store.getState() as unknown as { messageToasts: unknown[] }).messageToasts, ).toEqual([]); }); From 26e5340ecdf0ca08ebfe3878a72ebc2e3ebbe634 Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Fri, 30 Oct 2026 18:14:41 -0700 Subject: [PATCH 12/12] test(home): add Playwright E2E coverage for Recents refresh after delete Adds recent-activity-refresh.spec.ts, covering all six combinations of {Favorite, Mine, All} x {Dashboards, Charts}: create a throwaway asset, view it to seed a real Recents "Viewed" entry, delete it from the given tab, and assert the Recents entry disappears promptly rather than lingering. This was prompted by a video-capture run that appeared to show a stale Recents entry surviving 15+ seconds after a Mine-tab dashboard delete. A from-scratch repro (fresh SQLite DB, fresh backend/webpack, no leftover state) could not reproduce it: the entry disappeared immediately, across dashboards and charts, on every tab. These tests pin that correct behavior down as a regression guard, since nothing previously covered it end-to-end in a real browser. Extends HomePage with helpers for the Dashboards/Charts card lists (tab switching, card lookup scoped to the right panel, kebab-menu delete) and the Recents panel, reused across all six tests. Co-Authored-By: Evan Rusackas Co-Authored-By: Claude Sonnet 5 --- .../playwright/pages/HomePage.ts | 106 +++++++++ .../home/recent-activity-refresh.spec.ts | 203 ++++++++++++++++++ 2 files changed, 309 insertions(+) create mode 100644 superset-frontend/playwright/tests/home/recent-activity-refresh.spec.ts diff --git a/superset-frontend/playwright/pages/HomePage.ts b/superset-frontend/playwright/pages/HomePage.ts index d8ce8314e6c0..38ff047bd59b 100644 --- a/superset-frontend/playwright/pages/HomePage.ts +++ b/superset-frontend/playwright/pages/HomePage.ts @@ -20,9 +20,21 @@ import { Locator, Page } from '@playwright/test'; import { gotoWithRetry } from '../helpers/navigation'; import { URL } from '../utils/urls'; +import { DeleteConfirmationModal } from '../components/modals'; + +/** The two card lists on Home that this page object can drive. */ +export type HomeAssetKind = 'Dashboards' | 'Charts'; + +/** The three list tabs shared by both the Dashboards and Charts sections. */ +export type HomeListTab = 'Favorite' | 'Mine' | 'All'; /** * Home/welcome page object. + * + * Covers the "Recents" activity panel and the Dashboards/Charts card lists, + * including the card kebab menu's delete (or, with SOFT_DELETE on, archive) + * action -- used to verify that deleting a dashboard/chart promptly refreshes + * the Recents panel (see recent-activity-refresh.spec.ts). */ export class HomePage { private readonly page: Page; @@ -48,4 +60,98 @@ export class HomePage { getRecentsSection(): Locator { return this.page.getByText(HomePage.SECTION_NAMES.RECENTS, { exact: true }); } + + /** + * Expands the Recents panel if it is currently collapsed. + * + * Collapse state persists across sessions (it is saved to localStorage), + * so a prior run/test can leave it collapsed, hiding its content -- and + * with it, the "Viewed" tab {@link getRecentsViewedCount} depends on. + */ + async expandRecents(): Promise { + const header = this.page.getByRole('button', { name: 'Recents' }); + if ((await header.getAttribute('aria-expanded')) === 'false') { + await header.click(); + } + } + + /** + * Gets the collapsible panel for a given card list ("Dashboards" or + * "Charts"). Scoping to this panel (rather than matching card/tab text + * loosely on the whole page) is what keeps lookups from also matching the + * visually-similar cards in the Recents panel above it. + */ + getPanel(kind: HomeAssetKind): Locator { + return this.page + .locator('.ant-collapse-item') + .filter({ has: this.page.getByText(kind, { exact: true }) }); + } + + /** + * Switches a card list's Favorite/Mine/All tab. + */ + async switchTab(kind: HomeAssetKind, tab: HomeListTab): Promise { + await this.getPanel(kind) + .getByRole('tab', { name: tab, exact: true }) + .click(); + } + + /** + * Gets a specific card by its title within a card list panel. + */ + getCard(kind: HomeAssetKind, title: string): Locator { + return this.getPanel(kind) + .locator('[data-test="styled-card"]') + .filter({ hasText: title }); + } + + /** + * Gets the count of a title's occurrences within the Recents "Viewed" tab + * specifically -- used to assert that a just-deleted item's Recents entry + * has (or has not yet) disappeared. + */ + async getRecentsViewedCount(title: string): Promise { + const recentsPanel = this.page + .locator('.ant-collapse-item') + .filter({ has: this.getRecentsSection() }); + const viewedTab = recentsPanel.getByRole('tab', { + name: 'Viewed', + exact: true, + }); + if (await viewedTab.count()) { + await viewedTab.click(); + } + return recentsPanel.getByText(title, { exact: true }).count(); + } + + /** + * Deletes (or, with SOFT_DELETE on, archives) a card via its kebab menu. + * + * The kebab button is only revealed on hover, so a plain `.click()` -- no + * actionability wait sees it as interactable until the hover state is + * actually entered -- needs a real `.hover()` first, not `force: true`. + * + * @param kind - Which card list the card belongs to. + * @param title - The card's title. + * @param menuTestId - The kebab menu's `data-test` (differs between + * dashboard and chart cards). + * @param deleteOptionTestId - The delete/archive menu item's `data-test` + * (also differs between dashboard and chart cards). + */ + async deleteCardViaMenu( + kind: HomeAssetKind, + title: string, + menuTestId: string, + deleteOptionTestId: string, + ): Promise { + const card = this.getCard(kind, title).first(); + await card.hover(); + await card.locator(`[data-test="${menuTestId}"]`).click(); + await this.page.locator(`[data-test="${deleteOptionTestId}"]`).click(); + + const deleteModal = new DeleteConfirmationModal(this.page); + await deleteModal.waitForVisible(); + await deleteModal.confirmDeletion(); + await deleteModal.waitForHidden(); + } } diff --git a/superset-frontend/playwright/tests/home/recent-activity-refresh.spec.ts b/superset-frontend/playwright/tests/home/recent-activity-refresh.spec.ts new file mode 100644 index 000000000000..696e00d197ce --- /dev/null +++ b/superset-frontend/playwright/tests/home/recent-activity-refresh.spec.ts @@ -0,0 +1,203 @@ +/** + * 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. + */ + +/** + * Regression coverage for the Home page's "Recents" panel after a + * dashboard/chart is deleted from the Dashboards or Charts card list. + * + * Before this fix, deleting an item from some tabs (notably Mine) could + * leave its stale "Viewed" entry sitting in Recents after the delete had + * already succeeded, which reads as if the delete silently failed. Each + * test here creates a throwaway dashboard/chart, views it to seed a real + * Recents entry, deletes it from a specific tab, and asserts the Recents + * entry disappears promptly rather than polling/sleeping for a fixed + * window -- a regression that reintroduces staleness should fail here + * instead of flaking. + */ +import { testWithAssets, expect } from '../../helpers/fixtures'; +import { apiPostDashboard } from '../../helpers/api/dashboard'; +import { apiPostChart } from '../../helpers/api/chart'; +import { getDatasetByName } from '../../helpers/api/dataset'; +import { apiPost } from '../../helpers/api/requests'; +import { DashboardPage } from '../../pages/DashboardPage'; +import { ExplorePage } from '../../pages/ExplorePage'; +import { HomePage, HomeListTab } from '../../pages/HomePage'; +import { Toast } from '../../components/core'; +import { TIMEOUT } from '../../utils/constants'; + +// Uncomment to force video recording when running this file locally (e.g. to +// produce a demo recording). CI relies on the 'retain-on-failure' policy in +// playwright.config.ts instead, so this stays off for the contributed test. +// testWithAssets.use({ video: 'on' }); + +const DATASET_NAME = 'birth_names'; + +/** Short, readable, still worker-unique: no epoch timestamps in the title. */ +function assetName(prefix: string, testInfo: { parallelIndex: number }) { + return `${prefix}_${testInfo.parallelIndex}`; +} + +for (const tab of ['Favorite', 'Mine', 'All'] as HomeListTab[]) { + testWithAssets( + `deleting a dashboard from the ${tab} tab refreshes Recents`, + async ({ page, testAssets }, testInfo) => { + const name = assetName(`PR_Dash_${tab}`, testInfo); + + const createResp = await apiPostDashboard(page, { + dashboard_title: name, + }); + expect(createResp.ok()).toBe(true); + const { id: dashboardId } = await createResp.json(); + testAssets.trackDashboard(dashboardId); + + if (tab === 'Favorite') { + const favResp = await apiPost( + page, + `api/v1/dashboard/${dashboardId}/favorites/`, + ); + expect(favResp.ok()).toBe(true); + } + + // View it so a "mount_dashboard" log entry seeds a real Recents card. + // Logging is debounced client-side (loggerMiddleware batches events up + // to 1s before sending), so wait for the actual beacon request rather + // than assuming it has landed once the page itself has loaded. + const dashboardPage = new DashboardPage(page); + const logRequest = page.waitForResponse( + resp => + resp.request().method() === 'POST' && + resp.url().includes('/log/') && + (resp.request().postData() ?? '').includes('mount_dashboard'), + { timeout: TIMEOUT.API_RESPONSE }, + ); + await dashboardPage.gotoById(dashboardId); + await dashboardPage.waitForLoad(); + await logRequest; + + const homePage = new HomePage(page); + await homePage.goto(); + await homePage.expandRecents(); + + await expect(async () => { + expect(await homePage.getRecentsViewedCount(name)).toBeGreaterThan(0); + }).toPass({ timeout: TIMEOUT.API_RESPONSE }); + + await homePage.switchTab('Dashboards', tab); + await expect(homePage.getCard('Dashboards', name)).toBeVisible(); + + await homePage.deleteCardViaMenu( + 'Dashboards', + name, + 'dashboard-card-menu', + 'dashboard-card-option-delete-button', + ); + + const toast = new Toast(page); + await expect(toast.getSuccess()).toBeVisible(); + + await expect(homePage.getCard('Dashboards', name)).toHaveCount(0); + await expect(async () => { + expect(await homePage.getRecentsViewedCount(name)).toBe(0); + }).toPass({ timeout: TIMEOUT.API_RESPONSE }); + }, + ); +} + +for (const tab of ['Favorite', 'Mine', 'All'] as HomeListTab[]) { + testWithAssets( + `deleting a chart from the ${tab} tab refreshes Recents`, + async ({ page, testAssets }, testInfo) => { + testWithAssets.setTimeout(TIMEOUT.SLOW_TEST); + const name = assetName(`PR_Chart_${tab}`, testInfo); + + const dataset = await getDatasetByName(page, DATASET_NAME); + if (!dataset) { + throw new Error(`Dataset ${DATASET_NAME} not found`); + } + + const createResp = await apiPostChart(page, { + slice_name: name, + viz_type: 'table', + datasource_id: dataset.id, + datasource_type: 'table', + params: JSON.stringify({ + viz_type: 'table', + datasource: `${dataset.id}__table`, + query_mode: 'aggregate', + groupby: ['name'], + metrics: ['count'], + adhoc_filters: [], + row_limit: 5, + }), + }); + expect(createResp.ok()).toBe(true); + const chartBody = await createResp.json(); + const chartId: number = chartBody.result?.id ?? chartBody.id; + testAssets.trackChart(chartId); + + if (tab === 'Favorite') { + const favResp = await apiPost( + page, + `api/v1/chart/${chartId}/favorites/`, + ); + expect(favResp.ok()).toBe(true); + } + + // View it in Explore so a "mount_explorer" log entry seeds a real + // Recents card. See the dashboard test above for why this waits for + // the actual (debounced) log beacon rather than just the page load. + const explorePage = new ExplorePage(page); + const logRequest = page.waitForResponse( + resp => + resp.request().method() === 'POST' && + resp.url().includes('/log/') && + (resp.request().postData() ?? '').includes('mount_explorer'), + { timeout: TIMEOUT.API_RESPONSE }, + ); + await explorePage.goto(chartId); + await logRequest; + + const homePage = new HomePage(page); + await homePage.goto(); + await homePage.expandRecents(); + + await expect(async () => { + expect(await homePage.getRecentsViewedCount(name)).toBeGreaterThan(0); + }).toPass({ timeout: TIMEOUT.API_RESPONSE }); + + await homePage.switchTab('Charts', tab); + await expect(homePage.getCard('Charts', name)).toBeVisible(); + + await homePage.deleteCardViaMenu( + 'Charts', + name, + 'chart-card-menu', + 'chart-list-delete-option', + ); + + const toast = new Toast(page); + await expect(toast.getSuccess()).toBeVisible(); + + await expect(homePage.getCard('Charts', name)).toHaveCount(0); + await expect(async () => { + expect(await homePage.getRecentsViewedCount(name)).toBe(0); + }).toPass({ timeout: TIMEOUT.API_RESPONSE }); + }, + ); +}