Conversation
WalkthroughEnhanced iOS push notification registration with exponential backoff retry logic, delegate error detection, and restructured initialization orchestration. Added guarded action handling, expanded telemetry, broadened instrumentation filename exceptions, and extended error pattern detection. Updated corresponding test suite to cover new initialization flow. Changes
Sequence DiagramsequenceDiagram
participant App as App/Provider
participant Context as NotificationsContext
participant Capacitor as Capacitor
participant PushNotifications as iOS PushNotifications
participant Sentry as Error Telemetry
App->>Context: Mount & Initialize (isActive=true)
Context->>Capacitor: Check isActive state
Capacitor-->>Context: isActive response
alt isActive === true
Context->>PushNotifications: removeAllListeners()
PushNotifications-->>Context: Success
Context->>PushNotifications: addListener (registration, action, error)
PushNotifications-->>Context: Listeners attached
Context->>PushNotifications: requestPermissions()
PushNotifications-->>Context: Permissions granted/denied
Context->>Context: registerWithRetry(maxRetries=3)
loop Retry Logic (exponential backoff)
Context->>PushNotifications: register()
alt Registration Success
PushNotifications-->>Context: Registration OK
Context->>Context: Break retry loop
else Delegate Error Detected
Context->>Sentry: Capture delegate error
Sentry-->>Context: Error logged
Context->>Context: Wait & retry (2^attempt ms)
else Non-delegate Error
PushNotifications-->>Context: Error
Context->>Sentry: Capture registration error
Sentry-->>Context: Error logged
Context->>Context: Rethrow (exhausted retries)
end
end
Context->>App: Initialization complete
else isActive === false
Context-->>App: Skip initialization
end
opt Push action received
PushNotifications->>Context: Trigger action handler
Context->>Context: handlePushNotificationAction (validate profile)
Context->>Context: resolveRedirectUrl(notificationData)
Context->>App: Navigate to resolved URL
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (3)
components/notifications/NotificationsContext.tsx (3)
46-60: NewisActivegating is reasonable;registrationAttemptRefcurrently unused stateThe added
isActivecheck in theuseEffectguard makes sense to avoid initializing push while the app is inactive, and the dependency array correctly includesisActiveso initialization can occur once the app becomes active.
registrationAttemptRefis only ever reset to0(in the effect and on successful registration) and never read or incremented, so at the moment it does not influence behavior. If you intended it for telemetry or conditional logic (e.g. to avoid repeated registration beyond a certain count), consider wiring it intoregisterWithRetryor downstream logging; otherwise, it could be removed to keep state minimal.
61-72: Rethrowing initialization errors will now produce unhandled rejectionsBy adding
throw error;in bothinitializeNotificationsandinitializePushNotifications, any failure in the push setup path will now reject the async functions. SinceinitializeNotificationsis invoked from the effect withoutawaitor a.catch(...), these rejections will surface as unhandled promise rejections at runtime.If this is intentional so that global handlers/Sentry can capture the failures, the change is fine. If you instead want to log/report but avoid unhandled rejections impacting the runtime, you may want to catch at the call site and handle/report there rather than rethrowing.
Also applies to: 152-155
73-100: Retry helper logic is sound; consider minor robustness improvementsThe new
registerWithRetryhelper correctly:
- Bounds attempts by
maxRetries.- Retries only for known delegate-related errors with exponential backoff and a cap.
- Resurfaces non-matching errors (or final failures) immediately.
A couple of small points you may want to refine:
- The delegate error detection is based on substring matches in the error message. This is pragmatic, but brittle across Capacitor/iOS versions; please verify on-device that the actual error messages do contain these substrings and keep in mind they may change between versions.
- Since
maxRetriesis a parameter, you might eventually want to pass an explicit value from the call site or promote it to a named constant for easier tuning/experimentation.Overall, the retry behavior and backoff math look correct and should help with transient iOS delegate timing issues.
Also applies to: 141-149
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/notifications/NotificationsContext.tsx(4 hunks)instrumentation-client.ts(1 hunks)utils/error-sanitizer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
instrumentation-client.tsutils/error-sanitizer.tscomponents/notifications/NotificationsContext.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
instrumentation-client.tsutils/error-sanitizer.tscomponents/notifications/NotificationsContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
instrumentation-client.tsutils/error-sanitizer.tscomponents/notifications/NotificationsContext.tsx
@(instrumentation-client.ts|sentry.server.config.ts|sentry.edge.config.ts)
📄 CodeRabbit inference engine (AGENTS.md)
Errors should be filtered in the Sentry
beforeSendcallback; client-side filtering happens ininstrumentation-client.ts, server-side insentry.server.config.ts, and edge runtime insentry.edge.config.ts
Files:
instrumentation-client.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Applies to @(instrumentation-client.ts|sentry.server.config.ts|sentry.edge.config.ts) : Errors should be filtered in the Sentry `beforeSend` callback; client-side filtering happens in `instrumentation-client.ts`, server-side in `sentry.server.config.ts`, and edge runtime in `sentry.edge.config.ts`
Applied to files:
instrumentation-client.ts
🧬 Code graph analysis (1)
components/notifications/NotificationsContext.tsx (2)
components/auth/Auth.tsx (1)
useAuth(95-97)components/waves/drops/wave-drops-all/utils/delay.ts (1)
delay(1-4)
⏰ 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 (3)
utils/error-sanitizer.ts (1)
102-102: LGTM! The new IndexedDB error pattern is well-formed.The pattern correctly identifies database connection closing errors with flexible whitespace handling and case-insensitive matching, consistent with the existing patterns in the array.
instrumentation-client.ts (1)
24-24: Filename exceptions correctly filter external wallet extension errors.The additions of "injectLeap.js" and "inject.chrome" follow the established pattern of filtering errors from browser extension injection scripts outside developer control. These represent standard extension injection patterns:
inpage.js: MetaMask/web3 provider injectioninjectLeap.js: Leap wallet extension injectioninject.chrome: Generic Chrome extension injectionThe filtering logic correctly checks both
frame.filenameandframe.abs_pathusing string matching, and thebeforeSendcallback is the appropriate location per Sentry guidelines for client-side error filtering.components/notifications/NotificationsContext.tsx (1)
3-10: Capacitor/notifications imports look correct and fully usedAll newly added imports (
ApiIdentity,useCapacitor,commonApiPost,Device,PushNotifications, related types) are used consistently in this module; no unused or missing symbols spotted.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
__tests__/components/notifications/NotificationsContext.test.tsx (2)
78-86: Redundant mock clearing in beforeEach.
jest.clearAllMocks()on line 82 already clears all mocks. The individualmockClear()calls on lines 83-85 are redundant.beforeEach(() => { mockIsActive = true; - const { PushNotifications } = require("@capacitor/push-notifications"); jest.clearAllMocks(); - PushNotifications.removeAllListeners.mockClear(); - PushNotifications.addListener.mockClear(); - PushNotifications.register.mockClear(); });
124-139: Test is orphaned outside of any describe block.This test (
removes notifications when functions called) is placed outside thedescribeblocks, which affects test organization and reporting. Consider moving it into an appropriatedescribeblock.Consider wrapping this test in a describe block or moving it into one of the existing suites for better organization:
+describe("NotificationsContext notification removal", () => { it("removes notifications when functions called", async () => { // ... test body }); +});components/notifications/NotificationsContext.tsx (2)
90-92: Props should be marked asreadonly.As per coding guidelines, React component props should be prefixed with
readonly.-export const NotificationsProvider: React.FC<{ children: React.ReactNode }> = ({ +export const NotificationsProvider: React.FC<{ readonly children: React.ReactNode }> = ({ children, }) => {
308-308: Consider adding type definition for notification data.Using
anytype reduces type safety. Consider defining an interface for expected notification data structure.interface NotificationData { redirect?: keyof typeof redirectConfig; [key: string]: unknown; } const resolveRedirectUrl = (notificationData: NotificationData) => { // ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/components/notifications/NotificationsContext.test.tsx(3 hunks)components/notifications/NotificationsContext.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/notifications/NotificationsContext.tsx__tests__/components/notifications/NotificationsContext.test.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx__tests__/components/notifications/NotificationsContext.test.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
components/notifications/NotificationsContext.tsx__tests__/components/notifications/NotificationsContext.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
components/notifications/NotificationsContext.tsx__tests__/components/notifications/NotificationsContext.test.tsx
**/@(__tests__|*.test).{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests should live in
__tests__/orComponentName.test.tsx; mock external dependencies and APIs in tests
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions
Applied to files:
components/notifications/NotificationsContext.tsx
📚 Learning: 2025-12-03T14:52:34.255Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.255Z
Learning: Applies to **/@(__tests__|*.test).{ts,tsx} : Tests should live in `__tests__/` or `ComponentName.test.tsx`; mock external dependencies and APIs in tests
Applied to files:
__tests__/components/notifications/NotificationsContext.test.tsx
🧬 Code graph analysis (2)
components/notifications/NotificationsContext.tsx (3)
components/auth/Auth.tsx (1)
useAuth(95-97)generated/models/ApiIdentity.ts (1)
ApiIdentity(17-169)components/notifications/stable-device-id.ts (1)
getStableDeviceId(10-27)
__tests__/components/notifications/NotificationsContext.test.tsx (1)
components/notifications/NotificationsContext.tsx (1)
useNotificationsContext(334-342)
🪛 GitHub Check: SonarCloud Code Analysis
components/notifications/NotificationsContext.tsx
[failure] 132-132: Remove this use of the "void" operator.
__tests__/components/notifications/NotificationsContext.test.tsx
[warning] 106-106: Remove this useless assignment to variable "result".
⏰ 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)
__tests__/components/notifications/NotificationsContext.test.tsx (2)
11-18: Good pattern for dynamic mock values.Using a getter for
isActiveallows the mock to return different values per test. This is a clean approach for testing conditional behavior.
50-52: Deterministic mock for stable device ID.Good addition to ensure consistent test behavior.
components/notifications/NotificationsContext.tsx (6)
24-27: Well-defined constants for retry configuration.Good practice to extract magic numbers into named constants with clear intent.
29-39: Delegate error detection is well-implemented.The pattern matching approach for iOS delegate errors is clean and maintainable.
41-67: Solid exponential backoff implementation.The retry logic with exponential backoff and max delay cap is correctly implemented. The early return on success and re-throw on final failure are handled properly.
148-213: Well-structured initialization with proper error handling.The function properly:
- Cleans up existing listeners before adding new ones
- Handles iOS-specific timing issues
- Uses Sentry for error tracking
- Re-throws errors for caller handling
225-239: Initialization effect with proper gating.The use of
initializationRefto prevent duplicate initializations for the same profile is a good pattern. Error handling with Sentry integration is appropriate.
280-306: Good error handling with Sentry integration.The registration function properly catches errors, logs them, and reports to Sentry with appropriate tags.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
components/notifications/NotificationsContext.tsx (4)
98-111: Simplify notification removal error handling and guardnotification.dataTwo small improvements here:
Unreachable
.catchonremoveDeliveredNotifications
removeDeliveredNotificationsalready wrapsPushNotifications.removeDeliveredNotificationsin atry/catchand never rethrows, so the.catchattached inhandlePushNotificationActionwill never see a rejection. Consider either:
- Letting
removeDeliveredNotificationspropagate errors (no internaltry/catch) and keep the.catchat the call site, or- Keeping the internal
try/catch(and adding Sentry if desired) and removing the.catchfromhandlePushNotificationAction.This avoids having two overlapping error-handling strategies for the same operation.
Defensive default for
notification.data
To avoid a hard crash if a push arrives withoutdatapopulated, you can safely default it to an empty object:
const notificationData = notification.data;
const notificationData = notification.data ?? {}; const notificationProfileId = notificationData.profile_id;That keeps the logic the same when `data` is present, but prevents property access on `undefined`.Also applies to: 113-154
156-221: AwaitregisterPushNotificationinside the registration listenerInside
initializePushNotifications, the registration listener is markedasyncbut does not awaitregisterPushNotification, which returns a Promise. SinceregisterPushNotificationalready catches and logs/Sentrys its own errors, awaiting it is safe and avoids an unnecessary floating promise:- await PushNotifications.addListener("registration", async (token) => { - registerPushNotification( + await PushNotifications.addListener("registration", async (token) => { + await registerPushNotification( stableDeviceId, deviceInfo, token.value, profile ); });This makes the callback’s async signature meaningful and keeps the flow easier to reason about.
223-231: Allow re-initialization after a failed notifications setupThe
useEffectsetsinitializationRef.current = profileIdbefore callinginitializeNotifications. If initialization throws, the catch logs/Sentrys the error but leavesinitializationRef.currentset, so you won’t automatically retry for that profile (even if the failure was transient).Consider resetting the ref on failure so future renders can re-attempt initialization:
- if (isCapacitor && isActive && initializationRef.current !== profileId) { - initializationRef.current = profileId; - initializeNotifications(connectedProfile ?? undefined).catch((error) => { - console.error("Failed to initialize push notifications", error); - Sentry.captureException(error, { - tags: { - component: "NotificationsProvider", - operation: "initializeNotifications", - }, - }); - }); - } + if (isCapacitor && isActive && initializationRef.current !== profileId) { + initializationRef.current = profileId; + initializeNotifications(connectedProfile ?? undefined).catch((error) => { + console.error("Failed to initialize push notifications", error); + Sentry.captureException(error, { + tags: { + component: "NotificationsProvider", + operation: "initializeNotifications", + }, + }); + initializationRef.current = null; + }); + }This keeps the “once per profileId” behavior on success but makes the system more resilient to transient initialization failures.
Also applies to: 233-247
249-261: Align wave/all delivered removal with the rest of the error and Sentry strategyThe helpers for removing wave-specific and all delivered notifications are currently less defensive than the rest of the file:
removeWaveDeliveredNotificationsdoesn’t catch errors fromgetDeliveredNotificationsorremoveDeliveredNotifications.removeAllDeliveredNotificationscatches errors but only logs to the console, without Sentry tagging.To make these pathways consistent with your other Sentry-instrumented operations, you could wrap both in
try/catchblocks and capture exceptions with tags:const removeWaveDeliveredNotifications = useCallback( async (waveId: string) => { if (isIos) { - const deliveredNotifications = - await PushNotifications.getDeliveredNotifications(); - const waveNotifications = deliveredNotifications.notifications.filter( - (notification) => notification.data.wave_id === waveId - ); - await removeDeliveredNotifications(waveNotifications); + try { + const deliveredNotifications = + await PushNotifications.getDeliveredNotifications(); + const waveNotifications = + deliveredNotifications.notifications.filter( + (notification) => notification.data.wave_id === waveId + ); + await removeDeliveredNotifications(waveNotifications); + } catch (error) { + console.error("Error removing wave delivered notifications", error); + Sentry.captureException(error, { + tags: { + component: "NotificationsProvider", + operation: "removeWaveDeliveredNotifications", + }, + }); + } } }, [isIos, removeDeliveredNotifications] );const removeAllDeliveredNotifications = useCallback(async () => { if (isIos) { try { await PushNotifications.removeAllDeliveredNotifications(); } catch (error) { - console.error("Error removing all delivered notifications", error); + console.error("Error removing all delivered notifications", error); + Sentry.captureException(error, { + tags: { + component: "NotificationsProvider", + operation: "removeAllDeliveredNotifications", + }, + }); } } }, [isIos]);This brings these paths in line with the rest of your telemetry and reduces the chance of silent failures.
Also applies to: 263-271
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
__tests__/components/notifications/NotificationsContext.test.tsx(3 hunks)components/notifications/NotificationsContext.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/notifications/NotificationsContext.test.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/notifications/NotificationsContext.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
components/notifications/NotificationsContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
components/notifications/NotificationsContext.tsx
🧬 Code graph analysis (1)
components/notifications/NotificationsContext.tsx (4)
components/waves/drops/wave-drops-all/utils/delay.ts (1)
delay(1-4)components/auth/Auth.tsx (1)
useAuth(95-97)generated/models/ApiIdentity.ts (1)
ApiIdentity(17-169)components/notifications/stable-device-id.ts (1)
getStableDeviceId(10-27)
⏰ 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 (3)
components/notifications/NotificationsContext.tsx (3)
3-12: Imports and client-only wiring look consistentNew imports for models, Capacitor plugins, Sentry, and router are all used correctly in a client component and match the usages below; no changes needed here.
24-67: Retry/backoff helper for iOS delegate errors looks solidThe delegate-pattern detection and bounded exponential backoff in
registerWithRetryare correct and avoid unbounded retries while still handling transient delegate issues; the overall control flow (retry only on delegate errors, otherwise rethrow) is sound.
307-312: Sentry capture inregisterPushNotificationis appropriateAdding Sentry capture with specific
componentandoperationtags here gives you good visibility into backend registration failures without changing the behavior of the calling code; looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/notifications/NotificationsContext.tsx (2)
60-61: Redundantcontinuestatement.The
continueat line 61 is unnecessary since it's the last statement before the end of the loop iteration. Removing it simplifies the code without changing behavior.await new Promise((resolve) => setTimeout(resolve, delay)); - continue; }
325-349: Consider adding proper typing fornotificationData.Using
anybypasses TypeScript's type safety. Defining an interface for the expected notification data structure would improve maintainability and catch potential issues at compile time.+interface NotificationData { + redirect?: keyof typeof redirectConfig; + profile_id?: string; + path?: string; + handle?: string; + id?: string; + wave_id?: string; + drop_id?: string; + [key: string]: unknown; +} + -const resolveRedirectUrl = (notificationData: any) => { +const resolveRedirectUrl = (notificationData: NotificationData) => { const { redirect, ...params } = notificationData;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/notifications/NotificationsContext.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/notifications/NotificationsContext.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
components/notifications/NotificationsContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
components/notifications/NotificationsContext.tsx
🧬 Code graph analysis (1)
components/notifications/NotificationsContext.tsx (3)
components/auth/Auth.tsx (1)
useAuth(95-97)generated/models/ApiIdentity.ts (1)
ApiIdentity(17-169)components/notifications/stable-device-id.ts (1)
getStableDeviceId(10-27)
⏰ 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 (5)
components/notifications/NotificationsContext.tsx (5)
113-146: LGTM!The refactor to pass
routerInstanceandprofileInstanceas parameters rather than closing over them directly addresses potential stale closure concerns. The profile validation logic correctly prevents processing notifications meant for different profiles.
148-213: LGTM!The initialization flow is well-structured: it removes all listeners before adding new ones to prevent duplication on re-initialization, adds comprehensive event handlers with Sentry telemetry, and properly propagates errors for the caller to handle.
225-240: LGTM!The initialization effect correctly gates on
isActiveto ensure Capacitor is ready, usesinitializationRefto deduplicate initializations for the same profile, and properly resets the ref on failure to allow retry. Error capture to Sentry provides good observability.
297-323: LGTM!The registration function properly handles errors by logging and capturing to Sentry. The silent failure behavior (not re-throwing) appears intentional to prevent a registration failure from disrupting the overall notification flow.
242-280: LGTM!The notification removal functions have consistent error handling with Sentry capture, proper dependency arrays, and are correctly memoized in the context value.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/notifications/NotificationsContext.tsx (1)
13-20: AlignNotificationsProviderReact typings with guidelines (readonlyprops, noReact.FC).To match the stated guidelines (named React types +
readonlyprops), you can drop the defaultReactimport, use aReactNodetype import, and give the provider an explicit props interface withreadonly:-import React, { - createContext, - useCallback, - useContext, - useEffect, - useMemo, - useRef, -} from "react"; +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useRef, + type ReactNode, +} from "react"; @@ -interface NotificationData { +interface NotificationData { @@ -} - -export const NotificationsProvider: React.FC<{ children: React.ReactNode }> = ({ - children, -}) => { +} + +interface NotificationsProviderProps { + readonly children: ReactNode; +} + +export const NotificationsProvider = ({ + children, +}: NotificationsProviderProps) => {This removes
React.FC/React.ReactNode, uses named React types, and enforcesreadonlyon the props as requested.Also applies to: 89-105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/notifications/NotificationsContext.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/notifications/NotificationsContext.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
components/notifications/NotificationsContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
components/notifications/NotificationsContext.tsx
🪛 Biome (2.1.2)
components/notifications/NotificationsContext.tsx
[error] 44-44: This code will never be reached ...
... because either this statement will return from the function ...
... or this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
⏰ 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)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/notifications/NotificationsContext.tsx (1)
259-261: Guard against missingnotification.datawhen filtering wave notifications.The filter at line 260 accesses
notification.data.wave_idwithout optional chaining. Per Capacitor docs,notification.datacan beundefined, which would throw a runtime error.const waveNotifications = deliveredNotifications.notifications.filter( - (notification) => notification.data.wave_id === waveId + (notification) => notification.data?.wave_id === waveId );
🧹 Nitpick comments (1)
components/notifications/NotificationsContext.tsx (1)
78-99: Consider adding param validation in redirect resolvers.The resolver functions expect specific params (e.g.,
wave_idforwaves), butNotificationDatamarks them optional. If a required param is missing, URLs like/waves?wave=undefinedwould be produced without throwing.This is low-risk if the notification server guarantees correct payloads, but defensive validation could prevent malformed redirects.
waves: ({ wave_id, drop_id }: { wave_id: string; drop_id: string }) => { + if (!wave_id) return null; const base = `/waves?wave=${wave_id}`; return drop_id ? `${base}&serialNo=${drop_id}` : base; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/notifications/NotificationsContext.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/notifications/NotificationsContext.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
components/notifications/NotificationsContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
components/notifications/NotificationsContext.tsx
🧬 Code graph analysis (1)
components/notifications/NotificationsContext.tsx (2)
generated/models/ApiIdentity.ts (1)
ApiIdentity(17-169)components/notifications/stable-device-id.ts (1)
getStableDeviceId(10-27)
⏰ 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 (7)
components/notifications/NotificationsContext.tsx (7)
24-39: LGTM! Well-structured retry configuration and delegate error detection.The constants for retry logic with exponential backoff are reasonable, and the
isDelegateErrorhelper correctly identifies iOS delegate-related registration failures by pattern matching.
41-67: LGTM! Retry logic is now correctly structured.The exponential backoff with
continueat line 61 ensures the loop proceeds to the next attempt for delegate errors, while non-delegate errors or final attempts correctly throw. This addresses the previously flagged issue.
109-157: LGTM! Clean implementation with proper error handling.The
notification.data ?? {}fallback at line 131 safely handles missing data. The awaitedremoveDeliveredNotificationsat line 143 addresses the previously flaggedvoidoperator issue. Profile validation correctly prevents actions on mismatched notifications.
159-224: LGTM! Robust push notification initialization with proper cleanup.Good practices observed:
removeAllListeners()before adding new listeners prevents duplicates- Sentry telemetry on registration errors aids debugging
- iOS-specific delay before
registerWithRetry()handles delegate timing issues- Error is re-thrown for caller to handle with appropriate context
236-251: LGTM! Initialization effect properly gated on Capacitor active state.The
isActivecheck ensures push notification initialization only occurs when the Capacitor app is active. TheinitializationRefpattern prevents redundant re-initialization for the same profile, and error handling correctly resets the ref to allow retry.
277-334: LGTM! Consistent error handling and telemetry across all operations.Both
removeAllDeliveredNotificationsandregisterPushNotificationproperly capture exceptions to Sentry with appropriate tags, enabling effective error triage in production.
336-360: LGTM! Safe redirect URL resolution with comprehensive fallbacks.The
resolveRedirectUrlfunction gracefully handles missing redirect types, unknown types, and resolution errors, returningnullin each failure case rather than throwing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
components/notifications/NotificationsContext.tsx (3)
3-22: Align component typing with guidelines (readonlychildren, named React types).
NotificationsProviderstill usesReact.FC<{ children: React.ReactNode }>plus a defaultReactimport. To match the stated guidelines and modern React style you can:
- import
FC/ReactNodeas types; and- mark props as
readonly.For example:
-import React, { - createContext, - useCallback, - useContext, - useEffect, - useMemo, - useRef, -} from "react"; +import type { FC, ReactNode } from "react"; +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useRef, +} from "react"; @@ -export const NotificationsProvider: React.FC<{ children: React.ReactNode }> = ({ +export const NotificationsProvider: FC<{ readonly children: ReactNode }> = ({As per coding guidelines.
Also applies to: 104-110
93-103: Harden notification data handling and tighten typing for redirects.Right now
notificationDatais taken asnotification.data ?? {}without checking that it’s an object, andNotificationDatais effectively wide due to the index signature plus the cast inresolveRedirectUrl. To reduce surprises from malformed payloads you could first coercenotification.datato a predictable object:- const notificationData = notification.data ?? {}; + const rawData = notification.data; + const notificationData: NotificationData = + rawData && typeof rawData === "object" ? (rawData as NotificationData) : {};This ensures the profile-id check and
resolveRedirectUrl(notificationData)both operate on a consistent object shape, avoiding odd cases where a primitive payload is treated as params.Also applies to: 127-160, 339-363
110-110: Clarify initialization semantics for logged‑out users and consider a small telemetry tweak.
- With
const initializationRef = useRef<string | null>(null);andconst profileId = connectedProfile?.id ?? null;, the effect will not initialize when there is never aconnectedProfile(both values staynull), so logged‑out sessions won’t callinitializeNotificationseven ifisCapacitor && isActiveis true. If you intend to register push notifications even without a profile, consider a distinct sentinel for “never initialized”, e.g.:- const initializationRef = useRef<string | null>(null); + const initializationRef = useRef<string | null | undefined>(undefined);This preserves the “one init per profile id” behavior while allowing a first run for the
null(no‑profile) case.
- The removal helpers (
removeWaveDeliveredNotifications,removeAllDeliveredNotifications) andregisterPushNotificationnow have solid logging and tagging. For consistency, you might also route failures fromremoveDeliveredNotificationsthrough the same telemetry mechanism instead of only logging to the console.Also applies to: 162-237, 239-254, 256-278, 280-295, 311-337
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/notifications/NotificationsContext.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/notifications/NotificationsContext.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/notifications/NotificationsContext.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
Files:
components/notifications/NotificationsContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
Files:
components/notifications/NotificationsContext.tsx
🧬 Code graph analysis (1)
components/notifications/NotificationsContext.tsx (3)
components/auth/Auth.tsx (1)
useAuth(95-97)generated/models/ApiIdentity.ts (1)
ApiIdentity(17-169)components/notifications/stable-device-id.ts (1)
getStableDeviceId(10-27)
⏰ 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 (1)
components/notifications/NotificationsContext.tsx (1)
24-40: Retry control flow around registration now correctly distinguishes delegate errors.The updated
isDelegateError+registerWithRetrylogic now:
- retries only when the error matches a delegate pattern and there are attempts left; and
- rethrows immediately otherwise or on the final attempt.
This removes the previous “always throw” / no‑retry behavior and should also satisfy the unreachable‑code warning while keeping the exponential backoff bounded.
Also applies to: 41-70
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
components/user/collected/UserPageCollected.tsx (3)
38-49: Exported API change (ProfileCollectedFilters.szn→number) needs a strict invariant (e.g.,0 = All, positive ints only).
Right nowszncan becomeNaN(or negative) from URL params, which violates that likely invariant and can ripple into UI and react-query keys. Consider documenting/encoding the invariant in the type (e.g.,numberbut always normalized to0 | positive int).
145-175: Hardensznparsing (radix + NaN/negative guard) and satisfy Sonar by usingNumber.parseInt.
szn ? parseInt(szn) : 0allowsNaN/negative values; also omits radix.@@ - szn: szn ? parseInt(szn) : 0, + szn: (() => { + if (!szn) return 0; + const parsed = Number.parseInt(szn, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : 0; + })(),
332-345:setSznshould normalize/validate inputs before writing URL params.
As written,NaNwould serialize to"NaN"(truthy) and get persisted into the URL.@@ - const setSzn = async (szn: number): Promise<void> => { + const setSzn = async (szn: number): Promise<void> => { + const normalizedSzn = + Number.isFinite(szn) && szn > 0 ? Math.trunc(szn) : 0; const items: QueryUpdateInput[] = [ { name: "szn", - value: szn ? szn.toString() : null, + value: normalizedSzn ? normalizedSzn.toString() : null, }, @@components/user/collected/cards/UserPageCollectedCardsNoCards.tsx (1)
39-43: Remove unnecessary Effect; compute during render.This Effect derives state from props, which is an anti-pattern in React 19. The msg value should be computed directly during render rather than stored in state and synced via Effect.
Based on learnings, prefer refactors aligned with React 19.2 conventions.
Apply this diff to compute during render and convert to Server Component:
-"use client"; - import { CollectedCollectionType, CollectionSeized } from "@/entities/IProfile"; import { assertUnreachable } from "@/helpers/AllowlistToolHelpers"; -import { useEffect, useState } from "react"; import { ProfileCollectedFilters } from "../UserPageCollected"; export default function UserPageCollectedCardsNoCards({ filters, }: { readonly filters: ProfileCollectedFilters; }) { - const getMsg = (): string => { + const msg = (() => { if (filters.seized !== CollectionSeized.NOT_SEIZED) { return "No cards to display"; } switch (filters.collection) { case null: return "Congratulations, full setter!"; case CollectedCollectionType.MEMES: if (filters.szn === 0) { return "Congratulations, The Memes full setter!"; } return `Congratulations, ${filters.szn} full setter!`; case CollectedCollectionType.GRADIENTS: return "Congratulations, Gradient full setter!"; case CollectedCollectionType.MEMELAB: return "Congratulations, Meme Lab full setter!"; case CollectedCollectionType.NEXTGEN: return "Congratulations, Next Gen full setter!"; case CollectedCollectionType.NETWORK: return "No network tokens found"; default: assertUnreachable(filters.collection); return ""; } - }; - - const [msg, setMsg] = useState(getMsg()); - - useEffect(() => { - setMsg(getMsg()); - }, [filters]); + })(); return ( <div className="tw-py-4 tw-text-sm tw-italic tw-text-iron-500">{msg}</div> ); }components/user/collected/filters/UserPageCollectedFilters.tsx (1)
44-51: Remove commented-out legacyUserPageCollectedFiltersSzncomponent.Lines 196-200 contain commented JSX that violates the TSX guideline requiring self-explanatory, comment-free code. The
SeasonsDropdowncomponent is now the active implementation; the old component reference should be deleted entirely.{getShowSzn(filters.collection) && ( - // <UserPageCollectedFiltersSzn - // selected={filters.szn} - // containerRef={containerRef} - // setSelected={setSzn} - // /> <SeasonsDropdown selectedSeason={filters.szn} setSelectedSeason={setSzn} /> )}The type signature
setSzn: (szn: number) => voidis correct and consistent withProfileCollectedFilters.szn: numberandSeasonsDropdown's contract where0represents "All".
🧹 Nitpick comments (2)
components/user/collected/cards/UserPageCollectedCardsNoCards.tsx (1)
21-24: Consider adding "SZN" prefix for clarity.Line 24 interpolates the season number directly without a prefix, which may be less clear than "Congratulations, SZN ${filters.szn} full setter!" for consistency with how seasons are typically referenced.
Apply this diff for improved clarity:
- return `Congratulations, ${filters.szn} full setter!`; + return `Congratulations, SZN ${filters.szn} full setter!`;__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx (1)
39-118: Add mock cleanup to prevent cross-test leakage.Recommend adding
afterEach(() => jest.clearAllMocks());(ormockCommonApiFetch.mockReset()), since each test relies on fresh mock resolution. Based on learnings, keep tests independent/deterministic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx(1 hunks)components/seasons-dropdown/SeasonsDropdown.module.scss(1 hunks)components/seasons-dropdown/SeasonsDropdown.tsx(1 hunks)components/the-memes/TheMemes.tsx(1 hunks)components/user/collected/UserPageCollected.tsx(4 hunks)components/user/collected/cards/UserPageCollectedCardsNoCards.tsx(2 hunks)components/user/collected/filters/UserPageCollectedFilters.tsx(6 hunks)components/user/collected/filters/UserPageCollectedFiltersSzn.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- components/user/collected/filters/UserPageCollectedFiltersSzn.tsx
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
components/user/collected/cards/UserPageCollectedCardsNoCards.tsxcomponents/the-memes/TheMemes.tsxcomponents/seasons-dropdown/SeasonsDropdown.tsx__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsxcomponents/user/collected/filters/UserPageCollectedFilters.tsxcomponents/user/collected/UserPageCollected.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
components/user/collected/cards/UserPageCollectedCardsNoCards.tsxcomponents/the-memes/TheMemes.tsxcomponents/seasons-dropdown/SeasonsDropdown.tsx__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsxcomponents/user/collected/filters/UserPageCollectedFilters.tsxcomponents/user/collected/UserPageCollected.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks rules)
Follow existing code style and naming conventions; maintain clean code standards (measured by SonarQube)
Files:
components/user/collected/cards/UserPageCollectedCardsNoCards.tsxcomponents/the-memes/TheMemes.tsxcomponents/seasons-dropdown/SeasonsDropdown.tsx__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsxcomponents/user/collected/filters/UserPageCollectedFilters.tsxcomponents/user/collected/UserPageCollected.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
**/*.{ts,tsx}: Must passtsc --noEmitfor TypeScript type checking
Prefer Server Components over Client Components; use Server Functions/Server Actions ('use server') for mutations
Remove unnecessary Effects; if Effect only derives state, compute during render instead
UseuseEffectEventfor non-reactive logic inside Effects to avoid unnecessary re-runs
Use framework APIs:<Link>for internal links,next/imagefor images, adopt Next's ESLint rules
Use'use cache'directive and Cache Components features for explicit opt-in caching in Next.js 16
Use TypeScript and React functional components with hooks
When parsing Seize URLs or similar, fail fast if base origin is unavailable; do not fall back to placeholder origins
Replace<img>elements with<Image />fromnext/image
Use<Link href="/path">for internal navigation instead of plain HTML links
Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with'use server'directive
Files:
components/user/collected/cards/UserPageCollectedCardsNoCards.tsxcomponents/the-memes/TheMemes.tsxcomponents/seasons-dropdown/SeasonsDropdown.tsx__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsxcomponents/user/collected/filters/UserPageCollectedFilters.tsxcomponents/user/collected/UserPageCollected.tsx
**/@(__tests__|*.test).{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests should live in
__tests__/orComponentName.test.tsx; mock external dependencies and APIs in tests
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Place tests in
__tests__/directory or asComponentName.test.tsxalongside components
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Mock external dependencies and APIs in tests
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use Jest +
ts-jestfor TypeScript testing
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Functions must have ≤ 15 cognitive complexity; extract deep ternaries (>3 levels) and break down complex logic
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
__tests__/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (tests/AGENTS.md)
__tests__/**/*.{ts,tsx,js}: Preferfor...ofloops overforEachas it allowsbreak/continueand works with async/await
Usearray.at(-1)andarray.at(-2)instead of index-based array access for negative indexing
UseString.prototype.replaceAll()instead ofreplace()for global string replacements
UseglobalThis.fetch()instead of directfetch()calls
Organize imports with one import per module in order: external → internal → types, with no duplicates
Useelement.remove()instead ofparent.removeChild(element)for DOM manipulation
Catch errors only when meaningful; no empty catch blocks; log errors with context
Avoid double negatives in code; prefer explicit logic and remove redundant annotations; use optional chaining (?.)
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
__tests__/**/{components,contexts,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use semantic HTML elements (
<label>,<output>) over ARIA attributes when possible; every form control must have a label
Files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
🧠 Learnings (12)
📚 Learning: 2025-12-03T14:52:34.271Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.271Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions
Applied to files:
components/user/collected/cards/UserPageCollectedCardsNoCards.tsxcomponents/the-memes/TheMemes.tsxcomponents/seasons-dropdown/SeasonsDropdown.tsxcomponents/user/collected/filters/UserPageCollectedFilters.tsxcomponents/user/collected/UserPageCollected.tsx
📚 Learning: 2025-12-05T10:55:30.871Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-05T10:55:30.871Z
Learning: Applies to **/*.{ts,tsx} : Remove unnecessary Effects; if Effect only derives state, compute during render instead
Applied to files:
components/the-memes/TheMemes.tsx
📚 Learning: 2025-12-03T14:52:34.271Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.271Z
Learning: Applies to **/@(__tests__|*.test).{ts,tsx} : Tests should live in `__tests__/` or `ComponentName.test.tsx`; mock external dependencies and APIs in tests
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:30.871Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-05T10:55:30.871Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies and APIs in tests
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/*.{ts,tsx,js} : Avoid double negatives in code; prefer explicit logic and remove redundant annotations; use optional chaining (`?.`)
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/__tests__/**/*.test.{ts,tsx,js} : Test high-risk areas including happy path workflows, invalid input errors, edge cases/boundaries, component & API interactions, and performance/security when relevant
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/__tests__/**/components/**/*.test.{ts,tsx} : Test accessibility with keyboard navigation and screen reader compatibility
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/__tests__/**/*.test.{ts,tsx,js} : Keep tests independent, deterministic, and fast with production-like data
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/__tests__/**/components/**/*.test.{ts,tsx} : Use testing-library/react + user-event for React component tests
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/{components,contexts,hooks}/**/*.{ts,tsx} : Use semantic HTML elements (`<label>`, `<output>`) over ARIA attributes when possible; every form control must have a label
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/app/api/**/__tests__/**/*.test.{ts,tsx,js} : API integration tests should be located in `app/api`
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/*.{ts,tsx,js} : Use `globalThis.fetch()` instead of direct `fetch()` calls
Applied to files:
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx
🧬 Code graph analysis (3)
components/seasons-dropdown/SeasonsDropdown.tsx (1)
entities/ISeason.ts (1)
MemeSeason(1-8)
__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx (1)
components/seasons-dropdown/SeasonsDropdown.tsx (1)
SeasonsDropdown(19-163)
components/user/collected/filters/UserPageCollectedFilters.tsx (1)
components/seasons-dropdown/SeasonsDropdown.tsx (1)
SeasonsDropdown(19-163)
🪛 GitHub Check: SonarCloud Code Analysis
components/seasons-dropdown/SeasonsDropdown.module.scss
[warning] 50-50: Text does not meet the minimal contrast requirement with its background.
components/user/collected/UserPageCollected.tsx
[warning] 166-166: Prefer Number.parseInt over parseInt.
⏰ 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 (3)
components/user/collected/UserPageCollected.tsx (1)
403-405: API param write is consistent with0 = All—keep it that way after normalization.
if (filters.szn)correctly omits season whenszn === 0; just make surefilters.szncan’t becomeNaN(see parsing/normalization comments).components/the-memes/TheMemes.tsx (1)
451-456: Header change looks fine.__tests__/components/seasons-dropdown/SeasonsDropdown.test.tsx (1)
1-37: Tests likely failing: toggle label expectsSZN: SZN2but component rendersSZN: 2.Either update expectations to
SZN: 2, or (better) updatecomponents/seasons-dropdown/SeasonsDropdown.tsxto useMemeSeason.display/SZN${id}so the UI matches the intended “SZN2” label. Based on learnings, tests should remain deterministic and aligned to real UI contracts.⛔ Skipped due to learnings
Learnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-03T14:52:34.271Z Learning: Applies to **/@(__tests__|*.test).{ts,tsx} : Tests should live in `__tests__/` or `ComponentName.test.tsx`; mock external dependencies and APIs in testsLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __tests__/AGENTS.md:0-0 Timestamp: 2025-12-05T10:55:43.476Z Learning: Applies to __tests__/**/*.{ts,tsx,js} : Avoid double negatives in code; prefer explicit logic and remove redundant annotations; use optional chaining (`?.`)Learnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __tests__/AGENTS.md:0-0 Timestamp: 2025-12-05T10:55:43.476Z Learning: Applies to __tests__/**/{components,contexts,hooks}/**/*.{ts,tsx} : Use semantic HTML elements (`<label>`, `<output>`) over ARIA attributes when possible; every form control must have a labelLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __tests__/AGENTS.md:0-0 Timestamp: 2025-12-05T10:55:43.476Z Learning: Applies to __tests__/**/__tests__/**/*.test.{ts,tsx,js} : Test high-risk areas including happy path workflows, invalid input errors, edge cases/boundaries, component & API interactions, and performance/security when relevantLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __mocks__/AGENTS.md:0-0 Timestamp: 2025-11-25T08:37:21.058Z Learning: Applies to __mocks__/**/__mocks__/** : Document expected behaviour in the mock file when it is not obviousLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __tests__/AGENTS.md:0-0 Timestamp: 2025-12-05T10:55:43.476Z Learning: Applies to __tests__/**/__tests__/**/*.test.{ts,tsx,js} : Write tests following Arrange – Act – Assert pattern with one behaviour per test and clear, descriptive namesLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: GEMINI.md:0-0 Timestamp: 2025-12-05T10:55:30.871Z Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies and APIs in testsLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __tests__/AGENTS.md:0-0 Timestamp: 2025-12-05T10:55:43.476Z Learning: Applies to __tests__/**/__tests__/**/components/**/*.test.{ts,tsx} : Use testing-library/react + user-event for React component testsLearnt from: CR Repo: 6529-Collections/6529seize-frontend PR: 0 File: __tests__/AGENTS.md:0-0 Timestamp: 2025-12-05T10:55:43.476Z Learning: Applies to __tests__/**/*.{ts,tsx} : Functions must have ≤ 15 cognitive complexity; extract deep ternaries (>3 levels) and break down complex logic
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
__tests__/components/notifications/NotificationsContext.test.tsx (2)
78-122: Excellent test coverage for the new initialization flow.The tests comprehensively verify the isActive-gated initialization behavior. The 2000ms timeout at line 119 properly accounts for the exponential backoff retry logic mentioned in the PR objectives.
Note: Lines 83-85 are redundant since
jest.clearAllMocks()on line 82 already clears all mocks.Apply this diff to remove redundant mock clearing:
beforeEach(() => { mockIsActive = true; const { PushNotifications } = require("@capacitor/push-notifications"); jest.clearAllMocks(); - PushNotifications.removeAllListeners.mockClear(); - PushNotifications.addListener.mockClear(); - PushNotifications.register.mockClear(); });
54-54: Consider addingreadonlymodifier for consistency.The coding guideline specifies that props should be marked
readonlyin React components. While this is a test helper, applying the guideline consistently improves type safety.As per coding guidelines, apply this diff:
-const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( +const wrapper: React.FC<{ readonly children: React.ReactNode }> = ({ children }) => ( <NotificationsProvider>{children}</NotificationsProvider> );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
__tests__/components/notifications/NotificationsContext.test.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Do not include any comments in the code; it should be self-explanatory
Write correct, up-to-date, bug-free, fully componentized, secure, and efficient code
Include all required imports and ensure proper naming of key components
Use NextJS features that match the current version
**/*.{ts,tsx,js,jsx}: Replace<img>elements with<Image />fromnext/imageto satisfy@next/next/no-img-elementESLint rule
Use<Link href="/path">from Next.js for internal navigation instead of plain HTML links to satisfy@next/next/no-html-link-for-pagesESLint rule
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{tsx,jsx}: Use FontAwesome for icons in React components
Use TailwindCSS for styling in React components
Use react-query for data fetching
Always addreadonlybefore props in React components
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks)
Use framework APIs: internal links should use<Link>, images should usenext/image, and adopt Next's ESLint rules (Core Web Vitals)
**/*.{js,jsx,ts,tsx}: Code must satisfy ESLint (Next's Core Web Vitals + React Hooks rules)
Follow existing code style and naming conventions; maintain clean code standards (measured by SonarQube)
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Must passtsc --noEmittype checking
Prefer direct named imports for React hooks and types (import { useMemo, useRef, FC, etc. } from "react") overReact.namespace usage (React.useMemo,React.useRef, etc.)
If thereact-hooks/exhaustive-depslint rule is triggered: if the Effect only derives state, remove the Effect and compute during render; if listening to an external system and needing fresh props/state, wrap non-reactive logic inuseEffectEvent
**/*.{ts,tsx}: Must passtsc --noEmitfor TypeScript type checking
Prefer Server Components over Client Components; use Server Functions/Server Actions ('use server') for mutations
Remove unnecessary Effects; if Effect only derives state, compute during render instead
UseuseEffectEventfor non-reactive logic inside Effects to avoid unnecessary re-runs
Use framework APIs:<Link>for internal links,next/imagefor images, adopt Next's ESLint rules
Use'use cache'directive and Cache Components features for explicit opt-in caching in Next.js 16
Use TypeScript and React functional components with hooks
When parsing Seize URLs or similar, fail fast if base origin is unavailable; do not fall back to placeholder origins
Replace<img>elements with<Image />fromnext/image
Use<Link href="/path">for internal navigation instead of plain HTML links
Move data fetches to Server Components; handle mutations through Server Functions/Server Actions with'use server'directive
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
**/@(__tests__|*.test).{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests should live in
__tests__/orComponentName.test.tsx; mock external dependencies and APIs in tests
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Place tests in
__tests__/directory or asComponentName.test.tsxalongside components
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Mock external dependencies and APIs in tests
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use Jest +
ts-jestfor TypeScript testing
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Functions must have ≤ 15 cognitive complexity; extract deep ternaries (>3 levels) and break down complex logic
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
__tests__/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (tests/AGENTS.md)
__tests__/**/*.{ts,tsx,js}: Preferfor...ofloops overforEachas it allowsbreak/continueand works with async/await
Usearray.at(-1)andarray.at(-2)instead of index-based array access for negative indexing
UseString.prototype.replaceAll()instead ofreplace()for global string replacements
UseglobalThis.fetch()instead of directfetch()calls
Organize imports with one import per module in order: external → internal → types, with no duplicates
Useelement.remove()instead ofparent.removeChild(element)for DOM manipulation
Catch errors only when meaningful; no empty catch blocks; log errors with context
Avoid double negatives in code; prefer explicit logic and remove redundant annotations; use optional chaining (?.)
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
__tests__/**/{components,contexts,hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use semantic HTML elements (
<label>,<output>) over ARIA attributes when possible; every form control must have a label
Files:
__tests__/components/notifications/NotificationsContext.test.tsx
🧠 Learnings (4)
📚 Learning: 2025-12-03T14:52:34.271Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.271Z
Learning: Applies to **/@(__tests__|*.test).{ts,tsx} : Tests should live in `__tests__/` or `ComponentName.test.tsx`; mock external dependencies and APIs in tests
Applied to files:
__tests__/components/notifications/NotificationsContext.test.tsx
📚 Learning: 2025-12-05T10:55:30.871Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-05T10:55:30.871Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies and APIs in tests
Applied to files:
__tests__/components/notifications/NotificationsContext.test.tsx
📚 Learning: 2025-12-05T10:55:43.476Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-12-05T10:55:43.476Z
Learning: Applies to __tests__/**/*.{ts,tsx,js} : Avoid double negatives in code; prefer explicit logic and remove redundant annotations; use optional chaining (`?.`)
Applied to files:
__tests__/components/notifications/NotificationsContext.test.tsx
📚 Learning: 2025-12-03T14:52:34.271Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-03T14:52:34.271Z
Learning: Fix with modernization (no `// eslint-disable` unless explicitly instructed); prefer refactors aligned with React 19.2, React Compiler, and Next.js 16 conventions
Applied to files:
__tests__/components/notifications/NotificationsContext.test.tsx
🧬 Code graph analysis (1)
__tests__/components/notifications/NotificationsContext.test.tsx (1)
components/notifications/NotificationsContext.tsx (1)
useNotificationsContext(365-373)
⏰ 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 (3)
__tests__/components/notifications/NotificationsContext.test.tsx (3)
11-18: LGTM! Dynamic mock state pattern is well-implemented.The getter pattern correctly enables tests to toggle
isActivedynamically, which is essential for testing the isActive-gated initialization logic introduced in the PR.
50-52: LGTM! Deterministic device ID mock.Properly mocks the stable device ID for deterministic test behavior.
106-106: Past review feedback addressed.The unused
resultvariable has been correctly removed from both initialization tests. These calls torenderHookonly trigger initialization side effects and don't need to accessresult.current.Also applies to: 151-151
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.