From e356eb0e3c8b1490d45e89ce79447a2efdebe20c Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Thu, 16 Nov 2023 03:31:24 +0530 Subject: [PATCH 01/18] feat : adds function to reject task requests --- constants/taskRequests.ts | 6 ++++- controllers/tasksRequests.js | 43 +++++++++++++++++++++++++----------- models/taskRequests.js | 36 +++++++++++++++++++++++++----- routes/taskRequests.js | 3 ++- 4 files changed, 68 insertions(+), 20 deletions(-) diff --git a/constants/taskRequests.ts b/constants/taskRequests.ts index c1b4933f1..1ca3a0360 100644 --- a/constants/taskRequests.ts +++ b/constants/taskRequests.ts @@ -5,6 +5,10 @@ const TASK_REQUEST_STATUS = { DENIED: "DENIED", }; +const TASK_REQUEST_UPDATE_ACTION = { + APPROVE: "approve", + REJECT: "reject", +}; const TASK_REQUEST_TYPE = { ASSIGNMENT: "ASSIGNMENT", CREATION: "CREATION", @@ -14,4 +18,4 @@ const MIGRATION_TYPE = { ADD_NEW_FIELDS: "add-new-fields", REMOVE_OLD_FIELDS: "remove-redundant-fields", }; -module.exports = { TASK_REQUEST_STATUS, TASK_REQUEST_TYPE, MIGRATION_TYPE }; +module.exports = { TASK_REQUEST_STATUS, TASK_REQUEST_TYPE, MIGRATION_TYPE, TASK_REQUEST_UPDATE_ACTION }; diff --git a/controllers/tasksRequests.js b/controllers/tasksRequests.js index 16f2f67fe..9d1a4d5ee 100644 --- a/controllers/tasksRequests.js +++ b/controllers/tasksRequests.js @@ -1,5 +1,5 @@ const { INTERNAL_SERVER_ERROR, SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); -const { TASK_REQUEST_TYPE, MIGRATION_TYPE } = require("../constants/taskRequests"); +const { TASK_REQUEST_TYPE, MIGRATION_TYPE, TASK_REQUEST_UPDATE_ACTION } = require("../constants/taskRequests"); const { addLog } = require("../models/logs"); const taskRequestsModel = require("../models/taskRequests"); const tasksModel = require("../models/tasks.js"); @@ -95,7 +95,7 @@ const addTaskRequests = async (req, res) => { break; } } - const newTaskRequest = await taskRequestsModel.createRequest(taskRequestData, req.userData.username); + const newTaskRequest = await taskRequestsModel.createRequest(taskRequestData, req.userData.id); if (newTaskRequest.isCreationRequestApproved) { return res.boom.conflict("Task exists for the given issue."); @@ -109,9 +109,9 @@ const addTaskRequests = async (req, res) => { meta: { taskRequestId: newTaskRequest.id, action: "create", - createdBy: req.userData.username, + createdBy: req.userData.id, createdAt: Date.now(), - lastModifiedBy: req.userData.username, + lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), }, body: newTaskRequest.taskRequest, @@ -167,14 +167,29 @@ const addOrUpdate = async (req, res) => { } }; -const approveTaskRequest = async (req, res) => { +const updateTaskRequests = async (req, res) => { try { const { taskRequestId, user } = req.body; if (!taskRequestId) { return res.boom.badRequest("taskRequestId not provided"); } - const response = await taskRequestsModel.approveTaskRequest(taskRequestId, user); + const { action = TASK_REQUEST_UPDATE_ACTION.APPROVE } = req.query; + + let response = {}; + switch (action) { + case TASK_REQUEST_UPDATE_ACTION.APPROVE: { + response = await taskRequestsModel.approveTaskRequest(taskRequestId, user, req.userData.id); + break; + } + case TASK_REQUEST_UPDATE_ACTION.REJECT: { + response = await taskRequestsModel.rejectTaskRequest(taskRequestId, req.userData.id); + break; + } + default: { + return res.boom.badRequest("Unknown action"); + } + } if (response.taskRequestNotFound) { return res.boom.badRequest("Task request not found."); @@ -186,17 +201,19 @@ const approveTaskRequest = async (req, res) => { return res.boom.badRequest("Task request was previously approved or rejected."); } - await updateUserStatusOnTaskUpdate(user.username); + if (action && action === TASK_REQUEST_UPDATE_ACTION.APPROVE) { + await updateUserStatusOnTaskUpdate(user.username); + } const taskRequestLog = { type: "taskRequests", meta: { taskRequestId: taskRequestId, action: "update", - subAction: "approve", - createdBy: req.userData.username, + subAction: action, + createdBy: req.userData.id, createdAt: Date.now(), - lastModifiedBy: req.userData.username, + lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), }, body: response.taskRequest, @@ -204,8 +221,8 @@ const approveTaskRequest = async (req, res) => { await addLog(taskRequestLog.type, taskRequestLog.meta, taskRequestLog.body); return res.status(200).json({ - message: `Task successfully assigned to user ${response.approvedTo}`, - taskRequest: response.taskRequest, + message: `Task updated successfully.`, + taskRequest: response?.taskRequest, }); } catch (err) { logger.error("Error while approving task request", err); @@ -237,7 +254,7 @@ const migrateTaskRequests = async (req, res) => { } }; module.exports = { - approveTaskRequest, + updateTaskRequests, addOrUpdate, fetchTaskRequests, fetchTaskRequestById, diff --git a/models/taskRequests.js b/models/taskRequests.js index 0f5f09078..377b3ee08 100644 --- a/models/taskRequests.js +++ b/models/taskRequests.js @@ -90,7 +90,7 @@ const fetchTaskRequestById = async (taskRequestId) => { }; }; -const createRequest = async (data, authenticatedUsername) => { +const createRequest = async (data, authenticatedUserId) => { try { const queryFieldPath = data.requestType === TASK_REQUEST_TYPE.CREATION ? "externalIssueUrl" : "taskId"; const queryValue = data.requestType === TASK_REQUEST_TYPE.CREATION ? data.externalIssueUrl : data.taskId; @@ -136,7 +136,7 @@ const createRequest = async (data, authenticatedUsername) => { const updatedTaskRequest = { requestors: updatedRequestors, users: updatedUsers, - lastModifiedBy: authenticatedUsername, + lastModifiedBy: authenticatedUserId, lastModifiedAt: Date.now(), }; await taskRequestsCollection.doc(taskRequestRef.id).update(updatedTaskRequest); @@ -157,9 +157,9 @@ const createRequest = async (data, authenticatedUsername) => { externalIssueUrl: data.externalIssueUrl, requestType: data.requestType, users: [userRequest], - createdBy: authenticatedUsername, + createdBy: authenticatedUserId, createdAt: Date.now(), - lastModifiedBy: authenticatedUsername, + lastModifiedBy: authenticatedUserId, lastModifiedAt: Date.now(), }; if (!newTaskRequest.externalIssueUrl) delete newTaskRequest.externalIssueUrl; @@ -230,7 +230,7 @@ const addOrUpdate = async (taskId, userId) => { * @param userId { Object }: user whose being approved * @return {Promise<{approvedTo: string, taskRequest: Object}>} */ -const approveTaskRequest = async (taskRequestId, user) => { +const approveTaskRequest = async (taskRequestId, user, authenticatedUserId) => { try { return await firestore.runTransaction(async (transaction) => { const taskRequestDocRef = taskRequestsCollection.doc(taskRequestId); @@ -267,6 +267,8 @@ const approveTaskRequest = async (taskRequestId, user) => { users: taskRequestData.users, approvedTo: user.id, status: TASK_REQUEST_STATUS.APPROVED, + lastModifiedBy: authenticatedUserId, + lastModifiedAt: Date.now(), }; // End of TODO const updateTaskRequestPromise = transaction.update(taskRequestDocRef, updatedTaskRequest); @@ -300,6 +302,8 @@ const approveTaskRequest = async (taskRequestId, user) => { const updatedTaskRequest = { approvedTo: user.id, status: TASK_REQUEST_STATUS.APPROVED, + lastModifiedBy: authenticatedUserId, + lastModifiedAt: Date.now(), }; let userRequestData; if (taskRequestData.users) { @@ -338,6 +342,27 @@ const approveTaskRequest = async (taskRequestId, user) => { } }; +const rejectTaskRequest = async (taskRequestId, authenticatedUserId) => { + const taskRequestDoc = taskRequestsCollection.doc(taskRequestId); + const taskRequestData = (await taskRequestDoc.get()).data(); + if (!taskRequestData) { + return { taskRequestNotFound: true }; + } + + if ( + taskRequestData.status === TASK_REQUEST_STATUS.APPROVED || + taskRequestData.status === TASK_REQUEST_STATUS.DENIED + ) { + return { isTaskRequestInvalid: true }; + } + const updatedTaskRequest = { + status: TASK_REQUEST_STATUS.DENIED, + lastModifiedBy: authenticatedUserId, + lastModifiedAt: Date.now(), + }; + await taskRequestDoc.update(updatedTaskRequest); + return { ...taskRequestData, ...updatedTaskRequest }; +}; const addNewFields = async () => { const taskRequestsSnapshots = (await taskRequestsCollection.get()).docs; @@ -408,4 +433,5 @@ module.exports = { approveTaskRequest, addNewFields, removeOldField, + rejectTaskRequest, }; diff --git a/routes/taskRequests.js b/routes/taskRequests.js index bdc227c5f..314763afc 100644 --- a/routes/taskRequests.js +++ b/routes/taskRequests.js @@ -10,7 +10,8 @@ const validators = require("../middlewares/validators/task-requests"); router.get("/", authenticate, authorizeRoles([SUPERUSER]), taskRequests.fetchTaskRequests); router.get("/:id", authenticate, authorizeRoles([SUPERUSER]), taskRequests.fetchTaskRequestById); router.post("/addOrUpdate", authenticate, validateUser, taskRequests.addOrUpdate); -router.patch("/approve", authenticate, authorizeRoles([SUPERUSER]), validateUser, taskRequests.approveTaskRequest); +router.patch("/approve", authenticate, authorizeRoles([SUPERUSER]), validateUser, taskRequests.updateTaskRequests); +router.patch("/", authenticate, authorizeRoles([SUPERUSER]), validateUser, taskRequests.updateTaskRequests); router.post("/", authenticate, validators.postTaskRequests, taskRequests.addTaskRequests); router.post("/migrations", authenticate, authorizeRoles([SUPERUSER]), taskRequests.migrateTaskRequests); From ab691c1a9b7f0da1ee93d991577cc001b3225548 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Fri, 17 Nov 2023 15:24:18 +0530 Subject: [PATCH 02/18] feat : adds tests and minor changes in validation --- middlewares/taskRequests.js | 5 + models/taskRequests.js | 2 +- test/integration/taskRequests.test.js | 367 ++++++++++++++++++++++++- test/unit/models/task-requests.test.js | 37 ++- 4 files changed, 402 insertions(+), 9 deletions(-) diff --git a/middlewares/taskRequests.js b/middlewares/taskRequests.js index b87c57487..2a2ff0b15 100644 --- a/middlewares/taskRequests.js +++ b/middlewares/taskRequests.js @@ -1,5 +1,6 @@ const { SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); const dataAccess = require("../services/dataAccessLayer"); +const { TASK_REQUEST_UPDATE_ACTION } = require("../constants/taskRequests"); /** * Validates user id for task request * @@ -7,6 +8,10 @@ const dataAccess = require("../services/dataAccessLayer"); */ async function validateUser(req, res, next) { try { + const { action } = req.query; + if (action === TASK_REQUEST_UPDATE_ACTION.REJECT) { + return next(); + } const { userId } = req.body; if (!userId) { diff --git a/models/taskRequests.js b/models/taskRequests.js index b07e534e0..c84033dc1 100644 --- a/models/taskRequests.js +++ b/models/taskRequests.js @@ -458,7 +458,7 @@ const rejectTaskRequest = async (taskRequestId, authenticatedUserId) => { lastModifiedAt: Date.now(), }; await taskRequestDoc.update(updatedTaskRequest); - return { ...taskRequestData, ...updatedTaskRequest }; + return { taskRequest: { ...taskRequestData, ...updatedTaskRequest } }; }; const addNewFields = async () => { const taskRequestsSnapshots = (await taskRequestsCollection.get()).docs; diff --git a/test/integration/taskRequests.test.js b/test/integration/taskRequests.test.js index eb6233b75..73bd1a30c 100644 --- a/test/integration/taskRequests.test.js +++ b/test/integration/taskRequests.test.js @@ -21,7 +21,7 @@ const { MIGRATION_TYPE } = require("../../constants/taskRequests"); chai.use(chaiHttp); const config = require("config"); -const { TASK_REQUEST_TYPE } = require("../../constants/taskRequests"); +const { TASK_REQUEST_TYPE, TASK_REQUEST_UPDATE_ACTION } = require("../../constants/taskRequests"); const usersUtils = require("../../utils/users"); const githubService = require("../../services/githubService"); const { userState } = require("../../constants/userStatus"); @@ -433,7 +433,7 @@ describe("Task Requests", function () { } expect(res).to.have.status(200); - expect(res.body.message).to.equal(`Task successfully assigned to user ${member.username}`); + expect(res.body.message).to.equal(`Task updated successfully.`); return done(); }); }); @@ -639,6 +639,369 @@ describe("Task Requests", function () { }); }); + describe("PATCH /taskRequests/ - updates task request", function () { + let activeUserId, oooUserId; + + describe("When the user is super user", function () { + beforeEach(async function () { + userId = await addUser(member); + activeUserId = await addUser(activeMember); + oooUserId = await addUser(member2); + superUserId = await addUser(superUser); + + jwt = authService.generateAuthToken({ userId: superUserId }); + sinon.stub(authService, "verifyAuthToken").callsFake(() => ({ userId: superUserId })); + + taskId = (await tasksModel.updateTask(taskData[4])).taskId; + + await userStatusModel.updateUserStatus(userId, idleUserStatus); + await userStatusModel.updateUserStatus(activeUserId, activeUserStatus); + await userStatusModel.updateUserStatus(oooUserId, { ...oooUserStatus }); + await taskRequestsModel.addOrUpdate(taskId, userId); + }); + afterEach(async function () { + sinon.restore(); + await cleanDb(); + }); + + it("should match response for successful approval", function (done) { + sinon.stub(taskRequestsModel, "approveTaskRequest").resolves({ approvedTo: member.username, taskRequest: {} }); + chai + .request(app) + .patch("/taskRequests/") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ + taskRequestId: taskId, + userId, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(200); + expect(res.body.message).to.equal(`Task updated successfully.`); + return done(); + }); + }); + + it("should match response for successful rejection", function (done) { + sinon.stub(taskRequestsModel, "rejectTaskRequest").resolves({ taskRequest: {} }); + chai + .request(app) + .patch("/taskRequests/") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .send({ + taskRequestId: taskId, + userId, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(200); + expect(res.body.message).to.equal(`Task updated successfully.`); + return done(); + }); + }); + it("should throw 400 error when taskRequestId is missing", function (done) { + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ + userId: oooUserId, + }) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(400); + expect(res.body.message).to.equal("taskRequestId not provided"); + return done(); + }); + }); + + it("should throw 400 error when task request id provided doesn't exist", function (done) { + sinon.stub(taskRequestsModel, "approveTaskRequest").resolves({ taskRequestNotFound: true }); + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ taskRequestId: taskId, userId, activeUserId }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res.body.message).to.equal("Task request not found."); + expect(res).to.have.status(400); + + return done(); + }); + }); + + it("should throw 400 error when task request id provided doesn't exist for rejection", function (done) { + sinon.stub(taskRequestsModel, "rejectTaskRequest").resolves({ taskRequestNotFound: true }); + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .send({ taskRequestId: taskId, userId, activeUserId }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res.body.message).to.equal("Task request not found."); + expect(res).to.have.status(400); + + return done(); + }); + }); + + it("should throw 400 error when user did not request for a task", function (done) { + sinon.stub(taskRequestsModel, "approveTaskRequest").resolves({ isUserInvalid: true }); + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ taskRequestId: taskId, userId, activeUserId }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res.body.message).to.equal("User request not available."); + expect(res).to.have.status(400); + + return done(); + }); + }); + it("should throw 400 error when task was previously approved or rejected.", function (done) { + sinon.stub(taskRequestsModel, "approveTaskRequest").resolves({ isTaskRequestInvalid: true }); + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ taskRequestId: taskId, userId, activeUserId }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res.body.message).to.equal("Task request was previously approved or rejected."); + expect(res).to.have.status(400); + + return done(); + }); + }); + + it("should throw 400 error when task was previously approved or rejected for rejection", function (done) { + sinon.stub(taskRequestsModel, "rejectTaskRequest").resolves({ isTaskRequestInvalid: true }); + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .send({ taskRequestId: taskId, userId, activeUserId }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res.body.message).to.equal("Task request was previously approved or rejected."); + expect(res).to.have.status(400); + + return done(); + }); + }); + it("should throw 400 error when userId is missing", function (done) { + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ taskRequestId: taskId }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body.message).to.equal("userId not provided"); + return done(); + }); + }); + + describe("Checks the user status", function () { + it("Should change the user status to ACTIVE when request is successful for approval", async function () { + sinon + .stub(taskRequestsModel, "approveTaskRequest") + .resolves({ approvedTo: member.username, taskRequest: { taskRequestId: taskId } }); + const res = await chai + .request(app) + .patch("/taskRequests") + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + taskRequestId: taskId, + userId, + }); + + expect(res).to.have.status(200); + const userStatus = await userStatusModel.getUserStatus(userId); + expect(userStatus.data.currentStatus.state).to.be.equal(userState.ACTIVE); + }); + it("Should not change the user status to ACTIVE when request is successful for rejection", async function () { + sinon + .stub(taskRequestsModel, "rejectTaskRequest") + .resolves({ approvedTo: member.username, taskRequest: { taskRequestId: taskId } }); + const res = await chai + .request(app) + .patch("/taskRequests") + .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + taskRequestId: taskId, + userId, + }); + + expect(res).to.have.status(200); + const userStatus = await userStatusModel.getUserStatus(userId); + expect(userStatus.data.currentStatus.state).to.be.equal(userState.IDLE); + }); + it("Should not change the user status to ACTIVE when request is unsuccessful", async function () { + sinon.stub(taskRequestsModel, "approveTaskRequest").resolves({ isTaskRequestInvalid: true }); + const res = await chai + .request(app) + .patch("/taskRequests/approve") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ + taskRequestId: taskId, + userId, + }); + + expect(res).to.have.status(400); + const userStatus = await userStatusModel.getUserStatus(userId); + expect(userStatus.data.currentStatus.state).to.be.equal(userState.IDLE); + }); + }); + }); + + describe("task request logs", function () { + beforeEach(async function () { + userId = await addUser(member); + superUserId = await addUser(superUser); + + jwt = authService.generateAuthToken({ userId: superUserId }); + sinon.stub(authService, "verifyAuthToken").callsFake(() => ({ userId: superUserId })); + await userStatusModel.updateUserStatus(userId, idleUserStatus); + taskId = (await tasksModel.updateTask(taskData[4])).taskId; + }); + afterEach(async function () { + sinon.restore(); + await cleanDb(); + }); + it("should save logs of approved requests", async function () { + sinon + .stub(taskRequestsModel, "approveTaskRequest") + .resolves({ approvedTo: member.username, taskRequest: { taskRequestId: taskId } }); + await chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ + taskRequestId: taskId, + userId, + }); + const logsRef = await logsModel.where("type", "==", "taskRequests").get(); + let taskRequestLogs; + logsRef.forEach((data) => { + taskRequestLogs = data.data(); + }); + expect(taskRequestLogs.body.taskRequestId).to.be.equal(taskId); + }); + + it("should save logs of rejected requests", async function () { + sinon.stub(taskRequestsModel, "rejectTaskRequest").resolves({ taskRequest: { taskRequestId: taskId } }); + await chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .send({ + taskRequestId: taskId, + userId, + }); + const logsRef = await logsModel.where("type", "==", "taskRequests").get(); + let taskRequestLogs; + logsRef.forEach((data) => { + taskRequestLogs = data.data(); + }); + expect(taskRequestLogs.body.taskRequestId).to.be.equal(taskId); + }); + it("should not save logs of failed requests", async function () { + sinon.stub(taskRequestsModel, "approveTaskRequest").resolves({ taskRequestNotFound: true }); + await chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ + taskRequestId: taskId, + userId, + }); + const logsRef = await logsModel.where("type", "==", "taskRequests").get(); + let taskRequestLogs; + logsRef.forEach((data) => { + taskRequestLogs = data.data(); + }); + expect(taskRequestLogs).to.be.equal(undefined); + }); + }); + describe("When the user is not super user", function () { + before(async function () { + userId = await addUser(member); + jwt = authService.generateAuthToken({ userId }); + sinon.stub(authService, "verifyAuthToken").callsFake(() => ({ userId })); + + taskId = (await tasksModel.updateTask(taskData[4])).taskId; + await userStatusModel.updateUserStatus(userId, idleUserStatus); + await taskRequestsModel.addOrUpdate(taskId, userId); + }); + + it("should return unauthorized user response", function (done) { + chai + .request(app) + .patch("/taskRequests") + .set("cookie", `${cookieName}=${jwt}`) + .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .send({ + taskRequestId: taskId, + userId, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(401); + return done(); + }); + }); + }); + }); describe("POST /taskRequests", function () { let fetchIssuesByIdStub; let fetchTaskStub; diff --git a/test/unit/models/task-requests.test.js b/test/unit/models/task-requests.test.js index da8292478..2d98c2572 100644 --- a/test/unit/models/task-requests.test.js +++ b/test/unit/models/task-requests.test.js @@ -8,6 +8,7 @@ const { fetchPaginatedTaskRequests, addNewFields, removeOldField, + rejectTaskRequest, } = require("./../../../models/taskRequests"); const { TASK_REQUEST_TYPE, @@ -348,11 +349,12 @@ describe("Task requests | models", function () { describe("approveTaskRequest", function () { const user = { id: "user123", username: "testUser" }; const taskRequestId = "taskRequest123"; + const authenticatedUserId = "userId"; it("should approve a task request for creation", async function () { const existingTaskRequest = { ...mockData.existingTaskRequest, requestType: TASK_REQUEST_TYPE.CREATION }; await taskRequestsCollection.doc(taskRequestId).set(existingTaskRequest); - const result = await approveTaskRequest(taskRequestId, user); + const result = await approveTaskRequest(taskRequestId, user, authenticatedUserId); const approvedTaskRequest = result.taskRequest; expect(approvedTaskRequest.status).to.equal(TASK_REQUEST_STATUS.APPROVED); expect(approvedTaskRequest.approvedTo).to.equal(user.id); @@ -368,7 +370,7 @@ describe("Task requests | models", function () { const existingTaskRequest = { ...mockData.existingTaskRequest, requestType: TASK_REQUEST_TYPE.ASSIGNMENT }; await taskRequestsCollection.doc(taskRequestId).set(existingTaskRequest); await tasksCollection.doc(existingTaskRequest.taskId).set(tasksData[0]); - const result = await approveTaskRequest(taskRequestId, user); + const result = await approveTaskRequest(taskRequestId, user, authenticatedUserId); const approvedTaskRequest = result.taskRequest; expect(approvedTaskRequest.status).to.equal(TASK_REQUEST_STATUS.APPROVED); expect(approvedTaskRequest.approvedTo).to.equal(user.id); @@ -382,23 +384,23 @@ describe("Task requests | models", function () { const existingTaskRequest = { ...mockData.existingTaskRequest }; await taskRequestsCollection.doc(taskRequestId).set(existingTaskRequest); const invalidUser = { id: "invalidUserId", username: "invalidUser" }; - const result = await approveTaskRequest(taskRequestId, invalidUser); + const result = await approveTaskRequest(taskRequestId, invalidUser, authenticatedUserId); expect(result.isUserInvalid).to.be.equal(true); }); it("should handle task request not found", async function () { - const result = await approveTaskRequest("nonExistentTaskRequestId", user); + const result = await approveTaskRequest("nonExistentTaskRequestId", user, authenticatedUserId); expect(result.taskRequestNotFound).to.be.equal(true); }); it("should handle invalid task request status", async function () { const existingTaskRequest = { ...mockData.existingTaskRequest, status: TASK_REQUEST_STATUS.APPROVED }; await taskRequestsCollection.doc(taskRequestId).set(existingTaskRequest); - const result = await approveTaskRequest(taskRequestId, user); + const result = await approveTaskRequest(taskRequestId, user, authenticatedUserId); expect(result.isTaskRequestInvalid).to.be.equal(true); }); it("should throw an error for general approval failure", async function () { sinon.stub(firestore, "runTransaction").rejects(new Error("Transaction failed")); try { - await approveTaskRequest(taskRequestId, user); + await approveTaskRequest(taskRequestId, user, authenticatedUserId); expect.fail("Error in approving task: Transaction failed"); } catch (err) { expect(err.message).to.equal("Transaction failed"); @@ -406,6 +408,29 @@ describe("Task requests | models", function () { }); }); + describe("rejectTaskRequest", function () { + const taskRequestId = "taskRequest123"; + const authenticatedUserId = "userId"; + it("should reject a task request", async function () { + const existingTaskRequest = { ...mockData.existingTaskRequest }; + await taskRequestsCollection.doc(taskRequestId).set(existingTaskRequest); + const result = await rejectTaskRequest(taskRequestId, authenticatedUserId); + const rejectedTaskRequest = result.taskRequest; + expect(rejectedTaskRequest.status).to.equal(TASK_REQUEST_STATUS.DENIED); + expect(rejectedTaskRequest.lastModifiedBy).to.equal("userId"); + }); + it("should handle task request not found", async function () { + const result = await rejectTaskRequest("nonExistentTaskRequestId", authenticatedUserId); + expect(result.taskRequestNotFound).to.be.equal(true); + }); + it("should handle invalid task request status", async function () { + const existingTaskRequest = { ...mockData.existingTaskRequest, status: TASK_REQUEST_STATUS.APPROVED }; + await taskRequestsCollection.doc(taskRequestId).set(existingTaskRequest); + const result = await rejectTaskRequest(taskRequestId, authenticatedUserId); + expect(result.isTaskRequestInvalid).to.be.equal(true); + }); + }); + describe("Task requests migrations", function () { const taskRequestId1 = "123"; const taskRequestId2 = "456"; From e60da8ead5c7bb8a2b00191f0d3e7ca32c67f619 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Fri, 24 Nov 2023 18:48:56 +0530 Subject: [PATCH 03/18] chore : rename and add jsdoc for models function --- controllers/tasksRequests.js | 16 +++++++-------- models/taskRequests.js | 39 ++++++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/controllers/tasksRequests.js b/controllers/tasksRequests.js index 568dd0e1f..c7ee205b8 100644 --- a/controllers/tasksRequests.js +++ b/controllers/tasksRequests.js @@ -178,14 +178,14 @@ const updateTaskRequests = async (req, res) => { const { action = TASK_REQUEST_UPDATE_ACTION.APPROVE } = req.query; - let response = {}; + let updateTaskRequestResponse = {}; switch (action) { case TASK_REQUEST_UPDATE_ACTION.APPROVE: { - response = await taskRequestsModel.approveTaskRequest(taskRequestId, user, req.userData.id); + updateTaskRequestResponse = await taskRequestsModel.approveTaskRequest(taskRequestId, user, req.userData.id); break; } case TASK_REQUEST_UPDATE_ACTION.REJECT: { - response = await taskRequestsModel.rejectTaskRequest(taskRequestId, req.userData.id); + updateTaskRequestResponse = await taskRequestsModel.rejectTaskRequest(taskRequestId, req.userData.id); break; } default: { @@ -193,13 +193,13 @@ const updateTaskRequests = async (req, res) => { } } - if (response.taskRequestNotFound) { + if (updateTaskRequestResponse.taskRequestNotFound) { return res.boom.badRequest("Task request not found."); } - if (response.isUserInvalid) { + if (updateTaskRequestResponse.isUserInvalid) { return res.boom.badRequest("User request not available."); } - if (response.isTaskRequestInvalid) { + if (updateTaskRequestResponse.isTaskRequestInvalid) { return res.boom.badRequest("Task request was previously approved or rejected."); } @@ -218,13 +218,13 @@ const updateTaskRequests = async (req, res) => { lastModifiedBy: req.userData.id, lastModifiedAt: Date.now(), }, - body: response.taskRequest, + body: updateTaskRequestResponse.taskRequest, }; await addLog(taskRequestLog.type, taskRequestLog.meta, taskRequestLog.body); return res.status(200).json({ message: `Task updated successfully.`, - taskRequest: response?.taskRequest, + taskRequest: updateTaskRequestResponse?.taskRequest, }); } catch (err) { logger.error("Error while approving task request", err); diff --git a/models/taskRequests.js b/models/taskRequests.js index c84033dc1..40fc49ccd 100644 --- a/models/taskRequests.js +++ b/models/taskRequests.js @@ -187,6 +187,25 @@ const fetchTaskRequestById = async (taskRequestId) => { }; }; +/** + * Creates a task request with user details. + * + * @param {Object} data - The data for creating the task request. + * @param {string} data.userId - The ID of the user making the request. + * @param {string} data.proposedDeadline - The proposed deadline for the task. + * @param {string} data.proposedStartDate - The proposed start date for the task. + * @param {string} data.description - The description of the task request. + * @param {string} data.taskTitle - The title of the task. + * @param {string} data.taskId - The ID of the task (optional). + * @param {string} data.externalIssueUrl - The external issue URL (optional). + * @param {string} data.requestType - The type of the task request (CREATION | ASSIGNMENT). + * @param {string} authenticatedUserId - The ID of the authenticated user creating the request. + * @returns {Promise<{ + * id: string, + * isCreate: boolean, + * taskRequest: Object, + * }|{isCreationRequestApproved: boolean}|{alreadyRequesting: boolean}>} + */ const createRequest = async (data, authenticatedUserId) => { try { const queryFieldPath = data.requestType === TASK_REQUEST_TYPE.CREATION ? "externalIssueUrl" : "taskId"; @@ -321,11 +340,15 @@ const addOrUpdate = async (taskId, userId) => { }; /** - * Approves task request to user + * Approves a task request for a user. * - * @param taskRequestId { string }: id of task request - * @param userId { Object }: user whose being approved - * @return {Promise<{approvedTo: string, taskRequest: Object}>} + * @param {string} taskRequestId - The ID of the task request. + * @param {Object} user - The user to whom the task request is being approved. + * @param {string} authenticatedUserId - The ID of the authenticated user performing the approval. + * @returns {Promise<{ + * approvedTo: string, + * taskRequest: Object, + * }|{taskRequestNotFound: boolean}|{isUserInvalid: boolean}|{isTaskRequestInvalid: boolean}>} */ const approveTaskRequest = async (taskRequestId, user, authenticatedUserId) => { try { @@ -439,6 +462,13 @@ const approveTaskRequest = async (taskRequestId, user, authenticatedUserId) => { } }; +/** + * Rejects a task request. + * + * @param {string} taskRequestId - The ID of the task request. + * @param {string} authenticatedUserId - The ID of the authenticated or logged in user performing the rejection. + * @returns {Promise<{taskRequest: Object}|{taskRequestNotFound: boolean}|{isTaskRequestInvalid: boolean}>} + */ const rejectTaskRequest = async (taskRequestId, authenticatedUserId) => { const taskRequestDoc = taskRequestsCollection.doc(taskRequestId); const taskRequestData = (await taskRequestDoc.get()).data(); @@ -460,6 +490,7 @@ const rejectTaskRequest = async (taskRequestId, authenticatedUserId) => { await taskRequestDoc.update(updatedTaskRequest); return { taskRequest: { ...taskRequestData, ...updatedTaskRequest } }; }; + const addNewFields = async () => { const taskRequestsSnapshots = (await taskRequestsCollection.get()).docs; From 0fa0dfc71cb0d60ac3a6777d09d980a7ba58ef7a Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Fri, 24 Nov 2023 19:03:47 +0530 Subject: [PATCH 04/18] chore : update comment --- models/taskRequests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/taskRequests.js b/models/taskRequests.js index 40fc49ccd..fa71ef4f7 100644 --- a/models/taskRequests.js +++ b/models/taskRequests.js @@ -191,7 +191,7 @@ const fetchTaskRequestById = async (taskRequestId) => { * Creates a task request with user details. * * @param {Object} data - The data for creating the task request. - * @param {string} data.userId - The ID of the user making the request. + * @param {string} data.userId - The ID of the user to whom the task request is being created. * @param {string} data.proposedDeadline - The proposed deadline for the task. * @param {string} data.proposedStartDate - The proposed start date for the task. * @param {string} data.description - The description of the task request. From 8685912c6575a357663995dc7cf494f3648d152b Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Sat, 25 Nov 2023 17:40:27 +0530 Subject: [PATCH 05/18] refactor: rename variables --- constants/taskRequests.ts | 4 ++-- controllers/tasksRequests.js | 10 ++++---- middlewares/taskRequests.js | 4 ++-- models/taskRequests.js | 14 ++++++++--- test/integration/taskRequests.test.js | 34 +++++++++++++-------------- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/constants/taskRequests.ts b/constants/taskRequests.ts index 2509bd3c2..d37dd68a3 100644 --- a/constants/taskRequests.ts +++ b/constants/taskRequests.ts @@ -5,7 +5,7 @@ const TASK_REQUEST_STATUS = { DENIED: "DENIED", }; -const TASK_REQUEST_UPDATE_ACTION = { +const TASK_REQUEST_ACTIONS = { APPROVE: "approve", REJECT: "reject", }; @@ -50,5 +50,5 @@ module.exports = { TASK_REQUEST_ERROR_MESSAGE, TASK_REQUEST_SORT_VALUES, MIGRATION_TYPE, - TASK_REQUEST_UPDATE_ACTION, + TASK_REQUEST_ACTIONS, }; diff --git a/controllers/tasksRequests.js b/controllers/tasksRequests.js index c7ee205b8..4d2090c10 100644 --- a/controllers/tasksRequests.js +++ b/controllers/tasksRequests.js @@ -1,5 +1,5 @@ const { INTERNAL_SERVER_ERROR, SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); -const { TASK_REQUEST_TYPE, MIGRATION_TYPE, TASK_REQUEST_UPDATE_ACTION } = require("../constants/taskRequests"); +const { TASK_REQUEST_TYPE, MIGRATION_TYPE, TASK_REQUEST_ACTIONS } = require("../constants/taskRequests"); const { addLog } = require("../models/logs"); const taskRequestsModel = require("../models/taskRequests"); const tasksModel = require("../models/tasks.js"); @@ -176,15 +176,15 @@ const updateTaskRequests = async (req, res) => { return res.boom.badRequest("taskRequestId not provided"); } - const { action = TASK_REQUEST_UPDATE_ACTION.APPROVE } = req.query; + const { action = TASK_REQUEST_ACTIONS.APPROVE } = req.query; let updateTaskRequestResponse = {}; switch (action) { - case TASK_REQUEST_UPDATE_ACTION.APPROVE: { + case TASK_REQUEST_ACTIONS.APPROVE: { updateTaskRequestResponse = await taskRequestsModel.approveTaskRequest(taskRequestId, user, req.userData.id); break; } - case TASK_REQUEST_UPDATE_ACTION.REJECT: { + case TASK_REQUEST_ACTIONS.REJECT: { updateTaskRequestResponse = await taskRequestsModel.rejectTaskRequest(taskRequestId, req.userData.id); break; } @@ -203,7 +203,7 @@ const updateTaskRequests = async (req, res) => { return res.boom.badRequest("Task request was previously approved or rejected."); } - if (action && action === TASK_REQUEST_UPDATE_ACTION.APPROVE) { + if (action && action === TASK_REQUEST_ACTIONS.APPROVE) { await updateUserStatusOnTaskUpdate(user.username); } diff --git a/middlewares/taskRequests.js b/middlewares/taskRequests.js index 2a2ff0b15..bca7af322 100644 --- a/middlewares/taskRequests.js +++ b/middlewares/taskRequests.js @@ -1,6 +1,6 @@ const { SOMETHING_WENT_WRONG } = require("../constants/errorMessages"); const dataAccess = require("../services/dataAccessLayer"); -const { TASK_REQUEST_UPDATE_ACTION } = require("../constants/taskRequests"); +const { TASK_REQUEST_ACTIONS } = require("../constants/taskRequests"); /** * Validates user id for task request * @@ -9,7 +9,7 @@ const { TASK_REQUEST_UPDATE_ACTION } = require("../constants/taskRequests"); async function validateUser(req, res, next) { try { const { action } = req.query; - if (action === TASK_REQUEST_UPDATE_ACTION.REJECT) { + if (action === TASK_REQUEST_ACTIONS.REJECT) { return next(); } const { userId } = req.body; diff --git a/models/taskRequests.js b/models/taskRequests.js index fa71ef4f7..5ee5b3940 100644 --- a/models/taskRequests.js +++ b/models/taskRequests.js @@ -201,10 +201,12 @@ const fetchTaskRequestById = async (taskRequestId) => { * @param {string} data.requestType - The type of the task request (CREATION | ASSIGNMENT). * @param {string} authenticatedUserId - The ID of the authenticated user creating the request. * @returns {Promise<{ + * isCreationRequestApproved: boolean | undefined, + * alreadyRequesting: boolean | undefined, * id: string, * isCreate: boolean, * taskRequest: Object, - * }|{isCreationRequestApproved: boolean}|{alreadyRequesting: boolean}>} + * }>} */ const createRequest = async (data, authenticatedUserId) => { try { @@ -348,7 +350,10 @@ const addOrUpdate = async (taskId, userId) => { * @returns {Promise<{ * approvedTo: string, * taskRequest: Object, - * }|{taskRequestNotFound: boolean}|{isUserInvalid: boolean}|{isTaskRequestInvalid: boolean}>} + * taskRequestNotFound: boolean | undefined + * isUserInvalid: boolean | undefined + * isTaskRequestInvalid: boolean | undefined + * }>} */ const approveTaskRequest = async (taskRequestId, user, authenticatedUserId) => { try { @@ -467,7 +472,10 @@ const approveTaskRequest = async (taskRequestId, user, authenticatedUserId) => { * * @param {string} taskRequestId - The ID of the task request. * @param {string} authenticatedUserId - The ID of the authenticated or logged in user performing the rejection. - * @returns {Promise<{taskRequest: Object}|{taskRequestNotFound: boolean}|{isTaskRequestInvalid: boolean}>} + * @returns {Promise<{taskRequest: Object + * taskRequestNotFound: boolean | undefined + * isTaskRequestInvalid: boolean | undefined + * }>} */ const rejectTaskRequest = async (taskRequestId, authenticatedUserId) => { const taskRequestDoc = taskRequestsCollection.doc(taskRequestId); diff --git a/test/integration/taskRequests.test.js b/test/integration/taskRequests.test.js index bf8e169dd..2b947f51a 100644 --- a/test/integration/taskRequests.test.js +++ b/test/integration/taskRequests.test.js @@ -21,7 +21,7 @@ const { MIGRATION_TYPE } = require("../../constants/taskRequests"); chai.use(chaiHttp); const config = require("config"); -const { TASK_REQUEST_TYPE, TASK_REQUEST_UPDATE_ACTION } = require("../../constants/taskRequests"); +const { TASK_REQUEST_TYPE, TASK_REQUEST_ACTIONS } = require("../../constants/taskRequests"); const usersUtils = require("../../utils/users"); const githubService = require("../../services/githubService"); const { userState } = require("../../constants/userStatus"); @@ -670,7 +670,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests/") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, @@ -692,7 +692,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests/") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .query({ action: TASK_REQUEST_ACTIONS.REJECT }) .send({ taskRequestId: taskId, userId, @@ -712,7 +712,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ userId: oooUserId, }) @@ -732,7 +732,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, activeUserId }) .end((err, res) => { if (err) { @@ -752,7 +752,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .query({ action: TASK_REQUEST_ACTIONS.REJECT }) .send({ taskRequestId: taskId, userId, activeUserId }) .end((err, res) => { if (err) { @@ -772,7 +772,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, activeUserId }) .end((err, res) => { if (err) { @@ -791,7 +791,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, activeUserId }) .end((err, res) => { if (err) { @@ -811,7 +811,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .query({ action: TASK_REQUEST_ACTIONS.REJECT }) .send({ taskRequestId: taskId, userId, activeUserId }) .end((err, res) => { if (err) { @@ -829,7 +829,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId }) .end((err, res) => { if (err) { @@ -850,7 +850,7 @@ describe("Task Requests", function () { const res = await chai .request(app) .patch("/taskRequests") - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .set("cookie", `${cookieName}=${jwt}`) .send({ taskRequestId: taskId, @@ -868,7 +868,7 @@ describe("Task Requests", function () { const res = await chai .request(app) .patch("/taskRequests") - .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .query({ action: TASK_REQUEST_ACTIONS.REJECT }) .set("cookie", `${cookieName}=${jwt}`) .send({ taskRequestId: taskId, @@ -885,7 +885,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests/approve") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, @@ -920,7 +920,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, @@ -939,7 +939,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.REJECT }) + .query({ action: TASK_REQUEST_ACTIONS.REJECT }) .send({ taskRequestId: taskId, userId, @@ -957,7 +957,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, @@ -986,7 +986,7 @@ describe("Task Requests", function () { .request(app) .patch("/taskRequests") .set("cookie", `${cookieName}=${jwt}`) - .query({ action: TASK_REQUEST_UPDATE_ACTION.APPROVE }) + .query({ action: TASK_REQUEST_ACTIONS.APPROVE }) .send({ taskRequestId: taskId, userId, From 55b1f529159b0ec75147fcbf9e880538f114562b Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Tue, 5 Dec 2023 19:44:52 +0530 Subject: [PATCH 06/18] feat: models function to fetch user discord id when they miss progress updates --- config/default.js | 1 + constants/tasks.ts | 15 +- models/discordactions.js | 184 ++++++++++++++++++ .../discordResponse/discord-response.js | 4 +- test/unit/models/discordactions.test.js | 130 ++++++++++++- utils/time.js | 10 +- 6 files changed, 339 insertions(+), 5 deletions(-) diff --git a/config/default.js b/config/default.js index 5826bcda1..1740aa439 100644 --- a/config/default.js +++ b/config/default.js @@ -14,6 +14,7 @@ module.exports = { discordUnverifiedRoleId: "", discordDeveloperRoleId: "", discordMavenRoleId: "", + discordMissedUpdatesRoleId:"", githubApi: { baseUrl: "https://api.github.com", org: "Real-Dev-Squad", diff --git a/constants/tasks.ts b/constants/tasks.ts index 24ec9db14..fa71d25f7 100644 --- a/constants/tasks.ts +++ b/constants/tasks.ts @@ -37,6 +37,19 @@ const MAPPED_TASK_STATUS = { UNASSIGNED: "AVAILABLE", }; +const COMPLETED_TASK_STATUS = { + VERIFIED: "VERIFIED", + DONE: "DONE", + COMPLETED: "COMPLETED", +}; const TASK_SIZE = 5; -module.exports = { TASK_TYPE, TASK_STATUS, TASK_STATUS_OLD, MAPPED_TASK_STATUS, TASK_SIZE, DEFAULT_TASK_PRIORITY }; +module.exports = { + TASK_TYPE, + TASK_STATUS, + TASK_STATUS_OLD, + MAPPED_TASK_STATUS, + TASK_SIZE, + DEFAULT_TASK_PRIORITY, + COMPLETED_TASK_STATUS, +}; diff --git a/models/discordactions.js b/models/discordactions.js index 17b54766a..3e4cc9cf3 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -15,9 +15,23 @@ const dataAccess = require("../services/dataAccessLayer"); const { getDiscordMembers, addRoleToUser, removeRoleFromUser } = require("../services/discordService"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const discordMavenRoleId = config.get("discordMavenRoleId"); +const discordMissedUpdatesRoleId = config.get("discordMissedUpdatesRoleId"); + const userStatusModel = firestore.collection("usersStatus"); const usersUtils = require("../utils/users"); const { getUsersBasedOnFilter, fetchUser } = require("./users"); +const { + convertDaysToMilliseconds, + convertMillisToSeconds, + convertTimestampToUTCStartOrEndOfDay, +} = require("../utils/time"); +const { chunks } = require("../utils/array"); +const tasksModel = firestore.collection("tasks"); +const progressesModel = firestore.collection("progresses"); +const { COMPLETED_TASK_STATUS } = require("../constants/tasks.ts"); +const { FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users"); +const discordService = require("../services/discordService"); +const user = require("../test/fixtures/user/user"); /** * @@ -817,6 +831,175 @@ const updateUsersWith31DaysPlusOnboarding = async () => { } }; +const addMissedProgressUpdatesRoleInDiscord = async (options = {}) => { + const { cursor, size = 500, excludedDates = [], dateGap = 3 } = options; + const stats = { + tasks: 0, + missedUpdatesTasks: 0, + }; + try { + const discordUsersPromise = discordService.getDiscordMembers(); + const missedUpdatesRoleId = discordMissedUpdatesRoleId; + + const completedTasksStatusList = Object.values(COMPLETED_TASK_STATUS); + let gapWindowStart = Date.now() - convertDaysToMilliseconds(dateGap); + const gapWindowEnd = Date.now(); + + excludedDates.forEach((timestamp) => { + if (timestamp > gapWindowStart && timestamp < gapWindowEnd) { + gapWindowStart -= convertDaysToMilliseconds(1); + } + }); + + let taskQuery = tasksModel + .where("status", "not-in", completedTasksStatusList) + .where("startedOn", "<", convertMillisToSeconds(gapWindowStart)) + .orderBy("assignee") // this order by will eliminate the documents which has no assignee field. https://firebase.google.com/docs/firestore/query-data/order-limit-data#limitations + .limit(size); + + if (cursor) { + const data = await tasksModel.doc(cursor).get(); + if (!data.data()) { + return { + statusCode: 400, + error: "Bad Request", + message: `Invalid cursor: ${cursor}`, + }; + } + taskQuery = taskQuery.startAfter(data); + } + + const usersMap = new Map(); + const progressCountPromise = []; + const tasksQuerySnapshot = await taskQuery.get(); + + stats.tasks = tasksQuerySnapshot.size; + tasksQuerySnapshot.forEach((doc) => { + const taskAssignee = doc.data().assignee; + const taskId = doc.id; + + if (usersMap.has(taskAssignee)) { + const userData = usersMap.get(taskAssignee); + userData.tasksCount++; + } else { + usersMap.set(taskAssignee, { + tasksCount: 1, + latestProgressCount: dateGap + 1, + isActive: false, + }); + } + const updateTasksIdMap = async () => { + const progressSnapshot = await progressesModel + .where("type", "==", "task") + .where("taskId", "==", taskId) + .where("date", ">=", convertTimestampToUTCStartOrEndOfDay(gapWindowStart)) + .where("date", "<=", convertTimestampToUTCStartOrEndOfDay(gapWindowEnd, true)) + .count() + .get(); + const userData = usersMap.get(taskAssignee); + userData.latestProgressCount = Math.min(progressSnapshot.data().count, userData.latestProgressCount); + + if (userData.latestProgressCount === 0) { + stats.missedUpdatesTasks++; + } + }; + progressCountPromise.push(updateTasksIdMap()); + }); + + const userIdChunks = chunks(Array.from(usersMap.keys()), FIRESTORE_IN_CLAUSE_SIZE); + const userStatusSnapshotPromise = userIdChunks.map( + async (userIdList) => + await userStatusModel + .where("currentStatus.state", "==", userState.ACTIVE) + .where("userId", "in", userIdList) + .get() + ); + const userDetailsPromise = userIdChunks.map( + async (userIdList) => + await userModel + .where("roles.archived", "==", false) + .where(admin.firestore.FieldPath.documentId(), "in", userIdList) + .get() + ); + + const userStatusChunks = await Promise.all(userStatusSnapshotPromise); + + userStatusChunks.forEach((userStatusList) => + userStatusList.forEach((doc) => { + usersMap.get(doc.data().userId).isActive = true; + }) + ); + + const userDetailsListChunks = await Promise.all(userDetailsPromise); + userDetailsListChunks.forEach((userList) => { + userList.forEach((doc) => { + const userData = usersMap.get(doc.id); + userData.discordId = doc.data().discordId; + }); + }); + + const discordUserList = await Promise.all(discordUsersPromise); + + const discordUserMap = new Map(); + discordUserList.forEach((discordUser) => { + const discordUserData = { isBot: !!discordUser.user.bot }; + discordUser.roles.forEach((roleId) => { + switch (roleId) { + case discordDeveloperRoleId: { + discordUserData.isDeveloper = true; + break; + } + case discordMavenRoleId: { + discordUserData.isMaven = true; + break; + } + case missedUpdatesRoleId: { + discordUserData.hasMissedUpdatesRole = true; + break; + } + } + }); + discordUserMap.set(discordUser.user.id, discordUserData); + }); + + await Promise.all(progressCountPromise); + + for (const [userId, userData] of usersMap.entries()) { + const discordUserData = discordUserMap.get(userData.discordId); + const isDiscordMember = !!discordUserData; + const shouldAddRole = + userData.latestProgressCount === 0 && + userData.isActive && + isDiscordMember && + discordUserData.isDeveloper && + !discordUserData.isMaven && + !discordUserData.isBot && + !discordUserData.hasMissedUpdatesRole; + + if (!shouldAddRole) { + usersMap.delete(userId); + } + } + + const usersToAddRole = []; + for (const userData of usersMap.values()) { + usersToAddRole.push(userData.discordId); + } + const resultDataLength = tasksQuerySnapshot.docs.length; + const isLast = size && resultDataLength === size; + const lastVisible = isLast && tasksQuerySnapshot.docs[resultDataLength - 1]; + + if (lastVisible) { + stats.cursor = lastVisible.id; + } + + return { usersToAddRole, ...stats }; + } catch (err) { + logger.error("Error while running the add missed roles script", err); + throw err; + } +}; + module.exports = { createNewRole, removeMemberGroup, @@ -834,4 +1017,5 @@ module.exports = { updateUsersNicknameStatus, updateIdle7dUsersOnDiscord, updateUsersWith31DaysPlusOnboarding, + addMissedProgressUpdatesRoleInDiscord, }; diff --git a/test/fixtures/discordResponse/discord-response.js b/test/fixtures/discordResponse/discord-response.js index 4267e908e..37a6193dd 100644 --- a/test/fixtures/discordResponse/discord-response.js +++ b/test/fixtures/discordResponse/discord-response.js @@ -8,7 +8,7 @@ const getDiscordMembers = [ nick: "jhon", pending: false, premium_since: null, - roles: ["hero", "racer"], + roles: ["hero", "racer", "9876543210"], user: { id: "123456789098765432", username: "jhondoe", @@ -31,7 +31,7 @@ const getDiscordMembers = [ nick: "david", pending: false, premium_since: null, - roles: ["hero", "racer"], + roles: ["hero", "racer", "9876543210"], user: { id: "12345678909867666", username: "david", diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index 86e0837de..9ff625f05 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -7,6 +7,14 @@ const discordRoleModel = firestore.collection("discord-roles"); const memberRoleModel = firestore.collection("member-group-roles"); const userModel = firestore.collection("users"); const admin = require("firebase-admin"); +const tasksData = require("../../fixtures/tasks/tasks")(); +const tasks = require("../../../models/tasks"); +const addUser = require("../../utils/addUser"); +const userStatusData = require("../../fixtures/userStatus/userStatus"); +const { getDiscordMembers } = require("../../fixtures/discordResponse/discord-response"); +const discordService = require("../../../services/discordService"); +const { TASK_STATUS } = require("../../../constants/tasks"); +const tasksModel = firestore.collection("tasks"); const { createNewRole, @@ -18,16 +26,22 @@ const { enrichGroupDataWithMembershipInfo, fetchGroupToUserMapping, updateUsersNicknameStatus, + addMissedProgressUpdatesRoleInDiscord, } = require("../../../models/discordactions"); const { groupData, roleData, existingRole, memberGroupData } = require("../../fixtures/discordactions/discordactions"); const cleanDb = require("../../utils/cleanDb"); const { userPhotoVerificationData } = require("../../fixtures/user/photo-verification"); const userData = require("../../fixtures/user/user")(); -const userStatusModel = firestore.collection("usersStatus"); +const userStatusModel = require("../../../models/userStatus"); const { getStatusData } = require("../../fixtures/userStatus/userStatus"); const usersStatusData = getStatusData(); const dataAccessLayer = require("../../../services/dataAccessLayer"); const { ONE_DAY_IN_MS } = require("../../../constants/users"); +const { createProgress } = require("../../../controllers/progresses"); +const { createProgressDocument } = require("../../../models/progresses"); +const user = require("../../fixtures/user/user"); +const { stubbedModelTaskProgressData } = require("../../fixtures/progress/progresses"); +const { convertDaysToMilliseconds } = require("../../../utils/time"); chai.should(); @@ -567,4 +581,118 @@ describe("discordactions", function () { } }).timeout(10000); }); + + describe.only("addMissedProgressUpdatesRoleInDiscord", function () { + const idleUser = { ...userData[9], discordId: getDiscordMembers[0].user.id }; + const activeUserWithProgressUpdates = { ...userData[10], discordId: getDiscordMembers[1].user.id }; + const activeUserWithNoUpdates = { ...userData[0], discordId: getDiscordMembers[2].user.id }; + const userNotInDiscord = { ...userData[4], discordId: "Not in discord" }; + const { + idleStatus: idleUserStatus, + activeStatus: activeUserStatus, + userStatusDataForOooState: oooUserStatus, + } = userStatusData; + let taskId; + beforeEach(async function () { + const userIdList = await Promise.all([ + await addUser(idleUser), // idle user with no task progress updates + await addUser(activeUserWithProgressUpdates), // active user with task progress updates + await addUser(activeUserWithNoUpdates), // active user with no task progress updates + await addUser(userNotInDiscord), // OOO user with + ]); + await Promise.all([ + await userStatusModel.updateUserStatus(userIdList[0], idleUserStatus), + await userStatusModel.updateUserStatus(userIdList[1], activeUserStatus), + await userStatusModel.updateUserStatus(userIdList[2], activeUserStatus), + await userStatusModel.updateUserStatus(userIdList[3], oooUserStatus), + ]); + + const tasksPromise = []; + + for (let index = 0; index < 4; index++) { + const task = tasksData[index]; + const validTask = { + ...task, + assignee: userIdList[index], + startedOn: (new Date().getTime() - convertDaysToMilliseconds(5)) / 1000, + endsOn: (new Date().getTime() + convertDaysToMilliseconds(4)) / 1000, + status: TASK_STATUS.IN_PROGRESS, + }; + + tasksPromise.push(await tasksModel.add(validTask)); + } + const taskIdList = (await Promise.all(tasksPromise)).map((tasksDoc) => tasksDoc.id); + taskId = taskIdList[0]; + const progressDataList = []; + + const date = new Date(); + date.setDate(date.getDate() - 1); + const progressData = stubbedModelTaskProgressData(null, taskIdList[2], date.getTime(), date.valueOf()); + progressDataList.push(progressData); + const date2 = new Date(); + date2.setDate(date2.getDate() - 3); + const progressData2 = stubbedModelTaskProgressData(null, taskIdList[2], date2.getTime(), date.valueOf()); + progressDataList.push(progressData2); + + await Promise.all(progressDataList.map(async (progress) => await createProgressDocument(progress))); + + sinon.stub(discordService, "getDiscordMembers").returns(getDiscordMembers); + }); + afterEach(async function () { + sinon.restore(); + await cleanDb(); + }); + it("should list of users who missed updating progress", async function () { + const result = await addMissedProgressUpdatesRoleInDiscord(); + expect(result).to.be.an("object"); + expect(result).to.be.deep.equal({ + tasks: 4, + missedUpdatesTasks: 3, + usersToAddRole: [activeUserWithProgressUpdates.discordId], + }); + }); + it("should not list of users who are not active and who missed updating progress", async function () { + const result = await addMissedProgressUpdatesRoleInDiscord(); + expect(result).to.be.an("object"); + expect(result.usersToAddRole).to.not.contain(idleUser.discordId); + expect(result.usersToAddRole).to.not.contain(userNotInDiscord.discordId); + }); + + it("should not list of users when exception days are added", async function () { + const date = new Date(); + date.setDate(date.getDate() - 1); + const date2 = new Date(); + date.setDate(date2.getDate() - 2); + const date3 = new Date(); + date.setDate(date3.getDate() - 3); + const date4 = new Date(); + date.setDate(date4.getDate() - 4); + // this should now only get users who have not provided any update in last 7 days instead of 3; + const result = await addMissedProgressUpdatesRoleInDiscord({ + excludedDates: [date.valueOf(), date2.valueOf(), date3.valueOf(), date4.valueOf()], + }); + expect(result).to.be.an("object"); + expect(result).to.be.deep.equal({ + tasks: 0, + missedUpdatesTasks: 0, + usersToAddRole: [], + }); + }); + + it("should process only 1 task when size is passed as 1", async function () { + const result = await addMissedProgressUpdatesRoleInDiscord({ size: 1 }); + + expect(result).to.be.an("object"); + expect(result.tasks).to.be.equal(1); + }); + it("should fetch process tasks when cursor is passed", async function () { + const result = await addMissedProgressUpdatesRoleInDiscord({ size: 4 }); + + expect(result).to.be.an("object"); + expect(result).to.haveOwnProperty("cursor"); + const nextResult = await addMissedProgressUpdatesRoleInDiscord({ size: 4, cursor: result.cursor }); + expect(nextResult).to.be.an("object"); + expect(nextResult).to.not.haveOwnProperty("cursor"); + }); + }); }); diff --git a/utils/time.js b/utils/time.js index 50c5374f2..3d8518abf 100644 --- a/utils/time.js +++ b/utils/time.js @@ -24,7 +24,14 @@ const convertHoursToMilliseconds = (hours) => { const convertDaysToMilliseconds = (days) => { return days * 24 * 60 * 60 * 1000; }; - +/** + * Converts milliseconds to seconds + * @param milliseconds {number} : to be converted + * @returns {number} : seconds + */ +const convertMillisToSeconds = (milliseconds) => { + return milliseconds / 1000; +}; /** * Returns time in seconds of timestamp after given duration * @param timestamp {integer} : base time in milliseconds @@ -82,4 +89,5 @@ module.exports = { getTimeInSecondAfter, getBeforeHourTime, convertTimestampToUTCStartOrEndOfDay, + convertMillisToSeconds, }; From c89a9fb9da5c564d3a648bbeb72ac9c53c0362b7 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Tue, 5 Dec 2023 20:02:22 +0530 Subject: [PATCH 07/18] chore: lint fix --- config/default.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/default.js b/config/default.js index 59bcb8c46..b607a7a61 100644 --- a/config/default.js +++ b/config/default.js @@ -14,7 +14,7 @@ module.exports = { discordUnverifiedRoleId: "", discordDeveloperRoleId: "", discordMavenRoleId: "", - discordMissedUpdatesRoleId:"", + discordMissedUpdatesRoleId: "discordMissedUpdatesRoleId", githubApi: { baseUrl: "https://api.github.com", org: "Real-Dev-Squad", From 410c03d3299e5323bbba1b62ccf7b60a2011c4bc Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Tue, 5 Dec 2023 20:09:55 +0530 Subject: [PATCH 08/18] fix: failing tests --- test/unit/models/discordactions.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index 5700f0ff4..0e2ce1362 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -4,10 +4,12 @@ const sinon = require("sinon"); const firestore = require("../../../utils/firestore"); const photoVerificationModel = firestore.collection("photo-verification"); const discordRoleModel = firestore.collection("discord-roles"); +const userStatusCollection = firestore.collection("usersStatus"); const memberRoleModel = firestore.collection("member-group-roles"); const userModel = firestore.collection("users"); const admin = require("firebase-admin"); const tasksData = require("../../fixtures/tasks/tasks")(); + const addUser = require("../../utils/addUser"); const userStatusData = require("../../fixtures/userStatus/userStatus"); const { getDiscordMembers } = require("../../fixtures/discordResponse/discord-response"); @@ -488,7 +490,7 @@ describe("discordactions", function () { const addedUsersStatusPromise = usersStatusData.map(async (data, index) => { const { id } = addedUers[index]; const statusData = { ...data, userId: id }; - const { id: userStatusId } = await userStatusModel.add(statusData); + const { id: userStatusId } = await userStatusCollection.add(statusData); return { ...statusData, id: userStatusId }; }); @@ -639,7 +641,7 @@ describe("discordactions", function () { progressDataList.push(progressData); const date2 = new Date(); date2.setDate(date2.getDate() - 3); - const progressData2 = stubbedModelTaskProgressData(null, taskIdList[2], date2.getTime(), date.valueOf()); + const progressData2 = stubbedModelTaskProgressData(null, taskIdList[2], date2.getTime(), date2.valueOf()); progressDataList.push(progressData2); await Promise.all(progressDataList.map(async (progress) => await createProgressDocument(progress))); From a244d767fe30471ccf5e78eacd5334ddf8bdb144 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Fri, 8 Dec 2023 04:15:25 +0530 Subject: [PATCH 09/18] chore: refactors and fix test --- models/discordactions.js | 31 +++++++------------------ test/unit/models/discordactions.test.js | 26 +++++++++------------ utils/progresses.js | 12 ++++++++++ utils/tasks.js | 14 +++++++++++ 4 files changed, 45 insertions(+), 38 deletions(-) diff --git a/models/discordactions.js b/models/discordactions.js index 21cae8df9..99cc7a435 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -21,17 +21,13 @@ const discordMissedUpdatesRoleId = config.get("discordMissedUpdatesRoleId"); const userStatusModel = firestore.collection("usersStatus"); const usersUtils = require("../utils/users"); const { getUsersBasedOnFilter, fetchUser } = require("./users"); -const { - convertDaysToMilliseconds, - convertMillisToSeconds, - convertTimestampToUTCStartOrEndOfDay, -} = require("../utils/time"); +const { convertDaysToMilliseconds } = require("../utils/time"); const { chunks } = require("../utils/array"); const tasksModel = firestore.collection("tasks"); -const progressesModel = firestore.collection("progresses"); -const { COMPLETED_TASK_STATUS } = require("../constants/tasks.ts"); const { FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users"); const discordService = require("../services/discordService"); +const { buildTasksQueryForMissedUpdates } = require("../utils/tasks"); +const { buildProgressQueryForMissedUpdates } = require("../utils/progresses"); /** * @@ -844,7 +840,7 @@ const updateUsersWith31DaysPlusOnboarding = async () => { } }; -const addMissedProgressUpdatesRoleInDiscord = async (options = {}) => { +const getMissedProgressUpdatesUsers = async (options = {}) => { const { cursor, size = 500, excludedDates = [], dateGap = 3 } = options; const stats = { tasks: 0, @@ -854,21 +850,15 @@ const addMissedProgressUpdatesRoleInDiscord = async (options = {}) => { const discordUsersPromise = discordService.getDiscordMembers(); const missedUpdatesRoleId = discordMissedUpdatesRoleId; - const completedTasksStatusList = Object.values(COMPLETED_TASK_STATUS); let gapWindowStart = Date.now() - convertDaysToMilliseconds(dateGap); const gapWindowEnd = Date.now(); - excludedDates.forEach((timestamp) => { if (timestamp > gapWindowStart && timestamp < gapWindowEnd) { gapWindowStart -= convertDaysToMilliseconds(1); } }); - let taskQuery = tasksModel - .where("status", "not-in", completedTasksStatusList) - .where("startedOn", "<", convertMillisToSeconds(gapWindowStart)) - .orderBy("assignee") // this order by will eliminate the documents which has no assignee field. https://firebase.google.com/docs/firestore/query-data/order-limit-data#limitations - .limit(size); + let taskQuery = buildTasksQueryForMissedUpdates(gapWindowStart, size); if (cursor) { const data = await tasksModel.doc(cursor).get(); @@ -902,13 +892,8 @@ const addMissedProgressUpdatesRoleInDiscord = async (options = {}) => { }); } const updateTasksIdMap = async () => { - const progressSnapshot = await progressesModel - .where("type", "==", "task") - .where("taskId", "==", taskId) - .where("date", ">=", convertTimestampToUTCStartOrEndOfDay(gapWindowStart)) - .where("date", "<=", convertTimestampToUTCStartOrEndOfDay(gapWindowEnd, true)) - .count() - .get(); + const progressQuery = buildProgressQueryForMissedUpdates(taskId, gapWindowStart, gapWindowEnd); + const progressSnapshot = await progressQuery.get(); const userData = usersMap.get(taskAssignee); userData.latestProgressCount = Math.min(progressSnapshot.data().count, userData.latestProgressCount); @@ -1055,7 +1040,7 @@ module.exports = { updateUsersNicknameStatus, updateIdle7dUsersOnDiscord, updateUsersWith31DaysPlusOnboarding, - addMissedProgressUpdatesRoleInDiscord, + getMissedProgressUpdatesUsers, getUserDiscordInvite, addInviteToInviteModel, }; diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index 0e2ce1362..b73ceced4 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -27,7 +27,7 @@ const { enrichGroupDataWithMembershipInfo, fetchGroupToUserMapping, updateUsersNicknameStatus, - addMissedProgressUpdatesRoleInDiscord, + getMissedProgressUpdatesUsers, addInviteToInviteModel, getUserDiscordInvite, } = require("../../../models/discordactions"); @@ -591,7 +591,7 @@ describe("discordactions", function () { }).timeout(10000); }); - describe("addMissedProgressUpdatesRoleInDiscord", function () { + describe("getMissedProgressUpdatesUsers", function () { let activeUserWithProgressUpdates; let idleUser; let userNotInDiscord; @@ -639,10 +639,6 @@ describe("discordactions", function () { date.setDate(date.getDate() - 1); const progressData = stubbedModelTaskProgressData(null, taskIdList[2], date.getTime(), date.valueOf()); progressDataList.push(progressData); - const date2 = new Date(); - date2.setDate(date2.getDate() - 3); - const progressData2 = stubbedModelTaskProgressData(null, taskIdList[2], date2.getTime(), date2.valueOf()); - progressDataList.push(progressData2); await Promise.all(progressDataList.map(async (progress) => await createProgressDocument(progress))); @@ -653,7 +649,7 @@ describe("discordactions", function () { await cleanDb(); }); it("should list of users who missed updating progress", async function () { - const result = await addMissedProgressUpdatesRoleInDiscord(); + const result = await getMissedProgressUpdatesUsers(); expect(result).to.be.an("object"); expect(result).to.be.deep.equal({ tasks: 4, @@ -663,7 +659,7 @@ describe("discordactions", function () { }); it("should not list of users who are not active and who missed updating progress", async function () { - const result = await addMissedProgressUpdatesRoleInDiscord(); + const result = await getMissedProgressUpdatesUsers(); expect(result).to.be.an("object"); expect(result.usersToAddRole).to.not.contain(idleUser.discordId); expect(result.usersToAddRole).to.not.contain(userNotInDiscord.discordId); @@ -673,13 +669,13 @@ describe("discordactions", function () { const date = new Date(); date.setDate(date.getDate() - 1); const date2 = new Date(); - date.setDate(date2.getDate() - 2); + date2.setDate(date2.getDate() - 2); const date3 = new Date(); - date.setDate(date3.getDate() - 3); + date3.setDate(date3.getDate() - 3); const date4 = new Date(); - date.setDate(date4.getDate() - 4); + date4.setDate(date4.getDate() - 4); // this should now only get users who have not provided any update in last 7 days instead of 3; - const result = await addMissedProgressUpdatesRoleInDiscord({ + const result = await getMissedProgressUpdatesUsers({ excludedDates: [date.valueOf(), date2.valueOf(), date3.valueOf(), date4.valueOf()], }); expect(result).to.be.an("object"); @@ -691,17 +687,17 @@ describe("discordactions", function () { }); it("should process only 1 task when size is passed as 1", async function () { - const result = await addMissedProgressUpdatesRoleInDiscord({ size: 1 }); + const result = await getMissedProgressUpdatesUsers({ size: 1 }); expect(result).to.be.an("object"); expect(result.tasks).to.be.equal(1); }); it("should fetch process tasks when cursor is passed", async function () { - const result = await addMissedProgressUpdatesRoleInDiscord({ size: 4 }); + const result = await getMissedProgressUpdatesUsers({ size: 4 }); expect(result).to.be.an("object"); expect(result).to.haveOwnProperty("cursor"); - const nextResult = await addMissedProgressUpdatesRoleInDiscord({ size: 4, cursor: result.cursor }); + const nextResult = await getMissedProgressUpdatesUsers({ size: 4, cursor: result.cursor }); expect(nextResult).to.be.an("object"); expect(nextResult).to.not.haveOwnProperty("cursor"); }); diff --git a/utils/progresses.js b/utils/progresses.js index e2ae033e0..62a6b67b9 100644 --- a/utils/progresses.js +++ b/utils/progresses.js @@ -2,11 +2,14 @@ const { NotFound } = require("http-errors"); const { fetchTask } = require("../models/tasks"); const { fetchUser } = require("../models/users"); const fireStore = require("../utils/firestore"); +const progressesModel = fireStore.collection("progresses"); + const { PROGRESSES_RESPONSE_MESSAGES: { PROGRESS_DOCUMENT_NOT_FOUND }, MILLISECONDS_IN_DAY, PROGRESS_VALID_SORT_FIELDS, } = require("../constants/progresses"); +const { convertTimestampToUTCStartOrEndOfDay } = require("./time"); const progressesCollection = fireStore.collection("progresses"); /** @@ -211,6 +214,14 @@ const buildQueryToSearchProgressByDay = (pathParams) => { return query; }; +const buildProgressQueryForMissedUpdates = (taskId, startTimestamp, endTimestamp) => { + return progressesModel + .where("type", "==", "task") + .where("taskId", "==", taskId) + .where("date", ">=", convertTimestampToUTCStartOrEndOfDay(startTimestamp)) + .where("date", "<=", convertTimestampToUTCStartOrEndOfDay(endTimestamp, true)) + .count(); +}; module.exports = { getProgressDateTimestamp, buildQueryForPostingProgress, @@ -222,4 +233,5 @@ module.exports = { buildRangeProgressQuery, getProgressRecords, buildQueryToSearchProgressByDay, + buildProgressQueryForMissedUpdates, }; diff --git a/utils/tasks.js b/utils/tasks.js index a2c14615b..d09cc8887 100644 --- a/utils/tasks.js +++ b/utils/tasks.js @@ -1,5 +1,9 @@ const { getUsername, getUserId, getParticipantUsernames, getParticipantUserIds } = require("./users"); const { TASK_TYPE, MAPPED_TASK_STATUS } = require("../constants/tasks"); +const fireStore = require("../utils/firestore"); +const tasksModel = fireStore.collection("tasks"); +const { COMPLETED_TASK_STATUS } = require("../constants/tasks.ts"); +const { convertMillisToSeconds } = require("./time"); const fromFirestoreData = async (task) => { if (!task) { @@ -110,10 +114,20 @@ const parseSearchQuery = (queryString) => { return searchParams; }; +const buildTasksQueryForMissedUpdates = (startedOnTimestamp, size) => { + const completedTasksStatusList = Object.values(COMPLETED_TASK_STATUS); + return tasksModel + .where("status", "not-in", completedTasksStatusList) + .where("startedOn", "<", convertMillisToSeconds(startedOnTimestamp)) + .orderBy("assignee") + .limit(size); +}; + module.exports = { fromFirestoreData, toFirestoreData, buildTasks, transformQuery, parseSearchQuery, + buildTasksQueryForMissedUpdates, }; From 6cbe27c84b984694e7ccee5a234312111a57d417 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Fri, 8 Dec 2023 04:45:35 +0530 Subject: [PATCH 10/18] fix: failing tests --- test/fixtures/discordResponse/discord-response.js | 4 ++-- test/unit/models/discordactions.test.js | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/fixtures/discordResponse/discord-response.js b/test/fixtures/discordResponse/discord-response.js index 37a6193dd..4267e908e 100644 --- a/test/fixtures/discordResponse/discord-response.js +++ b/test/fixtures/discordResponse/discord-response.js @@ -8,7 +8,7 @@ const getDiscordMembers = [ nick: "jhon", pending: false, premium_since: null, - roles: ["hero", "racer", "9876543210"], + roles: ["hero", "racer"], user: { id: "123456789098765432", username: "jhondoe", @@ -31,7 +31,7 @@ const getDiscordMembers = [ nick: "david", pending: false, premium_since: null, - roles: ["hero", "racer", "9876543210"], + roles: ["hero", "racer"], user: { id: "12345678909867666", username: "david", diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index b73ceced4..728c04db8 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -641,8 +641,10 @@ describe("discordactions", function () { progressDataList.push(progressData); await Promise.all(progressDataList.map(async (progress) => await createProgressDocument(progress))); - - sinon.stub(discordService, "getDiscordMembers").returns(getDiscordMembers); + const discordMembers = [...getDiscordMembers]; + discordMembers[0].roles.push("9876543210"); + discordMembers[1].roles.push("9876543210"); + sinon.stub(discordService, "getDiscordMembers").returns(discordMembers); }); afterEach(async function () { sinon.restore(); From 19b5ce56ef15dc695378edd6c12d8a50f9795960 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Fri, 8 Dec 2023 21:47:49 +0530 Subject: [PATCH 11/18] feat: adds excluded days of week logic --- models/discordactions.js | 13 +++++++- test/unit/models/discordactions.test.js | 41 +++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/models/discordactions.js b/models/discordactions.js index 99cc7a435..05598e99f 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -841,7 +841,7 @@ const updateUsersWith31DaysPlusOnboarding = async () => { }; const getMissedProgressUpdatesUsers = async (options = {}) => { - const { cursor, size = 500, excludedDates = [], dateGap = 3 } = options; + const { cursor, size = 500, excludedDates = [], excludedDays = [0], dateGap = 3 } = options; const stats = { tasks: 0, missedUpdatesTasks: 0, @@ -858,6 +858,17 @@ const getMissedProgressUpdatesUsers = async (options = {}) => { } }); + if (excludedDays.length === 7) { + return { usersToAddRole: [], ...stats }; + } + + for (let i = gapWindowEnd; i >= gapWindowStart; i -= convertDaysToMilliseconds(1)) { + const day = new Date(i).getDay(); + if (excludedDays.includes(day)) { + gapWindowStart -= convertDaysToMilliseconds(1); + } + } + let taskQuery = buildTasksQueryForMissedUpdates(gapWindowStart, size); if (cursor) { diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index 728c04db8..430a9c105 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -595,6 +595,7 @@ describe("discordactions", function () { let activeUserWithProgressUpdates; let idleUser; let userNotInDiscord; + let activeUserId; beforeEach(async function () { idleUser = { ...userData[9], discordId: getDiscordMembers[0].user.id }; activeUserWithProgressUpdates = { ...userData[10], discordId: getDiscordMembers[1].user.id }; @@ -611,6 +612,7 @@ describe("discordactions", function () { await addUser(activeUserWithNoUpdates), // active user with no task progress updates await addUser(userNotInDiscord), // OOO user with ]); + activeUserId = userIdList[2]; await Promise.all([ await userStatusModel.updateUserStatus(userIdList[0], idleUserStatus), await userStatusModel.updateUserStatus(userIdList[1], activeUserStatus), @@ -625,12 +627,12 @@ describe("discordactions", function () { const validTask = { ...task, assignee: userIdList[index], - startedOn: (new Date().getTime() - convertDaysToMilliseconds(5)) / 1000, + startedOn: (new Date().getTime() - convertDaysToMilliseconds(7)) / 1000, endsOn: (new Date().getTime() + convertDaysToMilliseconds(4)) / 1000, status: TASK_STATUS.IN_PROGRESS, }; - tasksPromise.push(await tasksModel.add(validTask)); + tasksPromise.push(tasksModel.add(validTask)); } const taskIdList = (await Promise.all(tasksPromise)).map((tasksDoc) => tasksDoc.id); const progressDataList = []; @@ -676,7 +678,6 @@ describe("discordactions", function () { date3.setDate(date3.getDate() - 3); const date4 = new Date(); date4.setDate(date4.getDate() - 4); - // this should now only get users who have not provided any update in last 7 days instead of 3; const result = await getMissedProgressUpdatesUsers({ excludedDates: [date.valueOf(), date2.valueOf(), date3.valueOf(), date4.valueOf()], }); @@ -688,6 +689,40 @@ describe("discordactions", function () { }); }); + it("should not list of users when all days of week are excluded", async function () { + const result = await getMissedProgressUpdatesUsers({ + excludedDays: [0, 1, 2, 3, 4, 5, 6], + }); + expect(result).to.be.an("object"); + expect(result).to.be.deep.equal({ + tasks: 0, + missedUpdatesTasks: 0, + usersToAddRole: [], + }); + }); + it("should not list any users since 5 days of weeks are excluded", async function () { + const oneMonthOldTask = { ...tasksData[0] }; + oneMonthOldTask.assignee = activeUserId; + oneMonthOldTask.startedOn = (new Date().getTime() - convertDaysToMilliseconds(30)) / 1000; + oneMonthOldTask.endsOn = (new Date().getTime() + convertDaysToMilliseconds(4)) / 1000; + const taskId = (await tasksModel.add(oneMonthOldTask)).id; + const date = new Date(); + date.setDate(date.getDate() - 29); + const progressData = stubbedModelTaskProgressData(null, taskId, date.getTime(), date.valueOf()); + await createProgressDocument(progressData); + + const result = await getMissedProgressUpdatesUsers({ + excludedDays: [0, 1, 2, 3, 4, 5], + dateGap: 3, + }); + expect(result).to.be.an("object"); + expect(result).to.be.deep.equal({ + tasks: 1, + missedUpdatesTasks: 0, + usersToAddRole: [], + }); + }); + it("should process only 1 task when size is passed as 1", async function () { const result = await getMissedProgressUpdatesUsers({ size: 1 }); From e4f3d3063d45b790641de99351f37573397e6c0d Mon Sep 17 00:00:00 2001 From: Ajeyakrishna Date: Mon, 11 Dec 2023 01:41:36 +0530 Subject: [PATCH 12/18] fix: adds validation and improves comments --- test/unit/models/discordactions.test.js | 2 +- utils/time.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index 430a9c105..61a9c2267 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -610,7 +610,7 @@ describe("discordactions", function () { await addUser(idleUser), // idle user with no task progress updates await addUser(activeUserWithProgressUpdates), // active user with task progress updates await addUser(activeUserWithNoUpdates), // active user with no task progress updates - await addUser(userNotInDiscord), // OOO user with + await addUser(userNotInDiscord), // OOO user with no task progress updates ]); activeUserId = userIdList[2]; await Promise.all([ diff --git a/utils/time.js b/utils/time.js index 3d8518abf..dcb7db7ee 100644 --- a/utils/time.js +++ b/utils/time.js @@ -30,6 +30,7 @@ const convertDaysToMilliseconds = (days) => { * @returns {number} : seconds */ const convertMillisToSeconds = (milliseconds) => { + if (typeof milliseconds !== "number") throw Error("Not a number"); return milliseconds / 1000; }; /** From 80f5784d34024a8a99e2f683b98dfd9a68f49982 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna <98796547+Ajeyakrishna-k@users.noreply.github.com> Date: Tue, 12 Dec 2023 18:05:19 +0530 Subject: [PATCH 13/18] Fix api failure when a single user discord details were not present (#1755) * fix: handles failure due to missing discordId * fix : failing test due roles undefined * chore: remove roles import * fix: failing test due to users archived * fix: increase timeout to test * fix: failing test * chore: uncomment a test case line --------- Co-authored-by: Shubham Sharma <68867418+skv93-coder@users.noreply.github.com> --- models/discordactions.js | 15 ++++++++---- test/integration/discordactions.test.js | 31 ++++++++++++++++++------- test/unit/models/discordactions.test.js | 2 -- utils/users.js | 13 +++++++---- 4 files changed, 42 insertions(+), 19 deletions(-) diff --git a/models/discordactions.js b/models/discordactions.js index 17c31c436..b6d7efac8 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -470,7 +470,7 @@ const updateUsersNicknameStatus = async (lastNicknameUpdate) => { const today = new Date().getTime(); - const nicknameUpdatePromises = []; + let successfulUpdates = 0; const nicknameUpdateBatches = []; const totalUsersStatus = usersStatusDocs.length; @@ -505,14 +505,19 @@ const updateUsersNicknameStatus = async (lastNicknameUpdate) => { } }); - const settledPromises = await Promise.all(promises); - nicknameUpdatePromises.push(...settledPromises); + const settledPromises = await Promise.allSettled(promises); + + settledPromises.forEach((result) => { + if (result.status === "fulfilled" && !!result.value) { + successfulUpdates++; + } else { + logger.error(`Error while updating nickname: ${result.reason}`); + } + }); await new Promise((resolve) => setTimeout(resolve, 5000)); } - const successfulUpdates = nicknameUpdatePromises.length; - const res = { totalUsersStatus, successfulNicknameUpdates: successfulUpdates, diff --git a/test/integration/discordactions.test.js b/test/integration/discordactions.test.js index 57d6a933f..c0ad7f066 100644 --- a/test/integration/discordactions.test.js +++ b/test/integration/discordactions.test.js @@ -40,7 +40,7 @@ const { updateUserStatus } = require("../../models/userStatus"); const { generateUserStatusData } = require("../fixtures/userStatus/userStatus"); const { getDiscordMembers } = require("../fixtures/discordResponse/discord-response"); const { getOnboarding31DPlusMembers } = require("../fixtures/discordResponse/discord-response"); - +const discordRolesModel = require("../../models/discordactions"); chai.use(chaiHttp); const { userStatusDataForOooState } = require("../fixtures/userStatus/userStatus"); const { generateCronJobToken } = require("../utils/generateBotToken"); @@ -392,7 +392,12 @@ describe("Discord actions", function () { describe("POST /discord-actions/nickname/status", function () { let jwtToken; beforeEach(async function () { - const { id } = await userModel.add({ ...userData[0] }); + const userData2 = { ...userData[1] }; + delete userData2.discordId; + const [{ id }, { id: userId2 }] = await Promise.all([ + userModel.add({ ...userData[0] }), + userModel.add(userData2), + ]); const statusData = { ...userStatusDataForOooState, futureStatus: { @@ -402,7 +407,17 @@ describe("Discord actions", function () { }, userId: id, }; - await userStatusModel.add(statusData); + const statusData2 = { + ...userStatusDataForOooState, + futureStatus: { + state: "ACTIVE", + updatedAt: 1668211200000, + from: 1668709800000, + }, + userId: userId2, + }; + await Promise.all([userStatusModel.add(statusData), userStatusModel.add(statusData2)]); + jwtToken = generateCronJobToken({ name: CRON_JOB_HANDLER }); }); @@ -435,9 +450,9 @@ describe("Discord actions", function () { expect(res.body).to.deep.equal({ message: "Updated discord users nickname based on status", data: { - totalUsersStatus: 1, + totalUsersStatus: 2, successfulNicknameUpdates: 1, - unsuccessfulNicknameUpdates: 0, + unsuccessfulNicknameUpdates: 1, }, }); return done(); @@ -445,9 +460,9 @@ describe("Discord actions", function () { }).timeout(10000); it("should return object with 0 successful updates when user nickname changes", function (done) { - const response = "Error occurred while updating user's nickname"; - fetchStub.returns(Promise.reject(response)); + sinon.stub(discordRolesModel, "updateUsersNicknameStatus").throws(new Error()); + sinon.stub(); chai .request(app) .post("/discord-actions/nickname/status") @@ -464,7 +479,7 @@ describe("Discord actions", function () { expect(res.body.message).to.equal("An internal server error occurred"); return done(); }); - }); + }).timeout(10000); }); describe("POST /discord-actions/discord-roles", function () { before(async function () { diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index ce70375a7..d1d696ffc 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -7,7 +7,6 @@ const discordRoleModel = firestore.collection("discord-roles"); const memberRoleModel = firestore.collection("member-group-roles"); const userModel = firestore.collection("users"); const admin = require("firebase-admin"); - const { createNewRole, getAllGroupRoles, @@ -448,7 +447,6 @@ describe("discordactions", function () { beforeEach(async function () { fetchStub = sinon.stub(global, "fetch"); dataAccessLayerStub = sinon.stub(dataAccessLayer, "retrieveUsers"); - addedUers.forEach(({ username, discordId, id }) => { dataAccessLayerStub.withArgs(sinon.match({ id })).resolves({ user: { diff --git a/utils/users.js b/utils/users.js index 0b02bfe68..239801b09 100644 --- a/utils/users.js +++ b/utils/users.js @@ -4,7 +4,7 @@ const userModel = firestore.collection("users"); const { months, discordNicknameLength } = require("../constants/users"); const dataAccessLayer = require("../services/dataAccessLayer"); const discordService = require("../services/discordService"); - +const ROLES = require("../constants/roles"); const addUserToDBForTest = async (userData) => { await userModel.add(userData); }; @@ -271,10 +271,15 @@ const generateOOONickname = (username = "", from, until) => { */ const updateNickname = async (userId, status = {}) => { try { - const { user: { discordId, username } = {} } = await dataAccessLayer.retrieveUsers({ id: userId }); - if (!discordId || !username) { - throw new Error("Username or discordId unavailable"); + const { + user: { discordId, username, roles = {} }, + discordJoinedAt = {}, + } = await dataAccessLayer.retrieveUsers({ id: userId }); + + if (!discordId || !username || !discordJoinedAt || roles[ROLES.ARCHIVED]) { + throw new Error("User details unavailable"); } + try { const nickname = generateOOONickname(username, status.from, status.until); From 3aa1f18afec2a13ed37b95dce927ef365deb5a7d Mon Sep 17 00:00:00 2001 From: Ajeyakrishna <98796547+Ajeyakrishna-k@users.noreply.github.com> Date: Tue, 12 Dec 2023 23:03:48 +0530 Subject: [PATCH 14/18] Updates ts import (#1778) * fix: updates the ts import * chore: remove unwanted space --- utils/tasks.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utils/tasks.js b/utils/tasks.js index d09cc8887..c04107ce0 100644 --- a/utils/tasks.js +++ b/utils/tasks.js @@ -1,8 +1,7 @@ const { getUsername, getUserId, getParticipantUsernames, getParticipantUserIds } = require("./users"); -const { TASK_TYPE, MAPPED_TASK_STATUS } = require("../constants/tasks"); +const { TASK_TYPE, MAPPED_TASK_STATUS, COMPLETED_TASK_STATUS } = require("../constants/tasks"); const fireStore = require("../utils/firestore"); const tasksModel = fireStore.collection("tasks"); -const { COMPLETED_TASK_STATUS } = require("../constants/tasks.ts"); const { convertMillisToSeconds } = require("./time"); const fromFirestoreData = async (task) => { From 61433fc3806f208328a977bd80608285726745ee Mon Sep 17 00:00:00 2001 From: Ajeyakrishna <98796547+Ajeyakrishna-k@users.noreply.github.com> Date: Thu, 14 Dec 2023 05:05:27 +0530 Subject: [PATCH 15/18] Adds route and controller for missed update user list api (#1766) * feat: adds route and controller functions for missed update * feat: adds tests for controller and validator of missed update script * fix: correct sinon import * fix: update validator test for query params * fix: failing test due modification of shallow copy of test data * chore: remove unnecessary array spreading * chore: fix typo in discord member mock data index * chore: fixes lint issues --- constants/constants.ts | 12 +- constants/tasks.ts | 5 + controllers/tasks.js | 32 ++++- middlewares/validators/tasks.js | 64 ++++++++- routes/tasks.js | 11 +- test/integration/tasks.test.js | 123 +++++++++++++++++- test/unit/middlewares/tasks-validator.test.js | 93 ++++++++++++- 7 files changed, 333 insertions(+), 7 deletions(-) diff --git a/constants/constants.ts b/constants/constants.ts index 692eec686..9d36b9094 100644 --- a/constants/constants.ts +++ b/constants/constants.ts @@ -1,5 +1,15 @@ const DOCUMENT_WRITE_SIZE = 500; +const daysOfWeek = { + sun: 0, + mon: 1, + tue: 2, + wed: 3, + thu: 4, + fri: 5, + sat: 6, +}; module.exports = { - DOCUMENT_WRITE_SIZE, + DOCUMENT_WRITE_SIZE, + daysOfWeek, }; diff --git a/constants/tasks.ts b/constants/tasks.ts index fa71d25f7..ba29fc2ee 100644 --- a/constants/tasks.ts +++ b/constants/tasks.ts @@ -44,6 +44,10 @@ const COMPLETED_TASK_STATUS = { }; const TASK_SIZE = 5; +const tasksUsersStatus = { + MISSED_UPDATES: "missed-updates", +}; + module.exports = { TASK_TYPE, TASK_STATUS, @@ -52,4 +56,5 @@ module.exports = { TASK_SIZE, DEFAULT_TASK_PRIORITY, COMPLETED_TASK_STATUS, + tasksUsersStatus, }; diff --git a/controllers/tasks.js b/controllers/tasks.js index 9c743d688..44de1405f 100644 --- a/controllers/tasks.js +++ b/controllers/tasks.js @@ -1,5 +1,5 @@ const tasks = require("../models/tasks"); -const { TASK_STATUS, TASK_STATUS_OLD } = require("../constants/tasks"); +const { TASK_STATUS, TASK_STATUS_OLD, tasksUsersStatus } = require("../constants/tasks"); const { addLog } = require("../models/logs"); const { USER_STATUS } = require("../constants/users"); const { addOrUpdate, getRdsUserInfoByGitHubUsername } = require("../models/users"); @@ -13,6 +13,9 @@ const { updateUserStatusOnTaskUpdate, updateStatusOnTaskCompletion } = require(" const dataAccess = require("../services/dataAccessLayer"); const { parseSearchQuery } = require("../utils/tasks"); const { addTaskCreatedAtAndUpdatedAtFields } = require("../services/tasks"); +const { RQLQueryParser } = require("../utils/RQLParser"); +const { getMissedProgressUpdatesUsers } = require("../models/discordactions"); +const { daysOfWeek } = require("../constants/constants"); /** * Creates new task * @@ -475,6 +478,32 @@ const updateStatus = async (req, res) => { } }; +const getUsersHandler = async (req, res) => { + try { + const { size, cursor, q: queryString } = req.query; + const rqlParser = new RQLQueryParser(queryString); + const { "days-count": daysGap, weekday, date, status } = rqlParser.getFilterQueries(); + if (!!status && status.length === 1 && status[0].value === tasksUsersStatus.MISSED_UPDATES) { + if (daysGap && daysGap > 1) { + return res.boom.badRequest("number of days gap provided cannot be greater than 1"); + } + const response = await getMissedProgressUpdatesUsers({ + cursor: cursor, + size: size && Number.parseInt(size), + excludedDates: date?.map((date) => Number.parseInt(date.value)), + excludedDays: weekday?.map((day) => daysOfWeek[day.value]), + dateGap: !!daysGap && daysGap.length === 1 && Number.parseInt(daysGap[0].value), + }); + return res.status(200).json(response); + } else { + return res.boom.badRequest("Unknown type and query"); + } + } catch (error) { + logger.error("Error in fetching users details of tasks", error); + return res.boom.badImplementation(INTERNAL_SERVER_ERROR); + } +}; + module.exports = { addNewTask, fetchTasks, @@ -486,4 +515,5 @@ module.exports = { overdueTasks, assignTask, updateStatus, + getUsersHandler, }; diff --git a/middlewares/validators/tasks.js b/middlewares/validators/tasks.js index e3367d03b..bb85612a1 100644 --- a/middlewares/validators/tasks.js +++ b/middlewares/validators/tasks.js @@ -1,7 +1,9 @@ const joi = require("joi"); const { DINERO, NEELAM } = require("../../constants/wallets"); -const { TASK_STATUS, TASK_STATUS_OLD, MAPPED_TASK_STATUS } = require("../../constants/tasks"); - +const { TASK_STATUS, TASK_STATUS_OLD, MAPPED_TASK_STATUS, tasksUsersStatus } = require("../../constants/tasks"); +const { RQLQueryParser } = require("../../utils/RQLParser"); +const { Operators } = require("../../typeDefinitions/rqlParser"); +const { daysOfWeek } = require("../../constants/constants"); const TASK_STATUS_ENUM = Object.values(TASK_STATUS); const MAPPED_TASK_STATUS_ENUM = Object.keys(MAPPED_TASK_STATUS); @@ -190,10 +192,68 @@ const getTasksValidator = async (req, res, next) => { res.boom.badRequest(error.details[0].message); } }; +const getUsersValidator = async (req, res, next) => { + const queryParamsSchema = joi.object().keys({ + cursor: joi.string().optional(), + q: joi.string().optional(), + size: joi.number().integer().min(1).max(2013), + }); + const filtersSchema = joi.object().keys({ + status: joi + .array() + .items( + joi.object().keys({ + value: joi.string().valid(...Object.values(tasksUsersStatus)), + operator: joi.string().valid(Operators.INCLUDE), + }) + ) + .required(), + "days-count": joi + .array() + .items( + joi.object().keys({ + value: joi.number().integer().min(1).max(10), + operator: joi.string().valid(Operators.EXCLUDE), + }) + ) + .optional(), + weekday: joi + .array() + .items( + joi.object().keys({ + value: joi.string().valid(...Object.keys(daysOfWeek)), + operator: joi.string().valid(Operators.EXCLUDE), + }) + ) + .optional(), + date: joi + .array() + .items( + joi.object().keys({ + value: joi.date().timestamp(), + operator: joi.string().valid(Operators.EXCLUDE), + }) + ) + .optional(), + }); + try { + const { q: queryString } = req.query; + const rqlQueryParser = new RQLQueryParser(queryString); + await Promise.all([ + queryParamsSchema.validateAsync(req.query), + filtersSchema.validateAsync(rqlQueryParser.getFilterQueries()), + ]); + next(); + } catch (error) { + logger.error(`Error validating get tasks for users query : ${error}`); + res.boom.badRequest(error.details[0].message); + } +}; module.exports = { createTask, updateTask, updateSelfTask, getTasksValidator, + getUsersValidator, }; diff --git a/routes/tasks.js b/routes/tasks.js index b62fc2478..862100fff 100644 --- a/routes/tasks.js +++ b/routes/tasks.js @@ -2,12 +2,19 @@ const express = require("express"); const router = express.Router(); const authenticate = require("../middlewares/authenticate"); const tasks = require("../controllers/tasks"); -const { createTask, updateTask, updateSelfTask, getTasksValidator } = require("../middlewares/validators/tasks"); +const { + createTask, + updateTask, + updateSelfTask, + getTasksValidator, + getUsersValidator, +} = require("../middlewares/validators/tasks"); const authorizeRoles = require("../middlewares/authorizeRoles"); const { APPOWNER, SUPERUSER } = require("../constants/roles"); const assignTask = require("../middlewares/assignTask"); const { cacheResponse, invalidateCache } = require("../utils/cache"); const { ALL_TASKS } = require("../constants/cacheKeys"); +const { verifyCronJob } = require("../middlewares/authorizeBot"); router.get("/", getTasksValidator, cacheResponse({ invalidationKey: ALL_TASKS, expiry: 10 }), tasks.fetchTasks); router.get("/self", authenticate, tasks.getSelfTasks); @@ -40,6 +47,8 @@ router.patch( ); router.patch("/assign/self", authenticate, invalidateCache({ invalidationKeys: [ALL_TASKS] }), tasks.assignTask); +router.get("/users/discord", verifyCronJob, getUsersValidator, tasks.getUsersHandler); + router.post("/migration", authenticate, authorizeRoles([SUPERUSER]), tasks.updateStatus); module.exports = router; diff --git a/test/integration/tasks.test.js b/test/integration/tasks.test.js index 0720e3503..70cf4735f 100644 --- a/test/integration/tasks.test.js +++ b/test/integration/tasks.test.js @@ -9,20 +9,30 @@ const tasks = require("../../models/tasks"); const authService = require("../../services/authService"); const addUser = require("../utils/addUser"); const userModel = require("../../models/users"); +const userStatusModel = require("../../models/userStatus"); const config = require("config"); const cookieName = config.get("userToken.cookieName"); const userData = require("../fixtures/user/user")(); const tasksData = require("../fixtures/tasks/tasks")(); const { DINERO, NEELAM } = require("../../constants/wallets"); const cleanDb = require("../utils/cleanDb"); -const { TASK_STATUS } = require("../../constants/tasks"); +const { TASK_STATUS, tasksUsersStatus } = require("../../constants/tasks"); const updateTaskStatus = require("../fixtures/tasks/tasks1")(); +const userStatusData = require("../fixtures/userStatus/userStatus"); +const tasksModel = firestore.collection("tasks"); +const discordService = require("../../services/discordService"); +const { CRON_JOB_HANDLER } = require("../../constants/bot"); chai.use(chaiHttp); const appOwner = userData[3]; const superUser = userData[4]; let jwt, superUserJwt; +const { createProgressDocument } = require("../../models/progresses"); +const { stubbedModelTaskProgressData } = require("../fixtures/progress/progresses"); +const { convertDaysToMilliseconds } = require("../../utils/time"); +const { getDiscordMembers } = require("../fixtures/discordResponse/discord-response"); +const { generateCronJobToken } = require("../utils/generateBotToken"); const taskData = [ { @@ -1293,6 +1303,117 @@ describe("Tasks", function () { }); }); + describe("GET /tasks/users", function () { + let activeUserWithProgressUpdates; + let idleUser; + let userNotInDiscord; + let jwtToken; + beforeEach(async function () { + await cleanDb(); + idleUser = { ...userData[9], discordId: getDiscordMembers[0].user.id }; + activeUserWithProgressUpdates = { ...userData[10], discordId: getDiscordMembers[1].user.id }; + const activeUserWithNoUpdates = { ...userData[0], discordId: getDiscordMembers[2].user.id }; + userNotInDiscord = { ...userData[4], discordId: "Not in discord" }; + const { + idleStatus: idleUserStatus, + activeStatus: activeUserStatus, + userStatusDataForOooState: oooUserStatus, + } = userStatusData; + const userIdList = await Promise.all([ + await addUser(idleUser), // idle user with no task progress updates + await addUser(activeUserWithProgressUpdates), // active user with task progress updates + await addUser(activeUserWithNoUpdates), // active user with no task progress updates + await addUser(userNotInDiscord), // OOO user with + ]); + await Promise.all([ + await userStatusModel.updateUserStatus(userIdList[0], idleUserStatus), + await userStatusModel.updateUserStatus(userIdList[1], activeUserStatus), + await userStatusModel.updateUserStatus(userIdList[2], activeUserStatus), + await userStatusModel.updateUserStatus(userIdList[3], oooUserStatus), + ]); + + const tasksPromise = []; + + for (let index = 0; index < 4; index++) { + const task = tasksData[index]; + const validTask = { + ...task, + assignee: userIdList[index], + startedOn: (new Date().getTime() - convertDaysToMilliseconds(7)) / 1000, + endsOn: (new Date().getTime() + convertDaysToMilliseconds(4)) / 1000, + status: TASK_STATUS.IN_PROGRESS, + }; + + tasksPromise.push(tasksModel.add(validTask)); + } + const taskIdList = (await Promise.all(tasksPromise)).map((tasksDoc) => tasksDoc.id); + const progressDataList = []; + + const date = new Date(); + date.setDate(date.getDate() - 1); + const progressData = stubbedModelTaskProgressData(null, taskIdList[2], date.getTime(), date.valueOf()); + progressDataList.push(progressData); + + await Promise.all(progressDataList.map(async (progress) => await createProgressDocument(progress))); + const discordMembers = [...getDiscordMembers].map((user) => { + return { ...user }; + }); + const roles1 = [...discordMembers[0].roles, "9876543210"]; + const roles2 = [...discordMembers[1].roles, "9876543210"]; + discordMembers[0].roles = roles1; + discordMembers[1].roles = roles2; + sinon.stub(discordService, "getDiscordMembers").returns(discordMembers); + jwtToken = generateCronJobToken({ name: CRON_JOB_HANDLER }); + }); + afterEach(async function () { + sinon.restore(); + await cleanDb(); + }); + it("should return successful response with user id list", async function () { + const response = await chai + .request(app) + .get("/tasks/users/discord") + .query({ q: `status:${tasksUsersStatus.MISSED_UPDATES}` }) + .set("Authorization", `Bearer ${jwtToken}`); + expect(response.body).to.be.deep.equal({ + usersToAddRole: [activeUserWithProgressUpdates.discordId], + tasks: 4, + missedUpdatesTasks: 3, + }); + expect(response.status).to.be.equal(200); + }); + it("should return successful response with user id when all params are passed", async function () { + const response = await chai + .request(app) + .get("/tasks/users/discord") + .query({ + size: 3, + q: `status:${tasksUsersStatus.MISSED_UPDATES} -weekday:sun -weekday:mon -weekday:tue -weekday:wed -weekday:thu -weekday:fri -date:231423432 -days-count:4`, + }) + .set("Authorization", `Bearer ${jwtToken}`); + expect(response.body).to.be.deep.equal({ + usersToAddRole: [], + tasks: 0, + missedUpdatesTasks: 0, + }); + expect(response.status).to.be.equal(200); + }); + + it("should return bad request error when status is not passed", async function () { + const response = await chai + .request(app) + .get("/tasks/users/discord") + .query({}) + .set("Authorization", `Bearer ${jwtToken}`); + expect(response.body).to.be.deep.equal({ + error: "Bad Request", + message: '"status" is required', + statusCode: 400, + }); + expect(response.status).to.be.equal(400); + }); + }); + describe("PATCH /tasks/:id should update the tasks by SuperUser", function () { beforeEach(async function () { const superUserId = await addUser(superUser); diff --git a/test/unit/middlewares/tasks-validator.test.js b/test/unit/middlewares/tasks-validator.test.js index 38ded1ebc..2189b6825 100644 --- a/test/unit/middlewares/tasks-validator.test.js +++ b/test/unit/middlewares/tasks-validator.test.js @@ -2,10 +2,11 @@ const Sinon = require("sinon"); const { getTasksValidator, createTask, + getUsersValidator, updateTask: updateTaskValidator, } = require("../../../middlewares/validators/tasks"); const { expect } = require("chai"); -const { TASK_STATUS } = require("../../../constants/tasks"); +const { TASK_STATUS, tasksUsersStatus } = require("../../../constants/tasks"); describe("getTasks validator", function () { it("should pass the request when no values for query params dev or status is passed", async function () { @@ -615,4 +616,94 @@ describe("getTasks validator", function () { await updateTaskValidator(req, res, nextMiddlewareSpy); expect(nextMiddlewareSpy.callCount).to.be.equal(0); }); + describe("getUsersValidator | Validator", function () { + it("should pass the request when valid query parameters are provided", async function () { + const req = { + query: { + size: 10, + cursor: "someCursor", + q: `status:${tasksUsersStatus.MISSED_UPDATES} -days-count:2 -date:123423432 -weekday:sun`, + }, + }; + const res = {}; + const nextMiddlewareSpy = Sinon.spy(); + await getUsersValidator(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(1); + }); + it("should pass the request when multiple valid query parameters are provided", async function () { + const req = { + query: { + size: 10, + cursor: "someCursor", + q: `status:${tasksUsersStatus.MISSED_UPDATES} -days-count:2 -date:123423432 -weekday:sun -weekday:mon`, + }, + }; + const res = {}; + const nextMiddlewareSpy = Sinon.spy(); + await getUsersValidator(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(1); + }); + it("should pass the request when only required query parameters are provided", async function () { + const req = { + query: { + q: `status:${tasksUsersStatus.MISSED_UPDATES}`, + }, + }; + const res = {}; + const nextMiddlewareSpy = Sinon.spy(); + await getUsersValidator(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(1); + }); + + it("should not pass validation when invalid query parameters are provided", async function () { + const req = { + query: { + invalidParam: "someValue", + }, + }; + const res = { + boom: { + badRequest: Sinon.spy(), + }, + }; + const nextMiddlewareSpy = Sinon.spy(); + await getUsersValidator(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(0); + expect(res.boom.badRequest.callCount).to.be.equal(1); + }); + + it("should not pass validation when required parameters are missing", async function () { + const req = { + query: { + size: "someQuery", + }, + }; + const res = { + boom: { + badRequest: Sinon.spy(), + }, + }; + const nextMiddlewareSpy = Sinon.spy(); + await getUsersValidator(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(0); + expect(res.boom.badRequest.callCount).to.be.equal(1); + }); + + it("should not pass validation when invalid filter parameters are provided", async function () { + const req = { + query: { + q: "date:invalidOperator:2023-01-01", + }, + }; + const res = { + boom: { + badRequest: Sinon.spy(), + }, + }; + const nextMiddlewareSpy = Sinon.spy(); + await getUsersValidator(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(0); + expect(res.boom.badRequest.callCount).to.be.equal(1); + }); + }); }); From 61cadee78ca03dcac9dd0c039e5e22cd949719b7 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna <98796547+Ajeyakrishna-k@users.noreply.github.com> Date: Thu, 14 Dec 2023 05:10:57 +0530 Subject: [PATCH 16/18] Adds role id to config (#1782) * feat: adds route and controller functions for missed update * feat: adds tests for controller and validator of missed update script * fix: correct sinon import * fix: update validator test for query params * fix: failing test due modification of shallow copy of test data * chore: remove unnecessary array spreading * chore: fix typo in discord member mock data index * chore: fixes lint issues * chore: adds role ids to config --- config/default.js | 2 +- config/production.js | 1 + config/staging.js | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/config/default.js b/config/default.js index b607a7a61..91f44377c 100644 --- a/config/default.js +++ b/config/default.js @@ -14,7 +14,7 @@ module.exports = { discordUnverifiedRoleId: "", discordDeveloperRoleId: "", discordMavenRoleId: "", - discordMissedUpdatesRoleId: "discordMissedUpdatesRoleId", + discordMissedUpdatesRoleId: "", githubApi: { baseUrl: "https://api.github.com", org: "Real-Dev-Squad", diff --git a/config/production.js b/config/production.js index 56cf7a28c..cb21e9ac7 100644 --- a/config/production.js +++ b/config/production.js @@ -6,6 +6,7 @@ module.exports = { discordUnverifiedRoleId: "1103047289330745386", discordDeveloperRoleId: "915490782939582485", discordMavenRoleId: "875564640438997043", + discordMissedUpdatesRoleId: "1183553844811153458", userToken: { cookieName: "rds-session", }, diff --git a/config/staging.js b/config/staging.js index fea453041..8ba88722c 100644 --- a/config/staging.js +++ b/config/staging.js @@ -6,6 +6,7 @@ module.exports = { discordUnverifiedRoleId: "1120875993771544687", discordDeveloperRoleId: "1121445071213056071", discordMavenRoleId: "1152361736456896586", + discordMissedUpdatesRoleId: "1184201657404362772", enableFileLogs: false, enableConsoleLogs: true, From 78b04f4a7204319b4393ec205370144e307efe94 Mon Sep 17 00:00:00 2001 From: Randhir Kumar Singh <97341921+heyrandhir@users.noreply.github.com> Date: Thu, 14 Dec 2023 18:24:22 +0530 Subject: [PATCH 17/18] Enhancement : Restrict Task Status Change to Available for Task assigned to Self (#1785) --- middlewares/validators/tasks.js | 15 ++++++++++--- test/integration/tasks.test.js | 21 +++++++++++++++++++ test/unit/middlewares/tasks-validator.test.js | 19 +++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/middlewares/validators/tasks.js b/middlewares/validators/tasks.js index bb85612a1..c6bbe6dfe 100644 --- a/middlewares/validators/tasks.js +++ b/middlewares/validators/tasks.js @@ -1,4 +1,5 @@ const joi = require("joi"); +const { BadRequest } = require("http-errors"); const { DINERO, NEELAM } = require("../../constants/wallets"); const { TASK_STATUS, TASK_STATUS_OLD, MAPPED_TASK_STATUS, tasksUsersStatus } = require("../../constants/tasks"); const { RQLQueryParser } = require("../../utils/RQLParser"); @@ -118,14 +119,18 @@ const updateTask = async (req, res, next) => { }; const updateSelfTask = async (req, res, next) => { + const validStatus = [...TASK_STATUS_ENUM, ...Object.values(TASK_STATUS_OLD)].filter( + (item) => item !== TASK_STATUS.AVAILABLE + ); const schema = joi .object() .strict() .keys({ status: joi .string() - .valid(...TASK_STATUS_ENUM, ...Object.values(TASK_STATUS_OLD)) - .optional(), + .valid(...validStatus) + .optional() + .error(new BadRequest(`The value for the 'status' field is invalid.`)), percentCompleted: joi.number().integer().min(0).max(100).optional(), }); try { @@ -133,7 +138,11 @@ const updateSelfTask = async (req, res, next) => { next(); } catch (error) { logger.error(`Error validating updateSelfTask payload : ${error}`); - res.boom.badRequest(error.details[0].message); + if (error instanceof BadRequest) { + res.boom.badRequest(error.message); + } else { + res.boom.badRequest(error.details[0].message); + } } }; diff --git a/test/integration/tasks.test.js b/test/integration/tasks.test.js index 70cf4735f..a288b3a77 100644 --- a/test/integration/tasks.test.js +++ b/test/integration/tasks.test.js @@ -921,7 +921,26 @@ describe("Tasks", function () { isNoteworthy: true, }; + it("Should throw 400 Bad Request if the user tries to update the status of a task to AVAILABLE", function (done) { + chai + .request(app) + .patch(`/tasks/self/${taskId1}`) + .set("cookie", `${cookieName}=${jwt}`) + .send(taskStatusData) + .end((err, res) => { + if (err) { + return done(err); + } + expect(res).to.have.status(400); + expect(res.body).to.be.a("object"); + expect(res.body.error).to.equal("Bad Request"); + expect(res.body.message).to.equal("The value for the 'status' field is invalid."); + return done(); + }); + }); + it("Should update the task status for given self taskid", function (done) { + taskStatusData.status = "IN_PROGRESS"; chai .request(app) .patch(`/tasks/self/${taskId1}`) @@ -1002,6 +1021,7 @@ describe("Tasks", function () { }); it("Should return 404 if task doesnt exist", function (done) { + taskStatusData.status = "IN_PROGRESS"; chai .request(app) .patch("/tasks/self/wrongtaskId") @@ -1043,6 +1063,7 @@ describe("Tasks", function () { }); it("Should give 403 if status is already 'VERIFIED' ", async function () { + taskStatusData.status = "IN_PROGRESS"; taskId = (await tasks.updateTask({ ...taskData, assignee: appOwner.username })).taskId; const res = await chai .request(app) diff --git a/test/unit/middlewares/tasks-validator.test.js b/test/unit/middlewares/tasks-validator.test.js index 2189b6825..43faf5655 100644 --- a/test/unit/middlewares/tasks-validator.test.js +++ b/test/unit/middlewares/tasks-validator.test.js @@ -2,6 +2,7 @@ const Sinon = require("sinon"); const { getTasksValidator, createTask, + updateSelfTask, getUsersValidator, updateTask: updateTaskValidator, } = require("../../../middlewares/validators/tasks"); @@ -706,4 +707,22 @@ describe("getTasks validator", function () { expect(res.boom.badRequest.callCount).to.be.equal(1); }); }); + + describe("updateSelfTask Validator", function () { + it("should not pass the request when status is AVAILABLE", async function () { + const req = { + body: { + status: "AVAILABLE", + }, + }; + const res = { + boom: { + badRequest: Sinon.spy(), + }, + }; + const nextMiddlewareSpy = Sinon.spy(); + await updateSelfTask(req, res, nextMiddlewareSpy); + expect(nextMiddlewareSpy.callCount).to.be.equal(0); + }); + }); }); From 639ca1d287e12465a3dda74d520fe63dde0d0359 Mon Sep 17 00:00:00 2001 From: Ajeyakrishna <98796547+Ajeyakrishna-k@users.noreply.github.com> Date: Fri, 15 Dec 2023 20:47:26 +0530 Subject: [PATCH 18/18] Fix missed updates script (#1787) * fix: minor changes to missed updates * fix: adds an extra status to query * fix: use the correct function for time conversion * chore: update test case data * feat: update response for missed updates details * chore: updates response messages for tests --- controllers/tasks.js | 10 ++++++++-- models/discordactions.js | 12 ++++++------ test/integration/tasks.test.js | 20 +++++++++++++------- test/unit/models/discordactions.test.js | 4 ++-- utils/tasks.js | 9 ++++----- utils/time.js | 2 +- 6 files changed, 34 insertions(+), 23 deletions(-) diff --git a/controllers/tasks.js b/controllers/tasks.js index 44de1405f..45238d807 100644 --- a/controllers/tasks.js +++ b/controllers/tasks.js @@ -492,9 +492,15 @@ const getUsersHandler = async (req, res) => { size: size && Number.parseInt(size), excludedDates: date?.map((date) => Number.parseInt(date.value)), excludedDays: weekday?.map((day) => daysOfWeek[day.value]), - dateGap: !!daysGap && daysGap.length === 1 && Number.parseInt(daysGap[0].value), + dateGap: !!daysGap && daysGap.length === 1 ? Number.parseInt(daysGap[0].value) : null, }); - return res.status(200).json(response); + + if (response.error) { + return res.boom.badRequest(response.message); + } + return res + .status(200) + .json({ message: "Discord details of users with status missed updates fetched successfully", data: response }); } else { return res.boom.badRequest("Unknown type and query"); } diff --git a/models/discordactions.js b/models/discordactions.js index 2a7abf149..8adb45990 100644 --- a/models/discordactions.js +++ b/models/discordactions.js @@ -21,7 +21,7 @@ const discordMissedUpdatesRoleId = config.get("discordMissedUpdatesRoleId"); const userStatusModel = firestore.collection("usersStatus"); const usersUtils = require("../utils/users"); const { getUsersBasedOnFilter, fetchUser } = require("./users"); -const { convertDaysToMilliseconds } = require("../utils/time"); +const { convertDaysToMilliseconds, convertMillisToSeconds } = require("../utils/time"); const { chunks } = require("../utils/array"); const tasksModel = firestore.collection("tasks"); const { FIRESTORE_IN_CLAUSE_SIZE } = require("../constants/users"); @@ -874,13 +874,12 @@ const getMissedProgressUpdatesUsers = async (options = {}) => { } } - let taskQuery = buildTasksQueryForMissedUpdates(gapWindowStart, size); + let taskQuery = buildTasksQueryForMissedUpdates(size); if (cursor) { const data = await tasksModel.doc(cursor).get(); if (!data.data()) { return { - statusCode: 400, error: "Bad Request", message: `Invalid cursor: ${cursor}`, }; @@ -894,7 +893,9 @@ const getMissedProgressUpdatesUsers = async (options = {}) => { stats.tasks = tasksQuerySnapshot.size; tasksQuerySnapshot.forEach((doc) => { - const taskAssignee = doc.data().assignee; + const { assignee: taskAssignee, startedOn: taskStartedOn } = doc.data(); + if (!taskAssignee || taskStartedOn >= convertMillisToSeconds(gapWindowStart)) return; + const taskId = doc.id; if (usersMap.has(taskAssignee)) { @@ -952,8 +953,7 @@ const getMissedProgressUpdatesUsers = async (options = {}) => { }); }); - const discordUserList = await Promise.all(discordUsersPromise); - + const discordUserList = await discordUsersPromise; const discordUserMap = new Map(); discordUserList.forEach((discordUser) => { const discordUserData = { isBot: !!discordUser.user.bot }; diff --git a/test/integration/tasks.test.js b/test/integration/tasks.test.js index a288b3a77..57e009a6c 100644 --- a/test/integration/tasks.test.js +++ b/test/integration/tasks.test.js @@ -1397,9 +1397,12 @@ describe("Tasks", function () { .query({ q: `status:${tasksUsersStatus.MISSED_UPDATES}` }) .set("Authorization", `Bearer ${jwtToken}`); expect(response.body).to.be.deep.equal({ - usersToAddRole: [activeUserWithProgressUpdates.discordId], - tasks: 4, - missedUpdatesTasks: 3, + message: "Discord details of users with status missed updates fetched successfully", + data: { + usersToAddRole: [activeUserWithProgressUpdates.discordId], + tasks: 4, + missedUpdatesTasks: 3, + }, }); expect(response.status).to.be.equal(200); }); @@ -1408,14 +1411,17 @@ describe("Tasks", function () { .request(app) .get("/tasks/users/discord") .query({ - size: 3, + size: 5, q: `status:${tasksUsersStatus.MISSED_UPDATES} -weekday:sun -weekday:mon -weekday:tue -weekday:wed -weekday:thu -weekday:fri -date:231423432 -days-count:4`, }) .set("Authorization", `Bearer ${jwtToken}`); expect(response.body).to.be.deep.equal({ - usersToAddRole: [], - tasks: 0, - missedUpdatesTasks: 0, + message: "Discord details of users with status missed updates fetched successfully", + data: { + usersToAddRole: [], + tasks: 4, + missedUpdatesTasks: 0, + }, }); expect(response.status).to.be.equal(200); }); diff --git a/test/unit/models/discordactions.test.js b/test/unit/models/discordactions.test.js index ac453e3d3..2042f224c 100644 --- a/test/unit/models/discordactions.test.js +++ b/test/unit/models/discordactions.test.js @@ -682,7 +682,7 @@ describe("discordactions", function () { }); expect(result).to.be.an("object"); expect(result).to.be.deep.equal({ - tasks: 0, + tasks: 4, missedUpdatesTasks: 0, usersToAddRole: [], }); @@ -716,7 +716,7 @@ describe("discordactions", function () { }); expect(result).to.be.an("object"); expect(result).to.be.deep.equal({ - tasks: 1, + tasks: 5, missedUpdatesTasks: 0, usersToAddRole: [], }); diff --git a/utils/tasks.js b/utils/tasks.js index c04107ce0..f1b4616c6 100644 --- a/utils/tasks.js +++ b/utils/tasks.js @@ -1,8 +1,7 @@ const { getUsername, getUserId, getParticipantUsernames, getParticipantUserIds } = require("./users"); -const { TASK_TYPE, MAPPED_TASK_STATUS, COMPLETED_TASK_STATUS } = require("../constants/tasks"); +const { TASK_TYPE, MAPPED_TASK_STATUS, COMPLETED_TASK_STATUS, TASK_STATUS } = require("../constants/tasks"); const fireStore = require("../utils/firestore"); const tasksModel = fireStore.collection("tasks"); -const { convertMillisToSeconds } = require("./time"); const fromFirestoreData = async (task) => { if (!task) { @@ -113,11 +112,11 @@ const parseSearchQuery = (queryString) => { return searchParams; }; -const buildTasksQueryForMissedUpdates = (startedOnTimestamp, size) => { +const buildTasksQueryForMissedUpdates = (size) => { const completedTasksStatusList = Object.values(COMPLETED_TASK_STATUS); return tasksModel - .where("status", "not-in", completedTasksStatusList) - .where("startedOn", "<", convertMillisToSeconds(startedOnTimestamp)) + .where("status", "not-in", [...completedTasksStatusList, TASK_STATUS.AVAILABLE]) + .orderBy("status") .orderBy("assignee") .limit(size); }; diff --git a/utils/time.js b/utils/time.js index dcb7db7ee..5910a7bdc 100644 --- a/utils/time.js +++ b/utils/time.js @@ -31,7 +31,7 @@ const convertDaysToMilliseconds = (days) => { */ const convertMillisToSeconds = (milliseconds) => { if (typeof milliseconds !== "number") throw Error("Not a number"); - return milliseconds / 1000; + return Math.round(milliseconds / 1000); }; /** * Returns time in seconds of timestamp after given duration