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

Refactor getUser controller function based on query parameter #2370

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Refactor getUser controller function based on query parameter #2370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9dc9d72
Refactoring user controller into smaller functions based on query par…
Prakhar-FF13 Jan 24, 2025
1eb75fc
Moving refactored mini functions from utils to controllers folder
Prakhar-FF13 Jan 25, 2025
cf511b3
Updating user integration test after refactoring
Prakhar-FF13 Jan 26, 2025
220e533
1. Refactored query parameter based functions to services/users file …
Prakhar-FF13 Jan 27, 2025
3b9d7bd
Returning the original status code and message for departed users que…
Prakhar-FF13 Feb 5, 2025
5d92aca
1. Adding explicit status code.
Prakhar-FF13 Feb 12, 2025
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
216 changes: 64 additions & 152 deletions controllers/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const { profileDiffStatus } = require("../constants/profileDiff");
const { logType } = require("../constants/logs");
const ROLES = require("../constants/roles");
const dataAccess = require("../services/dataAccessLayer");
const { isLastPRMergedWithinDays } = require("../services/githubService");
const logger = require("../utils/logger");
const { SOMETHING_WENT_WRONG, INTERNAL_SERVER_ERROR } = require("../constants/errorMessages");
const { OVERDUE_TASKS } = require("../constants/users");
Expand All @@ -20,7 +19,6 @@ const { setInDiscordFalseScript, setUserDiscordNickname } = require("../services
const { generateDiscordProfileImageUrl } = require("../utils/discord-actions");
const { addRoleToUser, getDiscordMembers } = require("../services/discordService");
const { fetchAllUsers } = require("../models/users");
const { getOverdueTasks } = require("../models/tasks");
const { getQualifiers } = require("../utils/helper");
const { parseSearchQuery } = require("../utils/users");
const { getFilteredPRsOrIssues } = require("../utils/pullRequests");
Expand All @@ -31,12 +29,12 @@ const {
USERS_PATCH_HANDLER_SUCCESS_MESSAGES,
} = require("../constants/users");
const { addLog } = require("../models/logs");
const { getUserStatus } = require("../models/userStatus");
const config = require("config");
const { generateUniqueUsername } = require("../services/users");
const userService = require("../services/users");
const { NotFound, BadRequest, InternalServerError } = require("http-errors");
const discordDeveloperRoleId = config.get("discordDeveloperRoleId");
const usersCollection = firestore.collection("users");
const userService = require("../services/users");

const verifyUser = async (req, res) => {
const userId = req.userData.id;
Expand Down Expand Up @@ -89,208 +87,122 @@ const getUserById = async (req, res) => {

const getUsers = async (req, res) => {
try {
// getting user details by id if present.
const { q, dev: devParam, query } = req.query;
const reqQueryObject = req.query;
const { q, dev: devParam, query, departed, id, profile: profileParam, discordId } = reqQueryObject;
const userData = req.userData || {};

const profile = profileParam === "true";
const isDeparted = departed === "true";
const dev = devParam === "true";
const queryString = (dev ? q : query) || "";
const transformedQuery = parseSearchQuery(queryString);
const { filterBy, days } = transformedQuery;
const qualifiers = getQualifiers(queryString);
// Should throw an error if the new query parameter is without feature flag
if (q && !dev) {
return res.boom.notFound("Route not found");
}
// getting user details by id if present.

if (req.query.id) {
const id = req.query.id;
let result, user;
try {
result = await dataAccess.retrieveUsers({ id: id });
user = result.user;
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
}
if (!result.userExists) {
return res.boom.notFound("User doesn't exist");
}
return res.json({

if (id) {
const user = await userService.findUserById(id);
return res.status(200).json({
message: "User returned successfully!",
user,
user: user,
});
}

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

if (profile) {
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);
}
const user = await userService.getUserByProfileData(userData);
return res.status(200).send(user);
}

if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") {
return res.boom.badRequest(`Days is required for filterBy ${transformedQuery?.filterBy}`);
}

const { filterBy, days } = transformedQuery;
if (filterBy === "unmerged_prs" && days) {
try {
const inDiscordUser = await dataAccess.retrieveUsersWithRole(ROLES.INDISCORD);
const users = [];

for (const user of inDiscordUser) {
const username = user.github_id;
const isMerged = await isLastPRMergedWithinDays(username, days);
if (!isMerged) {
users.push(user.id);
}
}

return res.json({
message: "Inactive users returned successfully!",
count: users.length,
users: users,
});
} catch (error) {
logger.error(`Error while fetching all users: ${error}`);
return res.boom.serverUnavailable("Something went wrong please contact admin");
}
const users = await userService.getUsersByUnmergedPrs(days);
return res.status(200).json({
message: "Inactive users returned successfully!",
count: users.length,
users: users,
});
}

// getting user details by discord id if present.
const discordId = req.query.discordId;

if (req.query.discordId) {
if (discordId) {
if (dev) {
let result, user;
try {
result = await dataAccess.retrieveUsers({ discordId });
user = result.user;
if (!result.userExists) {
return res.json({
message: "User not found",
user: null,
});
}

const userStatusResult = await getUserStatus(user.id);
if (userStatusResult.userStatusExists) {
user.state = userStatusResult.data.currentStatus.state;
}
} catch (error) {
logger.error(`Error while fetching user: ${error}`);
return res.boom.serverUnavailable(INTERNAL_SERVER_ERROR);
}
return res.json({
message: "User returned successfully!",
user,
const user = await userService.getUserByDiscordId(discordId);
return res.status(200).json({
message: user ? "User returned successfully!" : "User not found",
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 change?

user: user,
});
} else {
return res.boom.notFound("Route not found");
}
}

const isDeparted = req.query.departed === "true";

if (isDeparted) {
if (!dev) {
return res.boom.notFound("Route not found");
}
try {
const result = await dataAccess.retrieveUsers({ query: req.query });
const departedUsers = await userService.getUsersWithIncompleteTasks(result.users);
if (departedUsers.length === 0) return res.status(204).send();
return res.json({
message: "Users with abandoned tasks fetched successfully",
users: departedUsers,
links: {
next: result.nextId ? getPaginationLink(req.query, "next", result.nextId) : "",
prev: result.prevId ? getPaginationLink(req.query, "prev", result.prevId) : "",
},
});
} catch (error) {
logger.error("Error when fetching users who abandoned tasks:", error);
return res.boom.badImplementation(INTERNAL_SERVER_ERROR);
}
const { result, departedUsers } = await userService.getDepartedUsers(reqQueryObject);
if (departedUsers.length === 0) return res.status(204).send();
return res.status(200).json({
message: "Users with abandoned tasks fetched successfully",
users: departedUsers,
links: {
next: result.nextId ? getPaginationLink(reqQueryObject, "next", result.nextId) : "",
prev: result.prevId ? getPaginationLink(reqQueryObject, "prev", result.prevId) : "",
},
});
}

if (transformedQuery?.filterBy === OVERDUE_TASKS) {
try {
const tasksData = await getOverdueTasks(days);
if (!tasksData.length) {
return res.json({
message: "No users found",
users: [],
});
}
const userIds = new Set();
const usersData = [];

tasksData.forEach((task) => {
if (task.assignee) {
userIds.add(task.assignee);
}
});

const userInfo = await dataAccess.retrieveUsers({ userIds: Array.from(userIds) });
userInfo.forEach((user) => {
if (!user.roles.archived) {
const userTasks = tasksData.filter((task) => task.assignee === user.id);
const userData = {
id: user.id,
discordId: user.discordId,
username: user.username,
};
if (dev) {
userData.tasks = userTasks;
}
usersData.push(userData);
}
});

return res.json({
message: "Users returned successfully!",
count: usersData.length,
users: usersData,
if (filterBy === OVERDUE_TASKS) {
const users = await userService.getUsersByOverDueTasks(days, dev);
if (!users || users.length === 0) {
return res.status(200).json({
message: "No users found",
users: [],
Copy link
Member

Choose a reason for hiding this comment

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

why we removed the count field?

});
} catch (error) {
const errorMessage = `Error while fetching users and tasks: ${error}`;
logger.error(errorMessage);
return res.boom.serverUnavailable("Something went wrong, please contact admin");
}
return res.status(200).json({
message: "Users returned successfully!",
count: users.length,
users: users,
});
}

if (qualifiers?.filterBy) {
const allPRs = await getFilteredPRsOrIssues(qualifiers);
const usernames = getUsernamesFromPRs(allPRs);
const users = await dataAccess.retrieveUsers({ usernames: usernames });
return res.json({
return res.status(200).json({
message: "Users returned successfully!",
users,
});
}

const data = await dataAccess.retrieveUsers({ query: req.query });
const data = await dataAccess.retrieveUsers({ query: reqQueryObject });

return res.json({
return res.status(200).json({
message: "Users returned successfully!",
users: data.users,
links: {
next: data.nextId ? getPaginationLink(req.query, "next", data.nextId) : "",
prev: data.prevId ? getPaginationLink(req.query, "prev", data.prevId) : "",
next: data.nextId ? getPaginationLink(reqQueryObject, "next", data.nextId) : "",
prev: data.prevId ? getPaginationLink(reqQueryObject, "prev", data.prevId) : "",
},
});
} catch (error) {
logger.error(`Error while fetching all users: ${error}`);
return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
} catch (e) {
switch (true) {
case e instanceof NotFound:
return res.boom.notFound(e.message);
case e instanceof BadRequest:
return res.boom.badRequest(e.message);
case e instanceof InternalServerError:
return res.boom.internal(e.message);
default:
return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
}
}
};

Expand Down
Loading