Feature : Discord Group edit functionality for super users #2437
Conversation
…ionality-for-super-users Add Discord group editing API
Summary by CodeRabbit
WalkthroughA new feature was implemented to allow superusers to update Discord group role names and descriptions via a PATCH API endpoint. This includes request validation middleware, a controller for handling the update logic, integration with the Discord API to update roles, and the corresponding route definition. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant AuthMiddleware
participant Validator
participant Controller
participant DiscordService
participant Database
Client->>Router: PATCH /groups/:groupId
Router->>AuthMiddleware: Authenticate & Authorize
AuthMiddleware->>Validator: validateGroupRoleUpdateBody
Validator->>Controller: updateGroupRoleDetails
Controller->>Database: Find group role by groupId
alt Role name update requested
Controller->>DiscordService: updateGroupRoleInDiscord
DiscordService-->>Controller: Success/Failure
alt Failure
Controller->>Client: Error response
end
end
Controller->>Database: Update role in DB
Controller->>Client: Success response
Possibly related issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
controllers/discordactions.js(3 hunks)middlewares/validators/discordactions.js(2 hunks)routes/discordactions.js(2 hunks)services/discordService.js(2 hunks)
🧰 Additional context used
🪛 ESLint
controllers/discordactions.js
[error] 141-142: Replace ⏎········await·discordRolesModel.isGroupRoleExists({·rolename with ·await·discordRolesModel.isGroupRoleExists({⏎········rolename,⏎·····
(prettier/prettier)
[error] 163-167: Replace ⏎········roleData.roleid,⏎········rolename,⏎········req.userData⏎······ with roleData.roleid,·rolename,·req.userData
(prettier/prettier)
🔇 Additional comments (3)
services/discordService.js (1)
143-161: LGTM! Clean implementation of Discord role update service.The function follows the established patterns in the codebase with proper error handling and consistent response format. Note that this only updates the role name in Discord, which appears to be a limitation of the Discord API endpoint.
middlewares/validators/discordactions.js (1)
19-32: Excellent validation implementation!The Joi schema correctly ensures at least one field is provided for updates while making both fields optional. The use of
.or("rolename", "description")prevents empty update requests.routes/discordactions.js (1)
42-49: Proper security implementation for sensitive operation.The middleware chain correctly enforces authentication, Discord verification, superuser authorization, and request validation. This provides appropriate protection for group role modification functionality.
| const { roleExists: nameExists, existingRoles: nameExistingRoles } = | ||
| await discordRolesModel.isGroupRoleExists({ rolename }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix formatting issues identified by ESLint.
The static analysis tools have identified formatting inconsistencies that should be addressed.
Apply this formatting fix:
- const { roleExists: nameExists, existingRoles: nameExistingRoles } =
- await discordRolesModel.isGroupRoleExists({ rolename });
+ const { roleExists: nameExists, existingRoles: nameExistingRoles } =
+ await discordRolesModel.isGroupRoleExists({
+ rolename,
+ });- const discordUpdate = await updateGroupRoleInDiscord(
- roleData.roleid,
- rolename,
- req.userData
- );
+ const discordUpdate = await updateGroupRoleInDiscord(roleData.roleid, rolename, req.userData);Also applies to: 163-167
🧰 Tools
🪛 ESLint
[error] 141-142: Replace ⏎········await·discordRolesModel.isGroupRoleExists({·rolename with ·await·discordRolesModel.isGroupRoleExists({⏎········rolename,⏎·····
(prettier/prettier)
🤖 Prompt for AI Agents
In controllers/discordactions.js at lines 141-142 and also lines 163-167, there
are formatting inconsistencies flagged by ESLint. Adjust the indentation,
spacing, and line breaks to conform to the project's ESLint rules, ensuring
consistent and clean code style. Review both code blocks and reformat them
accordingly to fix these issues.
| const updateGroupRoleDetails = async (req, res) => { | ||
| const { groupId } = req.params; | ||
| let { rolename, description } = req.body; | ||
| try { | ||
| const { roleExists, existingRoles } = await discordRolesModel.isGroupRoleExists({ groupId }); | ||
| if (!roleExists) { | ||
| return res.boom.notFound("Group role not found"); | ||
| } | ||
|
|
||
| const updateData = {}; | ||
| if (rolename) { | ||
| if (!rolename.startsWith("group-")) { | ||
| rolename = `group-${rolename}`; | ||
| } | ||
|
|
||
| const { roleExists: nameExists, existingRoles: nameExistingRoles } = | ||
| await discordRolesModel.isGroupRoleExists({ rolename }); | ||
| if (nameExists) { | ||
| const existingId = nameExistingRoles.docs[0].id; | ||
| if (existingId !== groupId) { | ||
| return res.status(409).json({ message: "Role already exists!" }); | ||
| } | ||
| } | ||
|
|
||
| updateData.rolename = rolename; | ||
| } | ||
| if (description !== undefined) { | ||
| updateData.description = description; | ||
| } | ||
|
|
||
| if (Object.keys(updateData).length === 0) { | ||
| return res.status(400).json({ message: "Nothing to update" }); | ||
| } | ||
|
|
||
| const roleData = existingRoles.data(); | ||
|
|
||
| if (updateData.rolename) { | ||
| const discordUpdate = await updateGroupRoleInDiscord( | ||
| roleData.roleid, | ||
| rolename, | ||
| req.userData | ||
| ); | ||
| if (!discordUpdate.success) { | ||
| return res.boom.badImplementation(discordUpdate.message); | ||
| } | ||
| } | ||
|
|
||
| await discordRolesModel.updateGroupRole(updateData, groupId); | ||
|
|
||
| return res.status(200).json({ message: "Group role updated successfully" }); | ||
| } catch (error) { | ||
| logger.error(`Error while updating group role: ${error}`); | ||
| return res.boom.badImplementation("Internal server error"); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding audit logging for consistency.
The implementation logic is solid with proper validation, conflict checking, and Discord-database synchronization. However, consider adding audit logging similar to the deleteGroupRole function for tracking group role modifications.
Add logging after successful update:
+ const groupUpdateLog = {
+ type: "group-role-update",
+ meta: {
+ userId: req.userData.id,
+ },
+ body: {
+ groupId: groupId,
+ updateData,
+ action: "update",
+ },
+ };
+ await addLog(groupUpdateLog.type, groupUpdateLog.meta, groupUpdateLog.body);
+
return res.status(200).json({ message: "Group role updated successfully" });📝 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.
| const updateGroupRoleDetails = async (req, res) => { | |
| const { groupId } = req.params; | |
| let { rolename, description } = req.body; | |
| try { | |
| const { roleExists, existingRoles } = await discordRolesModel.isGroupRoleExists({ groupId }); | |
| if (!roleExists) { | |
| return res.boom.notFound("Group role not found"); | |
| } | |
| const updateData = {}; | |
| if (rolename) { | |
| if (!rolename.startsWith("group-")) { | |
| rolename = `group-${rolename}`; | |
| } | |
| const { roleExists: nameExists, existingRoles: nameExistingRoles } = | |
| await discordRolesModel.isGroupRoleExists({ rolename }); | |
| if (nameExists) { | |
| const existingId = nameExistingRoles.docs[0].id; | |
| if (existingId !== groupId) { | |
| return res.status(409).json({ message: "Role already exists!" }); | |
| } | |
| } | |
| updateData.rolename = rolename; | |
| } | |
| if (description !== undefined) { | |
| updateData.description = description; | |
| } | |
| if (Object.keys(updateData).length === 0) { | |
| return res.status(400).json({ message: "Nothing to update" }); | |
| } | |
| const roleData = existingRoles.data(); | |
| if (updateData.rolename) { | |
| const discordUpdate = await updateGroupRoleInDiscord( | |
| roleData.roleid, | |
| rolename, | |
| req.userData | |
| ); | |
| if (!discordUpdate.success) { | |
| return res.boom.badImplementation(discordUpdate.message); | |
| } | |
| } | |
| await discordRolesModel.updateGroupRole(updateData, groupId); | |
| return res.status(200).json({ message: "Group role updated successfully" }); | |
| } catch (error) { | |
| logger.error(`Error while updating group role: ${error}`); | |
| return res.boom.badImplementation("Internal server error"); | |
| } | |
| }; | |
| const updateGroupRoleDetails = async (req, res) => { | |
| const { groupId } = req.params; | |
| let { rolename, description } = req.body; | |
| try { | |
| const { roleExists, existingRoles } = await discordRolesModel.isGroupRoleExists({ groupId }); | |
| if (!roleExists) { | |
| return res.boom.notFound("Group role not found"); | |
| } | |
| const updateData = {}; | |
| if (rolename) { | |
| if (!rolename.startsWith("group-")) { | |
| rolename = `group-${rolename}`; | |
| } | |
| const { roleExists: nameExists, existingRoles: nameExistingRoles } = | |
| await discordRolesModel.isGroupRoleExists({ rolename }); | |
| if (nameExists) { | |
| const existingId = nameExistingRoles.docs[0].id; | |
| if (existingId !== groupId) { | |
| return res.status(409).json({ message: "Role already exists!" }); | |
| } | |
| } | |
| updateData.rolename = rolename; | |
| } | |
| if (description !== undefined) { | |
| updateData.description = description; | |
| } | |
| if (Object.keys(updateData).length === 0) { | |
| return res.status(400).json({ message: "Nothing to update" }); | |
| } | |
| const roleData = existingRoles.data(); | |
| if (updateData.rolename) { | |
| const discordUpdate = await updateGroupRoleInDiscord( | |
| roleData.roleid, | |
| rolename, | |
| req.userData | |
| ); | |
| if (!discordUpdate.success) { | |
| return res.boom.badImplementation(discordUpdate.message); | |
| } | |
| } | |
| await discordRolesModel.updateGroupRole(updateData, groupId); | |
| // Audit log for the update | |
| const groupUpdateLog = { | |
| type: "group-role-update", | |
| meta: { | |
| userId: req.userData.id, | |
| }, | |
| body: { | |
| groupId: groupId, | |
| updateData, | |
| action: "update", | |
| }, | |
| }; | |
| await addLog(groupUpdateLog.type, groupUpdateLog.meta, groupUpdateLog.body); | |
| return res.status(200).json({ message: "Group role updated successfully" }); | |
| } catch (error) { | |
| logger.error(`Error while updating group role: ${error}`); | |
| return res.boom.badImplementation("Internal server error"); | |
| } | |
| }; |
🧰 Tools
🪛 ESLint
[error] 141-142: Replace ⏎········await·discordRolesModel.isGroupRoleExists({·rolename with ·await·discordRolesModel.isGroupRoleExists({⏎········rolename,⏎·····
(prettier/prettier)
[error] 163-167: Replace ⏎········roleData.roleid,⏎········rolename,⏎········req.userData⏎······ with roleData.roleid,·rolename,·req.userData
(prettier/prettier)
🤖 Prompt for AI Agents
In controllers/discordactions.js around lines 126 to 180, after the successful
update of the group role (right after the call to
discordRolesModel.updateGroupRole), add audit logging to record the modification
event. This should include relevant details such as the user who made the
change, the groupId, and the updated fields to maintain consistency with the
existing audit logs like those in deleteGroupRole. Ensure the log entry clearly
indicates a successful update of the group role.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole); | ||
| router.patch( | ||
| "/groups/:groupId", | ||
| authenticate, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
|
/korbit-review |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Incomplete Success Status Handling ▹ view | |
 |
Unsafe Error Response Parsing ▹ view | |
|
Insufficient error context in logs ▹ view | |
 |
Insufficient Error Context ▹ view | |
|
Undefined Logger Reference ▹ view | |
|
Missing error logging for Discord update failure ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| middlewares/validators/discordactions.js | ✅ |
| routes/discordactions.js | ✅ |
| services/discordService.js | ✅ |
| controllers/discordactions.js | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| await schema.validateAsync(req.body); | ||
| next(); | ||
| } catch (error) { | ||
| logger.error(`Error validating updateGroupRole payload : ${error}`); |
There was a problem hiding this comment.
Undefined Logger Reference 
Tell me more
What is the issue?
The 'logger' object is being used but is not imported or defined in the file.
Why this matters
This will cause a ReferenceError when the code attempts to log errors, potentially crashing the application and preventing proper error tracking.
Suggested change ∙ Feature Preview
Import the logger at the top of the file:
const logger = require('../utils/logger');Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| } catch (error) { | ||
| logger.error(`Error while updating group role: ${error}`); | ||
| return res.boom.badImplementation("Internal server error"); |
There was a problem hiding this comment.
Insufficient error context in logs 
Tell me more
What is the issue?
The catch block in updateGroupRoleDetails function logs the error with minimal context and returns a generic error message.
Why this matters
Without specific error context in logs, debugging production issues becomes more difficult and time-consuming. The generic error message provides no actionable information to API consumers.
Suggested change ∙ Feature Preview
} catch (error) {
logger.error(`Error while updating group role for groupId ${groupId}. Error details: ${error.message}. Stack: ${error.stack}`);
return res.boom.badImplementation(`Failed to update group role: ${error.message}`);Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| if (!discordUpdate.success) { | ||
| return res.boom.badImplementation(discordUpdate.message); | ||
| } |
There was a problem hiding this comment.
Missing error logging for Discord update failure
Tell me more
What is the issue?
The error handling for Discord update failure doesn't log the error, losing valuable debugging information.
Why this matters
When Discord updates fail, there's no error log created, making it impossible to track and debug these failures in production.
Suggested change ∙ Feature Preview
if (!discordUpdate.success) {
logger.error(`Failed to update Discord role ${roleData.roleid} with name ${rolename}. Error: ${discordUpdate.message}`);
return res.boom.badImplementation(discordUpdate.message);
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| const data = await response.json(); | ||
| return { success: false, message: data.message }; | ||
| } catch (err) { | ||
| logger.error("Error updating role on Discord", err); |
There was a problem hiding this comment.
Insufficient Error Context
Tell me more
What is the issue?
Error log missing contextual information about the roleId and roleName
Why this matters
Without the role details in the error log, debugging becomes more difficult as the specific role causing the error cannot be identified.
Suggested change ∙ Feature Preview
logger.error("Error updating role on Discord", { roleId, roleName, error: err });Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| if (response.status === 200) { | ||
| return { success: true }; | ||
| } |
There was a problem hiding this comment.
Incomplete Success Status Handling 
Tell me more
What is the issue?
The function only checks for status 200 but Discord API might return other valid success codes like 201 or 204.
Why this matters
Valid role updates might be incorrectly reported as failures if Discord returns a different success status code.
Suggested change ∙ Feature Preview
Use a range check for success status codes:
if (response.status >= 200 && response.status < 300) {
return { success: true };
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
| const data = await response.json(); | ||
| return { success: false, message: data.message }; |
There was a problem hiding this comment.
Unsafe Error Response Parsing
Tell me more
What is the issue?
The function assumes the error response will always contain a JSON body with a message property.
Why this matters
The function may throw an unexpected error if Discord returns a non-JSON error response or a different error format.
Suggested change ∙ Feature Preview
Add safety checks for error response parsing:
try {
const data = await response.json();
return { success: false, message: data.message || 'Unknown error occurred' };
} catch (parseError) {
return { success: false, message: `Request failed with status ${response.status}` };
}Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
Date:
Developer Name:
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes
/korbit-review