Conversation
Signed-off-by: Simo <simo@6529.io>
WalkthroughAdds auth-aware loading, error/timeout, retry and reauth flows to Notifications; propagates AbortController signals through notifications queries and prefetch; centralizes ProfileActivity target-type enum into Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Notifications UI
participant Auth as AuthContext / useAuth
participant Query as useNotificationsQuery
participant API as Notifications API
UI->>Auth: Check isAuthenticated / identity / activeProfileProxy
alt Not authenticated
UI-->>UI: Render "Connect wallet" state
else Authenticated
UI->>Query: useInfiniteQuery({ identity, cause, ... }, signal)
Query->>API: fetchNotifications(params, signal)
API-->>Query: Data or Error
alt Success
Query-->>UI: pages, hasNextPage
UI-->>UI: Render list / load more
else Unauthorized (401 / "unauthorized")
Query-->>UI: Error
UI->>Auth: requestAuth()
alt Reauth success
UI->>Query: refetch()
else Reauth failed
UI-->>UI: Show error toast
end
else Other error
Query-->>UI: Error
UI-->>UI: Show error toast
end
par Loading timeout
UI-->>UI: After LOAD_TIMEOUT_MS show timeout toast
end
end
sequenceDiagram
autonumber
participant Prefetch as usePrefetchNotifications
participant API as Notifications API
Prefetch->>API: fetchNotifications({ pageParam?, signal })
Note over Prefetch,API: AbortController signal is propagated to support cancellation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursorrules)
Files:
🧬 Code graph analysis (1)components/brain/notifications/Notifications.tsx (8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (18)
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: 0
🧹 Nitpick comments (6)
hooks/useNotificationsQuery.tsx (4)
41-46: Add readonly modifiers to props interface.To follow repo guidelines, mark hook props as readonly.
[As per coding guidelines]
-interface UseNotificationsQueryProps { - /** - * If true, reverse the notifications order (e.g. for a "descending" / "newest first" display). - */ - reverse?: boolean; +interface UseNotificationsQueryProps { + /** + * If true, reverse the notifications order (e.g. for a "descending" / "newest first" display). + */ + readonly reverse?: boolean; @@ - identity?: string | null; + readonly identity?: string | null; @@ - activeProfileProxy?: boolean; + readonly activeProfileProxy?: boolean; @@ - limit?: string; + readonly limit?: string; @@ - cause?: ApiNotificationCause[] | null; + readonly cause?: ApiNotificationCause[] | null; }
114-126: Harden unauthorized detection in retry policy.Message matching can miss 401s without “unauthorized” text. Also check status fields when present.
- retry: (failureCount, error: unknown) => { - if (typeof error === "string") { - if (error.toLowerCase().includes("unauthorized")) { - return false; - } - } else if (error instanceof Error) { - if (/unauthorized/i.test(error.message)) { - return false; - } - } - return failureCount < 3; - }, + retry: (failureCount, error: unknown) => { + const status = + (error as any)?.status ?? + (error as any)?.response?.status ?? + (error as any)?.cause?.status; + if (status === 401) return false; + if (typeof error === "string" && /unauthorized/i.test(error)) return false; + if (error instanceof Error && /unauthorized/i.test(error.message)) return false; + return failureCount < 3; + },
166-175: Also pass AbortSignal in prefetchInfiniteQuery.Prefetch queryFns receive a signal too; propagate it so prefetches can be canceled.
- queryClient.prefetchInfiniteQuery({ + queryClient.prefetchInfiniteQuery({ queryKey: getIdentityNotificationsQueryKey(identity, limit, cause), - queryFn: ({ pageParam }: { pageParam?: number | null }) => - fetchNotifications({ limit, cause, pageParam }), + queryFn: ({ + pageParam, + signal, + }: { + pageParam?: number | null; + signal?: AbortSignal; + }) => fetchNotifications({ limit, cause, pageParam, signal }), initialPageParam: null,
62-64: Use null-safe check for pageParam.Avoid skipping id 0 (if ever possible) by checking nullish.
- if (pageParam) { + if (pageParam != null) { params.id_less_than = String(pageParam); }__tests__/components/brain/notifications/Notifications.test.tsx (1)
112-124: Add tests for unauthorized error and timeout states.Cover that:
- On unauthorized queryError, requestAuth is triggered and an error toast is shown.
- When loading exceeds the timeout, the timeout message/toast renders with “Try again” action.
I can draft two tests with mocked queryError and timed waitFor advancing timers if you want.
Also applies to: 134-146, 155-166
components/brain/notifications/Notifications.tsx (1)
138-157: Broaden unauthorized detection to include HTTP status.Relying on message text can miss 401s. Consider reading status fields when available.
- const getErrorDetails = (error: unknown) => { - if (error instanceof Error) { - const message = error.message?.trim() || DEFAULT_ERROR_MESSAGE; - return { - message, - isUnauthorized: /unauthorized/i.test(message), - }; - } - if (typeof error === "string") { - const message = error.trim() || DEFAULT_ERROR_MESSAGE; - return { - message, - isUnauthorized: /unauthorized/i.test(message), - }; - } - return { - message: DEFAULT_ERROR_MESSAGE, - isUnauthorized: false, - }; - }; + const getErrorDetails = (error: unknown) => { + const status = + (error as any)?.status ?? + (error as any)?.response?.status ?? + (error as any)?.cause?.status; + if (error instanceof Error) { + const message = error.message?.trim() || DEFAULT_ERROR_MESSAGE; + return { message, isUnauthorized: status === 401 || /unauthorized/i.test(message) }; + } + if (typeof error === "string") { + const message = error.trim() || DEFAULT_ERROR_MESSAGE; + return { message, isUnauthorized: status === 401 || /unauthorized/i.test(message) }; + } + return { message: DEFAULT_ERROR_MESSAGE, isUnauthorized: status === 401 }; + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
__tests__/components/brain/notifications/Notifications.test.tsx(6 hunks)components/brain/notifications/Notifications.tsx(10 hunks)hooks/useNotificationsQuery.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsFollow existing code style and naming conventions
Files:
components/brain/notifications/Notifications.tsxhooks/useNotificationsQuery.tsx__tests__/components/brain/notifications/Notifications.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/brain/notifications/Notifications.tsxhooks/useNotificationsQuery.tsx__tests__/components/brain/notifications/Notifications.test.tsx
__tests__/**
📄 CodeRabbit inference engine (tests/AGENTS.md)
Place Jest test suites under the
__tests__directory mirroring source folders (e.g., components, contexts, hooks, utils)
Files:
__tests__/components/brain/notifications/Notifications.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/brain/notifications/Notifications.test.tsx
{**/__tests__/**,**/*.test.{ts,tsx}}
📄 CodeRabbit inference engine (AGENTS.md)
{**/__tests__/**,**/*.test.{ts,tsx}}: If coverage on a modified file is below 80%, add meaningful tests to raise it to at least 80%
Mock external dependencies and APIs in tests
Files:
__tests__/components/brain/notifications/Notifications.test.tsx
**/__tests__/**
📄 CodeRabbit inference engine (AGENTS.md)
Place tests in
__tests__/directories when organizing standalone test suites
Files:
__tests__/components/brain/notifications/Notifications.test.tsx
**/*.test.tsx
📄 CodeRabbit inference engine (AGENTS.md)
When co-locating tests with components, name them
ComponentName.test.tsx
Files:
__tests__/components/brain/notifications/Notifications.test.tsx
🧬 Code graph analysis (1)
components/brain/notifications/Notifications.tsx (3)
components/auth/Auth.tsx (1)
AuthContext(83-93)components/brain/notifications/NotificationsCauseFilter.tsx (2)
NotificationFilter(8-11)NotificationsCauseFilter(31-137)hooks/useNotificationsQuery.tsx (1)
useNotificationsQuery(77-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
hooks/useNotificationsQuery.tsx (2)
54-60: AbortSignal propagation looks correct.Forwarding signal into commonApiFetch enables proper cancellation. Good addition.
Also applies to: 70-75
101-110: Good useInfiniteQuery signature with AbortSignal.Correct v5 object-form and signal passthrough.
__tests__/components/brain/notifications/Notifications.test.tsx (1)
5-7: Mocks and auth wiring look consistent.The new mocks (requestAuth, setActiveProfileProxy, setToast) and resets align with the component’s auth-aware behavior.
Also applies to: 21-37, 105-110
components/brain/notifications/Notifications.tsx (5)
41-49: Auth/context state integration LGTM.Clean separation of auth flags, refs, and local error/timeout state.
Also applies to: 56-67
248-272: Error effect behavior is solid.Single-shot toasts and gated re-auth trigger prevent loops. Good.
273-304: Timeout handling is clear and user-friendly.Nice separation of timer and toast effects; respects auth/profile states.
Also applies to: 305-318
338-347: Retry, re-auth, and proxy-switch handlers LGTM.Handlers reset local state, call refetch with cancel, and surface errors via toast.
Also applies to: 348-358, 359-371
372-388: UI state gating and conditional rendering read well.Loading vs error vs empty vs data paths are mutually exclusive; filter shown only when authenticated.
Also applies to: 516-543, 571-576
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/brain/notifications/Notifications.tsx (1)
37-59: Consider adding aria-label to the action button.The action button would benefit from an
aria-labelattribute to provide context for screen reader users, especially since the label might be generic like "Try again" without surrounding context being read.Apply this diff:
<button type="button" onClick={action.handler} + aria-label={`${message} ${action.label}`} className="tw-inline-flex tw-items-center tw-justify-center tw-rounded-lg tw-border tw-border-iron-500 tw-bg-transparent tw-px-4 tw-py-2 tw-text-sm tw-font-medium tw-text-iron-100 desktop-hover:hover:tw-bg-iron-800 focus:tw-outline-none focus:tw-ring-2 focus:tw-ring-iron-300"> {action.label} </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/brain/notifications/Notifications.tsx(10 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsFollow existing code style and naming conventions
Files:
components/brain/notifications/Notifications.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/brain/notifications/Notifications.tsx
🧬 Code graph analysis (1)
components/brain/notifications/Notifications.tsx (8)
types/feed.types.ts (1)
TypedNotification(135-143)types/dropInteractionTypes.ts (1)
ActiveDropState(8-12)components/common/SpinnerLoader.tsx (1)
SpinnerLoader(8-57)components/brain/my-stream/layout/MyStreamNoItems.tsx (1)
MyStreamNoItems(4-126)components/brain/notifications/NotificationsWrapper.tsx (1)
NotificationsWrapper(21-79)components/auth/Auth.tsx (1)
AuthContext(83-93)components/brain/notifications/NotificationsCauseFilter.tsx (2)
NotificationFilter(8-11)NotificationsCauseFilter(31-137)hooks/useNotificationsQuery.tsx (1)
useNotificationsQuery(77-148)
🔇 Additional comments (17)
components/brain/notifications/Notifications.tsx (17)
1-29: LGTM!The imports are well-organized and follow TypeScript best practices by using
typeimports for type-only dependencies.
31-35: LGTM!The timeout and error message constants are well-defined and provide clear user feedback.
61-156: LGTM!The content resolution logic is well-structured with clear state prioritization. The function correctly handles all authentication and error states with appropriate user messaging and actionable recovery options.
163-200: LGTM!The component setup is well-organized with appropriate use of refs to track transient states (toasts shown, re-auth triggered) and correctly computed derived values for authentication and loading states.
204-240: LGTM!The mark-as-read logic correctly guards against duplicate calls and properly resets state when authentication status changes.
242-258: LGTM!The query hook is correctly configured with conditional identity to gate notifications loading on authentication state.
260-286: LGTM!The error normalization function defensively handles multiple error shapes (Error instances, strings, unknown objects) and correctly identifies unauthorized conditions for triggering re-authentication.
288-326: LGTM!The reload handling correctly guards on authentication state and properly cleans up the URL parameter in all paths (unauthenticated, success, and error).
328-344: LGTM!The fetch-older callback correctly guards on authentication and pagination state, preventing unnecessary or duplicate fetches.
346-375: LGTM!The scroll initialization effects correctly handle initial positioning, empty states, and filter changes to maintain consistent scroll behavior.
377-446: LGTM!The error and timeout handling effects are well-structured with appropriate guards, cleanup logic, and use of refs to prevent duplicate toasts. The timeout mechanism provides good user feedback for slow loading scenarios.
448-465: LGTM!The scroll position maintenance during prepending is correctly implemented to prevent jarring jumps when older notifications are loaded.
467-499: LGTM!The retry handlers provide comprehensive error recovery options with proper state cleanup and user feedback through error toasts.
501-521: LGTM!The computed state variables are well-defined with clear boolean logic that correctly coordinates authentication checks, error states, timeout conditions, and data availability to determine the appropriate UI state.
523-594: LGTM!The stick-to-bottom behavior is robustly implemented with fallback from ResizeObserver to MutationObserver, proper RAF scheduling, and careful cleanup. The dependencies correctly include state variables that affect when the sticky behavior should apply.
596-626: LGTM!The scroll handler correctly manages pinned-to-bottom state and infinite scroll triggering, with proper guards based on authentication and UI state.
628-666: LGTM!The render logic is clean and correctly gates the cause filter on authentication state while passing all necessary parameters to the content resolver.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
components/brain/notifications/Notifications.tsx (4)
179-182: Enhance error handling: gate toasts/re-auth per unique error.Currently, error toast and re-auth trigger only once until queryError becomes null. If subsequent errors differ, users won’t see updated messages or re-auth. Track last error message and reset flags when it changes.
const errorToastShownRef = useRef(false); const reauthTriggeredRef = useRef(false); const timeoutToastShownRef = useRef(false); + const lastErrorMessageRef = useRef<string | null>(null);
378-402: Reset per error message before toasting/reauth.Apply the new lastErrorMessageRef to avoid suppressing new error messages.
- const { message, isUnauthorized } = getErrorDetails(queryError); + const { message, isUnauthorized } = getErrorDetails(queryError); + if (lastErrorMessageRef.current !== message) { + errorToastShownRef.current = false; + reauthTriggeredRef.current = false; + lastErrorMessageRef.current = message; + }
595-595: Observer effect deps include UI flags.This recreates observers more often. Optional: depend only on items and container existence; still schedule stick if flags change via implicit mutations.
21-27: Type-only imports (optional).Imports ActiveDropState and NotificationFilter are types; consider using import type to improve treeshaking and clarity.
-import { ActiveDropState } from "@/types/dropInteractionTypes"; +import type { ActiveDropState } from "@/types/dropInteractionTypes"; @@ -import NotificationsCauseFilter, { - NotificationFilter, -} from "./NotificationsCauseFilter"; +import NotificationsCauseFilter, { type NotificationFilter } from "./NotificationsCauseFilter";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/brain/notifications/Notifications.tsx(10 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsFollow existing code style and naming conventions
Files:
components/brain/notifications/Notifications.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/brain/notifications/Notifications.tsx
🧬 Code graph analysis (1)
components/brain/notifications/Notifications.tsx (8)
types/feed.types.ts (1)
TypedNotification(135-143)types/dropInteractionTypes.ts (1)
ActiveDropState(8-12)components/common/SpinnerLoader.tsx (1)
SpinnerLoader(8-57)components/brain/my-stream/layout/MyStreamNoItems.tsx (1)
MyStreamNoItems(4-126)components/brain/notifications/NotificationsWrapper.tsx (1)
NotificationsWrapper(21-79)components/auth/Auth.tsx (1)
AuthContext(83-93)components/brain/notifications/NotificationsCauseFilter.tsx (2)
NotificationFilter(8-11)NotificationsCauseFilter(31-137)hooks/useNotificationsQuery.tsx (1)
useNotificationsQuery(77-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
components/brain/notifications/Notifications.tsx (17)
32-36: Good: Clear UX messages and timeout guard.LOAD_TIMEOUT_MS and user-facing copy improve reliability and UX.
62-78: Props immutability looks good.readonly applied consistently to props.
80-157: Solid, explicit render-state gating.Clear separation of loading/auth/proxy/error/empty states; reduces conditional clutter in main render.
165-173: Auth-context usage is appropriate.Pulls only needed fields; keeps component concerns focused.
188-190: Good: Local UI state for timeout/error.Keeps query layer decoupled from presentation layer.
196-200: Auth derivations are correct and readable.isAuthenticated excludes proxy usage; separate flags make render logic simpler.
223-236: Mark-all-as-read guard is correct.Runs once per auth session and respects reload param. Good error isolation.
237-242: Good: Reset one-shot flag on de-auth.Prevents stale hasMarkedAllAsRead across auth transitions.
251-259: Query usage is correct; auth-gated identity.react-query integration aligns with guidelines; reverse ordering handled in hook.
261-287: Normalize errors; consider broadening unauthorized detection.getErrorDetails is good. Verify if backend ever returns 403 for expired/invalid auth; if yes, treat it as non-retriable/reauth as well.
Would you like me to scan the repo for API error handling patterns to confirm 401 vs 403?
289-327: Reload flow is safe.Clears query param even when unauthenticated; refetch+mark-read sequence is reasonable.
330-345: Infinite scroll trigger guarded well.Prevents duplicate loads and respects auth.
403-448: Timeout UX is thoughtful.Stops timer on success/error/auth changes and avoids toast spam with a ref.
468-477: Retry handler is correct.Clears UI flags and uses cancelRefetch to leverage abortable queries.
478-501: Auth/proxy actions: good error surfaces.Friendly toasts and defensive logging.
502-518: State flags cover all UI branches.Loader/empty/error gating avoids flicker and keeps scroll behavior sane.
652-657: Auth-gated filter is correct.Prevents prefetch/interaction when unauthenticated.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/brain/notifications/Notifications.tsx (1)
42-59: The aria-label issue has been resolved.The previous review comment about removing
aria-labelfrom the button has been addressed. The button now correctly relies on its visible text content without an overriding aria-label attribute.
🧹 Nitpick comments (2)
components/brain/notifications/Notifications.tsx (2)
378-455: Consider consolidating timeout effects.The timeout logic is spread across two effects (lines 411-441 and 443-455). While functionally correct, the second effect could be merged into the first to reduce complexity.
Apply this diff to consolidate:
useEffect(() => { if (isSuccess) { setHasTimedOut(false); timeoutToastShownRef.current = false; return; } if (errorMessage || !isAuthenticated || isLoadingProfile) { setHasTimedOut(false); timeoutToastShownRef.current = false; return; } if (isInitialQueryDone) { return; } const timerId = globalThis.setTimeout(() => { setHasTimedOut(true); + if (!timeoutToastShownRef.current) { + setToast({ + message: LOAD_TIMEOUT_MESSAGE, + type: "warning", + }); + timeoutToastShownRef.current = true; + } }, LOAD_TIMEOUT_MS); return () => { globalThis.clearTimeout(timerId); }; }, [ isSuccess, errorMessage, isAuthenticated, isInitialQueryDone, isLoadingProfile, + setToast, ]); - - useEffect(() => { - if (hasTimedOut) { - if (!timeoutToastShownRef.current) { - setToast({ - message: LOAD_TIMEOUT_MESSAGE, - type: "warning", - }); - timeoutToastShownRef.current = true; - } - } else { - timeoutToastShownRef.current = false; - } - }, [hasTimedOut, setToast]);
533-604: Consider memoizing derived values in dependency array.The layout effect dependency array (line 604) includes
showLoader,showNoItems, andshowErrorState, which are derived values recomputed on every render. While functionally correct, this could cause the effect to re-run more frequently than necessary.Consider memoizing these values with
useMemoif performance becomes a concern:const stateFlags = useMemo( () => ({ showLoader, showNoItems, showErrorState }), [showLoader, showNoItems, showErrorState] );Then update the dependency array:
- }, [items, showLoader, showNoItems, showErrorState]); + }, [items, stateFlags]);However, given the simplicity of these boolean computations, the current approach is acceptable and the optimization may not be necessary unless profiling indicates a performance issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/brain/notifications/Notifications.tsx(10 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsFollow existing code style and naming conventions
Files:
components/brain/notifications/Notifications.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/brain/notifications/Notifications.tsx
🧬 Code graph analysis (1)
components/brain/notifications/Notifications.tsx (9)
types/feed.types.ts (1)
TypedNotification(135-143)types/dropInteractionTypes.ts (1)
ActiveDropState(8-12)components/common/SpinnerLoader.tsx (1)
SpinnerLoader(8-57)components/brain/my-stream/layout/MyStreamNoItems.tsx (1)
MyStreamNoItems(4-126)components/brain/notifications/NotificationsWrapper.tsx (1)
NotificationsWrapper(21-79)components/auth/Auth.tsx (1)
AuthContext(83-93)components/brain/my-stream/layout/LayoutContext.tsx (1)
useLayout(455-455)components/brain/notifications/NotificationsCauseFilter.tsx (2)
NotificationFilter(8-11)NotificationsCauseFilter(31-137)hooks/useNotificationsQuery.tsx (1)
useNotificationsQuery(77-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
components/brain/notifications/Notifications.tsx (9)
1-36: LGTM! Imports and constants are well-structured.The imports are properly organized, type imports use the
typekeyword, and the new constants (LOAD_TIMEOUT_MS,DEFAULT_ERROR_MESSAGE,LOAD_TIMEOUT_MESSAGE) are clearly named and appropriately scoped.
61-156: Well-structured content resolver.The
resolveNotificationsContentfunction centralizes state-driven rendering logic with clear early returns for each state. TheNotificationsContentParamsinterface properly usesreadonlymodifiers as per coding guidelines.
163-221: Component initialization is sound.The context destructuring, refs, and state initialization are well-organized. The new error tracking refs (lines 178-181) effectively prevent duplicate toasts. The
markAllAsReadmutation includes appropriate error handling with toast notifications.
223-259: Authentication-gated effects are correctly implemented.The effects properly guard against running when not authenticated (lines 224-226, 238-240). The conditional identity assignment at line 254 correctly aligns with the query's
enabledcondition, ensuring the query doesn't run whenidentityisundefined.
261-287: Robust error normalization.The
getErrorDetailsfunction comprehensively handles various error types (Error objects, strings, unknown types) and correctly extracts status codes from multiple possible locations. The unauthorized detection logic covers both status codes and message patterns.
289-327: Reload flow is well-implemented.The reload effect correctly guards against running when not authenticated (lines 303-306) and properly cleans up the reload parameter after processing. The promise chain with error handling and final cleanup is appropriate.
476-509: Retry handlers are well-implemented.All three retry handlers (
handleRetry,handleAuthRetry,handleProxyDisable) properly reset error state, handle failures with appropriate error messages, and show toast notifications to inform users of issues.
511-531: Computed rendering states are correctly derived.The computed values (
showLoader,showNoItems,showErrorState,shouldEnableInfiniteScroll) properly account for authentication state, error conditions, timeout state, and data availability to determine which UI state to render.
606-677: Scroll handling and rendering logic are sound.The
handleScrollcallback correctly usesshouldEnableInfiniteScrollto conditionally trigger older notifications fetching. The conditional rendering of the filter (lines 661-666) based onisAuthenticatedensures the filter only appears when appropriate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/brain/notifications/Notifications.tsx (1)
261-287: Consider movinggetErrorDetailsoutside the component.The function is pure (no props/state/closures) and will be recreated on every render. Moving it above the component definition provides a minor performance benefit.
+const getErrorDetails = (error: unknown) => { + const status = + (error as any)?.status ?? + (error as any)?.response?.status ?? + (error as any)?.cause?.status; + + if (error instanceof Error) { + const message = error.message?.trim() || DEFAULT_ERROR_MESSAGE; + return { + message, + isUnauthorized: status === 401 || /unauthorized/i.test(message), + }; + } + + if (typeof error === "string") { + const message = error.trim() || DEFAULT_ERROR_MESSAGE; + return { + message, + isUnauthorized: status === 401 || /unauthorized/i.test(message), + }; + } + + return { + message: DEFAULT_ERROR_MESSAGE, + isUnauthorized: status === 401, + }; +}; + export default function Notifications({ activeDrop, setActiveDrop }: NotificationsProps) { - const getErrorDetails = (error: unknown) => { - const status = - (error as any)?.status ?? - (error as any)?.response?.status ?? - (error as any)?.cause?.status; - - if (error instanceof Error) { - const message = error.message?.trim() || DEFAULT_ERROR_MESSAGE; - return { - message, - isUnauthorized: status === 401 || /unauthorized/i.test(message), - }; - } - - if (typeof error === "string") { - const message = error.trim() || DEFAULT_ERROR_MESSAGE; - return { - message, - isUnauthorized: status === 401 || /unauthorized/i.test(message), - }; - } - - return { - message: DEFAULT_ERROR_MESSAGE, - isUnauthorized: status === 401, - }; - };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/brain/notifications/Notifications.tsx(10 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before propsFollow existing code style and naming conventions
Files:
components/brain/notifications/Notifications.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for stylingUse React functional components with hooks for UI components
Files:
components/brain/notifications/Notifications.tsx
🧬 Code graph analysis (1)
components/brain/notifications/Notifications.tsx (8)
types/feed.types.ts (1)
TypedNotification(135-143)types/dropInteractionTypes.ts (1)
ActiveDropState(8-12)components/common/SpinnerLoader.tsx (1)
SpinnerLoader(8-57)components/brain/my-stream/layout/MyStreamNoItems.tsx (1)
MyStreamNoItems(4-126)components/brain/notifications/NotificationsWrapper.tsx (1)
NotificationsWrapper(21-79)components/auth/Auth.tsx (1)
AuthContext(83-93)components/brain/notifications/NotificationsCauseFilter.tsx (2)
NotificationFilter(8-11)NotificationsCauseFilter(31-137)hooks/useNotificationsQuery.tsx (1)
useNotificationsQuery(77-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
components/brain/notifications/Notifications.tsx (12)
32-35: LGTM: Timeout and error constants are well-defined.The 15-second timeout and user-facing error messages are reasonable and improve UX when loading stalls.
37-59: LGTM: Accessible state message rendering.The
renderStateMessagehelper centralizes empty-state UI and the button correctly relies on visible text without aria-label overrides.
61-156: LGTM: Well-structured content resolution.The
resolveNotificationsContenthelper cleanly centralizes state-driven rendering with a logical priority: authentication → errors → data. All props are readonly as required.
243-259: LGTM: Authentication-gated notifications query.Passing
undefinedforidentitywhen not authenticated correctly prevents the query from running. The destructuredisSuccessandqueryErrorenable the new error-handling flows.
378-409: LGTM: Comprehensive error handling with deduplication.The error flow normalizes errors via
getErrorDetails, shows user-facing toasts, and triggers re-authentication for 401 errors. The refs (errorToastShownRef,reauthTriggeredRef,lastErrorMessageRef) correctly prevent duplicate toasts and auth requests.
411-441: LGTM: Proper timeout handling with cleanup.The timeout logic is well-guarded (only runs when authenticated and actually loading) and properly cleans up the timer. The 15-second threshold is reasonable for user experience.
443-455: LGTM: Timeout toast with deduplication.The timeout warning toast is shown once using the ref-based deduplication pattern consistent with error handling above.
476-509: LGTM: Well-structured retry and auth handlers.All three handlers (
handleRetry,handleAuthRetry,handleProxyDisable) correctly useuseCallback, handle errors gracefully, and provide user feedback via toasts.
511-531: LGTM: Clear computed display states.The boolean flags (
showLoader,showNoItems,showErrorState,shouldEnableInfiniteScroll) correctly model mutually exclusive UI states with proper authentication gating.
661-666: LGTM: Authentication-gated filter rendering.Conditionally rendering the cause filter only when authenticated is correct and consistent with the authentication-aware refactor.
638-677: LGTM: Clean rendering structure.The final rendering correctly passes all computed states to
resolveNotificationsContentand conditionally shows the filter. The scroll container and handlers remain properly structured.
163-677: Excellent authentication-aware refactor.This refactor successfully introduces authentication-gated notifications loading with comprehensive error handling, timeout detection, and retry/re-authentication flows. The code is well-structured with clear separation of concerns:
- Authentication gating: Identity is conditionally passed to the query, preventing unnecessary API calls
- Error handling: Normalized error detection with 401/unauthorized handling and automatic re-authentication
- Timeout handling: 15-second timeout with warning toast provides good UX for slow loads
- State management: Refs prevent duplicate toasts and auth requests
- UI flow:
resolveNotificationsContentcleanly handles all display statesThe implementation aligns well with the PR objectives and maintains consistency with the broader authentication patterns introduced across the codebase.
|
3 participants
Summary by CodeRabbit