[feat]: add discord auto-join#404
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds end-to-end Discord OAuth 2.0 integration and Pro Guild membership automation. A new ChangesDiscord OAuth & Pro Guild Automation
Sequence Diagram(s)sequenceDiagram
actor User
participant WebApp as Web (Account/Checkout)
participant APIServer as API Server
participant discordService as Discord Service
participant DiscordOAuth as Discord OAuth
participant PrismaDB as Database
User->>WebApp: Click "Join Pro Community"
WebApp->>APIServer: GET /discord/connect-url (Bearer token)
APIServer->>discordService: buildAuthorizationUrl(state)
APIServer-->>WebApp: { authUrl }
WebApp->>DiscordOAuth: Redirect to authUrl
User->>DiscordOAuth: Authorize app
DiscordOAuth->>APIServer: GET /auth/discord/callback?code=&state=
APIServer->>discordService: exchangeCodeForToken(code)
discordService->>DiscordOAuth: POST /oauth2/token
DiscordOAuth-->>discordService: token response
discordService->>DiscordOAuth: GET /users/@me
DiscordOAuth-->>discordService: user profile
discordService->>PrismaDB: upsert account + update user.discordUserId
discordService->>DiscordOAuth: PUT /guilds/{guildId}/members/{userId} (bot)
DiscordOAuth-->>discordService: 201/204
APIServer-->>WebApp: Redirect to success URL ?discord=joined
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
apps/api/src/services/payment.service.ts (1)
343-356: ⚡ Quick winAdd structured context to Discord sync error log.
Per coding guidelines, error logs should include context (userId, endpoint, timestamp) for debugging. The current log message lacks the user identifier.
♻️ Proposed fix
} catch (error) { - // don't fail payment flow if discord sync fails - console.error("Failed to sync Discord membership after payment:", error); + // don't fail payment flow if discord sync fails + console.error( + JSON.stringify({ + level: "error", + service: "paymentService", + operation: "createSubscription", + event: "discord_sync_failed", + userId, + subscriptionId: subscription.id, + timestamp: new Date().toISOString(), + errorMessage: error instanceof Error ? error.message : String(error), + }) + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/services/payment.service.ts` around lines 343 - 356, The error log in the catch block within the Discord sync section lacks necessary debugging context. Update the console.error call to include the userId parameter (which is available in scope) along with the error message to provide proper context for troubleshooting Discord membership sync failures. Include the userId in the log message formatted with the error details to aid debugging.Source: Coding guidelines
apps/api/src/index.ts (2)
370-398: ⚖️ Poor tradeoffSequential updates without transaction risk partial state on failure.
If the job fails midway (e.g., after expiring some subscriptions but before completing the loop), some subscriptions will be marked
EXPIREDwhile others remainACTIVEdespite being pastendDate. Consider wrapping the batch in a transaction, or at minimum, ensure the job can be safely re-run (idempotent).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/index.ts` around lines 370 - 398, The for loop iterating over expiredSubscriptions performs sequential updates to subscription status without a transaction, risking inconsistent state if the job fails midway. Wrap the entire for loop in a database transaction using prismaModule.prisma.$transaction() to make all subscription status updates and Discord removal operations atomic, ensuring either all subscriptions are processed or none are, making the job safely re-entrant if it needs to be retried on failure.
380-386: ⚡ Quick winAvoid
as any; narrow with a proper type.The coding guidelines require avoiding
anyand using type guards for dynamic data. Define an interface for the user with the optionaldiscordUserIdfield, or use a type guard to narrow the type safely.♻️ Proposed fix
+interface UserWithDiscord { + discordUserId?: string | null; +} + for (const subscription of expiredSubscriptions) { await prismaModule.prisma.subscription.update({ where: { id: subscription.id }, data: { status: SUBSCRIPTION_STATUS.EXPIRED }, }); if (!discordService.isAutomationEnabled()) { continue; } - const discordUserId = (subscription.user as any).discordUserId as - | string - | null - | undefined; + const discordUserId = (subscription.user as UserWithDiscord).discordUserId; if (!discordUserId) { continue; }Alternatively, if
discordUserIdis defined in the PrismaUsermodel schema (as indicated by the migration in this PR stack), the type should already be available from the generated Prisma types. Verify that theinclude: { user: true }relation returns the correct typed user object.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/index.ts` around lines 380 - 386, Replace the `as any` type assertion when accessing `discordUserId` from `subscription.user` with proper typing. First, verify that the Prisma User model includes the `discordUserId` field (as indicated by the migration) and that the subscription query properly includes the user relation. If the Prisma types are correctly generated, remove the `as any` cast and directly access `subscription.user.discordUserId` to rely on the proper type from the generated Prisma types. If the field is optional or conditionally present, define a type guard or interface that properly represents the user object with the optional `discordUserId` field instead of using `any`.Source: Coding guidelines
apps/web/src/components/checkout/checkout-confirmation.tsx (1)
20-20: 💤 Low value
successMessagestate is never set and renders nothing.The state is initialized to
null, cleared in the handler, but never assigned a meaningful value. On successful API response, the component redirects away viawindow.location.hrefbefore any success message could be shown. This results in dead code.Either remove the unused state and JSX, or if a success message is needed for a different flow, set it appropriately before redirect.
🧹 Remove unused successMessage state
const [error, setError] = useState<string | null>(null); - const [successMessage, setSuccessMessage] = useState<string | null>(null); const [isJoining, setIsJoining] = useState(false);setIsJoining(true); setError(null); - setSuccessMessage(null);{error && <p className="text-error-text text-sm mt-2">{error}</p>} - {successMessage && ( - <p className="text-success-text text-sm mt-2">{successMessage}</p> - )}Also applies to: 39-39, 130-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/checkout/checkout-confirmation.tsx` at line 20, Remove the unused successMessage state from the checkout-confirmation component since it is initialized to null and never assigned a meaningful value before the component redirects via window.location.href. Delete the successMessage state declaration (the useState hook), remove any JSX that renders the successMessage, and remove any code that clears or attempts to set this state variable across all locations (lines 20, 39, and 130-132 as indicated in the comment).apps/api/src/services/discord.service.ts (1)
114-121: ⚡ Quick winreplace
anywith prisma input types on persistence payloads.using
anyhere removes compile-time guarantees on a critical write path.as per coding guidelines, "avoid
anytype; useunknownfor truly dynamic data and narrow with type guards".Also applies to: 154-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/services/discord.service.ts` around lines 114 - 121, The `updateData` and `createData` variables are declared with `any` type, which removes compile-time type safety on these critical database write operations. Replace the `any` type annotations with the appropriate Prisma input types for the Discord account/token entity being updated and created. Determine the correct Prisma model name from your schema and use its corresponding CreateInput and UpdateInput types instead of `any`. This fix should be applied to both occurrences mentioned in the comment.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/index.ts`:
- Around line 269-330: The OAuth callback endpoint at "/auth/discord/callback"
is missing rate limiting protection that other Discord endpoints have. Add the
apiLimiter middleware to this route definition (similar to how it's applied to
the "/discord/connect-url" and "/discord/community-status" endpoints) to prevent
attackers from brute-forcing state tokens or flooding the Discord API with
excessive token exchange requests. Apply the middleware to the app.get call for
the callback endpoint to enforce the rate limit on all incoming requests to this
endpoint.
In `@apps/api/src/services/discord.service.ts`:
- Around line 71-77: All fetch calls to the Discord API endpoints in this
service (the oauth2/token endpoint at lines 71-77 and other Discord API calls at
lines 88-93, 172-184, 195-203, and 220-228) are missing timeout configuration,
which can cause requests to hang indefinitely if Discord is slow or
unresponsive. Add a timeout signal or abort controller to each fetch call to
enforce a reasonable timeout duration (typically a few seconds for external API
calls). Apply this consistently across all Discord API calls in the file to
prevent request handlers from being pinned by slow/hung Discord services.
- Around line 141-155: The account upsert and user update operations are not
atomic, creating a race condition where the account could be reassigned to a new
user while the old user still references the discord ID. Wrap both the account
upsert and user update operations in a prisma.$transaction call to ensure
atomicity. Additionally, add validation logic before the transaction to check if
the discordUserId is already assigned to a different user and block the
reassignment if it would create a cross-user conflict, ensuring data consistency
between the account and user tables.
In `@apps/api/src/utils/discord-oauth-state.ts`:
- Around line 3-8: The getStateSecret function currently uses a hardcoded
fallback string "discord-oauth-state-fallback" which creates a security
vulnerability by allowing forged state values when environment variables are
missing. Remove the hardcoded fallback and instead throw an error if both
DISCORD_OAUTH_STATE_SECRET and JWT_SECRET environment variables are undefined.
This ensures the application fails closed (safely) rather than proceeding with a
predictable secret that could be exploited.
---
Nitpick comments:
In `@apps/api/src/index.ts`:
- Around line 370-398: The for loop iterating over expiredSubscriptions performs
sequential updates to subscription status without a transaction, risking
inconsistent state if the job fails midway. Wrap the entire for loop in a
database transaction using prismaModule.prisma.$transaction() to make all
subscription status updates and Discord removal operations atomic, ensuring
either all subscriptions are processed or none are, making the job safely
re-entrant if it needs to be retried on failure.
- Around line 380-386: Replace the `as any` type assertion when accessing
`discordUserId` from `subscription.user` with proper typing. First, verify that
the Prisma User model includes the `discordUserId` field (as indicated by the
migration) and that the subscription query properly includes the user relation.
If the Prisma types are correctly generated, remove the `as any` cast and
directly access `subscription.user.discordUserId` to rely on the proper type
from the generated Prisma types. If the field is optional or conditionally
present, define a type guard or interface that properly represents the user
object with the optional `discordUserId` field instead of using `any`.
In `@apps/api/src/services/discord.service.ts`:
- Around line 114-121: The `updateData` and `createData` variables are declared
with `any` type, which removes compile-time type safety on these critical
database write operations. Replace the `any` type annotations with the
appropriate Prisma input types for the Discord account/token entity being
updated and created. Determine the correct Prisma model name from your schema
and use its corresponding CreateInput and UpdateInput types instead of `any`.
This fix should be applied to both occurrences mentioned in the comment.
In `@apps/api/src/services/payment.service.ts`:
- Around line 343-356: The error log in the catch block within the Discord sync
section lacks necessary debugging context. Update the console.error call to
include the userId parameter (which is available in scope) along with the error
message to provide proper context for troubleshooting Discord membership sync
failures. Include the userId in the log message formatted with the error details
to aid debugging.
In `@apps/web/src/components/checkout/checkout-confirmation.tsx`:
- Line 20: Remove the unused successMessage state from the checkout-confirmation
component since it is initialized to null and never assigned a meaningful value
before the component redirects via window.location.href. Delete the
successMessage state declaration (the useState hook), remove any JSX that
renders the successMessage, and remove any code that clears or attempts to set
this state variable across all locations (lines 20, 39, and 130-132 as indicated
in the comment).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a9d6e29-f0ab-46f3-91a8-d308ebb56a09
📒 Files selected for processing (9)
apps/api/.env.exampleapps/api/prisma/migrations/20260617090052_add_discord_user_id/migration.sqlapps/api/prisma/schema.prismaapps/api/src/index.tsapps/api/src/services/discord.service.tsapps/api/src/services/payment.service.tsapps/api/src/utils/discord-oauth-state.tsapps/web/src/app/(main)/dashboard/account/page.tsxapps/web/src/components/checkout/checkout-confirmation.tsx
apsinghdev
force-pushed
the
feat/discord-integration
branch
from
June 18, 2026 08:11
a0d9882 to
21505ba
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api/src/index.ts (1)
188-188: ⚡ Quick winremove
anyfrom typed api paths.line 188 (
error: any) and lines 380-383 (subscription.user as any) bypass type safety and can hide contract drift. useunknownfor caught errors and strongly type the included user shape.as per coding guidelines, "apps/api/src/**/*.ts: avoid
anytype; useunknownfor truly dynamic data and narrow with type guards."proposed fix
- } catch (error: any) { + } catch (error: unknown) {- const expiredSubscriptions = await prismaModule.prisma.subscription.findMany({ + const expiredSubscriptions = await prismaModule.prisma.subscription.findMany({ where: { status: SUBSCRIPTION_STATUS.ACTIVE, endDate: { lt: new Date() }, }, include: { - user: true, + user: { select: { discordUserId: true } }, }, take: 500, }); @@ - const discordUserId = (subscription.user as any).discordUserId as - | string - | null - | undefined; + const discordUserId = subscription.user.discordUserId;Also applies to: 347-354, 380-383
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/index.ts` at line 188, Replace the `any` type annotation in the catch block (error: any) with `unknown` to maintain type safety for error handling. Additionally, remove the `as any` type assertion from subscription.user and replace it with explicit typing that properly defines the user shape instead of bypassing type checking. Apply the same changes throughout the file where similar `any` annotations appear in catch blocks and type assertions, following the coding guideline to avoid `any` and use `unknown` for dynamic data that requires type narrowing.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/index.ts`:
- Around line 337-340: The CRON_SECRET validation is currently being performed
lazily within an endpoint handler rather than at application startup, which
violates the fail-fast principle for required environment variables. Move the
CRON_SECRET check (currently checking if !cronSecret) from inside the endpoint
handler to the startup/initialization code at the top level of index.ts,
ensuring the application fails immediately during startup if this required
environment variable is missing. This aligns with the guideline that required
environment variables must be validated during startup in index.ts.
- Around line 347-398: The subscription status is being updated to
SUBSCRIPTION_STATUS.EXPIRED before the Discord removal attempt is made, and
subsequent runs only query for ACTIVE subscriptions. When Discord removal fails
in the catch block, the subscription remains marked as EXPIRED and becomes
invisible to future runs, preventing retry attempts. Fix this by moving the
subscription status update for the EXPIRED status to occur only after a
successful Discord removal (or when Discord automation is disabled). Keep the
subscription in ACTIVE status if Discord removal fails so the next run can retry
the operation.
- Around line 209-217: The verifyToken(token) call on line 210 (and line 242 in
the similar route) currently allows token verification failures to fall into the
outer catch block, which returns a generic 500 error. Instead, add a dedicated
try-catch block around the verifyToken(token) call that catches token
verification errors and returns a 401 status code with an appropriate error
message like "Unauthorized" or "Invalid token", while allowing other unexpected
errors to be caught by the existing outer catch block that returns 500. This
ensures authentication failures are properly distinguished from internal server
errors.
---
Nitpick comments:
In `@apps/api/src/index.ts`:
- Line 188: Replace the `any` type annotation in the catch block (error: any)
with `unknown` to maintain type safety for error handling. Additionally, remove
the `as any` type assertion from subscription.user and replace it with explicit
typing that properly defines the user shape instead of bypassing type checking.
Apply the same changes throughout the file where similar `any` annotations
appear in catch blocks and type assertions, following the coding guideline to
avoid `any` and use `unknown` for dynamic data that requires type narrowing.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93e13375-6677-4697-80c4-dd012ad422ea
📒 Files selected for processing (9)
apps/api/.env.exampleapps/api/prisma/migrations/20260617090052_add_discord_user_id/migration.sqlapps/api/prisma/schema.prismaapps/api/src/index.tsapps/api/src/services/discord.service.tsapps/api/src/services/payment.service.tsapps/api/src/utils/discord-oauth-state.tsapps/web/src/app/(main)/dashboard/account/page.tsxapps/web/src/components/checkout/checkout-confirmation.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/prisma/migrations/20260617090052_add_discord_user_id/migration.sql
- apps/api/src/services/payment.service.ts
- apps/web/src/app/(main)/dashboard/account/page.tsx
- apps/web/src/components/checkout/checkout-confirmation.tsx
- apps/api/src/services/discord.service.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 by CodeRabbit
Release Notes
New Features
UI Enhancements