From 3bca9c4119e3b58869b722a32d4ed84f8c053770 Mon Sep 17 00:00:00 2001 From: Palak Gupta Date: Mon, 25 Mar 2024 12:33:41 +0530 Subject: [PATCH] backend/activity-feed: refactor code and add test cases (#1982) --- constants/logs.ts | 1 + models/logs.js | 24 +++- test/integration/logs.test.js | 8 +- test/unit/models/logs.test.js | 8 +- test/unit/utils/logs.test.js | 218 ++++++++++++++++++++++++++++++++++ utils/logs.js | 128 +++++++++++++------- 6 files changed, 336 insertions(+), 51 deletions(-) create mode 100644 test/unit/utils/logs.test.js diff --git a/constants/logs.ts b/constants/logs.ts index 705a3ebc3..2c3c19cec 100644 --- a/constants/logs.ts +++ b/constants/logs.ts @@ -12,6 +12,7 @@ export const logType = { EXTERNAL_SERVICE: "EXTERNAL_SERVICE", EXTENSION_REQUESTS: "extensionRequests", TASK: "task", + TASK_REQUESTS: "taskRequests", ...REQUEST_LOG_TYPE, }; diff --git a/models/logs.js b/models/logs.js index 7dbbf42ed..6a2e16161 100644 --- a/models/logs.js +++ b/models/logs.js @@ -5,7 +5,13 @@ 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 { getUsersListFromLogs, formatLogsForFeed, mapify, convertTimestamp } = require("../utils/logs"); +const { + getUsersListFromLogs, + formatLogsForFeed, + mapify, + convertTimestamp, + getTasksFromLogs, +} = require("../utils/logs"); const SIZE = 25; /** @@ -205,16 +211,22 @@ const fetchAllLogs = async (query) => { allLogs.push({ ...doc.data() }); }); } - const userList = await getUsersListFromLogs(allLogs); - const usersMap = mapify(userList, "id"); - if (allLogs.length === 0) { - return []; + return { + allLogs: [], + prev: null, + next: null, + 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 formattedLogs = formatLogsForFeed(data, usersMap); + const formattedLogs = formatLogsForFeed(data, usersMap, tasksMap); if (!Object.keys(formattedLogs).length) return null; return { ...formattedLogs, type: data.type, timestamp: convertTimestamp(data.timestamp) }; }); diff --git a/test/integration/logs.test.js b/test/integration/logs.test.js index a460f672d..d47dec912 100644 --- a/test/integration/logs.test.js +++ b/test/integration/logs.test.js @@ -142,7 +142,8 @@ describe("/logs", function () { expect(res.body.message).to.equal("All Logs fetched successfully"); expect(res.body.data).to.lengthOf(7); expect(res.body.data[0]).to.contain({ - user: "joygupta", + username: "joygupta", + taskTitle: "Untitled Task", taskId: "mZB0akqPUa1GQQdrgsx7", extensionRequestId: "y79PXir0s82qNAzeIn8S", status: "PENDING", @@ -170,7 +171,7 @@ describe("/logs", function () { }); }); - it("should return 204, if no logs are present", function (done) { + it("if no logs are present, should return valid response", function (done) { chai .request(app) .get("/logs?type=REQUEST_CREATED1&dev=true") @@ -179,7 +180,8 @@ describe("/logs", function () { if (err) { return done(err); } - expect(res).to.have.status(204); + expect(res).to.have.status(200); + expect(res.body.data).to.have.lengthOf(0); return done(); }); }); diff --git a/test/unit/models/logs.test.js b/test/unit/models/logs.test.js index 0dea4e0d8..d0840d508 100644 --- a/test/unit/models/logs.test.js +++ b/test/unit/models/logs.test.js @@ -15,6 +15,8 @@ const cookieName = config.get("userToken.cookieName"); const authService = require("../../../services/authService"); const { extensionRequestLogs } = require("../../fixtures/logs/extensionRequests"); const { LOGS_FETCHED_SUCCESSFULLY } = require("../../../constants/logs"); +const tasks = require("../../../models/tasks"); +const tasksData = require("../../fixtures/tasks/tasks")(); chai.use(chaiHttp); const superUser = userData[4]; const userToBeMadeMember = userData[1]; @@ -133,6 +135,10 @@ describe("Logs", function () { describe("GET /logs", function () { before(async function () { await addLogs(); + const tasksPromise = tasksData.map(async (task) => { + await tasks.updateTask(task); + }); + await Promise.all(tasksPromise); }); after(async function () { @@ -195,7 +201,7 @@ describe("Logs", function () { it("Should return null if no logs are presnet the logs for specific types", async function () { await cleanDb(); const result = await logsQuery.fetchAllLogs({}); - expect(result).to.lengthOf(0); + expect(result.allLogs).to.lengthOf(0); }); it("should throw an error and log it", async function () { diff --git a/test/unit/utils/logs.test.js b/test/unit/utils/logs.test.js new file mode 100644 index 000000000..c9a995a8a --- /dev/null +++ b/test/unit/utils/logs.test.js @@ -0,0 +1,218 @@ +const { expect } = require("chai"); +const { formatLogsForFeed, mapify } = require("../../../utils/logs"); + +describe("logs utils", function () { + describe("formatLogsForFeed", function () { + const usersMap = { + user1: { username: "palak-gupta" }, + user2: { username: "mock-2" }, + }; + + const tasksMap = { + task1: { title: "Link details page to status site" }, + task2: { title: "Introduce /ooo command on discord" }, + }; + + it("should format logs for OOO type", function () { + const logsSnapshot = { + meta: { + createdAt: 1710181066410, + createdBy: "user1", + requestId: "request123", + action: "create", + }, + type: "REQUEST_CREATED", + body: { + createdAt: 1710181064968, + requestedBy: "user1", + from: 1710288000000, + until: 1710288050000, + id: "NOAWuKmazlIJHaN7Pihg", + state: "PENDING", + type: "OOO", + message: "For testing purpose", + updatedAt: 1710181064968, + }, + timestamp: { + _seconds: 1710181066, + _nanoseconds: 410000000, + }, + }; + + const formattedLog = formatLogsForFeed(logsSnapshot, usersMap); + + expect(formattedLog).to.deep.equal({ + user: "palak-gupta", + requestId: "request123", + from: 1710288000000, + until: 1710288050000, + message: "For testing purpose", + }); + }); + + it("should format logs for extensionRequests type", function () { + const logsSnapshot = { + meta: { + extensionRequestId: "po1gNOCXUP2IFsChcmn8", + userId: "user2", + taskId: "task1", + username: "techlord", + }, + type: "extensionRequests", + body: { + status: "APPROVED", + }, + timestamp: { + _seconds: 1709316797, + _nanoseconds: 616000000, + }, + }; + + const formattedLog = formatLogsForFeed(logsSnapshot, usersMap, tasksMap); + + expect(formattedLog).to.deep.equal({ + extensionRequestId: "po1gNOCXUP2IFsChcmn8", + status: "APPROVED", + taskId: "task1", + taskTitle: "Link details page to status site", + type: "extensionRequests", + user: "mock-2", + userId: "user2", + username: "techlord", + }); + }); + + it("should return empty object when usersMap does not contain requestedBy user", function () { + const invalidLogsSnapshot = { + meta: { requestId: "request123", taskId: "task456" }, + body: { + type: "OOO", + requestedBy: 3, // User not in usersMap + from: "2024-03-24", + until: "2024-03-25", + message: "Out of office", + extensionRequestId: "extension123", + }, + }; + + const formattedLog = formatLogsForFeed(invalidLogsSnapshot, usersMap); + + expect(formattedLog).to.deep.equal({}); + }); + + it("should format logs for task type", function () { + const logsSnapshot = { + meta: { + userId: "user1", + taskId: "task2", + username: "shubham-sharma", + }, + type: "task", + body: { + new: { + percentCompleted: 40, + }, + subType: "update", + }, + timestamp: { + _seconds: 1711273137, + _nanoseconds: 96000000, + }, + }; + + const formattedLog = formatLogsForFeed(logsSnapshot, usersMap, tasksMap); + + expect(formattedLog).to.deep.equal({ + percentCompleted: 40, + subType: "update", + taskId: "task2", + taskTitle: "Introduce /ooo command on discord", + type: "task", + user: "shubham-sharma", + userId: "user1", + username: "shubham-sharma", + }); + }); + + it("should format logs for PROFILE_DIFF_REJECTED type", function () { + const logsSnapshot = { + meta: { + rejectedBy: "user2", + userId: "user1", + }, + type: "PROFILE_DIFF_REJECTED", + body: { + profileDiffId: "F8e0II1X7qZwzA1CbF0l", + message: "", + }, + timestamp: { + _seconds: 1708098695, + _nanoseconds: 709000000, + }, + }; + + const formattedLog = formatLogsForFeed(logsSnapshot, usersMap); + + expect(formattedLog).to.deep.equal({ + user: "palak-gupta", + rejectedBy: "mock-2", + message: "", + }); + }); + + it("should format logs for PROFILE_DIFF_APPROVED type", function () { + const logsSnapshot = { + meta: { + approvedBy: "user1", + userId: "user2", + }, + type: "PROFILE_DIFF_APPROVED", + body: { + profileDiffId: "7sPvm4ooC1PyC91A5KVS", + message: "", + }, + timestamp: { + _seconds: 1707253607, + _nanoseconds: 697000000, + }, + }; + + const formattedLog = formatLogsForFeed(logsSnapshot, usersMap); + + expect(formattedLog).to.deep.equal({ + approvedBy: "palak-gupta", + user: "mock-2", + message: "", + }); + }); + }); + + describe("mapify function", function () { + const data = [ + { username: "palak", id: "100", task: "task2" }, + { username: "mock-user-1", id: "101", task: "task23" }, + { username: "mock-user-2", id: "102", task: "task4" }, + ]; + + it("mapify data based on username", function () { + const mapifiedData = mapify(data, "username"); + expect(mapifiedData).to.deep.equal({ + "mock-user-1": { + id: "101", + username: "mock-user-1", + task: "task23", + }, + "mock-user-2": { + id: "102", + username: "mock-user-2", + task: "task4", + }, + palak: { + id: "100", + username: "palak", + task: "task2", + }, + }); + }); + }); +}); diff --git a/utils/logs.js b/utils/logs.js index 5232f8299..94d5a10b0 100644 --- a/utils/logs.js +++ b/utils/logs.js @@ -1,8 +1,9 @@ const admin = require("firebase-admin"); const { logType } = require("../constants/logs"); const usersService = require("../services/dataAccessLayer"); -const { EXTENSION_REQUEST_STATUS } = require("../constants/extensionRequests"); - +const firestore = require("./firestore"); +const tasksModel = firestore.collection("tasks"); +const { _ } = require("lodash"); async function getUsersListFromLogs(allLogs) { const userIds = new Set(); for (const log of allLogs) { @@ -13,23 +14,53 @@ async function getUsersListFromLogs(allLogs) { return await usersService.fetchUsersForKeyValues(admin.firestore.FieldPath.documentId(), Array.from(userIds)); } -function formatLogsForFeed(logs, usersMap) { +async function getTasksFromLogs(allLogs) { + const taskDetails = []; + const taskIds = new Set(); + for (const log of allLogs) { + if (!taskIds.has(log.meta?.taskId || log.body?.taskId)) { + taskIds.add(log.meta?.taskId || log.body?.taskId); + } + } + if (Array.from(taskIds).filter((e) => e).length !== 0) { + const data = await tasksModel + .where( + admin.firestore.FieldPath.documentId(), + "in", + Array.from(taskIds).filter((e) => e) + ) + .get(); + data.forEach((doc) => { + taskDetails.push({ + id: doc.id, + ...doc.data(), + }); + }); + } + return taskDetails; +} + +function formatLogsForFeed(logs, usersMap, tasksMap) { switch (logs.type) { case logType.EXTENSION_REQUESTS: - return formatExtensionRequestsLog(logs, usersMap); + return formatExtensionRequestsLog(logs, usersMap, tasksMap); case logType.REQUEST_CREATED: - return formatRequestCreatedLogs(logs, usersMap); + case logType.REQUEST_APPROVED: + case logType.REQUEST_REJECTED: + return formatOOORequestLogs(logs, usersMap, logType.type); case logType.TASK: - return formatTaskUpdateLogs(logs, usersMap); + return formatTaskUpdateLogs(logs, usersMap, tasksMap); case logType.PROFILE_DIFF_APPROVED: case logType.PROFILE_DIFF_REJECTED: return formatProfileDiffLogs(logs, usersMap, logs.type); + case logType.TASK_REQUESTS: + return formatTaskRequestsLogs(logs, usersMap, tasksMap); default: return {}; } } -function formatRequestCreatedLogs(logsSnapshot, usersMap) { +function formatOOORequestLogs(logsSnapshot, usersMap, type) { const { meta, body } = logsSnapshot; switch (logsSnapshot.body.type) { case "OOO": @@ -39,57 +70,71 @@ function formatRequestCreatedLogs(logsSnapshot, usersMap) { from: body.from, until: body.until, message: body.message, - taskId: meta.taskId, - extensionRequestId: body.extensionRequestId, }; default: return {}; } } -function formatExtensionRequestsLog(logsSnapshot, usersMap) { - const { meta, body } = logsSnapshot; - const formattedLog = { - user: logsSnapshot.meta.username ?? usersMap[logsSnapshot.meta.userId]?.username, +function formatExtensionRequestsLog(logsSnapshot, usersMap, tasksMap) { + const { meta } = logsSnapshot; + return { + user: + usersMap[logsSnapshot.meta.userId]?.username ?? + usersMap[logsSnapshot.meta.createdBy]?.username ?? + logsSnapshot.body.username, taskId: meta.taskId, - extensionRequestId: body.extensionRequestId, - status: body.status, + taskTitle: tasksMap[logsSnapshot.meta.taskId]?.title ?? "Untitled Task", + ...flattenObject(logsSnapshot), }; - - if (body.status === EXTENSION_REQUEST_STATUS.PENDING) { - formattedLog.user = logsSnapshot.meta.username ?? usersMap[logsSnapshot.meta.assignee]?.username; - formattedLog.newEndsOn = body.newEndsOn; - formattedLog.oldEndsOn = body.oldEndsOn; - } - - return formattedLog; } -function formatTaskUpdateLogs(logsSnapshot, usersMap) { - const { meta, body } = logsSnapshot; - switch (logsSnapshot.body.subType) { - case "update": - return { - user: usersMap[meta.userId]?.username, - taskId: meta.taskId, - percentCompleted: body.new?.percentCompleted ?? "", - status: body.new?.status ?? "", - endsOn: body.new?.endsOn ?? "", - }; - default: - return {}; - } +function formatTaskUpdateLogs(logsSnapshot, usersMap, tasksMap) { + const { meta } = logsSnapshot; + return { + user: logsSnapshot.meta.username ?? usersMap[meta.userId]?.username, + taskId: meta.taskId, + taskTitle: tasksMap[meta.taskId]?.title, + ...flattenObject(logsSnapshot), + }; } function formatProfileDiffLogs(logsSnapshot, usersMap, type) { - const { meta } = logsSnapshot; + const { meta, body } = logsSnapshot; const actionKey = type === logType.PROFILE_DIFF_APPROVED ? "approvedBy" : "rejectedBy"; return { user: usersMap[meta.userId]?.username, + message: body.message, // eslint-disable-next-line security/detect-object-injection - actionKey: usersMap[meta[actionKey]]?.username, + [actionKey]: usersMap[meta[actionKey]]?.username, }; } +function formatTaskRequestsLogs(logsSnapshot, usersMap, tasksMap) { + const { meta, body } = logsSnapshot; + const formattedData = flattenObject(logsSnapshot); + return { + user: usersMap[meta.lastModifiedBy]?.username, + taskId: meta.taskId, + taskTitle: tasksMap[body.taskId]?.title, + proposedStartDate: formattedData.users[0].proposedStartDate, + proposedDeadline: formattedData.users[0].proposedDeadline, + ..._.omit(formattedData, "users"), + }; +} + +function flattenObject(obj, prefix = "") { + return Object.keys(obj).reduce((acc, key) => { + if (key !== "timestamp") { + if (typeof obj[key] === "object" && !Array.isArray(obj[key])) { + return { ...acc, ...flattenObject(obj[key], `${key}`) }; + } else { + return { ...acc, [`${key}`]: obj[key] }; + } + } + return acc; + }, {}); +} + function mapify(array, key) { const mappifiedObj = {}; array.forEach((element) => { @@ -107,8 +152,9 @@ function convertTimestamp(timestamp) { } module.exports = { + mapify, convertTimestamp, - getUsersListFromLogs, + getTasksFromLogs, formatLogsForFeed, - mapify, + getUsersListFromLogs, };