Feature/group role edit feature for superusers #299
Conversation
Summary by CodeRabbit
WalkthroughA new feature for editing Discord guild roles was implemented. This includes the introduction of a handler to process PATCH requests for updating roles, a utility function to perform the update via the Discord API, and a new response constant for successful updates. Type definitions were expanded to include the update payload structure. The API's main router was updated to support the new PATCH endpoint. Comprehensive unit tests were added for both the handler and the utility function to ensure correct behavior across various scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (editGuildRoleHandler)
participant AuthVerifier
participant DiscordAPI
Client->>API (editGuildRoleHandler): PATCH /roles/:roleId (with headers, body)
API->>API: Validate headers and parameters
API->>AuthVerifier: Verify authorization token
AuthVerifier-->>API: Token valid/invalid
API->>DiscordAPI: PATCH /guilds/:guildId/roles/:roleId (with new name, mentionable, reason)
DiscordAPI-->>API: Success or failure response
API-->>Client: 204 No Content (on success) or error response
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:
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: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/constants/responses.ts(1 hunks)src/controllers/editGuildRolesHandler.ts(1 hunks)src/index.ts(2 hunks)src/typeDefinitions/discordMessage.types.d.ts(1 hunks)src/utils/editGroupRole.ts(1 hunks)tests/unit/handlers/editGuildRoleHandler.test.ts(1 hunks)tests/unit/utils/editGuildRole.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/utils/editGroupRole.ts (2)
src/constants/urls.ts (1)
DISCORD_BASE_URL(5-5)src/constants/responses.ts (2)
ROLE_UPDATED(37-37)INTERNAL_SERVER_ERROR(23-24)
tests/unit/utils/editGuildRole.test.ts (3)
src/constants/urls.ts (1)
DISCORD_BASE_URL(5-5)tests/fixtures/fixture.ts (1)
guildEnv(59-63)src/utils/editGroupRole.ts (1)
editGuildRole(7-37)
tests/unit/handlers/editGuildRoleHandler.test.ts (2)
tests/fixtures/fixture.ts (2)
generateDummyRequestObject(125-141)guildEnv(59-63)src/controllers/editGuildRolesHandler.ts (1)
editGuildRoleHandler(10-45)
src/controllers/editGuildRolesHandler.ts (3)
src/utils/verifyAuthToken.ts (1)
verifyNodejsBackendAuthToken(31-40)src/typeDefinitions/discordMessage.types.d.ts (1)
updateRole(93-97)src/utils/editGroupRole.ts (1)
editGuildRole(7-37)
🔇 Additional comments (14)
src/constants/responses.ts (1)
37-37: Looks good - added ROLE_UPDATED constantThe addition of the
ROLE_UPDATEDconstant follows the same pattern as other response messages in this file and will be used to indicate successful role updates.src/index.ts (2)
27-27: Import statement looks goodThe import for the new handler function follows the same pattern as other imports in this file.
69-69:✅ Verification successful
Route definition is appropriate
The new PATCH endpoint for
/roles/:roleIdcorrectly maps to theeditGuildRoleHandlerfunction. This follows RESTful API conventions where PATCH is used for partial updates.Note that this path parameter should match the expected param name in the handler. Make sure the handler uses
roleIdas the parameter name to extract from the request.
🏁 Script executed:
#!/bin/bash # Check how the roleId is used in the handler rg -t ts "editGuildRoleHandler" -A 10 -B 10Length of output: 14278
Route
/roles/:roleIdparameter matches handler
The handler insrc/controllers/editGuildRolesHandler.tsusesconst roleId = decodeURI(request.params?.roleId ?? "");so the
:roleIdpath parameter aligns correctly. Ready to merge.tests/unit/utils/editGuildRole.test.ts (3)
11-20: LGTM: Mock request parameters are properly set up.The test correctly constructs the expected URL and request parameters, including headers and body, which will be used to verify that the utility makes the correct API call to Discord.
27-43: Test properly verifies that the reason is passed as a header.This test effectively verifies that when a reason is provided to the
editGuildRolefunction, it is correctly passed as theX-Audit-Log-Reasonheader in the PATCH request to Discord's API.
45-53: Test for successful response is comprehensive.This test correctly verifies that when the Discord API responds with a 204 status code (success), the function returns the
ROLE_UPDATEDresponse constant.tests/unit/handlers/editGuildRoleHandler.test.ts (6)
1-14: Test setup is properly structured.The test imports all necessary dependencies, sets up proper Jest hooks to clear and restore mocks between tests, and defines a constant roleId to be used across tests.
15-26: Authorization test is properly implemented.This test correctly verifies that the handler returns a BAD_SIGNATURE response when the authorization header is missing.
27-42: Feature flag test is properly implemented.This test effectively verifies that the handler returns a NOT_IMPLEMENTED response when the dev flag is set to "false", confirming that the feature can be toggled off as intended.
43-58: Request validation test is properly implemented.This test correctly verifies that the handler returns a BAD_REQUEST response when the roleId parameter is empty or invalid.
59-78: Token verification failure test is properly implemented.This test correctly mocks the token verification function to reject and verifies that the handler returns an INTERNAL_SERVER_ERROR response.
100-128: Success path test is comprehensively implemented.This test correctly mocks both the token verification and the role update function to succeed, and verifies that the handler returns a 204 No Content response, along with checking that the mocked functions were called as expected.
src/controllers/editGuildRolesHandler.ts (2)
10-16: Handler correctly extracts and sets up parameters.The handler properly extracts the authorization header, reason header, roleId parameter, and dev flag from the request.
17-26: Request validation is well implemented.The handler properly validates the authorization header, dev flag, and roleId parameter, returning appropriate error responses when validation fails.
| export interface updateRole { | ||
| roleid: string; | ||
| rolename: string; | ||
| mentionable: boolean; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Interface definition looks good
The updateRole interface correctly defines the structure needed for role updates with appropriate types for each property.
One nitpick: Consider using camelCase for property names (roleId instead of roleid) to maintain consistency with TypeScript naming conventions.
export interface updateRole {
- roleid: string;
+ roleId: string;
rolename: string;
mentionable: boolean;
}📝 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.
| export interface updateRole { | |
| roleid: string; | |
| rolename: string; | |
| mentionable: boolean; | |
| } | |
| export interface updateRole { | |
| roleId: string; | |
| rolename: string; | |
| mentionable: boolean; | |
| } |
| export async function editGuildRole( | ||
| rolename: string, | ||
| roleid: string, | ||
| env: env, | ||
| reason?: string | ||
| ) { | ||
| const editGuildRoleUrl = `${DISCORD_BASE_URL}/guilds/${env.DISCORD_GUILD_ID}/roles/${roleid}`; | ||
|
|
||
| const headers: HeadersInit = createDiscordHeaders({ | ||
| reason, | ||
| token: env.DISCORD_TOKEN, | ||
| }); | ||
| const data = { | ||
| name: rolename, | ||
| mentionable: true, | ||
| }; | ||
| try { | ||
| const response = await fetch(editGuildRoleUrl, { | ||
| method: "PATCH", | ||
| headers, | ||
| body: JSON.stringify(data), | ||
| }); | ||
| if (response.ok) { | ||
| return ROLE_UPDATED; | ||
| } else { | ||
| return INTERNAL_SERVER_ERROR; | ||
| } | ||
| } catch (err) { | ||
| return INTERNAL_SERVER_ERROR; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Function implementation needs improvement
The function correctly implements the Discord API call to update a role, but has a few issues:
- The function parameter list doesn't match the
updateRoleinterface - it doesn't accept thementionableparameter but hardcodes it totrue - There's no detailed error logging, which will make debugging difficult
Consider these improvements:
export async function editGuildRole(
rolename: string,
roleid: string,
env: env,
+ mentionable: boolean = true,
reason?: string
) {
const editGuildRoleUrl = `${DISCORD_BASE_URL}/guilds/${env.DISCORD_GUILD_ID}/roles/${roleid}`;
const headers: HeadersInit = createDiscordHeaders({
reason,
token: env.DISCORD_TOKEN,
});
const data = {
name: rolename,
- mentionable: true,
+ mentionable,
};
try {
const response = await fetch(editGuildRoleUrl, {
method: "PATCH",
headers,
body: JSON.stringify(data),
});
if (response.ok) {
return ROLE_UPDATED;
} else {
+ const errorData = await response.json().catch(() => null);
+ console.error("Failed to update role:", response.status, errorData);
return INTERNAL_SERVER_ERROR;
}
} catch (err) {
+ console.error("Exception when updating role:", err);
return INTERNAL_SERVER_ERROR;
}
}Let's verify if the mentionable property is used elsewhere:
🏁 Script executed:
#!/bin/bash
# Search for references to mentionable in the codebase to see if the property should be configurable
rg "mentionable" --type ts -A 3 -B 3Length of output: 3413
Align function signature with updateRole interface and improve error logging
To ensure consistency with the updateRole type and make debugging easier:
• File needing changes:
- src/utils/editGroupRole.ts
• What to update:
- Add a
mentionable: boolean = trueparameter. - Use
mentionableinstead of hard-codingtruein the request body. - Log response errors and caught exceptions for better visibility.
export async function editGuildRole(
rolename: string,
roleid: string,
env: env,
+ mentionable: boolean = true,
reason?: string
) {
const editGuildRoleUrl = `${DISCORD_BASE_URL}/guilds/${env.DISCORD_GUILD_ID}/roles/${roleid}`;
const headers: HeadersInit = createDiscordHeaders({
reason,
token: env.DISCORD_TOKEN,
});
const data = {
name: rolename,
- mentionable: true,
+ mentionable,
};
try {
const response = await fetch(editGuildRoleUrl, {
method: "PATCH",
headers,
body: JSON.stringify(data),
});
if (response.ok) {
return ROLE_UPDATED;
} else {
+ const errorData = await response.json().catch(() => null);
+ console.error("Failed to update role:", response.status, errorData);
return INTERNAL_SERVER_ERROR;
}
} catch (err) {
+ console.error("Exception when updating role:", err);
return INTERNAL_SERVER_ERROR;
}
}📝 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.
| export async function editGuildRole( | |
| rolename: string, | |
| roleid: string, | |
| env: env, | |
| reason?: string | |
| ) { | |
| const editGuildRoleUrl = `${DISCORD_BASE_URL}/guilds/${env.DISCORD_GUILD_ID}/roles/${roleid}`; | |
| const headers: HeadersInit = createDiscordHeaders({ | |
| reason, | |
| token: env.DISCORD_TOKEN, | |
| }); | |
| const data = { | |
| name: rolename, | |
| mentionable: true, | |
| }; | |
| try { | |
| const response = await fetch(editGuildRoleUrl, { | |
| method: "PATCH", | |
| headers, | |
| body: JSON.stringify(data), | |
| }); | |
| if (response.ok) { | |
| return ROLE_UPDATED; | |
| } else { | |
| return INTERNAL_SERVER_ERROR; | |
| } | |
| } catch (err) { | |
| return INTERNAL_SERVER_ERROR; | |
| } | |
| } | |
| export async function editGuildRole( | |
| rolename: string, | |
| roleid: string, | |
| env: env, | |
| mentionable: boolean = true, | |
| reason?: string | |
| ) { | |
| const editGuildRoleUrl = `${DISCORD_BASE_URL}/guilds/${env.DISCORD_GUILD_ID}/roles/${roleid}`; | |
| const headers: HeadersInit = createDiscordHeaders({ | |
| reason, | |
| token: env.DISCORD_TOKEN, | |
| }); | |
| const data = { | |
| name: rolename, | |
| mentionable, | |
| }; | |
| try { | |
| const response = await fetch(editGuildRoleUrl, { | |
| method: "PATCH", | |
| headers, | |
| body: JSON.stringify(data), | |
| }); | |
| if (response.ok) { | |
| return ROLE_UPDATED; | |
| } else { | |
| const errorData = await response.json().catch(() => null); | |
| console.error("Failed to update role:", response.status, errorData); | |
| return INTERNAL_SERVER_ERROR; | |
| } | |
| } catch (err) { | |
| console.error("Exception when updating role:", err); | |
| return INTERNAL_SERVER_ERROR; | |
| } | |
| } |
| import { DISCORD_BASE_URL } from "../../../src/constants/urls"; | ||
| import { editGuildRole } from "../../../src/utils/editGroupRole"; | ||
| import JSONResponse from "../../../src/utils/JsonResponse"; | ||
| import { guildEnv } from "../../fixtures/fixture"; | ||
| import * as response from "../../../src/constants/responses"; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Inconsistent naming between file and import path.
There's a naming inconsistency between the test file name (editGuildRole.test.ts) and the imported function path (src/utils/editGroupRole.ts). This could lead to confusion - one uses "Guild" and the other uses "Group".
Consider standardizing the naming convention across the project for consistency.
| it("should return INTERNAL_SERVER_ERROR when response is not ok", async () => { | ||
| const expectedErrorResponse = new Response(response.INTERNAL_SERVER_ERROR); | ||
| jest.spyOn(global, "fetch").mockRejectedValue(expectedErrorResponse); | ||
| const result = await editGuildRole(rolename, roleid, guildEnv); | ||
| expect(result).toEqual(response.INTERNAL_SERVER_ERROR); | ||
| expect(global.fetch).toHaveBeenCalledTimes(1); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Missing test for non-204 successful responses.
While the test covers the rejected Promise case, it doesn't test the scenario where the API responds with a status other than 204 but doesn't throw an error (the else branch in the utility function).
Consider adding a test for when the API responds with a non-204 status code:
it("should return INTERNAL_SERVER_ERROR when response status is not 204", async () => {
const expectedErrorResponse = new Response(null, {
status: 400,
});
jest.spyOn(global, "fetch").mockResolvedValue(expectedErrorResponse);
const result = await editGuildRole(rolename, roleid, guildEnv);
expect(result).toEqual(response.INTERNAL_SERVER_ERROR);
expect(global.fetch).toHaveBeenCalledTimes(1);
});| params: { | ||
| roleId: roleId, | ||
| }, | ||
| query: { | ||
| dev: "false", | ||
| }, | ||
| method: "PATCH", | ||
| headers: { Authorization: "Bearer testtoken" }, | ||
| }); | ||
| const response = await editGuildRoleHandler(mockRequest, guildEnv); | ||
| const jsonResponse = await response.json(); | ||
| expect(jsonResponse).toEqual(responseConstants.NOT_IMPLEMENTED); | ||
| }); | ||
| it("should return BAD_REQUEST when roleId is not valid", async () => { | ||
| const mockRequest = generateDummyRequestObject({ | ||
| url: "/roles", | ||
| params: { | ||
| roleId: "", | ||
| }, | ||
| query: { | ||
| dev: "true", | ||
| }, | ||
| method: "PATCH", | ||
| headers: { Authorization: "Bearer testtoken" }, | ||
| }); | ||
| const response = await editGuildRoleHandler(mockRequest, guildEnv); | ||
| const jsonResponse = await response.json(); | ||
| expect(jsonResponse).toEqual(responseConstants.BAD_REQUEST); | ||
| }); | ||
| it("should return INTERNAL_SERVER_ERROR when token is not verified", async () => { | ||
| const expectedResponse = responseConstants.INTERNAL_SERVER_ERROR; | ||
| const mockRequest = generateDummyRequestObject({ | ||
| url: "/roles", | ||
| params: { | ||
| roleId: roleId, | ||
| }, | ||
| query: { | ||
| dev: "true", | ||
| }, | ||
| method: "PATCH", | ||
| headers: { Authorization: "Bearer testtoken" }, | ||
| }); | ||
| jest | ||
| .spyOn(verifyTokenUtils, "verifyNodejsBackendAuthToken") | ||
| .mockRejectedValue(expectedResponse); | ||
| const response = await editGuildRoleHandler(mockRequest, guildEnv); | ||
| const jsonResponse = await response.json(); | ||
| expect(jsonResponse).toEqual(expectedResponse); | ||
| }); | ||
| it("should return INTERNAL_SERVER_ERROR when update fails", async () => { | ||
| const expectedResponse = responseConstants.INTERNAL_SERVER_ERROR; | ||
| const mockRequest = generateDummyRequestObject({ | ||
| url: "/roles", | ||
| params: { | ||
| roleId: roleId, | ||
| }, | ||
| query: { | ||
| dev: "true", | ||
| }, | ||
| method: "PATCH", | ||
| headers: { Authorization: "Bearer testtoken" }, | ||
| body: JSON.stringify({ rolename: "New Role Name" }), | ||
| }); | ||
| jest | ||
| .spyOn(verifyTokenUtils, "verifyNodejsBackendAuthToken") | ||
| .mockResolvedValueOnce(); | ||
| const response = await editGuildRoleHandler(mockRequest, guildEnv); | ||
| const jsonResponse = await response.json(); | ||
| expect(jsonResponse).toEqual(expectedResponse); | ||
| }); | ||
| it("should return ok response when role is updated", async () => { | ||
| const expectedResponse = new Response(null, { | ||
| status: 204, | ||
| }); | ||
| const mockRequest = generateDummyRequestObject({ | ||
| url: "/roles", | ||
| method: "PATCH", | ||
| params: { | ||
| roleId: roleId, | ||
| }, | ||
| query: { | ||
| dev: "true", | ||
| }, | ||
| headers: { Authorization: "Bearer testtoken" }, | ||
| json: () => Promise.resolve({ rolename: "something" }), | ||
| }); | ||
| const verifyTokenSpy = jest | ||
| .spyOn(verifyTokenUtils, "verifyNodejsBackendAuthToken") | ||
| .mockResolvedValueOnce(); | ||
| const editGuildRoleSpy = jest | ||
| .spyOn(editGuildRoleUtils, "editGuildRole") | ||
| .mockResolvedValueOnce(responseConstants.ROLE_UPDATED); | ||
| const response = await editGuildRoleHandler(mockRequest, guildEnv); | ||
|
|
||
| expect(verifyTokenSpy).toHaveBeenCalledTimes(1); | ||
| expect(editGuildRoleSpy).toHaveBeenCalledTimes(1); | ||
| expect(response).toEqual(expectedResponse); | ||
| expect(response.status).toEqual(expectedResponse.status); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Missing test for malformed request body.
The test suite doesn't cover the scenario where the request body is malformed or doesn't contain the required fields.
Consider adding a test for handling malformed JSON:
it("should return INTERNAL_SERVER_ERROR when the request body is malformed", async () => {
const mockRequest = generateDummyRequestObject({
url: "/roles",
params: {
roleId: roleId,
},
query: {
dev: "true",
},
method: "PATCH",
headers: { Authorization: "Bearer testtoken" },
json: () => Promise.reject(new Error("Invalid JSON")),
});
jest
.spyOn(verifyTokenUtils, "verifyNodejsBackendAuthToken")
.mockResolvedValueOnce();
const response = await editGuildRoleHandler(mockRequest, guildEnv);
const jsonResponse = await response.json();
expect(jsonResponse).toEqual(responseConstants.INTERNAL_SERVER_ERROR);
expect(response.status).toBe(500);
});| // This function updates group-role in discord. | ||
| import * as response from "../constants/responses"; | ||
| import { IRequest } from "itty-router"; | ||
| import { env } from "../typeDefinitions/default.types"; | ||
| import JSONResponse from "../utils/JsonResponse"; | ||
| import { verifyNodejsBackendAuthToken } from "../utils/verifyAuthToken"; | ||
| import { updateRole } from "../typeDefinitions/discordMessage.types"; | ||
| import { editGuildRole } from "../utils/editGroupRole"; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Inconsistent naming between file and import path.
Similar to the test file, there's a naming inconsistency between the handler file name (editGuildRolesHandler.ts) and the imported utility function path (editGroupRole.ts). One uses "Guild" and the other uses "Group".
Standardize the naming convention across the project for consistency.
| try { | ||
| await verifyNodejsBackendAuthToken(authHeader, env); | ||
| const body: updateRole = await request.json(); | ||
| const result = await editGuildRole(body.rolename, roleId, env, reason); | ||
|
|
||
| if (result === response.ROLE_UPDATED) { | ||
| return new Response(null, { status: 204 }); | ||
| } else { | ||
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | ||
| status: 500, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing validation for request body.
The handler parses the request body as an updateRole object but doesn't validate that the required rolename field is present.
Add validation for the required fields in the request body:
try {
await verifyNodejsBackendAuthToken(authHeader, env);
const body: updateRole = await request.json();
+ if (!body.rolename) {
+ return new JSONResponse(response.BAD_REQUEST, { status: 400 });
+ }
const result = await editGuildRole(body.rolename, roleId, env, reason);📝 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.
| try { | |
| await verifyNodejsBackendAuthToken(authHeader, env); | |
| const body: updateRole = await request.json(); | |
| const result = await editGuildRole(body.rolename, roleId, env, reason); | |
| if (result === response.ROLE_UPDATED) { | |
| return new Response(null, { status: 204 }); | |
| } else { | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| try { | |
| await verifyNodejsBackendAuthToken(authHeader, env); | |
| const body: updateRole = await request.json(); | |
| if (!body.rolename) { | |
| return new JSONResponse(response.BAD_REQUEST, { status: 400 }); | |
| } | |
| const result = await editGuildRole(body.rolename, roleId, env, reason); | |
| if (result === response.ROLE_UPDATED) { | |
| return new Response(null, { status: 204 }); | |
| } else { | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } |
| } catch (err) { | ||
| console.log("Error updating guild role: ", err); | ||
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | ||
| status: 500, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Error handling is too generic.
The catch block logs the error but doesn't distinguish between different types of errors, such as unauthorized access, rate limiting, or server errors. This makes debugging more difficult.
Consider enhancing error logging to provide more context:
} catch (err) {
- console.log("Error updating guild role: ", err);
+ console.error("Error updating guild role: ", {
+ roleId,
+ error: err instanceof Error ? err.message : String(err),
+ stack: err instanceof Error ? err.stack : undefined
+ });
return new JSONResponse(response.INTERNAL_SERVER_ERROR, {
status: 500,
});
}📝 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.
| } catch (err) { | |
| console.log("Error updating guild role: ", err); | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| } | |
| } catch (err) { | |
| console.error("Error updating guild role: ", { | |
| roleId, | |
| error: err instanceof Error ? err.message : String(err), | |
| stack: err instanceof Error ? err.stack : undefined | |
| }); | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| } |
| // This function updates group-role in discord. | ||
| import * as response from "../constants/responses"; | ||
| import { IRequest } from "itty-router"; | ||
| import { env } from "../typeDefinitions/default.types"; | ||
| import JSONResponse from "../utils/JsonResponse"; | ||
| import { verifyNodejsBackendAuthToken } from "../utils/verifyAuthToken"; | ||
| import { updateRole } from "../typeDefinitions/discordMessage.types"; | ||
| import { editGuildRole } from "../utils/editGroupRole"; | ||
|
|
||
| export async function editGuildRoleHandler(request: IRequest, env: env) { | ||
| const authHeader = request.headers.get("Authorization"); | ||
| const reason = request.headers.get("X-Audit-Log-Reason"); | ||
| const roleId = decodeURI(request.params?.roleId ?? ""); | ||
| const { dev } = request.query; | ||
| const devFlag = dev === "true"; | ||
|
|
||
| if (!authHeader) { | ||
| return new JSONResponse(response.BAD_SIGNATURE, { status: 401 }); | ||
| } | ||
|
|
||
| if (!devFlag) { | ||
| return new JSONResponse(response.NOT_IMPLEMENTED, { status: 501 }); | ||
| } | ||
| if (!roleId) { | ||
| return new JSONResponse(response.BAD_REQUEST, { status: 400 }); | ||
| } | ||
| try { | ||
| await verifyNodejsBackendAuthToken(authHeader, env); | ||
| const body: updateRole = await request.json(); | ||
| const result = await editGuildRole(body.rolename, roleId, env, reason); | ||
|
|
||
| if (result === response.ROLE_UPDATED) { | ||
| return new Response(null, { status: 204 }); | ||
| } else { | ||
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | ||
| status: 500, | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| console.log("Error updating guild role: ", err); | ||
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | ||
| status: 500, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add documentation for feature flag.
The handler implements a feature flag (dev=true) but doesn't document this in the file comments. Adding documentation would help other developers understand when and how the feature can be enabled.
Enhance the file comment to explain the feature flag:
-// This function updates group-role in discord.
+// This function updates group-role in discord.
+// Feature is behind a feature flag: set query parameter dev=true to enable it.
+// Example: PATCH /roles/:roleId?dev=true📝 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.
| // This function updates group-role in discord. | |
| import * as response from "../constants/responses"; | |
| import { IRequest } from "itty-router"; | |
| import { env } from "../typeDefinitions/default.types"; | |
| import JSONResponse from "../utils/JsonResponse"; | |
| import { verifyNodejsBackendAuthToken } from "../utils/verifyAuthToken"; | |
| import { updateRole } from "../typeDefinitions/discordMessage.types"; | |
| import { editGuildRole } from "../utils/editGroupRole"; | |
| export async function editGuildRoleHandler(request: IRequest, env: env) { | |
| const authHeader = request.headers.get("Authorization"); | |
| const reason = request.headers.get("X-Audit-Log-Reason"); | |
| const roleId = decodeURI(request.params?.roleId ?? ""); | |
| const { dev } = request.query; | |
| const devFlag = dev === "true"; | |
| if (!authHeader) { | |
| return new JSONResponse(response.BAD_SIGNATURE, { status: 401 }); | |
| } | |
| if (!devFlag) { | |
| return new JSONResponse(response.NOT_IMPLEMENTED, { status: 501 }); | |
| } | |
| if (!roleId) { | |
| return new JSONResponse(response.BAD_REQUEST, { status: 400 }); | |
| } | |
| try { | |
| await verifyNodejsBackendAuthToken(authHeader, env); | |
| const body: updateRole = await request.json(); | |
| const result = await editGuildRole(body.rolename, roleId, env, reason); | |
| if (result === response.ROLE_UPDATED) { | |
| return new Response(null, { status: 204 }); | |
| } else { | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| } catch (err) { | |
| console.log("Error updating guild role: ", err); | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| } | |
| // This function updates group-role in discord. | |
| // Feature is behind a feature flag: set query parameter dev=true to enable it. | |
| // Example: PATCH /roles/:roleId?dev=true | |
| import * as response from "../constants/responses"; | |
| import { IRequest } from "itty-router"; | |
| import { env } from "../typeDefinitions/default.types"; | |
| import JSONResponse from "../utils/JsonResponse"; | |
| import { verifyNodejsBackendAuthToken } from "../utils/verifyAuthToken"; | |
| import { updateRole } from "../typeDefinitions/discordMessage.types"; | |
| import { editGuildRole } from "../utils/editGroupRole"; | |
| export async function editGuildRoleHandler(request: IRequest, env: env) { | |
| const authHeader = request.headers.get("Authorization"); | |
| const reason = request.headers.get("X-Audit-Log-Reason"); | |
| const roleId = decodeURI(request.params?.roleId ?? ""); | |
| const { dev } = request.query; | |
| const devFlag = dev === "true"; | |
| if (!authHeader) { | |
| return new JSONResponse(response.BAD_SIGNATURE, { status: 401 }); | |
| } | |
| if (!devFlag) { | |
| return new JSONResponse(response.NOT_IMPLEMENTED, { status: 501 }); | |
| } | |
| if (!roleId) { | |
| return new JSONResponse(response.BAD_REQUEST, { status: 400 }); | |
| } | |
| try { | |
| await verifyNodejsBackendAuthToken(authHeader, env); | |
| const body: updateRole = await request.json(); | |
| const result = await editGuildRole(body.rolename, roleId, env, reason); | |
| if (result === response.ROLE_UPDATED) { | |
| return new Response(null, { status: 204 }); | |
| } else { | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| } catch (err) { | |
| console.log("Error updating guild role: ", err); | |
| return new JSONResponse(response.INTERNAL_SERVER_ERROR, { | |
| status: 500, | |
| }); | |
| } | |
| } |
| import { env } from "../typeDefinitions/default.types"; | ||
| import JSONResponse from "../utils/JsonResponse"; | ||
| import { verifyNodejsBackendAuthToken } from "../utils/verifyAuthToken"; | ||
| import { updateRole } from "../typeDefinitions/discordMessage.types"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Mismatch between interface and implementation.
The updateRole interface imported from discordMessage.types likely has fields roleid, rolename, and mentionable based on context, but only rolename is used in the implementation.
🏁 Script executed:
#!/bin/bash
# Check the updateRole interface definition
grep -A 10 "interface updateRole" src/typeDefinitions/discordMessage.types.ts || echo "Interface not found"Length of output: 205
🏁 Script executed:
#!/bin/bash
# Locate the updateRole interface definition
grep -R "interface updateRole" -n -A5 .
# Display the handler to see how updateRole is used
echo
echo "----- editGuildRolesHandler.ts (first 30 lines) -----"
sed -n '1,30p' src/controllers/editGuildRolesHandler.tsLength of output: 1893
🏁 Script executed:
#!/bin/bash
# Locate the editGuildRole implementation and display its signature
grep -R "export async function editGuildRole" -n src/utils/editGroupRole.ts || grep -R "function editGuildRole" -n src/utils/editGroupRole.ts
echo
echo "----- src/utils/editGroupRole.ts -----"
sed -n '1,50p' src/utils/editGroupRole.tsLength of output: 1180
Align the updateRole interface with its usage
The updateRole type includes roleid and mentionable, but the handler only uses rolename (and a path‐derived roleId), and the helper always sets mentionable: true. We should either remove unused fields from the interface or wire them into the implementation:
• In src/controllers/editGuildRolesHandler.ts
– Drop roleid/mentionable from the type or pull body.mentionable if you intend to honor the client’s flag.
– Pass body.roleid instead of the path param if that’s the source of truth.
• In src/utils/editGroupRole.ts
– Update the data payload to use a mentionable parameter.
– Adjust the function signature to accept (rolename: string, roleid: string, env: env, reason?: string, mentionable?: boolean).
Example diff for using mentionable from the request:
--- a/src/controllers/editGuildRolesHandler.ts
+++ b/src/controllers/editGuildRolesHandler.ts
@@ -26,7 +26,8 @@ export async function editGuildRoleHandler(request: IRequest, env: env) {
await verifyNodejsBackendAuthToken(authHeader, env);
const body: updateRole = await request.json();
- const result = await editGuildRole(body.rolename, roleId, env, reason);
+ const result = await editGuildRole(
+ body.rolename, body.roleid, env, reason, body.mentionable
+ );
return new JSONResponse(result);--- a/src/utils/editGroupRole.ts
+++ b/src/utils/editGroupRole.ts
@@ export async function editGuildRole(
- rolename: string,
- roleid: string,
- env: env,
- reason?: string
+) rolename: string,
+) roleid: string,
+) env: env,
+) reason?: string,
+) mentionable: boolean = true
) {
const editGuildRoleUrl = `${DISCORD_BASE_URL}/guilds/${env.DISCORD_GUILD_ID}/roles/${roleid}`;
@@
- const data = {
- name: rolename,
- mentionable: true,
- };
+ const data = { name: rolename, mentionable };
try {
const response = await fetch(editGuildRoleUrl, {
Date: 29th Aprill
Developer Name:
akshat
Issue Ticket Number
Real-Dev-Squad/website-dashboard#898
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes