Skip to content

Commit b6351b7

Browse files
authored
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
1 parent 24d91f5 commit b6351b7

File tree

5 files changed

+120
-1
lines changed

5 files changed

+120
-1
lines changed

controllers/extensionRequests.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,22 @@ const getSelfExtensionRequests = async (req, res) => {
202202
* @param res {Object} - Express response object
203203
*/
204204
const updateExtensionRequest = async (req, res) => {
205+
const { dev } = req.query;
206+
const isDev = dev === "true";
205207
try {
206208
const extensionRequest = await extensionRequestsQuery.fetchExtensionRequest(req.params.id);
207209
if (!extensionRequest.extensionRequestData) {
208210
return res.boom.notFound("Extension Request not found");
209211
}
210212

213+
if (
214+
isDev &&
215+
!req.userData?.roles.super_user &&
216+
extensionRequest.extensionRequestData.status !== EXTENSION_REQUEST_STATUS.PENDING
217+
) {
218+
return res.boom.badRequest("Only pending extension request can be updated");
219+
}
220+
211221
if (req.body.assignee) {
212222
const { taskData: task } = await tasks.fetchTask(extensionRequest.extensionRequestData.taskId);
213223
if (task.assignee !== (await getUsername(req.body.assignee))) {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
const skipAuthorizeRolesUnderFF = (authorizeMiddleware) => {
2+
return (req, res, next) => {
3+
const { dev } = req.query;
4+
const isDev = dev === "true";
5+
if (isDev) {
6+
next();
7+
} else {
8+
authorizeMiddleware(req, res, next);
9+
}
10+
};
11+
};
12+
13+
module.exports = skipAuthorizeRolesUnderFF;

routes/extensionRequests.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,17 @@ const {
1010
updateExtensionRequestStatus,
1111
getExtensionRequestsValidator,
1212
} = require("../middlewares/validators/extensionRequests");
13+
const skipAuthorizeRolesUnderFF = require("../middlewares/skipAuthorizeRolesWrapper");
1314

1415
router.post("/", authenticate, createExtensionRequest, extensionRequests.createTaskExtensionRequest);
1516
router.get("/", authenticate, getExtensionRequestsValidator, extensionRequests.fetchExtensionRequests);
1617
router.get("/self", authenticate, extensionRequests.getSelfExtensionRequests);
1718
router.get("/:id", authenticate, authorizeRoles([SUPERUSER, APPOWNER]), extensionRequests.getExtensionRequest);
19+
// remove the skipAuthorizeRolesUnderFF & authorizeRoles middleware when removing the feature flag
1820
router.patch(
1921
"/:id",
2022
authenticate,
21-
authorizeRoles([SUPERUSER, APPOWNER]),
23+
skipAuthorizeRolesUnderFF(authorizeRoles([SUPERUSER, APPOWNER])),
2224
updateExtensionRequest,
2325
extensionRequests.updateExtensionRequest
2426
);

test/integration/extensionRequests.test.js

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,57 @@ describe("Extension Requests", function () {
921921
});
922922
});
923923

924+
it("User should be able to update the extensionRequest for the given extensionRequestId", function (done) {
925+
chai
926+
.request(app)
927+
.patch(`/extension-requests/${extensionRequestId4}?dev=true`)
928+
.set("cookie", `${cookieName}=${jwt}`)
929+
.send({
930+
title: "new-title",
931+
})
932+
.end((err, res) => {
933+
if (err) {
934+
return done(err);
935+
}
936+
expect(res).to.have.status(204);
937+
return done();
938+
});
939+
});
940+
941+
it("User should not be able to update the extensionRequest if already approved", function (done) {
942+
chai
943+
.request(app)
944+
.patch(`/extension-requests/${extensionRequestId1}?dev=true`)
945+
.set("cookie", `${cookieName}=${jwt}`)
946+
.send({
947+
title: "new-title",
948+
})
949+
.end((err, res) => {
950+
if (err) {
951+
return done(err);
952+
}
953+
expect(res).to.have.status(400);
954+
return done();
955+
});
956+
});
957+
958+
it("Super user should be able to update the extensionRequest if already approved", function (done) {
959+
chai
960+
.request(app)
961+
.patch(`/extension-requests/${extensionRequestId1}?dev=true`)
962+
.set("cookie", `${cookieName}=${superUserJwt}`)
963+
.send({
964+
title: "new-title",
965+
})
966+
.end((err, res) => {
967+
if (err) {
968+
return done(err);
969+
}
970+
expect(res).to.have.status(204);
971+
return done();
972+
});
973+
});
974+
924975
it("Should return 400 if assignee of the extensionrequest is upated with a different user", function (done) {
925976
chai
926977
.request(app)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
const chai = require("chai");
2+
const sinon = require("sinon");
3+
const { assert } = chai;
4+
const skipAuthorizeRolesUnderFF = require("../../../middlewares/skipAuthorizeRolesWrapper");
5+
6+
describe("skipAuthorizeRolesUnderFF Middleware", function () {
7+
let req, res, next, authorizeMiddleware;
8+
9+
beforeEach(function () {
10+
req = { query: {} };
11+
res = {};
12+
next = sinon.spy();
13+
authorizeMiddleware = sinon.spy();
14+
});
15+
16+
it("should call next() when dev is true", function () {
17+
req.query.dev = "true";
18+
19+
const middleware = skipAuthorizeRolesUnderFF(authorizeMiddleware);
20+
middleware(req, res, next);
21+
22+
assert.isTrue(next.calledOnce, "next() should be called once");
23+
assert.isFalse(authorizeMiddleware.called, "authorizeMiddleware should not be called");
24+
});
25+
26+
it("should call authorizeMiddleware when dev is false", function () {
27+
req.query.dev = "false";
28+
29+
const middleware = skipAuthorizeRolesUnderFF(authorizeMiddleware);
30+
middleware(req, res, next);
31+
32+
assert.isTrue(authorizeMiddleware.calledOnce, "authorizeMiddleware should be called once");
33+
assert.isFalse(next.called, "next() should not be called");
34+
});
35+
36+
it("should call authorizeMiddleware when dev is not provided", function () {
37+
const middleware = skipAuthorizeRolesUnderFF(authorizeMiddleware);
38+
middleware(req, res, next);
39+
40+
assert.isTrue(authorizeMiddleware.calledOnce, "authorizeMiddleware should be called once");
41+
assert.isFalse(next.called, "next() should not be called");
42+
});
43+
});

0 commit comments

Comments
 (0)