Conversation
There was a problem hiding this comment.
Pull request overview
This PR bumps the version to 2.12.1 and includes several bug fixes and improvements, along with an upgrade of the discord.js library from 14.14.1 to 14.25.1.
Changes:
- Fixed a critical bug in auth.service.ts where the userInAuth array was not being properly cleaned up due to using
filter()instead ofsplice() - Improved the getAllMembers function to properly validate that all guild members are cached before returning the cache
- Changed the HTTP method for updating user roles from POST to PUT (breaking API change)
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Version bumped to 2.12.1 and discord.js upgraded from 14.14.1 to 14.25.1 |
| package-lock.json | Dependency lockfile updated to reflect discord.js upgrade and transitive dependency updates |
| src/services/discord.service.ts | Fixed getAllMembers cache validation logic and added explicit return type; added Collection import |
| src/services/auth.service.ts | Fixed bug where userInAuth array wasn't properly cleaned up by replacing filter() with splice() |
| src/routes/roles/users/me/index.ts | Changed HTTP method from POST to PUT for the updateMyRoles endpoint |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| router.get("/", getJwtRequired, runAsyncWrapper(getMyRoles)); | ||
| router.post("/", getJwtRequired, runAsyncWrapper(updateMyRoles)); | ||
| router.put("/", getJwtRequired, runAsyncWrapper(updateMyRoles)); |
There was a problem hiding this comment.
Changing the HTTP method from POST to PUT is a breaking change for API clients. This will cause any existing clients using POST to fail with a 404 error. Consider:
- Keeping both methods temporarily for backward compatibility
- Documenting this breaking change in release notes
- Coordinating with frontend/client teams before deployment
The change itself is semantically correct since the endpoint updates resources (PUT is more appropriate than POST for updates), but it requires careful migration planning.
| router.put("/", getJwtRequired, runAsyncWrapper(updateMyRoles)); | |
| router.put("/", getJwtRequired, runAsyncWrapper(updateMyRoles)); | |
| router.post("/", getJwtRequired, runAsyncWrapper(updateMyRoles)); |
No description provided.