Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
5 changes: 5 additions & 0 deletions constants/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ export const REQUEST_LOG_TYPE = {
REQUEST_CANCELLED: "REQUEST_CANCELLED",
REQUEST_UPDATED: "REQUEST_UPDATED",
PENDING_REQUEST_FOUND: "PENDING_REQUEST_FOUND",
REQUEST_ALREADY_APPROVED: "REQUEST_ALREADY_APPROVED",
REQUEST_ALREADY_REJECTED: "REQUEST_ALREADY_REJECTED",
//comment for testing purposes
};

export const REQUEST_CREATED_SUCCESSFULLY = "Request created successfully";
Expand All @@ -39,7 +42,9 @@ export const REQUEST_ALREADY_REJECTED = "Request already rejected";
export const ERROR_WHILE_FETCHING_REQUEST = "Error while fetching request";
export const ERROR_WHILE_CREATING_REQUEST = "Error while creating request";
export const ERROR_WHILE_UPDATING_REQUEST = "Error while updating request";
export const ERROR_WHILE_ACKNOWLEDGING_REQUEST = "Error while acknowledging request";

export const REQUEST_ID_REQUIRED = "Request id is required";
export const REQUEST_DOES_NOT_EXIST = "Request does not exist";
export const REQUEST_ALREADY_PENDING = "Request already exists please wait for approval or rejection";
export const UNAUTHORIZED_TO_CREATE_OOO_REQUEST = "Unauthorized to create OOO request";
Expand Down
50 changes: 49 additions & 1 deletion controllers/oooRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
REQUEST_ALREADY_PENDING,
USER_STATUS_NOT_FOUND,
OOO_STATUS_ALREADY_EXIST,
UNAUTHORIZED_TO_UPDATE_REQUEST,
ERROR_WHILE_ACKNOWLEDGING_REQUEST,
REQUEST_ID_REQUIRED,
} from "../constants/requests";
import { statusState } from "../constants/userStatus";
import { logType } from "../constants/logs";
Expand All @@ -20,9 +23,11 @@ import { getRequestByKeyValues, getRequests, updateRequest } from "../models/req
import { createUserFutureStatus } from "../models/userFutureStatus";
import { getUserStatus, addFutureStatus } from "../models/userStatus";
import { createOooRequest, validateUserStatus } from "../services/oooRequest";
import * as oooRequestService from "../services/oooRequest";
import { CustomResponse } from "../typeDefinitions/global";
import { OooRequestCreateRequest, OooRequestResponse, OooStatusRequest } from "../types/oooRequest";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse, OooStatusRequest } from "../types/oooRequest";
import { UpdateRequest } from "../types/requests";
import { NextFunction } from "express";

/**
* Controller to handle the creation of OOO requests.
Expand Down Expand Up @@ -148,3 +153,46 @@ export const updateOooRequestController = async (req: UpdateRequest, res: Custom
return res.boom.badImplementation(ERROR_WHILE_UPDATING_REQUEST);
}
};

/**
* Acknowledges an Out-of-Office (OOO) request
*
* @param {AcknowledgeOooRequest} req - The request object.
* @param {OooRequestResponse} res - The response object.
* @returns {Promise<OooRequestResponse>} Resolves with success or failure.
*/
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
)
: Promise<OooRequestResponse> => {
try {
const dev = req.query.dev === "true";
if(!dev) return res.boom.notImplemented("Feature not implemented");

const isSuperuser = req.userData?.roles?.super_user;
if (!isSuperuser) {
return res.boom.forbidden(UNAUTHORIZED_TO_UPDATE_REQUEST);
}

const requestBody = req.body;
const superUserId = req.userData.id;
const requestId = req.params.id;

if (!requestId) {
return res.boom.badRequest(REQUEST_ID_REQUIRED);
}

const response = await oooRequestService.acknowledgeOooRequest(requestId, requestBody, superUserId);

return res.status(200).json({
message: response.message,
});
}
catch(error){
logger.error(ERROR_WHILE_ACKNOWLEDGING_REQUEST, error);
next(error);
return res;
}
};
13 changes: 8 additions & 5 deletions controllers/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {
} from "../constants/requests";
import { getRequests } from "../models/requests";
import { getPaginatedLink } from "../utils/helper";
import { createOooRequestController, updateOooRequestController } from "./oooRequests";
import { OooRequestCreateRequest, OooRequestResponse } from "../types/oooRequest";
import { acknowledgeOooRequest, createOooRequestController, updateOooRequestController } from "./oooRequests";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../types/oooRequest";
import { CustomResponse } from "../typeDefinitions/global";
import { ExtensionRequestRequest, ExtensionRequestResponse } from "../types/extensionRequests";
import { createTaskExtensionRequest, updateTaskExtensionRequest } from "./extensionRequestsv2";
Expand All @@ -16,8 +16,7 @@ import { createTaskRequestController } from "./taskRequestsv2";
import { OnboardingExtensionCreateRequest, OnboardingExtensionResponse, UpdateOnboardingExtensionStateRequest } from "../types/onboardingExtension";
import { createOnboardingExtensionRequestController, updateOnboardingExtensionRequestController, updateOnboardingExtensionRequestState } from "./onboardingExtension";
import { UpdateOnboardingExtensionRequest } from "../types/onboardingExtension";

import { Request } from "express";
import { NextFunction, Request } from "express";

export const createRequestController = async (
req: OooRequestCreateRequest | ExtensionRequestRequest | TaskRequestRequest | OnboardingExtensionCreateRequest,
Expand Down Expand Up @@ -121,9 +120,13 @@ export const getRequestsController = async (req: any, res: any) => {
* @param {CustomResponse} res - The response object.
* @returns {Promise<void>} Resolves or sends an error for invalid types.
*/
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse) => {
export const updateRequestBeforeAcknowledgedController = async (req: Request, res: CustomResponse, next: NextFunction) => {
const type = req.body.type;

switch(type){
case REQUEST_TYPE.OOO:
await acknowledgeOooRequest(req as AcknowledgeOooRequest, res as OooRequestResponse, next);
break;
case REQUEST_TYPE.ONBOARDING:
await updateOnboardingExtensionRequestController(req as UpdateOnboardingExtensionRequest, res as OnboardingExtensionResponse);
break;
Expand Down
45 changes: 44 additions & 1 deletion middlewares/validators/oooRequests.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";

export const createOooStatusRequestValidator = async (
req: OooRequestCreateRequest,
Expand Down Expand Up @@ -38,3 +38,46 @@ export const createOooStatusRequestValidator = async (

await schema.validateAsync(req.body, { abortEarly: false });
};

const schema = joi
.object()
.strict()
.keys({
comment: joi.string().optional()
.messages({
"string.empty": "comment cannot be empty",
}),
status: joi
.string()
.valid(REQUEST_STATE.APPROVED, REQUEST_STATE.REJECTED)
.required()
.messages({
"any.only": "status must be APPROVED or REJECTED",
}),
type: joi.string().equal(REQUEST_TYPE.OOO).required().messages({
"type.any": "type is required",
})
});

/**
* Middleware to validate the acknowledge Out-Of-Office (OOO) request payload.
*
* @param {AcknowledgeOooRequest} req - The request object containing the body to be validated.
* @param {OooRequestResponse} res - The response object used to send error responses if validation fails.
* @param {NextFunction} next - The next middleware function to call if validation succeeds.
* @returns {Promise<void>} Resolves or sends errors.
*/
export const acknowledgeOooRequest = async (
req: AcknowledgeOooRequest,
res: OooRequestResponse,
next: NextFunction
): Promise<void> => {
try {
await schema.validateAsync(req.body, { abortEarly: false });
next();
} catch (error) {
const errorMessages = error.details.map((detail:{message: string}) => detail.message);
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider using Joi's built-in types for better type safety

Instead of inline type annotation, consider using Joi's ValidationErrorItem type.

-    const errorMessages = error.details.map((detail:{message: string}) => detail.message);
+    const errorMessages = error.details.map((detail) => detail.message);

Note: The error from Joi already has proper typing when caught from validateAsync.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const errorMessages = error.details.map((detail:{message: string}) => detail.message);
const errorMessages = error.details.map((detail) => detail.message);
🤖 Prompt for AI Agents
In middlewares/validators/oooRequests.ts at line 79, replace the inline type
annotation for the map callback parameter with Joi's built-in
ValidationErrorItem type to improve type safety. Import ValidationErrorItem from
Joi and use it to type the detail parameter in the map function. This leverages
Joi's existing typings and ensures consistency and correctness.

logger.error(`Error while validating request payload : ${errorMessages}`);
return res.boom.badRequest(errorMessages);
}
};
16 changes: 11 additions & 5 deletions middlewares/validators/requests.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import joi from "joi";
import { NextFunction } from "express";
import { REQUEST_STATE, REQUEST_TYPE } from "../../constants/requests";
import { OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
import { createOooStatusRequestValidator } from "./oooRequests";
import { AcknowledgeOooRequest, OooRequestCreateRequest, OooRequestResponse } from "../../types/oooRequest";
import { acknowledgeOooRequest, createOooStatusRequestValidator } from "./oooRequests";
import { createExtensionRequestValidator } from "./extensionRequestsv2";
import {createTaskRequestValidator} from "./taskRequests";
import { ExtensionRequestRequest, ExtensionRequestResponse } from "../../types/extensionRequests";
Expand Down Expand Up @@ -125,18 +125,24 @@ export const getRequestsMiddleware = async (req: OooRequestCreateRequest, res: O
/**
* Validates update requests based on their type.
*
* @param {UpdateOnboardingExtensionRequest} req - Request object.
* @param {UpdateOnboardingExtensionRequest | AcknowledgeOooRequest} req - Request object.
* @param {CustomResponse} res - Response object.
* @param {NextFunction} next - Next middleware if valid.
* @returns {Promise<void>} Resolves or sends errors.
*/
export const updateRequestValidator = async (
req: UpdateOnboardingExtensionRequest,
req: UpdateOnboardingExtensionRequest | AcknowledgeOooRequest,
res: CustomResponse,
next: NextFunction
): Promise<void> => {
const type = req.body.type;

switch (type) {
case REQUEST_TYPE.OOO:
await acknowledgeOooRequest(
req,
res as OooRequestResponse, next);
break;
case REQUEST_TYPE.ONBOARDING:
await updateOnboardingExtensionRequestValidator(
req,
Expand All @@ -145,4 +151,4 @@ export const updateRequestValidator = async (
default:
return res.boom.badRequest("Invalid type");
}
};
};
82 changes: 75 additions & 7 deletions models/requests.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import firestore from "../utils/firestore";
const requestModel = firestore.collection("requests");
import { REQUEST_ALREADY_APPROVED, REQUEST_ALREADY_REJECTED, REQUEST_STATE } from "../constants/requests";
import { REQUEST_ALREADY_APPROVED, REQUEST_ALREADY_REJECTED, REQUEST_STATE, REQUEST_TYPE } from "../constants/requests";
import {
ERROR_WHILE_FETCHING_REQUEST,
ERROR_WHILE_CREATING_REQUEST,
ERROR_WHILE_UPDATING_REQUEST,
REQUEST_DOES_NOT_EXIST,
} from "../constants/requests";
import { getUserId } from "../utils/users";
import { NotFound } from "http-errors";
import { fetchUser } from "./users";
const SIZE = 5;

export const createRequest = async (body: any) => {
Expand All @@ -29,7 +31,7 @@ export const createRequest = async (body: any) => {
}
};

export const updateRequest = async (id: string, body: any, lastModifiedBy: string, type:string) => {
export const updateRequest = async (id: string, body: any, lastModifiedBy: string, type: string) => {
try {
const existingRequestDoc = await requestModel.doc(id).get();
if (!existingRequestDoc.exists) {
Expand Down Expand Up @@ -69,6 +71,21 @@ export const updateRequest = async (id: string, body: any, lastModifiedBy: strin
}
};

export const getRequestById = async (id: string) => {
try {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please check with @Suvidh-kaushik what function he used for the impersonation feature to get request?

Copy link
Contributor

Choose a reason for hiding this comment

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

fetch by id was handled in a seperate model function

const requestDoc = await requestModel.doc(id).get();

if (!requestDoc.exists) {
throw new NotFound(REQUEST_DOES_NOT_EXIST);
}

return requestDoc.data();
} catch (error) {
logger.error(ERROR_WHILE_FETCHING_REQUEST, error);
throw error;
}
};

export const getRequests = async (query: any) => {
let { id, type, requestedBy, state, prev, next, page, size = SIZE } = query;
const dev = query.dev === "true";
Expand All @@ -88,11 +105,10 @@ export const getRequests = async (query: any) => {
...requestDoc.data(),
};
}
if(requestedBy && dev){

if (requestedBy && dev) {
requestQuery = requestQuery.where("requestedBy", "==", requestedBy);
}
else if (requestedBy) {
} else if (requestedBy) {
const requestedByUserId = await getUserId(requestedBy);
requestQuery = requestQuery.where("requestedBy", "==", requestedByUserId);
}
Expand Down Expand Up @@ -149,6 +165,59 @@ export const getRequests = async (query: any) => {
return null;
}

// todo: remove this once previous OOO requests are removed form the database
Copy link
Member

Choose a reason for hiding this comment

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

  • Could you please create an issue ticket and assign it to the appropriate person so we don't missed it?

// @ankush and @suraj had a discussion to manually update or remove the previous OOO requests
if (type === REQUEST_TYPE.OOO) {
const oooRequests = [];
if (!dev) {
for (const request of allRequests) {
if (request.status) {
const modifiedRequest = {
id: request.id,
type: request.type,
from: request.from,
until: request.until,
message: request.reason,
state: request.status,
lastModifiedBy: request.lastModifiedBy ?? "",
requestedBy: request.userId,
reason: request.comment ?? "",
createdAt: request.createdAt,
updatedAt: request.updatedAt,
};
oooRequests.push(modifiedRequest);
} else {
oooRequests.push(request);
}
}
} else {
for (const request of allRequests) {
if (request.state) {
const userResponse: any = await fetchUser({ userId: request.requestedBy });
const username = userResponse.user.username;

const modifiedRequest = {
id: request.id,
type: request.type,
from: request.from,
until: request.until,
reason: request.message,
status: request.state,
lastModifiedBy: request.lastModifiedBy ?? null,
requestedBy: username,
comment: request.reason ?? null,
createdAt: request.createdAt,
updatedAt: request.updatedAt,
userId: request.requestedBy,
};
oooRequests.push(modifiedRequest);
} else {
oooRequests.push(request);
}
}
}
allRequests = oooRequests;
Copy link
Member

Choose a reason for hiding this comment

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

Please cross-check that we are sending the requestedBy userId. As per my understanding, when we created the OOO request earlier, we were storing the username in the DB, so there might be some bad data that could cause issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

}
return {
allRequests,
prev: prevDoc.empty ? null : prevDoc.docs[0].id,
Expand Down Expand Up @@ -190,4 +259,3 @@ export const getRequestByKeyValues = async (keyValues: KeyValues) => {
throw error;
}
};

Loading
Loading