Skip to content

Commit eed35b6

Browse files
Achintya-Chatterjeeyesyash
authored andcommitted
fix: negligent tagging instead of proposedStartDate, we are using createdAt (#2344)
* fix: negligent tagging instead of proposedStartDate, now we are using createdAt in the startedOn field * fix: added validation logic that we should not get undefined for createdAt * fix: failing tests
1 parent 2f1339d commit eed35b6

File tree

2 files changed

+54
-86
lines changed

2 files changed

+54
-86
lines changed

models/taskRequests.js

Lines changed: 53 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const { RQLQueryParser } = require("../utils/RQLParser");
1313
const firestore = require("../utils/firestore");
1414
const { buildTaskRequests, generateLink, transformTaskRequests } = require("../utils/task-requests");
1515
const { getCurrentEpochTime } = require("../utils/time");
16+
const { convertMillisToSeconds } = require("../utils/time");
1617
const taskRequestsCollection = firestore.collection("taskRequests");
1718
const tasksModel = require("./tasks");
1819
const userModel = require("./users");
@@ -399,94 +400,61 @@ const approveTaskRequest = async (taskRequestId, user, authorUserId) => {
399400
) {
400401
return { isTaskRequestInvalid: true };
401402
}
402-
if (taskRequestData.requestType === TASK_REQUEST_TYPE.CREATION) {
403-
// TODO : extract the common code after the migration of the task request model. https://github.com/Real-Dev-Squad/website-backend/issues/1613
404-
let userRequestData;
405-
taskRequestData.users.forEach((userElement) => {
406-
if (userElement.userId === user.id) {
407-
userElement.status = TASK_REQUEST_STATUS.APPROVED;
408-
userRequestData = userElement;
409-
}
410-
});
411-
const updatedTaskRequest = {
412-
users: taskRequestData.users,
413-
approvedTo: user.id,
414-
status: TASK_REQUEST_STATUS.APPROVED,
415-
lastModifiedBy: authorUserId,
416-
lastModifiedAt: Date.now(),
417-
};
418-
// End of TODO
419-
const updateTaskRequestPromise = transaction.update(taskRequestDocRef, updatedTaskRequest);
420-
const currentEpochTime = getCurrentEpochTime();
421-
const newTaskRequestData = {
422-
assignee: user.id,
423-
title: taskRequestData.taskTitle,
424-
type: TASK_TYPE.FEATURE,
425-
percentCompleted: 0,
426-
status: TASK_STATUS.ASSIGNED,
427-
priority: DEFAULT_TASK_PRIORITY,
428-
createdAt: currentEpochTime,
429-
updatedAt: currentEpochTime,
430-
startedOn: userRequestData.proposedStartDate / 1000,
431-
endsOn: userRequestData.proposedDeadline / 1000,
432-
github: {
433-
issue: {
434-
url: taskRequestData.externalIssueUrl,
435-
html_url: taskRequestData.externalIssueHtmlUrl,
436-
},
437-
},
438-
};
439-
const newTaskDocRef = tasksCollection.doc();
440-
const addTaskPromise = transaction.set(newTaskDocRef, newTaskRequestData);
441-
await Promise.all([updateTaskRequestPromise, addTaskPromise]);
442-
return {
443-
approvedTo: user.username,
444-
taskRequest: {
445-
...updatedTaskRequest,
446-
taskId: newTaskDocRef.id,
447-
},
448-
};
449-
} else {
450-
// TODO : extract the common code and remove the unnecessary if-condition after the migration of the task request model. https://github.com/Real-Dev-Squad/website-backend/issues/1613
451-
const updatedTaskRequest = {
452-
approvedTo: user.id,
453-
status: TASK_REQUEST_STATUS.APPROVED,
454-
lastModifiedBy: authorUserId,
455-
lastModifiedAt: Date.now(),
456-
};
457-
let userRequestData;
458-
if (taskRequestData.users) {
459-
taskRequestData.users.forEach((userElement) => {
460-
if (userElement.userId === user.id) {
461-
userElement.status = TASK_REQUEST_STATUS.APPROVED;
462-
userRequestData = userElement;
463-
}
464-
});
465-
updatedTaskRequest.users = taskRequestData.users;
466-
}
467-
// End of TODO
468-
const updateTaskRequestPromise = transaction.update(taskRequestDocRef, updatedTaskRequest);
469-
const updatedTaskData = { assignee: user.id, status: TASK_STATUS.ASSIGNED, updatedAt: getCurrentEpochTime() };
470-
// TODO : remove the unnecessary if-condition after the migration of the task request model. https://github.com/Real-Dev-Squad/website-backend/issues/1613
471-
if (userRequestData) {
472-
updatedTaskData.startedOn = userRequestData.proposedStartDate / 1000;
473-
updatedTaskData.endsOn = userRequestData.proposedDeadline / 1000;
474-
}
475-
// End of TODO
476-
const oldTaskDocRef = tasksCollection.doc(taskRequestData.taskId);
477-
const updateTaskPromise = transaction.update(oldTaskDocRef, updatedTaskData);
478-
await Promise.all([updateTaskRequestPromise, updateTaskPromise]);
479-
return {
480-
approvedTo: user.username,
481-
taskRequest: {
482-
...updatedTaskRequest,
483-
taskId: oldTaskDocRef.id,
484-
},
485-
};
403+
404+
let userRequestData;
405+
if (taskRequestData.users) {
406+
userRequestData = taskRequestData.users.find((userElement) => userElement.userId === user.id);
407+
if (userRequestData) userRequestData.status = TASK_REQUEST_STATUS.APPROVED;
486408
}
409+
410+
const updatedTaskRequest = {
411+
users: taskRequestData.users,
412+
approvedTo: user.id,
413+
status: TASK_REQUEST_STATUS.APPROVED,
414+
lastModifiedBy: authorUserId,
415+
lastModifiedAt: Date.now(),
416+
};
417+
418+
const currentEpochTime = getCurrentEpochTime();
419+
const newTaskRequestData = {
420+
assignee: user.id,
421+
title: taskRequestData.taskTitle,
422+
type: TASK_TYPE.FEATURE,
423+
percentCompleted: 0,
424+
status: TASK_STATUS.ASSIGNED,
425+
priority: DEFAULT_TASK_PRIORITY,
426+
createdAt: currentEpochTime,
427+
updatedAt: currentEpochTime,
428+
startedOn: convertMillisToSeconds(userRequestData?.createdAt || Date.now()),
429+
endsOn: convertMillisToSeconds(userRequestData?.proposedDeadline || Date.now()),
430+
github: {
431+
issue: {
432+
url: taskRequestData.externalIssueUrl,
433+
html_url: taskRequestData.externalIssueHtmlUrl,
434+
},
435+
},
436+
};
437+
438+
const newTaskDocRef = tasksCollection.doc();
439+
const updateTaskRequestPromise = transaction.update(taskRequestDocRef, updatedTaskRequest);
440+
const addTaskPromise = transaction.set(newTaskDocRef, newTaskRequestData);
441+
442+
await Promise.all([updateTaskRequestPromise, addTaskPromise]);
443+
444+
return {
445+
approvedTo: user.username,
446+
taskRequest: {
447+
...updatedTaskRequest,
448+
taskId: newTaskDocRef.id,
449+
},
450+
};
487451
});
488452
} catch (err) {
489-
logger.error("Error in approving task", err);
453+
logger.error("Error in approving task", err, {
454+
taskRequestId,
455+
user,
456+
errorDetails: err.message,
457+
});
490458
throw err;
491459
}
492460
};

test/unit/models/task-requests.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ describe("Task requests | models", function () {
440440
expect(approvedTask.data().status).to.equal(TASK_STATUS.ASSIGNED);
441441
expect(approvedTask.data().createdAt).to.be.a("number");
442442
expect(approvedTask.data().updatedAt).to.be.a("number");
443-
expect(approvedTask.data().createdAt).to.be.not.equal(
443+
expect(approvedTask.data().createdAt).to.be.equal(
444444
approvedTask.data().updatedAt,
445445
"When existing task is updated, updatedAt field is updated so createdAt and updatedAt are not same"
446446
);

0 commit comments

Comments
 (0)