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.

Sign up for GitHub

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

Fix/allow users to view their own application status #2080

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix/allow users to view their own application status #2080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion controllers/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ const addUserIntro = async (req, res) => {
}
};

const getUserIntro = async (req, res) => {
/* const getUserIntro = async (req, res) => {
try {
const data = await userQuery.getJoinData(req.params.userId);
if (data.length) {
Expand All @@ -737,6 +737,33 @@ const getUserIntro = async (req, res) => {
logger.error("Could Not Get User Data", err);
return res.boom.badImplementation(INTERNAL_SERVER_ERROR);
}
}; */
const getUserIntro = async (req, res) => {
try {
const { userId } = req.params;
const loggedInUserId = req.userData.id;
const data = await userQuery.getJoinData(userId);

if (data.length) {
if (userId === loggedInUserId || req.userData.roles.super_user) {
return res.json({
message: "User data returned",
data: data,
});
} else {
return res.status(403).json({
message: "You're not authorized to view this page",
});
}
} else {
return res.status(404).json({
message: "Data Not Found",
});
}
} catch (err) {
logger.error("Could Not Get User Data", err);
return res.boom.badImplementation(INTERNAL_SERVER_ERROR);
}
};

/**
Expand Down
2 changes: 1 addition & 1 deletion routes/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
users.updateDiscordUserNickname
);
router.get("/:username", users.getUser);
router.get("/:userId/intro", authenticate, authorizeRoles([SUPERUSER]), users.getUserIntro);
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

As we already have application route why you can't fetch with it

Copy link
Member Author

@Achintya-Chatterjee Achintya-Chatterjee Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Have to change many code in website-www, to use this application route. Also new user's could only see their application status in the /intro route only.

Copy link
Member

Choose a reason for hiding this comment

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

But this is not the good solution
And ask with Ankush or Tejas is your PR is going to merge or my

Copy link
Member Author

@Achintya-Chatterjee Achintya-Chatterjee Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Only superusers would be able to accept new user request, no one else, also if you're not the applicants you will always get you are not authorised to view this. Also for your information this a temporary fix, will reverse this after all the new applicants get's onboarded into RDS discord server

How are you telling that it's not a good solution

Copy link
Member

Choose a reason for hiding this comment

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

But there is already route present for view the application then what is the need of this new route?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't build this from the ground up, it was there, just did some modifications, also previously told you that if I wanted to use the application route , will have to change many code.

router.get("/:userId/intro", authenticate, users.getUserIntro);
Dismissed Show dismissed Hide dismissed
router.put("/self/intro", authenticate, userValidator.validateJoinData, users.addUserIntro);
router.get("/:id/skills", users.getUserSkills);
router.get("/:id/badges", getUserBadges);
Expand Down
Loading