Skip to content

Commit 2f1339d

Browse files
pankajjsyesyash
authored andcommitted
Fix: skip users having approved onboarding extension when applying groupOnboarding31d+ role (#2330)
* feat: added tests to handle edges cases * fix: added fix to skip onboarding users having valid approved extension request * fix: added integration test to handle edge case * feat: optimize user filtering by skipping those with valid onboarding extensions - Replaced sequential user processing with Promise.all to improve performance. - Enhanced error logging to include user IDs for better traceability. - Introduced a single timestamp variable to ensure consistent time comparisons.
1 parent fd8d4c5 commit 2f1339d

File tree

3 files changed

+155
-2
lines changed

3 files changed

+155
-2
lines changed

models/discordactions.js

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ const { FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users");
2828
const discordService = require("../services/discordService");
2929
const { buildTasksQueryForMissedUpdates } = require("../utils/tasks");
3030
const { buildProgressQueryForMissedUpdates } = require("../utils/progresses");
31+
const { getRequestByKeyValues } = require("./requests");
32+
const { REQUEST_TYPE, REQUEST_STATE } = require("../constants/requests");
3133
const allMavens = [];
3234

3335
/**
@@ -753,12 +755,59 @@ const updateIdle7dUsersOnDiscord = async (dev) => {
753755
};
754756
};
755757

758+
/**
759+
* Filters out onboarding users who have an approved onboarding extension request that is still valid.
760+
*
761+
* This function iterates through the given list of onboarding users and checks if each user has a valid
762+
* approved onboarding extension request. If a valid extension request exists with a `newEndsOn`
763+
* date greater than the current date, the user is skipped. Otherwise, the user is added to the
764+
* returned list.
765+
*
766+
* @async
767+
* @function skipOnboardingUsersHavingApprovedExtensionRequest
768+
* @param {Array<Object>} [users=[]] - An array of user objects to be filtered. Each user object
769+
* must have an `id` property.
770+
* @returns {Promise<Array<Object>>} A promise that resolves to an array of users who do not have
771+
* a valid approved onboarding extension request.
772+
*/
773+
const skipOnboardingUsersHavingApprovedExtensionRequest = async (users = []) => {
774+
const currentTime = Date.now();
775+
776+
const results = await Promise.all(
777+
users.map(async (user) => {
778+
try {
779+
const latestApprovedExtension = await getRequestByKeyValues({
780+
type: REQUEST_TYPE.ONBOARDING,
781+
state: REQUEST_STATE.APPROVED,
782+
userId: user.id,
783+
});
784+
785+
if (latestApprovedExtension && latestApprovedExtension.newEndsOn > currentTime) {
786+
return null;
787+
}
788+
789+
return user;
790+
} catch (error) {
791+
logger.error(`Error while fetching latest approved extension for user ${user.id}:`, error);
792+
return null;
793+
}
794+
})
795+
);
796+
797+
return results.filter(Boolean);
798+
};
799+
756800
const updateUsersWith31DaysPlusOnboarding = async () => {
757801
try {
758-
const allOnboardingUsers31DaysCompleted = await getUsersBasedOnFilter({
802+
let allOnboardingUsers31DaysCompleted = await getUsersBasedOnFilter({
759803
state: userState.ONBOARDING,
760804
time: "31d",
761805
});
806+
807+
allOnboardingUsers31DaysCompleted = await skipOnboardingUsersHavingApprovedExtensionRequest(
808+
allOnboardingUsers31DaysCompleted
809+
);
810+
762811
const discordMembers = await getDiscordMembers();
763812
const groupOnboardingRole = await getGroupRole("group-onboarding-31d+");
764813
const groupOnboardingRoleId = groupOnboardingRole.role.roleid;
@@ -1131,4 +1180,5 @@ module.exports = {
11311180
addInviteToInviteModel,
11321181
groupUpdateLastJoinDate,
11331182
deleteGroupRole,
1183+
skipOnboardingUsersHavingApprovedExtensionRequest,
11341184
};

test/integration/discordactions.test.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ chai.use(chaiHttp);
4848
const { userStatusDataForOooState } = require("../fixtures/userStatus/userStatus");
4949
const { generateCronJobToken } = require("../utils/generateBotToken");
5050
const { CRON_JOB_HANDLER } = require("../../constants/bot");
51+
const { createRequest } = require("../../models/requests");
52+
const { REQUEST_TYPE, REQUEST_STATE } = require("../../constants/requests");
53+
const { convertDaysToMilliseconds } = require("../../utils/time");
5154

5255
describe("Discord actions", function () {
5356
let superUserId;
@@ -853,6 +856,8 @@ describe("Discord actions", function () {
853856
});
854857

855858
describe("PUT /discord-actions/group-onboarding-31d-plus", function () {
859+
let userId;
860+
856861
beforeEach(async function () {
857862
userData[0] = {
858863
...userData[0],
@@ -884,6 +889,7 @@ describe("Discord actions", function () {
884889
const addUsersPromises = allUsers.map((user) => addUser(user));
885890
const userIds = await Promise.all(addUsersPromises);
886891

892+
userId = userIds[0];
887893
const updateUserStatusPromises = userIds.map((userId, index) => {
888894
if (index === 3) return updateUserStatus(userId, generateUserStatusData("IDLE", new Date(), new Date()));
889895
return updateUserStatus(userId, generateUserStatusData("ONBOARDING", new Date(), new Date()));
@@ -905,6 +911,28 @@ describe("Discord actions", function () {
905911
await cleanDb();
906912
});
907913

914+
it("should filter users who have approved extension request and update groupOnboarding31d+ role", function (done) {
915+
createRequest({
916+
type: REQUEST_TYPE.ONBOARDING,
917+
state: REQUEST_STATE.APPROVED,
918+
userId: userId,
919+
newEndsOn: Date.now() + convertDaysToMilliseconds(2),
920+
});
921+
chai
922+
.request(app)
923+
.put(`/discord-actions/group-onboarding-31d-plus`)
924+
.set("Cookie", `${cookieName}=${superUserAuthToken}`)
925+
.end((err, res) => {
926+
if (err) return done(err);
927+
expect(res).to.have.status(201);
928+
expect(res.body.message).to.be.equal("All Users with 31 Days Plus Onboarding are updated successfully.");
929+
expect(res.body.totalOnboardingUsers31DaysCompleted.count).to.be.equal(2);
930+
expect(res.body.totalOnboarding31dPlusRoleApplied.count).to.be.equal(2);
931+
expect(res.body.totalOnboarding31dPlusRoleRemoved.count).to.be.equal(1);
932+
return done();
933+
});
934+
});
935+
908936
it("should update role for onboarding users with 31 days completed", function (done) {
909937
chai
910938
.request(app)

test/unit/models/discordactions.test.js

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const {
4040
removeMemberGroup,
4141
getGroupRoleByName,
4242
getGroupRolesForUser,
43+
skipOnboardingUsersHavingApprovedExtensionRequest,
4344
} = require("../../../models/discordactions");
4445
const {
4546
groupData,
@@ -63,6 +64,8 @@ const { stubbedModelTaskProgressData } = require("../../fixtures/progress/progre
6364
const { convertDaysToMilliseconds } = require("../../../utils/time");
6465
const { generateUserStatusData } = require("../../fixtures/userStatus/userStatus");
6566
const { userState } = require("../../../constants/userStatus");
67+
const { REQUEST_TYPE, REQUEST_STATE } = require("../../../constants/requests");
68+
const { createRequest } = require("../../../models/requests");
6669

6770
chai.should();
6871

@@ -1064,6 +1067,7 @@ describe("discordactions", function () {
10641067

10651068
describe("updateUsersWith31DaysPlusOnboarding", function () {
10661069
let fetchStub;
1070+
let userId0;
10671071

10681072
beforeEach(async function () {
10691073
fetchStub = sinon.stub(global, "fetch");
@@ -1087,7 +1091,7 @@ describe("discordactions", function () {
10871091
roles: { archived: false, in_discord: true },
10881092
};
10891093

1090-
const userId0 = await addUser(userData[0]);
1094+
userId0 = await addUser(userData[0]);
10911095
const userId1 = await addUser(userData[1]);
10921096
const userId2 = await addUser(userData[2]);
10931097

@@ -1133,6 +1137,48 @@ describe("discordactions", function () {
11331137
await cleanDb();
11341138
});
11351139

1140+
it("should add grouponboarding31D when user has an approved extension request but dealine has been passed", async function () {
1141+
await createRequest({
1142+
type: REQUEST_TYPE.ONBOARDING,
1143+
state: REQUEST_STATE.APPROVED,
1144+
newEndsOn: Date.now() - convertDaysToMilliseconds(2),
1145+
userId: userId0,
1146+
});
1147+
1148+
const res = await updateUsersWith31DaysPlusOnboarding();
1149+
expect(res.totalOnboardingUsers31DaysCompleted.count).to.equal(2);
1150+
expect(res.totalOnboarding31dPlusRoleApplied.count).to.equal(1);
1151+
expect(res.totalOnboarding31dPlusRoleRemoved.count).to.equal(1);
1152+
});
1153+
1154+
it("should add grouponboarding31D when user does not have approved extension request", async function () {
1155+
await createRequest({
1156+
type: REQUEST_TYPE.ONBOARDING,
1157+
state: REQUEST_STATE.PENDING,
1158+
newEndsOn: Date.now() + convertDaysToMilliseconds(2),
1159+
userId: userId0,
1160+
});
1161+
1162+
const res = await updateUsersWith31DaysPlusOnboarding();
1163+
expect(res.totalOnboardingUsers31DaysCompleted.count).to.equal(2);
1164+
expect(res.totalOnboarding31dPlusRoleApplied.count).to.equal(1);
1165+
expect(res.totalOnboarding31dPlusRoleRemoved.count).to.equal(1);
1166+
});
1167+
1168+
it("should not add grouponboarding31D when user has approved extension request", async function () {
1169+
await createRequest({
1170+
type: REQUEST_TYPE.ONBOARDING,
1171+
state: REQUEST_STATE.APPROVED,
1172+
newEndsOn: Date.now() + convertDaysToMilliseconds(2),
1173+
userId: userId0,
1174+
});
1175+
1176+
const res = await updateUsersWith31DaysPlusOnboarding();
1177+
expect(res.totalOnboardingUsers31DaysCompleted.count).to.equal(1);
1178+
expect(res.totalOnboarding31dPlusRoleApplied.count).to.equal(1);
1179+
expect(res.totalOnboarding31dPlusRoleRemoved.count).to.equal(1);
1180+
});
1181+
11361182
it("apply, or remove grouponboarding31D", async function () {
11371183
const res = await updateUsersWith31DaysPlusOnboarding();
11381184

@@ -1343,4 +1389,33 @@ describe("discordactions", function () {
13431389
}
13441390
});
13451391
});
1392+
1393+
describe("skipOnboardingUsersHavingApprovedExtensionRequest", function () {
1394+
const userId0 = "11111";
1395+
const userId1 = "12345";
1396+
1397+
afterEach(async function () {
1398+
sinon.restore();
1399+
await cleanDb();
1400+
});
1401+
1402+
it("should return filtered users", async function () {
1403+
await createRequest({
1404+
state: REQUEST_STATE.APPROVED,
1405+
type: REQUEST_TYPE.ONBOARDING,
1406+
newEndsOn: Date.now() + convertDaysToMilliseconds(2),
1407+
userId: userId0,
1408+
});
1409+
const users = await skipOnboardingUsersHavingApprovedExtensionRequest([{ id: userId0 }, { id: userId1 }]);
1410+
expect(users.length).to.equal(1);
1411+
expect(users[0].id).to.equal(userId1);
1412+
});
1413+
1414+
it("should not return filtered users", async function () {
1415+
const users = await skipOnboardingUsersHavingApprovedExtensionRequest([{ id: userId0 }, { id: userId1 }]);
1416+
expect(users.length).to.equal(2);
1417+
expect(users[0].id).to.equal(userId0);
1418+
expect(users[1].id).to.equal(userId1);
1419+
});
1420+
});
13461421
});

0 commit comments

Comments
 (0)