Skip to content

Commit 07b8720

Browse files
AnujChhikaraAchintya-Chatterjeeprakashchoudhary07
authored
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>
1 parent 444e34c commit 07b8720

File tree

6 files changed

+283
-4
lines changed

6 files changed

+283
-4
lines changed

‎controllers/progresses.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ const getProgressRangeData = async (req, res) => {
217217

218218
const getProgressBydDateController = async (req, res) => {
219219
try {
220-
const data = await getProgressByDate(req.params);
220+
const data = await getProgressByDate(req.params, req.query);
221221
return res.json({
222222
message: PROGRESS_DOCUMENT_RETRIEVAL_SUCCEEDED,
223223
data,

‎middlewares/validators/progresses.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ const validateGetProgressRecordsQuery = async (req, res, next) => {
6363
taskId: joi.string().optional().allow("").messages({
6464
"string.base": "taskId must be a string",
6565
}),
66+
dev: joi.boolean().optional().messages({
67+
"boolean.base": "dev must be a boolean value (true or false).",
68+
}),
6669
orderBy: joi
6770
.string()
6871
.optional()
@@ -92,6 +95,7 @@ const validateGetRangeProgressRecordsParams = async (req, res, next) => {
9295
taskId: joi.string().optional(),
9396
startDate: joi.date().iso().required(),
9497
endDate: joi.date().iso().min(joi.ref("startDate")).required(),
98+
dev: joi.boolean().optional(),
9599
})
96100
.xor("userId", "taskId")
97101
.messages({
@@ -121,6 +125,7 @@ const validateGetDayProgressParams = async (req, res, next) => {
121125
}),
122126
typeId: joi.string().required(),
123127
date: joi.date().iso().required(),
128+
dev: joi.boolean().optional(),
124129
});
125130
try {
126131
await schema.validateAsync(req.params, { abortEarly: false });

‎models/progresses.js‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const {
1313
getProgressDateTimestamp,
1414
buildQueryToSearchProgressByDay,
1515
} = require("../utils/progresses");
16+
const { retrieveUsers } = require("../services/dataAccessLayer");
1617
const { PROGRESS_ALREADY_CREATED, PROGRESS_DOCUMENT_NOT_FOUND } = PROGRESSES_RESPONSE_MESSAGES;
1718

1819
/**
@@ -47,9 +48,14 @@ const createProgressDocument = async (progressData) => {
4748
* @throws {Error} If the userId or taskId is invalid or does not exist.
4849
**/
4950
const getProgressDocument = async (queryParams) => {
51+
const { dev } = queryParams;
5052
await assertUserOrTaskExists(queryParams);
5153
const query = buildQueryToFetchDocs(queryParams);
5254
const progressDocs = await getProgressDocs(query);
55+
56+
if (dev === "true") {
57+
return await addUserDetailsToProgressDocs(progressDocs);
58+
}
5359
return progressDocs;
5460
};
5561

@@ -77,16 +83,59 @@ const getRangeProgressData = async (queryParams) => {
7783
* @returns {Promise<object>} A Promise that resolves with the progress records of the queried user or task.
7884
* @throws {Error} If the userId or taskId is invalid or does not exist.
7985
**/
80-
async function getProgressByDate(pathParams) {
86+
async function getProgressByDate(pathParams, queryParams) {
8187
const { type, typeId, date } = pathParams;
88+
const { dev } = queryParams;
8289
await assertUserOrTaskExists({ [TYPE_MAP[type]]: typeId });
8390
const query = buildQueryToSearchProgressByDay({ [TYPE_MAP[type]]: typeId, date });
8491
const result = await query.get();
8592
if (!result.size) {
8693
throw new NotFound(PROGRESS_DOCUMENT_NOT_FOUND);
8794
}
8895
const doc = result.docs[0];
89-
return { id: doc.id, ...doc.data() };
96+
const docData = doc.data();
97+
if (dev === "true") {
98+
const { user: userData } = await retrieveUsers({ id: docData.userId });
99+
return { id: doc.id, ...docData, userData };
100+
}
101+
102+
return { id: doc.id, ...docData };
90103
}
91104

92-
module.exports = { createProgressDocument, getProgressDocument, getRangeProgressData, getProgressByDate };
105+
/**
106+
* Adds user details to progress documents by fetching unique users.
107+
* This function retrieves user details for each user ID in the progress documents and attaches the user data to each document.
108+
*
109+
* @param {Array<object>} progressDocs - An array of progress documents. Each document should include a `userId` property.
110+
* @returns {Promise<Array<object>>} A Promise that resolves to an array of progress documents with the `userData` field populated.
111+
* If an error occurs while fetching the user details, the `userData` field will be set to `null` for each document.
112+
*/
113+
const addUserDetailsToProgressDocs = async (progressDocs) => {
114+
try {
115+
const uniqueUserIds = [...new Set(progressDocs.map((doc) => doc.userId))];
116+
117+
const uniqueUsersData = await retrieveUsers({
118+
userIds: uniqueUserIds,
119+
});
120+
const allUsers = uniqueUsersData.flat();
121+
const userByIdMap = allUsers.reduce((lookup, user) => {
122+
if (user) lookup[user.id] = user;
123+
return lookup;
124+
}, {});
125+
126+
return progressDocs.map((doc) => {
127+
const userDetails = userByIdMap[doc.userId] || null;
128+
return { ...doc, userData: userDetails };
129+
});
130+
} catch (err) {
131+
return progressDocs.map((doc) => ({ ...doc, userData: null }));
132+
}
133+
};
134+
135+
module.exports = {
136+
createProgressDocument,
137+
getProgressDocument,
138+
getRangeProgressData,
139+
getProgressByDate,
140+
addUserDetailsToProgressDocs,
141+
};

‎test/integration/progressesTasks.test.js‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,76 @@ describe("Test Progress Updates API for Tasks", function () {
222222
});
223223
});
224224

225+
it("Returns the progress array for the task with userData object", function (done) {
226+
chai
227+
.request(app)
228+
.get(`/progresses?taskId=${taskId1}&dev=true`)
229+
.end((err, res) => {
230+
if (err) return done(err);
231+
expect(res).to.have.status(200);
232+
expect(res.body).to.have.keys(["message", "data", "count"]);
233+
expect(res.body.data).to.be.an("array");
234+
expect(res.body.message).to.be.equal("Progress document retrieved successfully.");
235+
res.body.data.forEach((progress) => {
236+
expect(progress).to.have.keys([
237+
"id",
238+
"taskId",
239+
"type",
240+
"completed",
241+
"planned",
242+
"blockers",
243+
"userData",
244+
"userId",
245+
"createdAt",
246+
"date",
247+
]);
248+
});
249+
return done();
250+
});
251+
});
252+
253+
it("Returns the progress array for the task without userData field if dev is false", function (done) {
254+
chai
255+
.request(app)
256+
.get(`/progresses?taskId=${taskId1}&dev=false`)
257+
.end((err, res) => {
258+
if (err) return done(err);
259+
expect(res).to.have.status(200);
260+
expect(res.body).to.have.keys(["message", "data", "count"]);
261+
expect(res.body.data).to.be.an("array");
262+
expect(res.body.message).to.be.equal("Progress document retrieved successfully.");
263+
res.body.data.forEach((progress) => {
264+
expect(progress).to.have.keys([
265+
"id",
266+
"taskId",
267+
"type",
268+
"completed",
269+
"planned",
270+
"blockers",
271+
"userId",
272+
"createdAt",
273+
"date",
274+
]);
275+
});
276+
return done();
277+
});
278+
});
279+
280+
it("Returns a 404 error when the task does not exist", function (done) {
281+
chai
282+
.request(app)
283+
.get(`/progresses?taskId=nonExistingTaskId&dev=true`)
284+
.end((err, res) => {
285+
if (err) return done(err);
286+
287+
expect(res).to.have.status(404);
288+
expect(res.body).to.have.keys(["message"]);
289+
expect(res.body.message).to.be.equal(`Task with id nonExistingTaskId does not exist.`);
290+
291+
return done();
292+
});
293+
});
294+
225295
it("Gives 400 status when anything other than -date or date is supplied", function (done) {
226296
chai
227297
.request(app)
@@ -311,6 +381,35 @@ describe("Test Progress Updates API for Tasks", function () {
311381
});
312382
});
313383

384+
it("Returns the progress array for all the tasks with userData object", function (done) {
385+
chai
386+
.request(app)
387+
.get(`/progresses?type=task&dev=true`)
388+
.end((err, res) => {
389+
if (err) return done(err);
390+
expect(res).to.have.status(200);
391+
expect(res.body).to.have.keys(["message", "data", "count"]);
392+
expect(res.body.data).to.be.an("array");
393+
expect(res.body.message).to.be.equal("Progress document retrieved successfully.");
394+
expect(res.body.count).to.be.equal(4);
395+
res.body.data.forEach((progress) => {
396+
expect(progress).to.have.keys([
397+
"id",
398+
"taskId",
399+
"type",
400+
"completed",
401+
"planned",
402+
"blockers",
403+
"userData",
404+
"userId",
405+
"createdAt",
406+
"date",
407+
]);
408+
});
409+
return done();
410+
});
411+
});
412+
314413
it("Returns 400 for bad request", function (done) {
315414
chai
316415
.request(app)

‎test/integration/progressesUsers.test.js‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,60 @@ describe("Test Progress Updates API for Users", function () {
226226
});
227227
});
228228

229+
it("Returns the progress array for a specific user with userData object", function (done) {
230+
chai
231+
.request(app)
232+
.get(`/progresses?userId=${userId1}&dev=true`)
233+
.end((err, res) => {
234+
if (err) return done(err);
235+
expect(res).to.have.status(200);
236+
expect(res.body).to.have.keys(["message", "data", "count"]);
237+
expect(res.body.data).to.be.an("array");
238+
expect(res.body.message).to.be.equal("Progress document retrieved successfully.");
239+
res.body.data.forEach((progress) => {
240+
expect(progress).to.have.keys([
241+
"id",
242+
"type",
243+
"completed",
244+
"planned",
245+
"blockers",
246+
"userId",
247+
"userData",
248+
"createdAt",
249+
"date",
250+
]);
251+
});
252+
return done();
253+
});
254+
});
255+
256+
it("Returns the progress array for all the user with userData object when dev is true", function (done) {
257+
chai
258+
.request(app)
259+
.get(`/progresses?type=user&dev=true`)
260+
.end((err, res) => {
261+
if (err) return done(err);
262+
expect(res).to.have.status(200);
263+
expect(res.body).to.have.keys(["message", "data", "count"]);
264+
expect(res.body.data).to.be.an("array");
265+
expect(res.body.message).to.be.equal("Progress document retrieved successfully.");
266+
res.body.data.forEach((progress) => {
267+
expect(progress).to.have.keys([
268+
"id",
269+
"type",
270+
"completed",
271+
"planned",
272+
"blockers",
273+
"userId",
274+
"userData",
275+
"createdAt",
276+
"date",
277+
]);
278+
});
279+
return done();
280+
});
281+
});
282+
229283
it("Returns 400 for bad request", function (done) {
230284
chai
231285
.request(app)
@@ -370,6 +424,31 @@ describe("Test Progress Updates API for Users", function () {
370424
});
371425
});
372426

427+
it("Returns the progress data for a specific user with userData object", function (done) {
428+
chai
429+
.request(app)
430+
.get(`/progresses/user/${userId}/date/2023-05-02?dev=true`)
431+
.end((err, res) => {
432+
if (err) return done(err);
433+
expect(res).to.have.status(200);
434+
expect(res.body).to.have.keys(["message", "data"]);
435+
expect(res.body.data).to.be.an("object");
436+
expect(res.body.message).to.be.equal("Progress document retrieved successfully.");
437+
expect(res.body.data).to.have.keys([
438+
"id",
439+
"type",
440+
"completed",
441+
"planned",
442+
"blockers",
443+
"userData",
444+
"userId",
445+
"createdAt",
446+
"date",
447+
]);
448+
return done();
449+
});
450+
});
451+
373452
it("Should return 404 No progress records found if the document doesn't exist", function (done) {
374453
chai
375454
.request(app)

‎test/unit/models/progresses.test.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
const chai = require("chai");
2+
const sinon = require("sinon");
3+
const { expect } = chai;
4+
const { addUserDetailsToProgressDocs } = require("../../../models/progresses");
5+
const cleanDb = require("../../utils/cleanDb");
6+
const users = require("../../../models/users");
7+
const userDataArray = require("../../fixtures/user/user")();
8+
const { removeSensitiveInfo } = require("../../../services/dataAccessLayer");
9+
describe("getProgressDocument", function () {
10+
afterEach(function () {
11+
cleanDb();
12+
sinon.restore();
13+
});
14+
15+
it("should add userData to progress documents correctly", async function () {
16+
const userData = userDataArray[0];
17+
const userData2 = userDataArray[1];
18+
const { userId } = await users.addOrUpdate(userData);
19+
const { userId: userId2 } = await users.addOrUpdate(userData2);
20+
const updatedUserData = { ...userData, id: userId };
21+
const updatedUserData2 = { ...userData2, id: userId2 };
22+
removeSensitiveInfo(updatedUserData);
23+
removeSensitiveInfo(updatedUserData2);
24+
const mockProgressDocs = [
25+
{ userId: userId, taskId: 101 },
26+
{ userId: userId2, taskId: 102 },
27+
];
28+
29+
const result = await addUserDetailsToProgressDocs(mockProgressDocs);
30+
31+
expect(result).to.deep.equal([
32+
{ userId, taskId: 101, userData: updatedUserData },
33+
{ userId: userId2, taskId: 102, userData: updatedUserData2 },
34+
]);
35+
});
36+
37+
it("should handle errors and set userData as null", async function () {
38+
const userData = userDataArray[0];
39+
await users.addOrUpdate(userData);
40+
41+
const mockProgressDocs = [{ userId: "userIdNotExists", taskId: 101 }];
42+
43+
const result = await addUserDetailsToProgressDocs(mockProgressDocs);
44+
45+
expect(result).to.deep.equal([{ userId: "userIdNotExists", taskId: 101, userData: null }]);
46+
});
47+
});

0 commit comments

Comments
 (0)