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 5 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
204 changes: 58 additions & 146 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,183 +87,88 @@ 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");
}

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

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.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);
}
const user = await userService.getUserByDiscordId(discordId);
return res.json({
message: "User returned successfully!",
user,
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.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);
}
});

if (filterBy === OVERDUE_TASKS) {
const users = await userService.getUsersByOverDueTasks(days, dev);
if (!users || users.length === 0) {
return res.json({
message: "Users returned successfully!",
count: usersData.length,
users: usersData,
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.json({
message: "Users returned successfully!",
count: users.length,
users: users,
});
}

if (qualifiers?.filterBy) {
Expand All @@ -278,18 +181,27 @@ const getUsers = async (req, res) => {
});
}

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

return res.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}`);
} catch (e) {
if (e instanceof NotFound) {
return res.boom.notFound(e.message);
}
if (e instanceof BadRequest) {
return res.boom.BadRequest(e.message);
}
if (e instanceof InternalServerError) {
return res.boom.internal(e.message);
}

return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
}
};
Expand Down
Loading