Skip to content

Commit

Permalink
feat: implements a new GET route /users?profile=true (#2201)
Browse files Browse the repository at this point in the history
* adding new route for query param profile

* removing some mixed changes of other commit

* modified test cases and and controller logic

* Implemented GET route /users?profile=true to fetch user's details

* refactored: moved authentication logic from controller to middleware

* test cases fo authCondition file

* updated test cases and routes and middleware of users

* bug fix for using only in test cases

* code refactor and updated function naming
  • Loading branch information
vikasosmium authored Oct 16, 2024
1 parent 9a5e799 commit ba269bd
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 1 deletion.
21 changes: 21 additions & 0 deletions controllers/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,26 @@ const getUsers = async (req, res) => {
});
}

const profile = req.query.profile === "true";

if (profile) {
if (dev) {
if (!req.userData.id) {
return res.boom.badRequest("User ID not provided.");
}

try {
const result = await dataAccess.retrieveUsers({ id: req.userData.id });
return res.send(result.user);
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}
} else {
return res.boom.badRequest("Route not found");
}
}

if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") {
return res.boom.badRequest(`Days is required for filterBy ${transformedQuery?.filterBy}`);
}
Expand Down Expand Up @@ -393,6 +413,7 @@ const getSelfDetails = async (req, res) => {
* @param req.body {Object} - User object
* @param res {Object} - Express response object
*/

const updateSelf = async (req, res) => {
try {
const { id: userId, roles: userRoles, discordId } = req.userData;
Expand Down
10 changes: 10 additions & 0 deletions middlewares/authenticateProfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const authenticateProfile = (authenticate) => {
return async (req, res, next) => {
if (req.query.profile === "true") {
return await authenticate(req, res, next);
}
return next();
};
};

module.exports = authenticateProfile;
1 change: 1 addition & 0 deletions middlewares/validators/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ async function getUsers(req, res, next) {
}),
query: joi.string().optional(),
q: joi.string().optional(),
profile: joi.string().valid("true").optional(),
filterBy: joi.string().optional(),
days: joi.string().optional(),
dev: joi.string().optional(),
Expand Down
3 changes: 2 additions & 1 deletion routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ const checkIsVerifiedDiscord = require("../middlewares/verifydiscord");
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
const ROLES = require("../constants/roles");
const { Services } = require("../constants/bot");
const authenticateProfile = require("../middlewares/authenticateProfile");

router.post("/", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), users.markUnverified);
router.post("/update-in-discord", authenticate, authorizeRoles([SUPERUSER]), users.setInDiscordScript);
router.post("/verify", authenticate, users.verifyUser);
router.get("/userId/:userId", users.getUserById);
router.patch("/self", authenticate, userValidator.updateUser, users.updateSelf);
router.get("/", userValidator.getUsers, users.getUsers);
router.get("/", authenticateProfile(authenticate), userValidator.getUsers, users.getUsers);
router.get("/self", authenticate, users.getSelfDetails);
router.get("/isDeveloper", authenticate, users.isDeveloper);
router.get("/isUsernameAvailable/:username", authenticate, users.getUsernameAvailabilty);
Expand Down
57 changes: 57 additions & 0 deletions test/integration/users.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,63 @@ describe("Users", function () {
return done();
});
});

it("Should return the logged-in user's details when profile and dev is true", function (done) {
chai
.request(app)
.get("/users?profile=true&dev=true")
.set("cookie", `${cookieName}=${jwt}`)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(200);
expect(res.body).to.be.a("object");
expect(res.body).to.not.have.property("phone");
expect(res.body).to.not.have.property("email");
expect(res.body).to.not.have.property("chaincode");

return done();
});
});

it("Should throw an error when there is no feature flag given", function (done) {
chai
.request(app)
.get("/users?profile=true")
.set("cookie", `${cookieName}=${jwt}`)
.end((err, res) => {
if (err) {
return done(err);
}
expect(res).to.have.status(400);
expect(res.body).to.be.an("object");
expect(res.body.message).to.equal("Route not found");
return done();
});
});

it("Should return 401 if not logged in", function (done) {
chai
.request(app)
.get("/users?profile=true&dev=true")
.set("cookie", `${cookieName}=invalid_token`)
.end((err, res) => {
if (err) {
return done();
}

expect(res).to.have.status(401);
expect(res.body).to.be.an("object");
expect(res.body).to.eql({
statusCode: 401,
error: "Unauthorized",
message: "Unauthenticated User",
});

return done();
});
});
});

describe("GET /users/self", function () {
Expand Down
48 changes: 48 additions & 0 deletions test/unit/middlewares/authenticateProfile.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
const chai = require("chai");
const sinon = require("sinon");
const { expect } = chai;
const authenticateProfile = require("../../../middlewares/authenticateProfile.js");

describe("authenticateProfile Middleware", function () {
let req, res, next, authenticateStub, auth;

beforeEach(function () {
authenticateStub = sinon.spy();
auth = authenticateProfile(authenticateStub);

req = {
query: {},
};
res = {
boom: {
unauthorized: sinon.spy(),
forbidden: sinon.spy(),
},
};
next = sinon.spy();
});

it("should call authenticate when profile query is true", async function () {
req.query.profile = "true";
await auth(req, res, next);

expect(authenticateStub.withArgs(req, res, next).calledOnce).to.equal(true);
expect(next.calledOnce).to.equal(false);
});

it("should call next when profile query is not true", async function () {
req.query.profile = "false";

await auth(req, res, next);

expect(authenticateStub.calledOnce).to.equal(false);
expect(next.calledOnce).to.equal(true);
});

it("should call next when profile query is missing", async function () {
await auth(req, res, next);

expect(authenticateStub.calledOnce).to.equal(false);
expect(next.calledOnce).to.equal(true);
});
});
35 changes: 35 additions & 0 deletions test/unit/middlewares/user-validator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,41 @@ describe("Middleware | Validators | User", function () {
});
expect(nextSpy.calledOnce).to.be.equal(false);
});

it("Allows the request pass to next", async function () {
const req = {
query: {
profile: "true",
dev: "true",
},
};

const res = {};
const next = sinon.spy();

await getUsers(req, res, next);
expect(next.calledOnce).to.be.equal(true);
});

it("Stops the request for passing on to next", async function () {
const req = {
query: {
profile: "false",
},
};

const res = {
boom: {
badRequest: () => {},
},
};
const nextSpy = sinon.spy();

await getUsers(req, res, nextSpy).catch((err) => {
expect(err).to.be.an.instanceOf(Error);
});
expect(nextSpy.calledOnce).to.be.equal(false);
});
});

describe("validateGenerateUsernameQuery Middleware", function () {
Expand Down

0 comments on commit ba269bd

Please sign in to comment.