Merge pull request #2296 from Akshat187/develop

Updated the logic to return 404 instead of 200 when wrong taskRequest Id is sent

Author: Akshat187

models/taskRequests.js

@@ -171,16 +171,21 @@ const fetchTaskRequestById = async (taskRequestId) => {
     const taskRequestSnapshot = await taskRequestsCollection.doc(taskRequestId).get();
     const taskRequestData = taskRequestSnapshot.data();
 
-    if (taskRequestData) {
-      taskRequestData.id = taskRequestSnapshot.id;
-      taskRequestData.url = new URL(`/taskRequests/${taskRequestData.id}`, config.get("services.rdsUi.baseUrl"));
+    if (!taskRequestData) {
+      return {
+        taskRequestExists: false,
+      };
     }
+
+    taskRequestData.id = taskRequestSnapshot.id;
+    taskRequestData.url = new URL(`/taskRequests/${taskRequestData.id}`, config.get("services.rdsUi.baseUrl"));
+
     return {
       taskRequestData,
       taskRequestExists: true,
     };
   } catch (err) {
-    logger.error("Error in updating task", err);
+    logger.error("Error in fetching taskRequest by Id", err);
   }
 
   return {

test/integration/taskRequests.test.js

@@ -176,6 +176,22 @@ describe("Task Requests", function () {
           });
       });
 
+      it("should return 404 if the task request is not found for the given Id", function (done) {
+        chai
+          .request(app)
+          .get(`/taskRequests/1234`)
+          .set("cookie", `${cookieName}=${jwt}`)
+          .end((err, res) => {
+            if (err) {
+              return done(err);
+            }
+
+            expect(res).to.have.status(404);
+            expect(res.body.message).to.be.equal("Task request not found");
+            return done();
+          });
+      });
+
       it("should return 404 if the resource is not found", function (done) {
         sinon.stub(taskRequestsModel, "fetchTaskRequestById").callsFake(() => []);
 
@@ -196,21 +212,26 @@ describe("Task Requests", function () {
     });
 
     describe("When the user is not a super user", function () {
+      let taskRequestId;
+
       before(async function () {
         userId = await addUser(member);
-        sinon.stub(authService, "verifyAuthToken").callsFake(() => ({ userId }));
-        jwt = authService.generateAuthToken({ userId });
+        sinon.stub(authService, "verifyAuthToken").callsFake(() => ({
+          userId,
+        }));
+        jwt = authService.generateAuthToken({
+          userId,
+        });
 
         taskId = (await tasksModel.updateTask(taskData[4])).taskId;
-
         await userStatusModel.updateUserStatus(userId, idleUserStatus);
-        await taskRequestsModel.addOrUpdate(taskId, userId);
+        taskRequestId = (await taskRequestsModel.addOrUpdate(taskId, userId)).id;
       });
 
       it("should be successful when the user is not a super user", function (done) {
         chai
           .request(app)
-          .get(`/taskRequests/taskrequstid`)
+          .get(`/taskRequests/${taskRequestId}`)
           .set("cookie", `${cookieName}=${jwt}`)
           .end((err, res) => {
             if (err) {

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

@@ -1,11 +1,13 @@
 const chai = require("chai");
 const sinon = require("sinon");
 const { expect } = chai;
+const assert = require("chai").assert;
 const {
   createRequest,
   fetchTaskRequests,
   approveTaskRequest,
   fetchPaginatedTaskRequests,
+  fetchTaskRequestById,
   addNewFields,
   removeOldField,
   addUsersCountAndCreatedAt,
@@ -197,6 +199,30 @@ describe("Task requests | models", function () {
     });
   });
 
+  describe("fetchTaskRequestById", function () {
+    afterEach(async function () {
+      await cleanDb();
+    });
+
+    it("should return task request data when task request exists", async function () {
+      const requestData = mockData.taskRequestData;
+      const addedTaskRequest = await createRequest(requestData, "testUser");
+      const result = await fetchTaskRequestById(addedTaskRequest.id);
+      assert.isTrue(result.taskRequestExists, "Task request should exist");
+
+      expect(result.taskRequestExists).to.be.equal(true);
+      expect(result.taskRequestData.id).to.equal(addedTaskRequest.id);
+    });
+
+    it("should return taskRequestExists as false when task request does not exist", async function () {
+      const mockTaskRequestId = "taskRequest1234";
+      const requestData = mockData.taskRequestData;
+      await createRequest(requestData, "testUser");
+      const result = await fetchTaskRequestById(mockTaskRequestId);
+      expect(result.taskRequestExists).to.be.equal(false);
+    });
+  });
+
   describe("fetchTaskRequests", function () {
     beforeEach(async function () {
       const approvedTaskRequest = {