fix: view error handling #2813
fix: view error handling #2813
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds refresh metadata to ViewResult, surfaces refresh errors in SingleView via a dialog and adjusted error/loading branches, and refactors useViewData to compute a central query key and perform selective query invalidation for specific view sections. Changes
Sequence DiagramsequenceDiagram
participant User
participant SingleView
participant useViewData
participant QueryClient
User->>SingleView: Request refresh (force refresh)
SingleView->>useViewData: handleForceRefresh()
activate useViewData
useViewData->>useViewData: Compute viewQueryKey
useViewData->>useViewData: Read latest result -> currentName/currentNamespace
useViewData->>useViewData: Compute refsToInvalidate
useViewData->>QueryClient: Invalidate keys for refsToInvalidate (view-result, view-table, view-variables)
deactivate useViewData
QueryClient->>useViewData: Trigger refetches
useViewData->>SingleView: Return updated viewResult (may include refreshStatus/refreshError/responseSource)
alt refreshStatus == "error" and responseSource == "cache"
SingleView->>SingleView: open refresh error Dialog (shows refreshError)
SingleView->>User: display dialog + cached content
else normal refresh
SingleView->>User: display refreshed content
end
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)src/pages/views/components/SingleView.tsx (3)
src/pages/views/hooks/useViewData.ts (2)
🔇 Additional comments (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/pages/views/hooks/useViewData.ts:
- Around line 125-134: The callback that computes currentNamespace/currentName
and refsToInvalidate uses viewResult as a fallback but viewResult is missing
from the useCallback dependency array; update the dependency arrays for the
callback(s) that reference currentNamespace/currentName/refsToInvalidate (and
the similar callback around lines 155-163) to include viewResult so the closure
sees updated viewResult values, ensuring sectionsToRefresh, result, and
viewResult are all listed as dependencies alongside any other existing deps.
🧹 Nitpick comments (2)
src/pages/views/components/SingleView.tsx (2)
35-39: Dialog doesn't auto-close on successful refresh.The
useEffectonly opens the dialog when an error occurs but doesn't close it when the refresh eventually succeeds (i.e., whenrefreshErrorbecomesundefined). This is likely fine since users can dismiss manually, but for better UX you may want to auto-close.🔎 Optional: Auto-close dialog on success
useEffect(() => { if (refreshError && isCachedResponse) { setRefreshErrorOpen(true); + } else if (!refreshError) { + setRefreshErrorOpen(false); } }, [refreshError, isCachedResponse, viewResult?.requestFingerprint]);
62-65: Consider using a shared spinner component.The inline spinner implementation works, but if the codebase has a shared
SpinnerorLoadingcomponent, using it would improve consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/audit-report/types/index.tssrc/pages/views/components/SingleView.tsxsrc/pages/views/hooks/useViewData.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/views/hooks/useViewData.ts (1)
src/query-client.ts (1)
queryClient(6-26)
src/pages/views/components/SingleView.tsx (3)
src/components/ErrorViewer.tsx (1)
ErrorViewer(174-201)src/components/ui/dialog.tsx (5)
Dialog(110-110)DialogContent(115-115)DialogHeader(116-116)DialogTitle(118-118)DialogDescription(119-119)src/ui/Age/Age.tsx (1)
Age(24-105)
🔇 Additional comments (5)
src/pages/audit-report/types/index.ts (1)
436-439: LGTM! Well-structured refresh metadata fields.The new optional fields cleanly extend
ViewResultfor refresh error handling. The separation betweenrefreshStatus(operation outcome) andresponseSource(data origin) allows the UI to distinguish between a failed refresh that returns cached data vs. a successful cache hit.src/pages/views/hooks/useViewData.ts (2)
52-54: Clean refactor of query key computation.Extracting
viewQueryKeyinto a dedicated variable improves readability and makes the conditional logic easier to follow.
142-154: Selective invalidation logic looks correct.The change to invalidate only section refs that don't match the current view is a good optimization—it avoids redundant invalidation of the query that was just refetched.
src/pages/views/components/SingleView.tsx (2)
41-84: Well-structured error and loading state handling.The guard clauses properly handle the error, loading, and not-found states before rendering the main content. The order is correct: error state takes precedence, then loading, then not-found.
88-126: Refresh error dialog implementation looks good.The dialog correctly displays the error message with context about cached data being shown. Using
requestFingerprintin the effect dependency ensures the dialog responds to new requests.
adityathebe
force-pushed
the
fix/view-error
branch
from
c6f846d to
5688cc2
Compare
resolves: flanksource/mission-control#2597
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.