From fe63b6c05522be9a46b2ea4143472fb64f92933f Mon Sep 17 00:00:00 2001 From: anuj chhikara Date: Fri, 6 Feb 2026 22:51:32 +0530 Subject: [PATCH 1/7] feat: enhance Discord invite generation and role handling --- controllers/discordactions.js | 1 + controllers/external-accounts.js | 25 +--------------------- middlewares/checkCanGenerateDiscordLink.ts | 7 +++++- routes/discordactions.js | 2 +- types/global.d.ts | 2 +- utils/discord-actions.js | 5 ++++- 6 files changed, 14 insertions(+), 28 deletions(-) diff --git a/controllers/discordactions.js b/controllers/discordactions.js index 1b21180c4..7ad3fed4d 100644 --- a/controllers/discordactions.js +++ b/controllers/discordactions.js @@ -505,6 +505,7 @@ const generateInviteForUser = async (req, res) => { const inviteOptions = { channelId: channelId, + role: req.approvedApplicationRole, }; const response = await fetch(`${DISCORD_BASE_URL}/invite`, { method: "POST", diff --git a/controllers/external-accounts.js b/controllers/external-accounts.js index d369573ed..f3e16d0e1 100644 --- a/controllers/external-accounts.js +++ b/controllers/external-accounts.js @@ -5,7 +5,6 @@ const { addOrUpdate, getUsersByRole, updateUsersInBatch } = require("../models/u const { retrieveDiscordUsers, fetchUsersForKeyValues } = require("../services/dataAccessLayer"); const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../constants/external-accounts"); const removeDiscordRoleUtils = require("../utils/removeDiscordRoleFromUser"); -const addDiscordRoleUtils = require("../utils/addDiscordRoleToUser"); const config = require("config"); const logger = require("../utils/logger"); const { markUnDoneTasksOfArchivedUsersBacklog } = require("../models/tasks"); @@ -80,29 +79,7 @@ const linkExternalAccount = async (req, res) => { ); if (!unverifiedRoleRemovalResponse.success) { - // Tolerable errors that should not block role assignment - const tolerableErrors = ["Role doesn't exist", "Role deletion from database failed"]; - const isTolerableError = tolerableErrors.some((err) => unverifiedRoleRemovalResponse.message.includes(err)); - - if (!isTolerableError) { - const message = `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin`; - return res.boom.internal(message, { message }); - } - logger.info( - `Tolerable error during unverified role removal for Discord ID: ${attributes.discordId}: ${unverifiedRoleRemovalResponse.message}` - ); - } - - const developerRoleId = config.get("discordDeveloperRoleId"); - const newRoleId = config.get("discordNewRoleId"); - - try { - await addDiscordRoleUtils.addDiscordRoleToUser(attributes.discordId, developerRoleId, "Developer"); - await addDiscordRoleUtils.addDiscordRoleToUser(attributes.discordId, newRoleId, "New"); - logger.info(`Roles (Developer, New) assigned successfully for Discord ID: ${attributes.discordId}`); - } catch (roleError) { - logger.error(`Error assigning roles after verification: ${roleError}`); - const message = `Your discord profile has been linked but role assignment failed. Please contact admin`; + const message = `Unverified role removal failed. Please contact admin`; return res.boom.internal(message, { message }); } diff --git a/middlewares/checkCanGenerateDiscordLink.ts b/middlewares/checkCanGenerateDiscordLink.ts index 5f1693943..e936b45f6 100644 --- a/middlewares/checkCanGenerateDiscordLink.ts +++ b/middlewares/checkCanGenerateDiscordLink.ts @@ -23,11 +23,16 @@ const checkCanGenerateDiscordLink = async (req: CustomRequest, res: CustomRespon } const approvedApplication = applications.find((application: { status: string; }) => application.status === 'accepted'); - + if (!approvedApplication) { return res.boom.forbidden("Only users with an accepted application can generate a Discord invite link."); } + if (approvedApplication.isNew !== true) { + return res.boom.forbidden("No applications found."); + } + + req.approvedApplicationRole = approvedApplication.role; return next(); } catch (error) { return res.boom.badImplementation("An error occurred while checking user applications."); diff --git a/routes/discordactions.js b/routes/discordactions.js index b0f197b86..0f8a8cdb6 100644 --- a/routes/discordactions.js +++ b/routes/discordactions.js @@ -47,7 +47,7 @@ router.get("/invite", disableRoute, authenticate, getUserDiscordInvite); * Short-circuit this POST method for this endpoint * Refer https://github.com/Real-Dev-Squad/todo-action-items/issues/269 for more details. */ -router.post("/invite", disableRoute, authenticate, checkCanGenerateDiscordLink, generateInviteForUser); +router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser); router.delete("/roles", authenticate, checkIsVerifiedDiscord, deleteRole); router.get("/roles", authenticate, checkIsVerifiedDiscord, getGroupsRoleId); diff --git a/types/global.d.ts b/types/global.d.ts index 28e766270..db3c4a19b 100644 --- a/types/global.d.ts +++ b/types/global.d.ts @@ -42,4 +42,4 @@ export type userData = { }; export type CustomResponse = Response & { boom: Boom }; -export type CustomRequest = Request & { userData }; +export type CustomRequest = Request & { userData; approvedApplicationRole?: string }; diff --git a/utils/discord-actions.js b/utils/discord-actions.js index 1a14f70e1..09c51b93e 100644 --- a/utils/discord-actions.js +++ b/utils/discord-actions.js @@ -55,7 +55,10 @@ const generateDiscordInviteLink = async () => { const response = await fetch(`${DISCORD_BASE_URL}/invite`, { method: "POST", body: JSON.stringify(inviteOptions), - headers: { "Content-Type": "application/json", Authorization: `Bearer ${authToken}` }, + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${authToken}`, + }, }); if (!response.ok) { const error = await response.json(); From 2f2999f9e45ec5c7141d5c2d643383abb14626b2 Mon Sep 17 00:00:00 2001 From: anuj chhikara Date: Tue, 10 Feb 2026 21:57:41 +0530 Subject: [PATCH 2/7] chore: remove formattig changes --- utils/discord-actions.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/utils/discord-actions.js b/utils/discord-actions.js index 09c51b93e..1a14f70e1 100644 --- a/utils/discord-actions.js +++ b/utils/discord-actions.js @@ -55,10 +55,7 @@ const generateDiscordInviteLink = async () => { const response = await fetch(`${DISCORD_BASE_URL}/invite`, { method: "POST", body: JSON.stringify(inviteOptions), - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${authToken}`, - }, + headers: { "Content-Type": "application/json", Authorization: `Bearer ${authToken}` }, }); if (!response.ok) { const error = await response.json(); From 24e170da1bac4d67a91352ac2f81dd3e5c2f2303 Mon Sep 17 00:00:00 2001 From: anuj chhikara Date: Tue, 10 Feb 2026 22:58:01 +0530 Subject: [PATCH 3/7] feat: enhance group role creation by adding query parameter validation --- controllers/discordactions.js | 4 +- middlewares/validators/discordactions.js | 10 +- test/integration/discordactions.test.js | 190 +++++++++++++++++++++ test/integration/external-accounts.test.js | 63 ++----- 4 files changed, 213 insertions(+), 54 deletions(-) diff --git a/controllers/discordactions.js b/controllers/discordactions.js index 7ad3fed4d..6312c3c09 100644 --- a/controllers/discordactions.js +++ b/controllers/discordactions.js @@ -7,6 +7,7 @@ const discordServices = require("../services/discordService"); const { fetchAllUsers, fetchUser } = require("../models/users"); const { generateCloudFlareHeaders } = require("../utils/discord-actions"); const { addLog } = require("../models/logs"); +const logger = require("../utils/logger"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const discordMavenRoleId = config.get("discordMavenRoleId"); @@ -23,7 +24,8 @@ const DISCORD_BASE_URL = config.get("services.discordBot.baseUrl"); const createGroupRole = async (req, res) => { try { - const rolename = `group-${req.body.rolename}`; + const { role = false } = req.query; + const rolename = role ? req.body.rolename : `group-${req.body.rolename}`; const { roleExists } = await discordRolesModel.isGroupRoleExists({ rolename }); diff --git a/middlewares/validators/discordactions.js b/middlewares/validators/discordactions.js index 42eaf8775..eb57aeb31 100644 --- a/middlewares/validators/discordactions.js +++ b/middlewares/validators/discordactions.js @@ -1,14 +1,20 @@ const Joi = require("joi"); const { validateMillisecondsTimestamp } = require("./utils"); +const logger = require("../../utils/logger"); const validateGroupRoleBody = async (req, res, next) => { - const schema = Joi.object({ + const bodySchema = Joi.object({ rolename: Joi.string().trim().required(), description: Joi.string().trim(), }); + const querySchema = Joi.object({ + role: Joi.boolean().default(false).optional(), + }); + try { - await schema.validateAsync(req.body); + await bodySchema.validateAsync(req.body); + req.query = await querySchema.validateAsync(req.query); next(); } catch (error) { logger.error(`Error validating createGroupRole payload : ${error}`); diff --git a/test/integration/discordactions.test.js b/test/integration/discordactions.test.js index ac5c7af1a..0b885acd2 100644 --- a/test/integration/discordactions.test.js +++ b/test/integration/discordactions.test.js @@ -1377,4 +1377,194 @@ describe("Discord actions", function () { }); }); }); + + describe("POST /discord-actions/groups (createGroupRole)", function () { + let testUserId; + let testUserAuthToken; + + beforeEach(async function () { + testUserId = await addUser(userData[0]); + testUserAuthToken = authService.generateAuthToken({ userId: testUserId }); + + fetchStub.returns( + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ id: "discord-role-id-123" }), + }) + ); + + sinon.stub(discordRolesModel, "isGroupRoleExists").resolves({ + roleExists: false, + }); + }); + + afterEach(async function () { + await cleanDb(); + }); + + it("should create a group role with 'group-' prefix when role query param is not provided", function (done) { + const roleData = { + rolename: "developers", + description: "Test group role", + }; + + chai + .request(app) + .post("/discord-actions/groups") + .set("cookie", `${cookieName}=${testUserAuthToken}`) + .send(roleData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(201); + expect(res.body).to.be.an("object"); + expect(res.body.message).to.equal("Role created successfully!"); + expect(res.body).to.have.property("id"); + expect(fetchStub.calledOnce).to.equal(true); + const fetchCall = fetchStub.getCall(0); + const requestBody = JSON.parse(fetchCall.args[1].body); + expect(requestBody.rolename).to.equal("group-developers"); + + return done(); + }); + }); + + it("should create a group role with 'group-' prefix when role=false", function (done) { + const roleData = { + rolename: "developers", + description: "Test group role", + }; + + chai + .request(app) + .post("/discord-actions/groups?role=false") + .set("cookie", `${cookieName}=${testUserAuthToken}`) + .send(roleData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(201); + expect(res.body).to.be.an("object"); + expect(res.body.message).to.equal("Role created successfully!"); + const fetchCall = fetchStub.getCall(0); + const requestBody = JSON.parse(fetchCall.args[1].body); + expect(requestBody.rolename).to.equal("group-developers"); + + return done(); + }); + }); + + it("should create a custom role without prefix when role=true", function (done) { + const roleData = { + rolename: "developers", + description: "Test custom role", + }; + + chai + .request(app) + .post("/discord-actions/groups?role=true") + .set("cookie", `${cookieName}=${testUserAuthToken}`) + .send(roleData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(201); + expect(res.body).to.be.an("object"); + expect(res.body.message).to.equal("Role created successfully!"); + const fetchCall = fetchStub.getCall(0); + const requestBody = JSON.parse(fetchCall.args[1].body); + expect(requestBody.rolename).to.equal("developers"); + + return done(); + }); + }); + + it("should return 400 when role query param is not a boolean", function (done) { + const roleData = { + rolename: "developers", + description: "Test role", + }; + + chai + .request(app) + .post("/discord-actions/groups?role=invalid") + .set("cookie", `${cookieName}=${testUserAuthToken}`) + .send(roleData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body.error).to.equal("Bad Request"); + + return done(); + }); + }); + + it("should return 400 when role already exists", function (done) { + sinon.restore(); + fetchStub = sinon.stub(global, "fetch"); + fetchStub.returns( + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ id: "discord-role-id-123" }), + }) + ); + + sinon.stub(discordRolesModel, "isGroupRoleExists").resolves({ + roleExists: true, + }); + + const roleData = { + rolename: "developers", + description: "Test role", + }; + + chai + .request(app) + .post("/discord-actions/groups?role=true") + .set("cookie", `${cookieName}=${testUserAuthToken}`) + .send(roleData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body.message).to.equal("Role already exists!"); + + return done(); + }); + }); + + it("should handle string 'true' and 'false' as boolean values", function (done) { + const roleData = { + rolename: "testrole", + description: "Test role", + }; + + chai + .request(app) + .post("/discord-actions/groups?role=true") + .set("cookie", `${cookieName}=${testUserAuthToken}`) + .send(roleData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(201); + + // Verify role was created without prefix + const fetchCall = fetchStub.getCall(0); + const requestBody = JSON.parse(fetchCall.args[1].body); + expect(requestBody.rolename).to.equal("testrole"); + + return done(); + }); + }); + }); }); diff --git a/test/integration/external-accounts.test.js b/test/integration/external-accounts.test.js index a9753f582..72da8b520 100644 --- a/test/integration/external-accounts.test.js +++ b/test/integration/external-accounts.test.js @@ -13,12 +13,12 @@ const { usersFromRds, getDiscordMembers } = require("../fixtures/discordResponse const Sinon = require("sinon"); const { INTERNAL_SERVER_ERROR } = require("../../constants/errorMessages"); const removeDiscordRoleUtils = require("../../utils/removeDiscordRoleFromUser"); -const addDiscordRoleUtils = require("../../utils/addDiscordRoleToUser"); const firestore = require("../../utils/firestore"); const userData = require("../fixtures/user/user")(); const userModel = firestore.collection("users"); const tasksModel = firestore.collection("tasks"); const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../../constants/external-accounts"); +const config = require("config"); chai.use(chaiHttp); const cookieName = config.get("userToken.cookieName"); @@ -527,7 +527,7 @@ describe("External Accounts", function () { }); }); - it("Should return 204 when valid action is provided and assign Developer and New roles", async function () { + it("Should return 200 when valid token is provided and link discord account successfully", async function () { await externalAccountsModel.addExternalAccountData(externalAccountData[2]); const getUserResponseBeforeUpdate = await chai .request(app) @@ -544,11 +544,6 @@ describe("External Accounts", function () { message: "Role deleted successfully", }); - const addDiscordRoleStub = Sinon.stub(addDiscordRoleUtils, "addDiscordRoleToUser").resolves({ - success: true, - message: "Role added successfully", - }); - const response = await chai .request(app) .patch(`/external-accounts/link/${externalAccountData[2].token}`) @@ -559,12 +554,6 @@ describe("External Accounts", function () { expect(response.body).to.have.property("message"); expect(response.body.message).to.equal("Your discord profile has been linked successfully"); - expect(addDiscordRoleStub.calledTwice).to.equal(true); - expect(addDiscordRoleStub.firstCall.args[0]).to.equal(externalAccountData[2].attributes.discordId); - expect(addDiscordRoleStub.firstCall.args[2]).to.equal("Developer"); - expect(addDiscordRoleStub.secondCall.args[0]).to.equal(externalAccountData[2].attributes.discordId); - expect(addDiscordRoleStub.secondCall.args[2]).to.equal("New"); - const updatedUserDetails = await chai .request(app) .get("/users/self") @@ -575,10 +564,9 @@ describe("External Accounts", function () { expect(updatedUserDetails.body).to.have.property("discordJoinedAt"); removeDiscordRoleStub.restore(); - addDiscordRoleStub.restore(); }); - it("Should return 500 when addDiscordRole fails after successful verification", async function () { + it("Should return 200 when unverified role removal succeeds", async function () { await externalAccountsModel.addExternalAccountData(externalAccountData[2]); const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ @@ -586,36 +574,25 @@ describe("External Accounts", function () { message: "Role deleted successfully", }); - const addDiscordRoleStub = Sinon.stub(addDiscordRoleUtils, "addDiscordRoleToUser").rejects( - new Error("Role assignment failed") - ); - const response = await chai .request(app) .patch(`/external-accounts/link/${externalAccountData[2].token}`) .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) .set("Cookie", `${cookieName}=${newUserJWT}`); - expect(response).to.have.status(500); + expect(response).to.have.status(200); expect(response.body).to.be.an("object"); expect(response.body).to.have.property("message"); - expect(response.body.message).to.equal( - "Your discord profile has been linked but role assignment failed. Please contact admin" - ); + expect(response.body.message).to.equal("Your discord profile has been linked successfully"); expect(removeDiscordRoleStub.calledOnce).to.equal(true); expect(removeDiscordRoleStub.firstCall.args[1]).to.equal(externalAccountData[2].attributes.discordId); expect(removeDiscordRoleStub.firstCall.args[2]).to.equal(config.get("discordUnverifiedRoleId")); - expect(addDiscordRoleStub.calledTwice).to.not.equal(true); - expect(addDiscordRoleStub.firstCall.args[0]).to.equal(externalAccountData[2].attributes.discordId); - expect(addDiscordRoleStub.firstCall.args[2]).to.equal("Developer"); - expect(addDiscordRoleStub.calledOnce).to.equal(true); // New role was not assigned removeDiscordRoleStub.restore(); - addDiscordRoleStub.restore(); }); - it("Should continue with role assignment when removeDiscordRole fails because role doesn't exist (tolerable error)", async function () { + it("Should return 500 when removeDiscordRole fails", async function () { await externalAccountsModel.addExternalAccountData(externalAccountData[2]); const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ @@ -623,24 +600,18 @@ describe("External Accounts", function () { message: "Role doesn't exist", }); - const addDiscordRoleStub = Sinon.stub(addDiscordRoleUtils, "addDiscordRoleToUser").resolves({ - success: true, - message: "Role added successfully", - }); - const response = await chai .request(app) .patch(`/external-accounts/link/${externalAccountData[2].token}`) .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) .set("Cookie", `${cookieName}=${newUserJWT}`); - expect(response).to.have.status(200); + expect(response).to.have.status(500); expect(response.body).to.be.an("object"); expect(response.body).to.have.property("message"); - expect(response.body.message).to.equal("Your discord profile has been linked successfully"); + expect(response.body.message).to.equal("Unverified role removal failed. Please contact admin"); removeDiscordRoleStub.restore(); - addDiscordRoleStub.restore(); }); it("Should return 500 when removeDiscordRole fails because role deletion from discord failed", async function () { @@ -657,19 +628,15 @@ describe("External Accounts", function () { .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) .set("Cookie", `${cookieName}=${newUserJWT}`); - const unverifiedRoleRemovalResponse = await removeDiscordRoleStub(); - expect(response).to.have.status(500); expect(response.body).to.be.an("object"); expect(response.body).to.have.property("message"); - expect(response.body.message).to.equal( - `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin` - ); + expect(response.body.message).to.equal("Unverified role removal failed. Please contact admin"); removeDiscordRoleStub.restore(); }); - it("Should continue with role assignment when removeDiscordRole fails because role deletion from database failed (tolerable error)", async function () { + it("Should return 500 when removeDiscordRole fails because role deletion from database failed", async function () { await externalAccountsModel.addExternalAccountData(externalAccountData[2]); const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ @@ -677,24 +644,18 @@ describe("External Accounts", function () { message: "Role deletion from database failed", }); - const addDiscordRoleStub = Sinon.stub(addDiscordRoleUtils, "addDiscordRoleToUser").resolves({ - success: true, - message: "Role added successfully", - }); - const response = await chai .request(app) .patch(`/external-accounts/link/${externalAccountData[2].token}`) .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) .set("Cookie", `${cookieName}=${newUserJWT}`); - expect(response).to.have.status(200); + expect(response).to.have.status(500); expect(response.body).to.be.an("object"); expect(response.body).to.have.property("message"); - expect(response.body.message).to.equal("Your discord profile has been linked successfully"); + expect(response.body.message).to.equal("Unverified role removal failed. Please contact admin"); removeDiscordRoleStub.restore(); - addDiscordRoleStub.restore(); }); }); }); From 869e0bb4c8679ce39297e9bb1f267df8634d37d0 Mon Sep 17 00:00:00 2001 From: Anuj Chhikara <107175639+AnujChhikara@users.noreply.github.com> Date: Wed, 11 Feb 2026 02:03:10 +0530 Subject: [PATCH 4/7] feat: improve application update logic and validation (#2561) * feat: enhance application update functionality with validation and error handling - Added a new error message for editing applications too soon. - Implemented a function to build the update payload for applications. - Updated the application update logic to include user authorization and time-based restrictions. - Refactored the application validator to include comprehensive validation for update data. - Adjusted routes to use the new validation function for application updates. * feat: add validation for application update empty payload * fix: update validation for numberOfHours in application update to allow a maximum of 168 hours * refactor: enhance application update payload handling and validation * refactor: streamline application update process and logging * feat: enhance application update responses * refactor: update social link validation to use consistent property name "phoneNumber" --------- Co-authored-by: Amit Prakash <34869115+iamitprakash@users.noreply.github.com> --- constants/application.ts | 5 ++ controllers/applications.ts | 37 ++++---- middlewares/validators/application.ts | 88 +++++++++++++++---- models/applications.ts | 59 +++++++++++-- routes/applications.ts | 3 +- test/integration/application.test.ts | 38 ++++---- .../middlewares/application-validator.test.ts | 24 ++--- test/unit/models/application.test.ts | 10 --- types/application.d.ts | 17 +++- utils/application.ts | 48 +++++++++- 10 files changed, 245 insertions(+), 84 deletions(-) diff --git a/constants/application.ts b/constants/application.ts index 9cd9b6398..60f5d0a15 100644 --- a/constants/application.ts +++ b/constants/application.ts @@ -16,6 +16,7 @@ const APPLICATION_ROLES = { const API_RESPONSE_MESSAGES = { APPLICATION_CREATED_SUCCESS: "Application created successfully", + APPLICATION_UPDATED_SUCCESS: "Application updated successfully", APPLICATION_RETURN_SUCCESS: "Applications returned successfully", NUDGE_SUCCESS: "Nudge sent successfully", FEEDBACK_SUBMITTED_SUCCESS: "Application feedback submitted successfully", @@ -23,8 +24,12 @@ const API_RESPONSE_MESSAGES = { const APPLICATION_ERROR_MESSAGES = { APPLICATION_ALREADY_REVIEWED: "Application has already been reviewed", + APPLICATION_NOT_FOUND: "Application not found", + APPLICATION_EDIT_UNAUTHORIZED: "You are not authorized to edit this application", NUDGE_TOO_SOON: "Nudge unavailable. You'll be able to nudge again after 24 hours.", NUDGE_ONLY_PENDING_ALLOWED: "Nudge unavailable. Only pending applications can be nudged.", + EDIT_TOO_SOON: "You can edit your application again 24 hours after your last edit.", + EMPTY_UPDATE_PAYLOAD: "Update payload must include at least one editable field.", }; const APPLICATION_LOG_MESSAGES = { diff --git a/controllers/applications.ts b/controllers/applications.ts index 169ba9db4..0809348fa 100644 --- a/controllers/applications.ts +++ b/controllers/applications.ts @@ -8,6 +8,7 @@ const { createApplicationService } = require("../services/applicationService"); const { Conflict } = require("http-errors"); const logger = require("../utils/logger"); const { APPLICATION_STATUS_TYPES } = require("../constants/application"); +const { buildApplicationUpdatePayload } = require("../utils/application"); const getAllOrUserApplication = async (req: CustomRequest, res: CustomResponse): Promise => { try { @@ -105,26 +106,26 @@ const updateApplication = async (req: CustomRequest, res: CustomResponse) => { try { const { applicationId } = req.params; const rawBody = req.body; + const dataToUpdate = buildApplicationUpdatePayload(rawBody); + const userId = req.userData.id; + const username = req.userData.username; - const applicationLog = { - type: logType.APPLICATION_UPDATED, - meta: { - applicationId, - username: req.userData.username, - userId: req.userData.id, - }, - body: rawBody, - }; - - const promises = [ - ApplicationModel.updateApplication(rawBody, applicationId), - addLog(applicationLog.type, applicationLog.meta, applicationLog.body), - ]; + const result = await ApplicationModel.updateApplication(dataToUpdate, applicationId, userId, username, rawBody); - await Promise.all(promises); - return res.json({ - message: "Application updated successfully!", - }); + switch (result.status) { + case APPLICATION_STATUS.notFound: + return res.boom.notFound(APPLICATION_ERROR_MESSAGES.APPLICATION_NOT_FOUND); + case APPLICATION_STATUS.unauthorized: + return res.boom.unauthorized(APPLICATION_ERROR_MESSAGES.APPLICATION_EDIT_UNAUTHORIZED); + case APPLICATION_STATUS.tooSoon: + return res.boom.conflict(APPLICATION_ERROR_MESSAGES.EDIT_TOO_SOON); + case APPLICATION_STATUS.success: + return res.json({ + message: API_RESPONSE_MESSAGES.APPLICATION_UPDATED_SUCCESS, + }); + default: + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } } catch (err) { logger.error(`Error while updating the application: ${err}`); return res.boom.badImplementation(INTERNAL_SERVER_ERROR); diff --git a/middlewares/validators/application.ts b/middlewares/validators/application.ts index bd8c79d97..6f4293ff3 100644 --- a/middlewares/validators/application.ts +++ b/middlewares/validators/application.ts @@ -6,24 +6,24 @@ const { APPLICATION_STATUS_TYPES, APPLICATION_ROLES } = require("../../constants const { phoneNumberRegex } = require("../../constants/subscription-validator"); const logger = require("../../utils/logger"); +const socialLinkSchema = joi + .object({ + phoneNumber: joi.string().optional().regex(phoneNumberRegex).message('"phoneNumber" must be in a valid format'), + github: joi.string().min(1).optional(), + instagram: joi.string().min(1).optional(), + linkedin: joi.string().min(1).optional(), + twitter: joi.string().min(1).optional(), + peerlist: joi.string().min(1).optional(), + behance: joi.string().min(1).optional(), + dribbble: joi.string().min(1).optional(), + }) + .optional(); + const validateApplicationData = async (req: CustomRequest, res: CustomResponse, next: NextFunction) => { - if (req.body.socialLink?.phoneNo) { - req.body.socialLink.phoneNo = req.body.socialLink.phoneNo.trim(); + if (req.body.socialLink?.phoneNumber) { + req.body.socialLink.phoneNumber = req.body.socialLink.phoneNumber.trim(); } - const socialLinkSchema = joi - .object({ - phoneNo: joi.string().optional().regex(phoneNumberRegex).message('"phoneNo" must be in a valid format'), - github: joi.string().min(1).optional(), - instagram: joi.string().min(1).optional(), - linkedin: joi.string().min(1).optional(), - twitter: joi.string().min(1).optional(), - peerlist: joi.string().min(1).optional(), - behance: joi.string().min(1).optional(), - dribbble: joi.string().min(1).optional(), - }) - .optional(); - const schema = joi .object() .strict() @@ -65,11 +65,11 @@ const validateApplicationData = async (req: CustomRequest, res: CustomResponse, next(); } catch (error) { logger.error(`Error in validating application data: ${error}`); - res.boom.badRequest(error.details[0].message); + return res.boom.badRequest(error.details[0].message); } }; -const validateApplicationUpdateData = async (req: CustomRequest, res: CustomResponse, next: NextFunction) => { +const validateApplicationFeedbackData = async (req: CustomRequest, res: CustomResponse, next: NextFunction) => { const schema = joi .object({ status: joi @@ -109,7 +109,56 @@ const validateApplicationUpdateData = async (req: CustomRequest, res: CustomResp next(); } catch (error) { logger.error(`Error in validating recruiter data: ${error}`); - res.boom.badRequest(error.details[0].message); + return res.boom.badRequest(error.details[0].message); + } +}; + +const validateApplicationUpdateData = async (req: CustomRequest, res: CustomResponse, next: NextFunction) => { + if (req.body.socialLink?.phoneNumber) { + req.body.socialLink.phoneNumber = req.body.socialLink.phoneNumber.trim(); + } + + const professionalSchema = joi + .object({ + institution: joi.string().min(1).optional(), + skills: joi.string().min(5).optional(), + }) + .optional(); + + const schema = joi + .object() + .strict() + .min(1) + .keys({ + imageUrl: joi.string().uri().optional(), + foundFrom: joi.string().min(1).optional(), + introduction: joi.string().min(1).optional(), + forFun: joi + .string() + .custom((value, helpers) => customWordCountValidator(value, helpers, 100)) + .optional(), + funFact: joi + .string() + .custom((value, helpers) => customWordCountValidator(value, helpers, 100)) + .optional(), + whyRds: joi + .string() + .custom((value, helpers) => customWordCountValidator(value, helpers, 100)) + .optional(), + numberOfHours: joi.number().min(1).max(168).optional(), + professional: professionalSchema, + socialLink: socialLinkSchema, + }) + .messages({ + "object.min": "Update payload must contain at least one allowed field.", + }); + + try { + await schema.validateAsync(req.body); + next(); + } catch (error) { + logger.error(`Error in validating application update data: ${error}`); + return res.boom.badRequest(error.details[0].message); } }; @@ -127,12 +176,13 @@ const validateApplicationQueryParam = async (req: CustomRequest, res: CustomResp next(); } catch (error) { logger.error(`Error validating query params : ${error}`); - res.boom.badRequest(error.details[0].message); + return res.boom.badRequest(error.details[0].message); } }; module.exports = { validateApplicationData, + validateApplicationFeedbackData, validateApplicationUpdateData, validateApplicationQueryParam, }; diff --git a/models/applications.ts b/models/applications.ts index 636178460..9f59b84f3 100644 --- a/models/applications.ts +++ b/models/applications.ts @@ -1,4 +1,6 @@ import { application } from "../types/application"; +import { addLog } from "./logs"; +const { logType } = require("../constants/logs"); const firestore = require("../utils/firestore"); const logger = require("../utils/logger"); const ApplicationsModel = firestore.collection("applicants"); @@ -128,13 +130,58 @@ const addApplication = async (data: application) => { } }; -const updateApplication = async (dataToUpdate: object, applicationId: string) => { - try { - await ApplicationsModel.doc(applicationId).update(dataToUpdate); - } catch (err) { - logger.error("Error in updating intro", err); - throw err; +const updateApplication = async ( + dataToUpdate: object, + applicationId: string, + userId: string, + username: string, + rawBody: object +) => { + const result = await firestore.runTransaction(async (transaction) => { + const currentTime = Date.now(); + const twentyFourHoursInMilliseconds = convertDaysToMilliseconds(1); + + const applicationRef = ApplicationsModel.doc(applicationId); + const applicationDoc = await transaction.get(applicationRef); + + if (!applicationDoc.exists) { + return { status: APPLICATION_STATUS.notFound }; + } + + const application = applicationDoc.data(); + + if (application.userId !== userId) { + return { status: APPLICATION_STATUS.unauthorized }; + } + + const lastEditAt = application.lastEditAt; + if (lastEditAt) { + const lastEditTimestamp = new Date(lastEditAt).getTime(); + const timeDifference = currentTime - lastEditTimestamp; + + if (timeDifference < twentyFourHoursInMilliseconds) { + return { status: APPLICATION_STATUS.tooSoon }; + } + } + + const requestBody = { + ...dataToUpdate, + lastEditAt: new Date(currentTime).toISOString(), + }; + transaction.update(applicationRef, requestBody); + + return { status: APPLICATION_STATUS.success }; + }); + + if (result.status === APPLICATION_STATUS.success) { + await addLog( + logType.APPLICATION_UPDATED, + { applicationId, username, userId }, + rawBody + ); } + + return result; }; const nudgeApplication = async ({ applicationId, userId }: { applicationId: string; userId: string }) => { diff --git a/routes/applications.ts b/routes/applications.ts index 2b309d883..e2e7d88aa 100644 --- a/routes/applications.ts +++ b/routes/applications.ts @@ -17,11 +17,12 @@ router.get( ); router.get("/:applicationId", authenticate, authorizeRoles([SUPERUSER]), applications.getApplicationById); router.post("/", authenticate, applicationValidator.validateApplicationData, applications.addApplication); +router.patch("/:applicationId", authenticate, applicationValidator.validateApplicationUpdateData, applications.updateApplication); router.patch( "/:applicationId/feedback", authenticate, authorizeRoles([SUPERUSER]), - applicationValidator.validateApplicationUpdateData, + applicationValidator.validateApplicationFeedbackData, applications.submitApplicationFeedback ); router.patch("/:applicationId/nudge", authenticate, applications.nudgeApplication); diff --git a/test/integration/application.test.ts b/test/integration/application.test.ts index 3d1212a3b..6fe07392c 100644 --- a/test/integration/application.test.ts +++ b/test/integration/application.test.ts @@ -689,21 +689,29 @@ describe("Application", function () { expect(res.body.nudgeCount).to.be.equal(1); const twentyFiveHoursAgo = new Date(Date.now() - 25 * 60 * 60 * 1000).toISOString(); - applicationModel.updateApplication({ lastNudgeAt: twentyFiveHoursAgo }, nudgeApplicationId).then(() => { - chai - .request(app) - .patch(`/applications/${nudgeApplicationId}/nudge`) - .set("cookie", `${cookieName}=${jwt}`) - .end(function (err, res) { - if (err) return done(err); - - expect(res).to.have.status(200); - expect(res.body.message).to.be.equal(API_RESPONSE_MESSAGES.NUDGE_SUCCESS); - expect(res.body.nudgeCount).to.be.equal(2); - expect(res.body.lastNudgeAt).to.be.a("string"); - done(); - }); - }); + applicationModel + .updateApplication( + { lastNudgeAt: twentyFiveHoursAgo }, + nudgeApplicationId, + userId, + appOwner.username, + {} + ) + .then(() => { + chai + .request(app) + .patch(`/applications/${nudgeApplicationId}/nudge`) + .set("cookie", `${cookieName}=${jwt}`) + .end(function (err, res) { + if (err) return done(err); + + expect(res).to.have.status(200); + expect(res.body.message).to.be.equal(API_RESPONSE_MESSAGES.NUDGE_SUCCESS); + expect(res.body.nudgeCount).to.be.equal(2); + expect(res.body.lastNudgeAt).to.be.a("string"); + done(); + }); + }) }); }); diff --git a/test/unit/middlewares/application-validator.test.ts b/test/unit/middlewares/application-validator.test.ts index 633d8231f..84c80faf9 100644 --- a/test/unit/middlewares/application-validator.test.ts +++ b/test/unit/middlewares/application-validator.test.ts @@ -111,7 +111,7 @@ describe("application validator test", function () { status: "accepted", feedback: "some feedback", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(1); }); @@ -119,7 +119,7 @@ describe("application validator test", function () { req.body = { batman: true, }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(0); }); @@ -127,7 +127,7 @@ describe("application validator test", function () { req.body = { status: "something", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(0); }); @@ -136,7 +136,7 @@ describe("application validator test", function () { status: "accepted", feedback: "Great work!", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(1); }); @@ -145,7 +145,7 @@ describe("application validator test", function () { status: "rejected", feedback: "Not a good fit", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(1); }); @@ -154,7 +154,7 @@ describe("application validator test", function () { status: "changes_requested", feedback: "Please update your skills section", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(1); }); @@ -162,7 +162,7 @@ describe("application validator test", function () { req.body = { status: "changes_requested", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(0); }); @@ -171,7 +171,7 @@ describe("application validator test", function () { status: "changes_requested", feedback: "", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(0); }); @@ -180,7 +180,7 @@ describe("application validator test", function () { status: "accepted", feedback: "", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(1); }); @@ -189,7 +189,7 @@ describe("application validator test", function () { status: "rejected", feedback: "", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(1); }); @@ -197,7 +197,7 @@ describe("application validator test", function () { req.body = { feedback: "Some feedback", }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(0); }); @@ -205,7 +205,7 @@ describe("application validator test", function () { req.body = { status: null, }; - await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + await applicationValidator.validateApplicationFeedbackData(req, res, nextSpy); expect(nextSpy.callCount).to.equal(0); }); }); diff --git a/test/unit/models/application.test.ts b/test/unit/models/application.test.ts index 07e1080aa..7a939a3bc 100644 --- a/test/unit/models/application.test.ts +++ b/test/unit/models/application.test.ts @@ -109,16 +109,6 @@ describe("applications", function () { }); }); - describe("updateApplication", function () { - it("should update a particular application", async function () { - const dataToUpdate = { status: "accepted" }; - await ApplicationModel.updateApplication(dataToUpdate, applicationId1); - const application = await ApplicationModel.getApplicationById(applicationId1); - - expect(application.status).to.be.equal("accepted"); - }); - }); - describe("addApplicationFeedback", function () { let testApplicationId: string; const reviewerName = "test-reviewer"; diff --git a/types/application.d.ts b/types/application.d.ts index 0e9a1c00e..b2b0f54db 100644 --- a/types/application.d.ts +++ b/types/application.d.ts @@ -1,7 +1,7 @@ export type ApplicationRole = "developer" | "designer" | "product_manager" | "project_manager" | "qa" | "social_media"; export type SocialLink = { - phoneNo?: string; + phoneNumber?: string; github?: string; instagram?: string; linkedin?: string; @@ -72,3 +72,18 @@ export type applicationPayload = { imageUrl?: string; socialLink?: SocialLink; }; + +export type applicationUpdatePayload = { + imageUrl?: string; + foundFrom?: string; + introduction?: string; + forFun?: string; + funFact?: string; + whyRds?: string; + numberOfHours?: number; + professional?: { + institution?: string; + skills?: string; + }; + socialLink?: SocialLink; +}; diff --git a/utils/application.ts b/utils/application.ts index 8eff73c63..b1a999612 100644 --- a/utils/application.ts +++ b/utils/application.ts @@ -1,4 +1,4 @@ -import { applicationPayload, application } from "../types/application"; +import { applicationPayload, application, applicationUpdatePayload } from "../types/application"; const getUserApplicationObject = (rawData: applicationPayload, userId: string, createdAt: string): application => { const data = { @@ -30,4 +30,48 @@ const getUserApplicationObject = (rawData: applicationPayload, userId: string, c return data; }; -module.exports = { getUserApplicationObject } +const FLAT_FIELD_MAP: Record, string> = { + imageUrl: "imageUrl", + foundFrom: "foundFrom", + introduction: "intro.introduction", + forFun: "intro.forFun", + funFact: "intro.funFact", + whyRds: "intro.whyRds", + numberOfHours: "intro.numberOfHours", +}; + +const PROFESSIONAL_KEYS = ["institution", "skills"] as const; +const SOCIAL_LINK_KEYS = ["phoneNumber", "github", "instagram", "linkedin", "twitter", "peerlist", "behance", "dribbble"] as const; + +const buildApplicationUpdatePayload = (body: applicationUpdatePayload): Record => { + const dataToUpdate: Record = {}; + + Object.entries(FLAT_FIELD_MAP).forEach(([key, path]) => { + const value = body[key as keyof typeof FLAT_FIELD_MAP]; + if (value != null) { + dataToUpdate[path] = value; + } + }); + + if (body.professional && typeof body.professional === "object") { + PROFESSIONAL_KEYS.forEach((key) => { + const value = body.professional![key]; + if (value != null) { + dataToUpdate[`professional.${key}`] = value; + } + }); + } + + if (body.socialLink && typeof body.socialLink === "object") { + SOCIAL_LINK_KEYS.forEach((key) => { + const value = body.socialLink![key]; + if (value != null) { + dataToUpdate[`socialLink.${key}`] = value; + } + }); + } + + return dataToUpdate; +}; + +module.exports = { getUserApplicationObject, buildApplicationUpdatePayload } From c775258e0f644d2d4fc45c8edeef409e4258249c Mon Sep 17 00:00:00 2001 From: Anuj Chhikara <107175639+AnujChhikara@users.noreply.github.com> Date: Thu, 12 Feb 2026 00:22:10 +0530 Subject: [PATCH 5/7] test: implement tests for application update functionality (#2562) * feat: enhance application update functionality with validation and error handling - Added a new error message for editing applications too soon. - Implemented a function to build the update payload for applications. - Updated the application update logic to include user authorization and time-based restrictions. - Refactored the application validator to include comprehensive validation for update data. - Adjusted routes to use the new validation function for application updates. * feat: add validation for application update empty payload * feat: implement comprehensive tests for application update functionality - Added integration tests for the PATCH /applications/:applicationId endpoint, covering various scenarios including successful updates, validation errors, and authorization checks. - Enhanced unit tests for the applications controller to validate update logic and error handling. - Introduced validation tests for application update data to ensure proper request structure and content. - Updated application model tests to verify correct behavior for update operations under different conditions. * fix: update validation for numberOfHours in application update to allow a maximum of 168 hours * fix: update test to reflect new maximum for numberOfHours validation in application update * refactor: enhance application update payload handling and validation * refactor: streamline application update process and logging * test: update application tests to remove logging dependency * feat: enhance application update responses * refactor: update social link validation to use consistent property name "phoneNumber" * refactor: remove updateApplication tests and logging dependency from application test suite * fix: standardize success message and update social link property name in application validation tests --------- Co-authored-by: Amit Prakash <34869115+iamitprakash@users.noreply.github.com> --- test/integration/application.test.ts | 147 ++++++++++++++++++ test/unit/controllers/applications.test.ts | 138 ++++++++++++++++ .../middlewares/application-validator.test.ts | 125 ++++++++++++++- 3 files changed, 409 insertions(+), 1 deletion(-) diff --git a/test/integration/application.test.ts b/test/integration/application.test.ts index 6fe07392c..8b48dcd78 100644 --- a/test/integration/application.test.ts +++ b/test/integration/application.test.ts @@ -356,6 +356,153 @@ describe("Application", function () { }); }); + describe("PATCH /applications/:applicationId", function () { + it("should return 200 and update application when owner sends valid payload", function (done) { + chai + .request(app) + .patch(`/applications/${applicationId1}`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ introduction: "Updated introduction text" }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(200); + expect(res.body.message).to.be.equal("Application updated successfully"); + return done(); + }); + }); + + it("should return 400 when request body is empty", function (done) { + chai + .request(app) + .patch(`/applications/${applicationId1}`) + .set("cookie", `${cookieName}=${jwt}`) + .send({}) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body.error).to.be.equal("Bad Request"); + expect(res.body.message).to.include("at least one allowed field"); + return done(); + }); + }); + + it("should return 400 when request body contains disallowed field", function (done) { + chai + .request(app) + .patch(`/applications/${applicationId1}`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ batman: true }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body.error).to.be.equal("Bad Request"); + return done(); + }); + }); + + it("should return 400 when imageUrl is not a valid URI", function (done) { + chai + .request(app) + .patch(`/applications/${applicationId1}`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ imageUrl: "not-a-valid-uri" }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body.error).to.be.equal("Bad Request"); + return done(); + }); + }); + + it("should return 404 when application does not exist", function (done) { + chai + .request(app) + .patch(`/applications/non-existent-application-id`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ introduction: "Updated" }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(404); + expect(res.body.error).to.be.equal("Not Found"); + expect(res.body.message).to.be.equal("Application not found"); + return done(); + }); + }); + + it("should return 401 when user is not authenticated", function (done) { + chai + .request(app) + .patch(`/applications/${applicationId1}`) + .send({ introduction: "Updated" }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(401); + expect(res.body.error).to.be.equal("Unauthorized"); + expect(res.body.message).to.be.equal("Unauthenticated User"); + return done(); + }); + }); + + it("should return 401 when user does not own the application", function (done) { + chai + .request(app) + .patch(`/applications/${applicationId1}`) + .set("cookie", `${cookieName}=${secondUserJwt}`) + .send({ introduction: "Updated" }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(401); + expect(res.body.error).to.be.equal("Unauthorized"); + expect(res.body.message).to.be.equal("You are not authorized to edit this application"); + return done(); + }); + }); + + it("should return 409 when edit is attempted within 24 hours of last edit", async function () { + const applicationForEditTest = { ...applicationsData[0], userId }; + const editTestApplicationId = await applicationModel.addApplication(applicationForEditTest); + + const firstRes = await chai + .request(app) + .patch(`/applications/${editTestApplicationId}`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ introduction: "First edit" }); + + expect(firstRes).to.have.status(200); + + const secondRes = await chai + .request(app) + .patch(`/applications/${editTestApplicationId}`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ foundFrom: "Second edit" }); + + expect(secondRes).to.have.status(409); + expect(secondRes.body.error).to.be.equal("Conflict"); + expect(secondRes.body.message).to.be.equal(APPLICATION_ERROR_MESSAGES.EDIT_TOO_SOON); + }); + }); + describe("PATCH /applications/:applicationId/feedback", function () { it("should return 200 if the user is super user and application feedback is submitted", function (done) { chai diff --git a/test/unit/controllers/applications.test.ts b/test/unit/controllers/applications.test.ts index 39aff8302..f13748a39 100644 --- a/test/unit/controllers/applications.test.ts +++ b/test/unit/controllers/applications.test.ts @@ -4,6 +4,144 @@ import { CustomRequest, CustomResponse } from "../../../types/global"; const applicationsController = require("../../../controllers/applications"); const ApplicationModel = require("../../../models/applications"); const { API_RESPONSE_MESSAGES, APPLICATION_ERROR_MESSAGES } = require("../../../constants/application"); +const { APPLICATION_STATUS } = require("../../../constants/application"); + +describe("updateApplication", () => { + let req: Partial; + let res: Partial & { + json: sinon.SinonSpy; + boom: { + notFound: sinon.SinonSpy; + unauthorized: sinon.SinonSpy; + conflict: sinon.SinonSpy; + badImplementation: sinon.SinonSpy; + }; + }; + let jsonSpy: sinon.SinonSpy; + let boomNotFound: sinon.SinonSpy; + let boomUnauthorized: sinon.SinonSpy; + let boomConflict: sinon.SinonSpy; + let boomBadImplementation: sinon.SinonSpy; + let updateApplicationStub: sinon.SinonStub; + + const mockApplicationId = "app-id-123"; + const mockUserId = "user-id-456"; + const mockUsername = "testuser"; + + beforeEach(() => { + jsonSpy = sinon.spy(); + boomNotFound = sinon.spy(); + boomUnauthorized = sinon.spy(); + boomConflict = sinon.spy(); + boomBadImplementation = sinon.spy(); + + req = { + params: { applicationId: mockApplicationId }, + body: { introduction: "Updated introduction text" }, + userData: { id: mockUserId, username: mockUsername }, + }; + + res = { + json: jsonSpy, + boom: { + notFound: boomNotFound, + unauthorized: boomUnauthorized, + conflict: boomConflict, + badImplementation: boomBadImplementation, + }, + }; + + updateApplicationStub = sinon.stub(ApplicationModel, "updateApplication"); + }); + + afterEach(() => { + sinon.restore(); + }); + + describe("Success cases", () => { + it("should return 200 and log when update succeeds", async () => { + updateApplicationStub.resolves({ status: APPLICATION_STATUS.success }); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + expect(updateApplicationStub.calledOnce).to.be.true; + expect(updateApplicationStub.firstCall.args[0]).to.deep.include({ "intro.introduction": "Updated introduction text" }); + expect(updateApplicationStub.firstCall.args[1]).to.equal(mockApplicationId); + expect(updateApplicationStub.firstCall.args[2]).to.equal(mockUserId); + expect(updateApplicationStub.firstCall.args[3]).to.equal(mockUsername); + expect(updateApplicationStub.firstCall.args[4]).to.deep.equal(req.body); + + expect(jsonSpy.calledOnce).to.be.true; + expect(jsonSpy.firstCall.args[0].message).to.equal("Application updated successfully"); + }); + + it("should build payload with professional and intro fields correctly", async () => { + req.body = { + professional: { institution: "MIT", skills: "React, Node" }, + whyRds: "one ".repeat(100).trim(), + }; + updateApplicationStub.resolves({ status: APPLICATION_STATUS.success }); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + const payload = updateApplicationStub.firstCall.args[0]; + expect(payload).to.have.property("professional.institution", "MIT"); + expect(payload).to.have.property("professional.skills", "React, Node"); + expect(payload).to.have.property("intro.whyRds"); + expect(jsonSpy.calledOnce).to.be.true; + }); + }); + + describe("Error cases", () => { + it("should return 404 when application is not found", async () => { + updateApplicationStub.resolves({ status: APPLICATION_STATUS.notFound }); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + expect(boomNotFound.calledOnce).to.be.true; + expect(boomNotFound.firstCall.args[0]).to.equal("Application not found"); + expect(jsonSpy.called).to.be.false; + }); + + it("should return 401 when user is not the owner", async () => { + updateApplicationStub.resolves({ status: APPLICATION_STATUS.unauthorized }); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + expect(boomUnauthorized.calledOnce).to.be.true; + expect(boomUnauthorized.firstCall.args[0]).to.equal("You are not authorized to edit this application"); + expect(jsonSpy.called).to.be.false; + }); + + it("should return 409 when edit is attempted within 24 hours", async () => { + updateApplicationStub.resolves({ status: APPLICATION_STATUS.tooSoon }); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + expect(boomConflict.calledOnce).to.be.true; + expect(boomConflict.firstCall.args[0]).to.equal(APPLICATION_ERROR_MESSAGES.EDIT_TOO_SOON); + expect(jsonSpy.called).to.be.false; + }); + + it("should return 500 when model returns unexpected status", async () => { + updateApplicationStub.resolves({ status: "unknown" }); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + expect(boomBadImplementation.calledOnce).to.be.true; + expect(jsonSpy.called).to.be.false; + }); + + it("should return 500 when model throws", async () => { + updateApplicationStub.rejects(new Error("DB error")); + + await applicationsController.updateApplication(req as CustomRequest, res as CustomResponse); + + expect(boomBadImplementation.calledOnce).to.be.true; + expect(jsonSpy.called).to.be.false; + }); + }); +}); describe("nudgeApplication", () => { let req: Partial; diff --git a/test/unit/middlewares/application-validator.test.ts b/test/unit/middlewares/application-validator.test.ts index 84c80faf9..3eb7a1687 100644 --- a/test/unit/middlewares/application-validator.test.ts +++ b/test/unit/middlewares/application-validator.test.ts @@ -89,7 +89,7 @@ describe("application validator test", function () { }); }); - describe("validateApplicationUpdateData", function () { + describe("validateApplicationFeedbackData", function () { let req: any; let res: any; let nextSpy: sinon.SinonSpy; @@ -210,6 +210,129 @@ describe("application validator test", function () { }); }); + describe("validateApplicationUpdateData", function () { + let req: any; + let res: any; + let nextSpy: sinon.SinonSpy; + + const validWordString = + "one two three four five six seven eight nine ten eleven twelve thirteen fourteen fifteen sixteen seventeen eighteen nineteen twenty " + + "twenty-one twenty-two twenty-three twenty-four twenty-five twenty-six twenty-seven twenty-eight twenty-nine thirty " + + "thirty-one thirty-two thirty-three thirty-four thirty-five thirty-six thirty-seven thirty-eight thirty-nine forty " + + "forty-one forty-two forty-three forty-four forty-five forty-six forty-seven forty-eight forty-nine fifty " + + "fifty-one fifty-two fifty-three fifty-four fifty-five fifty-six fifty-seven fifty-eight fifty-nine sixty " + + "sixty-one sixty-two sixty-three sixty-four sixty-five sixty-six sixty-seven sixty-eight sixty-nine seventy " + + "seventy-one seventy-two seventy-three seventy-four seventy-five seventy-six seventy-seven seventy-eight seventy-nine eighty " + + "eighty-one eighty-two eighty-three eighty-four eighty-five eighty-six eighty-seven eighty-eight eighty-nine ninety " + + "ninety-one ninety-two ninety-three ninety-four ninety-five ninety-six ninety-seven ninety-eight ninety-nine hundred"; + + beforeEach(function () { + req = { body: {} }; + res = { boom: { badRequest: Sinon.spy() } }; + nextSpy = Sinon.spy(); + }); + + it("should call next when body has at least one allowed field (introduction)", async function () { + req.body = { introduction: "Updated intro" }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + expect(res.boom.badRequest.called).to.be.false; + }); + + it("should call next when body has imageUrl as valid URI", async function () { + req.body = { imageUrl: "https://example.com/photo.jpg" }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + }); + + it("should call next when body has foundFrom", async function () { + req.body = { foundFrom: "LinkedIn" }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + }); + + it("should call next when body has numberOfHours within range", async function () { + req.body = { numberOfHours: 50 }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + }); + + it("should call next when body has professional with institution and skills", async function () { + req.body = { professional: { institution: "MIT", skills: "React, Node.js, TypeScript" } }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + }); + + it("should call next when body has socialLink with valid phoneNumber", async function () { + req.body = { socialLink: { phoneNumber: "+919876543210", github: "https://github.com/user" } }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + }); + + it("should call next when body has forFun/funFact/whyRds with at least 100 words", async function () { + req.body = { forFun: validWordString }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(1); + }); + + it("should not call next when body is empty", async function () { + req.body = {}; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.calledOnce).to.be.true; + expect(res.boom.badRequest.firstCall.args[0]).to.include("at least one allowed field"); + }); + + it("should not call next when body has only disallowed field", async function () { + req.body = { batman: true }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + + it("should not call next when imageUrl is not a valid URI", async function () { + req.body = { imageUrl: "not-a-uri" }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + + it("should not call next when professional.skills has fewer than 5 characters", async function () { + req.body = { professional: { skills: "abc" } }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + + it("should not call next when forFun has fewer than 100 words", async function () { + req.body = { forFun: "just a few words here" }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + + it("should not call next when numberOfHours is less than 1", async function () { + req.body = { numberOfHours: 0 }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + + it("should not call next when numberOfHours is greater than 168", async function () { + req.body = { numberOfHours: 170 }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + + it("should not call next when socialLink.phoneNumber has invalid format", async function () { + req.body = { socialLink: { phoneNumber: "invalid" } }; + await applicationValidator.validateApplicationUpdateData(req, res, nextSpy); + expect(nextSpy.callCount).to.equal(0); + expect(res.boom.badRequest.called).to.be.true; + }); + }); + describe("validateApplicationQueryParam", function () { let req: any; let res: any; From ce8dc24a904818b5ebc5d76c517a0c9351718388 Mon Sep 17 00:00:00 2001 From: anuj chhikara Date: Thu, 12 Feb 2026 09:36:43 +0530 Subject: [PATCH 6/7] refactor: clean up discord actions and improve error handling --- controllers/discordactions.js | 1 - controllers/external-accounts.js | 3 +-- middlewares/checkCanGenerateDiscordLink.ts | 4 ++-- test/integration/discordactions.test.js | 4 +--- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/controllers/discordactions.js b/controllers/discordactions.js index 6312c3c09..c5b74634d 100644 --- a/controllers/discordactions.js +++ b/controllers/discordactions.js @@ -7,7 +7,6 @@ const discordServices = require("../services/discordService"); const { fetchAllUsers, fetchUser } = require("../models/users"); const { generateCloudFlareHeaders } = require("../utils/discord-actions"); const { addLog } = require("../models/logs"); -const logger = require("../utils/logger"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const discordMavenRoleId = config.get("discordMavenRoleId"); diff --git a/controllers/external-accounts.js b/controllers/external-accounts.js index f3e16d0e1..a2719ba76 100644 --- a/controllers/external-accounts.js +++ b/controllers/external-accounts.js @@ -79,8 +79,7 @@ const linkExternalAccount = async (req, res) => { ); if (!unverifiedRoleRemovalResponse.success) { - const message = `Unverified role removal failed. Please contact admin`; - return res.boom.internal(message, { message }); + return res.boom.internal(null, { message: "Unverified role removal failed. Please contact admin" }); } return res.status(200).json({ message: "Your discord profile has been linked successfully" }); diff --git a/middlewares/checkCanGenerateDiscordLink.ts b/middlewares/checkCanGenerateDiscordLink.ts index e936b45f6..8f10c075e 100644 --- a/middlewares/checkCanGenerateDiscordLink.ts +++ b/middlewares/checkCanGenerateDiscordLink.ts @@ -28,8 +28,8 @@ const checkCanGenerateDiscordLink = async (req: CustomRequest, res: CustomRespon return res.boom.forbidden("Only users with an accepted application can generate a Discord invite link."); } - if (approvedApplication.isNew !== true) { - return res.boom.forbidden("No applications found."); + if (approvedApplication.isNew !== true || !approvedApplication.role) { + return res.boom.forbidden("Your are not eligible to generate a Discord invite link."); } req.approvedApplicationRole = approvedApplication.role; diff --git a/test/integration/discordactions.test.js b/test/integration/discordactions.test.js index 0b885acd2..69a1d7750 100644 --- a/test/integration/discordactions.test.js +++ b/test/integration/discordactions.test.js @@ -1541,7 +1541,7 @@ describe("Discord actions", function () { }); }); - it("should handle string 'true' and 'false' as boolean values", function (done) { + it("should handle string 'true' as true boolean value", function (done) { const roleData = { rolename: "testrole", description: "Test role", @@ -1557,8 +1557,6 @@ describe("Discord actions", function () { return done(err); } expect(res).to.have.status(201); - - // Verify role was created without prefix const fetchCall = fetchStub.getCall(0); const requestBody = JSON.parse(fetchCall.args[1].body); expect(requestBody.rolename).to.equal("testrole"); From 60ab85cf4562ab211e574ab59bdf36a1f3169b44 Mon Sep 17 00:00:00 2001 From: anuj chhikara Date: Thu, 12 Feb 2026 09:42:51 +0530 Subject: [PATCH 7/7] fix: correct typo in error message for Discord invite eligibility --- middlewares/checkCanGenerateDiscordLink.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/middlewares/checkCanGenerateDiscordLink.ts b/middlewares/checkCanGenerateDiscordLink.ts index 8f10c075e..9fd7278a6 100644 --- a/middlewares/checkCanGenerateDiscordLink.ts +++ b/middlewares/checkCanGenerateDiscordLink.ts @@ -29,7 +29,7 @@ const checkCanGenerateDiscordLink = async (req: CustomRequest, res: CustomRespon } if (approvedApplication.isNew !== true || !approvedApplication.role) { - return res.boom.forbidden("Your are not eligible to generate a Discord invite link."); + return res.boom.forbidden("You are not eligible to generate a Discord invite link."); } req.approvedApplicationRole = approvedApplication.role;