Skip to content

Commit

Permalink
Add unverified role to db and remove on /verify (#2257)
Browse files Browse the repository at this point in the history
* add unverified role to db and remove on /verify

* destructured role and variable naming

* fix: generic logs and removed unnecessary statement

* refactor: improve error handling and logging in removeDiscordRoleFromUser function

* refactor: simplify error message handling

* batch db insertion

* changed to typescript and modified unit tests
  • Loading branch information
shobhan-sundar-goutam authored Dec 3, 2024
1 parent fccab02 commit 74f8324
Show file tree
Hide file tree
Showing 7 changed files with 299 additions and 19 deletions.
3 changes: 3 additions & 0 deletions constants/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export const logType = {
TASKS_MISSED_UPDATES_ERRORS: "TASKS_MISSED_UPDATES_ERRORS",
DISCORD_INVITES: "DISCORD_INVITES",
EXTERNAL_SERVICE: "EXTERNAL_SERVICE",
ADD_UNVERIFIED_ROLE: "ADD_UNVERIFIED_ROLE",
REMOVE_ROLE_FROM_USER_SUCCESS: "REMOVE_ROLE_FROM_USER_SUCCESS",
REMOVE_ROLE_FROM_USER_FAILED: "REMOVE_ROLE_FROM_USER_FAILED",
EXTENSION_REQUESTS: "extensionRequests",
TASK: "task",
TASK_REQUESTS: "taskRequests",
Expand Down
20 changes: 11 additions & 9 deletions controllers/external-accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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 discordServices = require("../services/discordService");
const removeDiscordRoleUtils = require("../utils/removeDiscordRoleFromUser");
const config = require("config");
const logger = require("../utils/logger");
const { markUnDoneTasksOfArchivedUsersBacklog } = require("../models/tasks");
Expand Down Expand Up @@ -71,14 +71,16 @@ const linkExternalAccount = async (req, res) => {
userId
);

try {
const unverifiedRoleId = config.get("discordUnverifiedRoleId");
await discordServices.removeRoleFromUser(unverifiedRoleId, attributes.discordId, req.userData);
} catch (error) {
logger.error(`Error getting external account data: ${error}`);
return res.boom.internal("Role Deletion failed. Please contact admin.", {
message: "Role Deletion failed. Please contact admin.",
});
const unverifiedRoleId = config.get("discordUnverifiedRoleId");
const unverifiedRoleRemovalResponse = await removeDiscordRoleUtils.removeDiscordRoleFromUser(
req.userData,
attributes.discordId,
unverifiedRoleId
);

if (!unverifiedRoleRemovalResponse.success) {
const message = `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin`;
return res.boom.internal(message, { message });
}

return res.status(204).json({ message: "Your discord profile has been linked successfully" });
Expand Down
37 changes: 35 additions & 2 deletions controllers/users.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
const chaincodeQuery = require("../models/chaincodes");
const userQuery = require("../models/users");
const profileDiffsQuery = require("../models/profileDiffs");
const firestore = require("../utils/firestore");
const memberRoleModel = firestore.collection("member-group-roles");
const logsModel = firestore.collection("logs");
const admin = require("firebase-admin");
const logsQuery = require("../models/logs");
const imageService = require("../services/imageService");
const { profileDiffStatus } = require("../constants/profileDiff");
Expand Down Expand Up @@ -620,7 +624,7 @@ const markUnverified = async (req, res) => {
const unverifiedRoleId = config.get("discordUnverifiedRoleId");
const usersToApplyUnverifiedRole = [];
const addRolePromises = [];
const discordDeveloperRoleId = config.get("discordDeveloperRoleId");
const batchPromises = [];

allRdsLoggedInUsers.forEach((user) => {
rdsUserMap[user.discordId] = true;
Expand All @@ -636,11 +640,40 @@ const markUnverified = async (req, res) => {
}
});

const batchSize = 500;
const batches = Array.from({ length: Math.ceil(usersToApplyUnverifiedRole.length / batchSize) }, (_, index) =>
usersToApplyUnverifiedRole.slice(index * batchSize, index * batchSize + batchSize)
);

batches.forEach((batch) => {
const firestoreBatch = firestore.batch();

batch.forEach((id) => {
const memberRoleRef = memberRoleModel.doc(id);
const logRef = logsModel.doc();

firestoreBatch.set(memberRoleRef, {
roleid: unverifiedRoleId,
userid: id,
date: admin.firestore.Timestamp.fromDate(new Date()),
});

firestoreBatch.set(logRef, {
type: logType.ADD_UNVERIFIED_ROLE,
meta: { roleid: unverifiedRoleId, userid: id },
body: { message: "Unverified role added successfully" },
timestamp: admin.firestore.Timestamp.fromDate(new Date()),
});
});

batchPromises.push(firestoreBatch.commit());
});

usersToApplyUnverifiedRole.forEach((id) => {
addRolePromises.push(addRoleToUser(id, unverifiedRoleId));
});

await Promise.all(addRolePromises);
await Promise.all([...addRolePromises, ...batchPromises]);
return res.json({ message: "ROLES APPLIED SUCCESSFULLY" });
} catch (err) {
logger.error(err);
Expand Down
2 changes: 1 addition & 1 deletion models/discordactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ const updateGroupRole = async (roleData, docId) => {
*
* @param options { Object }: Data of the new role
* @param options.rolename String : name of the role
* @param options.roleId String : id of the role
* @param options.roleid String : id of the role
* @returns {Promise<discordRoleModel|Object>}
*/

Expand Down
76 changes: 69 additions & 7 deletions test/integration/external-accounts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ const externalAccountsModel = require("../../models/external-accounts");
const { usersFromRds, getDiscordMembers } = require("../fixtures/discordResponse/discord-response");
const Sinon = require("sinon");
const { INTERNAL_SERVER_ERROR } = require("../../constants/errorMessages");
const removeDiscordRoleUtils = require("../../utils/removeDiscordRoleFromUser");
const firestore = require("../../utils/firestore");
const userData = require("../fixtures/user/user")();
const userModel = firestore.collection("users");
const tasksModel = firestore.collection("tasks");
const discordServices = require("../../services/discordService");
const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../../constants/external-accounts");
chai.use(chaiHttp);
const cookieName = config.get("userToken.cookieName");
Expand Down Expand Up @@ -538,7 +538,10 @@ describe("External Accounts", function () {
expect(getUserResponseBeforeUpdate.body).to.not.have.property("discordId");
expect(getUserResponseBeforeUpdate.body).to.not.have.property("discordJoinedAt");

const removeRoleFromUserStub = Sinon.stub(discordServices, "removeRoleFromUser").resolves();
const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({
success: true,
message: "Role deleted successfully",
});

const response = await chai
.request(app)
Expand All @@ -557,26 +560,85 @@ describe("External Accounts", function () {
expect(updatedUserDetails.body).to.have.property("discordId");
expect(updatedUserDetails.body).to.have.property("discordJoinedAt");

removeRoleFromUserStub.restore();
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, "removeDiscordRoleFromUser").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 from discord failed", async function () {
await externalAccountsModel.addExternalAccountData(externalAccountData[2]);

const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({
success: false,
message: "Role deletion from discord 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();
});

it("Should return 500 when unverified role deletion failed", async function () {
it("Should return 500 when removeDiscordRole fails because role deletion from database failed", async function () {
await externalAccountsModel.addExternalAccountData(externalAccountData[2]);

const removeRoleFromUserStub = Sinon.stub(discordServices, "removeRoleFromUser").rejects();
const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({
success: false,
message: "Role deletion from database 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(`Role Deletion failed. Please contact admin.`);
expect(response.body.message).to.equal(
`User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin`
);

removeRoleFromUserStub.restore();
removeDiscordRoleStub.restore();
});
});
});
123 changes: 123 additions & 0 deletions test/unit/utils/remvoeDiscordRoleFromUser.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import chai from "chai";
import Sinon from "sinon";
import { logType } from "../../../constants/logs";
import discordActions from "../../../models/discordactions";
import { addLog } from "../../../models/logs";
import firestore from "../../../utils/firestore";
import { removeDiscordRoleFromUser } from "../../../utils/removeDiscordRoleFromUser";
import { groupData, memberGroupData } from "../../fixtures/discordactions/discordactions";
import addUser from "../../utils/addUser";
import cleanDb from "../../utils/cleanDb";
const { expect } = chai;
const discordRolesModel = firestore.collection("discord-roles");
const memberRoleModel = firestore.collection("member-group-roles");
const logsModel = firestore.collection("logs");
const userData = require("../../fixtures/user/user")();

describe("removeDiscordRoleFromUser", function () {
let userId;
let discordId;
let roleid;
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;
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 () {
await addLog(
logType.REMOVE_ROLE_FROM_USER_SUCCESS,
{ roleId: roleid, userid: discordId },
{ message: "Role removed successfully from user", userData: userData[0] }
);

fetchStub.returns(
Promise.resolve({ json: () => Promise.resolve({ success: true, message: "Role deleted successfully" }) })
);

const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid);
const successLog = await logsModel
.where("type", "==", logType.REMOVE_ROLE_FROM_USER_SUCCESS)
.where("meta.roleId", "==", roleid)
.where("meta.userid", "==", discordId)
.limit(1)
.get();

expect(isDiscordRoleRemoved.success).to.be.equal(true);
expect(isDiscordRoleRemoved.message).to.be.equal("Role deleted successfully");
expect(successLog.docs[0].data().body.message).to.be.equal("Role removed successfully from user");
});

it("should throw an error if role doesn't exist in database when attempting to remove", async function () {
roleid = "randomRoleId";

const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid);
const failedLog = await logsModel
.where("type", "==", logType.REMOVE_ROLE_FROM_USER_FAILED)
.where("meta.roleId", "==", roleid)
.limit(1)
.get();

expect(isDiscordRoleRemoved.success).to.be.equal(false);
expect(isDiscordRoleRemoved.message).to.be.equal("Role doesn't exist");
expect(failedLog.docs[0].data().body.message).to.be.equal("Role doesn't exist");
});

it("should throw an error if role deletion from discord failed", async function () {
fetchStub.rejects(new Error("Role deletion from discord failed"));

const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid);
const failedLog = await logsModel
.where("type", "==", logType.REMOVE_ROLE_FROM_USER_FAILED)
.where("meta.roleId", "==", roleid)
.where("meta.userid", "==", discordId)
.limit(1)
.get();

expect(isDiscordRoleRemoved.success).to.be.equal(false);
expect(isDiscordRoleRemoved.message).to.be.equal("Role deletion from discord failed");
expect(failedLog.docs[0].data().body.message).to.be.equal("Role deletion from discord failed");
});

it("should throw an error if role deleted from discord but not from database", async function () {
fetchStub.returns(Promise.resolve({ json: () => Promise.resolve({ success: true }) }));

const removeMemberGroupStub = Sinon.stub(discordActions, "removeMemberGroup").resolves({
roleId: roleid,
wasSuccess: false,
});

const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid);
const failedLog = await logsModel
.where("type", "==", logType.REMOVE_ROLE_FROM_USER_FAILED)
.where("meta.roleId", "==", roleid)
.where("meta.userid", "==", discordId)
.limit(1)
.get();

expect(isDiscordRoleRemoved.success).to.be.equal(false);
expect(isDiscordRoleRemoved.message).to.be.equal("Role deletion from database failed");
expect(failedLog.docs[0].data().body.message).to.be.equal("Role deletion from database failed");

removeMemberGroupStub.restore();
});
});
Loading

0 comments on commit 74f8324

Please sign in to comment.