From bc76e580d4924902397684ae6544ca6e1625ed76 Mon Sep 17 00:00:00 2001 From: Pankaj Sha Date: Thu, 2 Jan 2025 02:27:53 +0530 Subject: [PATCH 1/4] feat: added tests to handle edges cases --- test/unit/models/discordactions.test.js | 77 ++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index 00d92793f..1a21c6f61 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -40,6 +40,7 @@ const { removeMemberGroup, getGroupRoleByName, getGroupRolesForUser, + skipOnboardingUsersHavingApprovedExtensionRequest, } = require("../../../models/discordactions"); const { groupData, @@ -63,6 +64,8 @@ const { stubbedModelTaskProgressData } = require("../../fixtures/progress/progre const { convertDaysToMilliseconds } = require("../../../utils/time"); const { generateUserStatusData } = require("../../fixtures/userStatus/userStatus"); const { userState } = require("../../../constants/userStatus"); +const { REQUEST_TYPE, REQUEST_STATE } = require("../../../constants/requests"); +const { createRequest } = require("../../../models/requests"); chai.should(); @@ -1064,6 +1067,7 @@ describe("discordactions", function () { describe("updateUsersWith31DaysPlusOnboarding", function () { let fetchStub; + let userId0; beforeEach(async function () { fetchStub = sinon.stub(global, "fetch"); @@ -1087,7 +1091,7 @@ describe("discordactions", function () { roles: { archived: false, in_discord: true }, }; - const userId0 = await addUser(userData[0]); + userId0 = await addUser(userData[0]); const userId1 = await addUser(userData[1]); const userId2 = await addUser(userData[2]); @@ -1133,6 +1137,48 @@ describe("discordactions", function () { await cleanDb(); }); + it("should add grouponboarding31D when user has an approved extension request but dealine has been passed", async function () { + await createRequest({ + type: REQUEST_TYPE.ONBOARDING, + state: REQUEST_STATE.APPROVED, + newEndsOn: Date.now() - convertDaysToMilliseconds(2), + userId: userId0, + }); + + const res = await updateUsersWith31DaysPlusOnboarding(); + expect(res.totalOnboardingUsers31DaysCompleted.count).to.equal(2); + expect(res.totalOnboarding31dPlusRoleApplied.count).to.equal(1); + expect(res.totalOnboarding31dPlusRoleRemoved.count).to.equal(1); + }); + + it("should add grouponboarding31D when user does not have approved extension request", async function () { + await createRequest({ + type: REQUEST_TYPE.ONBOARDING, + state: REQUEST_STATE.PENDING, + newEndsOn: Date.now() + convertDaysToMilliseconds(2), + userId: userId0, + }); + + const res = await updateUsersWith31DaysPlusOnboarding(); + expect(res.totalOnboardingUsers31DaysCompleted.count).to.equal(2); + expect(res.totalOnboarding31dPlusRoleApplied.count).to.equal(1); + expect(res.totalOnboarding31dPlusRoleRemoved.count).to.equal(1); + }); + + it("should not add grouponboarding31D when user has approved extension request", async function () { + await createRequest({ + type: REQUEST_TYPE.ONBOARDING, + state: REQUEST_STATE.APPROVED, + newEndsOn: Date.now() + convertDaysToMilliseconds(2), + userId: userId0, + }); + + const res = await updateUsersWith31DaysPlusOnboarding(); + expect(res.totalOnboardingUsers31DaysCompleted.count).to.equal(1); + expect(res.totalOnboarding31dPlusRoleApplied.count).to.equal(1); + expect(res.totalOnboarding31dPlusRoleRemoved.count).to.equal(1); + }); + it("apply, or remove grouponboarding31D", async function () { const res = await updateUsersWith31DaysPlusOnboarding(); @@ -1343,4 +1389,33 @@ describe("discordactions", function () { } }); }); + + describe("skipOnboardingUsersHavingApprovedExtensionRequest", function () { + const userId0 = "11111"; + const userId1 = "12345"; + + afterEach(async function () { + sinon.restore(); + await cleanDb(); + }); + + it("should return filtered users", async function () { + await createRequest({ + state: REQUEST_STATE.APPROVED, + type: REQUEST_TYPE.ONBOARDING, + newEndsOn: Date.now() + convertDaysToMilliseconds(2), + userId: userId0, + }); + const users = await skipOnboardingUsersHavingApprovedExtensionRequest([{ id: userId0 }, { id: userId1 }]); + expect(users.length).to.equal(1); + expect(users[0].id).to.equal(userId1); + }); + + it("should not return filtered users", async function () { + const users = await skipOnboardingUsersHavingApprovedExtensionRequest([{ id: userId0 }, { id: userId1 }]); + expect(users.length).to.equal(2); + expect(users[0].id).to.equal(userId0); + expect(users[1].id).to.equal(userId1); + }); + }); }); From c4ef146abe1949bab1f88c09348e0aec3026c1ed Mon Sep 17 00:00:00 2001 From: Pankaj Sha Date: Thu, 2 Jan 2025 02:30:20 +0530 Subject: [PATCH 2/4] fix: added fix to skip onboarding users having valid approved extension request --- models/discordactions.js | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/models/discordactions.js b/models/discordactions.js index 365329068..a572ce00e 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -28,6 +28,8 @@ const { FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users"); const discordService = require("../services/discordService"); const { buildTasksQueryForMissedUpdates } = require("../utils/tasks"); const { buildProgressQueryForMissedUpdates } = require("../utils/progresses"); +const { getRequestByKeyValues } = require("./requests"); +const { REQUEST_TYPE, REQUEST_STATE } = require("../constants/requests"); const allMavens = []; /** @@ -753,12 +755,39 @@ const updateIdle7dUsersOnDiscord = async (dev) => { }; }; +const skipOnboardingUsersHavingApprovedExtensionRequest = async (users = []) => { + const filteredUsers = []; + for (const user of users) { + try { + const latestApprovedExtension = await getRequestByKeyValues({ + type: REQUEST_TYPE.ONBOARDING, + state: REQUEST_STATE.APPROVED, + userId: user.id, + }); + + if (latestApprovedExtension && latestApprovedExtension.newEndsOn > Date.now()) { + continue; + } + filteredUsers.push(user); + } catch (error) { + logger.error("Error while fetching latest approved extension: ", error); + } + } + + return filteredUsers; +}; + const updateUsersWith31DaysPlusOnboarding = async () => { try { - const allOnboardingUsers31DaysCompleted = await getUsersBasedOnFilter({ + let allOnboardingUsers31DaysCompleted = await getUsersBasedOnFilter({ state: userState.ONBOARDING, time: "31d", }); + + allOnboardingUsers31DaysCompleted = await skipOnboardingUsersHavingApprovedExtensionRequest( + allOnboardingUsers31DaysCompleted + ); + const discordMembers = await getDiscordMembers(); const groupOnboardingRole = await getGroupRole("group-onboarding-31d+"); const groupOnboardingRoleId = groupOnboardingRole.role.roleid; @@ -1131,4 +1160,5 @@ module.exports = { addInviteToInviteModel, groupUpdateLastJoinDate, deleteGroupRole, + skipOnboardingUsersHavingApprovedExtensionRequest, }; From 6040a74e4a9bdf3694ca07e63c5b25482018247a Mon Sep 17 00:00:00 2001 From: Pankaj Sha Date: Thu, 2 Jan 2025 12:29:20 +0530 Subject: [PATCH 3/4] fix: added integration test to handle edge case --- test/integration/discordactions.test.js | 28 +++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/integration/discordactions.test.js b/test/integration/discordactions.test.js index d709a7ec5..d5604897f 100644 --- a/test/integration/discordactions.test.js +++ b/test/integration/discordactions.test.js @@ -48,6 +48,9 @@ chai.use(chaiHttp); const { userStatusDataForOooState } = require("../fixtures/userStatus/userStatus"); const { generateCronJobToken } = require("../utils/generateBotToken"); const { CRON_JOB_HANDLER } = require("../../constants/bot"); +const { createRequest } = require("../../models/requests"); +const { REQUEST_TYPE, REQUEST_STATE } = require("../../constants/requests"); +const { convertDaysToMilliseconds } = require("../../utils/time"); describe("Discord actions", function () { let superUserId; @@ -853,6 +856,8 @@ describe("Discord actions", function () { }); describe("PUT /discord-actions/group-onboarding-31d-plus", function () { + let userId; + beforeEach(async function () { userData[0] = { ...userData[0], @@ -884,6 +889,7 @@ describe("Discord actions", function () { const addUsersPromises = allUsers.map((user) => addUser(user)); const userIds = await Promise.all(addUsersPromises); + userId = userIds[0]; const updateUserStatusPromises = userIds.map((userId, index) => { if (index === 3) return updateUserStatus(userId, generateUserStatusData("IDLE", new Date(), new Date())); return updateUserStatus(userId, generateUserStatusData("ONBOARDING", new Date(), new Date())); @@ -905,6 +911,28 @@ describe("Discord actions", function () { await cleanDb(); }); + it("should filter users who have approved extension request and update groupOnboarding31d+ role", function (done) { + createRequest({ + type: REQUEST_TYPE.ONBOARDING, + state: REQUEST_STATE.APPROVED, + userId: userId, + newEndsOn: Date.now() + convertDaysToMilliseconds(2), + }); + chai + .request(app) + .put(`/discord-actions/group-onboarding-31d-plus`) + .set("Cookie", `${cookieName}=${superUserAuthToken}`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(201); + expect(res.body.message).to.be.equal("All Users with 31 Days Plus Onboarding are updated successfully."); + expect(res.body.totalOnboardingUsers31DaysCompleted.count).to.be.equal(2); + expect(res.body.totalOnboarding31dPlusRoleApplied.count).to.be.equal(2); + expect(res.body.totalOnboarding31dPlusRoleRemoved.count).to.be.equal(1); + return done(); + }); + }); + it("should update role for onboarding users with 31 days completed", function (done) { chai .request(app) From 4b48bf59c0bb762c3ce1cd3c2298ba1334de21bd Mon Sep 17 00:00:00 2001 From: Pankaj Sha Date: Mon, 6 Jan 2025 22:00:50 +0530 Subject: [PATCH 4/4] 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. --- models/discordactions.js | 52 +++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/models/discordactions.js b/models/discordactions.js index a572ce00e..3ba9ee658 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -755,26 +755,46 @@ const updateIdle7dUsersOnDiscord = async (dev) => { }; }; +/** + * Filters out onboarding users who have an approved onboarding extension request that is still valid. + * + * This function iterates through the given list of onboarding users and checks if each user has a valid + * approved onboarding extension request. If a valid extension request exists with a `newEndsOn` + * date greater than the current date, the user is skipped. Otherwise, the user is added to the + * returned list. + * + * @async + * @function skipOnboardingUsersHavingApprovedExtensionRequest + * @param {Array} [users=[]] - An array of user objects to be filtered. Each user object + * must have an `id` property. + * @returns {Promise>} A promise that resolves to an array of users who do not have + * a valid approved onboarding extension request. + */ const skipOnboardingUsersHavingApprovedExtensionRequest = async (users = []) => { - const filteredUsers = []; - for (const user of users) { - try { - const latestApprovedExtension = await getRequestByKeyValues({ - type: REQUEST_TYPE.ONBOARDING, - state: REQUEST_STATE.APPROVED, - userId: user.id, - }); + const currentTime = Date.now(); + + const results = await Promise.all( + users.map(async (user) => { + try { + const latestApprovedExtension = await getRequestByKeyValues({ + type: REQUEST_TYPE.ONBOARDING, + state: REQUEST_STATE.APPROVED, + userId: user.id, + }); - if (latestApprovedExtension && latestApprovedExtension.newEndsOn > Date.now()) { - continue; + if (latestApprovedExtension && latestApprovedExtension.newEndsOn > currentTime) { + return null; + } + + return user; + } catch (error) { + logger.error(`Error while fetching latest approved extension for user ${user.id}:`, error); + return null; } - filteredUsers.push(user); - } catch (error) { - logger.error("Error while fetching latest approved extension: ", error); - } - } + }) + ); - return filteredUsers; + return results.filter(Boolean); }; const updateUsersWith31DaysPlusOnboarding = async () => {