From 6e8f25c3bbca4ab5f18af8fe9fbe6d2ea7a2e3d6 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Fri, 15 Nov 2024 15:26:00 +0530 Subject: [PATCH 1/9] Created users/departed-users API - users/departed-users retrieves users with assigned tasks who have left the Discord server Added test cases for the API and fixed test case issues arising from mock data additions used in current API testing. --- controllers/users.js | 11 +++++ models/users.js | 34 +++++++++++++ routes/users.js | 2 + test/fixtures/tasks/tasks.js | 84 ++++++++++++++++++++++++++++++++ test/fixtures/user/user.js | 63 ++++++++++++++++++++++++ test/integration/users.test.js | 55 ++++++++++++++++++++- test/unit/models/tasks.test.js | 8 +-- test/unit/services/tasks.test.js | 4 +- test/unit/services/users.test.js | 4 +- 9 files changed, 256 insertions(+), 9 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 1fa81c282..ba7c608a8 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -1029,6 +1029,16 @@ const updateUsernames = async (req, res) => { } }; +const getUsersWithAbandonedTasks = async (req, res) => { + try { + const data = await userQuery.fetchUsersWithAbandonedTasks(); + return res.status(200).json({ message: "Users with abandoned tasks fetched successfully", data }); + } catch (error) { + logger.error("Error in getting user who abandoned tasks:", error); + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } +}; + module.exports = { verifyUser, generateChaincode, @@ -1061,4 +1071,5 @@ module.exports = { isDeveloper, getIdentityStats, updateUsernames, + getUsersWithAbandonedTasks, }; diff --git a/models/users.js b/models/users.js index e0746de92..e7f5a00ab 100644 --- a/models/users.js +++ b/models/users.js @@ -27,6 +27,9 @@ const { formatUsername } = require("../utils/username"); const { logType } = require("../constants/logs"); const { addLog } = require("../services/logService"); +const tasksModel = firestore.collection("tasks"); +const { TASK_STATUS } = require("../constants/tasks"); +const { COMPLETED, DONE } = TASK_STATUS; /** * Adds or updates the user data * @@ -1030,6 +1033,36 @@ const updateUsersWithNewUsernames = async () => { } }; +const fetchUsersWithAbandonedTasks = async () => { + try { + const COMPLETED_STATUSES = [DONE, COMPLETED]; + const eligibleUsersWithTasks = []; + + const userSnapshot = await userModel + .where("roles.archived", "==", true) + .where("roles.in_discord", "==", false) + .get(); + + for (const userDoc of userSnapshot.docs) { + const user = userDoc.data(); + + // Check if the user has any tasks with status not in [Done, Complete] + const abandonedTasksQuerySnapshot = await tasksModel + .where("assigneeId", "==", user.id) + .where("status", "not-in", COMPLETED_STATUSES) + .get(); + + if (!abandonedTasksQuerySnapshot.empty) { + eligibleUsersWithTasks.push(user); + } + } + return eligibleUsersWithTasks; + } catch (error) { + logger.error(`Error in getting users who abandoned tasks: ${error}`); + throw error; + } +}; + module.exports = { addOrUpdate, fetchPaginatedUsers, @@ -1059,4 +1092,5 @@ module.exports = { fetchUserForKeyValue, getNonNickNameSyncedUsers, updateUsersWithNewUsernames, + fetchUsersWithAbandonedTasks, }; diff --git a/routes/users.js b/routes/users.js index 94d301cda..e8d19573d 100644 --- a/routes/users.js +++ b/routes/users.js @@ -13,6 +13,7 @@ const ROLES = require("../constants/roles"); const { Services } = require("../constants/bot"); const authenticateProfile = require("../middlewares/authenticateProfile"); +router.get("/departed-users", users.getUsersWithAbandonedTasks); router.post("/", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), users.markUnverified); router.post("/update-in-discord", authenticate, authorizeRoles([SUPERUSER]), users.setInDiscordScript); router.post("/verify", authenticate, users.verifyUser); @@ -67,6 +68,7 @@ router.patch("/profileURL", authenticate, userValidator.updateProfileURL, users. router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rejectProfileDiff); router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); router.get("/suggestedUsers/:skillId", authenticate, authorizeRoles([SUPERUSER]), users.getSuggestedUsers); + module.exports = router; router.post("/batch-username-update", authenticate, authorizeRoles([SUPERUSER]), users.updateUsernames); module.exports = router; diff --git a/test/fixtures/tasks/tasks.js b/test/fixtures/tasks/tasks.js index 1d5efa978..1768c6cd6 100644 --- a/test/fixtures/tasks/tasks.js +++ b/test/fixtures/tasks/tasks.js @@ -140,5 +140,89 @@ module.exports = () => { createdAt: 1644753600, updatedAt: 1644753600, }, + { + id: "task1_id", + title: "Abandoned Task 1", + type: "feature", + status: "IN_PROGRESS", + priority: "HIGH", + percentCompleted: 50, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "archived_user1", + assigneeId: "user1_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/1", + url: "https://api.github.com/repos/org/repo/issues/1", + }, + }, + dependsOn: [], + }, + { + id: "task2_id", + title: "Abandoned Task 2", + type: "bug", + status: "BLOCKED", + priority: "MEDIUM", + percentCompleted: 30, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "archived_user2", + assigneeId: "user2_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/2", + url: "https://api.github.com/repos/org/repo/issues/2", + }, + }, + dependsOn: [], + }, + { + id: "task3_id", + title: "Completed Archived Task", + type: "feature", + status: "DONE", + priority: "LOW", + percentCompleted: 100, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "archived_user1", + assigneeId: "user1_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/3", + url: "https://api.github.com/repos/org/repo/issues/3", + }, + }, + dependsOn: [], + }, + { + id: "task4_id", + title: "Active User Task", + type: "feature", + status: "IN_PROGRESS", + priority: "HIGH", + percentCompleted: 75, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "active_user", + assigneeId: "user3_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/4", + url: "https://api.github.com/repos/org/repo/issues/4", + }, + }, + dependsOn: [], + }, ]; }; diff --git a/test/fixtures/user/user.js b/test/fixtures/user/user.js index 485255efc..674718d61 100644 --- a/test/fixtures/user/user.js +++ b/test/fixtures/user/user.js @@ -442,5 +442,68 @@ module.exports = () => { url: "https://res.cloudinary.com/realdevsquad/image/upload/v1667685133/profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar.jpg", }, }, + { + id: "user1_id", + discordId: "123456789", + github_id: "github_user1", + username: "archived_user1", + first_name: "Archived", + last_name: "User One", + linkedin_id: "archived_user1", + github_display_name: "archived-user-1", + phone: "1234567890", + email: "archived1@test.com", + roles: { + archived: true, + in_discord: false, + }, + discordJoinedAt: "2024-01-01T00:00:00.000Z", + picture: { + publicId: "profile/user1", + url: "https://example.com/user1.jpg", + }, + }, + { + id: "user2_id", + discordId: "987654321", + github_id: "github_user2", + username: "archived_user2", + first_name: "Archived", + last_name: "User Two", + linkedin_id: "archived_user2", + github_display_name: "archived-user-2", + phone: "0987654321", + email: "archived2@test.com", + roles: { + archived: true, + in_discord: false, + }, + discordJoinedAt: "2024-01-02T00:00:00.000Z", + picture: { + publicId: "profile/user2", + url: "https://example.com/user2.jpg", + }, + }, + { + id: "user3_id", + discordId: "555555555", + github_id: "github_user3", + username: "active_user", + first_name: "Active", + last_name: "User", + linkedin_id: "active_user", + github_display_name: "active-user", + phone: "5555555555", + email: "active@test.com", + roles: { + archived: false, + in_discord: true, + }, + discordJoinedAt: "2024-01-03T00:00:00.000Z", + picture: { + publicId: "profile/user3", + url: "https://example.com/user3.jpg", + }, + }, ]; }; diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 708b87d2f..cda75c3d7 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -23,9 +23,12 @@ const { userStatusDataAfterSignup, userStatusDataAfterFillingJoinSection, } = require("../fixtures/userStatus/userStatus"); -const { addJoinData, addOrUpdate } = require("../../models/users"); +const { addJoinData, addOrUpdate, fetchUsersWithAbandonedTasks } = require("../../models/users"); const userStatusModel = require("../../models/userStatus"); const { MAX_USERNAME_LENGTH } = require("../../constants/users.ts"); +const tasksData = require("../fixtures/tasks/tasks")(); +const tasksModel = firestore.collection("tasks"); +const userDBModel = firestore.collection("users"); const userRoleUpdate = userData[4]; const userRoleUnArchived = userData[13]; @@ -2646,4 +2649,54 @@ describe("Users", function () { expect(res).to.have.status(401); }); }); + + describe("fetchUsersWithAbandonedTasks", function () { + beforeEach(async function () { + // Clean the database + await cleanDb(); + + // Add test users to the database + const userPromises = userData.map((user) => userDBModel.add(user)); + await Promise.all(userPromises); + + // Add test tasks to the database + const taskPromises = tasksData.map((task) => tasksModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + await cleanDb(); + }); + + it("should fetch users with abandoned tasks", async function () { + const usersWithAbandonedTasks = await fetchUsersWithAbandonedTasks(); + + expect(usersWithAbandonedTasks).to.be.an("array"); + expect(usersWithAbandonedTasks).to.have.lengthOf(2); // Two users with abandoned tasks + }); + + it("should not include user who are present in discord or not archived", async function () { + const usersWithAbandonedTasks = await fetchUsersWithAbandonedTasks(); + + usersWithAbandonedTasks.forEach((user) => { + expect(user.roles.in_discord).to.not.equal(true); + expect(user.roles.archived).to.not.equal(false); + }); + }); + + it("should return an empty array if there are no users with abandoned tasks", async function () { + await cleanDb(); + + // Add only active users + const activeUser = userData[11]; // Using the active user from our test data + await userDBModel.add(activeUser); + + // Add a task assigned to the active user + const activeTask = tasksData[11]; // Using the active user's task + await tasksModel.add(activeTask); + const usersWithAbandonedTasks = await fetchUsersWithAbandonedTasks(); + expect(usersWithAbandonedTasks).to.be.an("array"); + expect(usersWithAbandonedTasks).to.have.lengthOf(0); + }); + }); }); diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index 44a560fd5..db9ccd15e 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -303,12 +303,12 @@ describe("tasks", function () { overdueTask.endsOn = Date.now() / 1000 + 24 * 60 * 60 * 7; await tasks.updateTask(overdueTask); const usersWithOverdueTasks = await tasks.getOverdueTasks(days); - expect(usersWithOverdueTasks.length).to.be.equal(5); + expect(usersWithOverdueTasks.length).to.be.equal(8); }); it("should return all users which have overdue tasks if days is not passed", async function () { const usersWithOverdueTasks = await tasks.getOverdueTasks(); - expect(usersWithOverdueTasks.length).to.be.equal(4); + expect(usersWithOverdueTasks.length).to.be.equal(7); }); }); @@ -332,8 +332,8 @@ describe("tasks", function () { it("Should update task status COMPLETED to DONE", async function () { const res = await tasks.updateTaskStatus(); - expect(res.totalTasks).to.be.equal(8); - expect(res.totalUpdatedStatus).to.be.equal(8); + expect(res.totalTasks).to.be.equal(12); + expect(res.totalUpdatedStatus).to.be.equal(12); }); it("should throw an error if firebase batch operation fails", async function () { diff --git a/test/unit/services/tasks.test.js b/test/unit/services/tasks.test.js index 02d424134..8e675958c 100644 --- a/test/unit/services/tasks.test.js +++ b/test/unit/services/tasks.test.js @@ -46,7 +46,7 @@ describe("Tasks services", function () { const res = await updateTaskStatusToDone(tasks); expect(res).to.deep.equal({ - totalUpdatedStatus: 8, + totalUpdatedStatus: 12, totalOperationsFailed: 0, updatedTaskDetails: taskDetails, failedTaskDetails: [], @@ -66,7 +66,7 @@ describe("Tasks services", function () { expect(res).to.deep.equal({ totalUpdatedStatus: 0, - totalOperationsFailed: 8, + totalOperationsFailed: 12, updatedTaskDetails: [], failedTaskDetails: taskDetails, }); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index 60cf04c9d..611b1e64c 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -55,7 +55,7 @@ describe("Users services", function () { expect(res).to.deep.equal({ message: "Successfully completed batch updates", - totalUsersArchived: 20, + totalUsersArchived: 23, totalOperationsFailed: 0, updatedUserDetails: userDetails, failedUserDetails: [], @@ -76,7 +76,7 @@ describe("Users services", function () { expect(res).to.deep.equal({ message: "Firebase batch operation failed", totalUsersArchived: 0, - totalOperationsFailed: 20, + totalOperationsFailed: 23, updatedUserDetails: [], failedUserDetails: userDetails, }); From e79685eab0425e63bf2fb6c221f9c0d16f13510d Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Fri, 15 Nov 2024 22:21:18 +0530 Subject: [PATCH 2/9] - moved the function which returns the required data to service layer and the corresponding queries to model. - removed test cases from integration testing and moved it to unit testing/ - moved archieveUsers to users model as it was causing circular dependency. - created a separate fixture file for mock data required to test the feature. - reverted the test case changes done for other test cases due to mock data changes. --- controllers/users.js | 4 +- models/tasks.js | 15 ++ models/users.js | 79 ++++++--- services/users.js | 59 +++---- .../abandoned-tasks/departed-users.js | 154 ++++++++++++++++++ test/fixtures/tasks/tasks.js | 84 ---------- test/fixtures/user/user.js | 63 ------- test/integration/users.test.js | 55 +------ test/unit/models/tasks.test.js | 8 +- test/unit/models/users.test.js | 21 +++ test/unit/services/tasks.test.js | 4 +- test/unit/services/users.test.js | 63 ++++++- 12 files changed, 332 insertions(+), 277 deletions(-) create mode 100644 test/fixtures/abandoned-tasks/departed-users.js diff --git a/controllers/users.js b/controllers/users.js index ba7c608a8..f20cff5bd 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -29,7 +29,7 @@ const { const { addLog } = require("../models/logs"); const { getUserStatus } = require("../models/userStatus"); const config = require("config"); -const { generateUniqueUsername } = require("../services/users"); +const { generateUniqueUsername, getUsersWithIncompleteTasks } = require("../services/users"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const verifyUser = async (req, res) => { @@ -1031,7 +1031,7 @@ const updateUsernames = async (req, res) => { const getUsersWithAbandonedTasks = async (req, res) => { try { - const data = await userQuery.fetchUsersWithAbandonedTasks(); + const data = await getUsersWithIncompleteTasks(); return res.status(200).json({ message: "Users with abandoned tasks fetched successfully", data }); } catch (error) { logger.error("Error in getting user who abandoned tasks:", error); diff --git a/models/tasks.js b/models/tasks.js index 8b0754b1b..a752e1062 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -701,6 +701,20 @@ const markUnDoneTasksOfArchivedUsersBacklog = async (users) => { } }; +const fetchIncompleteTaskForUser = async (user) => { + const COMPLETED_STATUSES = [DONE, COMPLETED]; + try { + const incompleteTaskForUser = await tasksModel + .where("assigneeId", "==", user.id) + .where("status", "not-in", COMPLETED_STATUSES) + .get(); + return incompleteTaskForUser; + } catch (error) { + logger.error("Error when fetching incomplete tasks:", error); + throw error; + } +}; + module.exports = { updateTask, fetchTasks, @@ -720,4 +734,5 @@ module.exports = { updateTaskStatus, updateOrphanTasksStatus, markUnDoneTasksOfArchivedUsersBacklog, + fetchIncompleteTaskForUser, }; diff --git a/models/users.js b/models/users.js index e7f5a00ab..1656389dd 100644 --- a/models/users.js +++ b/models/users.js @@ -8,8 +8,12 @@ const firestore = require("../utils/firestore"); const { fetchWallet, createWallet } = require("../models/wallets"); const { updateUserStatus } = require("../models/userStatus"); const { arraysHaveCommonItem, chunks } = require("../utils/array"); -const { archiveUsers } = require("../services/users"); -const { ALLOWED_FILTER_PARAMS, FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users"); +const { + ALLOWED_FILTER_PARAMS, + FIRESTORE_IN_CLAUSE_SIZE, + USERS_PATCH_HANDLER_SUCCESS_MESSAGES, + USERS_PATCH_HANDLER_ERROR_MESSAGES, +} = require("../constants/users"); const { DOCUMENT_WRITE_SIZE } = require("../constants/constants"); const { userState } = require("../constants/userStatus"); const { BATCH_SIZE_IN_CLAUSE } = require("../constants/firebase"); @@ -27,9 +31,46 @@ const { formatUsername } = require("../utils/username"); const { logType } = require("../constants/logs"); const { addLog } = require("../services/logService"); -const tasksModel = firestore.collection("tasks"); -const { TASK_STATUS } = require("../constants/tasks"); -const { COMPLETED, DONE } = TASK_STATUS; +const archiveUsers = async (usersData) => { + const batch = firestore.batch(); + const usersBatch = []; + const summary = { + totalUsersArchived: 0, + totalOperationsFailed: 0, + updatedUserDetails: [], + failedUserDetails: [], + }; + + usersData.forEach((user) => { + const { id, first_name: firstName, last_name: lastName } = user; + const updatedUserData = { + ...user, + roles: { + ...user.roles, + archived: true, + }, + updated_at: Date.now(), + }; + batch.update(userModel.doc(id), updatedUserData); + usersBatch.push({ id, firstName, lastName }); + }); + + try { + await batch.commit(); + summary.totalUsersArchived += usersData.length; + summary.updatedUserDetails = [...usersBatch]; + return { + message: USERS_PATCH_HANDLER_SUCCESS_MESSAGES.ARCHIVE_USERS.SUCCESSFULLY_COMPLETED_BATCH_UPDATES, + ...summary, + }; + } catch (err) { + logger.error("Firebase batch Operation Failed!"); + summary.totalOperationsFailed += usersData.length; + summary.failedUserDetails = [...usersBatch]; + return { message: USERS_PATCH_HANDLER_ERROR_MESSAGES.ARCHIVE_USERS.BATCH_DATA_UPDATED_FAILED, ...summary }; + } +}; + /** * Adds or updates the user data * @@ -1033,37 +1074,21 @@ const updateUsersWithNewUsernames = async () => { } }; -const fetchUsersWithAbandonedTasks = async () => { +const fetchUsersNotInDiscordServer = async () => { try { - const COMPLETED_STATUSES = [DONE, COMPLETED]; - const eligibleUsersWithTasks = []; - - const userSnapshot = await userModel + const usersNotInDiscordServer = await userModel .where("roles.archived", "==", true) .where("roles.in_discord", "==", false) .get(); - - for (const userDoc of userSnapshot.docs) { - const user = userDoc.data(); - - // Check if the user has any tasks with status not in [Done, Complete] - const abandonedTasksQuerySnapshot = await tasksModel - .where("assigneeId", "==", user.id) - .where("status", "not-in", COMPLETED_STATUSES) - .get(); - - if (!abandonedTasksQuerySnapshot.empty) { - eligibleUsersWithTasks.push(user); - } - } - return eligibleUsersWithTasks; + return usersNotInDiscordServer; } catch (error) { - logger.error(`Error in getting users who abandoned tasks: ${error}`); + logger.error(`Error in getting users who are not in discord server: ${error}`); throw error; } }; module.exports = { + archiveUsers, addOrUpdate, fetchPaginatedUsers, fetchUser, @@ -1092,5 +1117,5 @@ module.exports = { fetchUserForKeyValue, getNonNickNameSyncedUsers, updateUsersWithNewUsernames, - fetchUsersWithAbandonedTasks, + fetchUsersNotInDiscordServer, }; diff --git a/services/users.js b/services/users.js index bb1d609b6..57e2bdaff 100644 --- a/services/users.js +++ b/services/users.js @@ -1,44 +1,29 @@ -const { USERS_PATCH_HANDLER_SUCCESS_MESSAGES, USERS_PATCH_HANDLER_ERROR_MESSAGES } = require("../constants/users"); const firestore = require("../utils/firestore"); const { formatUsername } = require("../utils/username"); const userModel = firestore.collection("users"); -const archiveUsers = async (usersData) => { - const batch = firestore.batch(); - const usersBatch = []; - const summary = { - totalUsersArchived: 0, - totalOperationsFailed: 0, - updatedUserDetails: [], - failedUserDetails: [], - }; - - usersData.forEach((user) => { - const { id, first_name: firstName, last_name: lastName } = user; - const updatedUserData = { - ...user, - roles: { - ...user.roles, - archived: true, - }, - updated_at: Date.now(), - }; - batch.update(userModel.doc(id), updatedUserData); - usersBatch.push({ id, firstName, lastName }); - }); +const { fetchUsersNotInDiscordServer } = require("../models/users"); +const { fetchIncompleteTaskForUser } = require("../models/tasks"); +const getUsersWithIncompleteTasks = async () => { try { - await batch.commit(); - summary.totalUsersArchived += usersData.length; - summary.updatedUserDetails = [...usersBatch]; - return { - message: USERS_PATCH_HANDLER_SUCCESS_MESSAGES.ARCHIVE_USERS.SUCCESSFULLY_COMPLETED_BATCH_UPDATES, - ...summary, - }; - } catch (err) { - logger.error("Firebase batch Operation Failed!"); - summary.totalOperationsFailed += usersData.length; - summary.failedUserDetails = [...usersBatch]; - return { message: USERS_PATCH_HANDLER_ERROR_MESSAGES.ARCHIVE_USERS.BATCH_DATA_UPDATED_FAILED, ...summary }; + const eligibleUsersWithTasks = []; + + const userSnapshot = await fetchUsersNotInDiscordServer(); + + for (const userDoc of userSnapshot.docs) { + const user = userDoc.data(); + + // Check if the user has any tasks with status not in [Done, Complete] + const abandonedTasksQuerySnapshot = await fetchIncompleteTaskForUser(user); + + if (!abandonedTasksQuerySnapshot.empty) { + eligibleUsersWithTasks.push(user); + } + } + return eligibleUsersWithTasks; + } catch (error) { + logger.error(`Error in getting users who abandoned tasks: ${error}`); + throw error; } }; @@ -63,6 +48,6 @@ const generateUniqueUsername = async (firstName, lastName) => { }; module.exports = { - archiveUsers, generateUniqueUsername, + getUsersWithIncompleteTasks, }; diff --git a/test/fixtures/abandoned-tasks/departed-users.js b/test/fixtures/abandoned-tasks/departed-users.js new file mode 100644 index 000000000..d16ca551b --- /dev/null +++ b/test/fixtures/abandoned-tasks/departed-users.js @@ -0,0 +1,154 @@ +const usersData = [ + { + id: "user1_id", + discordId: "123456789", + github_id: "github_user1", + username: "archived_user1", + first_name: "Archived", + last_name: "User One", + linkedin_id: "archived_user1", + github_display_name: "archived-user-1", + phone: "1234567890", + email: "archived1@test.com", + roles: { + archived: true, + in_discord: false, + }, + discordJoinedAt: "2024-01-01T00:00:00.000Z", + picture: { + publicId: "profile/user1", + url: "https://example.com/user1.jpg", + }, + }, + { + id: "user2_id", + discordId: "987654321", + github_id: "github_user2", + username: "archived_user2", + first_name: "Archived", + last_name: "User Two", + linkedin_id: "archived_user2", + github_display_name: "archived-user-2", + phone: "0987654321", + email: "archived2@test.com", + roles: { + archived: true, + in_discord: false, + }, + discordJoinedAt: "2024-01-02T00:00:00.000Z", + picture: { + publicId: "profile/user2", + url: "https://example.com/user2.jpg", + }, + }, + { + id: "user3_id", + discordId: "555555555", + github_id: "github_user3", + username: "active_user", + first_name: "Active", + last_name: "User", + linkedin_id: "active_user", + github_display_name: "active-user", + phone: "5555555555", + email: "active@test.com", + roles: { + archived: false, + in_discord: true, + }, + discordJoinedAt: "2024-01-03T00:00:00.000Z", + picture: { + publicId: "profile/user3", + url: "https://example.com/user3.jpg", + }, + }, +]; + +const tasksData = [ + { + id: "task1_id", + title: "Abandoned Task 1", + type: "feature", + status: "IN_PROGRESS", + priority: "HIGH", + percentCompleted: 50, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "archived_user1", + assigneeId: "user1_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/1", + url: "https://api.github.com/repos/org/repo/issues/1", + }, + }, + dependsOn: [], + }, + { + id: "task2_id", + title: "Abandoned Task 2", + type: "bug", + status: "BLOCKED", + priority: "MEDIUM", + percentCompleted: 30, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "archived_user2", + assigneeId: "user2_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/2", + url: "https://api.github.com/repos/org/repo/issues/2", + }, + }, + dependsOn: [], + }, + { + id: "task3_id", + title: "Completed Archived Task", + type: "feature", + status: "DONE", + priority: "LOW", + percentCompleted: 100, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "archived_user1", + assigneeId: "user1_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/3", + url: "https://api.github.com/repos/org/repo/issues/3", + }, + }, + dependsOn: [], + }, + { + id: "task4_id", + title: "Active User Task", + type: "feature", + status: "IN_PROGRESS", + priority: "HIGH", + percentCompleted: 75, + createdAt: 1727027666, + updatedAt: 1727027999, + startedOn: 1727027777, + endsOn: 1731542400, + assignee: "active_user", + assigneeId: "user3_id", + github: { + issue: { + html_url: "https://github.com/org/repo/issues/4", + url: "https://api.github.com/repos/org/repo/issues/4", + }, + }, + dependsOn: [], + }, +]; + +module.exports = { usersData, tasksData }; diff --git a/test/fixtures/tasks/tasks.js b/test/fixtures/tasks/tasks.js index 1768c6cd6..1d5efa978 100644 --- a/test/fixtures/tasks/tasks.js +++ b/test/fixtures/tasks/tasks.js @@ -140,89 +140,5 @@ module.exports = () => { createdAt: 1644753600, updatedAt: 1644753600, }, - { - id: "task1_id", - title: "Abandoned Task 1", - type: "feature", - status: "IN_PROGRESS", - priority: "HIGH", - percentCompleted: 50, - createdAt: 1727027666, - updatedAt: 1727027999, - startedOn: 1727027777, - endsOn: 1731542400, - assignee: "archived_user1", - assigneeId: "user1_id", - github: { - issue: { - html_url: "https://github.com/org/repo/issues/1", - url: "https://api.github.com/repos/org/repo/issues/1", - }, - }, - dependsOn: [], - }, - { - id: "task2_id", - title: "Abandoned Task 2", - type: "bug", - status: "BLOCKED", - priority: "MEDIUM", - percentCompleted: 30, - createdAt: 1727027666, - updatedAt: 1727027999, - startedOn: 1727027777, - endsOn: 1731542400, - assignee: "archived_user2", - assigneeId: "user2_id", - github: { - issue: { - html_url: "https://github.com/org/repo/issues/2", - url: "https://api.github.com/repos/org/repo/issues/2", - }, - }, - dependsOn: [], - }, - { - id: "task3_id", - title: "Completed Archived Task", - type: "feature", - status: "DONE", - priority: "LOW", - percentCompleted: 100, - createdAt: 1727027666, - updatedAt: 1727027999, - startedOn: 1727027777, - endsOn: 1731542400, - assignee: "archived_user1", - assigneeId: "user1_id", - github: { - issue: { - html_url: "https://github.com/org/repo/issues/3", - url: "https://api.github.com/repos/org/repo/issues/3", - }, - }, - dependsOn: [], - }, - { - id: "task4_id", - title: "Active User Task", - type: "feature", - status: "IN_PROGRESS", - priority: "HIGH", - percentCompleted: 75, - createdAt: 1727027666, - updatedAt: 1727027999, - startedOn: 1727027777, - endsOn: 1731542400, - assignee: "active_user", - assigneeId: "user3_id", - github: { - issue: { - html_url: "https://github.com/org/repo/issues/4", - url: "https://api.github.com/repos/org/repo/issues/4", - }, - }, - dependsOn: [], - }, ]; }; diff --git a/test/fixtures/user/user.js b/test/fixtures/user/user.js index 674718d61..485255efc 100644 --- a/test/fixtures/user/user.js +++ b/test/fixtures/user/user.js @@ -442,68 +442,5 @@ module.exports = () => { url: "https://res.cloudinary.com/realdevsquad/image/upload/v1667685133/profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar.jpg", }, }, - { - id: "user1_id", - discordId: "123456789", - github_id: "github_user1", - username: "archived_user1", - first_name: "Archived", - last_name: "User One", - linkedin_id: "archived_user1", - github_display_name: "archived-user-1", - phone: "1234567890", - email: "archived1@test.com", - roles: { - archived: true, - in_discord: false, - }, - discordJoinedAt: "2024-01-01T00:00:00.000Z", - picture: { - publicId: "profile/user1", - url: "https://example.com/user1.jpg", - }, - }, - { - id: "user2_id", - discordId: "987654321", - github_id: "github_user2", - username: "archived_user2", - first_name: "Archived", - last_name: "User Two", - linkedin_id: "archived_user2", - github_display_name: "archived-user-2", - phone: "0987654321", - email: "archived2@test.com", - roles: { - archived: true, - in_discord: false, - }, - discordJoinedAt: "2024-01-02T00:00:00.000Z", - picture: { - publicId: "profile/user2", - url: "https://example.com/user2.jpg", - }, - }, - { - id: "user3_id", - discordId: "555555555", - github_id: "github_user3", - username: "active_user", - first_name: "Active", - last_name: "User", - linkedin_id: "active_user", - github_display_name: "active-user", - phone: "5555555555", - email: "active@test.com", - roles: { - archived: false, - in_discord: true, - }, - discordJoinedAt: "2024-01-03T00:00:00.000Z", - picture: { - publicId: "profile/user3", - url: "https://example.com/user3.jpg", - }, - }, ]; }; diff --git a/test/integration/users.test.js b/test/integration/users.test.js index cda75c3d7..708b87d2f 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -23,12 +23,9 @@ const { userStatusDataAfterSignup, userStatusDataAfterFillingJoinSection, } = require("../fixtures/userStatus/userStatus"); -const { addJoinData, addOrUpdate, fetchUsersWithAbandonedTasks } = require("../../models/users"); +const { addJoinData, addOrUpdate } = require("../../models/users"); const userStatusModel = require("../../models/userStatus"); const { MAX_USERNAME_LENGTH } = require("../../constants/users.ts"); -const tasksData = require("../fixtures/tasks/tasks")(); -const tasksModel = firestore.collection("tasks"); -const userDBModel = firestore.collection("users"); const userRoleUpdate = userData[4]; const userRoleUnArchived = userData[13]; @@ -2649,54 +2646,4 @@ describe("Users", function () { expect(res).to.have.status(401); }); }); - - describe("fetchUsersWithAbandonedTasks", function () { - beforeEach(async function () { - // Clean the database - await cleanDb(); - - // Add test users to the database - const userPromises = userData.map((user) => userDBModel.add(user)); - await Promise.all(userPromises); - - // Add test tasks to the database - const taskPromises = tasksData.map((task) => tasksModel.add(task)); - await Promise.all(taskPromises); - }); - - afterEach(async function () { - await cleanDb(); - }); - - it("should fetch users with abandoned tasks", async function () { - const usersWithAbandonedTasks = await fetchUsersWithAbandonedTasks(); - - expect(usersWithAbandonedTasks).to.be.an("array"); - expect(usersWithAbandonedTasks).to.have.lengthOf(2); // Two users with abandoned tasks - }); - - it("should not include user who are present in discord or not archived", async function () { - const usersWithAbandonedTasks = await fetchUsersWithAbandonedTasks(); - - usersWithAbandonedTasks.forEach((user) => { - expect(user.roles.in_discord).to.not.equal(true); - expect(user.roles.archived).to.not.equal(false); - }); - }); - - it("should return an empty array if there are no users with abandoned tasks", async function () { - await cleanDb(); - - // Add only active users - const activeUser = userData[11]; // Using the active user from our test data - await userDBModel.add(activeUser); - - // Add a task assigned to the active user - const activeTask = tasksData[11]; // Using the active user's task - await tasksModel.add(activeTask); - const usersWithAbandonedTasks = await fetchUsersWithAbandonedTasks(); - expect(usersWithAbandonedTasks).to.be.an("array"); - expect(usersWithAbandonedTasks).to.have.lengthOf(0); - }); - }); }); diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index db9ccd15e..44a560fd5 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -303,12 +303,12 @@ describe("tasks", function () { overdueTask.endsOn = Date.now() / 1000 + 24 * 60 * 60 * 7; await tasks.updateTask(overdueTask); const usersWithOverdueTasks = await tasks.getOverdueTasks(days); - expect(usersWithOverdueTasks.length).to.be.equal(8); + expect(usersWithOverdueTasks.length).to.be.equal(5); }); it("should return all users which have overdue tasks if days is not passed", async function () { const usersWithOverdueTasks = await tasks.getOverdueTasks(); - expect(usersWithOverdueTasks.length).to.be.equal(7); + expect(usersWithOverdueTasks.length).to.be.equal(4); }); }); @@ -332,8 +332,8 @@ describe("tasks", function () { it("Should update task status COMPLETED to DONE", async function () { const res = await tasks.updateTaskStatus(); - expect(res.totalTasks).to.be.equal(12); - expect(res.totalUpdatedStatus).to.be.equal(12); + expect(res.totalTasks).to.be.equal(8); + expect(res.totalUpdatedStatus).to.be.equal(8); }); it("should throw an error if firebase batch operation fails", async function () { diff --git a/test/unit/models/users.test.js b/test/unit/models/users.test.js index 42165094f..32165ecea 100644 --- a/test/unit/models/users.test.js +++ b/test/unit/models/users.test.js @@ -21,6 +21,7 @@ const photoVerificationModel = firestore.collection("photo-verification"); const userData = require("../../fixtures/user/user"); const addUser = require("../../utils/addUser"); const { userState } = require("../../../constants/userStatus"); +const { usersData: abandonedUsersData } = require("../../fixtures/abandoned-tasks/departed-users"); /** * Test the model functions and validate the data stored */ @@ -527,4 +528,24 @@ describe("users", function () { expect(userDoc.user.roles.super_user).to.be.equal(false); }); }); + + describe("fetchUsersNotInDiscordServer", function () { + beforeEach(async function () { + // Clean the database + await cleanDb(); + + // Add test users to the database + const taskPromises = abandonedUsersData.map((task) => userModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + await cleanDb(); + }); + + it("should fetch users not in discord server", async function () { + const usersNotInDiscordServer = await users.fetchUsersNotInDiscordServer(); + expect(usersNotInDiscordServer.docs.length).to.be.equal(2); + }); + }); }); diff --git a/test/unit/services/tasks.test.js b/test/unit/services/tasks.test.js index 8e675958c..02d424134 100644 --- a/test/unit/services/tasks.test.js +++ b/test/unit/services/tasks.test.js @@ -46,7 +46,7 @@ describe("Tasks services", function () { const res = await updateTaskStatusToDone(tasks); expect(res).to.deep.equal({ - totalUpdatedStatus: 12, + totalUpdatedStatus: 8, totalOperationsFailed: 0, updatedTaskDetails: taskDetails, failedTaskDetails: [], @@ -66,7 +66,7 @@ describe("Tasks services", function () { expect(res).to.deep.equal({ totalUpdatedStatus: 0, - totalOperationsFailed: 12, + totalOperationsFailed: 8, updatedTaskDetails: [], failedTaskDetails: taskDetails, }); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index 611b1e64c..a0123f5e0 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -3,10 +3,15 @@ const { expect } = require("chai"); const firestore = require("../../../utils/firestore"); const userModel = firestore.collection("users"); +const tasksModel = firestore.collection("tasks"); const cleanDb = require("../../utils/cleanDb"); const userDataArray = require("../../fixtures/user/user")(); -const { archiveUsers, generateUniqueUsername } = require("../../../services/users"); -const { addOrUpdate } = require("../../../models/users"); +const { generateUniqueUsername, getUsersWithIncompleteTasks } = require("../../../services/users"); +const { addOrUpdate, archiveUsers } = require("../../../models/users"); +const { + usersData: abandonedUsersData, + tasksData: abandonedTasksData, +} = require("../../fixtures/abandoned-tasks/departed-users"); describe("Users services", function () { describe("archive inactive discord users in bulk", function () { @@ -55,7 +60,7 @@ describe("Users services", function () { expect(res).to.deep.equal({ message: "Successfully completed batch updates", - totalUsersArchived: 23, + totalUsersArchived: 20, totalOperationsFailed: 0, updatedUserDetails: userDetails, failedUserDetails: [], @@ -76,7 +81,7 @@ describe("Users services", function () { expect(res).to.deep.equal({ message: "Firebase batch operation failed", totalUsersArchived: 0, - totalOperationsFailed: 23, + totalOperationsFailed: 20, updatedUserDetails: [], failedUserDetails: userDetails, }); @@ -100,4 +105,54 @@ describe("Users services", function () { expect(newUsername).to.equal("john-doe-1"); }); }); + + describe("fetchUsersWithAbandonedTasks", function () { + beforeEach(async function () { + // Clean the database + await cleanDb(); + + // Add test users to the database + const userPromises = abandonedUsersData.map((user) => userModel.add(user)); + await Promise.all(userPromises); + + // Add test tasks to the database + const taskPromises = abandonedTasksData.map((task) => tasksModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + await cleanDb(); + }); + + it("should fetch users with abandoned tasks", async function () { + const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); + + expect(usersWithAbandonedTasks).to.be.an("array"); + expect(usersWithAbandonedTasks).to.have.lengthOf(2); // Two users with abandoned tasks + }); + + it("should not include user who are present in discord or not archived", async function () { + const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); + + usersWithAbandonedTasks.forEach((user) => { + expect(user.roles.in_discord).to.not.equal(true); + expect(user.roles.archived).to.not.equal(false); + }); + }); + + it("should return an empty array if there are no users with abandoned tasks", async function () { + await cleanDb(); + + // Add only active users + const activeUser = abandonedUsersData[2]; // Using the active user from our test data + await userModel.add(activeUser); + + // Add a task assigned to the active user + const activeTask = abandonedTasksData[3]; // Using the active user's task + await tasksModel.add(activeTask); + const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); + expect(usersWithAbandonedTasks).to.be.an("array"); + expect(usersWithAbandonedTasks).to.have.lengthOf(0); + }); + }); }); From d16df03ccfb360746b718a680bbe43576b6a184e Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Sat, 16 Nov 2024 09:36:20 +0530 Subject: [PATCH 3/9] Chore: Feedback changes - Added function documentation for all the modified and new model functions. - Implemented a negative unit test case for the `fetchUsersNotInDiscordServer` function. - Updated `fetchUsersNotInDiscordServer` to use `userId` as a parameter instead of the entire `user` object. - Added test cases for recent model changes. --- models/tasks.js | 10 ++++++++-- models/users.js | 11 +++++++++++ routes/users.js | 1 - services/users.js | 2 +- test/unit/models/users.test.js | 25 +++++++++++++++++++++++-- test/unit/services/users.test.js | 6 ++---- 6 files changed, 45 insertions(+), 10 deletions(-) diff --git a/models/tasks.js b/models/tasks.js index a752e1062..c9fa3885c 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -701,11 +701,17 @@ const markUnDoneTasksOfArchivedUsersBacklog = async (users) => { } }; -const fetchIncompleteTaskForUser = async (user) => { +/** + * Fetch incomplete tasks assigned to a specific user + * @param {string} userId - The unique identifier for the user. + * @returns {Promise} - A promise that resolves to an array of incomplete tasks for the given user. + * @throws {Error} - Throws an error if the database query fails. + */ +const fetchIncompleteTaskForUser = async (userId) => { const COMPLETED_STATUSES = [DONE, COMPLETED]; try { const incompleteTaskForUser = await tasksModel - .where("assigneeId", "==", user.id) + .where("assigneeId", "==", userId) .where("status", "not-in", COMPLETED_STATUSES) .get(); return incompleteTaskForUser; diff --git a/models/users.js b/models/users.js index 1656389dd..fa46d1c82 100644 --- a/models/users.js +++ b/models/users.js @@ -31,6 +31,12 @@ const { formatUsername } = require("../utils/username"); const { logType } = require("../constants/logs"); const { addLog } = require("../services/logService"); +/** + * Archive users by setting the roles.archived field to true. + * This function commits the write in batches to avoid reaching the maximum number of writes per batch. + * @param {Array} usersData - An array of user objects with the following properties: id, first_name, last_name + * @returns {Promise} - A promise that resolves with a summary object containing the number of users updated and failed, and an array of updated and failed user details. + */ const archiveUsers = async (usersData) => { const batch = firestore.batch(); const usersBatch = []; @@ -1074,6 +1080,11 @@ const updateUsersWithNewUsernames = async () => { } }; +/** + * Fetches users who are not in the Discord server. + * @returns {Promise} - A promise that resolves to a Firestore QuerySnapshot containing the users matching the criteria. + * @throws {Error} - Throws an error if the database query fails. + */ const fetchUsersNotInDiscordServer = async () => { try { const usersNotInDiscordServer = await userModel diff --git a/routes/users.js b/routes/users.js index e8d19573d..53e363934 100644 --- a/routes/users.js +++ b/routes/users.js @@ -68,7 +68,6 @@ router.patch("/profileURL", authenticate, userValidator.updateProfileURL, users. router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rejectProfileDiff); router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); router.get("/suggestedUsers/:skillId", authenticate, authorizeRoles([SUPERUSER]), users.getSuggestedUsers); - module.exports = router; router.post("/batch-username-update", authenticate, authorizeRoles([SUPERUSER]), users.updateUsernames); module.exports = router; diff --git a/services/users.js b/services/users.js index 57e2bdaff..3ef54989c 100644 --- a/services/users.js +++ b/services/users.js @@ -14,7 +14,7 @@ const getUsersWithIncompleteTasks = async () => { const user = userDoc.data(); // Check if the user has any tasks with status not in [Done, Complete] - const abandonedTasksQuerySnapshot = await fetchIncompleteTaskForUser(user); + const abandonedTasksQuerySnapshot = await fetchIncompleteTaskForUser(user.id); if (!abandonedTasksQuerySnapshot.empty) { eligibleUsersWithTasks.push(user); diff --git a/test/unit/models/users.test.js b/test/unit/models/users.test.js index 32165ecea..e0d98af4b 100644 --- a/test/unit/models/users.test.js +++ b/test/unit/models/users.test.js @@ -531,21 +531,42 @@ describe("users", function () { describe("fetchUsersNotInDiscordServer", function () { beforeEach(async function () { - // Clean the database await cleanDb(); - // Add test users to the database const taskPromises = abandonedUsersData.map((task) => userModel.add(task)); await Promise.all(taskPromises); }); afterEach(async function () { await cleanDb(); + sinon.restore(); }); it("should fetch users not in discord server", async function () { const usersNotInDiscordServer = await users.fetchUsersNotInDiscordServer(); expect(usersNotInDiscordServer.docs.length).to.be.equal(2); }); + + it("should return an empty array if there are no users in the database", async function () { + await cleanDb(); + + // Add only active users + const activeUser = abandonedUsersData[2]; // Using the active user from our test data + await userModel.add(activeUser); + + const usersNotInDiscordServer = await users.fetchUsersNotInDiscordServer(); + expect(usersNotInDiscordServer.docs.length).to.be.equal(0); + }); + + it("should handle errors gracefully if the database query fails", async function () { + sinon.stub(users, "fetchUsersNotInDiscordServer").throws(new Error("Database query failed")); + + try { + await users.fetchUsersNotInDiscordServer(); + expect.fail("Expected function to throw an error"); + } catch (error) { + expect(error.message).to.equal("Database query failed"); + } + }); }); }); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index a0123f5e0..e79927d21 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -143,12 +143,10 @@ describe("Users services", function () { it("should return an empty array if there are no users with abandoned tasks", async function () { await cleanDb(); - // Add only active users - const activeUser = abandonedUsersData[2]; // Using the active user from our test data + const activeUser = abandonedUsersData[2]; await userModel.add(activeUser); - // Add a task assigned to the active user - const activeTask = abandonedTasksData[3]; // Using the active user's task + const activeTask = abandonedTasksData[3]; await tasksModel.add(activeTask); const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); expect(usersWithAbandonedTasks).to.be.an("array"); From f8cce46c893e5e14bd218d157a83ee7494182f20 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Sat, 16 Nov 2024 09:59:49 +0530 Subject: [PATCH 4/9] chore: Added test cases for tasks model and removed unnecessary comments. --- test/unit/models/tasks.test.js | 43 ++++++++++++++++++++++++++++++++ test/unit/models/users.test.js | 6 ++--- test/unit/services/users.test.js | 3 --- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index 44a560fd5..32609800d 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -17,6 +17,10 @@ const dependencyModel = firestore.collection("TaskDependencies"); const tasksModel = firestore.collection("tasks"); const userData = require("../../fixtures/user/user"); const addUser = require("../../utils/addUser"); +const { + usersData: abandonedUsersData, + tasksData: abandonedTasksData, +} = require("../../fixtures/abandoned-tasks/departed-users"); describe("tasks", function () { afterEach(async function () { @@ -352,4 +356,43 @@ describe("tasks", function () { } }); }); + + describe("fetchIncompleteTaskForUser", function () { + beforeEach(async function () { + await cleanDb(); + + const taskPromises = abandonedTasksData.map((task) => tasksModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + await cleanDb(); + sinon.restore(); + }); + + it("should fetch tasks which are incomplete for the given user", async function () { + // using inactive user id from test data + const inactiveUser = abandonedUsersData[0]; + const incompleteTasks = await tasks.fetchIncompleteTaskForUser(inactiveUser.id); + expect(incompleteTasks.docs.length).to.be.equal(1); + }); + + it("should return an empty array if there are no tasks incomplete for the user", async function () { + // Using the active user from our test data + const activeUser = abandonedUsersData[2]; + const incompleteTasks = await tasks.fetchIncompleteTaskForUser(activeUser.id); + expect(incompleteTasks.docs.length).to.be.equal(0); + }); + + it("should handle errors gracefully if the database query fails", async function () { + sinon.stub(tasks, "fetchIncompleteTaskForUser").throws(new Error("Database query failed")); + + try { + await tasks.fetchIncompleteTaskForUser(); + expect.fail("Expected function to throw an error"); + } catch (error) { + expect(error.message).to.equal("Database query failed"); + } + }); + }); }); diff --git a/test/unit/models/users.test.js b/test/unit/models/users.test.js index e0d98af4b..1573a95dc 100644 --- a/test/unit/models/users.test.js +++ b/test/unit/models/users.test.js @@ -548,10 +548,8 @@ describe("users", function () { }); it("should return an empty array if there are no users in the database", async function () { - await cleanDb(); - - // Add only active users - const activeUser = abandonedUsersData[2]; // Using the active user from our test data + // Using the active user from our test data + const activeUser = abandonedUsersData[2]; await userModel.add(activeUser); const usersNotInDiscordServer = await users.fetchUsersNotInDiscordServer(); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index e79927d21..aad2bd54e 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -108,14 +108,11 @@ describe("Users services", function () { describe("fetchUsersWithAbandonedTasks", function () { beforeEach(async function () { - // Clean the database await cleanDb(); - // Add test users to the database const userPromises = abandonedUsersData.map((user) => userModel.add(user)); await Promise.all(userPromises); - // Add test tasks to the database const taskPromises = abandonedTasksData.map((task) => tasksModel.add(task)); await Promise.all(taskPromises); }); From aedda22d7170c54f6a63f8be2c9caeedfeb5c876 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Sat, 16 Nov 2024 10:03:08 +0530 Subject: [PATCH 5/9] Removed unnecessary comments. --- test/unit/models/tasks.test.js | 2 -- test/unit/services/users.test.js | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index 32609800d..d67e1bd5f 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -371,14 +371,12 @@ describe("tasks", function () { }); it("should fetch tasks which are incomplete for the given user", async function () { - // using inactive user id from test data const inactiveUser = abandonedUsersData[0]; const incompleteTasks = await tasks.fetchIncompleteTaskForUser(inactiveUser.id); expect(incompleteTasks.docs.length).to.be.equal(1); }); it("should return an empty array if there are no tasks incomplete for the user", async function () { - // Using the active user from our test data const activeUser = abandonedUsersData[2]; const incompleteTasks = await tasks.fetchIncompleteTaskForUser(activeUser.id); expect(incompleteTasks.docs.length).to.be.equal(0); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index aad2bd54e..6011a0ca8 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -125,7 +125,7 @@ describe("Users services", function () { const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); expect(usersWithAbandonedTasks).to.be.an("array"); - expect(usersWithAbandonedTasks).to.have.lengthOf(2); // Two users with abandoned tasks + expect(usersWithAbandonedTasks).to.have.lengthOf(2); }); it("should not include user who are present in discord or not archived", async function () { From 28b078dc8d7355ad992deb89cab7e7f6aeefd272 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Sat, 16 Nov 2024 19:25:37 +0530 Subject: [PATCH 6/9] Feedback changes: - Added controller test cases in test/integration. - Removed unecessary comments. --- models/users.js | 2 +- services/users.js | 1 - test/integration/users.test.js | 44 ++++++++++++++++++++++++++++++++++ test/unit/models/tasks.test.js | 1 + test/unit/models/users.test.js | 6 ++--- 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/models/users.js b/models/users.js index fa46d1c82..11d56a0da 100644 --- a/models/users.js +++ b/models/users.js @@ -1082,7 +1082,7 @@ const updateUsersWithNewUsernames = async () => { /** * Fetches users who are not in the Discord server. - * @returns {Promise} - A promise that resolves to a Firestore QuerySnapshot containing the users matching the criteria. + * @returns {Promise} - A promise resolving to a Firestore QuerySnapshot of matching users. * @throws {Error} - Throws an error if the database query fails. */ const fetchUsersNotInDiscordServer = async () => { diff --git a/services/users.js b/services/users.js index 3ef54989c..764480203 100644 --- a/services/users.js +++ b/services/users.js @@ -13,7 +13,6 @@ const getUsersWithIncompleteTasks = async () => { for (const userDoc of userSnapshot.docs) { const user = userDoc.data(); - // Check if the user has any tasks with status not in [Done, Complete] const abandonedTasksQuerySnapshot = await fetchIncompleteTaskForUser(user.id); if (!abandonedTasksQuerySnapshot.empty) { diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 708b87d2f..0c401bf6a 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -41,6 +41,12 @@ const { userPhotoVerificationData } = require("../fixtures/user/photo-verificati const Sinon = require("sinon"); const { INTERNAL_SERVER_ERROR, SOMETHING_WENT_WRONG } = require("../../constants/errorMessages"); const photoVerificationModel = firestore.collection("photo-verification"); +const userModel = firestore.collection("users"); +const taskModel = firestore.collection("tasks"); +const { + usersData: abandonedUsersData, + tasksData: abandonedTasksData, +} = require("../fixtures/abandoned-tasks/departed-users"); chai.use(chaiHttp); @@ -2646,4 +2652,42 @@ describe("Users", function () { expect(res).to.have.status(401); }); }); + + describe("GET /users/departed-users", function () { + beforeEach(async function () { + await cleanDb(); + const userPromises = abandonedUsersData.map((user) => userModel.add(user)); + await Promise.all(userPromises); + + const taskPromises = abandonedTasksData.map((task) => taskModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + Sinon.restore(); + await cleanDb(); + }); + + it("should return a list of users with abandoned tasks", async function () { + const res = await chai.request(app).get("/users/departed-users"); + + expect(res).to.have.status(200); + expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully"); + expect(res.body.data).to.be.an("array").with.lengthOf(2); + }); + + it("should return an empty array when no users have abandoned tasks", async function () { + await cleanDb(); + const user = abandonedUsersData[2]; + await userModel.add(user); + + const task = abandonedTasksData[3]; + await taskModel.add(task); + const res = await chai.request(app).get("/users/departed-users"); + + expect(res).to.have.status(200); + expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully"); + expect(res.body.data).to.be.an("array").with.lengthOf(0); + }); + }); }); diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index d67e1bd5f..530aba923 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -377,6 +377,7 @@ describe("tasks", function () { }); it("should return an empty array if there are no tasks incomplete for the user", async function () { + await cleanDb(); const activeUser = abandonedUsersData[2]; const incompleteTasks = await tasks.fetchIncompleteTaskForUser(activeUser.id); expect(incompleteTasks.docs.length).to.be.equal(0); diff --git a/test/unit/models/users.test.js b/test/unit/models/users.test.js index 1573a95dc..bea6f15aa 100644 --- a/test/unit/models/users.test.js +++ b/test/unit/models/users.test.js @@ -533,8 +533,8 @@ describe("users", function () { beforeEach(async function () { await cleanDb(); - const taskPromises = abandonedUsersData.map((task) => userModel.add(task)); - await Promise.all(taskPromises); + const userPromises = abandonedUsersData.map((user) => userModel.add(user)); + await Promise.all(userPromises); }); afterEach(async function () { @@ -548,7 +548,7 @@ describe("users", function () { }); it("should return an empty array if there are no users in the database", async function () { - // Using the active user from our test data + await cleanDb(); const activeUser = abandonedUsersData[2]; await userModel.add(activeUser); From 9c167c318c0df7e7baee35e198ddda3a4f3e8ef5 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Tue, 19 Nov 2024 09:14:15 +0530 Subject: [PATCH 7/9] - Added dev feature flag through the middleware to control access to the /users/departed-users API. - Implemented negative test cases for the /users/departed-users endpoint: - Tested the scenario where the dev flag is not provided, expecting a 404 status code with a "Route not found" message. - Tested the scenario where the database query fails, ensuring the API responds with a 500 status code and an appropriate error message. --- controllers/users.js | 6 ++++-- routes/users.js | 3 ++- test/integration/users.test.js | 23 ++++++++++++++++++----- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index f20cff5bd..39eb2021c 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -29,7 +29,8 @@ const { const { addLog } = require("../models/logs"); const { getUserStatus } = require("../models/userStatus"); const config = require("config"); -const { generateUniqueUsername, getUsersWithIncompleteTasks } = require("../services/users"); +const { generateUniqueUsername } = require("../services/users"); +const userService = require("../services/users"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const verifyUser = async (req, res) => { @@ -1031,7 +1032,8 @@ const updateUsernames = async (req, res) => { const getUsersWithAbandonedTasks = async (req, res) => { try { - const data = await getUsersWithIncompleteTasks(); + const data = await userService.getUsersWithIncompleteTasks(); + if (data.length === 0) res.status(204).send(); return res.status(200).json({ message: "Users with abandoned tasks fetched successfully", data }); } catch (error) { logger.error("Error in getting user who abandoned tasks:", error); diff --git a/routes/users.js b/routes/users.js index 53e363934..1ce4fc008 100644 --- a/routes/users.js +++ b/routes/users.js @@ -12,8 +12,9 @@ const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndSe const ROLES = require("../constants/roles"); const { Services } = require("../constants/bot"); const authenticateProfile = require("../middlewares/authenticateProfile"); +const { devFlagMiddleware } = require("../middlewares/devFlag"); -router.get("/departed-users", users.getUsersWithAbandonedTasks); +router.get("/departed-users", devFlagMiddleware, users.getUsersWithAbandonedTasks); router.post("/", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), users.markUnverified); router.post("/update-in-discord", authenticate, authorizeRoles([SUPERUSER]), users.setInDiscordScript); router.post("/verify", authenticate, users.verifyUser); diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 0c401bf6a..d847d3fce 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -47,7 +47,7 @@ const { usersData: abandonedUsersData, tasksData: abandonedTasksData, } = require("../fixtures/abandoned-tasks/departed-users"); - +const userService = require("../../services/users"); chai.use(chaiHttp); describe("Users", function () { @@ -2669,7 +2669,7 @@ describe("Users", function () { }); it("should return a list of users with abandoned tasks", async function () { - const res = await chai.request(app).get("/users/departed-users"); + const res = await chai.request(app).get("/users/departed-users?dev=true"); expect(res).to.have.status(200); expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully"); @@ -2683,11 +2683,24 @@ describe("Users", function () { const task = abandonedTasksData[3]; await taskModel.add(task); + const res = await chai.request(app).get("/users/departed-users?dev=true"); + + expect(res).to.have.status(204); + }); + + it("should fail if dev flag is not passed", async function () { const res = await chai.request(app).get("/users/departed-users"); + expect(res).to.have.status(404); + expect(res.body.message).to.be.equal("Route not found"); + }); - expect(res).to.have.status(200); - expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully"); - expect(res.body.data).to.be.an("array").with.lengthOf(0); + it("should handle errors gracefully if the database query fails", async function () { + Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(INTERNAL_SERVER_ERROR)); + + const res = await chai.request(app).get("/users/departed-users?dev=true"); + + expect(res).to.have.status(500); + expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR); }); }); }); From 2a94cf392b84607ec8869567b0eb00f3a862e208 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Sun, 24 Nov 2024 11:26:56 +0530 Subject: [PATCH 8/9] feat: Removed separate rotue for departed users and added the functionality under /users route with departed parameter. Added test cases for the same. --- controllers/users.js | 38 +++++--- middlewares/validators/user.js | 1 + models/users.js | 29 ++---- routes/users.js | 2 - services/users.js | 16 +--- test/integration/users.test.js | 101 ++++++++++----------- test/unit/models/users.test.js | 21 +++-- test/unit/services/dataAccessLayer.test.js | 78 ++++++++++++++++ test/unit/services/users.test.js | 35 ++++--- 9 files changed, 203 insertions(+), 118 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 39eb2021c..1307d432c 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -192,6 +192,32 @@ const getUsers = async (req, res) => { } } + const isDeparted = req.query.departed === "true"; + + if (isDeparted) { + let result; + if (dev) { + try { + result = await dataAccess.retrieveUsers({ query: req.query }); + const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); + if (departedUsers.length === 0) res.status(204).send(); + return res.json({ + message: "Users with abandoned tasks fetched successfully", + users: departedUsers, + links: { + next: result.nextId ? getPaginationLink(req.query, "next", result.nextId) : "", + prev: result.prevId ? getPaginationLink(req.query, "prev", result.prevId) : "", + }, + }); + } catch (error) { + logger.error("Error when fetching users who abandoned tasks:", error); + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } + } else { + return res.boom.notFound("Route not found"); + } + } + if (transformedQuery?.filterBy === OVERDUE_TASKS) { try { const tasksData = await getOverdueTasks(days); @@ -1030,17 +1056,6 @@ const updateUsernames = async (req, res) => { } }; -const getUsersWithAbandonedTasks = async (req, res) => { - try { - const data = await userService.getUsersWithIncompleteTasks(); - if (data.length === 0) res.status(204).send(); - return res.status(200).json({ message: "Users with abandoned tasks fetched successfully", data }); - } catch (error) { - logger.error("Error in getting user who abandoned tasks:", error); - return res.boom.badImplementation(INTERNAL_SERVER_ERROR); - } -}; - module.exports = { verifyUser, generateChaincode, @@ -1073,5 +1088,4 @@ module.exports = { isDeveloper, getIdentityStats, updateUsernames, - getUsersWithAbandonedTasks, }; diff --git a/middlewares/validators/user.js b/middlewares/validators/user.js index 7d21a5161..2604ff51d 100644 --- a/middlewares/validators/user.js +++ b/middlewares/validators/user.js @@ -200,6 +200,7 @@ async function getUsers(req, res, next) { filterBy: joi.string().optional(), days: joi.string().optional(), dev: joi.string().optional(), + departed: joi.string().optional(), roles: joi.optional().custom((value, helpers) => { if (value !== "member") { return helpers.message("only member role is supported"); diff --git a/models/users.js b/models/users.js index 11d56a0da..88a65a704 100644 --- a/models/users.js +++ b/models/users.js @@ -232,11 +232,11 @@ const getSuggestedUsers = async (skill) => { */ const fetchPaginatedUsers = async (query) => { const isDevMode = query.dev === "true"; - try { const size = parseInt(query.size) || 100; const doc = (query.next || query.prev) && (await userModel.doc(query.next || query.prev).get()); + const isArchived = query.departed === "true"; let dbQuery; /** * !!NOTE : At the time of writing we only support member in the role query @@ -245,9 +245,9 @@ const fetchPaginatedUsers = async (query) => { * if you're making changes to this code remove the archived check in the role query, example: role=archived,member */ if (query.roles === "member") { - dbQuery = userModel.where("roles.archived", "==", false).where("roles.member", "==", true); + dbQuery = userModel.where("roles.archived", "==", isArchived).where("roles.member", "==", true); } else { - dbQuery = userModel.where("roles.archived", "==", false).orderBy("username"); + dbQuery = userModel.where("roles.archived", "==", isArchived).orderBy("username"); } let compositeQuery = [dbQuery]; @@ -267,6 +267,10 @@ const fetchPaginatedUsers = async (query) => { } if (Object.keys(query).length) { + if (query.departed) { + compositeQuery = compositeQuery.map((query) => query.where("roles.in_discord", "==", false)); + dbQuery = dbQuery.where("roles.in_discord", "==", false); + } if (query.search) { const searchValue = query.search.toLowerCase().trim(); dbQuery = dbQuery.startAt(searchValue).endAt(searchValue + "\uf8ff"); @@ -1080,24 +1084,6 @@ const updateUsersWithNewUsernames = async () => { } }; -/** - * Fetches users who are not in the Discord server. - * @returns {Promise} - A promise resolving to a Firestore QuerySnapshot of matching users. - * @throws {Error} - Throws an error if the database query fails. - */ -const fetchUsersNotInDiscordServer = async () => { - try { - const usersNotInDiscordServer = await userModel - .where("roles.archived", "==", true) - .where("roles.in_discord", "==", false) - .get(); - return usersNotInDiscordServer; - } catch (error) { - logger.error(`Error in getting users who are not in discord server: ${error}`); - throw error; - } -}; - module.exports = { archiveUsers, addOrUpdate, @@ -1128,5 +1114,4 @@ module.exports = { fetchUserForKeyValue, getNonNickNameSyncedUsers, updateUsersWithNewUsernames, - fetchUsersNotInDiscordServer, }; diff --git a/routes/users.js b/routes/users.js index 1ce4fc008..94d301cda 100644 --- a/routes/users.js +++ b/routes/users.js @@ -12,9 +12,7 @@ const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndSe const ROLES = require("../constants/roles"); const { Services } = require("../constants/bot"); const authenticateProfile = require("../middlewares/authenticateProfile"); -const { devFlagMiddleware } = require("../middlewares/devFlag"); -router.get("/departed-users", devFlagMiddleware, users.getUsersWithAbandonedTasks); router.post("/", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), users.markUnverified); router.post("/update-in-discord", authenticate, authorizeRoles([SUPERUSER]), users.setInDiscordScript); router.post("/verify", authenticate, users.verifyUser); diff --git a/services/users.js b/services/users.js index 764480203..cf8fa4156 100644 --- a/services/users.js +++ b/services/users.js @@ -1,20 +1,14 @@ const firestore = require("../utils/firestore"); const { formatUsername } = require("../utils/username"); const userModel = firestore.collection("users"); -const { fetchUsersNotInDiscordServer } = require("../models/users"); -const { fetchIncompleteTaskForUser } = require("../models/tasks"); +const tasksQuery = require("../models/tasks"); -const getUsersWithIncompleteTasks = async () => { +const getUsersWithIncompleteTasks = async (users) => { + if (users.length === 0) return []; try { const eligibleUsersWithTasks = []; - - const userSnapshot = await fetchUsersNotInDiscordServer(); - - for (const userDoc of userSnapshot.docs) { - const user = userDoc.data(); - - const abandonedTasksQuerySnapshot = await fetchIncompleteTaskForUser(user.id); - + for (const user of users) { + const abandonedTasksQuerySnapshot = await tasksQuery.fetchIncompleteTaskForUser(user.id); if (!abandonedTasksQuerySnapshot.empty) { eligibleUsersWithTasks.push(user); } diff --git a/test/integration/users.test.js b/test/integration/users.test.js index d847d3fce..4c7143b8d 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -1447,6 +1447,56 @@ describe("Users", function () { }); }); + describe("GET /users?departed", function () { + beforeEach(async function () { + await cleanDb(); + const userPromises = abandonedUsersData.map((user) => userModel.doc(user.id).set(user)); + await Promise.all(userPromises); + + const taskPromises = abandonedTasksData.map((task) => taskModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + Sinon.restore(); + await cleanDb(); + }); + + it("should return a list of users with abandoned tasks", async function () { + const res = await chai.request(app).get("/users?dev=true&departed=true"); + expect(res).to.have.status(200); + expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully"); + expect(res.body).to.have.property("users").to.be.an("array").with.lengthOf(2); + }); + + it("should return an empty array when no users have abandoned tasks", async function () { + await cleanDb(); + const user = abandonedUsersData[2]; + await userModel.add(user); + + const task = abandonedTasksData[3]; + await taskModel.add(task); + const res = await chai.request(app).get("/users?dev=true&departed=true"); + + expect(res).to.have.status(204); + }); + + it("should fail if dev flag is not passed", async function () { + const res = await chai.request(app).get("/users?departed=true"); + expect(res).to.have.status(404); + expect(res.body.message).to.be.equal("Route not found"); + }); + + it("should handle errors gracefully if getUsersWithIncompleteTasks fails", async function () { + Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(INTERNAL_SERVER_ERROR)); + + const res = await chai.request(app).get("/users?departed=true&dev=true"); + + expect(res).to.have.status(500); + expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR); + }); + }); + describe("PUT /users/self/intro", function () { let userStatusData; @@ -2652,55 +2702,4 @@ describe("Users", function () { expect(res).to.have.status(401); }); }); - - describe("GET /users/departed-users", function () { - beforeEach(async function () { - await cleanDb(); - const userPromises = abandonedUsersData.map((user) => userModel.add(user)); - await Promise.all(userPromises); - - const taskPromises = abandonedTasksData.map((task) => taskModel.add(task)); - await Promise.all(taskPromises); - }); - - afterEach(async function () { - Sinon.restore(); - await cleanDb(); - }); - - it("should return a list of users with abandoned tasks", async function () { - const res = await chai.request(app).get("/users/departed-users?dev=true"); - - expect(res).to.have.status(200); - expect(res.body).to.have.property("message").that.equals("Users with abandoned tasks fetched successfully"); - expect(res.body.data).to.be.an("array").with.lengthOf(2); - }); - - it("should return an empty array when no users have abandoned tasks", async function () { - await cleanDb(); - const user = abandonedUsersData[2]; - await userModel.add(user); - - const task = abandonedTasksData[3]; - await taskModel.add(task); - const res = await chai.request(app).get("/users/departed-users?dev=true"); - - expect(res).to.have.status(204); - }); - - it("should fail if dev flag is not passed", async function () { - const res = await chai.request(app).get("/users/departed-users"); - expect(res).to.have.status(404); - expect(res.body.message).to.be.equal("Route not found"); - }); - - it("should handle errors gracefully if the database query fails", async function () { - Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(INTERNAL_SERVER_ERROR)); - - const res = await chai.request(app).get("/users/departed-users?dev=true"); - - expect(res).to.have.status(500); - expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR); - }); - }); }); diff --git a/test/unit/models/users.test.js b/test/unit/models/users.test.js index bea6f15aa..b0092f57c 100644 --- a/test/unit/models/users.test.js +++ b/test/unit/models/users.test.js @@ -529,7 +529,7 @@ describe("users", function () { }); }); - describe("fetchUsersNotInDiscordServer", function () { + describe("fetchPaginatedUsers - Departed Users", function () { beforeEach(async function () { await cleanDb(); @@ -543,24 +543,29 @@ describe("users", function () { }); it("should fetch users not in discord server", async function () { - const usersNotInDiscordServer = await users.fetchUsersNotInDiscordServer(); - expect(usersNotInDiscordServer.docs.length).to.be.equal(2); + const result = await users.fetchPaginatedUsers({ departed: "true" }); + expect(result.allUsers.length).to.be.equal(2); }); - it("should return an empty array if there are no users in the database", async function () { + it("should return no users if departed flag is false", async function () { + const result = await users.fetchPaginatedUsers({ departed: "false" }); + expect(result.allUsers.length).to.be.equal(0); + }); + + it("should return an empty array if there are no departed users in the database", async function () { await cleanDb(); const activeUser = abandonedUsersData[2]; await userModel.add(activeUser); - const usersNotInDiscordServer = await users.fetchUsersNotInDiscordServer(); - expect(usersNotInDiscordServer.docs.length).to.be.equal(0); + const result = await users.fetchPaginatedUsers({ departed: "true" }); + expect(result.allUsers.length).to.be.equal(0); }); it("should handle errors gracefully if the database query fails", async function () { - sinon.stub(users, "fetchUsersNotInDiscordServer").throws(new Error("Database query failed")); + sinon.stub(users, "fetchPaginatedUsers").throws(new Error("Database query failed")); try { - await users.fetchUsersNotInDiscordServer(); + await users.fetchPaginatedUsers(); expect.fail("Expected function to throw an error"); } catch (error) { expect(error.message).to.equal("Database query failed"); diff --git a/test/unit/services/dataAccessLayer.test.js b/test/unit/services/dataAccessLayer.test.js index b55c8475f..3d4748e4f 100644 --- a/test/unit/services/dataAccessLayer.test.js +++ b/test/unit/services/dataAccessLayer.test.js @@ -72,6 +72,7 @@ describe("Data Access Layer", function () { expect(user).to.not.have.property(key); }); }); + fetchUserStub.restore(); }); it("should return /users/self data and remove sensitive info", async function () { @@ -84,6 +85,83 @@ describe("Data Access Layer", function () { }); }); + describe("retrieveUsers - Departed Users", function () { + let fetchPaginatedUsersStub; + + beforeEach(function () { + fetchPaginatedUsersStub = sinon.stub(userQuery, "fetchPaginatedUsers"); + }); + + afterEach(function () { + sinon.restore(); + }); + + it("should fetch departed users when departed flag is true", async function () { + const departedUser = { + ...userData[12], + roles: { archived: true, in_discord: false }, + }; + fetchPaginatedUsersStub.returns( + Promise.resolve({ + allUsers: [departedUser], + nextId: 3, + prevId: 1, + }) + ); + + const result = await retrieveUsers({ query: { departed: "true" } }); + + expect(result.users.length).to.be.equal(1); + expect(result.users[0].roles.archived).to.equal(true); + expect(result.users[0].roles.in_discord).to.equal(false); + KEYS_NOT_ALLOWED[ACCESS_LEVEL.PUBLIC].forEach((key) => { + expect(result.users[0]).to.not.have.property(key); + }); + }); + + it("should ignore departed parameter when departed flag is not passed or passed as false", async function () { + fetchPaginatedUsersStub.returns( + Promise.resolve({ + allUsers: [], + nextId: "", + prevId: "", + }) + ); + + const result = await retrieveUsers({ query: { departed: "false" } }); + + expect(result.users.length).to.be.equal(0); + expect(result.nextId).to.equal(""); + expect(result.prevId).to.equal(""); + }); + + it("should handle empty result for departed users query", async function () { + fetchPaginatedUsersStub.returns( + Promise.resolve({ + allUsers: [], + nextId: "", + prevId: "", + }) + ); + const result = await retrieveUsers({ query: { departed: "true" } }); + expect(result.users.length).to.be.equal(0); + expect(result.nextId).to.equal(""); + expect(result.prevId).to.equal(""); + }); + + it("should handle database errors properly", async function () { + fetchPaginatedUsersStub.rejects(new Error("Database connection failed")); + + try { + await retrieveUsers({ query: { departed: true } }); + expect.fail("Expected function to throw an error"); + } catch (error) { + expect(error).to.be.an("error"); + expect(error.message).to.equal("Database connection failed"); + } + }); + }); + describe("retrieveDiscordUsers", function () { it("should fetch discord users and remove sensitive info", async function () { const fetchUserStub = sinon.stub(userQuery, "getDiscordUsers"); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index 6011a0ca8..501c3d1a7 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -12,6 +12,7 @@ const { usersData: abandonedUsersData, tasksData: abandonedTasksData, } = require("../../fixtures/abandoned-tasks/departed-users"); +const tasks = require("../../../models/tasks"); describe("Users services", function () { describe("archive inactive discord users in bulk", function () { @@ -110,9 +111,6 @@ describe("Users services", function () { beforeEach(async function () { await cleanDb(); - const userPromises = abandonedUsersData.map((user) => userModel.add(user)); - await Promise.all(userPromises); - const taskPromises = abandonedTasksData.map((task) => tasksModel.add(task)); await Promise.all(taskPromises); }); @@ -122,16 +120,18 @@ describe("Users services", function () { }); it("should fetch users with abandoned tasks", async function () { - const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); + const users = abandonedUsersData.slice(0, 2); + const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(users); expect(usersWithAbandonedTasks).to.be.an("array"); expect(usersWithAbandonedTasks).to.have.lengthOf(2); }); it("should not include user who are present in discord or not archived", async function () { - const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); + const users = abandonedUsersData.slice(0, 2); + const result = await getUsersWithIncompleteTasks(users); - usersWithAbandonedTasks.forEach((user) => { + result.forEach((user) => { expect(user.roles.in_discord).to.not.equal(true); expect(user.roles.archived).to.not.equal(false); }); @@ -140,14 +140,25 @@ describe("Users services", function () { it("should return an empty array if there are no users with abandoned tasks", async function () { await cleanDb(); - const activeUser = abandonedUsersData[2]; - await userModel.add(activeUser); - const activeTask = abandonedTasksData[3]; await tasksModel.add(activeTask); - const usersWithAbandonedTasks = await getUsersWithIncompleteTasks(); - expect(usersWithAbandonedTasks).to.be.an("array"); - expect(usersWithAbandonedTasks).to.have.lengthOf(0); + + const result = await getUsersWithIncompleteTasks([]); + expect(result).to.be.an("array"); + expect(result).to.have.lengthOf(0); + }); + + it("should throw an error if fetchIncompleteTaskForUser fails", async function () { + const users = abandonedUsersData.slice(0, 2); + Sinon.stub(tasks, "fetchIncompleteTaskForUser").throws(new Error("Database query failed")); + + try { + await getUsersWithIncompleteTasks([users]); + expect.fail("Expected function to throw an error"); + } catch (error) { + expect(error.message).to.equal("Database query failed"); + } + Sinon.restore(); }); }); }); From c03ce968cbe4c242abc09236dc34a4ad33f05e85 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Sun, 24 Nov 2024 18:42:04 +0530 Subject: [PATCH 9/9] chore: Added early return for departed not having dev flag and fixed 204 status return. --- controllers/users.js | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 1307d432c..c6cb0375e 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -195,27 +195,25 @@ const getUsers = async (req, res) => { const isDeparted = req.query.departed === "true"; if (isDeparted) { - let result; - if (dev) { - try { - result = await dataAccess.retrieveUsers({ query: req.query }); - const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); - if (departedUsers.length === 0) res.status(204).send(); - return res.json({ - message: "Users with abandoned tasks fetched successfully", - users: departedUsers, - links: { - next: result.nextId ? getPaginationLink(req.query, "next", result.nextId) : "", - prev: result.prevId ? getPaginationLink(req.query, "prev", result.prevId) : "", - }, - }); - } catch (error) { - logger.error("Error when fetching users who abandoned tasks:", error); - return res.boom.badImplementation(INTERNAL_SERVER_ERROR); - } - } else { + if (!dev) { return res.boom.notFound("Route not found"); } + try { + const result = await dataAccess.retrieveUsers({ query: req.query }); + const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); + if (departedUsers.length === 0) return res.status(204).send(); + return res.json({ + message: "Users with abandoned tasks fetched successfully", + users: departedUsers, + links: { + next: result.nextId ? getPaginationLink(req.query, "next", result.nextId) : "", + prev: result.prevId ? getPaginationLink(req.query, "prev", result.prevId) : "", + }, + }); + } catch (error) { + logger.error("Error when fetching users who abandoned tasks:", error); + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } } if (transformedQuery?.filterBy === OVERDUE_TASKS) {