Skip to content

discord linking#1843

Open
efstajas wants to merge 3 commits intomainfrom
jason/discord-linking
Open

discord linking#1843
efstajas wants to merge 3 commits intomainfrom
jason/discord-linking

Conversation

@efstajas
Copy link
Contributor

@efstajas efstajas commented Feb 1, 2026

Frontend piece for the new Discord account linking feature in wave-api

@efstajas efstajas changed the title Jason/discord linking discord linking Feb 1, 2026
@railway-app railway-app bot temporarily deployed to Drips App / app-pr-1843 February 1, 2026 16:58 Destroyed
@railway-app
Copy link

railway-app bot commented Feb 1, 2026

🚅 Deployed to the app-pr-1843 environment in Drips App

Service Status Web Updated (UTC)
App ✅ Success (View Logs) Web Feb 2, 2026 at 4:59 pm

@efstajas
Copy link
Contributor Author

efstajas commented Feb 1, 2026

Code Review: Discord Linking

Overview

This PR adds Discord account linking functionality to the Wave feature. Users can link their Discord accounts to receive automatic roles in the Drips Discord server based on their Wave activity (contributor/maintainer roles).

What the PR Does

  • Adds a new Discord spot icon component
  • Creates API utilities for Discord linking/unlinking operations
  • Adds a "Connected Accounts" section to the Identity & Payments settings page
  • Implements a full Discord OAuth flow with dedicated pages for:
    • Link initiation (/wave/link-discord)
    • OAuth callback handling (/wave/link-discord/callback)
    • Success confirmation (/wave/link-discord/callback/success)

Positive Aspects

  • Good security practices: Uses isSafePath() to validate return URLs and prevent open redirects
  • Proper error handling: Custom LinkingError class with error codes for different failure scenarios
  • Good UX: Confirmation modal before unlinking, loading states (linking/unlinking), clear error messages
  • Type safety: Zod schemas for API responses and state validation
  • SSR disabled appropriately for the callback page that needs client-side URL parsing
  • Clean component reuse: Uses existing FlowStepWrapper, Setting, AnnotationBox components

Suggestions for Improvement

1. Duplicated Code (Medium)

The linked account display and unlink logic is duplicated between:

  • identity-and-payments/+page.svelte
  • link-discord/+page.svelte

Also handleUnlinkDiscord is nearly identical in both files.

Suggestion: Extract a LinkedDiscordAccount.svelte component and a shared unlinkDiscord helper.

2. Missing Error Handling in redeemDiscordLink (Medium)

In src/lib/utils/wave/discord.ts, if the API call fails, the Zod parse will throw a generic error rather than a meaningful API error.

Suggestion: Check for error response before parsing.

3. Potential Race Condition (Low)

In callback/+page.svelte, if the user navigates away and back quickly, onMount could run multiple times while performLinking is still in progress.

Suggestion: Add an isLinking guard to prevent duplicate calls.

4. CSS Duplication (Low)

The .linked-account-card, .discord-avatar, and .account-info styles are duplicated between the two page components.

Suggestion: Extract to a shared CSS file or component.

5. Error Message Mapping (Low)

The expired error is thrown for HTTP 400, which could represent other errors too.


Security Considerations

  • Open redirect prevention is handled via isSafePath()
  • OAuth state parameter is validated ✓
  • CSRF protection relies on the state parameter from the API ✓

Summary

This is a well-structured PR that follows project conventions. The suggestions above are optional improvements that could be addressed in follow-up work.

Ready to merge 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant