Allow user to edit Extension Requests before approval (#2215)

* added edit ability  for ER

* fixing function name and export type

* fix feature flag value check

* error message and status code updated

* middleware tests added

* pending status check added

* added comment regarding middleware for FF removal

controllers/extensionRequests.js

@@ -202,12 +202,22 @@ const getSelfExtensionRequests = async (req, res) => {
  * @param res {Object} - Express response object
  */
 const updateExtensionRequest = async (req, res) => {
+  const { dev } = req.query;
+  const isDev = dev === "true";
   try {
     const extensionRequest = await extensionRequestsQuery.fetchExtensionRequest(req.params.id);
     if (!extensionRequest.extensionRequestData) {
       return res.boom.notFound("Extension Request not found");
     }
 
+    if (
+      isDev &&
+      !req.userData?.roles.super_user &&
+      extensionRequest.extensionRequestData.status !== EXTENSION_REQUEST_STATUS.PENDING
+    ) {
+      return res.boom.badRequest("Only pending extension request can be updated");
+    }
+
     if (req.body.assignee) {
       const { taskData: task } = await tasks.fetchTask(extensionRequest.extensionRequestData.taskId);
       if (task.assignee !== (await getUsername(req.body.assignee))) {

middlewares/skipAuthorizeRolesWrapper.js

@@ -0,0 +1,13 @@
+const skipAuthorizeRolesUnderFF = (authorizeMiddleware) => {
+  return (req, res, next) => {
+    const { dev } = req.query;
+    const isDev = dev === "true";
+    if (isDev) {
+      next();
+    } else {
+      authorizeMiddleware(req, res, next);
+    }
+  };
+};
+
+module.exports = skipAuthorizeRolesUnderFF;

routes/extensionRequests.js

@@ -10,15 +10,17 @@ const {
   updateExtensionRequestStatus,
   getExtensionRequestsValidator,
 } = require("../middlewares/validators/extensionRequests");
+const skipAuthorizeRolesUnderFF = require("../middlewares/skipAuthorizeRolesWrapper");
 
 router.post("/", authenticate, createExtensionRequest, extensionRequests.createTaskExtensionRequest);
 router.get("/", authenticate, getExtensionRequestsValidator, extensionRequests.fetchExtensionRequests);
 router.get("/self", authenticate, extensionRequests.getSelfExtensionRequests);
 router.get("/:id", authenticate, authorizeRoles([SUPERUSER, APPOWNER]), extensionRequests.getExtensionRequest);
+//  remove the skipAuthorizeRolesUnderFF & authorizeRoles middleware when removing the feature flag
 router.patch(
   "/:id",
   authenticate,
-  authorizeRoles([SUPERUSER, APPOWNER]),
+  skipAuthorizeRolesUnderFF(authorizeRoles([SUPERUSER, APPOWNER])),
   updateExtensionRequest,
   extensionRequests.updateExtensionRequest
 );

test/integration/extensionRequests.test.js

@@ -921,6 +921,57 @@ describe("Extension Requests", function () {
         });
     });
 
+    it("User should be able to update the extensionRequest for the given extensionRequestId", function (done) {
+      chai
+        .request(app)
+        .patch(`/extension-requests/${extensionRequestId4}?dev=true`)
+        .set("cookie", `${cookieName}=${jwt}`)
+        .send({
+          title: "new-title",
+        })
+        .end((err, res) => {
+          if (err) {
+            return done(err);
+          }
+          expect(res).to.have.status(204);
+          return done();
+        });
+    });
+
+    it("User should not be able to update the extensionRequest if already approved", function (done) {
+      chai
+        .request(app)
+        .patch(`/extension-requests/${extensionRequestId1}?dev=true`)
+        .set("cookie", `${cookieName}=${jwt}`)
+        .send({
+          title: "new-title",
+        })
+        .end((err, res) => {
+          if (err) {
+            return done(err);
+          }
+          expect(res).to.have.status(400);
+          return done();
+        });
+    });
+
+    it("Super user should be able to update the extensionRequest if already approved", function (done) {
+      chai
+        .request(app)
+        .patch(`/extension-requests/${extensionRequestId1}?dev=true`)
+        .set("cookie", `${cookieName}=${superUserJwt}`)
+        .send({
+          title: "new-title",
+        })
+        .end((err, res) => {
+          if (err) {
+            return done(err);
+          }
+          expect(res).to.have.status(204);
+          return done();
+        });
+    });
+
     it("Should return 400 if assignee of the extensionrequest is upated with a different user", function (done) {
       chai
         .request(app)

test/unit/middlewares/skipAuthorizeRolesWrapper.test.js

@@ -0,0 +1,43 @@
+const chai = require("chai");
+const sinon = require("sinon");
+const { assert } = chai;
+const skipAuthorizeRolesUnderFF = require("../../../middlewares/skipAuthorizeRolesWrapper");
+
+describe("skipAuthorizeRolesUnderFF Middleware", function () {
+  let req, res, next, authorizeMiddleware;
+
+  beforeEach(function () {
+    req = { query: {} };
+    res = {};
+    next = sinon.spy();
+    authorizeMiddleware = sinon.spy();
+  });
+
+  it("should call next() when dev is true", function () {
+    req.query.dev = "true";
+
+    const middleware = skipAuthorizeRolesUnderFF(authorizeMiddleware);
+    middleware(req, res, next);
+
+    assert.isTrue(next.calledOnce, "next() should be called once");
+    assert.isFalse(authorizeMiddleware.called, "authorizeMiddleware should not be called");
+  });
+
+  it("should call authorizeMiddleware when dev is false", function () {
+    req.query.dev = "false";
+
+    const middleware = skipAuthorizeRolesUnderFF(authorizeMiddleware);
+    middleware(req, res, next);
+
+    assert.isTrue(authorizeMiddleware.calledOnce, "authorizeMiddleware should be called once");
+    assert.isFalse(next.called, "next() should not be called");
+  });
+
+  it("should call authorizeMiddleware when dev is not provided", function () {
+    const middleware = skipAuthorizeRolesUnderFF(authorizeMiddleware);
+    middleware(req, res, next);
+
+    assert.isTrue(authorizeMiddleware.calledOnce, "authorizeMiddleware should be called once");
+    assert.isFalse(next.called, "next() should not be called");
+  });
+});