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 }); + }, + ); +} 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/ChartTable.test.tsx b/superset-frontend/src/features/home/ChartTable.test.tsx index b6c5509cc4ac..e9d4a8c456fa 100644 --- a/superset-frontend/src/features/home/ChartTable.test.tsx +++ b/superset-frontend/src/features/home/ChartTable.test.tsx @@ -210,9 +210,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(); @@ -249,4 +251,52 @@ 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); +}); + +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/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx index 2829634c362d..1207b9c9f333 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( @@ -94,6 +96,10 @@ function ChartTable({ [], false, ); + const handleRefreshData = (config?: Parameters[0]) => { + refreshData(config); + onActivityRefresh?.(); + }; const chartIds = useMemo(() => charts.map(c => c.id), [charts]); const [saveFavoriteStatus, favoriteStatus] = useFavoriteStatus( @@ -125,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) { @@ -232,10 +242,10 @@ function ChartTable({ hasPerm={hasPerm} showThumbnails={showThumbnails} bulkSelectEnabled={bulkSelectEnabled} - refreshData={refreshData} + 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 1b1cdfd6c1dc..39aa6ab84a6a 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,20 @@ test('handles dashboard deletion confirmation', async () => { toggleBulkSelect: jest.fn(), })); + // 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( @@ -410,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(); @@ -418,6 +443,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 () => { @@ -443,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( @@ -515,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: [], @@ -522,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/features/home/DashboardTable.tsx b/superset-frontend/src/features/home/DashboardTable.tsx index 3f3daa2dc952..238ac98028ff 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( @@ -134,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); @@ -260,12 +274,12 @@ function DashboardTable({ onConfirm={() => { handleDashboardDelete( dashboardToDelete, - refreshData, + handleRefreshData, addSuccessToast, addDangerToast, activeTab, user?.userId, - getData, + handleGetData, ); setDashboardToDelete(null); }} 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..67b0cf35c2ed --- /dev/null +++ b/superset-frontend/src/pages/Home/Home.refresh.test.tsx @@ -0,0 +1,370 @@ +/** + * 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 { + 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'; + +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, + mine, + }: { + onActivityRefresh?: () => void; + mine?: { id: number; dashboard_title: string }[]; + }) => ( + <> + + {mine?.map(d => ( + {`mine: ${d.dashboard_title}`} + ))} + + ), +})); +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', +}); + +const editedDashboard = (title: string, id: number) => ({ + id, + dashboard_title: title, + url: `/superset/dashboard/${id}/`, + 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 }); + }); + +// 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 () => { + 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; +let editableDashboardsResponder: () => unknown; +let editedDashboards: ReturnType[] = []; + +beforeAll(() => { + SupersetClient.configure().init(); +}); + +beforeEach(() => { + localStorage.clear(); + // Start with the Recents and Dashboards panels expanded. + setItem(LocalStorageKeys.HomepageCollapseState, ['recents', 'dashboards']); + fetchMock.get(chartsEndpoint, { result: [] }); + editedDashboards = []; + 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()); +}); + +afterEach(() => { + fetchMock.removeRoutes(); + fetchMock.clearHistory(); +}); + +const renderHome = async () => { + 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(); + 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 pending: Deferred[] = []; + recentResponder = () => deferResponse(pending); + + await userEvent.click(deleteButton); + await waitFor(() => expect(pending).toHaveLength(1)); + await userEvent.click(deleteButton); + await waitFor(() => expect(pending).toHaveLength(2)); + + // The newer refresh (post-delete data) resolves first... + 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. 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 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 unknown 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(); + 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(); +}); diff --git a/superset-frontend/src/pages/Home/index.tsx b/superset-frontend/src/pages/Home/index.tsx index 82ff403b81e2..f45fbf35fafe 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'); @@ -214,28 +219,42 @@ 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); + // 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; const data: ActivityData | null = {}; data[TableTab.Other] = res.other; 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( createErrorHandler((errMsg: unknown) => { + if (isStale) return; setActivityData(activityData => ({ ...activityData, [TableTab.Viewed]: [], @@ -257,44 +276,56 @@ 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); }); - }, [otherTabFilters]); + + return () => { + isStale = true; + }; + }, [otherTabFilters, refreshKey]); const handleToggle = () => { setChecked(!checked); @@ -379,6 +410,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { setActiveChild={setActiveChild} activityData={activityData} isFetchingActivityData={isFetchingActivityData} + refreshKey={refreshKey} /> ) : ( @@ -398,6 +430,7 @@ function Welcome({ user, addDangerToast }: WelcomeProps) { otherTabData={activityData?.[TableTab.Other]} otherTabFilters={otherTabFilters} otherTabTitle={otherTabTitle} + onActivityRefresh={handleRefreshActivity} /> ), }, @@ -418,6 +451,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..c06bbdefdcec 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 {