From 54d277985bbabac9b990e744d5cbdfc59f0a15a4 Mon Sep 17 00:00:00 2001 From: Arunodoy18 Date: Wed, 28 Jan 2026 01:14:52 +0530 Subject: [PATCH 1/2] fix(dashboard): handle favorite fetch for deleted dashboards --- .../src/components/FaveStar/index.tsx | 6 +- .../src/dashboard/actions/dashboardState.js | 20 ++++++- .../dashboard/actions/dashboardState.test.js | 59 +++++++++++++++++++ .../src/dashboard/components/Header/index.jsx | 7 ++- 4 files changed, 87 insertions(+), 5 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/components/FaveStar/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/FaveStar/index.tsx index 504341f60d0c..050fba3127aa 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/FaveStar/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/FaveStar/index.tsx @@ -42,7 +42,11 @@ export const FaveStar = ({ }: FaveStarProps) => { const theme = useTheme(); useEffect(() => { - fetchFaveStar?.(itemId); + // Only attempt to fetch favorite status if itemId is valid + // This prevents unnecessary API calls for deleted or invalid items + if (itemId && fetchFaveStar) { + fetchFaveStar(itemId); + } }, [fetchFaveStar, itemId]); const onClick = useCallback( diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 182b4d951284..3105db3e4ddb 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -113,15 +113,29 @@ export function fetchFaveStar(id) { .then(({ json }) => { dispatch(toggleFaveStar(!!json?.result?.[0]?.value)); }) - .catch(() => + .catch(async error => { + const errorObject = await getClientErrorObject(error); + + // HTTP 404: Dashboard no longer exists (expected case after deletion) + // This is a normal scenario when navigating after a dashboard has been deleted. + // Skipping error toast to prevent user-facing errors for expected behavior. + if (errorObject.status === 404) { + logging.debug( + `Favorite status fetch returned 404 for dashboard ID ${id}. ` + + `Dashboard may have been deleted. Skipping error notification.` + ); + return; + } + + // All other errors are unexpected and should be reported to the user dispatch( addDangerToast( t( 'There was an issue fetching the favorite status of this dashboard.', ), ), - ), - ); + ); + }); }; } diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js index a305a1209060..9fadbd25dea6 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.test.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.test.js @@ -24,6 +24,8 @@ import { SAVE_DASHBOARD_STARTED, saveDashboardRequest, SET_OVERRIDE_CONFIRM, + fetchFaveStar, + TOGGLE_FAVE_STAR, } from 'src/dashboard/actions/dashboardState'; import { UPDATE_COMPONENTS_PARENTS_LIST } from 'src/dashboard/actions/dashboardLayout'; import { @@ -234,4 +236,61 @@ describe('dashboardState actions', () => { ); }); }); + + // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks + describe('fetchFaveStar', () => { + test('dispatches TOGGLE_FAVE_STAR on successful fetch', async () => { + const dashboardId = 123; + const dispatch = sinon.stub(); + + getStub.restore(); + getStub = sinon.stub(SupersetClient, 'get').resolves({ + json: { + result: [{ id: dashboardId, value: true }], + }, + }); + + const thunk = fetchFaveStar(dashboardId); + await thunk(dispatch); + + await waitFor(() => expect(dispatch.callCount).toBe(1)); + expect(dispatch.getCall(0).args[0].type).toBe(TOGGLE_FAVE_STAR); + expect(dispatch.getCall(0).args[0].isStarred).toBe(true); + }); + + test('does not dispatch error toast on 404 response', async () => { + const dashboardId = 999; + const dispatch = sinon.stub(); + + getStub.restore(); + const error404 = new Error('Not found'); + error404.status = 404; + getStub = sinon.stub(SupersetClient, 'get').rejects(error404); + + const thunk = fetchFaveStar(dashboardId); + await thunk(dispatch); + + // Should not dispatch any action (no toast, no toggle) + expect(dispatch.callCount).toBe(0); + }); + + test('dispatches error toast on non-404 errors', async () => { + const dashboardId = 123; + const dispatch = sinon.stub(); + + getStub.restore(); + const error500 = new Error('Internal server error'); + error500.status = 500; + getStub = sinon.stub(SupersetClient, 'get').rejects(error500); + + const thunk = fetchFaveStar(dashboardId); + await thunk(dispatch); + + await waitFor(() => expect(dispatch.callCount).toBe(1)); + // Verify error toast action was dispatched (ADD_TOAST with danger type) + const dispatchedAction = dispatch.getCall(0).args[0]; + expect(dispatchedAction.type).toBe('ADD_TOAST'); + expect(dispatchedAction.payload.toastType).toBe('danger'); + }); + }); }); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index d7c262c6584f..278038ac2a55 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -592,7 +592,12 @@ const Header = () => { const faveStarProps = useMemo( () => ({ itemId: dashboardInfo.id, - fetchFaveStar: boundActionCreators.fetchFaveStar, + // Only fetch favorite status for valid, existing dashboards + // Skip fetch entirely if dashboard ID is falsy to prevent + // 404 errors when navigating after dashboard deletion + fetchFaveStar: dashboardInfo.id + ? boundActionCreators.fetchFaveStar + : undefined, saveFaveStar: boundActionCreators.saveFaveStar, isStarred, showTooltip: true, From b423288ea75fb6fa2fefe636b795a4c8d18eda06 Mon Sep 17 00:00:00 2001 From: Arunodoy18 Date: Thu, 29 Jan 2026 14:27:39 +0530 Subject: [PATCH 2/2] test(dashboard): fix favorite fetch test mocks and assertions --- .../src/dashboard/actions/dashboardState.js | 4 +-- .../dashboard/actions/dashboardState.test.js | 25 +++++++++++++------ .../src/dashboard/components/Header/index.jsx | 6 ++--- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 3105db3e4ddb..3161f302145e 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -115,14 +115,14 @@ export function fetchFaveStar(id) { }) .catch(async error => { const errorObject = await getClientErrorObject(error); - + // HTTP 404: Dashboard no longer exists (expected case after deletion) // This is a normal scenario when navigating after a dashboard has been deleted. // Skipping error toast to prevent user-facing errors for expected behavior. if (errorObject.status === 404) { logging.debug( `Favorite status fetch returned 404 for dashboard ID ${id}. ` + - `Dashboard may have been deleted. Skipping error notification.` + `Dashboard may have been deleted. Skipping error notification.`, ); return; } diff --git a/superset-frontend/src/dashboard/actions/dashboardState.test.js b/superset-frontend/src/dashboard/actions/dashboardState.test.js index 9fadbd25dea6..5d2c4d4f0306 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.test.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.test.js @@ -27,6 +27,7 @@ import { fetchFaveStar, TOGGLE_FAVE_STAR, } from 'src/dashboard/actions/dashboardState'; +import { ADD_TOAST } from 'src/components/MessageToasts/actions'; import { UPDATE_COMPONENTS_PARENTS_LIST } from 'src/dashboard/actions/dashboardLayout'; import { DASHBOARD_GRID_ID, @@ -261,10 +262,15 @@ describe('dashboardState actions', () => { test('does not dispatch error toast on 404 response', async () => { const dashboardId = 999; const dispatch = sinon.stub(); - + getStub.restore(); - const error404 = new Error('Not found'); - error404.status = 404; + const error404 = { + response: { + status: 404, + statusText: 'Not Found', + bodyUsed: false, + }, + }; getStub = sinon.stub(SupersetClient, 'get').rejects(error404); const thunk = fetchFaveStar(dashboardId); @@ -277,10 +283,15 @@ describe('dashboardState actions', () => { test('dispatches error toast on non-404 errors', async () => { const dashboardId = 123; const dispatch = sinon.stub(); - + getStub.restore(); - const error500 = new Error('Internal server error'); - error500.status = 500; + const error500 = { + response: { + status: 500, + statusText: 'Internal Server Error', + bodyUsed: false, + }, + }; getStub = sinon.stub(SupersetClient, 'get').rejects(error500); const thunk = fetchFaveStar(dashboardId); @@ -289,7 +300,7 @@ describe('dashboardState actions', () => { await waitFor(() => expect(dispatch.callCount).toBe(1)); // Verify error toast action was dispatched (ADD_TOAST with danger type) const dispatchedAction = dispatch.getCall(0).args[0]; - expect(dispatchedAction.type).toBe('ADD_TOAST'); + expect(dispatchedAction.type).toBe(ADD_TOAST); expect(dispatchedAction.payload.toastType).toBe('danger'); }); }); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 278038ac2a55..b36e673bd7b5 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -592,9 +592,9 @@ const Header = () => { const faveStarProps = useMemo( () => ({ itemId: dashboardInfo.id, - // Only fetch favorite status for valid, existing dashboards - // Skip fetch entirely if dashboard ID is falsy to prevent - // 404 errors when navigating after dashboard deletion + // Only fetch favorite status when dashboard ID exists. + // Falsy IDs indicate the dashboard hasn't been created yet or the component + // is unmounting. The fetchFaveStar action handles 404s for deleted dashboards. fetchFaveStar: dashboardInfo.id ? boundActionCreators.fetchFaveStar : undefined,