From 10bc222c9d333a9faf55b3471a19295c0688df71 Mon Sep 17 00:00:00 2001 From: Vignesh B S <55937928+VinuB-Dev@users.noreply.github.com> Date: Sat, 30 Nov 2024 23:30:12 +0530 Subject: [PATCH] Feat: Implement APIs for Tracking Departed Users with Assigned Tasks (#2268) * feat: Departed users api changes. * feat: Added test cases for departed users api changes. * 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. --- controllers/users.js | 25 ++++++++++++ middlewares/validators/user.js | 1 + models/tasks.js | 6 +-- models/users.js | 65 +++++++++++++++++++++++++++++--- services/users.js | 56 ++++++++++----------------- test/integration/users.test.js | 58 +++++++++++++++++++++++++++- test/unit/models/tasks.test.js | 1 - test/unit/models/users.test.js | 44 +++++++++++++++++++++ test/unit/services/users.test.js | 65 +++++++++++++++++++++++++++++++- 9 files changed, 273 insertions(+), 48 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 e532a59a0..d7e22bffe 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -752,9 +752,9 @@ const fetchIncompleteTasksByUserIds = async (userIds) => { try { const incompleteTasksQuery = await tasksModel.where("assigneeId", "in", userIds).get(); - const incompleteTaskForUsers = incompleteTasksQuery.docs.filter((task) => { - return !COMPLETED_STATUSES.includes(task.data().status); - }); + const incompleteTaskForUsers = incompleteTasksQuery.docs.filter( + (task) => !COMPLETED_STATUSES.includes(task.data().status) + ); return incompleteTaskForUsers; } catch (error) { diff --git a/models/users.js b/models/users.js index baa11144a..a47575bed 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"); @@ -1049,6 +1103,7 @@ const fetchUsersNotInDiscordServer = async () => { }; module.exports = { + archiveUsers, addOrUpdate, fetchPaginatedUsers, fetchUser, diff --git a/services/users.js b/services/users.js index bb1d609b6..240f5c507 100644 --- a/services/users.js +++ b/services/users.js @@ -1,44 +1,28 @@ -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: [], - }; +const tasksModel = require("../models/tasks"); - 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 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 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}`); + throw error; } }; @@ -63,6 +47,6 @@ const generateUniqueUsername = async (firstName, lastName) => { }; module.exports = { - archiveUsers, generateUniqueUsername, + getUsersWithIncompleteTasks, }; 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 2b5f76449..45dc241f6 100644 --- a/test/unit/models/tasks.test.js +++ b/test/unit/models/tasks.test.js @@ -378,7 +378,6 @@ 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.fetchIncompleteTasksByUserIds([activeUser.id]); expect(incompleteTasks.length).to.be.equal(0); diff --git a/test/unit/models/users.test.js b/test/unit/models/users.test.js index c3dd691ac..c66eec4c1 100644 --- a/test/unit/models/users.test.js +++ b/test/unit/models/users.test.js @@ -529,6 +529,50 @@ describe("users", function () { }); }); + 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"); + } + }); + }); + describe("fetchUsersNotInDiscordServer", function () { beforeEach(async function () { await cleanDb(); diff --git a/test/unit/services/users.test.js b/test/unit/services/users.test.js index 60cf04c9d..f0be625ef 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, "fetchIncompleteTasksByUserIds").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(); + }); + }); });