Skip to content

Commit

Permalink
feat: Integrate userData into Progresses API to reduce redundant calls (
Browse files Browse the repository at this point in the history
#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 <[email protected]>
Co-authored-by: Prakash Choudhary <[email protected]>
  • Loading branch information
3 people authored Jan 6, 2025
1 parent 444e34c commit 07b8720
Show file tree
Hide file tree
Showing 6 changed files with 283 additions and 4 deletions.
2 changes: 1 addition & 1 deletion controllers/progresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions middlewares/validators/progresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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 });
Expand Down
55 changes: 52 additions & 3 deletions models/progresses.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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;
};

Expand Down Expand Up @@ -77,16 +83,59 @@ const getRangeProgressData = async (queryParams) => {
* @returns {Promise<object>} 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 });

Check warning on line 89 in models/progresses.js

View workflow job for this annotation

GitHub Actions / build (20.11.x)

Generic Object Injection Sink
const query = buildQueryToSearchProgressByDay({ [TYPE_MAP[type]]: typeId, date });

Check warning on line 90 in models/progresses.js

View workflow job for this annotation

GitHub Actions / build (20.11.x)

Generic Object Injection Sink
const result = await query.get();
if (!result.size) {
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<object>} progressDocs - An array of progress documents. Each document should include a `userId` property.
* @returns {Promise<Array<object>>} 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,
};
99 changes: 99 additions & 0 deletions test/integration/progressesTasks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
79 changes: 79 additions & 0 deletions test/integration/progressesUsers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
47 changes: 47 additions & 0 deletions test/unit/models/progresses.test.js
Original file line number Diff line number Diff line change
@@ -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 }]);
});
});

0 comments on commit 07b8720

Please sign in to comment.