From 41e6a5c93935aea40a598e0cae0e187ca1e0adaa Mon Sep 17 00:00:00 2001 From: Vignesh B S <55937928+VinuB-Dev@users.noreply.github.com> Date: Sat, 30 Nov 2024 23:10:14 +0530 Subject: [PATCH] Feat: Implement API for Tracking Orphaned Tasks. (#2267) * feat: orphaned tasks API changes. * feat: Added tests for orphaned tasks. * chore: Returned without a block for usersnaptop empty and added spacing to make it more readable. * Refactor: Update fetchOrphanedTasks to use batch query for incomplete tasks - Replaced individual queries with `fetchIncompleteTasksByUserIds` to fetch tasks in batch for users not in the Discord server. - Improved performance by reducing the number of database queries. - Modified testcases based on the updates. * fix: Changed abandoned tasks to orphaned tasks. * Fix: Changed the validation type of orphaned to boolean instead of string. --- controllers/tasks.js | 37 ++++- middlewares/validators/tasks.js | 1 + models/tasks.js | 66 +++++++- models/users.js | 19 +++ services/tasks.js | 54 +++--- .../abandoned-tasks/departed-users.js | 154 ++++++++++++++++++ test/integration/tasks.test.js | 60 ++++++- test/unit/models/tasks.test.js | 43 +++++ test/unit/models/users.test.js | 41 +++++ test/unit/services/tasks.test.js | 73 ++++++++- 10 files changed, 516 insertions(+), 32 deletions(-) create mode 100644 test/fixtures/abandoned-tasks/departed-users.js diff --git a/controllers/tasks.js b/controllers/tasks.js index 9e36c639c..ca1557a7e 100644 --- a/controllers/tasks.js +++ b/controllers/tasks.js @@ -13,6 +13,7 @@ const { updateUserStatusOnTaskUpdate, updateStatusOnTaskCompletion } = require(" const dataAccess = require("../services/dataAccessLayer"); const { parseSearchQuery } = require("../utils/tasks"); const { addTaskCreatedAtAndUpdatedAtFields } = require("../services/tasks"); +const tasksService = require("../services/tasks"); const { RQLQueryParser } = require("../utils/RQLParser"); const { getMissedProgressUpdatesUsers } = require("../models/discordactions"); const { logType } = require("../constants/logs"); @@ -134,7 +135,19 @@ const fetchPaginatedTasks = async (query) => { const fetchTasks = async (req, res) => { try { - const { status, page, size, prev, next, q: queryString, assignee, title, userFeatureFlag } = req.query; + const { + status, + page, + size, + prev, + next, + q: queryString, + assignee, + title, + userFeatureFlag, + orphaned, + dev, + } = req.query; const transformedQuery = transformQuery(status, size, page, assignee, title); if (queryString !== undefined) { @@ -159,6 +172,28 @@ const fetchTasks = async (req, res) => { }); } + const isOrphaned = orphaned === "true"; + const isDev = dev === "true"; + if (isOrphaned) { + if (!isDev) { + return res.boom.notFound("Route not found"); + } + try { + const orphanedTasks = await tasksService.fetchOrphanedTasks(); + if (!orphanedTasks || orphanedTasks.length === 0) { + return res.sendStatus(204); + } + const tasksWithRdsAssigneeInfo = await fetchTasksWithRdsAssigneeInfo(orphanedTasks); + return res.status(200).json({ + message: "Orphan tasks fetched successfully", + data: tasksWithRdsAssigneeInfo, + }); + } catch (error) { + logger.error("Error in getting tasks which were orphaned", error); + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } + } + const paginatedTasks = await fetchPaginatedTasks({ ...transformedQuery, prev, next, userFeatureFlag }); return res.json({ message: "Tasks returned successfully!", diff --git a/middlewares/validators/tasks.js b/middlewares/validators/tasks.js index 5467137c8..d36677d5f 100644 --- a/middlewares/validators/tasks.js +++ b/middlewares/validators/tasks.js @@ -193,6 +193,7 @@ const getTasksValidator = async (req, res, next) => { return value; }, "Invalid query format"), userFeatureFlag: joi.string().optional(), + orphaned: joi.boolean().optional(), }); try { diff --git a/models/tasks.js b/models/tasks.js index 8b0754b1b..e532a59a0 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -4,7 +4,6 @@ const userModel = firestore.collection("users"); const ItemModel = firestore.collection("itemTags"); const dependencyModel = firestore.collection("taskDependencies"); const userUtils = require("../utils/users"); -const { updateTaskStatusToDone } = require("../services/tasks"); const { chunks } = require("../utils/array"); const { DOCUMENT_WRITE_SIZE } = require("../constants/constants"); const { fromFirestoreData, toFirestoreData, buildTasks } = require("../utils/tasks"); @@ -24,6 +23,42 @@ const { const { OLD_ACTIVE, OLD_BLOCKED, OLD_PENDING, OLD_COMPLETED } = TASK_STATUS_OLD; const { INTERNAL_SERVER_ERROR } = require("../constants/errorMessages"); +/** + * Update multiple tasks' status to DONE in one batch operation. + * @param {Object[]} tasksData - Tasks data to update, must contain 'id' and 'status' fields. + * @returns {Object} - Summary of the batch operation. + * @property {number} totalUpdatedStatus - Number of tasks that has their status updated to DONE. + * @property {number} totalOperationsFailed - Number of tasks that failed to update. + * @property {string[]} updatedTaskDetails - IDs of tasks that has their status updated to DONE. + * @property {string[]} failedTaskDetails - IDs of tasks that failed to update. + */ +const updateTaskStatusToDone = async (tasksData) => { + const batch = firestore.batch(); + const tasksBatch = []; + const summary = { + totalUpdatedStatus: 0, + totalOperationsFailed: 0, + updatedTaskDetails: [], + failedTaskDetails: [], + }; + tasksData.forEach((task) => { + const updateTaskData = { ...task, status: "DONE" }; + batch.update(tasksModel.doc(task.id), updateTaskData); + tasksBatch.push(task.id); + }); + try { + await batch.commit(); + summary.totalUpdatedStatus += tasksData.length; + summary.updatedTaskDetails = [...tasksBatch]; + return { ...summary }; + } catch (err) { + logger.error("Firebase batch Operation Failed!"); + summary.totalOperationsFailed += tasksData.length; + summary.failedTaskDetails = [...tasksBatch]; + return { ...summary }; + } +}; + /** * Adds and Updates tasks * @@ -701,6 +736,33 @@ const markUnDoneTasksOfArchivedUsersBacklog = async (users) => { } }; +/** + * 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 fetchIncompleteTasksByUserIds = async (userIds) => { + const COMPLETED_STATUSES = [DONE, COMPLETED]; + + if (!userIds || userIds.length === 0) { + return []; + } + try { + const incompleteTasksQuery = await tasksModel.where("assigneeId", "in", userIds).get(); + + const incompleteTaskForUsers = incompleteTasksQuery.docs.filter((task) => { + return !COMPLETED_STATUSES.includes(task.data().status); + }); + + return incompleteTaskForUsers; + } catch (error) { + logger.error("Error when fetching incomplete tasks for users:", error); + throw error; + } +}; + module.exports = { updateTask, fetchTasks, @@ -720,4 +782,6 @@ module.exports = { updateTaskStatus, updateOrphanTasksStatus, markUnDoneTasksOfArchivedUsersBacklog, + updateTaskStatusToDone, + fetchIncompleteTasksByUserIds, }; diff --git a/models/users.js b/models/users.js index e0746de92..baa11144a 100644 --- a/models/users.js +++ b/models/users.js @@ -1030,6 +1030,24 @@ 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 + .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 = { addOrUpdate, fetchPaginatedUsers, @@ -1059,4 +1077,5 @@ module.exports = { fetchUserForKeyValue, getNonNickNameSyncedUsers, updateUsersWithNewUsernames, + fetchUsersNotInDiscordServer, }; diff --git a/services/tasks.js b/services/tasks.js index 6e1a5dfe7..370e381ec 100644 --- a/services/tasks.js +++ b/services/tasks.js @@ -2,33 +2,8 @@ const firestore = require("../utils/firestore"); const tasksModel = firestore.collection("tasks"); const { chunks } = require("../utils/array"); const { DOCUMENT_WRITE_SIZE: FIRESTORE_BATCH_OPERATIONS_LIMIT } = require("../constants/constants"); - -const updateTaskStatusToDone = async (tasksData) => { - const batch = firestore.batch(); - const tasksBatch = []; - const summary = { - totalUpdatedStatus: 0, - totalOperationsFailed: 0, - updatedTaskDetails: [], - failedTaskDetails: [], - }; - tasksData.forEach((task) => { - const updateTaskData = { ...task, status: "DONE" }; - batch.update(tasksModel.doc(task.id), updateTaskData); - tasksBatch.push(task.id); - }); - try { - await batch.commit(); - summary.totalUpdatedStatus += tasksData.length; - summary.updatedTaskDetails = [...tasksBatch]; - return { ...summary }; - } catch (err) { - logger.error("Firebase batch Operation Failed!"); - summary.totalOperationsFailed += tasksData.length; - summary.failedTaskDetails = [...tasksBatch]; - return { ...summary }; - } -}; +const usersQuery = require("../models/users"); +const tasksQuery = require("../models/tasks"); const addTaskCreatedAtAndUpdatedAtFields = async () => { const operationStats = { @@ -83,7 +58,30 @@ const addTaskCreatedAtAndUpdatedAtFields = async () => { return operationStats; }; +const fetchOrphanedTasks = async () => { + try { + const userSnapshot = await usersQuery.fetchUsersNotInDiscordServer(); + + if (userSnapshot.empty) return []; + + const userIds = userSnapshot.docs.map((doc) => doc.id); + + const orphanedTasksQuerySnapshot = await tasksQuery.fetchIncompleteTasksByUserIds(userIds); + + if (orphanedTasksQuerySnapshot.empty) { + return []; + } + + const orphanedTasks = orphanedTasksQuerySnapshot.map((doc) => doc.data()); + + return orphanedTasks; + } catch (error) { + logger.error(`Error in getting tasks abandoned by users: ${error}`); + throw error; + } +}; + module.exports = { - updateTaskStatusToDone, addTaskCreatedAtAndUpdatedAtFields, + fetchOrphanedTasks, }; 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/tasks.test.js b/test/integration/tasks.test.js index 6a8442875..74d691e4d 100644 --- a/test/integration/tasks.test.js +++ b/test/integration/tasks.test.js @@ -25,7 +25,8 @@ const userDBModel = firestore.collection("users"); const discordService = require("../../services/discordService"); const { CRON_JOB_HANDLER } = require("../../constants/bot"); const { logType } = require("../../constants/logs"); - +const { INTERNAL_SERVER_ERROR } = require("../../constants/errorMessages"); +const tasksService = require("../../services/tasks"); chai.use(chaiHttp); const appOwner = userData[3]; @@ -37,6 +38,10 @@ const { stubbedModelTaskProgressData } = require("../fixtures/progress/progresse const { convertDaysToMilliseconds } = require("../../utils/time"); const { getDiscordMembers } = require("../fixtures/discordResponse/discord-response"); const { generateCronJobToken } = require("../utils/generateBotToken"); +const { + usersData: abandonedUsersData, + tasksData: abandonedTasksData, +} = require("../fixtures/abandoned-tasks/departed-users"); const taskData = [ { @@ -1633,4 +1638,57 @@ describe("Tasks", function () { }); }); }); + + describe("GET /tasks?orphaned", function () { + beforeEach(async function () { + await cleanDb(); + const userPromises = abandonedUsersData.map((user) => userDBModel.doc(user.id).set(user)); + await Promise.all(userPromises); + + const taskPromises = abandonedTasksData.map((task) => tasksModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + sinon.restore(); + await cleanDb(); + }); + + it("should return 204 status when no users are archived", async function () { + await cleanDb(); + + const user = abandonedUsersData[2]; + await userDBModel.add(user); + + const task = abandonedTasksData[3]; + await tasksModel.add(task); + + const res = await chai.request(app).get("/tasks?dev=true&orphaned=true").set("Accept", "application/json"); + + expect(res).to.have.status(204); + }); + + it("should fetch tasks assigned to archived and non-discord users", async function () { + const res = await chai.request(app).get("/tasks?dev=true&orphaned=true"); + + expect(res).to.have.status(200); + expect(res.body).to.have.property("message").that.equals("Orphan tasks fetched successfully"); + expect(res.body.data).to.be.an("array").with.lengthOf(2); + }); + + it("should fail if dev flag is not passed", async function () { + const res = await chai.request(app).get("/tasks?orphaned=true"); + 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(tasksService, "fetchOrphanedTasks").rejects(new Error(INTERNAL_SERVER_ERROR)); + + const res = await chai.request(app).get("/tasks?orphaned=true&dev=true"); + + expect(res).to.have.status(500); + expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR); + }); + }); }); diff --git a/test/unit/models/tasks.test.js b/test/unit/models/tasks.test.js index 44a560fd5..2b5f76449 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("fetchIncompleteTasksByUserIds", 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 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.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, "fetchIncompleteTasksByUserIds").throws(new Error("Database query failed")); + + try { + 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/models/users.test.js b/test/unit/models/users.test.js index 42165094f..c3dd691ac 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,44 @@ describe("users", function () { expect(userDoc.user.roles.super_user).to.be.equal(false); }); }); + + describe("fetchUsersNotInDiscordServer", function () { + beforeEach(async function () { + await cleanDb(); + + 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(); + + const activeUser = abandonedUsersData[2]; + 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/tasks.test.js b/test/unit/services/tasks.test.js index 02d424134..278180cde 100644 --- a/test/unit/services/tasks.test.js +++ b/test/unit/services/tasks.test.js @@ -3,9 +3,16 @@ const { expect } = require("chai"); const firestore = require("../../../utils/firestore"); const tasksModel = firestore.collection("tasks"); +const userModel = firestore.collection("users"); const cleanDb = require("../../utils/cleanDb"); const taskDataArray = require("../../fixtures/tasks/tasks")(); -const { updateTaskStatusToDone } = require("../../../services/tasks"); +const { fetchOrphanedTasks } = require("../../../services/tasks"); +const { + usersData: abandonedUsersData, + tasksData: abandonedTasksData, +} = require("../../fixtures/abandoned-tasks/departed-users"); +const { updateTaskStatusToDone } = require("../../../models/tasks"); +const tasksQuery = require("../../../models/tasks"); describe("Tasks services", function () { describe("task status COMPLETED to DONE in bulk", function () { @@ -72,4 +79,68 @@ describe("Tasks services", function () { }); }); }); + + describe("fetchOrphanedTasks", 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) => tasksModel.add(task)); + await Promise.all(taskPromises); + }); + + afterEach(async function () { + await cleanDb(); + }); + + it("should fetch tasks assigned to archived and non-discord users", async function () { + const orphanedTasks = await fetchOrphanedTasks(); + expect(orphanedTasks).to.be.an("array"); + expect(orphanedTasks).to.have.lengthOf(2); + }); + + it("should not include completed or done tasks", async function () { + const orphanedTasks = await fetchOrphanedTasks(); + + orphanedTasks.forEach((task) => { + expect(task.status).to.not.be.oneOf(["DONE", "COMPLETED"]); + }); + }); + + it("should not include tasks from active users", async function () { + const orphanedTasks = await fetchOrphanedTasks(); + + orphanedTasks.forEach((task) => { + expect(task.assignee).to.not.equal("active_user"); + }); + }); + + it("should handle case when no users are archived", async function () { + await cleanDb(); + + const activeUser = abandonedUsersData[2]; + await userModel.add(activeUser); + + const activeTask = abandonedTasksData[3]; + await tasksModel.add(activeTask); + + const orphanedTasks = await fetchOrphanedTasks(); + expect(orphanedTasks).to.be.an("array"); + expect(orphanedTasks).to.have.lengthOf(0); + }); + + it("should handle errors gracefully if getUsersWithIncompleteTasks fails", async function () { + Sinon.stub(tasksQuery, "fetchIncompleteTasksByUserIds").throws(new Error("Database query failed")); + + try { + await fetchOrphanedTasks(); + expect.fail("Expected function to throw an error"); + } catch (error) { + expect(error.message).to.equal("Database query failed"); + } + Sinon.restore(); + }); + }); });