System announcements and email verfication#185
Merged
Conversation
FirepitProject ID: Tip HTTPS and SSL certificates are handled automatically for all your Sites |
Contributor
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR adds a comprehensive instance announcements system and email verification feature to the application. It introduces new Appwrite collections and database schemas for announcements and deliveries, adds two feature flags ( ChangesAnnouncement & Email Verification Feature Implementation
Sequence DiagramsequenceDiagram
participant User as User / Admin
participant UI as Login / Announcement UI
participant Server as Next.js Server
participant Auth as Appwrite Auth
participant DB as Appwrite DB
participant Email as Email Service
rect rgba(100, 150, 255, 0.5)
Note over User,Email: Email Verification Flow
User->>UI: Enter email & password, submit login
UI->>Server: loginAction(formData)
Server->>Auth: createEmailPasswordSession()
Auth-->>Server: session ($id, userId)
Server->>DB: Users.get(userId) → emailVerification
alt Email Verification Enabled & Not Verified
Server->>Auth: account.createEmailVerification()
Auth->>Email: Send verification email
Email-->>User: Verification email with secret link
Server->>Auth: Delete session
Server-->>UI: {verificationRequired: true}
UI->>User: Show "Check your email" toast
User->>Email: Click verification link
Email->>Server: GET /api/auth/verify-email?userId=X&secret=Y
Server->>Auth: account.updateVerification({userId, secret})
Auth-->>Server: success
Server-->>User: Redirect to /login?verified=1
User->>UI: See success toast
else Verified or Feature Disabled
Server-->>UI: {success: true}
UI->>User: Redirect to home
end
end
rect rgba(150, 200, 100, 0.5)
Note over User,Email: Announcement Dispatch Flow
User->>UI: Admin creates & schedules announcement
UI->>Server: createAnnouncementAction(announcement)
Server->>DB: Create announcement document
DB-->>Server: {id, status: "scheduled"}
Server-->>UI: {announcement, ...}
UI->>User: Show success toast
Note over Server,DB: Later: Scheduled time arrives
Server->>Server: dispatchScheduledAnnouncements()
Server->>DB: Query announcements where scheduledFor ≤ now
DB-->>Server: [announcement1, announcement2, ...]
Server->>DB: Update announcement to "dispatching"
Server->>DB: List all profile users (recipients)
DB-->>Server: [userId1, userId2, ...]
loop For each recipient
Server->>DB: Check delivery record (announcement+recipient)
alt Not delivered yet
Server->>DB: Ensure conversation exists
Server->>DB: Create/insert direct message
Server->>DB: Mark delivery as "delivered"
else Already delivered
Note over Server: Skip
end
end
Server->>DB: Aggregate delivery statuses
Server->>DB: Update announcement {status: "sent", deliverySummary: {...}}
Server-->>User: Announcements sent to all recipients
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
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)
✨ Simplify code
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 |
This was referenced Apr 19, 2026
Merged
Owner
Author
This was referenced Apr 19, 2026
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/auth-server.ts (1)
30-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't collapse the system sender into an unauthenticated session.
getServerSession()is also used by routes that intentionally special-case the reserved account. In particular,src/app/api/direct-messages/route.ts:436-442checkssession.$id === SYSTEM_SENDER_USER_IDto make announcement threads writable for the system sender, andsrc/app/api/direct-messages/route.ts:1227-1233uses the same identity to allow sends into those threads. Returningnullhere makes both branches unreachable, so the configured system sender can no longer read or post in its own announcement conversations.Suggested direction
- const systemSenderUserId = process.env.SYSTEM_SENDER_USER_ID?.trim() || null; @@ - if (systemSenderUserId && user.$id === systemSenderUserId) { - return null; - } - return { $id: user.$id, name: user.name, email: user.email,Keep
getServerSession()as the raw session lookup, and enforce the reserved-account block in higher-level interactive auth paths such asrequireAuth()or the login actions instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/auth-server.ts` around lines 30 - 62, getServerSession currently treats the configured SYSTEM_SENDER_USER_ID as unauthenticated and returns null when user.$id === systemSenderUserId; instead preserve and return the session for that reserved account (do not collapse it to null) and move the “deny or special-case” logic into higher-level auth gates (e.g., requireAuth(), login action handlers, or other interactive paths) so those functions explicitly enforce or special-case systemSenderUserId where needed; update the code paths that previously relied on getServerSession() returning null to check for systemSenderUserId themselves.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/setup-appwrite.ts`:
- Around line 1283-1286: Change the conversations index created via
ensureIndex("conversations", "idx_announcement_thread", "key",
["isSystemAnnouncementThread","announcementThreadKey"]) to use a "unique" index
type instead of "key"; update the ensureIndex call for "idx_announcement_thread"
to pass "unique" so the DB enforces uniqueness on (isSystemAnnouncementThread,
announcementThreadKey) and prevents duplicate system announcement threads
created by the read-then-create flow in src/lib/appwrite-announcements.ts.
In `@src/app/`(auth)/login/actions.ts:
- Around line 344-467: resendVerificationAction duplicates client/session/setup
and auth-error logic found in loginAction; extract shared pieces into small
helpers (e.g., a function to build Appwrite client/account like
createAppwriteClient(endpoint, project, apiKey), a function to create and return
an email/password session like createEmailPasswordSession(account, email,
password) that also handles systemSenderUserId check and optional
reserved-account cleanup, and a helper parseAuthError(error) to centralize the
error-to-user-message mapping) and call those from both resendVerificationAction
and loginAction; ensure helpers return enough context (session, accountUser, or
structured error) so resendVerificationAction can still perform its unique flow
(checking emailVerification and calling sendVerificationEmailForSession) and
always perform the best-effort session deletion via a shared cleanup helper
(deleteSessionBestEffort) to avoid duplicating try/catch cleanup code.
In `@src/app/`(auth)/login/page.tsx:
- Around line 25-43: The page keeps the verified query param in the URL so the
same toast reappears on refresh/navigation; after handling the verified value in
the useEffect (where you read searchParams.get("verified") and update
notifiedVerificationStatusRef.current), remove/consume that query param from the
URL so it no longer triggers again — use the Next.js client router (useRouter)
and call router.replace to navigate to the same path without the "verified"
param (do this after showing toast and after setting
notifiedVerificationStatusRef.current in the useEffect).
In `@src/app/`(auth)/register/page.tsx:
- Around line 68-70: When result.verificationRequired is true we currently call
toast.success(result.error) and router.push("/login") which drops the
previously-computed destination; instead preserve the pending post-signup
redirect by passing the existing destination through to the login flow (e.g.,
include destination as a query param or state when calling router.push) so after
verification and sign-in the user is redirected to the original
invite/deep-link. Update the branch that checks result.verificationRequired to
forward the destination variable (from the earlier lines) to router.push rather
than hardcoding "/login".
In `@src/app/admin/announcement-panel.tsx`:
- Around line 107-149: When starting a fresh fetch in loadAnnouncements (i.e.,
when cursorAfter is falsy), clear the first-page state before making the request
by resetting announcements and nextCursor so previous results cannot be mixed
with the new query; specifically, inside loadAnnouncements where you check if
(cursorAfter) { ... } else { ... }, call setAnnouncements([]) and
setNextCursor(undefined) (or null) before awaiting getAnnouncementsAction, then
proceed to setAnnouncements(result.items) and setNextCursor(result.nextCursor)
as you already do. Ensure you still set the appropriate loading flags
(setIsLoading / setIsLoadingMore) and preserve the existing error handling path.
In `@src/app/api/announcements/route.ts`:
- Around line 229-234: The error handling block using "if (error instanceof
Error) { return NextResponse.json({ success: false, error: error.message }, {
status: 400 }) }" can leak internal details; update the handler (the route
function) to map known error types (e.g., validation errors, Appwrite errors,
database errors) to specific safe messages and status codes, and for all other
unexpected errors return a generic message like "An unexpected error occurred"
(do not include error.message) while still logging the original error
server-side; keep the NextResponse.json shape { success: false, error: "<safe
message>" } and adjust status codes accordingly.
In `@src/app/api/direct-messages/route.ts`:
- Around line 1228-1236: The current guard only blocks replies when
isSystemAnnouncementThread is true, allowing conversations to be created with
SYSTEM_SENDER_USER_ID and bypassing read-only intent; update the logic so either
conversation creation or reply handling enforces the reserved account rule: in
the conversation creation branch (the handler for type=conversation) reject or
mark the thread as system-only if SYSTEM_SENDER_USER_ID is included unless
isSystemAnnouncementThread is explicitly set, or alternatively augment the POST
guard to also check whether the conversation's participants include
SYSTEM_SENDER_USER_ID and treat it as read-only (the functions/variables to
touch: the type=conversation request handling, getRelationshipStatus(),
isSystemAnnouncementThread, and SYSTEM_SENDER_USER_ID). Ensure the fix prevents
creating writable conversations with the reserved system sender and/or always
blocks POSTs to any thread involving SYSTEM_SENDER_USER_ID.
In `@src/app/chat/components/ConversationList.tsx`:
- Around line 194-196: The current early return on
conversation.isSystemAnnouncementThread prevents removal from
filteredConversations but later logic switches to unreadConversations and hides
read announcement threads when any other DM is unread; update the selection
logic that chooses between filteredConversations and unreadConversations (the
code that builds/uses unreadConversations and filteredConversations and the
place where the rendered list is chosen) so that system announcement threads are
always included: either add conversations with
conversation.isSystemAnnouncementThread into unreadConversations (or the unread
set) or take the union of unreadConversations and filteredConversations where
isSystemAnnouncementThread is true before rendering. Ensure you reference
conversation.isSystemAnnouncementThread, filteredConversations, and
unreadConversations when making the change.
In `@src/lib/appwrite-announcements.ts`:
- Around line 425-469: listAllProfileUserIds currently accumulates all user IDs
in memory (recipientIds) which can cause memory pressure for large user bases;
change the function to stream or process results in batches instead of returning
a full array—either convert it to an async generator that yields IDs as they are
retrieved or add a callback/handler parameter (e.g., onId or onBatch) that is
invoked per document or per page inside the while loop where
databases.listDocuments is called; keep the existing cursor handling
(cursorAfter, lastDocument) and filtering logic (excludeUserId, trimming userId)
but remove the final Array.from(new Set(...)) aggregation or de-duplicate
incrementally to avoid storing all IDs at once.
- Around line 856-928: The recipient dispatch loop in
dispatchScheduledAnnouncements currently awaits each dispatchToRecipient
sequentially which is slow for large recipient lists; change the loop to perform
batched parallel dispatch with a configurable concurrency limit: fetch
recipientIds via listAllProfileUserIds(systemSenderUserId), chunk recipientIds
into batches (size from a new DISPATCH_CONCURRENCY env/config value), and for
each chunk call Promise.all on dispatchToRecipient calls for that chunk (catch
per-recipient errors so one failure doesn't abort the batch); optionally add a
short delay between batches to avoid DB overload and keep rollup/finalize logic
unchanged (use function names dispatchScheduledAnnouncements,
dispatchToRecipient, listAllProfileUserIds, rollupDeliveryStatus,
finalizeAnnouncementDispatch to locate where to implement).
---
Outside diff comments:
In `@src/lib/auth-server.ts`:
- Around line 30-62: getServerSession currently treats the configured
SYSTEM_SENDER_USER_ID as unauthenticated and returns null when user.$id ===
systemSenderUserId; instead preserve and return the session for that reserved
account (do not collapse it to null) and move the “deny or special-case” logic
into higher-level auth gates (e.g., requireAuth(), login action handlers, or
other interactive paths) so those functions explicitly enforce or special-case
systemSenderUserId where needed; update the code paths that previously relied on
getServerSession() returning null to check for systemSenderUserId themselves.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f666dab5-cc16-4f6d-a54e-0497f0a40fb8
⛔ Files ignored due to path filters (2)
.env.local.exampleis excluded by none and included by nonebun.lockis excluded by!**/*.lockand included bybun.lock
📒 Files selected for processing (22)
package.jsonscripts/setup-appwrite.tssrc/__tests__/auth-server.test.tssrc/__tests__/feature-flags.test.tssrc/__tests__/login-security.test.tssrc/app/(auth)/login/actions.tssrc/app/(auth)/login/page.tsxsrc/app/(auth)/register/page.tsxsrc/app/admin/actions.tssrc/app/admin/announcement-panel.tsxsrc/app/admin/page.tsxsrc/app/api/announcements/dispatch/route.tssrc/app/api/announcements/route.tssrc/app/api/auth/verify-email/route.tssrc/app/api/direct-messages/route.tssrc/app/chat/components/ConversationList.tsxsrc/app/chat/components/DirectMessageView.tsxsrc/lib/appwrite-announcements.tssrc/lib/auth-server.tssrc/lib/feature-flags-definitions.tssrc/lib/feature-flags.tssrc/lib/types.ts
Owner
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved and changes approved. |
Owner
Author
Merge activity
|
acarlson33
changed the base branch from
04-15-polls_in_messages_and_server_discovery_customization_updates
to
graphite-base/185
acarlson33
force-pushed
the
04-17-system_announcements_and_email_verfication
branch
from
100a506 to
6cfd7a9
Compare
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.
This pull request introduces support for instance-wide system DM announcements, adds new feature flags, and enhances the Appwrite schema to support announcement delivery and improved login security. It also includes minor dependency updates and improved test coverage for new features.
Instance Announcements Support:
.env.local.examplefor announcements collections, system sender user ID, and announcement dispatcher secret. [1] [2]scripts/setup-appwrite.ts) to create new collections and attributes forannouncementsandannouncement_deliveries, including relevant indexes and fields for managing announcement delivery and status. [1] [2] [3]Feature Flags:
enable_instance_announcementsandenable_email_verificationfeature flags to the setup script and tests, and updated theFEATURE_FLAGSobject accordingly. [1] [2]Appwrite Schema Enhancements:
conversationsanddirect_messagescollections with new attributes and indexes to support system announcement threads, read-only reasons, announcement references, and priority tags. [1] [2]Login Security and Testing:
Dependency Updates:
next,posthog-js,@next/bundle-analyzer, andeslint-config-nextinpackage.json. [1] [2] [3]