From 9dc9d7254cb9a8b0db2fb3a2f1e15d5badb53c63 Mon Sep 17 00:00:00 2001 From: Prakhar Kumar Date: Fri, 24 Jan 2025 21:27:05 +0530 Subject: [PATCH 1/6] Refactoring user controller into smaller functions based on query parameters Signed-off-by: Prakhar Kumar --- controllers/users.js | 219 +++++++++++++------------------------------ utils/users.js | 157 ++++++++++++++++++++++++++++++- 2 files changed, 223 insertions(+), 153 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 6cbebe7b4..2b98ebc81 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -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"); @@ -20,21 +19,20 @@ 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"); const { getFilteredPaginationLink } = require("../utils/userStatus"); +const userUtils = require("../utils/users"); const { USERS_PATCH_HANDLER_ACTIONS, USERS_PATCH_HANDLER_ERROR_MESSAGES, 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 } = require("http-errors"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const usersCollection = firestore.collection("users"); @@ -90,204 +88,121 @@ 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"); + try { + if (id) { + const user = await userUtils.findUserById(id); + return res.json({ + message: "User returned successfully!", + user: user, + }); } - return res.json({ - message: "User returned successfully!", - user, - }); - } - const profile = req.query.profile === "true"; - - if (profile) { - if (!req.userData.id) { - return res.boom.badRequest("User ID not provided."); + if (profile) { + return res.send(await userUtils.getUserByProfileData(userData)); } - 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); + if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") { + return res.boom.badRequest(`Days is required for filterBy ${transformedQuery?.filterBy}`); } - } - - 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); - } - } + if (filterBy === "unmerged_prs" && days) { + const users = await userUtils.getUsersByUnmergedPrs(days); 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"); } - } - // getting user details by discord id if present. - const discordId = req.query.discordId; - - if (req.query.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); + if (discordId) { + if (dev) { + const user = await userUtils.getUserByDiscordId(discordId); + return res.json({ + message: user ? "User returned successfully!" : "User not found", + user: user, + }); + } else { + return res.boom.notFound("Route not found"); } - return res.json({ - message: "User returned successfully!", - 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 (isDeparted) { + if (!dev) { + return res.boom.notFound("Route not found"); + } + const { result, departedUsers } = await userUtils.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(req.query, "next", result.nextId) : "", - prev: result.prevId ? getPaginationLink(req.query, "prev", result.prevId) : "", + next: result.nextId ? getPaginationLink(reqQueryObject, "next", result.nextId) : "", + prev: result.prevId ? getPaginationLink(reqQueryObject, "prev", result.prevId) : "", }, }); - } catch (error) { - logger.error("Error when fetching users who abandoned tasks:", error); - return res.boom.badImplementation(INTERNAL_SERVER_ERROR); } - } - if (transformedQuery?.filterBy === OVERDUE_TASKS) { - try { - const tasksData = await getOverdueTasks(days); - if (!tasksData.length) { + if (filterBy === OVERDUE_TASKS) { + const users = await userUtils.getUsersByOverDueTasks(days, dev); + if (!users || users.length === 0) { 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: 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({ message: "Users returned successfully!", - count: usersData.length, - users: usersData, + users, }); - } catch (error) { - const errorMessage = `Error while fetching users and tasks: ${error}`; - logger.error(errorMessage); - return res.boom.serverUnavailable("Something went wrong, please contact admin"); } - } - if (qualifiers?.filterBy) { - const allPRs = await getFilteredPRsOrIssues(qualifiers); - const usernames = getUsernamesFromPRs(allPRs); - const users = await dataAccess.retrieveUsers({ usernames: usernames }); + const data = await dataAccess.retrieveUsers({ query: reqQueryObject }); + return res.json({ message: "Users returned successfully!", - users, + users: data.users, + links: { + next: data.nextId ? getPaginationLink(reqQueryObject, "next", data.nextId) : "", + prev: data.prevId ? getPaginationLink(reqQueryObject, "prev", data.prevId) : "", + }, }); - } - - const data = await dataAccess.retrieveUsers({ query: req.query }); + } catch (e) { + if (e instanceof NotFound) { + return res.boom.notFound(e.message); + } + if (e instanceof BadRequest) { + return res.boom.BadRequest(e.message); + } - 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) : "", - }, - }); + return res.boom.serverUnavailable(SOMETHING_WENT_WRONG); + } } catch (error) { logger.error(`Error while fetching all users: ${error}`); return res.boom.serverUnavailable(SOMETHING_WENT_WRONG); diff --git a/utils/users.js b/utils/users.js index 387aadc58..15fb4063a 100644 --- a/utils/users.js +++ b/utils/users.js @@ -4,10 +4,16 @@ const { months, discordNicknameLength } = require("../constants/users"); const dataAccessLayer = require("../services/dataAccessLayer"); const discordService = require("../services/discordService"); const ROLES = require("../constants/roles"); +const dataAccess = require("../services/dataAccessLayer"); +const logger = require("./logger"); const addUserToDBForTest = async (userData) => { await userModel.add(userData); }; - +const { NotFound, BadRequest } = require("http-errors"); +const { isLastPRMergedWithinDays } = require("../services/githubService"); +const { getUserStatus } = require("../models/userStatus"); +const userService = require("../services/users"); +const { getOverdueTasks } = require("../models/tasks"); /** * Used for receiving userId when providing username * @@ -291,6 +297,149 @@ const updateNickname = async (userId, status = {}) => { } }; +/** + * @param userId { string }: Id of the User + * @returns Promise + */ +const findUserById = async (userId) => { + let result; + try { + result = await dataAccess.retrieveUsers({ id: userId }); + if (!result.userExists) { + throw NotFound("User doesn't exist"); + } + return result.user; + } catch (error) { + logger.error(`Error while fetching user: ${error}`); + throw error; + } +}; + +/** + * @param userData { Object }: req.userData + * @returns Promise + */ +const getUserByProfileData = async (userData) => { + if (!userData.id) { + throw BadRequest("User ID not provided."); + } + + try { + const result = await dataAccess.retrieveUsers({ id: userData.id }); + return result.user; + } catch (error) { + logger.error(`Error while fetching user: ${error}`); + throw error; + } +}; + +/** + * @param days {number}: days since last unmerged pr. + * @returns Promise + */ +const getUsersByUnmergedPrs = async (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 users; + } catch (error) { + logger.error(`Error while fetching all users: ${error}`); + throw error; + } +}; + +/** + * @param discordId { string }: discordId of the user + * @returns Promise + */ +const getUserByDiscordId = async (discordId) => { + let result, user; + try { + result = await dataAccess.retrieveUsers({ discordId }); + user = result.user; + if (!result.userExists) { + return 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}`); + throw error; + } + return user; +}; + +/** + * @param queryObject { Object }: request query object + * @returns Promise + */ +const getDepartedUsers = async (queryObject) => { + try { + const result = await dataAccess.retrieveUsers({ query: queryObject }); + const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); + if (departedUsers.length === 0) return []; + return { result, departedUsers }; + } catch (error) { + logger.error("Error when fetching users who abandoned tasks:", error); + throw error; + } +}; + +/** + * @param days { number }: overdue days + * @param dev {boolean}: dev feature flag + * @returns Promise + */ +const getUsersByOverDueTasks = async (days, dev) => { + try { + const tasksData = await getOverdueTasks(days); + if (!tasksData.length) { + return []; + } + 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 usersData; + } catch (error) { + logger.error(`Error while fetching users and tasks: ${error}`); + throw error; + } +}; + module.exports = { addUserToDBForTest, getUserId, @@ -307,4 +456,10 @@ module.exports = { parseSearchQuery, generateOOONickname, updateNickname, + findUserById, + getUserByProfileData, + getUsersByUnmergedPrs, + getUserByDiscordId, + getDepartedUsers, + getUsersByOverDueTasks, }; From 1eb75fc995f51223837f5169d703e0247c5fcc91 Mon Sep 17 00:00:00 2001 From: Prakhar Kumar Date: Sat, 25 Jan 2025 17:44:33 +0530 Subject: [PATCH 2/6] Moving refactored mini functions from utils to controllers folder Signed-off-by: Prakhar Kumar --- controllers/users.js | 161 +++++++++++++++++++++++++++++++++++++++++-- utils/users.js | 155 ----------------------------------------- 2 files changed, 154 insertions(+), 162 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 2b98ebc81..0bf3006b1 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -1,3 +1,8 @@ +import { isLastPRMergedWithinDays } from "../services/githubService"; +import { getUserStatus } from "../models/userStatus"; +import userService from "../services/users"; +import { getOverdueTasks } from "../models/tasks"; + const chaincodeQuery = require("../models/chaincodes"); const userQuery = require("../models/users"); const profileDiffsQuery = require("../models/profileDiffs"); @@ -23,7 +28,6 @@ const { getQualifiers } = require("../utils/helper"); const { parseSearchQuery } = require("../utils/users"); const { getFilteredPRsOrIssues } = require("../utils/pullRequests"); const { getFilteredPaginationLink } = require("../utils/userStatus"); -const userUtils = require("../utils/users"); const { USERS_PATCH_HANDLER_ACTIONS, USERS_PATCH_HANDLER_ERROR_MESSAGES, @@ -106,7 +110,7 @@ const getUsers = async (req, res) => { try { if (id) { - const user = await userUtils.findUserById(id); + const user = await findUserById(id); return res.json({ message: "User returned successfully!", user: user, @@ -114,7 +118,7 @@ const getUsers = async (req, res) => { } if (profile) { - return res.send(await userUtils.getUserByProfileData(userData)); + return res.send(await getUserByProfileData(userData)); } if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") { @@ -122,7 +126,7 @@ const getUsers = async (req, res) => { } if (filterBy === "unmerged_prs" && days) { - const users = await userUtils.getUsersByUnmergedPrs(days); + const users = await getUsersByUnmergedPrs(days); return res.json({ message: "Inactive users returned successfully!", count: users.length, @@ -132,7 +136,7 @@ const getUsers = async (req, res) => { if (discordId) { if (dev) { - const user = await userUtils.getUserByDiscordId(discordId); + const user = await getUserByDiscordId(discordId); return res.json({ message: user ? "User returned successfully!" : "User not found", user: user, @@ -146,7 +150,7 @@ const getUsers = async (req, res) => { if (!dev) { return res.boom.notFound("Route not found"); } - const { result, departedUsers } = await userUtils.getDepartedUsers(reqQueryObject); + const { result, departedUsers } = await getDepartedUsers(reqQueryObject); if (departedUsers.length === 0) return res.status(204).send(); return res.json({ message: "Users with abandoned tasks fetched successfully", @@ -159,7 +163,7 @@ const getUsers = async (req, res) => { } if (filterBy === OVERDUE_TASKS) { - const users = await userUtils.getUsersByOverDueTasks(days, dev); + const users = await getUsersByOverDueTasks(days, dev); if (!users || users.length === 0) { return res.json({ message: "No users found", @@ -1038,6 +1042,149 @@ const updateProfile = async (req, res) => { } }; +/** + * @param userId { string }: Id of the User + * @returns Promise + */ +const findUserById = async (userId) => { + let result; + try { + result = await dataAccess.retrieveUsers({ id: userId }); + if (!result.userExists) { + throw NotFound("User doesn't exist"); + } + return result.user; + } catch (error) { + logger.error(`Error while fetching user: ${error}`); + throw error; + } +}; + +/** + * @param userData { Object }: req.userData + * @returns Promise + */ +const getUserByProfileData = async (userData) => { + if (!userData.id) { + throw BadRequest("User ID not provided."); + } + + try { + const result = await dataAccess.retrieveUsers({ id: userData.id }); + return result.user; + } catch (error) { + logger.error(`Error while fetching user: ${error}`); + throw error; + } +}; + +/** + * @param days {number}: days since last unmerged pr. + * @returns Promise + */ +const getUsersByUnmergedPrs = async (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 users; + } catch (error) { + logger.error(`Error while fetching all users: ${error}`); + throw error; + } +}; + +/** + * @param discordId { string }: discordId of the user + * @returns Promise + */ +const getUserByDiscordId = async (discordId) => { + let result, user; + try { + result = await dataAccess.retrieveUsers({ discordId }); + user = result.user; + if (!result.userExists) { + return 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}`); + throw error; + } + return user; +}; + +/** + * @param queryObject { Object }: request query object + * @returns Promise + */ +const getDepartedUsers = async (queryObject) => { + try { + const result = await dataAccess.retrieveUsers({ query: queryObject }); + const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); + if (!departedUsers || departedUsers.length === 0) return { departedUsers: [] }; + return { result, departedUsers }; + } catch (error) { + logger.error("Error when fetching users who abandoned tasks:", error); + throw error; + } +}; + +/** + * @param days { number }: overdue days + * @param dev {boolean}: dev feature flag + * @returns Promise + */ +const getUsersByOverDueTasks = async (days, dev) => { + try { + const tasksData = await getOverdueTasks(days); + if (!tasksData.length) { + return []; + } + 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 usersData; + } catch (error) { + logger.error(`Error while fetching users and tasks: ${error}`); + throw error; + } +}; + module.exports = { verifyUser, generateChaincode, diff --git a/utils/users.js b/utils/users.js index 15fb4063a..2a183eab1 100644 --- a/utils/users.js +++ b/utils/users.js @@ -4,16 +4,10 @@ const { months, discordNicknameLength } = require("../constants/users"); const dataAccessLayer = require("../services/dataAccessLayer"); const discordService = require("../services/discordService"); const ROLES = require("../constants/roles"); -const dataAccess = require("../services/dataAccessLayer"); const logger = require("./logger"); const addUserToDBForTest = async (userData) => { await userModel.add(userData); }; -const { NotFound, BadRequest } = require("http-errors"); -const { isLastPRMergedWithinDays } = require("../services/githubService"); -const { getUserStatus } = require("../models/userStatus"); -const userService = require("../services/users"); -const { getOverdueTasks } = require("../models/tasks"); /** * Used for receiving userId when providing username * @@ -297,149 +291,6 @@ const updateNickname = async (userId, status = {}) => { } }; -/** - * @param userId { string }: Id of the User - * @returns Promise - */ -const findUserById = async (userId) => { - let result; - try { - result = await dataAccess.retrieveUsers({ id: userId }); - if (!result.userExists) { - throw NotFound("User doesn't exist"); - } - return result.user; - } catch (error) { - logger.error(`Error while fetching user: ${error}`); - throw error; - } -}; - -/** - * @param userData { Object }: req.userData - * @returns Promise - */ -const getUserByProfileData = async (userData) => { - if (!userData.id) { - throw BadRequest("User ID not provided."); - } - - try { - const result = await dataAccess.retrieveUsers({ id: userData.id }); - return result.user; - } catch (error) { - logger.error(`Error while fetching user: ${error}`); - throw error; - } -}; - -/** - * @param days {number}: days since last unmerged pr. - * @returns Promise - */ -const getUsersByUnmergedPrs = async (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 users; - } catch (error) { - logger.error(`Error while fetching all users: ${error}`); - throw error; - } -}; - -/** - * @param discordId { string }: discordId of the user - * @returns Promise - */ -const getUserByDiscordId = async (discordId) => { - let result, user; - try { - result = await dataAccess.retrieveUsers({ discordId }); - user = result.user; - if (!result.userExists) { - return 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}`); - throw error; - } - return user; -}; - -/** - * @param queryObject { Object }: request query object - * @returns Promise - */ -const getDepartedUsers = async (queryObject) => { - try { - const result = await dataAccess.retrieveUsers({ query: queryObject }); - const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); - if (departedUsers.length === 0) return []; - return { result, departedUsers }; - } catch (error) { - logger.error("Error when fetching users who abandoned tasks:", error); - throw error; - } -}; - -/** - * @param days { number }: overdue days - * @param dev {boolean}: dev feature flag - * @returns Promise - */ -const getUsersByOverDueTasks = async (days, dev) => { - try { - const tasksData = await getOverdueTasks(days); - if (!tasksData.length) { - return []; - } - 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 usersData; - } catch (error) { - logger.error(`Error while fetching users and tasks: ${error}`); - throw error; - } -}; - module.exports = { addUserToDBForTest, getUserId, @@ -456,10 +307,4 @@ module.exports = { parseSearchQuery, generateOOONickname, updateNickname, - findUserById, - getUserByProfileData, - getUsersByUnmergedPrs, - getUserByDiscordId, - getDepartedUsers, - getUsersByOverDueTasks, }; From cf511b3e3c44c087b0cd0ce299395a7efcc96946 Mon Sep 17 00:00:00 2001 From: Prakhar Kumar Date: Sun, 26 Jan 2025 11:07:04 +0530 Subject: [PATCH 3/6] Updating user integration test after refactoring Signed-off-by: Prakhar Kumar --- test/integration/users.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 8ba547d3f..40f94594a 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -1488,12 +1488,12 @@ describe("Users", function () { }); it("should handle errors gracefully if getUsersWithIncompleteTasks fails", async function () { - Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(INTERNAL_SERVER_ERROR)); + Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(SOMETHING_WENT_WRONG)); const res = await chai.request(app).get("/users?departed=true&dev=true"); - expect(res).to.have.status(500); - expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR); + expect(res).to.have.status(503); + expect(res.body.message).to.be.equal(SOMETHING_WENT_WRONG); }); }); From 220e5336742cf1f763f7cfc2ab7f003cd81248e8 Mon Sep 17 00:00:00 2001 From: Prakhar Kumar Date: Mon, 27 Jan 2025 20:32:12 +0530 Subject: [PATCH 4/6] 1. Refactored query parameter based functions to services/users file for reusability. 2. Updated the integration tests after refactoring of code. Signed-off-by: Prakhar Kumar --- controllers/users.js | 309 +++++++++------------------------ services/users.js | 156 +++++++++++++++++ test/integration/users.test.js | 4 +- 3 files changed, 236 insertions(+), 233 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 0bf3006b1..e27eaae37 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -1,8 +1,3 @@ -import { isLastPRMergedWithinDays } from "../services/githubService"; -import { getUserStatus } from "../models/userStatus"; -import userService from "../services/users"; -import { getOverdueTasks } from "../models/tasks"; - const chaincodeQuery = require("../models/chaincodes"); const userQuery = require("../models/users"); const profileDiffsQuery = require("../models/profileDiffs"); @@ -39,6 +34,7 @@ const { generateUniqueUsername } = require("../services/users"); const { NotFound, BadRequest } = 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; @@ -91,7 +87,6 @@ const getUserById = async (req, res) => { const getUsers = async (req, res) => { try { - // getting user details by id if present. const reqQueryObject = req.query; const { q, dev: devParam, query, departed, id, profile: profileParam, discordId } = reqQueryObject; const userData = req.userData || {}; @@ -108,107 +103,102 @@ const getUsers = async (req, res) => { return res.boom.notFound("Route not found"); } - try { - if (id) { - const user = await findUserById(id); - return res.json({ - message: "User returned successfully!", - user: user, - }); - } - - if (profile) { - return res.send(await getUserByProfileData(userData)); - } + if (id) { + const user = await userService.findUserById(id); + return res.json({ + message: "User returned successfully!", + user: user, + }); + } - if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") { - return res.boom.badRequest(`Days is required for filterBy ${transformedQuery?.filterBy}`); - } + if (profile) { + return res.send(await userService.getUserByProfileData(userData)); + } - if (filterBy === "unmerged_prs" && days) { - const users = await getUsersByUnmergedPrs(days); - return res.json({ - message: "Inactive users returned successfully!", - count: users.length, - users: users, - }); - } + if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") { + return res.boom.badRequest(`Days is required for filterBy ${transformedQuery?.filterBy}`); + } - if (discordId) { - if (dev) { - const user = await getUserByDiscordId(discordId); - return res.json({ - message: user ? "User returned successfully!" : "User not found", - user: user, - }); - } else { - return res.boom.notFound("Route not found"); - } - } + if (filterBy === "unmerged_prs" && days) { + const users = await userService.getUsersByUnmergedPrs(days); + return res.json({ + message: "Inactive users returned successfully!", + count: users.length, + users: users, + }); + } - if (isDeparted) { - if (!dev) { - return res.boom.notFound("Route not found"); - } - const { result, departedUsers } = await getDepartedUsers(reqQueryObject); - if (departedUsers.length === 0) return res.status(204).send(); + if (discordId) { + if (dev) { + const user = await userService.getUserByDiscordId(discordId); 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) : "", - }, + message: user ? "User returned successfully!" : "User not found", + user: user, }); + } else { + return res.boom.notFound("Route not found"); } + } - if (filterBy === OVERDUE_TASKS) { - const users = await getUsersByOverDueTasks(days, dev); - if (!users || users.length === 0) { - return res.json({ - message: "No users found", - users: [], - }); - } - return res.json({ - message: "Users returned successfully!", - count: users.length, - users: users, - }); + if (isDeparted) { + if (!dev) { + return res.boom.notFound("Route not found"); } + 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 (qualifiers?.filterBy) { - const allPRs = await getFilteredPRsOrIssues(qualifiers); - const usernames = getUsernamesFromPRs(allPRs); - const users = await dataAccess.retrieveUsers({ usernames: usernames }); + if (filterBy === OVERDUE_TASKS) { + const users = await userService.getUsersByOverDueTasks(days, dev); + if (!users || users.length === 0) { return res.json({ - message: "Users returned successfully!", - users, + message: "No users found", + users: [], }); } + return res.json({ + message: "Users returned successfully!", + count: users.length, + users: users, + }); + } - const data = await dataAccess.retrieveUsers({ query: reqQueryObject }); - + if (qualifiers?.filterBy) { + const allPRs = await getFilteredPRsOrIssues(qualifiers); + const usernames = getUsernamesFromPRs(allPRs); + const users = await dataAccess.retrieveUsers({ usernames: usernames }); return res.json({ message: "Users returned successfully!", - users: data.users, - links: { - next: data.nextId ? getPaginationLink(reqQueryObject, "next", data.nextId) : "", - prev: data.prevId ? getPaginationLink(reqQueryObject, "prev", data.prevId) : "", - }, + users, }); - } catch (e) { - if (e instanceof NotFound) { - return res.boom.notFound(e.message); - } - if (e instanceof BadRequest) { - return res.boom.BadRequest(e.message); - } + } + + const data = await dataAccess.retrieveUsers({ query: reqQueryObject }); - return res.boom.serverUnavailable(SOMETHING_WENT_WRONG); + return res.json({ + message: "Users returned successfully!", + users: data.users, + links: { + next: data.nextId ? getPaginationLink(reqQueryObject, "next", data.nextId) : "", + prev: data.prevId ? getPaginationLink(reqQueryObject, "prev", data.prevId) : "", + }, + }); + } catch (e) { + if (e instanceof NotFound) { + return res.boom.notFound(e.message); } - } catch (error) { - logger.error(`Error while fetching all users: ${error}`); + if (e instanceof BadRequest) { + return res.boom.BadRequest(e.message); + } + return res.boom.serverUnavailable(SOMETHING_WENT_WRONG); } }; @@ -1042,149 +1032,6 @@ const updateProfile = async (req, res) => { } }; -/** - * @param userId { string }: Id of the User - * @returns Promise - */ -const findUserById = async (userId) => { - let result; - try { - result = await dataAccess.retrieveUsers({ id: userId }); - if (!result.userExists) { - throw NotFound("User doesn't exist"); - } - return result.user; - } catch (error) { - logger.error(`Error while fetching user: ${error}`); - throw error; - } -}; - -/** - * @param userData { Object }: req.userData - * @returns Promise - */ -const getUserByProfileData = async (userData) => { - if (!userData.id) { - throw BadRequest("User ID not provided."); - } - - try { - const result = await dataAccess.retrieveUsers({ id: userData.id }); - return result.user; - } catch (error) { - logger.error(`Error while fetching user: ${error}`); - throw error; - } -}; - -/** - * @param days {number}: days since last unmerged pr. - * @returns Promise - */ -const getUsersByUnmergedPrs = async (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 users; - } catch (error) { - logger.error(`Error while fetching all users: ${error}`); - throw error; - } -}; - -/** - * @param discordId { string }: discordId of the user - * @returns Promise - */ -const getUserByDiscordId = async (discordId) => { - let result, user; - try { - result = await dataAccess.retrieveUsers({ discordId }); - user = result.user; - if (!result.userExists) { - return 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}`); - throw error; - } - return user; -}; - -/** - * @param queryObject { Object }: request query object - * @returns Promise - */ -const getDepartedUsers = async (queryObject) => { - try { - const result = await dataAccess.retrieveUsers({ query: queryObject }); - const departedUsers = await userService.getUsersWithIncompleteTasks(result.users); - if (!departedUsers || departedUsers.length === 0) return { departedUsers: [] }; - return { result, departedUsers }; - } catch (error) { - logger.error("Error when fetching users who abandoned tasks:", error); - throw error; - } -}; - -/** - * @param days { number }: overdue days - * @param dev {boolean}: dev feature flag - * @returns Promise - */ -const getUsersByOverDueTasks = async (days, dev) => { - try { - const tasksData = await getOverdueTasks(days); - if (!tasksData.length) { - return []; - } - 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 usersData; - } catch (error) { - logger.error(`Error while fetching users and tasks: ${error}`); - throw error; - } -}; - module.exports = { verifyUser, generateChaincode, diff --git a/services/users.js b/services/users.js index 94646d896..5ea548b2a 100644 --- a/services/users.js +++ b/services/users.js @@ -2,6 +2,13 @@ const firestore = require("../utils/firestore"); const { formatUsername } = require("../utils/username"); const userModel = firestore.collection("users"); const tasksModel = require("../models/tasks"); +const dataAccess = require("./dataAccessLayer"); +const { NotFound, BadRequest } = require("http-errors"); +const logger = require("../utils/logger"); +const ROLES = require("../constants/roles"); +const { isLastPRMergedWithinDays } = require("./githubService"); +const { getUserStatus } = require("../models/userStatus"); +const { getOverdueTasks } = require("../models/tasks"); const getUsersWithIncompleteTasks = async (users) => { if (users.length === 0) return []; @@ -46,7 +53,156 @@ const generateUniqueUsername = async (firstName, lastName) => { } }; +/** + * @param userId { string }: Id of the User + * @returns Promise + */ +const findUserById = async (userId) => { + let result; + try { + result = await dataAccess.retrieveUsers({ id: userId }); + if (!result.userExists) { + throw NotFound("User doesn't exist"); + } + return result.user; + } catch (error) { + logger.error(`Error while fetching user: ${error}`); + throw error; + } +}; + +/** + * @param userData { Object }: req.userData + * @returns Promise + */ +const getUserByProfileData = async (userData) => { + if (!userData.id) { + throw BadRequest("User ID not provided."); + } + + try { + const result = await dataAccess.retrieveUsers({ id: userData.id }); + return result.user; + } catch (error) { + logger.error(`Error while fetching user: ${error}`); + throw error; + } +}; + +/** + * @param days {number}: days since last unmerged pr. + * @returns Promise + */ +const getUsersByUnmergedPrs = async (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 users; + } catch (error) { + logger.error(`Error while fetching all users: ${error}`); + throw error; + } +}; + +/** + * @param discordId { string }: discordId of the user + * @returns Promise + */ +const getUserByDiscordId = async (discordId) => { + let result, user; + try { + result = await dataAccess.retrieveUsers({ discordId }); + user = result.user; + if (!result.userExists) { + return 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}`); + throw error; + } + return user; +}; + +/** + * @param queryObject { Object }: request query object + * @returns Promise + */ +const getDepartedUsers = async (queryObject) => { + try { + const result = await dataAccess.retrieveUsers({ query: queryObject }); + const departedUsers = await getUsersWithIncompleteTasks(result.users); + if (!departedUsers || departedUsers.length === 0) return { departedUsers: [] }; + return { result, departedUsers }; + } catch (error) { + logger.error("Error when fetching users who abandoned tasks:", error); + throw error; + } +}; + +/** + * @param days { number }: overdue days + * @param dev {boolean}: dev feature flag + * @returns Promise + */ +const getUsersByOverDueTasks = async (days, dev) => { + try { + const tasksData = await getOverdueTasks(days); + if (!tasksData.length) { + return []; + } + 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 usersData; + } catch (error) { + logger.error(`Error while fetching users and tasks: ${error}`); + throw error; + } +}; + module.exports = { generateUniqueUsername, getUsersWithIncompleteTasks, + getUsersByOverDueTasks, + getDepartedUsers, + getUserByProfileData, + getUsersByUnmergedPrs, + getUserByDiscordId, + findUserById, }; diff --git a/test/integration/users.test.js b/test/integration/users.test.js index 40f94594a..a788ac2ff 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -47,7 +47,7 @@ const { usersData: abandonedUsersData, tasksData: abandonedTasksData, } = require("../fixtures/abandoned-tasks/departed-users"); -const userService = require("../../services/users"); +const dataAccess = require("../../services/dataAccessLayer"); chai.use(chaiHttp); describe("Users", function () { @@ -1488,7 +1488,7 @@ describe("Users", function () { }); it("should handle errors gracefully if getUsersWithIncompleteTasks fails", async function () { - Sinon.stub(userService, "getUsersWithIncompleteTasks").rejects(new Error(SOMETHING_WENT_WRONG)); + Sinon.stub(dataAccess, "retrieveUsers").throws(new Error(SOMETHING_WENT_WRONG)); const res = await chai.request(app).get("/users?departed=true&dev=true"); From 3b9d7bda0ef50ba0f9930a000accc1661c121757 Mon Sep 17 00:00:00 2001 From: Prakhar Kumar Date: Wed, 5 Feb 2025 20:44:46 +0530 Subject: [PATCH 5/6] Returning the original status code and message for departed users queryparam Signed-off-by: Prakhar Kumar --- controllers/users.js | 5 ++++- services/users.js | 5 +++-- test/integration/users.test.js | 6 +++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index e27eaae37..044ae24fc 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -31,7 +31,7 @@ const { const { addLog } = require("../models/logs"); const config = require("config"); const { generateUniqueUsername } = require("../services/users"); -const { NotFound, BadRequest } = require("http-errors"); +const { NotFound, BadRequest, InternalServerError } = require("http-errors"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); const usersCollection = firestore.collection("users"); const userService = require("../services/users"); @@ -198,6 +198,9 @@ const getUsers = async (req, res) => { 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); } diff --git a/services/users.js b/services/users.js index 5ea548b2a..049c92a1b 100644 --- a/services/users.js +++ b/services/users.js @@ -3,12 +3,13 @@ const { formatUsername } = require("../utils/username"); const userModel = firestore.collection("users"); const tasksModel = require("../models/tasks"); const dataAccess = require("./dataAccessLayer"); -const { NotFound, BadRequest } = require("http-errors"); +const { NotFound, BadRequest, InternalServerError } = require("http-errors"); const logger = require("../utils/logger"); const ROLES = require("../constants/roles"); const { isLastPRMergedWithinDays } = require("./githubService"); const { getUserStatus } = require("../models/userStatus"); const { getOverdueTasks } = require("../models/tasks"); +const { INTERNAL_SERVER_ERROR } = require("../constants/errorMessages"); const getUsersWithIncompleteTasks = async (users) => { if (users.length === 0) return []; @@ -149,7 +150,7 @@ const getDepartedUsers = async (queryObject) => { return { result, departedUsers }; } catch (error) { logger.error("Error when fetching users who abandoned tasks:", error); - throw error; + throw new InternalServerError(INTERNAL_SERVER_ERROR); } }; diff --git a/test/integration/users.test.js b/test/integration/users.test.js index a788ac2ff..f7d7f7a36 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -1488,12 +1488,12 @@ describe("Users", function () { }); it("should handle errors gracefully if getUsersWithIncompleteTasks fails", async function () { - Sinon.stub(dataAccess, "retrieveUsers").throws(new Error(SOMETHING_WENT_WRONG)); + Sinon.stub(dataAccess, "retrieveUsers").throws(new Error(INTERNAL_SERVER_ERROR)); const res = await chai.request(app).get("/users?departed=true&dev=true"); - expect(res).to.have.status(503); - expect(res.body.message).to.be.equal(SOMETHING_WENT_WRONG); + expect(res).to.have.status(500); + expect(res.body.message).to.be.equal(INTERNAL_SERVER_ERROR); }); }); From 5d92aca99a2143219330404043976283dac60b66 Mon Sep 17 00:00:00 2001 From: Prakhar Kumar Date: Wed, 12 Feb 2025 20:59:07 +0530 Subject: [PATCH 6/6] 1. Adding explicit status code. 2. Using const instead of let for direct return values. Signed-off-by: Prakhar Kumar --- controllers/users.js | 38 +++++++++++++++++++------------------- services/users.js | 10 ++++------ 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/controllers/users.js b/controllers/users.js index 044ae24fc..f873a7e3a 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -105,14 +105,15 @@ const getUsers = async (req, res) => { if (id) { const user = await userService.findUserById(id); - return res.json({ + return res.status(200).json({ message: "User returned successfully!", user: user, }); } if (profile) { - return res.send(await userService.getUserByProfileData(userData)); + const user = await userService.getUserByProfileData(userData); + return res.status(200).send(user); } if (!transformedQuery?.days && transformedQuery?.filterBy === "unmerged_prs") { @@ -121,7 +122,7 @@ const getUsers = async (req, res) => { if (filterBy === "unmerged_prs" && days) { const users = await userService.getUsersByUnmergedPrs(days); - return res.json({ + return res.status(200).json({ message: "Inactive users returned successfully!", count: users.length, users: users, @@ -131,7 +132,7 @@ const getUsers = async (req, res) => { if (discordId) { if (dev) { const user = await userService.getUserByDiscordId(discordId); - return res.json({ + return res.status(200).json({ message: user ? "User returned successfully!" : "User not found", user: user, }); @@ -146,7 +147,7 @@ const getUsers = async (req, res) => { } const { result, departedUsers } = await userService.getDepartedUsers(reqQueryObject); if (departedUsers.length === 0) return res.status(204).send(); - return res.json({ + return res.status(200).json({ message: "Users with abandoned tasks fetched successfully", users: departedUsers, links: { @@ -159,12 +160,12 @@ const getUsers = async (req, res) => { if (filterBy === OVERDUE_TASKS) { const users = await userService.getUsersByOverDueTasks(days, dev); if (!users || users.length === 0) { - return res.json({ + return res.status(200).json({ message: "No users found", users: [], }); } - return res.json({ + return res.status(200).json({ message: "Users returned successfully!", count: users.length, users: users, @@ -175,7 +176,7 @@ const getUsers = async (req, res) => { 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, }); @@ -183,7 +184,7 @@ const getUsers = async (req, res) => { const data = await dataAccess.retrieveUsers({ query: reqQueryObject }); - return res.json({ + return res.status(200).json({ message: "Users returned successfully!", users: data.users, links: { @@ -192,17 +193,16 @@ const getUsers = async (req, res) => { }, }); } catch (e) { - if (e instanceof NotFound) { - return res.boom.notFound(e.message); - } - if (e instanceof BadRequest) { - return res.boom.BadRequest(e.message); + 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); } - if (e instanceof InternalServerError) { - return res.boom.internal(e.message); - } - - return res.boom.serverUnavailable(SOMETHING_WENT_WRONG); } }; diff --git a/services/users.js b/services/users.js index 049c92a1b..51e5053ca 100644 --- a/services/users.js +++ b/services/users.js @@ -59,9 +59,8 @@ const generateUniqueUsername = async (firstName, lastName) => { * @returns Promise */ const findUserById = async (userId) => { - let result; try { - result = await dataAccess.retrieveUsers({ id: userId }); + const result = await dataAccess.retrieveUsers({ id: userId }); if (!result.userExists) { throw NotFound("User doesn't exist"); } @@ -119,10 +118,9 @@ const getUsersByUnmergedPrs = async (days) => { * @returns Promise */ const getUserByDiscordId = async (discordId) => { - let result, user; try { - result = await dataAccess.retrieveUsers({ discordId }); - user = result.user; + const result = await dataAccess.retrieveUsers({ discordId }); + const user = result.user; if (!result.userExists) { return null; } @@ -131,11 +129,11 @@ const getUserByDiscordId = async (discordId) => { if (userStatusResult.userStatusExists) { user.state = userStatusResult.data.currentStatus.state; } + return user; } catch (error) { logger.error(`Error while fetching user: ${error}`); throw error; } - return user; }; /**