From 589b9b2ae83fce9285d5adb737341b2c839be344 Mon Sep 17 00:00:00 2001 From: vinit717 Date: Wed, 30 Oct 2024 00:38:09 +0530 Subject: [PATCH 01/17] chore: add more query to search the logs --- models/logs.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/models/logs.js b/models/logs.js index 6a2e16161..e6c7d65c9 100644 --- a/models/logs.js +++ b/models/logs.js @@ -4,7 +4,7 @@ const logsModel = firestore.collection("logs"); const admin = require("firebase-admin"); const { logType, ERROR_WHILE_FETCHING_LOGS } = require("../constants/logs"); const { INTERNAL_SERVER_ERROR } = require("../constants/errorMessages"); -const { getFullName } = require("../utils/users"); +const { getFullName, getUserId } = require("../utils/users"); const { getUsersListFromLogs, formatLogsForFeed, @@ -164,18 +164,32 @@ const fetchLastAddedCacheLog = async (id) => { }; const fetchAllLogs = async (query) => { - let { type, prev, next, page, size = SIZE, format } = query; + let { type, prev, next, page, size = SIZE, format, userId, username, startDate, endDate } = query; size = parseInt(size); page = parseInt(page); try { let requestQuery = logsModel; + if (username) { + userId = await getUserId(username); + requestQuery = requestQuery.where("meta.userId", "==", userId); + } + if (type) { const logType = type.split(","); if (logType.length >= 1) requestQuery = requestQuery.where("type", "in", logType); } + if (startDate) { + const startTimestamp = { _seconds: parseInt(startDate / 1000), _nanoseconds: 0 }; + requestQuery = requestQuery.where("timestamp", ">=", startTimestamp); + } + if (endDate) { + const endTimestamp = { _seconds: parseInt(endDate / 1000), _nanoseconds: 0 }; + requestQuery = requestQuery.where("timestamp", "<=", endTimestamp); + } + requestQuery = requestQuery.orderBy("timestamp", "desc"); let requestQueryDoc = requestQuery; From 2c4438aa09e9977042f746e08d530678ea9af054 Mon Sep 17 00:00:00 2001 From: Rahul Goyal Date: Sun, 17 Nov 2024 00:40:28 +0530 Subject: [PATCH 02/17] Added seperate function for unobfuscated contact details and filtered id in update data step. --- controllers/users.js | 2 +- models/profileDiffs.js | 25 +++++++++++++++++++++++++ models/users.js | 10 +++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 1fa81c282..02e5e3f2d 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -654,7 +654,7 @@ const updateUser = async (req, res) => { try { const { id: profileDiffId, message } = req.body; - const profileDiffData = await profileDiffsQuery.fetchProfileDiff(profileDiffId); + const profileDiffData = await profileDiffsQuery.fetchProfileDiffUnobfuscated(profileDiffId); if (!profileDiffData) return res.boom.notFound("Profile Diff doesn't exist"); const { approval, timestamp, userId, ...profileDiff } = profileDiffData; diff --git a/models/profileDiffs.js b/models/profileDiffs.js index 494e192c9..f2e8895aa 100644 --- a/models/profileDiffs.js +++ b/models/profileDiffs.js @@ -133,6 +133,30 @@ const fetchProfileDiff = async (profileDiffId) => { } }; +/** + * Fetches the unobfuscated profileDiff data of the provided profileDiff Id + * @param profileDiffId profileDiffId of the diffs need to be fetched + * @returns unobfuscated profileDiff Data + */ +const fetchProfileDiffUnobfuscated = async (profileDiffId) => { + try { + const profileDiff = await profileDiffsModel.doc(profileDiffId).get(); + + if (!profileDiff.exists) { + return { profileDiffExists: false }; + } + const profileDiffData = profileDiff.data(); + return { + id: profileDiff.id, + profileDiffExists: true, + ...profileDiffData, + }; + } catch (err) { + logger.error("Error retrieving profile Diff", err); + throw err; + } +}; + /** Add profileDiff * * @param profileDiffData { Object }: Data to be added @@ -181,4 +205,5 @@ module.exports = { add, updateProfileDiff, fetchProfileDiffsWithPagination, + fetchProfileDiffUnobfuscated, }; diff --git a/models/users.js b/models/users.js index e0746de92..7f681d25a 100644 --- a/models/users.js +++ b/models/users.js @@ -42,9 +42,17 @@ const addOrUpdate = async (userData, userId = null) => { const isNewUser = !user.data(); // user exists update user if (user.data()) { + // remove id field if exist in fetched data or profileDiff + const currentData = user.data(); + if ("id" in currentData) { + delete currentData.id; + } + if ("id" in userData) { + delete userData.id; + } await userModel.doc(userId).set( { - ...user.data(), + ...currentData, ...userData, updated_at: Date.now(), }, From a6983a2e4545f52ac4caf5a7cf7d6695423fb5a0 Mon Sep 17 00:00:00 2001 From: RahulGoyal-tech Date: Wed, 20 Nov 2024 19:14:21 +0530 Subject: [PATCH 03/17] Removed id filtering for current data --- models/users.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/models/users.js b/models/users.js index 7f681d25a..74c54a4f1 100644 --- a/models/users.js +++ b/models/users.js @@ -43,16 +43,12 @@ const addOrUpdate = async (userData, userId = null) => { // user exists update user if (user.data()) { // remove id field if exist in fetched data or profileDiff - const currentData = user.data(); - if ("id" in currentData) { - delete currentData.id; - } if ("id" in userData) { delete userData.id; } await userModel.doc(userId).set( { - ...currentData, + ...user.data(), ...userData, updated_at: Date.now(), }, From e2aa779294c68a83d622e80e4c62e0c7153cd5b5 Mon Sep 17 00:00:00 2001 From: Rahul Goyal Date: Fri, 22 Nov 2024 08:32:06 +0530 Subject: [PATCH 04/17] Updated error message --- models/profileDiffs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/profileDiffs.js b/models/profileDiffs.js index f2e8895aa..34b64e340 100644 --- a/models/profileDiffs.js +++ b/models/profileDiffs.js @@ -152,7 +152,7 @@ const fetchProfileDiffUnobfuscated = async (profileDiffId) => { ...profileDiffData, }; } catch (err) { - logger.error("Error retrieving profile Diff", err); + logger.error("Error retrieving Unobfuscated profile Diff", err); throw err; } }; From d47204d832ae4a4a11cd016fde24c61fdd7e7a9f Mon Sep 17 00:00:00 2001 From: Rahul Goyal Date: Fri, 29 Nov 2024 08:42:09 +0530 Subject: [PATCH 05/17] Added dev feature flag --- controllers/users.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 02e5e3f2d..c1518b72c 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -653,8 +653,14 @@ const getUserImageForVerification = async (req, res) => { const updateUser = async (req, res) => { try { const { id: profileDiffId, message } = req.body; - - const profileDiffData = await profileDiffsQuery.fetchProfileDiffUnobfuscated(profileDiffId); + const devFeatureFlag = req.query.dev; + let profileDiffData; + if (devFeatureFlag === "true") { + profileDiffData = await profileDiffsQuery.fetchProfileDiffUnobfuscated(profileDiffId); + } else { + profileDiffData = await profileDiffsQuery.fetchProfileDiff(profileDiffId); + } + Object.freeze(profileDiffData); if (!profileDiffData) return res.boom.notFound("Profile Diff doesn't exist"); const { approval, timestamp, userId, ...profileDiff } = profileDiffData; 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 06/17] 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(); + }); + }); }); 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 07/17] 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(); + }); + }); }); From e1405a0aa76d17114f3362665b12cac9a6fd3ba5 Mon Sep 17 00:00:00 2001 From: vinit717 Date: Tue, 3 Dec 2024 19:49:02 +0530 Subject: [PATCH 08/17] chore: fix query and add dev flag --- models/logs.js | 46 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/models/logs.js b/models/logs.js index e6c7d65c9..085ee084e 100644 --- a/models/logs.js +++ b/models/logs.js @@ -164,14 +164,16 @@ const fetchLastAddedCacheLog = async (id) => { }; const fetchAllLogs = async (query) => { - let { type, prev, next, page, size = SIZE, format, userId, username, startDate, endDate } = query; + let { type, prev, next, page, size = SIZE, format, userId, username, startDate, endDate, dev } = query; + size = parseInt(size); page = parseInt(page); try { let requestQuery = logsModel; + const isDev = dev === "true"; - if (username) { + if (isDev && username) { userId = await getUserId(username); requestQuery = requestQuery.where("meta.userId", "==", userId); } @@ -181,13 +183,25 @@ const fetchAllLogs = async (query) => { if (logType.length >= 1) requestQuery = requestQuery.where("type", "in", logType); } - if (startDate) { - const startTimestamp = { _seconds: parseInt(startDate / 1000), _nanoseconds: 0 }; - requestQuery = requestQuery.where("timestamp", ">=", startTimestamp); - } - if (endDate) { - const endTimestamp = { _seconds: parseInt(endDate / 1000), _nanoseconds: 0 }; - requestQuery = requestQuery.where("timestamp", "<=", endTimestamp); + if (isDev && (startDate || endDate)) { + startDate = startDate ? parseInt(startDate) : null; + endDate = endDate ? parseInt(endDate) : null; + + if (startDate && endDate && startDate > endDate) { + throw new Error("Start date cannot be greater than end date."); + } + + const buildTimestamp = (date) => ({ + _seconds: Math.floor(date / 1000), + _nanoseconds: 0, + }); + + if (startDate) { + requestQuery = requestQuery.where("timestamp", ">=", buildTimestamp(startDate)); + } + if (endDate) { + requestQuery = requestQuery.where("timestamp", "<=", buildTimestamp(endDate)); + } } requestQuery = requestQuery.orderBy("timestamp", "desc"); @@ -219,12 +233,14 @@ const fetchAllLogs = async (query) => { const last = snapshot.docs[snapshot.docs.length - 1]; nextDoc = await requestQuery.startAfter(last).limit(1).get(); } + const allLogs = []; if (!snapshot.empty) { snapshot.forEach((doc) => { allLogs.push({ ...doc.data() }); }); } + if (allLogs.length === 0) { return { allLogs: [], @@ -233,17 +249,23 @@ const fetchAllLogs = async (query) => { page: page ? page + 1 : null, }; } + if (format === "feed") { - let logsData = []; const userList = await getUsersListFromLogs(allLogs); const taskIdList = await getTasksFromLogs(allLogs); const usersMap = mapify(userList, "id"); const tasksMap = mapify(taskIdList, "id"); - logsData = allLogs.map((data) => { + + const logsData = allLogs.map((data) => { const formattedLogs = formatLogsForFeed(data, usersMap, tasksMap); if (!Object.keys(formattedLogs).length) return null; - return { ...formattedLogs, type: data.type, timestamp: convertTimestamp(data.timestamp) }; + return { + ...formattedLogs, + type: data.type, + timestamp: convertTimestamp(data.timestamp), + }; }); + return { allLogs: logsData.filter((log) => log), prev: prevDoc.empty ? null : prevDoc.docs[0].id, From aaa2de4b6bd8eea9dac44bd8bca30aa6b61fd34a Mon Sep 17 00:00:00 2001 From: Vaibhav Singh Date: Wed, 4 Dec 2024 00:07:48 +0530 Subject: [PATCH 09/17] removed feature flag (#2281) --- routes/discordactions.js | 10 +--------- test/integration/discordactions.test.js | 12 ------------ 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/routes/discordactions.js b/routes/discordactions.js index 1d7621787..3869eca58 100644 --- a/routes/discordactions.js +++ b/routes/discordactions.js @@ -30,19 +30,11 @@ const ROLES = require("../constants/roles"); const { Services } = require("../constants/bot"); const { verifyCronJob } = require("../middlewares/authorizeBot"); const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService"); -const { devFlagMiddleware } = require("../middlewares/devFlag"); const router = express.Router(); router.post("/groups", authenticate, checkIsVerifiedDiscord, validateGroupRoleBody, createGroupRole); router.get("/groups", authenticate, checkIsVerifiedDiscord, getAllGroupRoles); -router.delete( - "/groups/:groupId", - authenticate, - checkIsVerifiedDiscord, - authorizeRoles([SUPERUSER]), - devFlagMiddleware, - deleteGroupRole -); +router.delete("/groups/:groupId", authenticate, checkIsVerifiedDiscord, authorizeRoles([SUPERUSER]), deleteGroupRole); router.post("/roles", authenticate, checkIsVerifiedDiscord, validateMemberRoleBody, addGroupRoleToMember); router.get("/invite", authenticate, getUserDiscordInvite); router.post("/invite", authenticate, checkCanGenerateDiscordLink, generateInviteForUser); diff --git a/test/integration/discordactions.test.js b/test/integration/discordactions.test.js index d943f85e6..574ea4858 100644 --- a/test/integration/discordactions.test.js +++ b/test/integration/discordactions.test.js @@ -233,18 +233,6 @@ describe("Discord actions", function () { await cleanDb(); }); - it("should return 404 when not in dev mode", function (done) { - chai - .request(app) - .delete(`/discord-actions/groups/${groupId}`) - .set("cookie", `${cookieName}=${superUserAuthToken}`) - .end((err, res) => { - expect(res).to.have.status(404); - expect(res.body.error).to.equal("Not Found"); - done(err); - }); - }); - it("should return 404 if group role not found", function (done) { sinon.stub(discordRolesModel, "isGroupRoleExists").resolves({ roleExists: false, From fccab02a78b8f51fcc9f8f438f2431813ef3b3cb Mon Sep 17 00:00:00 2001 From: Vikhyat Bhatnagar <52795644+vikhyat187@users.noreply.github.com> Date: Wed, 4 Dec 2024 00:24:32 +0530 Subject: [PATCH 10/17] Handling the conflict case where email has been already taken in AWS (#2271) * Fix: change env variables from obj to string This resolves the error faced in staging for the discord command developed by me /grant-aws-access, we were facing this issue when running this in stgaging, this is becuase in the node-config the values were being set as obj with name field as the actual value * lint-fix * removing console log statement * removing the extra log * handling conflict case, where user with email already exists in AWS --- controllers/awsAccess.ts | 6 ++++++ utils/awsFunctions.ts | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/controllers/awsAccess.ts b/controllers/awsAccess.ts index 779a4b8b7..82b229c82 100644 --- a/controllers/awsAccess.ts +++ b/controllers/awsAccess.ts @@ -21,6 +21,12 @@ export const addUserToAWSGroup = async (req, res) => { if (awsUserId === null){ // We need to create the user in AWS before and then fetch its Id userCreationResponse = await createUser(userInfoData.user.username, userInfoData.user.email); + + if (userCreationResponse.conflict){ + return res.status(400).json({ + error: `Username or Email is already being used, please use another email / username for creating account in AWS` + }) + } awsUserId = userCreationResponse.UserId; } diff --git a/utils/awsFunctions.ts b/utils/awsFunctions.ts index 260087b19..0429b8604 100644 --- a/utils/awsFunctions.ts +++ b/utils/awsFunctions.ts @@ -67,7 +67,10 @@ import { const command = new CreateUserCommand(params); return (await client.send(command)); } catch (error) { - console.error(`The error from create user ${error}`); + if (error.__type === 'ConflictException'){ + return { conflict: true }; + } + throw new Error(`Failed to create user: ${error instanceof Error ? error.message : String(error)}`); } }; @@ -93,7 +96,6 @@ export const addUserToGroup = async (groupId: string, awsUserId: string): Promis const command = new CreateGroupMembershipCommand(params); return (await client.send(command)); } catch (error) { - console.error("Error adding user to group:", error); if (error.__type === 'ConflictException'){ return { conflict: true }; } From 74f8324662bdc3d42134510662e7299514d875b4 Mon Sep 17 00:00:00 2001 From: Shobhan Sundar Goutam <81035407+shobhan-sundar-goutam@users.noreply.github.com> Date: Wed, 4 Dec 2024 02:45:12 +0530 Subject: [PATCH 11/17] Add unverified role to db and remove on /verify (#2257) * add unverified role to db and remove on /verify * destructured role and variable naming * fix: generic logs and removed unnecessary statement * refactor: improve error handling and logging in removeDiscordRoleFromUser function * refactor: simplify error message handling * batch db insertion * changed to typescript and modified unit tests --- constants/logs.ts | 3 + controllers/external-accounts.js | 20 +-- controllers/users.js | 37 +++++- models/discordactions.js | 2 +- test/integration/external-accounts.test.js | 76 ++++++++++- .../utils/remvoeDiscordRoleFromUser.test.ts | 123 ++++++++++++++++++ utils/removeDiscordRoleFromUser.ts | 57 ++++++++ 7 files changed, 299 insertions(+), 19 deletions(-) create mode 100644 test/unit/utils/remvoeDiscordRoleFromUser.test.ts create mode 100644 utils/removeDiscordRoleFromUser.ts diff --git a/constants/logs.ts b/constants/logs.ts index 6c7742a95..5988a5ae7 100644 --- a/constants/logs.ts +++ b/constants/logs.ts @@ -10,6 +10,9 @@ export const logType = { TASKS_MISSED_UPDATES_ERRORS: "TASKS_MISSED_UPDATES_ERRORS", DISCORD_INVITES: "DISCORD_INVITES", EXTERNAL_SERVICE: "EXTERNAL_SERVICE", + ADD_UNVERIFIED_ROLE: "ADD_UNVERIFIED_ROLE", + REMOVE_ROLE_FROM_USER_SUCCESS: "REMOVE_ROLE_FROM_USER_SUCCESS", + REMOVE_ROLE_FROM_USER_FAILED: "REMOVE_ROLE_FROM_USER_FAILED", EXTENSION_REQUESTS: "extensionRequests", TASK: "task", TASK_REQUESTS: "taskRequests", diff --git a/controllers/external-accounts.js b/controllers/external-accounts.js index a2995156b..f17e74b15 100644 --- a/controllers/external-accounts.js +++ b/controllers/external-accounts.js @@ -4,7 +4,7 @@ const { getDiscordMembers } = require("../services/discordService"); const { addOrUpdate, getUsersByRole, updateUsersInBatch } = require("../models/users"); const { retrieveDiscordUsers, fetchUsersForKeyValues } = require("../services/dataAccessLayer"); const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../constants/external-accounts"); -const discordServices = require("../services/discordService"); +const removeDiscordRoleUtils = require("../utils/removeDiscordRoleFromUser"); const config = require("config"); const logger = require("../utils/logger"); const { markUnDoneTasksOfArchivedUsersBacklog } = require("../models/tasks"); @@ -71,14 +71,16 @@ const linkExternalAccount = async (req, res) => { userId ); - try { - const unverifiedRoleId = config.get("discordUnverifiedRoleId"); - await discordServices.removeRoleFromUser(unverifiedRoleId, attributes.discordId, req.userData); - } catch (error) { - logger.error(`Error getting external account data: ${error}`); - return res.boom.internal("Role Deletion failed. Please contact admin.", { - message: "Role Deletion failed. Please contact admin.", - }); + const unverifiedRoleId = config.get("discordUnverifiedRoleId"); + const unverifiedRoleRemovalResponse = await removeDiscordRoleUtils.removeDiscordRoleFromUser( + req.userData, + attributes.discordId, + unverifiedRoleId + ); + + if (!unverifiedRoleRemovalResponse.success) { + const message = `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin`; + return res.boom.internal(message, { message }); } return res.status(204).json({ message: "Your discord profile has been linked successfully" }); diff --git a/controllers/users.js b/controllers/users.js index 79e554223..6c8cb3a0d 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -1,6 +1,10 @@ const chaincodeQuery = require("../models/chaincodes"); const userQuery = require("../models/users"); const profileDiffsQuery = require("../models/profileDiffs"); +const firestore = require("../utils/firestore"); +const memberRoleModel = firestore.collection("member-group-roles"); +const logsModel = firestore.collection("logs"); +const admin = require("firebase-admin"); const logsQuery = require("../models/logs"); const imageService = require("../services/imageService"); const { profileDiffStatus } = require("../constants/profileDiff"); @@ -620,7 +624,7 @@ const markUnverified = async (req, res) => { const unverifiedRoleId = config.get("discordUnverifiedRoleId"); const usersToApplyUnverifiedRole = []; const addRolePromises = []; - const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); + const batchPromises = []; allRdsLoggedInUsers.forEach((user) => { rdsUserMap[user.discordId] = true; @@ -636,11 +640,40 @@ const markUnverified = async (req, res) => { } }); + const batchSize = 500; + const batches = Array.from({ length: Math.ceil(usersToApplyUnverifiedRole.length / batchSize) }, (_, index) => + usersToApplyUnverifiedRole.slice(index * batchSize, index * batchSize + batchSize) + ); + + batches.forEach((batch) => { + const firestoreBatch = firestore.batch(); + + batch.forEach((id) => { + const memberRoleRef = memberRoleModel.doc(id); + const logRef = logsModel.doc(); + + firestoreBatch.set(memberRoleRef, { + roleid: unverifiedRoleId, + userid: id, + date: admin.firestore.Timestamp.fromDate(new Date()), + }); + + firestoreBatch.set(logRef, { + type: logType.ADD_UNVERIFIED_ROLE, + meta: { roleid: unverifiedRoleId, userid: id }, + body: { message: "Unverified role added successfully" }, + timestamp: admin.firestore.Timestamp.fromDate(new Date()), + }); + }); + + batchPromises.push(firestoreBatch.commit()); + }); + usersToApplyUnverifiedRole.forEach((id) => { addRolePromises.push(addRoleToUser(id, unverifiedRoleId)); }); - await Promise.all(addRolePromises); + await Promise.all([...addRolePromises, ...batchPromises]); return res.json({ message: "ROLES APPLIED SUCCESSFULLY" }); } catch (err) { logger.error(err); diff --git a/models/discordactions.js b/models/discordactions.js index 97115e952..bfe728cb8 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -158,7 +158,7 @@ const updateGroupRole = async (roleData, docId) => { * * @param options { Object }: Data of the new role * @param options.rolename String : name of the role - * @param options.roleId String : id of the role + * @param options.roleid String : id of the role * @returns {Promise} */ diff --git a/test/integration/external-accounts.test.js b/test/integration/external-accounts.test.js index d585565eb..00cb55e74 100644 --- a/test/integration/external-accounts.test.js +++ b/test/integration/external-accounts.test.js @@ -12,11 +12,11 @@ const externalAccountsModel = require("../../models/external-accounts"); const { usersFromRds, getDiscordMembers } = require("../fixtures/discordResponse/discord-response"); const Sinon = require("sinon"); const { INTERNAL_SERVER_ERROR } = require("../../constants/errorMessages"); +const removeDiscordRoleUtils = require("../../utils/removeDiscordRoleFromUser"); const firestore = require("../../utils/firestore"); const userData = require("../fixtures/user/user")(); const userModel = firestore.collection("users"); const tasksModel = firestore.collection("tasks"); -const discordServices = require("../../services/discordService"); const { EXTERNAL_ACCOUNTS_POST_ACTIONS } = require("../../constants/external-accounts"); chai.use(chaiHttp); const cookieName = config.get("userToken.cookieName"); @@ -538,7 +538,10 @@ describe("External Accounts", function () { expect(getUserResponseBeforeUpdate.body).to.not.have.property("discordId"); expect(getUserResponseBeforeUpdate.body).to.not.have.property("discordJoinedAt"); - const removeRoleFromUserStub = Sinon.stub(discordServices, "removeRoleFromUser").resolves(); + const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ + success: true, + message: "Role deleted successfully", + }); const response = await chai .request(app) @@ -557,13 +560,68 @@ describe("External Accounts", function () { expect(updatedUserDetails.body).to.have.property("discordId"); expect(updatedUserDetails.body).to.have.property("discordJoinedAt"); - removeRoleFromUserStub.restore(); + removeDiscordRoleStub.restore(); + }); + + it("Should return 500 when removeDiscordRole fails because role doesn't exist", async function () { + await externalAccountsModel.addExternalAccountData(externalAccountData[2]); + + const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ + success: false, + message: "Role doesn't exist", + }); + + const response = await chai + .request(app) + .patch(`/external-accounts/link/${externalAccountData[2].token}`) + .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) + .set("Cookie", `${cookieName}=${newUserJWT}`); + + const unverifiedRoleRemovalResponse = await removeDiscordRoleStub(); + + expect(response).to.have.status(500); + expect(response.body).to.be.an("object"); + expect(response.body).to.have.property("message"); + expect(response.body.message).to.equal( + `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin` + ); + + removeDiscordRoleStub.restore(); + }); + + it("Should return 500 when removeDiscordRole fails because role deletion from discord failed", async function () { + await externalAccountsModel.addExternalAccountData(externalAccountData[2]); + + const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ + success: false, + message: "Role deletion from discord failed", + }); + + const response = await chai + .request(app) + .patch(`/external-accounts/link/${externalAccountData[2].token}`) + .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) + .set("Cookie", `${cookieName}=${newUserJWT}`); + + const unverifiedRoleRemovalResponse = await removeDiscordRoleStub(); + + expect(response).to.have.status(500); + expect(response.body).to.be.an("object"); + expect(response.body).to.have.property("message"); + expect(response.body.message).to.equal( + `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin` + ); + + removeDiscordRoleStub.restore(); }); - it("Should return 500 when unverified role deletion failed", async function () { + it("Should return 500 when removeDiscordRole fails because role deletion from database failed", async function () { await externalAccountsModel.addExternalAccountData(externalAccountData[2]); - const removeRoleFromUserStub = Sinon.stub(discordServices, "removeRoleFromUser").rejects(); + const removeDiscordRoleStub = Sinon.stub(removeDiscordRoleUtils, "removeDiscordRoleFromUser").resolves({ + success: false, + message: "Role deletion from database failed", + }); const response = await chai .request(app) @@ -571,12 +629,16 @@ describe("External Accounts", function () { .query({ action: EXTERNAL_ACCOUNTS_POST_ACTIONS.DISCORD_USERS_SYNC }) .set("Cookie", `${cookieName}=${newUserJWT}`); + const unverifiedRoleRemovalResponse = await removeDiscordRoleStub(); + expect(response).to.have.status(500); expect(response.body).to.be.an("object"); expect(response.body).to.have.property("message"); - expect(response.body.message).to.equal(`Role Deletion failed. Please contact admin.`); + expect(response.body.message).to.equal( + `User details updated but ${unverifiedRoleRemovalResponse.message}. Please contact admin` + ); - removeRoleFromUserStub.restore(); + removeDiscordRoleStub.restore(); }); }); }); diff --git a/test/unit/utils/remvoeDiscordRoleFromUser.test.ts b/test/unit/utils/remvoeDiscordRoleFromUser.test.ts new file mode 100644 index 000000000..1fde9027c --- /dev/null +++ b/test/unit/utils/remvoeDiscordRoleFromUser.test.ts @@ -0,0 +1,123 @@ +import chai from "chai"; +import Sinon from "sinon"; +import { logType } from "../../../constants/logs"; +import discordActions from "../../../models/discordactions"; +import { addLog } from "../../../models/logs"; +import firestore from "../../../utils/firestore"; +import { removeDiscordRoleFromUser } from "../../../utils/removeDiscordRoleFromUser"; +import { groupData, memberGroupData } from "../../fixtures/discordactions/discordactions"; +import addUser from "../../utils/addUser"; +import cleanDb from "../../utils/cleanDb"; +const { expect } = chai; +const discordRolesModel = firestore.collection("discord-roles"); +const memberRoleModel = firestore.collection("member-group-roles"); +const logsModel = firestore.collection("logs"); +const userData = require("../../fixtures/user/user")(); + +describe("removeDiscordRoleFromUser", function () { + let userId; + let discordId; + let roleid; + let fetchStub; + + beforeEach(async function () { + userData[0].roles = { archived: false, in_discord: true }; + userId = await addUser(userData[0]); + discordId = userData[0].discordId; + userData[0] = { ...userData[0], id: userId }; + + const addRolePromises = memberGroupData.map(async (data) => { + await memberRoleModel.add(data); + }); + const discordRolesModelPromise = [discordRolesModel.add(groupData[0]), discordRolesModel.add(groupData[1])]; + await Promise.all(discordRolesModelPromise); + roleid = groupData[0].roleid; + await memberRoleModel.add({ roleid, userid: discordId }); + await Promise.all(addRolePromises); + + fetchStub = Sinon.stub(global, "fetch"); + }); + + afterEach(async function () { + await cleanDb(); + fetchStub.restore(); + }); + + it("should remove discord role successfully", async function () { + await addLog( + logType.REMOVE_ROLE_FROM_USER_SUCCESS, + { roleId: roleid, userid: discordId }, + { message: "Role removed successfully from user", userData: userData[0] } + ); + + fetchStub.returns( + Promise.resolve({ json: () => Promise.resolve({ success: true, message: "Role deleted successfully" }) }) + ); + + const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid); + const successLog = await logsModel + .where("type", "==", logType.REMOVE_ROLE_FROM_USER_SUCCESS) + .where("meta.roleId", "==", roleid) + .where("meta.userid", "==", discordId) + .limit(1) + .get(); + + expect(isDiscordRoleRemoved.success).to.be.equal(true); + expect(isDiscordRoleRemoved.message).to.be.equal("Role deleted successfully"); + expect(successLog.docs[0].data().body.message).to.be.equal("Role removed successfully from user"); + }); + + it("should throw an error if role doesn't exist in database when attempting to remove", async function () { + roleid = "randomRoleId"; + + const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid); + const failedLog = await logsModel + .where("type", "==", logType.REMOVE_ROLE_FROM_USER_FAILED) + .where("meta.roleId", "==", roleid) + .limit(1) + .get(); + + expect(isDiscordRoleRemoved.success).to.be.equal(false); + expect(isDiscordRoleRemoved.message).to.be.equal("Role doesn't exist"); + expect(failedLog.docs[0].data().body.message).to.be.equal("Role doesn't exist"); + }); + + it("should throw an error if role deletion from discord failed", async function () { + fetchStub.rejects(new Error("Role deletion from discord failed")); + + const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid); + const failedLog = await logsModel + .where("type", "==", logType.REMOVE_ROLE_FROM_USER_FAILED) + .where("meta.roleId", "==", roleid) + .where("meta.userid", "==", discordId) + .limit(1) + .get(); + + expect(isDiscordRoleRemoved.success).to.be.equal(false); + expect(isDiscordRoleRemoved.message).to.be.equal("Role deletion from discord failed"); + expect(failedLog.docs[0].data().body.message).to.be.equal("Role deletion from discord failed"); + }); + + it("should throw an error if role deleted from discord but not from database", async function () { + fetchStub.returns(Promise.resolve({ json: () => Promise.resolve({ success: true }) })); + + const removeMemberGroupStub = Sinon.stub(discordActions, "removeMemberGroup").resolves({ + roleId: roleid, + wasSuccess: false, + }); + + const isDiscordRoleRemoved = await removeDiscordRoleFromUser(userData[0], discordId, roleid); + const failedLog = await logsModel + .where("type", "==", logType.REMOVE_ROLE_FROM_USER_FAILED) + .where("meta.roleId", "==", roleid) + .where("meta.userid", "==", discordId) + .limit(1) + .get(); + + expect(isDiscordRoleRemoved.success).to.be.equal(false); + expect(isDiscordRoleRemoved.message).to.be.equal("Role deletion from database failed"); + expect(failedLog.docs[0].data().body.message).to.be.equal("Role deletion from database failed"); + + removeMemberGroupStub.restore(); + }); +}); diff --git a/utils/removeDiscordRoleFromUser.ts b/utils/removeDiscordRoleFromUser.ts new file mode 100644 index 000000000..78b45df33 --- /dev/null +++ b/utils/removeDiscordRoleFromUser.ts @@ -0,0 +1,57 @@ +import { logType } from "../constants/logs"; +import discordActions from "../models/discordactions"; +import { addLog } from "../models/logs"; +import discordServices from "../services/discordService"; +import { userData } from "../types/global"; + +/** + * Removes a Discord role from a user using Discord Id. + * + * @param {Object} userData - User data. + * @param {string} discordId - User's Discord ID. + * @param {string} roleId - Discord Role ID. + * + * @returns {Promise<{ success: boolean; message: string }>} - Result with success status and message. + */ +export const removeDiscordRoleFromUser = async ( + userData: userData, + discordId: string, + roleId: string +): Promise<{ success: boolean; message: string }> => { + try { + const { roleExists } = await discordActions.isGroupRoleExists({ roleid: roleId, rolename: null }); + + if (!roleExists) { + const message = "Role doesn't exist"; + await addLog(logType.REMOVE_ROLE_FROM_USER_FAILED, { roleId }, { message, userid: discordId, userData }); + throw new Error(message); + } + + try { + await discordServices.removeRoleFromUser(roleId, discordId, userData); + } catch (error) { + const message = "Role deletion from discord failed"; + await addLog(logType.REMOVE_ROLE_FROM_USER_FAILED, { roleId, userid: discordId, userData }, { message }); + throw new Error(message); + } + + const deleteResponse = await discordActions.removeMemberGroup(roleId, discordId); + if (deleteResponse && !deleteResponse.wasSuccess) { + const message = "Role deletion from database failed"; + await addLog(logType.REMOVE_ROLE_FROM_USER_FAILED, { roleId, userid: discordId }, { message, userData }); + throw new Error(message); + } + + await addLog( + logType.REMOVE_ROLE_FROM_USER_SUCCESS, + { roleId, userid: discordId }, + { message: "Role removed successfully from user", userData } + ); + + return { success: true, message: "Role deleted successfully" }; + } catch (error) { + logger.error(`Error removing role ${roleId} for user ${discordId}: ${error.message}`); + + return { success: false, message: error.message }; + } +}; From ef38a677f16360d71876272cfb3ec0a4e8451551 Mon Sep 17 00:00:00 2001 From: vinit717 Date: Wed, 4 Dec 2024 23:54:55 +0530 Subject: [PATCH 12/17] chore: write test for logs query --- test/integration/logs.test.js | 61 +++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/integration/logs.test.js b/test/integration/logs.test.js index d0e27657d..48f2c12c0 100644 --- a/test/integration/logs.test.js +++ b/test/integration/logs.test.js @@ -201,6 +201,67 @@ describe("/logs", function () { return done(); }); }); + + it("should return logs filtered by username, startDate, and endDate when dev flag is enabled", function (done) { + const username = "joygupta"; + const startDate = 1729841400000; + const endDate = 1729841500000; + chai + .request(app) + .get(`/logs?username=${username}&startDate=${startDate}&endDate=${endDate}&dev=true`) + .set("cookie", `${cookieName}=${superUserToken}`) + .end(function (err, res) { + if (err) { + return done(err); + } + expect(res).to.have.status(200); + expect(res.body.message).to.equal("All Logs fetched successfully"); + + expect(res.body.data).to.be.an("array"); + res.body.data.forEach((log) => { + expect(log).to.have.property("meta"); + expect(log).to.have.property("body"); + expect(log.meta).to.have.property("userId"); + const timestamp = log.timestamp._seconds * 1000; + expect(timestamp).to.be.at.least(startDate); + expect(timestamp).to.be.at.most(endDate); + }); + + return done(); + }); + }); + + it("should return an error if startDate is greater than endDate", function (done) { + const username = "joygupta"; + const startDate = 1729841500000; + const endDate = 1729841400000; + + chai + .request(app) + .get(`/logs?username=${username}&startDate=${startDate}&endDate=${endDate}&dev=true`) + .set("cookie", `${cookieName}=${superUserToken}`) + .end(function (_err, res) { + expect(res).to.have.status(500); + expect(res.body.error).to.equal("Start date cannot be greater than end date."); + return done(); + }); + }); + + it("should return an empty array if no logs match username and date range", function (done) { + const username = "nonexistentUser"; + const startDate = 1729841400000; + const endDate = 1729841500000; + + chai + .request(app) + .get(`/logs?username=${username}&startDate=${startDate}&endDate=${endDate}&dev=true`) + .set("cookie", `${cookieName}=${superUserToken}`) + .end(function (_err, res) { + expect(res).to.have.status(200); + expect(res.body.message).to.equal("All Logs fetched successfully"); + return done(); + }); + }); }); describe("Add logs when user doc is update", function () { From 46bc908b333426b6ed14fdf818d73efd2b2ca284 Mon Sep 17 00:00:00 2001 From: Vignesh B S <55937928+VinuB-Dev@users.noreply.github.com> Date: Thu, 5 Dec 2024 00:35:36 +0530 Subject: [PATCH 13/17] Fix: Handle firestore 'in' operator limitation by chunking queries; update field name from 'assigneeId' to 'assignee' (#2279) --- models/tasks.js | 20 +++++++++++++++---- services/tasks.js | 6 +++--- services/users.js | 2 +- .../abandoned-tasks/departed-users.js | 12 ++++------- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/models/tasks.js b/models/tasks.js index d7e22bffe..b7a2f40da 100644 --- a/models/tasks.js +++ b/models/tasks.js @@ -22,6 +22,7 @@ const { } = TASK_STATUS; const { OLD_ACTIVE, OLD_BLOCKED, OLD_PENDING, OLD_COMPLETED } = TASK_STATUS_OLD; const { INTERNAL_SERVER_ERROR } = require("../constants/errorMessages"); +const { BATCH_SIZE_IN_CLAUSE } = require("../constants/firebase"); /** * Update multiple tasks' status to DONE in one batch operation. @@ -750,11 +751,22 @@ const fetchIncompleteTasksByUserIds = async (userIds) => { return []; } try { - const incompleteTasksQuery = await tasksModel.where("assigneeId", "in", userIds).get(); + const userIdChunks = []; - const incompleteTaskForUsers = incompleteTasksQuery.docs.filter( - (task) => !COMPLETED_STATUSES.includes(task.data().status) - ); + for (let i = 0; i < userIds.length; i += BATCH_SIZE_IN_CLAUSE) { + userIdChunks.push(userIds.slice(i, i + BATCH_SIZE_IN_CLAUSE)); + } + + const promises = userIdChunks.map(async (userIdChunk) => { + const querySnapshot = await tasksModel.where("assignee", "in", userIdChunk).get(); + return querySnapshot.docs.map((doc) => doc.data()); + }); + + const snapshots = await Promise.all(promises); + + const incompleteTasks = snapshots.flat(); + + const incompleteTaskForUsers = incompleteTasks.filter((task) => !COMPLETED_STATUSES.includes(task.status)); return incompleteTaskForUsers; } catch (error) { diff --git a/services/tasks.js b/services/tasks.js index 370e381ec..bedf8a851 100644 --- a/services/tasks.js +++ b/services/tasks.js @@ -66,13 +66,13 @@ const fetchOrphanedTasks = async () => { const userIds = userSnapshot.docs.map((doc) => doc.id); - const orphanedTasksQuerySnapshot = await tasksQuery.fetchIncompleteTasksByUserIds(userIds); + const orphanedTasksData = await tasksQuery.fetchIncompleteTasksByUserIds(userIds); - if (orphanedTasksQuerySnapshot.empty) { + if (orphanedTasksData.empty) { return []; } - const orphanedTasks = orphanedTasksQuerySnapshot.map((doc) => doc.data()); + const orphanedTasks = orphanedTasksData; return orphanedTasks; } catch (error) { diff --git a/services/users.js b/services/users.js index 240f5c507..94646d896 100644 --- a/services/users.js +++ b/services/users.js @@ -15,7 +15,7 @@ const getUsersWithIncompleteTasks = async (users) => { return []; } - const userIdsWithIncompleteTasks = new Set(abandonedTasksQuerySnapshot.map((doc) => doc.data().assigneeId)); + const userIdsWithIncompleteTasks = new Set(abandonedTasksQuerySnapshot.map((doc) => doc.assignee)); const eligibleUsersWithTasks = users.filter((user) => userIdsWithIncompleteTasks.has(user.id)); diff --git a/test/fixtures/abandoned-tasks/departed-users.js b/test/fixtures/abandoned-tasks/departed-users.js index d16ca551b..ad58e877c 100644 --- a/test/fixtures/abandoned-tasks/departed-users.js +++ b/test/fixtures/abandoned-tasks/departed-users.js @@ -76,8 +76,7 @@ const tasksData = [ updatedAt: 1727027999, startedOn: 1727027777, endsOn: 1731542400, - assignee: "archived_user1", - assigneeId: "user1_id", + assignee: "user1_id", github: { issue: { html_url: "https://github.com/org/repo/issues/1", @@ -97,8 +96,7 @@ const tasksData = [ updatedAt: 1727027999, startedOn: 1727027777, endsOn: 1731542400, - assignee: "archived_user2", - assigneeId: "user2_id", + assignee: "user2_id", github: { issue: { html_url: "https://github.com/org/repo/issues/2", @@ -118,8 +116,7 @@ const tasksData = [ updatedAt: 1727027999, startedOn: 1727027777, endsOn: 1731542400, - assignee: "archived_user1", - assigneeId: "user1_id", + assignee: "user1_id", github: { issue: { html_url: "https://github.com/org/repo/issues/3", @@ -139,8 +136,7 @@ const tasksData = [ updatedAt: 1727027999, startedOn: 1727027777, endsOn: 1731542400, - assignee: "active_user", - assigneeId: "user3_id", + assignee: "user3_id", github: { issue: { html_url: "https://github.com/org/repo/issues/4", From 9cfad9d3977cb72104743f7b766921eac095f341 Mon Sep 17 00:00:00 2001 From: vinit717 Date: Thu, 5 Dec 2024 01:33:15 +0530 Subject: [PATCH 14/17] chore: fix test and error message --- controllers/logs.js | 3 +++ models/logs.js | 4 +++- test/integration/logs.test.js | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/controllers/logs.js b/controllers/logs.js index fa054e195..a2a2475b3 100644 --- a/controllers/logs.js +++ b/controllers/logs.js @@ -67,6 +67,9 @@ const fetchAllLogs = async (req, res) => { prev: prevUrl, }); } catch (err) { + if (err.statusCode) { + return res.status(err.statusCode).json({ error: err.message }); + } logger.error(ERROR_WHILE_FETCHING_LOGS, err); return res.boom.badImplementation(ERROR_WHILE_FETCHING_LOGS); } diff --git a/models/logs.js b/models/logs.js index 085ee084e..f66f5046b 100644 --- a/models/logs.js +++ b/models/logs.js @@ -188,7 +188,9 @@ const fetchAllLogs = async (query) => { endDate = endDate ? parseInt(endDate) : null; if (startDate && endDate && startDate > endDate) { - throw new Error("Start date cannot be greater than end date."); + const error = new Error("Start date cannot be greater than end date."); + error.statusCode = 400; + throw error; } const buildTimestamp = (date) => ({ diff --git a/test/integration/logs.test.js b/test/integration/logs.test.js index 48f2c12c0..0e36b8e69 100644 --- a/test/integration/logs.test.js +++ b/test/integration/logs.test.js @@ -241,7 +241,7 @@ describe("/logs", function () { .get(`/logs?username=${username}&startDate=${startDate}&endDate=${endDate}&dev=true`) .set("cookie", `${cookieName}=${superUserToken}`) .end(function (_err, res) { - expect(res).to.have.status(500); + expect(res).to.have.status(400); expect(res.body.error).to.equal("Start date cannot be greater than end date."); return done(); }); From 5228100795eea33a7d20ce50dbb5cee782eecd26 Mon Sep 17 00:00:00 2001 From: Vinit khandal <111434418+vinit717@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:32:51 +0530 Subject: [PATCH 15/17] Standardize Logs Collection for Consistent Querying (#2226) * chore: fix log type into lower case * chore: revert changes for make the logs working * chore: add logs migration route * chore: add migration model function * chore: add tests for migration api * chore: update route name * chore: update route name --- controllers/logs.js | 12 +++++++ models/logs.js | 62 +++++++++++++++++++++++++++++++++++ routes/logs.js | 1 + test/integration/logs.test.js | 30 +++++++++++++++++ 4 files changed, 105 insertions(+) diff --git a/controllers/logs.js b/controllers/logs.js index fa054e195..72bc55bb7 100644 --- a/controllers/logs.js +++ b/controllers/logs.js @@ -72,7 +72,19 @@ const fetchAllLogs = async (req, res) => { } }; +const updateLogs = async (req, res) => { + try { + const response = await logsQuery.updateLogs(); + return res.json({ + response, + }); + } catch (error) { + return res.boom.serverUnavailable(SOMETHING_WENT_WRONG); + } +}; + module.exports = { fetchLogs, fetchAllLogs, + updateLogs, }; diff --git a/models/logs.js b/models/logs.js index 6a2e16161..fe354db2f 100644 --- a/models/logs.js +++ b/models/logs.js @@ -250,10 +250,72 @@ const fetchAllLogs = async (query) => { } }; +const updateLogs = async () => { + const batchSize = 500; + let lastDoc = null; + let isCompleted = false; + + const summary = { + totalLogsProcessed: 0, + totalLogsUpdated: 0, + totalOperationsFailed: 0, + failedLogDetails: [], + }; + + try { + while (!isCompleted) { + let query = logsModel.orderBy("timestamp").limit(batchSize); + if (lastDoc) { + query = query.startAfter(lastDoc); + } + const snapshot = await query.get(); + + if (snapshot.empty) { + isCompleted = true; + continue; + } + + const batch = firestore.batch(); + snapshot.forEach((doc) => { + const data = doc.data(); + if (data.meta && data.meta.createdBy) { + const updatedMeta = { + ...data.meta, + userId: data.meta.createdBy, + }; + delete updatedMeta.createdBy; + + batch.update(doc.ref, { meta: updatedMeta }); + summary.totalLogsUpdated++; + } + summary.totalLogsProcessed++; + }); + + try { + await batch.commit(); + } catch (err) { + logger.error("Batch update failed for logs collection:", err); + summary.totalOperationsFailed += snapshot.docs.length; + summary.failedLogDetails.push(...snapshot.docs.map((doc) => doc.id)); + } + + lastDoc = snapshot.docs[snapshot.docs.length - 1]; + isCompleted = snapshot.docs.length < batchSize; + } + + logger.info("Migration completed:", summary); + return summary; + } catch (error) { + logger.error("Error during logs migration:", error); + throw error; + } +}; + module.exports = { addLog, fetchLogs, fetchCacheLogs, fetchLastAddedCacheLog, fetchAllLogs, + updateLogs, }; diff --git a/routes/logs.js b/routes/logs.js index cba4ec347..6f4d2b119 100644 --- a/routes/logs.js +++ b/routes/logs.js @@ -7,5 +7,6 @@ const { SUPERUSER } = require("../constants/roles"); router.get("/:type", authenticate, authorizeRoles([SUPERUSER]), logs.fetchLogs); router.get("/", authenticate, authorizeRoles([SUPERUSER]), logs.fetchAllLogs); +router.post("/migrate", authenticate, authorizeRoles([SUPERUSER]), logs.updateLogs); module.exports = router; diff --git a/test/integration/logs.test.js b/test/integration/logs.test.js index d0e27657d..5ad0dd1ed 100644 --- a/test/integration/logs.test.js +++ b/test/integration/logs.test.js @@ -203,6 +203,36 @@ describe("/logs", function () { }); }); + describe("Update logs", function () { + it("should run the migration and update logs successfully", async function () { + const res = await chai.request(app).post("/logs/migrate").set("cookie", `${cookieName}=${superUserToken}`).send(); + + expect(res).to.have.status(200); + expect(res.body).to.be.an("object"); + expect(res.body.response).to.have.property("totalLogsProcessed").that.is.a("number"); + expect(res.body.response).to.have.property("totalLogsUpdated").that.is.a("number"); + expect(res.body.response).to.have.property("totalOperationsFailed").that.is.a("number"); + expect(res.body.response).to.have.property("failedLogDetails").that.is.an("array"); + + expect(res.body.response.totalLogsProcessed).to.be.at.least(0); + expect(res.body.response.totalLogsUpdated).to.be.lessThanOrEqual(res.body.response.totalLogsProcessed); + + expect(res.body.response).to.have.all.keys( + "totalLogsProcessed", + "totalLogsUpdated", + "totalOperationsFailed", + "failedLogDetails" + ); + }); + + it("should return error if unauthorized user tries to run migration", async function () { + const res = await chai.request(app).post("/logs/migrate").set("cookie", `${cookieName}=invalidToken`).send(); + + expect(res).to.have.status(401); + expect(res.body).to.have.property("error").that.is.a("string"); + }); + }); + describe("Add logs when user doc is update", function () { let jwt; let userId; From daed096a455034ccb4408332dfed2901bd325df3 Mon Sep 17 00:00:00 2001 From: Vinit khandal <111434418+vinit717@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:33:27 +0530 Subject: [PATCH 16/17] Standardize logs meta field to use userId (#2248) * chore : fix logs meta field to use userId * chore: fix task log --- controllers/extensionRequests.js | 2 +- controllers/extensionRequestsv2.ts | 2 +- controllers/invites.ts | 2 +- controllers/oooRequests.ts | 4 ++-- controllers/taskRequestsv2.ts | 4 ++-- controllers/tasksRequests.js | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/controllers/extensionRequests.js b/controllers/extensionRequests.js index d3c92cc55..f4ebc0d97 100644 --- a/controllers/extensionRequests.js +++ b/controllers/extensionRequests.js @@ -74,7 +74,7 @@ const createTaskExtensionRequest = async (req, res) => { type: "extensionRequests", meta: { taskId: extensionBody.taskId, - createdBy: req.userData.id, + userId: req.userData.id, }, body: { extensionRequestId: extensionRequest.id, diff --git a/controllers/extensionRequestsv2.ts b/controllers/extensionRequestsv2.ts index df4bffae7..edde8482e 100644 --- a/controllers/extensionRequestsv2.ts +++ b/controllers/extensionRequestsv2.ts @@ -84,7 +84,7 @@ export const createTaskExtensionRequest = async (req: ExtensionRequestRequest, r taskId, requestId: extensionRequest.id, action: LOG_ACTION.CREATE, - createdBy: requestedBy, + userId: requestedBy, createdAt: Date.now(), }, body: extensionBody, diff --git a/controllers/invites.ts b/controllers/invites.ts index 1e553890a..5419c3ee1 100644 --- a/controllers/invites.ts +++ b/controllers/invites.ts @@ -32,7 +32,7 @@ export const createInviteLink = async (req: InviteBodyRequest, res: CustomRespon type: logType.DISCORD_INVITES, meta: { action: "create", - createdBy: logType.EXTERNAL_SERVICE, + userId: logType.EXTERNAL_SERVICE, createdAt: Date.now(), }, body: { diff --git a/controllers/oooRequests.ts b/controllers/oooRequests.ts index 5463db714..a3dea406b 100644 --- a/controllers/oooRequests.ts +++ b/controllers/oooRequests.ts @@ -41,7 +41,7 @@ export const createOooRequestController = async (req: OooRequestCreateRequest, r meta: { requestId: requestResult.id, action: LOG_ACTION.CREATE, - createdBy: userId, + userId: userId, createdAt: Date.now(), }, body: requestResult, @@ -84,7 +84,7 @@ export const updateOooRequestController = async (req: UpdateRequest, res: Custom meta: { requestId: requestId, action: LOG_ACTION.UPDATE, - createdBy: userId, + userId: userId, createdAt: Date.now(), }, body: requestResult, diff --git a/controllers/taskRequestsv2.ts b/controllers/taskRequestsv2.ts index 545634d35..327c56bc1 100644 --- a/controllers/taskRequestsv2.ts +++ b/controllers/taskRequestsv2.ts @@ -92,7 +92,7 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res: meta: { taskRequestId: updatedRequest.id, action: "update", - createdBy: req.userData.id, + userId: req.userData.id, createdAt: Date.now(), lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), @@ -150,7 +150,7 @@ export const createTaskRequestController = async (req: TaskRequestRequest, res: meta: { taskRequestId: newTaskRequest.id, action: "create", - createdBy: req.userData.id, + userId: req.userData.id, createdAt: Date.now(), lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), diff --git a/controllers/tasksRequests.js b/controllers/tasksRequests.js index d647e564e..1b4487334 100644 --- a/controllers/tasksRequests.js +++ b/controllers/tasksRequests.js @@ -104,7 +104,7 @@ const addTaskRequests = async (req, res) => { meta: { taskRequestId: newTaskRequest.id, action: "create", - createdBy: req.userData.id, + userId: req.userData.id, createdAt: Date.now(), lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), @@ -206,7 +206,7 @@ const updateTaskRequests = async (req, res) => { taskRequestId: taskRequestId, action: "update", subAction: action, - createdBy: req.userData.id, + userId: req.userData.id, createdAt: Date.now(), lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), From 5dc221cc0055429639eb3f18c91e29afe7f499c2 Mon Sep 17 00:00:00 2001 From: Vikas Singh <59792866+vikasosmium@users.noreply.github.com> Date: Fri, 6 Dec 2024 00:52:13 +0530 Subject: [PATCH 17/17] Remove feature flag from arts/userId?dev=true API Endpoint (#2289) * added new route to fetch user art * added new route under featureflag * added test cases for new route * wrote 2 new test cases and and one no content cond. * removing feature flag --- routes/arts.ts | 2 +- test/integration/arts.test.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/routes/arts.ts b/routes/arts.ts index 07d71e098..2b7e3c4ad 100644 --- a/routes/arts.ts +++ b/routes/arts.ts @@ -8,7 +8,7 @@ import { devFlagMiddleware } from "../middlewares/devFlag"; router.get("/", arts.fetchArts); router.get("/user/self", authenticate, arts.getSelfArts); // this route is soon going to be deprecated soon, please use /arts/:userId endpoint. router.get("/user/:userId", authenticate, arts.getUserArts); // this route is soon going to be deprecated soon, please use /arts/:userId endpoint. -router.get("/:userId", devFlagMiddleware, authenticate, arts.getUserArts); +router.get("/:userId", authenticate, arts.getUserArts); router.post("/user/add", authenticate, artValidator.createArt, arts.addArt); module.exports = router; diff --git a/test/integration/arts.test.js b/test/integration/arts.test.js index 0721558f3..9977caf24 100644 --- a/test/integration/arts.test.js +++ b/test/integration/arts.test.js @@ -152,7 +152,7 @@ describe("Arts", function () { it("Should get all the arts of the user", function (done) { chai .request(app) - .get(`/arts/${userId}?dev=true`) + .get(`/arts/${userId}`) .set("cookie", `${cookieName}=${jwt}`) .end((err, res) => { if (err) { @@ -172,7 +172,7 @@ describe("Arts", function () { it("Should return 401, for Unauthenticated User", function (done) { chai .request(app) - .get(`/arts/${userId}?dev=true`) + .get(`/arts/${userId}`) .end((err, res) => { if (err) { return done(err); @@ -195,7 +195,7 @@ describe("Arts", function () { chai .request(app) - .get(`/arts/${userId}?dev=true`) + .get(`/arts/${userId}`) .set("cookie", `${cookieName}=${jwt}`) .end((err, res) => { artsQuery.fetchUserArts.restore(); @@ -215,7 +215,7 @@ describe("Arts", function () { chai .request(app) - .get(`/arts/${userId}?dev=true`) + .get(`/arts/${userId}`) .set("cookie", `${cookieName}=${jwt}`) .end((err, res) => { artsQuery.fetchUserArts.restore();