remove unverified role on /verify (#2192)

* remove unverified role on /verify

* role actually got removed check added

* handled edge cases

* Update logger error utils/removeDiscordRole.js

Co-authored-by: Achintya Chatterjee <[email protected]>

* modified changes and edge cases

* added detailed messages

* fix: requested changes

* exported function directly

* fix: modified tests

---------

Co-authored-by: Achintya Chatterjee <[email protected]>

Author: shobhan-sundar-goutam

constants/discordRoles.ts

@@ -0,0 +1,3 @@
+export const DISCORD_ROLES = {
+  UNVERIFIED: "unverified",
+};

controllers/external-accounts.js

@@ -4,8 +4,10 @@ const { getDiscordMembers } = require("../services/discordService");
 const { addOrUpdate, getUsersByRole, updateUsersInBatch } = require("../models/users");
 const { retrieveDiscordUsers, fetchUsersForKeyValues } = require("../services/dataAccessLayer");
 const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../constants/external-accounts");
+const { DISCORD_ROLES } = require("../constants/discordRoles");
 const logger = require("../utils/logger");
 const { markUnDoneTasksOfArchivedUsersBacklog } = require("../models/tasks");
+const removeDiscordRoleUtils = require("../utils/removeDiscordRole");
 
 const addExternalAccountData = async (req, res) => {
   const createdOn = Date.now();
@@ -46,6 +48,7 @@ const getExternalAccountData = async (req, res) => {
     return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
   }
 };
+
 const linkExternalAccount = async (req, res) => {
   try {
     const { id: userId, roles } = req.userData;
@@ -69,6 +72,19 @@ const linkExternalAccount = async (req, res) => {
       userId
     );
 
+    const unverifiedRoleRemovalResponse = await removeDiscordRoleUtils.removeDiscordRole(
+      req.userData,
+      attributes.discordId,
+      undefined,
+      DISCORD_ROLES.UNVERIFIED
+    );
+
+    if (!unverifiedRoleRemovalResponse.success) {
+      return res.status(500).json({
+        message: `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin`,
+      });
+    }
+
     return res.status(204).json({ message: "Your discord profile has been linked successfully" });
   } catch (error) {
     logger.error(`Error getting external account data: ${error}`);

test/integration/external-accounts.test.js

@@ -17,6 +17,7 @@ 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 removeDiscordRoleUtils = require("../../utils/removeDiscordRole");
 chai.use(chaiHttp);
 const cookieName = config.get("userToken.cookieName");
 
@@ -537,6 +538,11 @@ describe("External Accounts", function () {
       expect(getUserResponseBeforeUpdate.body).to.not.have.property("discordId");
       expect(getUserResponseBeforeUpdate.body).to.not.have.property("discordJoinedAt");
 
+      const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRole").resolves({
+        success: true,
+        message: "Role deleted successfully",
+      });
+
       const response = await chai
         .request(app)
         .patch(`/external-accounts/link/${externalAccountData[2].token}`)
@@ -553,6 +559,60 @@ describe("External Accounts", function () {
       expect(updatedUserDetails.body.roles.in_discord).to.equal(true);
       expect(updatedUserDetails.body).to.have.property("discordId");
       expect(updatedUserDetails.body).to.have.property("discordJoinedAt");
+
+      removeDiscordRoleStub.restore();
+    });
+
+    it("Should return 500 when removeDiscordRole fails because role doesn't exist", async function () {
+      await externalAccountsModel.addExternalAccountData(externalAccountData[2]);
+
+      const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRole").resolves({
+        success: false,
+        message: "Role doesn't exist",
+      });
+
+      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}`);
+
+      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`
+      );
+
+      removeDiscordRoleStub.restore();
+    });
+
+    it("Should return 500 when removeDiscordRole fails because role deletion failed", async function () {
+      await externalAccountsModel.addExternalAccountData(externalAccountData[2]);
+
+      const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRole").resolves({
+        success: false,
+        message: "Role deletion 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}`);
+
+      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`
+      );
+
+      removeDiscordRoleStub.restore();
     });
   });
 });

test/unit/utils/removeDiscordRole.test.js

@@ -0,0 +1,108 @@
+const chai = require("chai");
+const Sinon = require("sinon");
+const { expect } = chai;
+const { removeDiscordRole } = require("../../../utils/removeDiscordRole");
+const addUser = require("../../utils/addUser");
+const cleanDb = require("../../utils/cleanDb");
+const firestore = require("../../../utils/firestore");
+const discordRolesModel = firestore.collection("discord-roles");
+const memberRoleModel = firestore.collection("member-group-roles");
+const userData = require("../../fixtures/user/user")();
+const { groupData, memberGroupData } = require("../../fixtures/discordactions/discordactions");
+
+describe("removeDiscordRole", function () {
+  let userId;
+  let discordId;
+  let roleid;
+  let rolename;
+  let fetchStub;
+
+  beforeEach(async function () {
+    userData[0].roles = { archived: false, in_discord: true };
+    userId = await addUser(userData[0]);
+    discordId = userData[0].discordId;
+    userData[0] = { ...userData[0], id: userId };
+
+    const addRolePromises = memberGroupData.map(async (data) => {
+      await memberRoleModel.add(data);
+    });
+    const discordRolesModelPromise = [discordRolesModel.add(groupData[0]), discordRolesModel.add(groupData[1])];
+    await Promise.all(discordRolesModelPromise);
+    roleid = groupData[0].roleid;
+    rolename = groupData[0].rolename;
+    await memberRoleModel.add({ roleid, userid: discordId });
+    await Promise.all(addRolePromises);
+
+    fetchStub = Sinon.stub(global, "fetch");
+  });
+
+  afterEach(async function () {
+    await cleanDb();
+    fetchStub.restore();
+  });
+
+  it("should remove discord role successfully", async function () {
+    fetchStub.returns(
+      Promise.resolve({ json: () => Promise.resolve({ success: true, message: "Role deleted successfully" }) })
+    );
+
+    const isDiscordRoleRemoved = await removeDiscordRole(userData[0], discordId, roleid, rolename);
+
+    expect(isDiscordRoleRemoved.success).to.be.equal(true);
+    expect(isDiscordRoleRemoved.message).to.be.equal("Role deleted successfully");
+  });
+
+  it("should throw an error if roleid and rolename doesn't exist in database when attempting to remove", async function () {
+    roleid = "randomRoleId";
+    rolename = "randomRoleName";
+
+    try {
+      await removeDiscordRole(userData[0], discordId, roleid, rolename);
+    } catch (error) {
+      expect(error.message).to.equal("Role doesn't exist");
+    }
+  });
+
+  it("should throw an error if roleid doesn't exist in database when attempting to remove", async function () {
+    roleid = "randomRoleId";
+    rolename = undefined;
+
+    try {
+      await removeDiscordRole(userData[0], discordId, roleid, rolename);
+    } catch (error) {
+      expect(error.message).to.equal("Role doesn't exist");
+    }
+  });
+
+  it("should throw an error if rolename doesn't exist in database when attempting to remove", async function () {
+    roleid = undefined;
+    rolename = "randomRoleName";
+
+    try {
+      await removeDiscordRole(userData[0], discordId, roleid, rolename);
+    } catch (error) {
+      expect(error.message).to.equal("Role doesn't exist");
+    }
+  });
+
+  it("should throw an error if rolename and roleid both are undefined", async function () {
+    roleid = undefined;
+    rolename = undefined;
+
+    try {
+      await removeDiscordRole(userData[0], discordId, roleid, rolename);
+    } catch (error) {
+      expect(error.message).to.equal("Role doesn't exist");
+    }
+  });
+
+  it("should throw an error if role deletion failed", async function () {
+    fetchStub.rejects(new Error("Role deletion failed"));
+
+    try {
+      await removeDiscordRole(userData[0], discordId, roleid, rolename);
+    } catch (error) {
+      expect(error.message).to.equal("Role deletion failed");
+    }
+  });
+});

utils/removeDiscordRole.js

@@ -0,0 +1,37 @@
+const discordRolesModel = require("../models/discordactions");
+const discordServices = require("../services/discordService");
+
+/**
+ * Removes a Discord role from a user, handling cases where either "roleid", "rolename", or both are provided.
+ *
+ * @param {Object} userData - User data.
+ * @param {string} discordId - User's Discord ID.
+ * @param {string} roleid - Role ID. If not provided, defaults to "undefined".
+ * @param {string} rolename - Role name. If not provided, defaults to "undefined".
+ *
+ * @returns {Promise<Object>} - Result with success status and message.
+ */
+export const removeDiscordRole = async (userData, discordId, roleid, rolename) => {
+  try {
+    const role = await discordRolesModel.isGroupRoleExists({ roleid, rolename });
+
+    if (!role.roleExists) {
+      throw new Error("Role doesn't exist");
+    }
+
+    const roleData = role.existingRoles.docs[0].data();
+
+    await discordServices.removeRoleFromUser(roleData.roleid, discordId, userData);
+
+    const { wasSuccess } = await discordRolesModel.removeMemberGroup(roleData.roleid, discordId);
+    if (!wasSuccess) {
+      throw new Error("Role deletion failed");
+    }
+
+    return { success: true, message: "Role deleted successfully" };
+  } catch (error) {
+    logger.error(`Error removing role ${rolename || roleid} for user ${discordId}: ${error.message}`);
+
+    return { success: false, message: error.message };
+  }
+};