feat: major refactors and adding spotify save functionality - #9
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds Spotify integration functionality to the Amplify Discord bot, allowing users to save songs and playlists from the current queue to their Spotify accounts. The changes also include refactoring of the environment variable handling and improvements to the audio playback system.
Key changes:
- Added Spotify OAuth authentication and save functionality for tracks and playlists
- Refactored environment variable encryption/decryption into a separate module
- Enhanced audio playback system with lazy resource loading and automatic queue progression
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Updated env handling to use new module and added auth server startup |
| src/helpers/spotify.ts | New module implementing Spotify OAuth and save functionality |
| src/helpers/server.ts | New Express server for handling Spotify OAuth callbacks |
| src/helpers/playback.ts | Enhanced with lazy loading, auto-progression, and queue snapshot functions |
| src/helpers/envs.ts | New module extracting env encryption/decryption logic |
| src/helpers/components.ts | Added "Save to Spotify" button to playback controls |
| src/helpers/buttons.ts | Added handler for Spotify save button interactions |
| package.json | Added Express and its type definitions as dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return saveMultiTrack(token, songs); | ||
| } catch { | ||
| return { | ||
| msg: "An error occured while attempting to save to spotify", |
There was a problem hiding this comment.
There is a spelling error: 'occured' should be 'occurred'.
| msg: "An error occured while attempting to save to spotify", | |
| msg: "An error occurred while attempting to save to spotify", |
| } | ||
| }); | ||
| app.listen(port, () => { | ||
| logger.info(`Spotify callback server listening on http:/127.0.0.1:${port}`); |
There was a problem hiding this comment.
Missing forward slash in URL: 'http:/127.0.0.1' should be 'http://127.0.0.1'.
| logger.info(`Spotify callback server listening on http:/127.0.0.1:${port}`); | |
| logger.info(`Spotify callback server listening on http://127.0.0.1:${port}`); |
|
|
||
| const SCOPES = ["user-library-modify", "playlist-modify-private"].join(" "); | ||
|
|
||
| const userTokens = new Map<string, TokenInfo>(); |
There was a problem hiding this comment.
User tokens are stored in memory only. This means tokens will be lost on server restart, requiring users to re-authenticate. Consider persisting tokens to a database or file system for better user experience.
| grant_type: "refresh_token", | ||
| refresh_token: token.refreshToken, | ||
| client_id: process.env.SPOTIFY_CLIENT_ID!, | ||
| client_secret: process.env.SPOTIFY_CLIENT_SECRET!, |
There was a problem hiding this comment.
Missing input validation for required environment variables. The function should validate that SPOTIFY_CLIENT_ID and SPOTIFY_CLIENT_SECRET are set before making the API call.
| client_secret: process.env.SPOTIFY_CLIENT_SECRET!, | |
| if (!process.env.SPOTIFY_CLIENT_ID || !process.env.SPOTIFY_CLIENT_SECRET) { | |
| // Required environment variables are missing | |
| return null; | |
| } | |
| const res = await fetch("https://accounts.spotify.com/api/token", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/x-www-form-urlencoded" }, | |
| body: new URLSearchParams({ | |
| grant_type: "refresh_token", | |
| refresh_token: token.refreshToken, | |
| client_id: process.env.SPOTIFY_CLIENT_ID, | |
| client_secret: process.env.SPOTIFY_CLIENT_SECRET, |
| if (songs.length <= 1) return saveSingleTrack(interaction.guildId!, token); | ||
|
|
||
| return saveMultiTrack(token, songs); | ||
| } catch { |
There was a problem hiding this comment.
Empty catch block discards error information. Consider logging the error to help with debugging when Spotify save operations fail.
| } catch { | |
| } catch (err) { | |
| console.error("Error in saveToUsersSpotify:", err); |
| // player.on(AudioPlayerStatus.Idle, () => { | ||
| // endSession(i.guildId!); | ||
| // }); | ||
| nextPlayback(i.guildId!); |
There was a problem hiding this comment.
Calling async function nextPlayback without await in an event handler could lead to unhandled promise rejections if the function throws an error.
| nextPlayback(i.guildId!); | |
| nextPlayback(i.guildId!).catch((err) => { | |
| logger.error(`Error in nextPlayback: ${err instanceof Error ? err.message : String(err)}`); | |
| }); |
3835591 to
717f6ab
Compare
244f7f4 to
db3ae84
Compare
No description provided.