From 07b8720b27a496adfc43d76ce4f2ceccca30d25a Mon Sep 17 00:00:00 2001 From: Anuj Chhikara <107175639+AnujChhikara@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:00:50 +0530 Subject: [PATCH] feat: Integrate userData into Progresses API to reduce redundant calls (#2311) * initial * fix typos * using batches to fetch userIds * refactor the function * added test for dev false case * added unit tests * fix response body containing email --------- Co-authored-by: Achintya Chatterjee <55826451+Achintya-Chatterjee@users.noreply.github.com> Co-authored-by: Prakash Choudhary <34452139+prakashchoudhary07@users.noreply.github.com> --- controllers/progresses.js | 2 +- middlewares/validators/progresses.js | 5 ++ models/progresses.js | 55 ++++++++++++- test/integration/progressesTasks.test.js | 99 ++++++++++++++++++++++++ test/integration/progressesUsers.test.js | 79 +++++++++++++++++++ test/unit/models/progresses.test.js | 47 +++++++++++ 6 files changed, 283 insertions(+), 4 deletions(-) create mode 100644 test/unit/models/progresses.test.js diff --git a/controllers/progresses.js b/controllers/progresses.js index cdf1f31bc..5e1b24870 100644 --- a/controllers/progresses.js +++ b/controllers/progresses.js @@ -217,7 +217,7 @@ const getProgressRangeData = async (req, res) => { const getProgressBydDateController = async (req, res) => { try { - const data = await getProgressByDate(req.params); + const data = await getProgressByDate(req.params, req.query); return res.json({ message: PROGRESS_DOCUMENT_RETRIEVAL_SUCCEEDED, data, diff --git a/middlewares/validators/progresses.js b/middlewares/validators/progresses.js index 27af88a39..a1a17ffcc 100644 --- a/middlewares/validators/progresses.js +++ b/middlewares/validators/progresses.js @@ -63,6 +63,9 @@ const validateGetProgressRecordsQuery = async (req, res, next) => { taskId: joi.string().optional().allow("").messages({ "string.base": "taskId must be a string", }), + dev: joi.boolean().optional().messages({ + "boolean.base": "dev must be a boolean value (true or false).", + }), orderBy: joi .string() .optional() @@ -92,6 +95,7 @@ const validateGetRangeProgressRecordsParams = async (req, res, next) => { taskId: joi.string().optional(), startDate: joi.date().iso().required(), endDate: joi.date().iso().min(joi.ref("startDate")).required(), + dev: joi.boolean().optional(), }) .xor("userId", "taskId") .messages({ @@ -121,6 +125,7 @@ const validateGetDayProgressParams = async (req, res, next) => { }), typeId: joi.string().required(), date: joi.date().iso().required(), + dev: joi.boolean().optional(), }); try { await schema.validateAsync(req.params, { abortEarly: false }); diff --git a/models/progresses.js b/models/progresses.js index 8e8a622d9..e406d9f15 100644 --- a/models/progresses.js +++ b/models/progresses.js @@ -13,6 +13,7 @@ const { getProgressDateTimestamp, buildQueryToSearchProgressByDay, } = require("../utils/progresses"); +const { retrieveUsers } = require("../services/dataAccessLayer"); const { PROGRESS_ALREADY_CREATED, PROGRESS_DOCUMENT_NOT_FOUND } = PROGRESSES_RESPONSE_MESSAGES; /** @@ -47,9 +48,14 @@ const createProgressDocument = async (progressData) => { * @throws {Error} If the userId or taskId is invalid or does not exist. **/ const getProgressDocument = async (queryParams) => { + const { dev } = queryParams; await assertUserOrTaskExists(queryParams); const query = buildQueryToFetchDocs(queryParams); const progressDocs = await getProgressDocs(query); + + if (dev === "true") { + return await addUserDetailsToProgressDocs(progressDocs); + } return progressDocs; }; @@ -77,8 +83,9 @@ const getRangeProgressData = async (queryParams) => { * @returns {Promise} A Promise that resolves with the progress records of the queried user or task. * @throws {Error} If the userId or taskId is invalid or does not exist. **/ -async function getProgressByDate(pathParams) { +async function getProgressByDate(pathParams, queryParams) { const { type, typeId, date } = pathParams; + const { dev } = queryParams; await assertUserOrTaskExists({ [TYPE_MAP[type]]: typeId }); const query = buildQueryToSearchProgressByDay({ [TYPE_MAP[type]]: typeId, date }); const result = await query.get(); @@ -86,7 +93,49 @@ async function getProgressByDate(pathParams) { throw new NotFound(PROGRESS_DOCUMENT_NOT_FOUND); } const doc = result.docs[0]; - return { id: doc.id, ...doc.data() }; + const docData = doc.data(); + if (dev === "true") { + const { user: userData } = await retrieveUsers({ id: docData.userId }); + return { id: doc.id, ...docData, userData }; + } + + return { id: doc.id, ...docData }; } -module.exports = { createProgressDocument, getProgressDocument, getRangeProgressData, getProgressByDate }; +/** + * Adds user details to progress documents by fetching unique users. + * This function retrieves user details for each user ID in the progress documents and attaches the user data to each document. + * + * @param {Array} progressDocs - An array of progress documents. Each document should include a `userId` property. + * @returns {Promise>} A Promise that resolves to an array of progress documents with the `userData` field populated. + * If an error occurs while fetching the user details, the `userData` field will be set to `null` for each document. + */ +const addUserDetailsToProgressDocs = async (progressDocs) => { + try { + const uniqueUserIds = [...new Set(progressDocs.map((doc) => doc.userId))]; + + const uniqueUsersData = await retrieveUsers({ + userIds: uniqueUserIds, + }); + const allUsers = uniqueUsersData.flat(); + const userByIdMap = allUsers.reduce((lookup, user) => { + if (user) lookup[user.id] = user; + return lookup; + }, {}); + + return progressDocs.map((doc) => { + const userDetails = userByIdMap[doc.userId] || null; + return { ...doc, userData: userDetails }; + }); + } catch (err) { + return progressDocs.map((doc) => ({ ...doc, userData: null })); + } +}; + +module.exports = { + createProgressDocument, + getProgressDocument, + getRangeProgressData, + getProgressByDate, + addUserDetailsToProgressDocs, +}; diff --git a/test/integration/progressesTasks.test.js b/test/integration/progressesTasks.test.js index 704c534db..14a00ffe6 100644 --- a/test/integration/progressesTasks.test.js +++ b/test/integration/progressesTasks.test.js @@ -222,6 +222,76 @@ describe("Test Progress Updates API for Tasks", function () { }); }); + it("Returns the progress array for the task with userData object", function (done) { + chai + .request(app) + .get(`/progresses?taskId=${taskId1}&dev=true`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(200); + expect(res.body).to.have.keys(["message", "data", "count"]); + expect(res.body.data).to.be.an("array"); + expect(res.body.message).to.be.equal("Progress document retrieved successfully."); + res.body.data.forEach((progress) => { + expect(progress).to.have.keys([ + "id", + "taskId", + "type", + "completed", + "planned", + "blockers", + "userData", + "userId", + "createdAt", + "date", + ]); + }); + return done(); + }); + }); + + it("Returns the progress array for the task without userData field if dev is false", function (done) { + chai + .request(app) + .get(`/progresses?taskId=${taskId1}&dev=false`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(200); + expect(res.body).to.have.keys(["message", "data", "count"]); + expect(res.body.data).to.be.an("array"); + expect(res.body.message).to.be.equal("Progress document retrieved successfully."); + res.body.data.forEach((progress) => { + expect(progress).to.have.keys([ + "id", + "taskId", + "type", + "completed", + "planned", + "blockers", + "userId", + "createdAt", + "date", + ]); + }); + return done(); + }); + }); + + it("Returns a 404 error when the task does not exist", function (done) { + chai + .request(app) + .get(`/progresses?taskId=nonExistingTaskId&dev=true`) + .end((err, res) => { + if (err) return done(err); + + expect(res).to.have.status(404); + expect(res.body).to.have.keys(["message"]); + expect(res.body.message).to.be.equal(`Task with id nonExistingTaskId does not exist.`); + + return done(); + }); + }); + it("Gives 400 status when anything other than -date or date is supplied", function (done) { chai .request(app) @@ -311,6 +381,35 @@ describe("Test Progress Updates API for Tasks", function () { }); }); + it("Returns the progress array for all the tasks with userData object", function (done) { + chai + .request(app) + .get(`/progresses?type=task&dev=true`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(200); + expect(res.body).to.have.keys(["message", "data", "count"]); + expect(res.body.data).to.be.an("array"); + expect(res.body.message).to.be.equal("Progress document retrieved successfully."); + expect(res.body.count).to.be.equal(4); + res.body.data.forEach((progress) => { + expect(progress).to.have.keys([ + "id", + "taskId", + "type", + "completed", + "planned", + "blockers", + "userData", + "userId", + "createdAt", + "date", + ]); + }); + return done(); + }); + }); + it("Returns 400 for bad request", function (done) { chai .request(app) diff --git a/test/integration/progressesUsers.test.js b/test/integration/progressesUsers.test.js index e70d5d317..f2458a576 100644 --- a/test/integration/progressesUsers.test.js +++ b/test/integration/progressesUsers.test.js @@ -226,6 +226,60 @@ describe("Test Progress Updates API for Users", function () { }); }); + it("Returns the progress array for a specific user with userData object", function (done) { + chai + .request(app) + .get(`/progresses?userId=${userId1}&dev=true`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(200); + expect(res.body).to.have.keys(["message", "data", "count"]); + expect(res.body.data).to.be.an("array"); + expect(res.body.message).to.be.equal("Progress document retrieved successfully."); + res.body.data.forEach((progress) => { + expect(progress).to.have.keys([ + "id", + "type", + "completed", + "planned", + "blockers", + "userId", + "userData", + "createdAt", + "date", + ]); + }); + return done(); + }); + }); + + it("Returns the progress array for all the user with userData object when dev is true", function (done) { + chai + .request(app) + .get(`/progresses?type=user&dev=true`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(200); + expect(res.body).to.have.keys(["message", "data", "count"]); + expect(res.body.data).to.be.an("array"); + expect(res.body.message).to.be.equal("Progress document retrieved successfully."); + res.body.data.forEach((progress) => { + expect(progress).to.have.keys([ + "id", + "type", + "completed", + "planned", + "blockers", + "userId", + "userData", + "createdAt", + "date", + ]); + }); + return done(); + }); + }); + it("Returns 400 for bad request", function (done) { chai .request(app) @@ -370,6 +424,31 @@ describe("Test Progress Updates API for Users", function () { }); }); + it("Returns the progress data for a specific user with userData object", function (done) { + chai + .request(app) + .get(`/progresses/user/${userId}/date/2023-05-02?dev=true`) + .end((err, res) => { + if (err) return done(err); + expect(res).to.have.status(200); + expect(res.body).to.have.keys(["message", "data"]); + expect(res.body.data).to.be.an("object"); + expect(res.body.message).to.be.equal("Progress document retrieved successfully."); + expect(res.body.data).to.have.keys([ + "id", + "type", + "completed", + "planned", + "blockers", + "userData", + "userId", + "createdAt", + "date", + ]); + return done(); + }); + }); + it("Should return 404 No progress records found if the document doesn't exist", function (done) { chai .request(app) diff --git a/test/unit/models/progresses.test.js b/test/unit/models/progresses.test.js new file mode 100644 index 000000000..28a57207b --- /dev/null +++ b/test/unit/models/progresses.test.js @@ -0,0 +1,47 @@ +const chai = require("chai"); +const sinon = require("sinon"); +const { expect } = chai; +const { addUserDetailsToProgressDocs } = require("../../../models/progresses"); +const cleanDb = require("../../utils/cleanDb"); +const users = require("../../../models/users"); +const userDataArray = require("../../fixtures/user/user")(); +const { removeSensitiveInfo } = require("../../../services/dataAccessLayer"); +describe("getProgressDocument", function () { + afterEach(function () { + cleanDb(); + sinon.restore(); + }); + + it("should add userData to progress documents correctly", async function () { + const userData = userDataArray[0]; + const userData2 = userDataArray[1]; + const { userId } = await users.addOrUpdate(userData); + const { userId: userId2 } = await users.addOrUpdate(userData2); + const updatedUserData = { ...userData, id: userId }; + const updatedUserData2 = { ...userData2, id: userId2 }; + removeSensitiveInfo(updatedUserData); + removeSensitiveInfo(updatedUserData2); + const mockProgressDocs = [ + { userId: userId, taskId: 101 }, + { userId: userId2, taskId: 102 }, + ]; + + const result = await addUserDetailsToProgressDocs(mockProgressDocs); + + expect(result).to.deep.equal([ + { userId, taskId: 101, userData: updatedUserData }, + { userId: userId2, taskId: 102, userData: updatedUserData2 }, + ]); + }); + + it("should handle errors and set userData as null", async function () { + const userData = userDataArray[0]; + await users.addOrUpdate(userData); + + const mockProgressDocs = [{ userId: "userIdNotExists", taskId: 101 }]; + + const result = await addUserDetailsToProgressDocs(mockProgressDocs); + + expect(result).to.deep.equal([{ userId: "userIdNotExists", taskId: 101, userData: null }]); + }); +});