From 36e067df2bd0346cb24c5d72db6b688cf1db91a0 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Tue, 26 Nov 2024 00:45:05 +0530 Subject: [PATCH 1/3] feat: Departed users api changes. --- controllers/users.js | 25 +++++++++++++ middlewares/validators/user.js | 1 + models/tasks.js | 21 +++++++++++ models/users.js | 65 +++++++++++++++++++++++++++++++--- services/users.js | 52 ++++++++------------------- 5 files changed, 122 insertions(+), 42 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 1fa81c282..c6cb0375e 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -30,6 +30,7 @@ const { addLog } = require("../models/logs"); const { getUserStatus } = require("../models/userStatus"); const config = require("config"); const { generateUniqueUsername } = require("../services/users"); +const userService = require("../services/users"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const verifyUser = async (req, res) => { @@ -191,6 +192,30 @@ const getUsers = async (req, res) => { } } + const isDeparted = req.query.departed === "true"; + + if (isDeparted) { + 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) { try { const tasksData = await getOverdueTasks(days); 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/tasks.js b/models/tasks.js index 8b0754b1b..c9fa3885c 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -701,6 +701,26 @@ const markUnDoneTasksOfArchivedUsersBacklog = async (users) => { } }; +/** + * 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", "==", userId) + .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 +740,5 @@ module.exports = { updateTaskStatus, updateOrphanTasksStatus, markUnDoneTasksOfArchivedUsersBacklog, + fetchIncompleteTaskForUser, }; diff --git a/models/users.js b/models/users.js index e0746de92..88a65a704 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,6 +31,52 @@ 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 = []; + 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 * @@ -182,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 @@ -195,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]; @@ -217,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"); @@ -1031,6 +1085,7 @@ const updateUsersWithNewUsernames = async () => { }; module.exports = { + archiveUsers, addOrUpdate, fetchPaginatedUsers, fetchUser, diff --git a/services/users.js b/services/users.js index bb1d609b6..cf8fa4156 100644 --- a/services/users.js +++ b/services/users.js @@ -1,44 +1,22 @@ -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 tasksQuery = require("../models/tasks"); +const getUsersWithIncompleteTasks = async (users) => { + if (users.length === 0) return []; 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 = []; + for (const user of users) { + const abandonedTasksQuerySnapshot = await tasksQuery.fetchIncompleteTaskForUser(user.id); + if (!abandonedTasksQuerySnapshot.empty) { + eligibleUsersWithTasks.push(user); + } + } + return eligibleUsersWithTasks; + } catch (error) { + logger.error(`Error in getting users who abandoned tasks: ${error}`); + throw error; } }; @@ -63,6 +41,6 @@ const generateUniqueUsername = async (firstName, lastName) => { }; module.exports = { - archiveUsers, generateUniqueUsername, + getUsersWithIncompleteTasks, }; From 30983e44f4183f605a7c620954ff54d4e9ec80bc Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Tue, 26 Nov 2024 00:55:25 +0530 Subject: [PATCH 2/3] feat: Added test cases for departed users api changes. --- .../abandoned-tasks/departed-users.js | 154 ++++++++++++++++++ test/integration/users.test.js | 58 ++++++- test/unit/models/tasks.test.js | 42 +++++ test/unit/models/users.test.js | 45 +++++ test/unit/services/users.test.js | 65 +++++++- 5 files changed, 361 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/abandoned-tasks/departed-users.js 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/integration/users.test.js b/test/integration/users.test.js index 708b87d2f..4c7143b8d 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -41,7 +41,13 @@ 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"); +const userService = require("../../services/users"); chai.use(chaiHttp); describe("Users", function () { @@ -1441,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; diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index 44a560fd5..530aba923 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,42 @@ 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 () { + 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 () { + await cleanDb(); + 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 42165094f..b0092f57c 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,48 @@ describe("users", function () { expect(userDoc.user.roles.super_user).to.be.equal(false); }); }); + + describe("fetchPaginatedUsers - Departed Users", function () { + beforeEach(async function () { + await cleanDb(); + + const userPromises = abandonedUsersData.map((user) => userModel.add(user)); + await Promise.all(userPromises); + }); + + afterEach(async function () { + await cleanDb(); + sinon.restore(); + }); + + it("should fetch users not in discord server", async function () { + const result = await users.fetchPaginatedUsers({ departed: "true" }); + expect(result.allUsers.length).to.be.equal(2); + }); + + 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 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, "fetchPaginatedUsers").throws(new Error("Database query failed")); + + try { + 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/users.test.js b/test/unit/services/users.test.js index 60cf04c9d..501c3d1a7 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -3,10 +3,16 @@ 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"); +const tasks = require("../../../models/tasks"); describe("Users services", function () { describe("archive inactive discord users in bulk", function () { @@ -100,4 +106,59 @@ describe("Users services", function () { expect(newUsername).to.equal("john-doe-1"); }); }); + + describe("fetchUsersWithAbandonedTasks", function () { + beforeEach(async function () { + await cleanDb(); + + 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 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 users = abandonedUsersData.slice(0, 2); + const result = await getUsersWithIncompleteTasks(users); + + result.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(); + + const activeTask = abandonedTasksData[3]; + await tasksModel.add(activeTask); + + 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 e11d4b7f15bb7977b2ea1dcc46484729f644a667 Mon Sep 17 00:00:00 2001 From: VinuB-Dev Date: Thu, 28 Nov 2024 11:44:28 +0530 Subject: [PATCH 3/3] Refactor: Update fetchIncompleteTasksByUserIds to use batch queries and filter in-memory - Replaced Firestore query with batch query to fetch incomplete tasks for multiple users at once. - Filter tasks by user IDs and completed statuses in-memory for improved efficiency. - Updated return structure to return an array directly instead of an object with `docs` property. - Updated test cases related to the same. --- models/tasks.js | 29 ++++++++++++++++++----------- services/users.js | 22 ++++++++++++++-------- test/unit/models/tasks.test.js | 14 +++++++------- test/unit/services/users.test.js | 2 +- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/models/tasks.js b/models/tasks.js index c9fa3885c..ae7665758 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -702,21 +702,28 @@ const markUnDoneTasksOfArchivedUsersBacklog = async (users) => { }; /** - * 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. + * Fetches all incomplete tasks for given user IDs. + * + * @param {string[]} userIds - The IDs of the users to fetch incomplete tasks for. + * @returns {Promise} - The query snapshot object. * @throws {Error} - Throws an error if the database query fails. */ -const fetchIncompleteTaskForUser = async (userId) => { +const fetchIncompleteTasksByUserIds = async (userIds) => { const COMPLETED_STATUSES = [DONE, COMPLETED]; + + if (!userIds || userIds.length === 0) { + return []; + } try { - const incompleteTaskForUser = await tasksModel - .where("assigneeId", "==", userId) - .where("status", "not-in", COMPLETED_STATUSES) - .get(); - return incompleteTaskForUser; + const incompleteTasksQuery = await tasksModel.where("assigneeId", "in", userIds).get(); + + const incompleteTaskForUsers = incompleteTasksQuery.docs.filter( + (task) => !COMPLETED_STATUSES.includes(task.data().status) + ); + + return incompleteTaskForUsers; } catch (error) { - logger.error("Error when fetching incomplete tasks:", error); + logger.error("Error when fetching incomplete tasks for users:", error); throw error; } }; @@ -740,5 +747,5 @@ module.exports = { updateTaskStatus, updateOrphanTasksStatus, markUnDoneTasksOfArchivedUsersBacklog, - fetchIncompleteTaskForUser, + fetchIncompleteTasksByUserIds, }; diff --git a/services/users.js b/services/users.js index cf8fa4156..240f5c507 100644 --- a/services/users.js +++ b/services/users.js @@ -1,21 +1,27 @@ const firestore = require("../utils/firestore"); const { formatUsername } = require("../utils/username"); const userModel = firestore.collection("users"); -const tasksQuery = require("../models/tasks"); +const tasksModel = require("../models/tasks"); const getUsersWithIncompleteTasks = async (users) => { if (users.length === 0) return []; + try { - const eligibleUsersWithTasks = []; - for (const user of users) { - const abandonedTasksQuerySnapshot = await tasksQuery.fetchIncompleteTaskForUser(user.id); - if (!abandonedTasksQuerySnapshot.empty) { - eligibleUsersWithTasks.push(user); - } + const userIds = users.map((user) => user.id); + + const abandonedTasksQuerySnapshot = await tasksModel.fetchIncompleteTasksByUserIds(userIds); + + if (abandonedTasksQuerySnapshot.empty) { + return []; } + + const userIdsWithIncompleteTasks = new Set(abandonedTasksQuerySnapshot.map((doc) => doc.data().assigneeId)); + + const eligibleUsersWithTasks = users.filter((user) => userIdsWithIncompleteTasks.has(user.id)); + return eligibleUsersWithTasks; } catch (error) { - logger.error(`Error in getting users who abandoned tasks: ${error}`); + logger.error(`Error in getting users who abandoned tasks: ${error}`); throw error; } }; diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index 530aba923..b88901936 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -371,23 +371,23 @@ describe("tasks", function () { }); it("should fetch tasks which are incomplete for the given user", async function () { - const inactiveUser = abandonedUsersData[0]; - const incompleteTasks = await tasks.fetchIncompleteTaskForUser(inactiveUser.id); - expect(incompleteTasks.docs.length).to.be.equal(1); + const userIds = abandonedUsersData.map((user) => user.id); + const incompleteTasks = await tasks.fetchIncompleteTasksByUserIds(userIds); + expect(incompleteTasks.length).to.be.equal(3); }); 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); + const incompleteTasks = await tasks.fetchIncompleteTasksByUserIds([activeUser.id]); + expect(incompleteTasks.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")); + sinon.stub(tasks, "fetchIncompleteTasksByUserIds").throws(new Error("Database query failed")); try { - await tasks.fetchIncompleteTaskForUser(); + await tasks.fetchIncompleteTasksByUserIds(); 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 501c3d1a7..f0be625ef 100644 --- a/test/unit/services/users.test.js +++ b/test/unit/services/users.test.js @@ -150,7 +150,7 @@ describe("Users services", function () { 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")); + Sinon.stub(tasks, "fetchIncompleteTasksByUserIds").throws(new Error("Database query failed")); try { await getUsersWithIncompleteTasks([users]);