Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR converts activist data storage from a single global singleton to per-chapter database instances. The ChangesChapter-Scoped Activist Storage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 43 minutes and 3 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend-v2/src/app/(authed)/events/useActivistRegistry.ts (1)
22-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
chapterIdis missing from theuseEffectdependency array —pnpm lintwill fail.
chapterIdis a hook parameter (not a ref or stable value) used on line 25, soreact-hooks/exhaustive-depsrequires it in the dependency array. Omitting it causes a lint error that would fail the CI/lint step called out in the coding guidelines.Since
chapterIdis stable within a session (chapter switching triggerswindow.location.href, a full-page reload), the fix is to document the invariant with an eslint-disable comment, consistent with the existing pattern on line 94:🛠️ Proposed fix
- }, []) + // eslint-disable-next-line react-hooks/exhaustive-deps -- chapterId is stable within a session; chapter switches trigger a full page reload + }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/`(authed)/events/useActivistRegistry.ts around lines 22 - 61, The useEffect depends on chapterId (used in getActivistStorage(chapterId)) but it's omitted from the dependency array causing a lint failure; update the effect to explicitly acknowledge the stability of chapterId by adding an eslint-disable comment for react-hooks/exhaustive-deps above the useEffect (matching the existing pattern used elsewhere) so the hook remains unchanged, e.g., place a single-line eslint-disable-next-line react-hooks/exhaustive-deps comment immediately before the useEffect that references chapterId and leave the body using getActivistStorage, registryRef.current.loadFromStorage, and setIsStorageLoaded as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend-v2/src/app/`(authed)/events/useActivistRegistry.ts:
- Around line 22-61: The useEffect depends on chapterId (used in
getActivistStorage(chapterId)) but it's omitted from the dependency array
causing a lint failure; update the effect to explicitly acknowledge the
stability of chapterId by adding an eslint-disable comment for
react-hooks/exhaustive-deps above the useEffect (matching the existing pattern
used elsewhere) so the hook remains unchanged, e.g., place a single-line
eslint-disable-next-line react-hooks/exhaustive-deps comment immediately before
the useEffect that references chapterId and leave the body using
getActivistStorage, registryRef.current.loadFromStorage, and setIsStorageLoaded
as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 52b61bf4-31fa-47a6-8044-8a7b72611b82
📒 Files selected for processing (5)
frontend-v2/src/app/(authed)/events/activist-storage.tsfrontend-v2/src/app/(authed)/events/event-form.tsxfrontend-v2/src/app/(authed)/events/events-page.tsxfrontend-v2/src/app/(authed)/events/useActivistRegistry.tsfrontend-v2/src/components/nav.tsx
The cache of activist names for attendance autofill mixed data from different chapters and shared one last-sync timestamp meaning many names would not get synced when switching chapters. This change makes caches per-chapter to fix this issue and keep chapter databases isolated from each other.
fixed by removing invalidateQueries before chapter switch navigation. Calling queryClient.invalidateQueries() immediately before window.location.href triggered React Query to fire refetch requests carrying the old session cookie. Each refetch response included a Set-Cookie header (written by renewAuthSession on the server) resetting the chapter back to the previous value. If those responses arrived after the chapter-switch redirect chain had already set the new chapter cookie, they would silently overwrite it — a non-deterministic race. Since window.location.href causes a full page reload, React Query's in-memory cache is discarded automatically; the invalidateQueries call was both redundant and the source of the race.
This avoids scoping the database to just activists in case we want to use it for other purposes, avoiding a combinatoric blowup of databases from crossing chapter IDs with entity types.
alexsapps
force-pushed
the
alex/fix-attendance-bug
branch
from
49828ef to
a37edbc
Compare
1 participant
Summary by CodeRabbit