Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implements a new GET route /users?profile=true #2201

Merged
merged 12 commits into from
Oct 16, 2024
Merged
25 changes: 25 additions & 0 deletions controllers/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,30 @@ const getUsers = async (req, res) => {
});
}

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

if (profile) {
vikasosmium marked this conversation as resolved.
Show resolved Hide resolved
if (dev) {
let user;

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

return res.send(user);
} else {
return res.boom.badRequest("Route not found");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (dev) {
let user;
try {
if (req.userData.id) {
const result = await dataAccess.retrieveUsers({ id: req.userData.id });
user = result.user;
} else {
return res.boom.badRequest("User ID not provided.");
}
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}
return res.send(user);
} else {
return res.boom.badRequest("Route not found");
}
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");
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made the suggested changes.

}

if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") {
return res.boom.badRequest(`Days is required for filterBy ${transformedQuery?.filterBy}`);
}
Expand Down Expand Up @@ -393,6 +417,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/authCondition.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const authCondition = (authenticate) => {
vikasosmium marked this conversation as resolved.
Show resolved Hide resolved
return async (req, res, next) => {
if (req.query.profile === "true") {
return await authenticate(req, res, next);
}
return next();
};
};

module.exports = authCondition;
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(),
vikasosmium marked this conversation as resolved.
Show resolved Hide resolved
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 { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
const ROLES = require("../constants/roles");
const { Services } = require("../constants/bot");
const authCondition = require("../middlewares/authCondition");

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("/", authCondition(authenticate), userValidator.getUsers, users.getUsers);
Fixed Show fixed Hide fixed
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 user's details", 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();
});
});

vikasosmium marked this conversation as resolved.
Show resolved Hide resolved
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/authCondition.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 authCondition = require("../../../middlewares/authCondition.js");

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

beforeEach(function () {
authenticateStub = sinon.spy();
auth = authCondition(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
Loading