Repository navigation
feat: spotify oauth account linking - #574
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds end-to-end Spotify integration: DB schema + migration, shared link service with token refresh, backend OAuth routes and auth service, frontend page and API client, Discord bot command and playback handler changes, plus tests across backend, bot, shared, and frontend packages. Changes
Sequence DiagramssequenceDiagram
participant User as User (Browser)
participant Backend as Backend API
participant SpotifyAPI as Spotify Authorization
participant DB as Database
User->>Backend: GET /api/spotify/connect
activate Backend
Backend->>Backend: Generate HMAC state (Discord ID)
Backend->>User: Set httpOnly state cookie\nRedirect to Spotify authorize
deactivate Backend
User->>SpotifyAPI: Authorize (consent)
activate SpotifyAPI
SpotifyAPI->>User: Redirect to /api/spotify/callback (code + state)
deactivate SpotifyAPI
User->>Backend: GET /api/spotify/callback (code,state)
activate Backend
Backend->>Backend: Validate HMAC state
Backend->>SpotifyAPI: POST /api/token (exchange code)
activate SpotifyAPI
SpotifyAPI->>Backend: Return access/refresh tokens
deactivate SpotifyAPI
Backend->>SpotifyAPI: GET /v1/me (profile)
activate SpotifyAPI
SpotifyAPI->>Backend: Return spotify id / display_name
deactivate SpotifyAPI
Backend->>DB: Upsert SpotifyLink (tokens, user info)
activate DB
DB-->>Backend: Confirm stored
deactivate DB
Backend->>User: Clear state cookie\nRedirect to frontend (success)
deactivate Backend
sequenceDiagram
participant User as User (Browser)
participant Frontend as Frontend UI
participant Backend as Backend API
participant DB as Database
rect rgba(100,150,200,0.5)
User->>Frontend: Open /spotify
Frontend->>Backend: GET /api/spotify/status
Backend->>DB: Query spotify_links by discordId
DB-->>Backend: Return link or null
Backend-->>Frontend: {configured, linked, username}
Frontend->>User: Render page state
end
rect rgba(200,150,100,0.5)
User->>Frontend: Click "Disconnect"
Frontend->>User: Confirm
User->>Frontend: Confirm
Frontend->>Backend: DELETE /api/spotify/unlink
Backend->>DB: Delete spotify_links row
DB-->>Backend: Confirm deleted (or not found)
Backend-->>Frontend: {success: true/false}
Frontend->>User: Update UI / Show error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Size Change: +1.8 kB (+0.56%) Total Size: 325 kB 📦 View Changed
ℹ️ View Unchanged
|
- Extract spotify command subcommands into separate handler functions to reduce execute() complexity from 16 to ~10 - Refactor spotifyHandler to eliminate duplication between track and playlist handlers via shared handleSpotifyUrl() function - Add comprehensive test coverage for spotifyHandler (7 tests) - Add comprehensive test coverage for spotifyConfig (6 tests) - Exclude spotify files from CPD to allow intentional OAuth patterns
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
packages/shared/src/services/SpotifyLinkService/index.spec.ts (1)
23-25: Restore mutated globals between specs.Later tests overwrite Spotify env vars and
global.fetch, but this setup only clears Jest mocks. That makes the suite order-dependent and can leak state into other files running in the same worker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/services/SpotifyLinkService/index.spec.ts` around lines 23 - 25, The test setup only calls jest.clearAllMocks() in beforeEach and doesn't restore mutated globals like process.env and global.fetch, causing order-dependent flaky tests; fix by capturing originals (e.g., const ORIGINAL_ENV and const originalFetch) in a setup hook (beforeAll) and restore them in an afterEach (or afterAll) hook, and also call jest.resetModules() as needed to avoid module cache leaks; update the existing beforeEach/jest.clearAllMocks() usage to include restoring process.env and global.fetch and optionally resetting modules so each spec runs with the original global state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/routes/spotify.ts`:
- Around line 212-217: The current callback logic sets state from
parsedQuery.data.state falling back to stateFromCookie which allows replayable
states; change the check in the callback that builds/validates state (the code
using parsedQuery.data.state and stateFromCookie) to require that the query
state exactly matches the cookie state (reject if missing or not equal) or
alternatively validate a signed state payload that includes an issued-at/nonce
and enforce a TTL; update the validation branch that currently assigns to the
state variable so it performs strict equality between parsedQuery.data.state and
stateFromCookie (or verifies the signature and issued-at within TTL) and
throws/returns an error when validation fails.
- Around line 126-129: The current handler maps all unlink failures to 404
because it treats any falsy return from spotifyLinkService.unlink(discordId) as
"no link", but unlink() already returns true for Prisma P2025 (no record) and
false for real internal failures; change the conditional so that a false return
becomes a 500 (internal error) instead of 404 (e.g., if (ok === false) {
res.status(500).json({ error: 'Unable to unlink Spotify account' }); return }),
and if you need to return 404 for the documented not-found path add an explicit
existence check (e.g., spotifyLinkService.findByDiscordId) or update
spotifyLinkService.unlink to return a clear enum/result object so the route can
distinguish success, not-found, and internal errors; reference
spotifyLinkService.unlink and the res.status(...) call to locate the change.
- Around line 185-189: The auth URL currently bakes the state into the
redirect_uri which will not match the fixed SPOTIFY_REDIRECT_URI used by
exchangeCodeForToken; change the logic in this route (the block that uses
resolveBackendBaseUrl, callbackUrl and authUrl) to use the fixed redirect URI
(process.env.SPOTIFY_REDIRECT_URI or the same canonical value
exchangeCodeForToken expects) for the redirect_uri parameter and pass the state
as the separate &state=... OAuth parameter instead (ensure both redirect_uri and
state are properly encodeURIComponent-ed so the values sent to Spotify exactly
match the value used when exchanging the code).
In `@packages/backend/src/services/SpotifyAuthService.ts`:
- Around line 20-53: The SpotifyAuthService outbound fetches to
'https://accounts.spotify.com/api/token' and 'https://api.spotify.com/v1/me'
must use a bounded timeout so the callback doesn't hang; update the code in
SpotifyAuthService to wrap both fetch calls with a shared timeout helper (e.g.,
fetchWithTimeout) or use AbortController: create an AbortController per request,
set a setTimeout to call controller.abort() after a configurable short timeout,
pass controller.signal to fetch, and clear the timer after the response; apply
this to the token exchange fetch and the user profile fetch so both fail fast on
network stalls.
In `@packages/frontend/src/pages/Spotify.tsx`:
- Around line 107-115: The code builds a Spotify profile link using display text
(status.username / spotifyUsername), which is not a stable identifier and
produces broken links; update the rendering in the Spotify page so you do not
construct a URL from status.username: either use a canonical profile URL or user
id returned by the API (e.g., use a field like status.profileUrl or status.id to
build the href) or remove the anchor and render {status.username} as plain text
(keep ExternalLink only when a valid canonical URL field exists). Locate the
anchor around {status.username} and change the href/source accordingly and
ensure rel/target remain correct when you do supply a real URL.
- Around line 81-97: The current conditional treats status === null (e.g., when
loadStatus() fails) the same as configured === false, causing the admin "Not
Configured" panel to show on transient errors; change the conditional in
Spotify.tsx to explicitly check status?.configured === false for that panel and
add a separate branch or fallback to handle status === null (an
error/loading/empty state) so the "Not Configured" UI only appears when status
exists and configured is false; update any relevant rendering around the status
variable and loadStatus() usage to avoid collapsing null into the not-configured
case.
In `@packages/shared/src/services/SpotifyLinkService/index.ts`:
- Around line 64-75: The refresh-token fetch call currently does an unbounded
network request; wrap this POST to 'https://accounts.spotify.com/api/token' in
the same timeout/abort logic used in the authorization-code exchange path:
create an AbortController, start a timeout (matching the existing fetch timeout
constant or value used elsewhere), pass controller.signal into the fetch
options, and clear the timer after the response arrives; ensure the
controller.abort() is invoked on timeout so the refresh flow fails fast on
network hangs. Reference the fetch call in this token refresh block and use
AbortController and the existing timeout value/name from the authorization-code
exchange implementation.
In `@prisma/schema.prisma`:
- Around line 397-399: The accessToken and refreshToken fields are stored
plaintext; update the Prisma model and persistence logic to store encrypted
ciphertext instead: add new fields (e.g., accessTokenEncrypted and
refreshTokenEncrypted as Bytes or String) preserving tokenExpiresAt, run a
migration, and stop writing plaintext to accessToken/refreshToken (or
deprecate/rename them). Implement application-level envelope encryption helpers
(e.g., encryptToken and decryptToken) that call your KMS (with managed key
rotation) to produce ciphertext+metadata/nonce and use those helpers inside the
repository/data-access methods that create/update/read the model (locate code
paths that write/read tokens). Add a migration script to re-encrypt existing
tokens (using current key) or rotate them securely, update tests and any code
referencing accessToken/refreshToken to use the decrypt helper, and ensure
secrets are never logged.
---
Nitpick comments:
In `@packages/shared/src/services/SpotifyLinkService/index.spec.ts`:
- Around line 23-25: The test setup only calls jest.clearAllMocks() in
beforeEach and doesn't restore mutated globals like process.env and
global.fetch, causing order-dependent flaky tests; fix by capturing originals
(e.g., const ORIGINAL_ENV and const originalFetch) in a setup hook (beforeAll)
and restore them in an afterEach (or afterAll) hook, and also call
jest.resetModules() as needed to avoid module cache leaks; update the existing
beforeEach/jest.clearAllMocks() usage to include restoring process.env and
global.fetch and optionally resetting modules so each spec runs with the
original global state.
🪄 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: 63ca8286-1a05-43b7-b849-d9d2473f2de9
📒 Files selected for processing (16)
packages/backend/src/routes/index.tspackages/backend/src/routes/spotify.tspackages/backend/src/services/SpotifyAuthService.tspackages/backend/tests/integration/routes/spotify.test.tspackages/bot/src/functions/music/commands/spotify.spec.tspackages/bot/src/functions/music/commands/spotify.tspackages/bot/src/spotify/index.tspackages/bot/src/spotify/spotifyConfig.tspackages/frontend/src/App.tsxpackages/frontend/src/pages/Spotify.tsxpackages/frontend/src/services/api.tspackages/shared/src/services/SpotifyLinkService/index.spec.tspackages/shared/src/services/SpotifyLinkService/index.tspackages/shared/src/services/index.tsprisma/migrations/20260412100000_add_spotify_link/migration.sqlprisma/schema.prisma
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: compressed-size
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
🔇 Additional comments (7)
prisma/migrations/20260412100000_add_spotify_link/migration.sql (1)
6-7: Duplicate of schema-level security concern (Line 6 to Line 7).This migration persists OAuth tokens in plaintext, matching the same issue already raised on
prisma/schema.prisma.packages/frontend/src/App.tsx (1)
43-43: Spotify route integration is consistent and correctly guarded.Line 43 and Line 198 to Line 201 follow the established lazy-load +
integrationsmodule guard pattern.Also applies to: 198-201
packages/bot/src/spotify/index.ts (1)
1-1: Good barrel export addition.Line 1 cleanly exposes
isSpotifyConfiguredfor consumers without widening the module surface unnecessarily.packages/backend/src/routes/index.ts (1)
8-8: Route registration wiring looks correct.Line 8 and Line 63 properly plug Spotify routes into the central setup pipeline.
Also applies to: 63-63
packages/shared/src/services/index.ts (1)
33-33: Service export is aligned with the shared entrypoint pattern.Line 33 cleanly exposes Spotify link service APIs for backend/bot consumers.
packages/bot/src/spotify/spotifyConfig.ts (1)
1-7: Configuration gate helper is straightforward and fit-for-purpose.The check is concise and matches the bot command’s usage pattern.
packages/frontend/src/services/api.ts (1)
392-401: Spotify API client additions are consistent with existing integration clients.Line 392 to Line 401 follows the same structure as other provider namespaces and keeps the frontend service surface coherent.
Comment on lines
+126
to
+129
| const ok = await spotifyLinkService.unlink(discordId) | ||
| if (!ok) { | ||
| res.status(404).json({ error: 'No Spotify link found' }) | ||
| return |
There was a problem hiding this comment.
Don’t map every unlink failure to 404.
spotifyLinkService.unlink() already returns true for Prisma P2025 and false for real failures, so this branch turns backend errors into “No Spotify link found” and makes the documented not-found path unreachable with the current service contract.
Suggested fix
- const ok = await spotifyLinkService.unlink(discordId)
- if (!ok) {
- res.status(404).json({ error: 'No Spotify link found' })
- return
- }
+ const link = await spotifyLinkService.getByDiscordId(discordId)
+ if (!link) {
+ res.status(404).json({ error: 'No Spotify link found' })
+ return
+ }
+ const ok = await spotifyLinkService.unlink(discordId)
+ if (!ok) {
+ res.status(500).json({ error: 'Failed to unlink' })
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const ok = await spotifyLinkService.unlink(discordId) | |
| if (!ok) { | |
| res.status(404).json({ error: 'No Spotify link found' }) | |
| return | |
| const link = await spotifyLinkService.getByDiscordId(discordId) | |
| if (!link) { | |
| res.status(404).json({ error: 'No Spotify link found' }) | |
| return | |
| } | |
| const ok = await spotifyLinkService.unlink(discordId) | |
| if (!ok) { | |
| res.status(500).json({ error: 'Failed to unlink' }) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/src/routes/spotify.ts` around lines 126 - 129, The current
handler maps all unlink failures to 404 because it treats any falsy return from
spotifyLinkService.unlink(discordId) as "no link", but unlink() already returns
true for Prisma P2025 (no record) and false for real internal failures; change
the conditional so that a false return becomes a 500 (internal error) instead of
404 (e.g., if (ok === false) { res.status(500).json({ error: 'Unable to unlink
Spotify account' }); return }), and if you need to return 404 for the documented
not-found path add an explicit existence check (e.g.,
spotifyLinkService.findByDiscordId) or update spotifyLinkService.unlink to
return a clear enum/result object so the route can distinguish success,
not-found, and internal errors; reference spotifyLinkService.unlink and the
res.status(...) call to locate the change.
Comment on lines
+185
to
+189
| const backendBaseUrl = resolveBackendBaseUrl(req) | ||
| const callbackUrl = `${backendBaseUrl}/api/spotify/callback?state=${encodeURIComponent(state)}` | ||
| const scopes = ['user-top-read', 'user-read-recently-played', 'user-library-read'] | ||
| const authUrl = `https://accounts.spotify.com/authorize?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodeURIComponent(callbackUrl)}&scope=${encodeURIComponent(scopes.join(' '))}` | ||
| res.redirect(authUrl) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Spotify's Authorization Code flow require the redirect_urisent to the token endpoint to exactly match theredirect_uri used in the authorize request?
💡 Result:
Yes, Spotify's Authorization Code flow requires the redirect_uri sent to the token endpoint to exactly match the redirect_uri used in the authorize request.
Citations:
- 1: https://developer.spotify.com/documentation/web-api/tutorials/code-flow
- 2: https://pastranadigital.github.io/Spotify-API-as-MkDocs/tutorials/Authorization-code/
🏁 Script executed:
#!/bin/bash
# Find the exchangeCodeForToken function and related OAuth callback code
rg -A 20 "exchangeCodeForToken" packages/backend/src/ --type tsRepository: LucasSantana-Dev/Lucky
Length of output: 8630
🏁 Script executed:
#!/bin/bash
# Search for the callback handler
rg -B 5 -A 15 "/api/spotify/callback" packages/backend/src/ --type tsRepository: LucasSantana-Dev/Lucky
Length of output: 2994
🏁 Script executed:
#!/bin/bash
# Look for getOAuthRedirectUri function
rg "getOAuthRedirectUri" packages/backend/src/ --type tsRepository: LucasSantana-Dev/Lucky
Length of output: 1137
🏁 Script executed:
#!/bin/bash
# Check for any helper functions related to Spotify OAuth
rg -B 2 -A 10 "resolveBackendBaseUrl" packages/backend/src/ --type tsRepository: LucasSantana-Dev/Lucky
Length of output: 4289
🏁 Script executed:
#!/bin/bash
# Check the getOAuthRedirectUri implementation
cat packages/backend/src/utils/oauthRedirectUri.tsRepository: LucasSantana-Dev/Lucky
Length of output: 1925
🏁 Script executed:
#!/bin/bash
# Check if SPOTIFY_REDIRECT_URI is set and how it's used
rg "SPOTIFY_REDIRECT_URI" packages/backend/src/ --type ts -B 2 -A 2Repository: LucasSantana-Dev/Lucky
Length of output: 945
Use a fixed redirect_uri and send OAuth state as a separate parameter.
The authorization request builds a redirect URI with state baked into the URL (/api/spotify/callback?state=xyz), but exchangeCodeForToken() sends process.env.SPOTIFY_REDIRECT_URI to Spotify's token endpoint. Spotify requires these to match exactly—baking state into the redirect_uri will cause it to mismatch and Spotify will reject the authorization code. State should be sent as an OAuth parameter instead.
Suggested fix
- const backendBaseUrl = resolveBackendBaseUrl(req)
- const callbackUrl = `${backendBaseUrl}/api/spotify/callback?state=${encodeURIComponent(state)}`
+ const callbackUrl = getOAuthRedirectUri(req)
const scopes = ['user-top-read', 'user-read-recently-played', 'user-library-read']
- const authUrl = `https://accounts.spotify.com/authorize?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodeURIComponent(callbackUrl)}&scope=${encodeURIComponent(scopes.join(' '))}`
+ const authUrl = `https://accounts.spotify.com/authorize?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodeURIComponent(callbackUrl)}&scope=${encodeURIComponent(scopes.join(' '))}&state=${encodeURIComponent(state)}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/src/routes/spotify.ts` around lines 185 - 189, The auth URL
currently bakes the state into the redirect_uri which will not match the fixed
SPOTIFY_REDIRECT_URI used by exchangeCodeForToken; change the logic in this
route (the block that uses resolveBackendBaseUrl, callbackUrl and authUrl) to
use the fixed redirect URI (process.env.SPOTIFY_REDIRECT_URI or the same
canonical value exchangeCodeForToken expects) for the redirect_uri parameter and
pass the state as the separate &state=... OAuth parameter instead (ensure both
redirect_uri and state are properly encodeURIComponent-ed so the values sent to
Spotify exactly match the value used when exchanging the code).
Comment on lines
+212
to
+217
| const state = | ||
| typeof parsedQuery.data.state === 'string' | ||
| ? parsedQuery.data.state | ||
| : typeof stateFromCookie === 'string' | ||
| ? stateFromCookie | ||
| : null |
There was a problem hiding this comment.
The callback still accepts replayable state values.
This prefers the query-string state over the cookie, so the 10-minute cookie on /connect does not actually expire old link URLs. Any previously issued signed state can be replayed later as long as it leaks. Require the cookie and query values to match, or sign an issued-at/nonce and enforce TTL on the callback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/src/routes/spotify.ts` around lines 212 - 217, The current
callback logic sets state from parsedQuery.data.state falling back to
stateFromCookie which allows replayable states; change the check in the callback
that builds/validates state (the code using parsedQuery.data.state and
stateFromCookie) to require that the query state exactly matches the cookie
state (reject if missing or not equal) or alternatively validate a signed state
payload that includes an issued-at/nonce and enforce a TTL; update the
validation branch that currently assigns to the state variable so it performs
strict equality between parsedQuery.data.state and stateFromCookie (or verifies
the signature and issued-at within TTL) and throws/returns an error when
validation fails.
Comment on lines
+20
to
+53
| try { | ||
| const res = await fetch('https://accounts.spotify.com/api/token', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Authorization': `Basic ${auth}`, | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| body: new URLSearchParams({ | ||
| grant_type: 'authorization_code', | ||
| code: code.trim(), | ||
| redirect_uri: redirectUri, | ||
| }).toString(), | ||
| }) | ||
|
|
||
| if (!res.ok) { | ||
| return null | ||
| } | ||
|
|
||
| const data = (await res.json().catch(() => null)) as { | ||
| access_token?: string | ||
| refresh_token?: string | ||
| expires_in?: number | ||
| error?: string | ||
| } | ||
|
|
||
| if (data?.error || !data?.access_token || !data?.refresh_token) { | ||
| return null | ||
| } | ||
|
|
||
| const userRes = await fetch('https://api.spotify.com/v1/me', { | ||
| headers: { | ||
| 'Authorization': `Bearer ${data.access_token}`, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Add bounded timeouts to the Spotify HTTP calls.
Both outbound requests run inline on the callback path. If Spotify stalls or a socket half-opens, this handler can sit open until the client gives up. Wrap both calls in a shared HTTP helper or attach an abort/timeout so the route fails fast instead of hanging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/src/services/SpotifyAuthService.ts` around lines 20 - 53,
The SpotifyAuthService outbound fetches to
'https://accounts.spotify.com/api/token' and 'https://api.spotify.com/v1/me'
must use a bounded timeout so the callback doesn't hang; update the code in
SpotifyAuthService to wrap both fetch calls with a shared timeout helper (e.g.,
fetchWithTimeout) or use AbortController: create an AbortController per request,
set a setTimeout to call controller.abort() after a configurable short timeout,
pass controller.signal to fetch, and clear the timer after the response; apply
this to the token exchange fetch and the user profile fetch so both fail fast on
network stalls.
Comment on lines
+81
to
+97
| {!status?.configured ? ( | ||
| <section className='surface-panel space-y-3 p-6'> | ||
| <h2 className='type-h2 text-lucky-text-primary'>Not Configured</h2> | ||
| <p className='type-body-sm text-lucky-text-secondary'> | ||
| Spotify integration is not configured on this bot. The server owner needs | ||
| to set | ||
| <code className='mx-1 rounded bg-lucky-bg-tertiary px-1.5 py-0.5 text-xs'> | ||
| SPOTIFY_CLIENT_ID | ||
| </code> | ||
| and | ||
| <code className='mx-1 rounded bg-lucky-bg-tertiary px-1.5 py-0.5 text-xs'> | ||
| SPOTIFY_CLIENT_SECRET | ||
| </code> | ||
| . | ||
| </p> | ||
| </section> | ||
| ) : status.linked ? ( |
There was a problem hiding this comment.
Don’t treat “status unavailable” as “not configured.”
If loadStatus() fails, status stays null, so this branch shows the admin-facing “Not Configured” panel for transient API/network errors. Gate this panel on status?.configured === false and keep a separate error/empty state for status === null.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/frontend/src/pages/Spotify.tsx` around lines 81 - 97, The current
conditional treats status === null (e.g., when loadStatus() fails) the same as
configured === false, causing the admin "Not Configured" panel to show on
transient errors; change the conditional in Spotify.tsx to explicitly check
status?.configured === false for that panel and add a separate branch or
fallback to handle status === null (an error/loading/empty state) so the "Not
Configured" UI only appears when status exists and configured is false; update
any relevant rendering around the status variable and loadStatus() usage to
avoid collapsing null into the not-configured case.
Comment on lines
+107
to
+115
| <a | ||
| href={`https://open.spotify.com/user/${status.username}`} | ||
| target='_blank' | ||
| rel='noopener noreferrer' | ||
| className='ml-1 inline-flex items-center gap-1 text-lucky-accent hover:text-lucky-accent-soft' | ||
| > | ||
| {status.username} | ||
| <ExternalLink className='h-3.5 w-3.5' /> | ||
| </a> |
There was a problem hiding this comment.
Don’t build the profile link from spotifyUsername.
status.username comes from spotifyUsername, which is populated from Spotify’s display name when available. That is presentation text, not a stable account identifier, so many users will get a broken or incorrect profile link here. Return a canonical profile URL or Spotify user id from the API instead, or render the username as plain text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/frontend/src/pages/Spotify.tsx` around lines 107 - 115, The code
builds a Spotify profile link using display text (status.username /
spotifyUsername), which is not a stable identifier and produces broken links;
update the rendering in the Spotify page so you do not construct a URL from
status.username: either use a canonical profile URL or user id returned by the
API (e.g., use a field like status.profileUrl or status.id to build the href) or
remove the anchor and render {status.username} as plain text (keep ExternalLink
only when a valid canonical URL field exists). Locate the anchor around
{status.username} and change the href/source accordingly and ensure rel/target
remain correct when you do supply a real URL.
Comment on lines
+64
to
+75
| const auth = Buffer.from(`${clientId}:${clientSecret}`).toString('base64') | ||
| const res = await fetch('https://accounts.spotify.com/api/token', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Authorization': `Basic ${auth}`, | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| body: new URLSearchParams({ | ||
| grant_type: 'refresh_token', | ||
| refresh_token: refreshToken, | ||
| }).toString(), | ||
| }) |
There was a problem hiding this comment.
Token refresh should also fail fast on network hangs.
This refresh path makes an unbounded outbound call to Spotify. A slow or wedged upstream connection will keep callers waiting far longer than necessary. Add the same timeout/abort handling here as in the authorization-code exchange path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/services/SpotifyLinkService/index.ts` around lines 64 -
75, The refresh-token fetch call currently does an unbounded network request;
wrap this POST to 'https://accounts.spotify.com/api/token' in the same
timeout/abort logic used in the authorization-code exchange path: create an
AbortController, start a timeout (matching the existing fetch timeout constant
or value used elsewhere), pass controller.signal into the fetch options, and
clear the timer after the response arrives; ensure the controller.abort() is
invoked on timeout so the refresh flow fails fast on network hangs. Reference
the fetch call in this token refresh block and use AbortController and the
existing timeout value/name from the authorization-code exchange implementation.
Comment on lines
+397
to
+399
| accessToken String | ||
| refreshToken String | ||
| tokenExpiresAt DateTime |
There was a problem hiding this comment.
Encrypt stored Spotify OAuth tokens (Line 397 to Line 399).
accessToken and refreshToken are currently stored in plaintext columns. These are sensitive credentials and should be encrypted at rest (with managed key rotation) before persistence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prisma/schema.prisma` around lines 397 - 399, The accessToken and
refreshToken fields are stored plaintext; update the Prisma model and
persistence logic to store encrypted ciphertext instead: add new fields (e.g.,
accessTokenEncrypted and refreshTokenEncrypted as Bytes or String) preserving
tokenExpiresAt, run a migration, and stop writing plaintext to
accessToken/refreshToken (or deprecate/rename them). Implement application-level
envelope encryption helpers (e.g., encryptToken and decryptToken) that call your
KMS (with managed key rotation) to produce ciphertext+metadata/nonce and use
those helpers inside the repository/data-access methods that create/update/read
the model (locate code paths that write/read tokens). Add a migration script to
re-encrypt existing tokens (using current key) or rotate them securely, update
tests and any code referencing accessToken/refreshToken to use the decrypt
helper, and ensure secrets are never logged.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/bot/src/functions/music/commands/play/spotifyHandler.spec.ts (1)
39-95: Add a regression test for playlist URLs throughhandleSpotifyTrack.Given URL routing can call
handleSpotifyTrack(...)forspotify.comlinks, add a case wheresearchResult.playlist !== nulland multiple tracks are returned, then assert playlist behavior is preserved (not truncated to one track).Suggested test case
+ it('does not truncate playlist URL results when routed through track handler', async () => { + const tracks = [ + { title: 'Track 1', url: 'https://spotify.com/track/1' }, + { title: 'Track 2', url: 'https://spotify.com/track/2' }, + ] + const player = createPlayer(() => + createSearchResult(true, tracks, { name: 'Playlist' }), + ) + + const result = await handleSpotifyTrack( + 'https://spotify.com/playlist/456', + createUser(), + 'guild-1', + 'channel-1', + player, + ) + + expect(result.success).toBe(true) + expect(result.isPlaylist).toBe(true) + expect(result.tracks).toEqual(tracks) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/functions/music/commands/play/spotifyHandler.spec.ts` around lines 39 - 95, Add a regression test to the handleSpotifyTrack suite that covers playlist Spotify URLs: create a player via createPlayer(() => createSearchResult(true, tracks, playlistMeta)) where searchResult.playlist is non-null and tracks contains multiple track objects, call handleSpotifyTrack with a spotify playlist URL and assert result.success is true, result.isPlaylist is true, result.tracks equals the full tracks array (not truncated to one), and relevant logs (e.g., debugLogMock) are called; place this alongside the existing track/empty/error tests so playlist handling for handleSpotifyTrack is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bot/src/functions/music/commands/play/spotifyHandler.ts`:
- Around line 67-79: The handler currently decides the returned tracks using
isPlaylistQuery rather than the computed isPlaylist, causing playlist URLs
routed to handleSpotifyTrack to be truncated; update the return to use the
computed isPlaylist (and guard for empty tracks) — i.e., in handleSpotifyTrack
replace the conditional tracks return (isPlaylistQuery ? tracks : [tracks[0]])
with (isPlaylist ? tracks : [tracks[0]]) and ensure isPlaylist is computed from
isPlaylistQuery, searchResult.playlist and tracks.length as shown, so playlists
detected by handleUrlQuery/handleSpotifyTrack are preserved.
---
Nitpick comments:
In `@packages/bot/src/functions/music/commands/play/spotifyHandler.spec.ts`:
- Around line 39-95: Add a regression test to the handleSpotifyTrack suite that
covers playlist Spotify URLs: create a player via createPlayer(() =>
createSearchResult(true, tracks, playlistMeta)) where searchResult.playlist is
non-null and tracks contains multiple track objects, call handleSpotifyTrack
with a spotify playlist URL and assert result.success is true, result.isPlaylist
is true, result.tracks equals the full tracks array (not truncated to one), and
relevant logs (e.g., debugLogMock) are called; place this alongside the existing
track/empty/error tests so playlist handling for handleSpotifyTrack is
validated.
🪄 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: 31419f12-9c29-4857-a063-7dadb9274b83
📒 Files selected for processing (5)
packages/bot/src/functions/music/commands/play/spotifyHandler.spec.tspackages/bot/src/functions/music/commands/play/spotifyHandler.tspackages/bot/src/functions/music/commands/spotify.tspackages/bot/src/spotify/spotifyConfig.spec.tssonar-project.properties
✅ Files skipped from review due to trivial changes (2)
- sonar-project.properties
- packages/bot/src/spotify/spotifyConfig.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bot/src/functions/music/commands/spotify.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
- GitHub Check: compressed-size
🔇 Additional comments (2)
packages/bot/src/functions/music/commands/play/spotifyHandler.ts (1)
95-113: Nice refactor: wrapper functions are now clean delegators.Keeping
handleSpotifyTrack(...)andhandleSpotifyPlaylist(...)as thin entrypoints improves maintainability and removes duplicate error/logging code.packages/bot/src/functions/music/commands/play/spotifyHandler.spec.ts (1)
97-172: Good coverage depth on playlist edge handling.The suite covers playlist success/error paths and the
playlist: null+ multi-track case, which is a valuable guard for provider inconsistencies.
Comment on lines
+67
to
+79
| const isPlaylist = isPlaylistQuery | ||
| ? searchResult.playlist !== null || tracks.length > 1 | ||
| : false | ||
|
|
||
| debugLog({ | ||
| message: `Found track: ${firstTrack.title}`, | ||
| data: { guildId }, | ||
| message: trackInfo, | ||
| data: { guildId, isPlaylist }, | ||
| }) | ||
|
|
||
| return { | ||
| success: true, | ||
| tracks: [firstTrack], | ||
| isPlaylist: false, | ||
| tracks: isPlaylistQuery ? tracks : [tracks[0]], | ||
| isPlaylist, |
There was a problem hiding this comment.
Playlist URLs can be truncated in the track handler path.
handleUrlQuery(...) routes all spotify.com URLs to handleSpotifyTrack(...) (see packages/bot/src/functions/music/commands/play/processor.ts, Line 102 in that file path block). With Line 78 and Line 79 here, non-playlist mode always returns only tracks[0] and isPlaylist: false, so playlist URLs can be reduced to one track.
Suggested fix
- const isPlaylist = isPlaylistQuery
- ? searchResult.playlist !== null || tracks.length > 1
- : false
+ const detectedPlaylist =
+ searchResult.playlist !== null ||
+ /spotify\.com\/playlist\//.test(query)
+ const isPlaylist = isPlaylistQuery
+ ? detectedPlaylist || tracks.length > 1
+ : detectedPlaylist
...
- tracks: isPlaylistQuery ? tracks : [tracks[0]],
+ tracks: isPlaylist ? tracks : [tracks[0]],
isPlaylist,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isPlaylist = isPlaylistQuery | |
| ? searchResult.playlist !== null || tracks.length > 1 | |
| : false | |
| debugLog({ | |
| message: `Found track: ${firstTrack.title}`, | |
| data: { guildId }, | |
| message: trackInfo, | |
| data: { guildId, isPlaylist }, | |
| }) | |
| return { | |
| success: true, | |
| tracks: [firstTrack], | |
| isPlaylist: false, | |
| tracks: isPlaylistQuery ? tracks : [tracks[0]], | |
| isPlaylist, | |
| const detectedPlaylist = | |
| searchResult.playlist !== null || | |
| /spotify\.com\/playlist\//.test(query) | |
| const isPlaylist = isPlaylistQuery | |
| ? detectedPlaylist || tracks.length > 1 | |
| : detectedPlaylist | |
| debugLog({ | |
| message: trackInfo, | |
| data: { guildId, isPlaylist }, | |
| }) | |
| return { | |
| success: true, | |
| tracks: isPlaylist ? tracks : [tracks[0]], | |
| isPlaylist, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/src/functions/music/commands/play/spotifyHandler.ts` around
lines 67 - 79, The handler currently decides the returned tracks using
isPlaylistQuery rather than the computed isPlaylist, causing playlist URLs
routed to handleSpotifyTrack to be truncated; update the return to use the
computed isPlaylist (and guard for empty tracks) — i.e., in handleSpotifyTrack
replace the conditional tracks return (isPlaylistQuery ? tracks : [tracks[0]])
with (isPlaylist ? tracks : [tracks[0]]) and ensure isPlaylist is computed from
isPlaylistQuery, searchResult.playlist and tracks.length as shown, so playlists
detected by handleUrlQuery/handleSpotifyTrack are preserved.
…fy routes in index test
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/backend/src/routes/spotify.ts (2)
182-185:⚠️ Potential issue | 🔴 CriticalPass
stateseparately and keepredirect_uricanonical.Spotify expects the code exchange to reuse the exact same
redirect_uri. Appending?state=...here changes that value and can make the token request fail.🔧 Suggested fix
- const backendBaseUrl = resolveBackendBaseUrl(req) - const callbackUrl = `${backendBaseUrl}/api/spotify/callback?state=${encodeURIComponent(state)}` + const callbackUrl = getOAuthRedirectUri(req) const scopes = ['user-top-read', 'user-read-recently-played', 'user-library-read'] - const authUrl = `https://accounts.spotify.com/authorize?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodeURIComponent(callbackUrl)}&scope=${encodeURIComponent(scopes.join(' '))}` + const authUrl = `https://accounts.spotify.com/authorize?client_id=${encodeURIComponent(clientId)}&response_type=code&redirect_uri=${encodeURIComponent(callbackUrl)}&scope=${encodeURIComponent(scopes.join(' '))}&state=${encodeURIComponent(state)}`Does Spotify's Authorization Code flow require the `redirect_uri` in the token request to exactly match the one used in the authorize request, and should OAuth `state` be sent as a separate parameter rather than embedded into the redirect URI?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/routes/spotify.ts` around lines 182 - 185, The authorize URL currently embeds the state into the redirect_uri (callbackUrl) which alters the canonical redirect_uri; change it so callbackUrl remains the plain backend redirect (built via resolveBackendBaseUrl and stored in callbackUrl) and add state as its own query parameter on the authUrl (i.e. keep redirect_uri exactly as callbackUrl when encoding, and append &state=${encodeURIComponent(state)} to authUrl instead of tacking ?state onto callbackUrl); also ensure the token exchange logic that calls the token endpoint uses the identical callbackUrl value for redirect_uri so the values match exactly.
209-214:⚠️ Potential issue | 🟠 MajorRequire the callback
stateto match the cookie.Falling back to whichever source exists keeps old signed link URLs replayable. At minimum, require both values and reject mismatches before decoding.
🔒 Suggested fix
- const state = - typeof parsedQuery.data.state === 'string' - ? parsedQuery.data.state - : typeof stateFromCookie === 'string' - ? stateFromCookie - : null - - if (!state) { + const queryState = + typeof parsedQuery.data.state === 'string' + ? parsedQuery.data.state + : null + const cookieState = + typeof stateFromCookie === 'string' ? stateFromCookie : null + + if (!queryState || !cookieState) { return res.redirect( `${frontendUrl}/?error=spotify_missing_state`, ) } + if (queryState !== cookieState) { + return res.redirect( + `${frontendUrl}/?error=spotify_invalid_state`, + ) + } + const state = queryState🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/routes/spotify.ts` around lines 209 - 214, The current fallback logic allows a single source of `state` (parsedQuery.data.state or stateFromCookie) which enables replay; instead, require both `parsedQuery.data.state` and `stateFromCookie` to be present and equal before proceeding: extract them into distinct variables (e.g., stateFromQuery = parsedQuery.data.state, stateFromCookie), check both are strings, if either is missing or they don't match return/reject (HTTP 400 or similar) immediately, and only then assign/use `state` for decoding and further processing; update the handler where `state` is computed to enforce this early validation.
🧹 Nitpick comments (1)
packages/backend/tests/unit/services/SpotifyAuthService.test.ts (1)
91-107: AssertexpiresInin the success case.
packages/backend/src/routes/spotify.tsderivestokenExpiresAtfrom this field, so the service can regress there without breaking this unit.🧪 Minimal assertion to add
expect(result?.accessToken).toBe('at') expect(result?.refreshToken).toBe('rt') + expect(result?.expiresIn).toBe(3600) expect(result?.spotifyId).toBe('spotify-user-1') expect(result?.spotifyUsername).toBe('Test User')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/tests/unit/services/SpotifyAuthService.test.ts` around lines 91 - 107, The test for exchangeCodeForToken is missing an assertion for the expires_in field returned from the token endpoint; update the 'returns token data on success' test in SpotifyAuthService.test.ts to assert that the returned result includes the correct expiresIn (e.g., expect(result?.expiresIn).toBe(3600)) so changes to tokenExpiresAt derivation will be caught; locate the test block that calls exchangeCodeForToken and add the expiresIn assertion alongside the existing accessToken/refreshToken/spotifyId/spotifyUsername assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/tests/unit/routes/index.test.ts`:
- Around line 96-98: The test currently mocks setupSpotifyRoutes but never
asserts it was called; update the unit test in index.test.ts to verify that
setupSpotifyRoutes was invoked when setupRoutes() runs (or when the route
registrar is executed) by adding an expectation that the mocked
setupSpotifyRoutes was called (e.g.,
toHaveBeenCalled()/toHaveBeenCalledWith(...) as appropriate), ensuring the test
fails if Spotify routes stop being wired by setupRoutes.
---
Duplicate comments:
In `@packages/backend/src/routes/spotify.ts`:
- Around line 182-185: The authorize URL currently embeds the state into the
redirect_uri (callbackUrl) which alters the canonical redirect_uri; change it so
callbackUrl remains the plain backend redirect (built via resolveBackendBaseUrl
and stored in callbackUrl) and add state as its own query parameter on the
authUrl (i.e. keep redirect_uri exactly as callbackUrl when encoding, and append
&state=${encodeURIComponent(state)} to authUrl instead of tacking ?state onto
callbackUrl); also ensure the token exchange logic that calls the token endpoint
uses the identical callbackUrl value for redirect_uri so the values match
exactly.
- Around line 209-214: The current fallback logic allows a single source of
`state` (parsedQuery.data.state or stateFromCookie) which enables replay;
instead, require both `parsedQuery.data.state` and `stateFromCookie` to be
present and equal before proceeding: extract them into distinct variables (e.g.,
stateFromQuery = parsedQuery.data.state, stateFromCookie), check both are
strings, if either is missing or they don't match return/reject (HTTP 400 or
similar) immediately, and only then assign/use `state` for decoding and further
processing; update the handler where `state` is computed to enforce this early
validation.
---
Nitpick comments:
In `@packages/backend/tests/unit/services/SpotifyAuthService.test.ts`:
- Around line 91-107: The test for exchangeCodeForToken is missing an assertion
for the expires_in field returned from the token endpoint; update the 'returns
token data on success' test in SpotifyAuthService.test.ts to assert that the
returned result includes the correct expiresIn (e.g.,
expect(result?.expiresIn).toBe(3600)) so changes to tokenExpiresAt derivation
will be caught; locate the test block that calls exchangeCodeForToken and add
the expiresIn assertion alongside the existing
accessToken/refreshToken/spotifyId/spotifyUsername assertions.
🪄 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: fbed3bfe-607c-4680-834e-54c9a3e88f14
📒 Files selected for processing (5)
packages/backend/src/routes/spotify.tspackages/backend/tests/integration/routes/spotify.test.tspackages/backend/tests/unit/routes/index.test.tspackages/backend/tests/unit/services/SpotifyAuthService.test.tssonar-project.properties
✅ Files skipped from review due to trivial changes (2)
- sonar-project.properties
- packages/backend/tests/integration/routes/spotify.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
- GitHub Check: compressed-size
Comment on lines
+96
to
+98
| jest.mock('../../../src/routes/spotify', () => ({ | ||
| setupSpotifyRoutes, | ||
| })) |
There was a problem hiding this comment.
Add an assertion for the new route registrar.
Right now this mock is never checked, so the test still passes if setupRoutes() stops wiring Spotify routes.
✅ Minimal test addition
expect(setupStarboardRoutes).toHaveBeenCalledWith(app)
expect(setupMusicRoutes).toHaveBeenCalledWith(app)
+ expect(setupSpotifyRoutes).toHaveBeenCalledWith(app)
expect(app.use).toHaveBeenCalledWith(errorHandler)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/backend/tests/unit/routes/index.test.ts` around lines 96 - 98, The
test currently mocks setupSpotifyRoutes but never asserts it was called; update
the unit test in index.test.ts to verify that setupSpotifyRoutes was invoked
when setupRoutes() runs (or when the route registrar is executed) by adding an
expectation that the mocked setupSpotifyRoutes was called (e.g.,
toHaveBeenCalled()/toHaveBeenCalledWith(...) as appropriate), ensuring the test
fails if Spotify routes stop being wired by setupRoutes.
This was referenced Apr 14, 2026
4 tasks done
LucasSantana-Dev
added a commit
that referenced
this pull request
May 13, 2026
* schema: add SpotifyLink model with migration * service: add SpotifyLinkService with token refresh logic * backend: add Spotify OAuth routes and auth service * bot: add /spotify command with link/unlink/status subcommands * frontend: add Spotify OAuth page and API integration * test: add Spotify route integration tests * test: fix SpotifyLinkService and spotify command test mocks * test: fix Spotify route test mock initialization * fix: reduce sonarcloud issues on spotify oauth - Extract spotify command subcommands into separate handler functions to reduce execute() complexity from 16 to ~10 - Refactor spotifyHandler to eliminate duplication between track and playlist handlers via shared handleSpotifyUrl() function - Add comprehensive test coverage for spotifyHandler (7 tests) - Add comprehensive test coverage for spotifyConfig (6 tests) - Exclude spotify files from CPD to allow intentional OAuth patterns * test(backend): add SpotifyAuthService tests and fix cpd exclusions * test(backend): add spotify route coverage for error paths * fix(backend): handle HMAC length mismatch in state verify, mock spotify routes in index test
This branch was successfully deployed
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
SpotifyLinkPrisma model (discordId, spotifyId, access/refresh tokens, expiry)SpotifyLinkService—getByDiscordId,getValidAccessToken(auto-refresh),set,unlinkGET /api/spotify/connect,GET /api/spotify/callback,DELETE /api/spotify/unlink,GET /api/spotify/status/spotify link/unlink/statusbot commanduser-top-read user-read-recently-played user-library-readEnv vars required
SPOTIFY_CLIENT_IDSPOTIFY_CLIENT_SECRETSPOTIFY_REDIRECT_URI(e.g.https://lucky.lucassantana.tech/api/spotify/callback)Test plan
/spotify linkDMs OAuth URL/spotify statusshows linked username/spotify unlinkremoves linkSummary by CodeRabbit
New Features
Backend
Tests