Skip to content

Commit fff581f

Browse files
resolved comments
1 parent da621e0 commit fff581f

File tree

4 files changed

+16
-34
lines changed

4 files changed

+16
-34
lines changed

controllers/users.js

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const dataAccess = require("../services/dataAccessLayer");
1414
const { isLastPRMergedWithinDays } = require("../services/githubService");
1515
const logger = require("../utils/logger");
1616
const { SOMETHING_WENT_WRONG, INTERNAL_SERVER_ERROR } = require("../constants/errorMessages");
17-
const { OVERDUE_TASKS } = require("../constants/users");
17+
const { OVERDUE_TASKS, ALL_USER_ROLES } = require("../constants/users");
1818
const { getPaginationLink, getUsernamesFromPRs, getRoleToUpdate } = require("../utils/users");
1919
const { setInDiscordFalseScript, setUserDiscordNickname } = require("../services/discordService");
2020
const { generateDiscordProfileImageUrl } = require("../utils/discord-actions");
@@ -470,9 +470,9 @@ const getSelfDetails = async (req, res) => {
470470
* @param res {Object} - Express response object
471471
*/
472472

473-
const updateSelf = async (req, res) => {
473+
const updateSelf = async (req, res, next) => {
474474
try {
475-
const { id: userId, roles: userRoles, discordId, incompleteUserDetails } = req.userData;
475+
const { id: userId, roles: userRoles, discordId, incompleteUserDetails, role: existingRole } = req.userData;
476476
const devFeatureFlag = req.query.dev === "true";
477477
const { user } = await dataAccess.retrieveUsers({ id: userId });
478478
const { first_name: firstName, last_name: lastName, role } = req.body;
@@ -488,7 +488,7 @@ const updateSelf = async (req, res) => {
488488
req.body.role = role;
489489
await userQuery.setIncompleteUserDetails(userId);
490490
} else {
491-
const alreadyHasRole = userRoles[role];
491+
const alreadyHasRole = ALL_USER_ROLES.includes(existingRole);
492492
if (role && !alreadyHasRole) {
493493
req.body.role = role;
494494
}
@@ -523,7 +523,6 @@ const updateSelf = async (req, res) => {
523523
}
524524
}
525525

526-
// Handle developer-specific logic for disabledRoles
527526
if (userRoles.in_discord && !user.incompleteUserDetails) {
528527
const membersInDiscord = await getDiscordMembers();
529528
if (!Array.isArray(membersInDiscord))
@@ -532,35 +531,17 @@ const updateSelf = async (req, res) => {
532531
if (discordMember) {
533532
const { roles } = discordMember;
534533
if (roles && roles.includes(discordDeveloperRoleId)) {
535-
// Developers can only update disabledRoles with dev flag
536-
if (req.body.disabledRoles) {
537-
if (devFeatureFlag) {
538-
const updatedUser = await userQuery.addOrUpdate(
539-
{ disabled_roles: rolesToDisable },
540-
userId,
541-
devFeatureFlag
542-
);
543-
if (updatedUser) {
544-
return res
545-
.status(200)
546-
.send({ message: "Privilege modified successfully!", disabled_roles: rolesToDisable });
547-
}
548-
} else {
549-
// disabledRoles without dev flag should return 403
550-
return res.boom.forbidden(
551-
"Developers can only update disabled_roles. Use profile service for updating other attributes."
552-
);
534+
if (req.body.disabledRoles && devFeatureFlag) {
535+
const updatedUser = await userQuery.addOrUpdate({ disabled_roles: rolesToDisable }, userId, devFeatureFlag);
536+
if (updatedUser) {
537+
return res
538+
.status(200)
539+
.send({ message: "Privilege modified successfully!", disabled_roles: rolesToDisable });
553540
}
554541
}
555-
// Check if developer is trying to update something other than disabledRoles
556-
const hasOtherFields = Object.keys(req.body).some(
557-
(key) => key !== "disabledRoles" && req.body[key] !== undefined
542+
return res.boom.forbidden(
543+
"Developers can only update disabled_roles. Use profile service for updating other attributes."
558544
);
559-
if (hasOtherFields) {
560-
return res.boom.forbidden(
561-
"Developers can only update disabled_roles. Use profile service for updating other attributes."
562-
);
563-
}
564545
}
565546
}
566547
}
@@ -576,7 +557,7 @@ const updateSelf = async (req, res) => {
576557
return res.boom.notFound("User not found");
577558
} catch (error) {
578559
logger.error(`Error while updating user: ${error}`);
579-
return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
560+
return next(error);
580561
}
581562
};
582563

test/fixtures/user/user.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ module.exports = () => {
6161
},
6262
nickname_synced: false,
6363
profileStatus: "BLOCKED",
64+
incompleteUserDetails: false,
6465
},
6566
{
6667
username: "pranavg",

test/integration/auth.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ describe("auth", function () {
457457
.query({ code: "codeReturnedByGoogle", state: rdsUiUrl })
458458
.redirects(0);
459459
expect(res).to.have.status(302);
460-
expect(res.headers.location).to.equal(rdsUiUrl);
460+
expect(res.headers.location).to.equal("https://my.realdevsquad.com/new-signup");
461461
});
462462

463463
it("should get the verified email and redirect the google user to the goto page on successful login", async function () {

test/integration/users.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2992,7 +2992,7 @@ describe("Users", function () {
29922992
let id, jwtoken;
29932993

29942994
beforeEach(async function () {
2995-
id = await addUser();
2995+
id = await addUser(userData[0]);
29962996
jwtoken = authService.generateAuthToken({ userId: id });
29972997
fetchStub = Sinon.stub(global, "fetch");
29982998
const discordMembers = [...getDiscordMembers];

0 commit comments

Comments
 (0)