feat: real-time notifications with GraphQL subscriptions#155
Open
0xDeon wants to merge 3 commits intoboundlessfi:mainfrom
Open
feat: real-time notifications with GraphQL subscriptions#1550xDeon wants to merge 3 commits intoboundlessfi:mainfrom
0xDeon wants to merge 3 commits intoboundlessfi:mainfrom
Conversation
|
@0xDeon Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughAdds a client-side real-time notification system: new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Client
participant NavBar as Global Navbar
participant NotifCenter as NotificationCenter
participant Hook as useNotifications
participant WSClient as GraphQL WS Client
participant Server as GraphQL Server
User->>NavBar: load page / mount
NavBar->>NotifCenter: render
NotifCenter->>Hook: call useNotifications()
Hook->>WSClient: subscribe ON_BOUNTY_UPDATED (enabled=true)
Hook->>WSClient: subscribe ON_NEW_APPLICATION
WSClient->>Server: open subscriptions
Server-->>WSClient: send event (bounty/app)
WSClient->>Hook: deliver payload
Hook->>Hook: normalize & upsert NotificationItem
Hook-->>NotifCenter: provide notifications + unreadCount
NotifCenter->>User: show badge / popover
User->>NotifCenter: click notification
NotifCenter->>Hook: markAsRead(id, type)
Hook->>Hook: set read=true, update unreadCount
NotifCenter->>User: update UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Comment |
|
@0xDeon is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/graphql/subscriptions.ts (1)
85-94: Avoid duplicate payload interfaces for bounty-updated events.
OnBountyUpdatedDataduplicatesBountyUpdatedDataexactly. Consolidating to a type alias avoids future drift.♻️ Suggested simplification
-export interface OnBountyUpdatedData { - bountyUpdated: { - id: string; - title: string; - status: string; - rewardAmount: number; - rewardCurrency: string; - updatedAt?: string | null; - }; -} +export type OnBountyUpdatedData = BountyUpdatedData;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/graphql/subscriptions.ts` around lines 85 - 94, The interface OnBountyUpdatedData duplicates BountyUpdatedData; to avoid drift, replace the duplicate interface declaration (OnBountyUpdatedData) with a type alias to BountyUpdatedData (e.g., type OnBountyUpdatedData = BountyUpdatedData) or remove OnBountyUpdatedData and update any references to use BountyUpdatedData directly, ensuring all places that referenced OnBountyUpdatedData now point to the single canonical BountyUpdatedData type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/use-notifications.ts`:
- Around line 27-29: The normaliseTimestamp function can throw for invalid date
strings; update normaliseTimestamp to defensively validate/parses the incoming
value (e.g., using Date.parse or constructing a Date and checking
isNaN(date.getTime())) and fall back to new Date().toISOString() for
null/undefined or invalid inputs. Keep the function signature and ensure it
returns a valid ISO string in all cases (so callers of normaliseTimestamp won't
throw on bad payloads).
- Around line 51-57: The hook retains prior notifications across auth
transitions; inside useNotifications add an effect that clears notifications
whenever the authenticated user changes (e.g., useEffect(() =>
setNotifications([]), [session?.user?.id]) or at minimum [session]) so that
notifications/state are reset on logout/login; reference the useNotifications
function and the notifications/setNotifications state variables and update the
effect to run when session (or session.user.id) changes.
---
Nitpick comments:
In `@lib/graphql/subscriptions.ts`:
- Around line 85-94: The interface OnBountyUpdatedData duplicates
BountyUpdatedData; to avoid drift, replace the duplicate interface declaration
(OnBountyUpdatedData) with a type alias to BountyUpdatedData (e.g., type
OnBountyUpdatedData = BountyUpdatedData) or remove OnBountyUpdatedData and
update any references to use BountyUpdatedData directly, ensuring all places
that referenced OnBountyUpdatedData now point to the single canonical
BountyUpdatedData type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9bb5aea-39c9-468a-8907-d540ba93dca7
📒 Files selected for processing (6)
components/global-navbar.tsxcomponents/notifications/notification-center.tsxhooks/use-graphql-subscription.tshooks/use-notifications.tslib/graphql/subscriptions.tslib/store.test.ts
💤 Files with no reviewable changes (1)
- lib/store.test.ts
0xDeon
changed the title
- Replace duplicate OnBountyUpdatedData interface with type alias to BountyUpdatedData - Harden normaliseTimestamp against invalid date strings (fallback to now) - Clear notification state on auth user change to prevent cross-user leakage
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/graphql/subscriptions.ts`:
- Around line 19-32: Update the test assertion that looks for the GraphQL
operation name from 'subscription BountyUpdated' to 'subscription
OnBountyUpdated' to match the exported
ON_BOUNTY_UPDATED_SUBSCRIPTION/BOUNTY_UPDATED_SUBSCRIPTION operation name;
modify the check in use-bounty-subscription.test (the assertion that inspects
c[0].query.includes(...)) accordingly and also verify any server-side
subscription resolver names that reference the old "BountyUpdated" operation are
renamed or aliased to "OnBountyUpdated" so the client and server operation names
match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9db2527-564e-4f38-bece-bb4142ab64c6
📒 Files selected for processing (2)
hooks/use-notifications.tslib/graphql/subscriptions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/use-notifications.ts
Align test assertion with renamed OnBountyUpdated operation name
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/__tests__/use-bounty-subscription.test.ts (1)
145-149: Consider adding a guard for the subscription lookup to improve test failure messages.If the subscription string doesn't match (e.g., if the operation name changes again),
callwill beundefinedandcall[1].nextwill throw a confusingTypeErrorinstead of a clear assertion failure.This same pattern exists at lines 115-118 and 174-177.
💡 Suggested improvement
// Find the call for bountyUpdated and trigger its callback const call = mockSubscribe.mock.calls.find((c) => c[0].query.includes("subscription OnBountyUpdated"), ); + expect(call).toBeDefined(); const callback = call[1].next;Apply the same pattern to the other
find()calls forBountyCreatedandBountyDeleted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/__tests__/use-bounty-subscription.test.ts` around lines 145 - 149, The test currently assumes mockSubscribe.mock.calls.find(...) returns a match and immediately accesses call[1].next, which causes a confusing TypeError when the subscription name changes; update each lookup (the three uses that search for "subscription OnBountyUpdated", "subscription OnBountyCreated" and "subscription OnBountyDeleted") to assert the result exists before accessing its elements: capture the result into a variable (e.g., call), add an explicit expect(call).toBeDefined() or throw a clear message if undefined, then safely extract callback = call[1].next; this will produce readable test failures when the subscription lookup fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/__tests__/use-bounty-subscription.test.ts`:
- Around line 145-149: The test currently assumes
mockSubscribe.mock.calls.find(...) returns a match and immediately accesses
call[1].next, which causes a confusing TypeError when the subscription name
changes; update each lookup (the three uses that search for "subscription
OnBountyUpdated", "subscription OnBountyCreated" and "subscription
OnBountyDeleted") to assert the result exists before accessing its elements:
capture the result into a variable (e.g., call), add an explicit
expect(call).toBeDefined() or throw a clear message if undefined, then safely
extract callback = call[1].next; this will produce readable test failures when
the subscription lookup fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e30f303-e4bc-4f9a-8294-bd349d6ffd80
📒 Files selected for processing (1)
hooks/__tests__/use-bounty-subscription.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements real-time in-app notifications using GraphQL subscriptions (
graphql-ws), allowing users to receive instant updates when bounties are updated or new applications are submitted.Changes
Modified
lib/graphql/subscriptions.ts- AddedON_BOUNTY_UPDATED_SUBSCRIPTIONandON_NEW_APPLICATION_SUBSCRIPTIONsubscription documents with typed response interfaceshooks/use-graphql-subscription.ts- Addedenabledparameter to conditionally activate subscriptions (guards against unauthenticated connections)components/global-navbar.tsx- IntegratedNotificationCentercomponent between rank badge and wallet actionsCreated
hooks/use-notifications.ts- Client-side notification state management hook that subscribes to bounty updates and new applications, with deduplication, read/unread tracking, and auto-capping at 25 notificationscomponents/notifications/notification-center.tsx- Bell icon popover UI with unread badge, notification list with relative timestamps, mark-as-read functionality, loading skeleton, and empty stateImplementation Details
type:idkey to prevent duplicates from reconnectionsuseGraphQLSubscriptionlifecycleAcceptance Criteria
Closes #131
Summary by CodeRabbit