fix(dashboard): prevent stale favorite status errors after navigation#38156
fix(dashboard): prevent stale favorite status errors after navigation#38156
Conversation
|
Related Documentation Checked 0 published document(s) in 2 knowledge base(s). No updates required. How did I do? Any feedback? |
Code Review Agent Run #bbdc61Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Sequence DiagramThe PR updates favorite-status thunks to ignore API responses unless the requested dashboard ID still matches the currently viewed dashboard, preventing stale state updates or error toasts after navigation. sequenceDiagram
participant Dashboard UI
participant Thunk
participant Superset API
participant Redux Store
Dashboard UI->>Thunk: fetchFaveStar(id) / saveFaveStar(id, isStarred)
Thunk->>Superset API: call favorite_status or favorites endpoint
Superset API-->>Thunk: response (success or error)
alt response for current dashboard (ids match)
Thunk->>Redux Store: dispatch state update or addDangerToast
Redux Store-->>Dashboard UI: updated favorite state or error toast
else stale response (ids differ)
Thunk-->>Thunk: silently ignore response (no dispatch)
Note right of Thunk: prevents toasts/state changes for old dashboard
end
Generated by CodeAnt AI |
Code Review Agent Run #d52feeActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
There was a problem hiding this comment.
Pull request overview
This PR fixes a race condition where navigating between dashboards could show error toasts for dashboards the user is no longer viewing. The fix prevents stale API responses from affecting the UI by checking if the requested dashboard ID matches the currently viewed dashboard ID before dispatching actions or showing error messages.
Changes:
- Modified
fetchFaveStarthunk to check if the dashboard ID is still current before dispatching actions or error toasts - Modified
saveFaveStarthunk with the same stale response protection - Both functions now accept
getStateparameter to access current dashboard state
| export function fetchFaveStar(id: number) { | ||
| return function fetchFaveStarThunk(dispatch: AppDispatch) { | ||
| return function fetchFaveStarThunk( | ||
| dispatch: AppDispatch, | ||
| getState: GetState, | ||
| ) { | ||
| return SupersetClient.get({ | ||
| endpoint: `/api/v1/dashboard/favorite_status/?q=${rison.encode([id])}`, | ||
| }) | ||
| .then(({ json }: { json: JsonObject }) => { | ||
| dispatch(toggleFaveStar(!!(json?.result as JsonObject[])?.[0]?.value)); | ||
| // Only update state if this is still the current dashboard | ||
| // This prevents stale responses from affecting the UI after navigation | ||
| const currentId = getState().dashboardInfo?.id; | ||
| if (currentId === id) { | ||
| dispatch( | ||
| toggleFaveStar(!!(json?.result as JsonObject[])?.[0]?.value), | ||
| ); | ||
| } | ||
| }) | ||
| .catch(() => | ||
| dispatch( | ||
| addDangerToast( | ||
| t( | ||
| 'There was an issue fetching the favorite status of this dashboard.', | ||
| .catch(() => { | ||
| // Only show error if this is still the current dashboard | ||
| // This prevents error toasts from appearing for dashboards the user | ||
| // has already navigated away from (e.g., deleted dashboards) | ||
| const currentId = getState().dashboardInfo?.id; | ||
| if (currentId === id) { | ||
| dispatch( | ||
| addDangerToast( | ||
| t( | ||
| 'There was an issue fetching the favorite status of this dashboard.', | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| ); | ||
| } | ||
| }); | ||
| }; | ||
| } |
There was a problem hiding this comment.
The race condition fix in fetchFaveStar should be covered by tests to ensure the stale response handling works correctly. The test should verify that when the dashboard ID changes between the API call and the response, no state updates or error toasts are dispatched. This is a critical behavior change that prevents user-facing bugs.
Consider adding test cases that:
- Verify actions are dispatched when dashboard ID matches
- Verify actions are NOT dispatched when dashboard ID changes (the race condition scenario)
- Cover both success and error response paths
The file superset-frontend/src/dashboard/actions/dashboardState.test.ts already has comprehensive tests for other thunks like saveDashboardRequest and would be the appropriate location for these tests.
| @@ -190,13 +206,21 @@ export function saveFaveStar(id: number, isStarred: boolean) { | |||
|
|
|||
| return apiCall | |||
| .then(() => { | |||
| dispatch(toggleFaveStar(!isStarred)); | |||
| // Only update state if this is still the current dashboard | |||
| const currentId = getState().dashboardInfo?.id; | |||
| if (currentId === id) { | |||
| dispatch(toggleFaveStar(!isStarred)); | |||
| } | |||
| }) | |||
| .catch(() => | |||
| dispatch( | |||
| addDangerToast(t('There was an issue favoriting this dashboard.')), | |||
| ), | |||
| ); | |||
| .catch(() => { | |||
| // Only show error if this is still the current dashboard | |||
| const currentId = getState().dashboardInfo?.id; | |||
| if (currentId === id) { | |||
| dispatch( | |||
| addDangerToast(t('There was an issue favoriting this dashboard.')), | |||
| ); | |||
| } | |||
| }); | |||
| }; | |||
| } | |||
There was a problem hiding this comment.
The race condition fix in saveFaveStar should be covered by tests to ensure the stale response handling works correctly. The test should verify that when the dashboard ID changes between the API call and the response, no state updates or error toasts are dispatched. This is a critical behavior change that prevents user-facing bugs.
Consider adding test cases that:
- Verify actions are dispatched when dashboard ID matches
- Verify actions are NOT dispatched when dashboard ID changes (the race condition scenario)
- Cover both success and error response paths
The file superset-frontend/src/dashboard/actions/dashboardState.test.ts already has comprehensive tests for other thunks like saveDashboardRequest and would be the appropriate location for these tests.
When navigating between dashboards, the previous dashboard's favorite_status API call could complete after the new dashboard loaded. If the previous dashboard had been deleted, this would show an error toast for a dashboard the user is no longer viewing. This fix checks if the requested dashboard ID still matches the current dashboard before dispatching actions (updating state or showing errors). This prevents stale responses from affecting the UI after navigation. Fixes #32533 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rusackas
force-pushed
the
fix/stale-dashboard-favorite-status-32533
branch
from
3fa9597 to
62e06ec
Compare
Sequence DiagramShows how favorite-status thunks now verify the current dashboard ID from Redux state before updating UI or showing error toasts, preventing stale API responses from affecting a newly viewed dashboard. sequenceDiagram
participant DashboardView
participant ReduxThunk
participant SupersetAPI
participant ReduxState
DashboardView->>ReduxThunk: fetchFaveStar(id) / saveFaveStar(id, ...)
ReduxThunk->>SupersetAPI: call favorite_status or favorites endpoint
SupersetAPI-->>ReduxThunk: response (success or error)
ReduxThunk->>ReduxState: read dashboardInfo.id (currentId)
alt currentId === requested id
ReduxThunk-->>DashboardView: dispatch toggleFaveStar or addDangerToast
else stale response (IDs differ)
ReduxThunk-->>ReduxState: silently ignore response (no dispatch)
end
Generated by CodeAnt AI |
Code Review Agent Run #e6c081Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
1 participant
SUMMARY
Fixes a race condition where navigating between dashboards could show error toasts for dashboards the user is no longer viewing.
The Problem:
When a user:
The
favorite_statusAPI call for the deleted dashboard (258) would complete and show an error toast: "There was an issue fetching the favorite status of this dashboard." This is confusing because the user is now viewing a different dashboard.Root Cause:
The
fetchFaveStarandsaveFaveStarRedux thunks would dispatch actions (including error toasts) regardless of whether the requested dashboard ID still matched the currently viewed dashboard. This allowed stale API responses to affect the UI after navigation.The Fix:
Before dispatching any actions in the thunk callbacks, check if
dashboardInfo.idfrom the current Redux state matches theidthat was originally requested. If they don't match, the response is stale and should be silently ignored.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
When navigating from a deleted dashboard to another dashboard, error toasts would appear for the deleted dashboard.
After:
Stale responses are silently ignored, preventing confusing error messages.
TESTING INSTRUCTIONS
Alternative test (without deletion):
ADDITIONAL INFORMATION
🤖 Generated with Claude Code